Better fix for #5993 (#6034)

* Better fix for #5993

Also a nice newline so the line doesn't get overwritten when running `./nodebb upgrade`

* Optimizations for custom homepage

Move it into a separate module

nodebb

@@ -466,7 +466,7 @@ var commands = {
 					next();
 				},
 				function (next) {
-					process.stdout.write('1. '.bold + 'Bringing base dependencies up to date... '.yellow);
+					process.stdout.write('1. '.bold + 'Bringing base dependencies up to date... \n'.yellow);
 					packageInstall.npmInstallProduction();
 					next();
 				},

src/controllers/home.js

@@ -0,0 +1,62 @@
+'use strict';
+
+var plugins = require('../plugins');
+var meta = require('../meta');
+var user = require('../user');
+var pubsub = require('../pubsub');
+
+var adminHomePageRoute;
+var getRoute;
+
+function configUpdated() {
+	adminHomePageRoute = (meta.config.homePageRoute || meta.config.homePageCustom || '').replace(/^\/+/, '') || 'categories';
+	getRoute = parseInt(meta.config.allowUserHomePage, 10) ? getRouteAllowUserHomePage : getRouteDisableUserHomePage;
+}
+
+function getRouteDisableUserHomePage(uid, next) {
+	next(null, adminHomePageRoute);
+}
+
+function getRouteAllowUserHomePage(uid, next) {
+	user.getSettings(uid, function (err, settings) {
+		if (err) {
+			return next(err);
+		}
+
+		var route = adminHomePageRoute;
+
+		if (settings.homePageRoute !== 'undefined' && settings.homePageRoute !== 'none') {
+			route = settings.homePageRoute || route;
+		}
+
+		next(null, route);
+	});
+}
+
+pubsub.on('config:update', configUpdated);
+configUpdated();
+
+module.exports = function (req, res, next) {
+	if (req.path !== '/' && req.path !== '/api/') {
+		return next();
+	}
+
+	getRoute(req.uid, function (err, route) {
+		if (err) {
+			return next(err);
+		}
+
+		var hook = 'action:homepage.get:' + route;
+
+		if (plugins.hasListeners(hook)) {
+			return plugins.fireHook(hook, {
+				req: req,
+				res: res,
+				next: next,
+			});
+		}
+
+		req.url = req.path + route;
+		next();
+	});
+};

src/controllers/index.js

@@ -3,7 +3,6 @@
 var async = require('async');
 var nconf = require('nconf');
 var validator = require('validator');
-var request = require('request');
 
 var meta = require('../meta');
 var user = require('../user');
@@ -13,6 +12,7 @@ var helpers = require('./helpers');
 
 var Controllers = module.exports;
 
+Controllers.home = require('./home');
 Controllers.topics = require('./topics');
 Controllers.posts = require('./posts');
 Controllers.categories = require('./categories');
@@ -36,52 +36,6 @@ Controllers.osd = require('./osd');
 Controllers['404'] = require('./404');
 Controllers.errors = require('./errors');
 
-Controllers.home = function (req, res, next) {
-	var route = meta.config.homePageRoute || (meta.config.homePageCustom || '').replace(/^\/+/, '') || 'categories';
-
-	async.waterfall([
-		function (next) {
-			user.getSettings(req.uid, next);
-		},
-		function (settings, next) {
-			if (parseInt(meta.config.allowUserHomePage, 10) === 1 && settings.homePageRoute !== 'undefined' && settings.homePageRoute !== 'none') {
-				route = settings.homePageRoute || route;
-			}
-
-			var hook = 'action:homepage.get:' + route;
-
-			if (plugins.hasListeners(hook)) {
-				return plugins.fireHook(hook, {
-					req: req,
-					res: res,
-					next: next,
-				});
-			}
-
-			if (route === 'categories' || route === '/') {
-				Controllers.categories.list(req, res, next);
-			} else if (route === 'unread') {
-				Controllers.unread.get(req, res, next);
-			} else if (route === 'recent') {
-				Controllers.recent.get(req, res, next);
-			} else if (route === 'popular') {
-				Controllers.popular.get(req, res, next);
-			} else {
-				var match = /^category\/(\d+)\/(.*)$/.exec(route);
-
-				if (match) {
-					req.params.topic_index = '1';
-					req.params.category_id = match[1];
-					req.params.slug = match[2];
-					Controllers.category.get(req, res, next);
-				} else {
-					request.get(nconf.get('url') + '/' + route).pipe(res);
-				}
-			}
-		},
-	], next);
-};
-
 Controllers.reset = function (req, res, next) {
 	if (req.params.code) {
 		async.waterfall([

src/meta/configs.js

@@ -121,11 +121,7 @@ function updateConfig(config) {
 }
 
 function updateLocalConfig(config) {
-	for (var field in config) {
-		if (config.hasOwnProperty(field)) {
-			Meta.config[field] = config[field];
-		}
-	}
+	Object.assign(Meta.config, config);
 }
 
 pubsub.on('config:update', function onConfigReceived(config) {

src/routes/index.js

@@ -22,8 +22,6 @@ var helpers = require('./helpers');
 var setupPageRoute = helpers.setupPageRoute;
 
 function mainRoutes(app, middleware, controllers) {
-	setupPageRoute(app, '/', middleware, [], controllers.home);
-
 	var loginRegisterMiddleware = [middleware.redirectToAccountIfLoggedIn];
 
 	setupPageRoute(app, '/login', middleware, loginRegisterMiddleware, controllers.login);
@@ -123,6 +121,9 @@ module.exports = function (app, middleware, hotswapIds, callback) {
 
 	app.use(middleware.stripLeadingSlashes);
 
+	// handle custom homepage routes
+	app.use(relativePath, controllers.home);
+
 	adminRoutes(router, middleware, controllers);
 	metaRoutes(router, middleware, controllers);
 	apiRoutes(router, middleware, controllers);

test/controllers.js

@@ -69,62 +69,80 @@ describe('Controllers', function () {
 	});
 
 	it('should load unread as home route', function (done) {
-		meta.config.homePageRoute = 'unread';
-		request(nconf.get('url'), function (err, res, body) {
+		meta.configs.set('homePageRoute', 'unread', function (err) {
 			assert.ifError(err);
-			assert.equal(res.statusCode, 200);
-			assert(body);
-			done();
+
+			request(nconf.get('url'), function (err, res, body) {
+				assert.ifError(err);
+				assert.equal(res.statusCode, 200);
+				assert(body);
+				done();
+			});
 		});
 	});
 
 	it('should load recent as home route', function (done) {
-		meta.config.homePageRoute = 'recent';
-		request(nconf.get('url'), function (err, res, body) {
+		meta.configs.set('homePageRoute', 'recent', function (err) {
 			assert.ifError(err);
-			assert.equal(res.statusCode, 200);
-			assert(body);
-			done();
+
+			request(nconf.get('url'), function (err, res, body) {
+				assert.ifError(err);
+				assert.equal(res.statusCode, 200);
+				assert(body);
+				done();
+			});
 		});
 	});
 
 	it('should load popular as home route', function (done) {
-		meta.config.homePageRoute = 'popular';
-		request(nconf.get('url'), function (err, res, body) {
+		meta.configs.set('homePageRoute', 'popular', function (err) {
 			assert.ifError(err);
-			assert.equal(res.statusCode, 200);
-			assert(body);
-			done();
+
+			request(nconf.get('url'), function (err, res, body) {
+				assert.ifError(err);
+				assert.equal(res.statusCode, 200);
+				assert(body);
+				done();
+			});
 		});
 	});
 
 	it('should load category as home route', function (done) {
-		meta.config.homePageRoute = 'category/1/test-category';
-		request(nconf.get('url'), function (err, res, body) {
+		meta.configs.set('homePageRoute', 'category/1/test-category', function (err) {
 			assert.ifError(err);
-			assert.equal(res.statusCode, 200);
-			assert(body);
-			done();
+
+			request(nconf.get('url'), function (err, res, body) {
+				assert.ifError(err);
+				assert.equal(res.statusCode, 200);
+				assert(body);
+				done();
+			});
 		});
 	});
 
 	it('should redirect to custom homepage', function (done) {
-		meta.config.homePageRoute = 'groups';
-		request(nconf.get('url'), function (err, res, body) {
+		meta.configs.set('homePageRoute', 'groups', function (err) {
 			assert.ifError(err);
-			assert.equal(res.statusCode, 200);
-			assert(body);
-			done();
+
+			request(nconf.get('url'), function (err, res, body) {
+				assert.ifError(err);
+				assert.equal(res.statusCode, 200);
+				assert(body);
+				done();
+			});
 		});
 	});
 
 	it('should 404 if custom homepage does not exist', function (done) {
-		meta.config.homePageRoute = 'thisroutedoesnotexist';
-		request(nconf.get('url'), function (err, res, body) {
+		meta.configs.set('homePageRoute', 'this-route-does-not-exist', function (err) {
 			assert.ifError(err);
-			assert.equal(res.statusCode, 404);
-			assert(body);
-			done();
+
+			request(nconf.get('url'), function (err, res, body) {
+				assert.ifError(err);
+				assert.equal(res.statusCode, 404);
+				assert(body);
+				done();
+			});
 		});
 	});
 
@@ -139,13 +157,16 @@ describe('Controllers', function () {
 			hook: 'action:homepage.get:custom',
 			method: hookMethod,
 		});
-		meta.config.homePageRoute = 'custom';
-		request(nconf.get('url'), function (err, res, body) {
+		meta.configs.set('homePageRoute', 'custom', function (err) {
 			assert.ifError(err);
-			assert.equal(res.statusCode, 200);
-			assert.equal(body, '"works"');
-			plugins.unregisterHook('myTestPlugin', 'action:homepage.get:custom', hookMethod);
-			done();
+
+			request(nconf.get('url'), function (err, res, body) {
+				assert.ifError(err);
+				assert.equal(res.statusCode, 200);
+				assert.equal(body, '"works"');
+				plugins.unregisterHook('myTestPlugin', 'action:homepage.get:custom', hookMethod);
+				done();
+			});
 		});
 	});