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

Removed event chain caused by settings date update

Browse Source 
refs https://linear.app/tryghost/issue/CORE-104/decouple-frontend-routing-events-from-urlserver-events

- The 'settings.timezone.edited' event triggers a roundtrip chain of calls in the frontend routing to the url services. It was all handled by event listeners and handler that clearly don't belong there.
- Extracted event realted listeners/handlers into methods and moved most of the logic to the "bootstrap" module, which soon is going to become a "RoutesManger"
- The result of this refactor - no more events going back and forth between frontend routing and the backend!
This commit is contained in:
Naz 2021-10-14 19:10:36 +02:00 committed by naz
parent 3bca65d868
commit 18344a16e2
9 changed files with 160 additions and 121 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

8
core/bridge.js
View File

@ -16,6 +16,7 @@ const config = require('./shared/config');
const logging = require('@tryghost/logging');
const tpl = require('@tryghost/tpl');
const themeEngine = require('./frontend/services/theme-engine');
const frontendRouting = require('./frontend/services/routing/bootstrap');
const settingsCache = require('./shared/settings-cache');
// Listen to settings.lang.edited, similar to the member service and models/base/listeners
@ -35,6 +36,13 @@ class Bridge {
debug('Active theme init18n');
this.getActiveTheme().initI18n({locale: model.get('value')});
});
// NOTE: eventually this event should somehow be listened on and handled by the URL Service
// for now this eliminates the need for the frontend routing to listen to
// server events
events.on('settings.timezone.edited', (model) => {
frontendRouting.handleTimezoneEdit(model);
});
}
getActiveTheme() {

47
core/frontend/services/routing/CollectionRouter.js
View File

@ -7,9 +7,6 @@ const middlewares = require('./middlewares');
const RSSRouter = require('./RSSRouter');
const bootstrap = require('./bootstrap');
// This emits its own routing events AND listens to settings.timezone.edited :(
const events = require('../../../server/lib/common/events');
/**
* @description Collection Router for post resource.
*
@ -60,7 +57,6 @@ class CollectionRouter extends ParentRouter {
debug(this.name, this.route, this.permalinks);
this._registerRoutes();
this._listeners();
}
/**
@ -128,43 +124,6 @@ class CollectionRouter extends ParentRouter {
next();
}
/**
* @description This router has listeners to react on changes which happen in Ghost.
* @private
*/
_listeners() {
/**
* CASE: timezone changes
*
* If your permalink contains a date reference, we have to regenerate the urls.
*
* e.g. /:year/:month/:day/:slug/ or /:day/:slug/
*/
this._onTimezoneEditedListener = this._onTimezoneEdited.bind(this);
events.on('settings.timezone.edited', this._onTimezoneEditedListener);
}
/**
* @description Helper function to handle a timezone change.
* @param settingModel
* @private
*/
_onTimezoneEdited(settingModel) {
const newTimezone = settingModel.attributes.value;
const previousTimezone = settingModel._previousAttributes.value;
if (newTimezone === previousTimezone) {
return;
}
if (this.getPermalinks().getValue().match(/:year|:month|:day/)) {
debug('_onTimezoneEdited: trigger regeneration');
// @NOTE: The connected url generator will listen on this event and regenerate urls.
this.emit('updated');
}
}
/**
* @description Get resource type of this router (always "posts")
* @returns {string}
@ -197,12 +156,6 @@ class CollectionRouter extends ParentRouter {
return urlUtils.createUrl(urlUtils.urlJoin(this.route.value, this.rssRouter.route.value), options.absolute, options.secure);
}
reset() {
if (this._onTimezoneEditedListener) {
events.removeListener('settings.timezone.edited', this._onTimezoneEditedListener);
}
}
}
module.exports = CollectionRouter;

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

@ -14,6 +14,8 @@ const events = require('../../../server/lib/common/events');
const defaultApiVersion = 'v4';
// This registry thing should be passed into here as a dependency once the module
// is rewritten into the class + DI pattern
const registry = require('./registry');
let siteRouter;
let _urlService;
@ -119,6 +121,38 @@ const start = (apiVersion, routerSettings) => {
debug('Routes:', registry.getAllRoutes());
};
/**
* This is a glue code to keep the implementation of routers away from
* this sort of logic. Ideally this method should not be ever called
* and handled completely on the URL Service layer without touching the frontend
* @param {Object} settingModel instance of the settings model
* @returns {void}
*/
const handleTimezoneEdit = (settingModel) => {
const newTimezone = settingModel.attributes.value;
const previousTimezone = settingModel._previousAttributes.value;
if (newTimezone === previousTimezone) {
return;
}
/**
* CASE: timezone changes
*
* If your permalink contains a date reference, we have to regenerate the urls.
*
* e.g. /:year/:month/:day/:slug/ or /:day/:slug/
*/
// NOTE: timezone change only affects the collection router with dated permalinks
const collectionRouter = registry.getRouterByName('CollectionRouter');
if (collectionRouter.getPermalinks().getValue().match(/:year|:month|:day/)) {
debug('handleTimezoneEdit: trigger regeneration');
this.internal.routerUpdated(collectionRouter);
}
};
module.exports.internal = {
owns: (routerId, id) => {
return _urlService.owns(routerId, id);
@ -135,7 +169,13 @@ module.exports.internal = {
events.emit('router.created', this);
_urlService.onRouterAddedType(router);
},
routerUpdated: (router) => {
_urlService.onRouterUpdated(router);
}
};
// Public API methods called out by the backend
module.exports.init = init;
module.exports.start = start;
module.exports.handleTimezoneEdit = handleTimezoneEdit;

13
core/frontend/services/routing/registry.js
View File

@ -41,6 +41,19 @@ module.exports = {
return routers[name];
},
/**
* Gets a router by it's internal router name
* @param {String} name internal router name
* @returns {Express-Router}
*/
getRouterByName(name) {
for (let routerKey in routers) {
if (routers[routerKey].name === name) {
return routers[routerKey];
}
}
},
/**
*
*

28
core/frontend/services/url/UrlGenerator.js
View File

@ -67,25 +67,25 @@ class UrlGenerator {
this._listeners();
}
/**
* @NOTE: currently only used if the permalink setting changes and it's used for this url generator.
* @TODO: https://github.com/TryGhost/Ghost/issues/10699
*/
regenerateResources() {
const myResources = this.urls.getByGeneratorId(this.uid);
myResources.forEach((object) => {
this.urls.removeResourceId(object.resource.data.id);
object.resource.release();
this._try(object.resource);
});
}
/**
* @description Helper function to register listeners for each url generator instance.
* @private
*/
_listeners() {
/**
* @NOTE: currently only used if the permalink setting changes and it's used for this url generator.
* @TODO: https://github.com/TryGhost/Ghost/issues/10699
*/
this.router.addListener('updated', () => {
const myResources = this.urls.getByGeneratorId(this.uid);
myResources.forEach((object) => {
this.urls.removeResourceId(object.resource.data.id);
object.resource.release();
this._try(object.resource);
});
});
/**
* Listen on two events:
*

9
core/frontend/services/url/UrlService.js
View File

@ -89,6 +89,15 @@ class UrlService {
this.urlGenerators.push(urlGenerator);
}
/**
* @description Router update handler - regenerates it's resources
* @param {ExpressRouter} router
*/
onRouterUpdated(router) {
const generator = this.urlGenerators.find(g => g.router.id === router.id);
generator.regenerateResources();
}
/**
* @description If the API version in the theme config changes, we have to reset urls and resources.
* @private

58
test/unit/frontend/services/routing/CollectionRouter.test.js
View File

@ -179,62 +179,4 @@ describe('UNIT - services/routing/CollectionRouter', function () {
});
});
});
describe('timezone changes', function () {
describe('no dated permalink', function () {
it('default', function () {
const collectionRouter = new CollectionRouter('/magic/', {permalink: '/:slug/'}, RESOURCE_CONFIG);
sinon.stub(collectionRouter, 'emit');
events.on.args[0][1]({
attributes: {value: 'America/Los_Angeles'},
_previousAttributes: {value: 'Europe/London'}
});
collectionRouter.emit.called.should.be.false();
});
it('tz has not changed', function () {
const collectionRouter = new CollectionRouter('/magic/', {permalink: '/:slug/'}, RESOURCE_CONFIG);
sinon.stub(collectionRouter, 'emit');
events.on.args[0][1]({
attributes: {value: 'America/Los_Angeles'},
_previousAttributes: {value: 'America/Los_Angeles'}
});
collectionRouter.emit.called.should.be.false();
});
});
describe('with dated permalink', function () {
it('default', function () {
const collectionRouter = new CollectionRouter('/magic/', {permalink: '/:year/:slug/'}, RESOURCE_CONFIG);
sinon.stub(collectionRouter, 'emit');
events.on.args[0][1]({
attributes: {value: 'America/Los_Angeles'},
_previousAttributes: {value: 'Europe/London'}
});
collectionRouter.emit.calledOnce.should.be.true();
});
it('tz has not changed', function () {
const collectionRouter = new CollectionRouter('/magic/', {permalink: '/:year/:slug/'}, RESOURCE_CONFIG);
sinon.stub(collectionRouter, 'emit');
events.on.args[0][1]({
attributes: {value: 'America/Los_Angeles'},
_previousAttributes: {value: 'America/Los_Angeles'}
});
collectionRouter.emit.called.should.be.false();
});
});
});
});

74
test/unit/frontend/services/routing/bootstrap.test.js vendored Normal file
View File

@ -0,0 +1,74 @@
const should = require('should');
const sinon = require('sinon');
const CollectionRouter = require('../../../../../core/frontend/services/routing/CollectionRouter');
const bootstrap = require('../../../../../core/frontend/services/routing/bootstrap');
const registry = require('../../../../../core/frontend/services/routing/registry');
const RESOURCE_CONFIG = {QUERY: {post: {controller: 'posts', resource: 'posts'}}};
describe('UNIT: services/routing/bootstrap', function () {
let routesUpdatedStub;
beforeEach(function () {
sinon.stub(bootstrap.internal, 'routerCreated').returns();
routesUpdatedStub = sinon.stub(bootstrap.internal, 'routerUpdated').returns();
});
afterEach(function () {
sinon.restore();
});
describe('timezone changes', function () {
describe('no dated permalink', function () {
it('default', function () {
const collectionRouter = new CollectionRouter('/magic/', {permalink: '/:slug/'}, RESOURCE_CONFIG);
sinon.stub(registry, 'getRouterByName').withArgs('CollectionRouter').returns(collectionRouter);
bootstrap.handleTimezoneEdit({
attributes: {value: 'America/Los_Angeles'},
_previousAttributes: {value: 'Europe/London'}
});
routesUpdatedStub.called.should.be.false();
});
it('tz has not changed', function () {
const collectionRouter = new CollectionRouter('/magic/', {permalink: '/:slug/'}, RESOURCE_CONFIG);
sinon.stub(registry, 'getRouterByName').withArgs('CollectionRouter').returns(collectionRouter);
bootstrap.handleTimezoneEdit({
attributes: {value: 'America/Los_Angeles'},
_previousAttributes: {value: 'America/Los_Angeles'}
});
routesUpdatedStub.called.should.be.false();
});
});
describe('with dated permalink', function () {
it('default', function () {
const collectionRouter = new CollectionRouter('/magic/', {permalink: '/:year/:slug/'}, RESOURCE_CONFIG);
sinon.stub(registry, 'getRouterByName').withArgs('CollectionRouter').returns(collectionRouter);
bootstrap.handleTimezoneEdit({
attributes: {value: 'America/Los_Angeles'},
_previousAttributes: {value: 'Europe/London'}
});
routesUpdatedStub.called.should.be.true();
});
it('tz has not changed', function () {
const collectionRouter = new CollectionRouter('/magic/', {permalink: '/:year/:slug/'}, RESOURCE_CONFIG);
sinon.stub(registry, 'getRouterByName').withArgs('CollectionRouter').returns(collectionRouter);
bootstrap.handleTimezoneEdit({
attributes: {value: 'America/Los_Angeles'},
_previousAttributes: {value: 'America/Los_Angeles'}
});
routesUpdatedStub.called.should.be.false();
});
});
});
});

4
test/unit/frontend/services/url/UrlGenerator.test.js
View File

@ -61,7 +61,6 @@ describe('Unit: services/url/UrlGenerator', function () {
const urlGenerator = new UrlGenerator(router, queue);
queue.register.calledTwice.should.be.true();
router.addListener.calledOnce.should.be.true();
should.not.exist(urlGenerator.filter);
});
@ -94,7 +93,8 @@ describe('Unit: services/url/UrlGenerator', function () {
id: 'object-id-1'
};
router.addListener.args[0][1]();
urlGenerator.regenerateResources();
urls.removeResourceId.calledTwice.should.be.true();
resource.release.calledOnce.should.be.true();
resource2.release.calledOnce.should.be.true();