From 588bcebc877a77a4a357cf54c1459524eaca5b16 Mon Sep 17 00:00:00 2001 From: Alexey Sachkov Date: Fri, 21 Jan 2022 23:11:44 +0300 Subject: [PATCH 1/4] [NFC] Outline config creation into a separate module This is done in order to have config creation rules unified in one place to avoid modifying them multiple times if they change. We already had 3 duplicated pieces of code initializing the config and there will be more config uses in future tests. This resolves #1960 --- server.js | 6 +----- server/config.js | 5 +++++ server/googleActions.js | 6 +----- server/token.js | 6 +----- 4 files changed, 8 insertions(+), 15 deletions(-) create mode 100644 server/config.js diff --git a/server.js b/server.js index c9ce63c8f..3a9b07b51 100644 --- a/server.js +++ b/server.js @@ -1,11 +1,7 @@ const DB = require('./server/db.js'); const server = require('./server/app.js'); -const config = require('nconf') - .argv() - .env({ lowerCase: true }) - .file('environment', { file: `config/${process.env.NODE_ENV}.json` }) - .file('defaults', { file: 'config/default.json' }); +const config = require('./server/config.js'); DB.connect(config).then(()=>{ // Ensure that we have successfully connected to the database diff --git a/server/config.js b/server/config.js new file mode 100644 index 000000000..fc50e68ba --- /dev/null +++ b/server/config.js @@ -0,0 +1,5 @@ +module.exports = require('nconf') + .argv() + .env({ lowerCase: true }) + .file('environment', { file: `config/${process.env.NODE_ENV}.json` }) + .file('defaults', { file: 'config/default.json' }); diff --git a/server/googleActions.js b/server/googleActions.js index 0050cb81d..b4eeba5e3 100644 --- a/server/googleActions.js +++ b/server/googleActions.js @@ -3,11 +3,7 @@ const _ = require('lodash'); const { google } = require('googleapis'); const { nanoid } = require('nanoid'); const token = require('./token.js'); -const config = require('nconf') - .argv() - .env({ lowerCase: true }) // Load environment variables - .file('environment', { file: `config/${process.env.NODE_ENV}.json` }) - .file('defaults', { file: 'config/default.json' }); +const config = require('./config.js'); //let oAuth2Client; diff --git a/server/token.js b/server/token.js index 40d76a484..70d6e01c5 100644 --- a/server/token.js +++ b/server/token.js @@ -1,11 +1,7 @@ const jwt = require('jwt-simple'); // Load configuration values -const config = require('nconf') - .argv() - .env({ lowerCase: true }) // Load environment variables - .file('environment', { file: `config/${process.env.NODE_ENV}.json` }) - .file('defaults', { file: 'config/default.json' }); +const config = require('./config.js'); // Generate an Access Token for the given User ID const generateAccessToken = (account)=>{ From 4e0ab4b39367cfba4361d57e7fea23e66ee816ef Mon Sep 17 00:00:00 2001 From: Alexey Sachkov Date: Fri, 28 Jan 2022 21:40:40 +0300 Subject: [PATCH 2/4] Adjust paths to config files --- server/config.js | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/server/config.js b/server/config.js index fc50e68ba..a0cb1a1b0 100644 --- a/server/config.js +++ b/server/config.js @@ -1,5 +1,7 @@ +const path = require('path'); + module.exports = require('nconf') .argv() .env({ lowerCase: true }) - .file('environment', { file: `config/${process.env.NODE_ENV}.json` }) - .file('defaults', { file: 'config/default.json' }); + .file('environment', { file: path.resolve(__dirname, '../config/${process.env.NODE_ENV}.json') }) + .file('default', { file: path.resolve(__dirname, '../config/default.json') }); From d7aa4afa6015fe30307d6ea81859b698164b404a Mon Sep 17 00:00:00 2001 From: Trevor Buckner Date: Sat, 29 Jan 2022 23:53:30 -0500 Subject: [PATCH 3/4] process.chdir in app.js. Everything works, no need for path.resolve. All tests pass. --- server.js | 1 - server/app.js | 25 ++++++++++--------------- server/config.js | 6 ++---- tests/routes/static-pages.test.js | 4 +--- 4 files changed, 13 insertions(+), 23 deletions(-) diff --git a/server.js b/server.js index 3a9b07b51..cb58f9bd3 100644 --- a/server.js +++ b/server.js @@ -1,6 +1,5 @@ const DB = require('./server/db.js'); const server = require('./server/app.js'); - const config = require('./server/config.js'); DB.connect(config).then(()=>{ diff --git a/server/app.js b/server/app.js index db2eaac5f..c9ebf6a19 100644 --- a/server/app.js +++ b/server/app.js @@ -4,6 +4,7 @@ const jwt = require('jwt-simple'); const express = require('express'); const yaml = require('js-yaml'); const app = express(); +const config = require('./config.js'); const homebrewApi = require('./homebrew.api.js'); const GoogleActions = require('./googleActions.js'); @@ -13,6 +14,9 @@ const asyncHandler = require('express-async-handler'); const brewAccessTypes = ['edit', 'share', 'raw']; +// Set working directory to project root +process.chdir(`${__dirname}/..`); + //Get the brew object from the HB database or Google Drive const getBrewFromId = asyncHandler(async (id, accessType)=>{ if(!brewAccessTypes.includes(accessType)) @@ -62,22 +66,13 @@ const splitTextStyleAndMetadata = (brew)=>{ } }; -app.use('/', serveCompressedStaticAssets(`${__dirname}/../build`)); - -process.chdir(__dirname); +app.use('/', serveCompressedStaticAssets(`build`)); //app.use(express.static(`${__dirname}/build`)); app.use(require('body-parser').json({ limit: '25mb' })); app.use(require('cookie-parser')()); app.use(require('./forcessl.mw.js')); -// FIXME: the config should be passed as an argument for the app -const config = require('nconf') - .argv() - .env({ lowerCase: true }) - .file('environment', { file: `config/${process.env.NODE_ENV}.json` }) - .file('defaults', { file: 'config/default.json' }); - //Account Middleware app.use((req, res, next)=>{ if(req.cookies && req.cookies.nc_session){ @@ -99,16 +94,16 @@ app.use(homebrewApi); app.use(require('./admin.api.js')); const HomebrewModel = require('./homebrew.model.js').model; -const welcomeText = require('fs').readFileSync('./../client/homebrew/pages/homePage/welcome_msg.md', 'utf8'); -const welcomeTextV3 = require('fs').readFileSync('./../client/homebrew/pages/homePage/welcome_msg_v3.md', 'utf8'); -const changelogText = require('fs').readFileSync('./../changelog.md', 'utf8'); -const faqText = require('fs').readFileSync('./../faq.md', 'utf8'); +const welcomeText = require('fs').readFileSync('client/homebrew/pages/homePage/welcome_msg.md', 'utf8'); +const welcomeTextV3 = require('fs').readFileSync('client/homebrew/pages/homePage/welcome_msg_v3.md', 'utf8'); +const changelogText = require('fs').readFileSync('changelog.md', 'utf8'); +const faqText = require('fs').readFileSync('faq.md', 'utf8'); String.prototype.replaceAll = function(s, r){return this.split(s).join(r);}; //Robots.txt app.get('/robots.txt', (req, res)=>{ - return res.sendFile(`${__dirname}/robots.txt`); + return res.sendFile(`robots.txt`, { root: process.cwd() }); }); //Home page diff --git a/server/config.js b/server/config.js index a0cb1a1b0..fc50e68ba 100644 --- a/server/config.js +++ b/server/config.js @@ -1,7 +1,5 @@ -const path = require('path'); - module.exports = require('nconf') .argv() .env({ lowerCase: true }) - .file('environment', { file: path.resolve(__dirname, '../config/${process.env.NODE_ENV}.json') }) - .file('default', { file: path.resolve(__dirname, '../config/default.json') }); + .file('environment', { file: `config/${process.env.NODE_ENV}.json` }) + .file('defaults', { file: 'config/default.json' }); diff --git a/tests/routes/static-pages.test.js b/tests/routes/static-pages.test.js index 0d86bc307..e69706f56 100644 --- a/tests/routes/static-pages.test.js +++ b/tests/routes/static-pages.test.js @@ -21,9 +21,7 @@ describe('Tests for static pages', ()=>{ return app.get('/faq').expect(200); }); - // FIXME: robots.txt file can't be properly loaded under testing environment, - // most likely due to __dirname being different from what is expected - it.skip('robots.txt works', ()=>{ + it('robots.txt works', ()=>{ return app.get('/robots.txt').expect(200); }); }); From ccbeca2cad578353151c23eae3d16841efa818a3 Mon Sep 17 00:00:00 2001 From: Trevor Buckner Date: Sun, 30 Jan 2022 00:13:35 -0500 Subject: [PATCH 4/4] Move process.chdir up so it occurs before config.js is ever called. app.js should be required before config to make sure process.cwd is updated first --- server/app.js | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/server/app.js b/server/app.js index c9ebf6a19..2437e3b38 100644 --- a/server/app.js +++ b/server/app.js @@ -1,4 +1,7 @@ /*eslint max-lines: ["warn", {"max": 300, "skipBlankLines": true, "skipComments": true}]*/ +// Set working directory to project root +process.chdir(`${__dirname}/..`); + const _ = require('lodash'); const jwt = require('jwt-simple'); const express = require('express'); @@ -14,9 +17,6 @@ const asyncHandler = require('express-async-handler'); const brewAccessTypes = ['edit', 'share', 'raw']; -// Set working directory to project root -process.chdir(`${__dirname}/..`); - //Get the brew object from the HB database or Google Drive const getBrewFromId = asyncHandler(async (id, accessType)=>{ if(!brewAccessTypes.includes(accessType))