TryGhost/Ghost
1
0
Fork 0
mirror of https://github.com/TryGhost/Ghost.git synced 2024-11-28 14:03:48 +03:00
Code Issues Projects Releases Wiki Activity

Decoupled frontend routing from url service

Browse Source 
refs https://linear.app/tryghost/issue/CORE-103/decouple-internal-frontend-code-from-url-module

- By becoming a parameter in the routing bootstrap process URL is Service no longer a "require" inside the frontend controllers but rather becomes a part of the "internal API" of the bootstrapper. This is not the end form of it, rather a step closer to decouplint routing from the URL serivce.
- The bootstrap module needs a facelift to have cleaner distinction between init/start methods. This is left for another time
This commit is contained in:
Naz 2021-10-13 16:14:33 +02:00 committed by naz
parent 0a9837ebcf
commit add30f3d5b
8 changed files with 48 additions and 31 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

15
core/frontend/services/routing/bootstrap.js vendored
View File

@ -16,6 +16,7 @@ const defaultApiVersion = 'v4';
const registry = require('./registry');
let siteRouter;
let _urlService;
/**
* @description The `init` function will return the wrapped parent express router and will start creating all
@ -29,7 +30,8 @@ let siteRouter;
* @param {Object} options
* @returns {ExpressRouter}
*/
const init = ({start = false, routerSettings, apiVersion}) => {
const init = ({start = false, routerSettings, apiVersion, urlService}) => {
_urlService = urlService;
debug('bootstrap init', start, apiVersion, routerSettings);
registry.resetAllRouters();
@ -113,5 +115,16 @@ const start = (apiVersion, routerSettings) => {
debug('Routes:', registry.getAllRoutes());
};
module.exports.internal = {
owns: (routerId, id) => {
return _urlService.owns(routerId, id);
},
getUrlByResourceId: (id, options) => {
return _urlService.getUrlByResourceId(id, options);
},
getResourceById: (resourceId) => {
return _urlService.getResourceById(resourceId);
}
};
module.exports.init = init;
module.exports.start = start;

4
core/frontend/services/routing/controllers/collection.js
View File

@ -3,7 +3,7 @@ const debug = require('@tryghost/debug')('services:routing:controllers:collectio
const tpl = require('@tryghost/tpl');
const errors = require('@tryghost/errors');
const security = require('@tryghost/security');
const urlService = require('../../url');
const bootstrap = require('../bootstrap');
const themeEngine = require('../../theme-engine');
const helpers = require('../helpers');
@ -72,7 +72,7 @@ module.exports = function collectionController(req, res, next) {
* People should always invert their filters to ensure that the database query loads unique posts per collection.
*/
result.posts = _.filter(result.posts, (post) => {
if (urlService.owns(res.routerOptions.identifier, post.id)) {
if (bootstrap.internal.owns(res.routerOptions.identifier, post.id)) {
return post;
}

4
core/frontend/services/routing/controllers/email-post.js
View File

@ -1,6 +1,6 @@
const debug = require('@tryghost/debug')('services:routing:controllers:emailpost');
const config = require('../../../../shared/config');
const urlService = require('../../url');
const bootstrap = require('../bootstrap');
const urlUtils = require('../../../../shared/url-utils');
const helpers = require('../helpers');
@ -48,7 +48,7 @@ module.exports = function emailPostController(req, res, next) {
}
if (post.status === 'published') {
return urlUtils.redirect301(res, urlService.getUrlByResourceId(post.id, {withSubdirectory: true}));
return urlUtils.redirect301(res, bootstrap.internal.getUrlByResourceId(post.id, {withSubdirectory: true}));
}
if (res.locals.apiVersion !== 'v0.1' && res.locals.apiVersion !== 'v2') {

4
core/frontend/services/routing/controllers/entry.js
View File

@ -1,7 +1,7 @@
const debug = require('@tryghost/debug')('services:routing:controllers:entry');
const url = require('url');
const config = require('../../../../shared/config');
const urlService = require('../../../services/url');
const bootstrap = require('../bootstrap');
const urlUtils = require('../../../../shared/url-utils');
const helpers = require('../helpers');
@ -60,7 +60,7 @@ module.exports = function entryController(req, res, next) {
*
* That's why we have to check against the router type.
*/
if (urlService.getResourceById(entry.id).config.type !== res.routerOptions.resourceType) {
if (bootstrap.internal.getResourceById(entry.id).config.type !== res.routerOptions.resourceType) {
debug('not my resource type');
return next();
}

4
core/frontend/services/routing/controllers/preview.js
View File

@ -1,6 +1,6 @@
const debug = require('@tryghost/debug')('services:routing:controllers:preview');
const config = require('../../../../shared/config');
const urlService = require('../../url');
const bootstrap = require('../bootstrap');
const urlUtils = require('../../../../shared/url-utils');
const helpers = require('../helpers');
@ -50,7 +50,7 @@ module.exports = function previewController(req, res, next) {
}
if (post.status === 'published') {
return urlUtils.redirect301(res, urlService.getUrlByResourceId(post.id, {withSubdirectory: true}));
return urlUtils.redirect301(res, bootstrap.internal.getUrlByResourceId(post.id, {withSubdirectory: true}));
}
if (res.locals.apiVersion !== 'v0.1' && res.locals.apiVersion !== 'v2') {

3
core/server/web/site/routes.js
View File

@ -1,9 +1,12 @@
const debug = require('@tryghost/debug')('routing');
const routing = require('../../../frontend/services/routing');
// NOTE: temporary import from the frontend, will become a backend service soon
const urlService = require('../../../frontend/services/url');
const routeSettings = require('../../services/route-settings');
module.exports = function siteRoutes(options = {}) {
debug('site Routes', options);
options.routerSettings = routeSettings.loadRouteSettingsSync();
options.urlService = urlService;
return routing.bootstrap.init(options);
};

33
test/unit/frontend/services/routing/controllers/collection.test.js
View File

@ -4,7 +4,7 @@ const sinon = require('sinon');
const testUtils = require('../../../../../utils');
const security = require('@tryghost/security');
const themeEngine = require('../../../../../../core/frontend/services/theme-engine');
const urlService = require('../../../../../../core/frontend/services/url');
const bootstrap = require('../../../../../../core/frontend/services/routing/bootstrap');
const controllers = require('../../../../../../core/frontend/services/routing/controllers');
const helpers = require('../../../../../../core/frontend/services/routing/helpers');
@ -23,6 +23,7 @@ describe('Unit - services/routing/controllers/collection', function () {
let renderStub;
let posts;
let postsPerPage;
let ownsStub;
beforeEach(function () {
postsPerPage = 5;
@ -55,8 +56,8 @@ describe('Unit - services/routing/controllers/collection', function () {
sinon.stub(helpers, 'renderEntries').returns(renderStub);
sinon.stub(urlService, 'owns');
urlService.owns.withArgs('identifier', posts[0].id).returns(true);
ownsStub = sinon.stub(bootstrap.internal, 'owns');
ownsStub.withArgs('identifier', posts[0].id).returns(true);
req = {
path: '/',
@ -93,7 +94,7 @@ describe('Unit - services/routing/controllers/collection', function () {
security.string.safe.calledOnce.should.be.false();
fetchDataStub.calledOnce.should.be.true();
secureStub.calledOnce.should.be.true();
urlService.owns.calledOnce.should.be.true();
ownsStub.calledOnce.should.be.true();
done();
}).catch(done);
});
@ -116,7 +117,7 @@ describe('Unit - services/routing/controllers/collection', function () {
security.string.safe.calledOnce.should.be.false();
fetchDataStub.calledOnce.should.be.true();
secureStub.calledOnce.should.be.true();
urlService.owns.calledOnce.should.be.true();
ownsStub.calledOnce.should.be.true();
done();
}).catch(done);
});
@ -141,7 +142,7 @@ describe('Unit - services/routing/controllers/collection', function () {
security.string.safe.calledOnce.should.be.false();
fetchDataStub.calledOnce.should.be.true();
secureStub.calledOnce.should.be.true();
urlService.owns.calledOnce.should.be.true();
ownsStub.calledOnce.should.be.true();
done();
}).catch(done);
});
@ -167,7 +168,7 @@ describe('Unit - services/routing/controllers/collection', function () {
fetchDataStub.calledOnce.should.be.true();
renderStub.calledOnce.should.be.false();
secureStub.calledOnce.should.be.false();
urlService.owns.calledOnce.should.be.false();
ownsStub.calledOnce.should.be.false();
done();
});
});
@ -190,7 +191,7 @@ describe('Unit - services/routing/controllers/collection', function () {
security.string.safe.calledOnce.should.be.true();
fetchDataStub.calledOnce.should.be.true();
secureStub.calledOnce.should.be.true();
urlService.owns.calledOnce.should.be.true();
ownsStub.calledOnce.should.be.true();
done();
}).catch(done);
});
@ -213,7 +214,7 @@ describe('Unit - services/routing/controllers/collection', function () {
security.string.safe.calledOnce.should.be.false();
fetchDataStub.calledOnce.should.be.true();
secureStub.calledOnce.should.be.true();
urlService.owns.calledOnce.should.be.true();
ownsStub.calledOnce.should.be.true();
done();
}).catch(done);
});
@ -237,7 +238,7 @@ describe('Unit - services/routing/controllers/collection', function () {
security.string.safe.calledOnce.should.be.false();
fetchDataStub.calledOnce.should.be.true();
secureStub.calledTwice.should.be.true();
urlService.owns.calledOnce.should.be.true();
ownsStub.calledOnce.should.be.true();
done();
}).catch(done);
});
@ -252,11 +253,11 @@ describe('Unit - services/routing/controllers/collection', function () {
res.routerOptions.filter = 'featured:true';
urlService.owns.reset();
urlService.owns.withArgs('identifier', posts[0].id).returns(false);
urlService.owns.withArgs('identifier', posts[1].id).returns(true);
urlService.owns.withArgs('identifier', posts[2].id).returns(false);
urlService.owns.withArgs('identifier', posts[3].id).returns(false);
ownsStub.reset();
ownsStub.withArgs('identifier', posts[0].id).returns(false);
ownsStub.withArgs('identifier', posts[1].id).returns(true);
ownsStub.withArgs('identifier', posts[2].id).returns(false);
ownsStub.withArgs('identifier', posts[3].id).returns(false);
fetchDataStub.withArgs({page: 1, slug: undefined, limit: postsPerPage}, res.routerOptions)
.resolves({
@ -276,7 +277,7 @@ describe('Unit - services/routing/controllers/collection', function () {
security.string.safe.calledOnce.should.be.false();
fetchDataStub.calledOnce.should.be.true();
secureStub.calledTwice.should.be.true();
urlService.owns.callCount.should.eql(4);
ownsStub.callCount.should.eql(4);
done();
}).catch(done);
});

12
test/unit/frontend/services/routing/controllers/entry.test.js
View File

@ -2,8 +2,8 @@ const should = require('should');
const sinon = require('sinon');
const testUtils = require('../../../../../utils');
const configUtils = require('../../../../../utils/configUtils');
const urlService = require('../../../../../../core/frontend/services/url');
const urlUtils = require('../../../../../../core/shared/url-utils');
const bootstrap = require('../../../../../../core/frontend/services/routing/bootstrap');
const controllers = require('../../../../../../core/frontend/services/routing/controllers');
const helpers = require('../../../../../../core/frontend/services/routing/helpers');
const EDITOR_URL = `/#/editor/post/`;
@ -41,7 +41,7 @@ describe('Unit - services/routing/controllers/entry', function () {
sinon.stub(urlUtils, 'redirectToAdmin');
sinon.stub(urlUtils, 'redirect301');
sinon.stub(urlService, 'getResourceById');
sinon.stub(bootstrap.internal, 'getResourceById');
req = {
path: '/',
@ -81,7 +81,7 @@ describe('Unit - services/routing/controllers/entry', function () {
res.routerOptions.resourceType = 'posts';
urlService.getResourceById.withArgs(post.id).returns({
bootstrap.internal.getResourceById.withArgs(post.id).returns({
config: {
type: 'posts'
}
@ -162,7 +162,7 @@ describe('Unit - services/routing/controllers/entry', function () {
req.path = post.url;
res.routerOptions.resourceType = 'posts';
urlService.getResourceById.withArgs(post.id).returns({
bootstrap.internal.getResourceById.withArgs(post.id).returns({
config: {
type: 'pages'
}
@ -186,7 +186,7 @@ describe('Unit - services/routing/controllers/entry', function () {
res.routerOptions.resourceType = 'posts';
urlService.getResourceById.withArgs(post.id).returns({
bootstrap.internal.getResourceById.withArgs(post.id).returns({
config: {
type: 'posts'
}
@ -215,7 +215,7 @@ describe('Unit - services/routing/controllers/entry', function () {
res.routerOptions.resourceType = 'posts';
urlService.getResourceById.withArgs(post.id).returns({
bootstrap.internal.getResourceById.withArgs(post.id).returns({
config: {
type: 'posts'
}