From 9c6992535bf2e7ee69f503aebc0b519fcd0536af Mon Sep 17 00:00:00 2001 From: Daniel Lockyer Date: Wed, 23 Oct 2024 12:11:41 +0200 Subject: [PATCH] Refactored URL service code to aid with debugging - replaced a couple of uses of lodash.each in favor of native for loops - tidied up `debug` statements and spacing - pulled out common statements into variables --- ghost/core/core/server/services/url/Queue.js | 27 ++++++++++--------- .../core/server/services/url/Resources.js | 7 ++--- .../core/server/services/url/UrlGenerator.js | 12 +++------ .../core/server/services/url/UrlService.js | 5 ++-- ghost/core/core/server/services/url/Urls.js | 9 +++---- 5 files changed, 27 insertions(+), 33 deletions(-) diff --git a/ghost/core/core/server/services/url/Queue.js b/ghost/core/core/server/services/url/Queue.js index 5909abe847..039ffa3b56 100644 --- a/ghost/core/core/server/services/url/Queue.js +++ b/ghost/core/core/server/services/url/Queue.js @@ -93,7 +93,7 @@ class Queue extends EventEmitter { }; } - debug('add', options.event, options.tolerance); + debug('register', options.event, options.tolerance); this.queue[options.event].subscribers.push(fn); } @@ -103,19 +103,20 @@ class Queue extends EventEmitter { * @param {Object} options */ run(options) { - const event = options.event; - const action = options.action; - const eventData = options.eventData; + const {event, action, eventData} = options; clearTimeout(this.toNotify[action].timeout); this.toNotify[action].timeout = null; - debug('run', action, event, this.queue[event].subscribers.length, this.toNotify[action].notified.length); + const subscribers = this.queue[event].subscribers; + const notified = this.toNotify[action].notified; - if (this.queue[event].subscribers.length && this.queue[event].subscribers.length !== this.toNotify[action].notified.length) { - const fn = this.queue[event].subscribers[this.toNotify[action].notified.length]; + debug('run', action, event, subscribers.length, notified.length); - debug('execute', action, event, this.toNotify[action].notified.length); + if (subscribers.length && subscribers.length !== notified.length) { + const fn = subscribers[notified.length]; + + debug('run.execute', action, event, notified.length); /** * @NOTE: Currently no async operations happen in the subscribers functions. @@ -124,7 +125,7 @@ class Queue extends EventEmitter { try { fn(eventData); - debug('executed', action, event, this.toNotify[action].notified.length); + debug('run.executed', action, event, notified.length); this.toNotify[action].notified.push(fn); this.run(options); } catch (err) { @@ -144,15 +145,15 @@ class Queue extends EventEmitter { // CASE 3: wait for more subscribers, i am still tolerant if (this.queue[event].tolerance === 0) { delete this.toNotify[action]; - debug('ended (1)', event, action); + debug('run.ended (1)', event, action); this.emit('ended', event); - } else if (this.queue[options.event].subscribers.length >= this.queue[options.event].requiredSubscriberCount && + } else if (subscribers.length >= this.queue[event].requiredSubscriberCount && this.toNotify[action].timeoutInMS > this.queue[event].tolerance) { delete this.toNotify[action]; - debug('ended (2)', event, action); + debug('run.ended (2)', event, action); this.emit('ended', event); } else { - debug('retry', event, action, this.toNotify[action].timeoutInMS); + debug('run.retry', event, action, this.toNotify[action].timeoutInMS); this.toNotify[action].timeoutInMS = this.toNotify[action].timeoutInMS * 1.1; diff --git a/ghost/core/core/server/services/url/Resources.js b/ghost/core/core/server/services/url/Resources.js index f37199aa1a..58b9086faf 100644 --- a/ghost/core/core/server/services/url/Resources.js +++ b/ghost/core/core/server/services/url/Resources.js @@ -128,12 +128,13 @@ class Resources { modelOptions.limit = options.limit; } + const now = Date.now(); const objects = await models.Base.Model.raw_knex.fetchAll(modelOptions); - debug('fetched', resourceConfig.type, objects.length); + debug('_fetch.fetched', resourceConfig.type, objects.length, `${Date.now() - now}ms`); - _.each(objects, (object) => { + for (const object of objects) { this.data[resourceConfig.type].push(new Resource(resourceConfig.type, object)); - }); + } if (objects.length && isSQLite) { options.offset = options.offset + options.limit; diff --git a/ghost/core/core/server/services/url/UrlGenerator.js b/ghost/core/core/server/services/url/UrlGenerator.js index 5320756b85..3ee0600279 100644 --- a/ghost/core/core/server/services/url/UrlGenerator.js +++ b/ghost/core/core/server/services/url/UrlGenerator.js @@ -1,4 +1,3 @@ -const _ = require('lodash'); const nql = require('@tryghost/nql'); const debug = require('@tryghost/debug')('services:url:generator'); const localUtils = require('../../../shared/url-utils'); @@ -121,16 +120,14 @@ class UrlGenerator { * @private */ _onInit() { - debug('_onInit', this.resourceType); - // @NOTE: get the resources of my type e.g. posts. const resources = this.resources.getAllByType(this.resourceType); - debug(resources.length); + debug('_onInit', this.resourceType, resources.length); - _.each(resources, (resource) => { + for (const resource of resources) { this._try(resource); - }); + } } /** @@ -141,7 +138,7 @@ class UrlGenerator { * @private */ _onAdded(event) { - debug('onAdded', this.toString()); + debug('_onAdded', this.toString()); // CASE: you are type "pages", but the incoming type is "users" if (event.type !== this.resourceType) { @@ -149,7 +146,6 @@ class UrlGenerator { } const resource = this.resources.getByIdAndType(event.type, event.id); - this._try(resource); } diff --git a/ghost/core/core/server/services/url/UrlService.js b/ghost/core/core/server/services/url/UrlService.js index c6ed4bb76b..0aba0c1b5e 100644 --- a/ghost/core/core/server/services/url/UrlService.js +++ b/ghost/core/core/server/services/url/UrlService.js @@ -1,5 +1,4 @@ -const _debug = require('@tryghost/debug')._base; -const debug = _debug('ghost:services:url:service'); +const debug = require('@tryghost/debug')('services:url:service'); const _ = require('lodash'); const errors = require('@tryghost/errors'); const labs = require('../../../shared/labs'); @@ -92,7 +91,7 @@ class UrlService { * @param {String} permalink */ onRouterAddedType(identifier, filter, resourceType, permalink) { - debug('Registering route: ', filter, resourceType, permalink); + debug('Registering route:', filter, resourceType, permalink); let urlGenerator = new UrlGenerator({ identifier, diff --git a/ghost/core/core/server/services/url/Urls.js b/ghost/core/core/server/services/url/Urls.js index 2e37db1636..6e973a1745 100644 --- a/ghost/core/core/server/services/url/Urls.js +++ b/ghost/core/core/server/services/url/Urls.js @@ -37,11 +37,8 @@ class Urls { * @param {string} options.url */ add(options) { - const url = options.url; - const generatorId = options.generatorId; - const resource = options.resource; - - debug('cache', url); + const {url, generatorId, resource} = options; + debug('add', resource.data.id, url); if (this.urls[resource.data.id]) { const error = new errors.InternalServerError({ @@ -131,7 +128,7 @@ class Urls { return; } - debug('removed', this.urls[id].url, this.urls[id].generatorId); + debug('removeResourceId', this.urls[id].url, this.urls[id].generatorId); events.emit('url.removed', { url: this.urls[id].url,