Added comments for url service

no issue

- jsdoc
- inline comments
This commit is contained in:
kirrg001 2019-04-21 00:14:04 +02:00
parent e1dca54bf7
commit e07c0ecdc4
6 changed files with 363 additions and 33 deletions

View File

@ -69,9 +69,14 @@ class Queue extends EventEmitter {
}
/**
* `tolerance`:
* @description Register a subscriber for this queue.
*
* tolerance:
* - 0: don't wait for more subscribers [default]
* - 100: wait long enough till all subscribers have registered (e.g. bootstrap)
*
* @param {Object} options
* @param {function} fn
*/
register(options, fn) {
if (!options.hasOwnProperty('tolerance')) {
@ -92,6 +97,10 @@ class Queue extends EventEmitter {
this.queue[options.event].subscribers.push(fn);
}
/**
* @description The queue runs & executes subscribers one by one (sequentially)
* @param {Object} options
*/
run(options) {
const event = options.event,
action = options.action,
@ -108,7 +117,7 @@ class Queue extends EventEmitter {
debug('execute', action, event, this.toNotify[action].notified.length);
/**
* Currently no async operations happen in the subscribers functions.
* @NOTE: Currently no async operations happen in the subscribers functions.
* We can trigger the functions sync.
*/
try {
@ -153,6 +162,16 @@ class Queue extends EventEmitter {
}
}
/**
* @description Start the queue from outside.
*
* CASE:
*
* - resources were fetched from database on bootstrap
* - resource was added
*
* @param options
*/
start(options) {
debug('start');
@ -185,7 +204,7 @@ class Queue extends EventEmitter {
}
}
// reset who was already notified
// @NOTE: reset who was already notified
this.toNotify[options.action] = {
event: options.event,
timeoutInMS: options.timeoutInMS || 50,
@ -196,6 +215,11 @@ class Queue extends EventEmitter {
this.run(options);
}
/**
* @description Hard reset queue from outside.
*
* Reset usually only happens if you e.g. switch the api version.
*/
reset() {
this.queue = {};
@ -206,6 +230,12 @@ class Queue extends EventEmitter {
this.toNotify = {};
}
/**
* @description Soft reset queue from outside.
*
* A soft reset does NOT clear the subscribers!
* Only used for test env currently.
*/
softReset() {
_.each(this.toNotify, (obj) => {
clearTimeout(obj.timeout);

View File

@ -1,6 +1,9 @@
const EventEmitter = require('events').EventEmitter,
common = require('../../lib/common');
/**
* Resource cache.
*/
class Resource extends EventEmitter {
constructor(type, obj) {
super();
@ -14,10 +17,20 @@ class Resource extends EventEmitter {
Object.assign(this.data, obj);
}
/**
* @description Get the type of the resource e.g. posts, users ...
* @returns {String} type
*/
getType() {
return this.config.type;
}
/**
* @description Reserve a resource.
*
* This happens if a url generator's conditions matches a resource.
* We have to reserve resources, because otherwise resources can appear in multiple url structures.
*/
reserve() {
if (!this.config.reserved) {
this.config.reserved = true;
@ -29,14 +42,32 @@ class Resource extends EventEmitter {
}
}
/**
* @description Release a resource.
*
* This happens if conditions of a url generator no longer matches a resource.
* e.g. change a post to a page.
*/
release() {
this.config.reserved = false;
}
/**
* @description Check whether a resource is reserved.
* @returns {boolean}
*/
isReserved() {
return this.config.reserved === true;
}
/**
* @description Update the resource cache.
*
* Emit update to subscribers - observer pattern.
* e.g. url generator will listen on it's own resource's.
*
* @param {Object} obj - raw resource data
*/
update(obj) {
Object.assign(this.data, obj);
@ -47,7 +78,15 @@ class Resource extends EventEmitter {
this.emit('updated', this);
}
/**
* @description Remove a resource.
*
* The fn is only useful to emit the action/event right now.
*
* CASE: url generator needs to know if one of it's resources/url should be removed.
*/
remove() {
// CASE: do not emit, if it is not reserved, because nobody will listen on events.
if (!this.isReserved()) {
return;
}

View File

@ -7,10 +7,12 @@ const models = require('../../models');
const common = require('../../lib/common');
/**
* At the moment Resource service is directly responsible for data population
* for URLs in UrlService. But because it's actually a storage of all possible
* @description At the moment the resources class is directly responsible for data population
* for URLs...but because it's actually a storage cache of all published
* resources in the system, could also be used as a cache for Content API in
* the future.
*
* Each entry in the database will be represented by a "Resource" (see /Resource.js).
*/
class Resources {
constructor(queue) {
@ -22,6 +24,14 @@ class Resources {
this._listeners();
}
/**
* @description Little helper to register on Ghost events and remember the listener functions to be able
* to unsubscribe.
*
* @param {String} eventName
* @param {Function} listener
* @private
*/
_listenOn(eventName, listener) {
this.listeners.push({
eventName: eventName,
@ -31,15 +41,23 @@ class Resources {
common.events.on(eventName, listener);
}
/**
* @description Little helper which get's called on class instantiation. It will subscribe to the
* database ready event to start fetching the data as early as possible.
*
* @private
*/
_listeners() {
/**
* We fetch the resources as early as possible.
* Currently the url service needs to use the settings cache,
* because we need to `settings.permalink`.
*/
this._listenOn('db.ready', this.fetchResources.bind(this));
}
/**
* @description Initialise the resource config. We currently fetch the data straight via the the model layer,
* but because Ghost supports multiple API versions, we have to ensure we load the correct data.
*
* @TODO: https://github.com/TryGhost/Ghost/issues/10360
* @private
*/
_initResourceConfig() {
if (!_.isEmpty(this.resourcesConfig)) {
return this.resourceConfig;
@ -49,11 +67,17 @@ class Resources {
this.resourcesConfig = require(`./configs/${this.resourcesAPIVersion}`);
}
/**
* @description Helper function to initialise data fetching. Each resource type needs to register resource/model
* events to get notified about updates/deletions/inserts.
*/
fetchResources() {
const ops = [];
debug('db ready. settings cache ready.');
debug('fetchResources');
this._initResourceConfig();
// NOTE: Iterate over all resource types (posts, users etc..) and call `_fetch`.
_.each(this.resourcesConfig, (resourceConfig) => {
this.data[resourceConfig.type] = [];
@ -92,13 +116,20 @@ class Resources {
});
}
/**
* @description The actual call to the model layer, which will execute raw knex queries to ensure performance.
* @param {Object} resourceConfig
* @param {Object} options
* @returns {Promise}
* @private
*/
_fetch(resourceConfig, options = {offset: 0, limit: 999}) {
debug('_fetch', resourceConfig.type, resourceConfig.modelOptions);
let modelOptions = _.cloneDeep(resourceConfig.modelOptions);
const isSQLite = config.get('database:client') === 'sqlite3';
// CASE: prevent "too many SQL variables" error on SQLite3
// CASE: prevent "too many SQL variables" error on SQLite3 (https://github.com/TryGhost/Ghost/issues/5810)
if (isSQLite) {
modelOptions.offset = options.offset;
modelOptions.limit = options.limit;
@ -119,6 +150,20 @@ class Resources {
});
}
/**
* @description Call the model layer to fetch a single resource via raw knex queries.
*
* This function was invented, because the model event is a generic event, which is independent of any
* api version behaviour. We have to ensure that a model matches the conditions of the configured api version
* in the theme.
*
* See https://github.com/TryGhost/Ghost/issues/10124.
*
* @param {Object} resourceConfig
* @param {String} id
* @returns {Promise}
* @private
*/
_fetchSingle(resourceConfig, id) {
let modelOptions = _.cloneDeep(resourceConfig.modelOptions);
modelOptions.id = id;
@ -126,6 +171,18 @@ class Resources {
return models.Base.Model.raw_knex.fetchAll(modelOptions);
}
/**
* @description Helper function to prepare the received model's relations.
*
* This helper was added to reduce the number of fields we keep in cache for relations.
*
* If we resolve (https://github.com/TryGhost/Ghost/issues/10360) and talk to the Content API,
* we could pass on e.g. `?include=authors&fields=authors.id,authors.slug`, but the API has to support it.
*
* @param {Bookshelf-Model} model
* @param {Object} resourceConfig
* @private
*/
_prepareModelSync(model, resourceConfig) {
const exclude = resourceConfig.modelOptions.exclude;
const withRelatedFields = resourceConfig.modelOptions.withRelatedFields;
@ -168,6 +225,19 @@ class Resources {
return obj;
}
/**
* @description Listener for "model added" event.
*
* If we receive an event from the model layer, we push the new resource into the queue.
* The subscribers (the url generators) have registered for this event and the queue will call
* all subscribers sequentially. The first generator, where the conditions match the resource, will
* own the resource and it's url.
*
* @param {String} type (post,user...)
* @param {Bookshelf-Model} model
* @returns {Promise}
* @private
*/
_onResourceAdded(type, model) {
debug('_onResourceAdded', type);
@ -217,6 +287,8 @@ class Resources {
}
/**
* @description Listener for "model updated" event.
*
* CASE:
* - post was fetched on bootstrap
* - that means, the post is already published
@ -229,6 +301,11 @@ class Resources {
* - resource exists and is owned by somebody
* - but the data changed and is maybe no longer owned?
* - e.g. featured:false changes and your filter requires featured posts
*
* @param {String} type (post,user...)
* @param {Bookshelf-Model} model
* @returns {Promise}
* @private
*/
_onResourceUpdated(type, model) {
debug('_onResourceUpdated', type);
@ -238,13 +315,14 @@ class Resources {
// NOTE: synchronous handling for post and pages so that their URL is available without a delay
// for more context and future improvements check https://github.com/TryGhost/Ghost/issues/10360
if (['posts', 'pages'].includes(type)) {
// CASE: search for the target resource in the cache
this.data[type].every((resource) => {
if (resource.data.id === model.id) {
const obj = this._prepareModelSync(model, resourceConfig);
resource.update(obj);
// CASE: pretend it was added
// CASE: Resource is not owned, try to add it again (data has changed, it could be that somebody will own it now)
if (!resource.isReserved()) {
this.queue.start({
event: 'added',
@ -270,6 +348,7 @@ class Resources {
.then(([dbResource]) => {
const resource = this.data[type].find(resource => (resource.data.id === model.id));
// CASE: cached resource exists, API conditions matched with the data in the db
if (resource && dbResource) {
resource.update(dbResource);
@ -293,12 +372,19 @@ class Resources {
}
}
/**
* @description Listener for "model removed" event.
* @param {String} type (post,user...)
* @param {Bookshelf-Model} model
* @private
*/
_onResourceRemoved(type, model) {
debug('_onResourceRemoved', type);
let index = null;
let resource;
// CASE: search for the cached resource and stop if it was found
this.data[type].every((_resource, _index) => {
if (_resource.data.id === model._previousAttributes.id) {
resource = _resource;
@ -316,22 +402,45 @@ class Resources {
return;
}
// remove the resource from cache
this.data[type].splice(index, 1);
resource.remove();
}
/**
* @description Get all cached resources.
* @returns {Object}
*/
getAll() {
return this.data;
}
/**
* @description Get all cached resourced by type.
* @param {String} type (post, user...)
* @returns {Object}
*/
getAllByType(type) {
return this.data[type];
}
/**
* @description Get all cached resourced by resource id and type.
* @param {String} type (post, user...)
* @param {String} id
* @returns {Object}
*/
getByIdAndType(type, id) {
return _.find(this.data[type], {data: {id: id}});
}
/**
* @description Reset this class instance.
*
* Is triggered if you switch API versions.
*
* @param {Object} options
*/
reset(options = {ignoreDBReady: false}) {
_.each(this.listeners, (obj) => {
if (obj.eventName === 'db.ready' && options.ignoreDBReady) {
@ -346,6 +455,10 @@ class Resources {
this.resourcesConfig = null;
}
/**
* @description Soft reset this class instance. Only used for test env.
* It will only clear the cache.
*/
softReset() {
this.data = {};
@ -354,6 +467,9 @@ class Resources {
});
}
/**
* @description Release all resources. Get's called during "reset".
*/
releaseAll() {
_.each(this.data, (resources, type) => {
_.each(this.data[type], (resource) => {

View File

@ -23,6 +23,14 @@ const _ = require('lodash'),
replacement: 'primary_author.slug'
}];
/**
* The UrlGenerator class is responsible to generate urls based on a router's conditions.
* It is the component which sits between routers and resources and connects them together.
* Each url generator can own resources. Each resource can only be owned by one generator,
* because each resource can only live on one url at a time.
*
* Each router is represented by a url generator.
*/
class UrlGenerator {
constructor(router, queue, resources, urls, position) {
this.router = router;
@ -43,10 +51,14 @@ class UrlGenerator {
this._listeners();
}
/**
* @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: remove in Ghost 2.0
* @TODO: https://github.com/TryGhost/Ghost/issues/10699
*/
this.router.addListener('updated', () => {
const myResources = this.urls.getByGeneratorId(this.uid);
@ -62,19 +74,25 @@ class UrlGenerator {
* Listen on two events:
*
* - init: bootstrap or url reset
* - added: resource was added
* - added: resource was added to the database
*/
this.queue.register({
event: 'init',
tolerance: 100
}, this._onInit.bind(this));
// @TODO: listen on added event per type (post optimisation)
this.queue.register({
event: 'added'
}, this._onAdded.bind(this));
}
/**
* @description Listener which get's called when the resources were fully fetched from the database.
*
* Each url generator will be called and can try to own resources now.
*
* @private
*/
_onInit() {
debug('_onInit', this.router.getResourceType());
@ -88,6 +106,11 @@ class UrlGenerator {
});
}
/**
* @description Listener which get's called when a resource was added on runtime.
* @param {String} event
* @private
*/
_onAdded(event) {
debug('onAdded', this.toString());
@ -101,6 +124,12 @@ class UrlGenerator {
this._try(resource);
}
/**
* @description Try to own a resource and generate it's url if so.
* @param {Resource} resource
* @returns {boolean}
* @private
*/
_try(resource) {
/**
* CASE: another url generator has taken this resource already.
@ -142,7 +171,9 @@ class UrlGenerator {
}
/**
* We currently generate relative urls without subdirectory.
* @description Generate url based on the permlink configuration of the target router.
*
* @NOTE We currently generate relative urls (https://github.com/TryGhost/Ghost/commit/7b0d5d465ba41073db0c3c72006da625fa11df32).
*/
_generateUrl(resource) {
const permalink = this.router.getPermalinks().getValue();
@ -150,8 +181,9 @@ class UrlGenerator {
}
/**
* @description Helper function to register resource listeners.
*
* I want to know if my resources changes.
* Register events of resource.
*
* If the owned resource get's updated, we simply release/free the resource and push it back to the queue.
* This is the easiest, less error prone implementation.
@ -191,6 +223,11 @@ class UrlGenerator {
resource.addListener('removed', onRemoved.bind(this));
}
/**
* @description Figure out if this url generator own's a resource id.
* @param {String} id
* @returns {boolean}
*/
hasId(id) {
const existingUrl = this.urls.getByResourceId(id);
@ -201,10 +238,18 @@ class UrlGenerator {
return false;
}
/**
* @description Get all urls of this url generator.
* @returns {Array}
*/
getUrls() {
return this.urls.getByGeneratorId(this.uid);
}
/**
* @description Override of `toString`
* @returns {string}
*/
toString() {
return this.router.toString();
}

View File

@ -8,6 +8,11 @@ const _debug = require('ghost-ignition').debug._base,
Resources = require('./Resources'),
localUtils = require('./utils');
/**
* The url service class holds all instances in a centralised place.
* It's the public API you can talk to.
* It will tell you if the url generation is in progress or not.
*/
class UrlService {
constructor() {
this.utils = localUtils;
@ -22,19 +27,17 @@ class UrlService {
this._listeners();
}
/**
* @description Helper function to register the listeners for this instance.
* @private
*/
_listeners() {
/**
* The purpose of this event is to notify the url service as soon as a router get's created.
*/
this._onRouterAddedListener = this._onRouterAddedType.bind(this);
common.events.on('router.created', this._onRouterAddedListener);
this._onThemeChangedListener = this._onThemeChangedListener.bind(this);
common.events.on('services.themes.api.changed', this._onThemeChangedListener);
/**
* The queue will notify us if url generation has started/finished.
*/
this._onQueueStartedListener = this._onQueueStarted.bind(this);
this.queue.addListener('started', this._onQueueStartedListener);
@ -42,18 +45,37 @@ class UrlService {
this.queue.addListener('ended', this._onQueueEnded.bind(this));
}
/**
* @description The queue will notify us if the queue has started with an event.
*
* The "init" event is basically the bootstrap event, which is the siganliser if url generation
* is in progress or not.
*
* @param {String} event
* @private
*/
_onQueueStarted(event) {
if (event === 'init') {
this.finished = false;
}
}
/**
* @description The queue will notify us if the queue has ended with an event.
* @param {String} event
* @private
*/
_onQueueEnded(event) {
if (event === 'init') {
this.finished = true;
}
}
/**
* @description Router was created, connect it with a url generator.
* @param {ExpressRouter} router
* @private
*/
_onRouterAddedType(router) {
// CASE: there are router types which do not generate resource urls
// e.g. static route router
@ -68,12 +90,18 @@ class UrlService {
this.urlGenerators.push(urlGenerator);
}
/**
* @description If the API version in the theme config changes, we have to reset urls and resources.
* @private
*/
_onThemeChangedListener() {
this.reset({keepListeners: true});
this.init();
}
/**
* @description Get Resource by url.
*
* You have a url and want to know which the url belongs to.
*
* It's in theory possible that multiple resources generate the same url,
@ -91,6 +119,10 @@ class UrlService {
* We only return the resource, which would be served.
*
* @NOTE: only accepts relative urls at the moment.
*
* @param {String} url
* @param {Object} options
* @returns {Object}
*/
getResource(url, options) {
options = options || {};
@ -133,6 +165,11 @@ class UrlService {
return objects[0].resource;
}
/**
* @description Get resource by id.
* @param {String} resourceId
* @returns {Object}
*/
getResourceById(resourceId) {
const object = this.urls.getByResourceId(resourceId);
@ -146,13 +183,16 @@ class UrlService {
return object.resource;
}
/**
* @description Figure out if url generation is in progress or not.
* @returns {boolean}
*/
hasFinished() {
return this.finished;
}
/**
* Get url by resource id.
* e.g. tags, authors, posts, pages
* @description Get url by resource id.
*
* If we can't find a url for an id, we have to return a url.
* There are many components in Ghost which call `getUrlByResourceId` and
@ -160,6 +200,10 @@ class UrlService {
* Or if you define no collections in your yaml file and serve a page.
* You will see a suggestion of posts, but they all don't belong to a collection.
* They would show localhost:2368/null/.
*
* @param {String} id
* @param {Object} options
* @returns {String}
*/
getUrlByResourceId(id, options) {
options = options || {};
@ -189,6 +233,12 @@ class UrlService {
return '/404/';
}
/**
* @description Check whether a router owns a resource id.
* @param {String} routerId
* @param {String} id
* @returns {boolean}
*/
owns(routerId, id) {
debug('owns', routerId, id);
@ -210,6 +260,12 @@ class UrlService {
return urlGenerator.hasId(id);
}
/**
* @description Get permlink structure for url.
* @param {String} url
* @param {object} options
* @returns {*}
*/
getPermalinkByUrl(url, options) {
options = options || {};
@ -223,10 +279,20 @@ class UrlService {
.getValue(options);
}
/**
* @description Internal helper to re-trigger fetching resources on theme change.
*
* @TODO: Either remove this helper or rename to `_init`, because it's a little confusing,
* because this service get's initalised via events.
*/
init() {
this.resources.fetchResources();
}
/**
* @description Reset this service.
* @param {Object} options
*/
reset(options = {}) {
debug('reset');
this.urlGenerators = [];
@ -243,6 +309,10 @@ class UrlService {
}
}
/**
* @description Reset the generators.
* @param {Object} options
*/
resetGenerators(options = {}) {
debug('resetGenerators');
this.finished = false;
@ -257,6 +327,9 @@ class UrlService {
}
}
/**
* @description Soft reset this service. Only used in test env.
*/
softReset() {
debug('softReset');
this.finished = false;

View File

@ -4,18 +4,26 @@ const localUtils = require('./utils');
const common = require('../../lib/common');
/**
* Keeps track of all urls.
* Each resource has exactly one url.
*
* Connector for url generator and resources.
*
* This class keeps track of all urls in the system.
* Each resource has exactly one url. Each url is owned by exactly one url generator id.
* This is a connector for url generator and resources.
* Stores relative urls by default.
*
* We have to have a centralised place where we keep track of all urls, otherwise
* we will never know if we generate the same url twice. Furthermore, it's easier
* to ask a centralised class instance if you want a url for a resource than
* iterating over all url generators and asking for it.
* You can easily ask `this.urls[resourceId]`.
*/
class Urls {
constructor() {
this.urls = {};
}
/**
* @description Add a url to the system.
* @param {Object} options
*/
add(options) {
const url = options.url;
const generatorId = options.generatorId;
@ -38,6 +46,7 @@ class Urls {
resource: resource
};
// @NOTE: Notify the whole system. Currently used for sitemaps service.
common.events.emit('url.added', {
url: {
relative: url,
@ -47,13 +56,19 @@ class Urls {
});
}
// @TODO: add an option to receive an absolute url
/**
* @description Get url by resource id.
* @param {String} id
* @returns {Object}
*/
getByResourceId(id) {
return this.urls[id];
}
/**
* Get all by `uid`.
* @description Get all urls by generator id.
* @param {String} generatorId
* @returns {Array}
*/
getByGeneratorId(generatorId) {
return _.reduce(Object.keys(this.urls), (toReturn, resourceId) => {
@ -66,6 +81,8 @@ class Urls {
}
/**
* @description Get by url.
*
* @NOTE:
* It's is in theory possible that:
*
@ -85,6 +102,10 @@ class Urls {
}, []);
}
/**
* @description Remove url.
* @param id
*/
removeResourceId(id) {
if (!this.urls[id]) {
return;
@ -100,10 +121,16 @@ class Urls {
delete this.urls[id];
}
/**
* @description Reset instance.
*/
reset() {
this.urls = {};
}
/**
* @description Soft reset instance.
*/
softReset() {
this.urls = {};
}