Skip to content

Commit

Permalink
Removed experimental URL cache
Browse files Browse the repository at this point in the history
fix https://linear.app/ghost/issue/ENG-1803/remove-url-cache-code

- this feature was intended to write a static cache of our URLs from the
  URL service, which should help boot become faster because we don't
  have to fetch them from the DB
- however, we added this a long time ago and never gave it the love to
  get it going correctly, so it's been sitting there unused for years
  and not providing any real benefit
- little bit of spring cleaning, we can clean up the code here and
  potentially revisit it in the future
  • Loading branch information
daniellockyer committed Feb 10, 2025
1 parent c7b631b commit db205ce
Show file tree
Hide file tree
Showing 24 changed files with 46 additions and 2,142 deletions.
8 changes: 3 additions & 5 deletions ghost/core/core/boot.js
Original file line number Diff line number Diff line change
Expand Up @@ -79,9 +79,8 @@ async function initDatabase({config}) {
* @param {object} options.ghostServer
* @param {object} options.config
* @param {object} options.bootLogger
* @param {boolean} options.frontend
*/
async function initCore({ghostServer, config, bootLogger, frontend}) {
async function initCore({ghostServer, config, bootLogger}) {
debug('Begin: initCore');

// URL Utils is a bit slow, put it here so the timing is visible separate from models
Expand Down Expand Up @@ -118,8 +117,7 @@ async function initCore({ghostServer, config, bootLogger, frontend}) {
onFinished: () => {
bootLogger.metric('url-service', urlServiceStart);
bootLogger.log('URL Service Ready');
},
urlCache: !frontend // hacky parameter to make the cache initialization kick in as we can't initialize labs before the boot
}
});
debug('End: Url Service');

Expand Down Expand Up @@ -563,7 +561,7 @@ async function bootGhost({backend = true, frontend = true, server = true} = {})

// Step 4 - Load Ghost with all its services
debug('Begin: Load Ghost Services & Apps');
await initCore({ghostServer, config, bootLogger, frontend});
await initCore({ghostServer, config, bootLogger});

// Instrument the knex instance and connection pool if prometheus is enabled
// Needs to be after initCore because the pool is destroyed and recreated in initCore, which removes the event listeners
Expand Down
75 changes: 0 additions & 75 deletions ghost/core/core/server/services/url/LocalFileCache.js

This file was deleted.

52 changes: 10 additions & 42 deletions ghost/core/core/server/services/url/UrlService.js
Original file line number Diff line number Diff line change
@@ -1,7 +1,6 @@
const debug = require('@tryghost/debug')('services:url:service');
const _ = require('lodash');
const errors = require('@tryghost/errors');
const labs = require('../../../shared/labs');
const UrlGenerator = require('./UrlGenerator');
const Queue = require('./Queue');
const Urls = require('./Urls');
Expand All @@ -15,16 +14,8 @@ const resourcesConfig = require('./config');
* It will tell you if the url generation is in progress or not.
*/
class UrlService {
/**
*
* @param {Object} [options]
* @param {Object} [options.cache] - cache handler instance
* @param {Function} [options.cache.read] - read cache by type
* @param {Function} [options.cache.write] - write into cache by type
*/
constructor({cache} = {}) {
constructor() {
this.utils = urlUtils;
this.cache = cache;
this.onFinished = null;
this.finished = false;
this.urlGenerators = [];
Expand Down Expand Up @@ -304,44 +295,21 @@ class UrlService {
* @description Initializes components needed for the URL Service to function
* @param {Object} options
* @param {Function} [options.onFinished] - callback when url generation is finished
* @param {Boolean} [options.urlCache] - whether to init using url cache or not
*/
async init({onFinished, urlCache} = {}) {
async init({onFinished} = {}) {
this.onFinished = onFinished;

let persistedUrls;
let persistedResources;

if (this.cache && (labs.isSet('urlCache') || urlCache)) {
persistedUrls = await this.cache.read('urls');
persistedResources = await this.cache.read('resources');
}

if (persistedUrls && persistedResources) {
this.urls.urls = persistedUrls;
this.resources.data = persistedResources;
this.resources.initEventListeners();

this._onQueueEnded('init');
} else {
this.resources.initEventListeners();
await this.resources.fetchResources();
// CASE: all resources are fetched, start the queue
this.queue.start({
event: 'init',
tolerance: 100,
requiredSubscriberCount: 1
});
}
this.resources.initEventListeners();
await this.resources.fetchResources();
// CASE: all resources are fetched, start the queue
this.queue.start({
event: 'init',
tolerance: 100,
requiredSubscriberCount: 1
});
}

async shutdown() {
if (!labs.isSet('urlCache')) {
return null;
}

await this.cache.write('urls', this.urls.urls);
await this.cache.write('resources', this.resources.getAll());
}

/**
Expand Down
22 changes: 1 addition & 21 deletions ghost/core/core/server/services/url/index.js
Original file line number Diff line number Diff line change
@@ -1,26 +1,6 @@
const config = require('../../../shared/config');
const LocalFileCache = require('./LocalFileCache');
const UrlService = require('./UrlService');

// NOTE: instead of a path we could give UrlService a "data-resolver" of some sort
// so it doesn't have to contain the logic to read data at all. This would be
// a possible improvement in the future
let writeDisabled = false;
let storagePath = config.getContentPath('data');

// TODO: remove this hack in favor of loading from the content path when it's possible to do so
// by mocking content folders in pre-boot phase
if (process.env.NODE_ENV.startsWith('test')){
storagePath = config.get('paths').urlCache;

// NOTE: prevents test suites from overwriting cache fixtures.
// A better solution would be injecting a different implementation of the
// cache based on the environment, this approach should do the trick for now
writeDisabled = true;
}

const cache = new LocalFileCache({storagePath, writeDisabled});
const urlService = new UrlService({cache});
const urlService = new UrlService();

// Singleton
module.exports = urlService;
Original file line number Diff line number Diff line change
Expand Up @@ -79,8 +79,7 @@
"useMinFiles": false,
"paths": {
"fixtures": "test/utils/fixtures/fixtures",
"defaultSettings": "test/utils/fixtures/default-settings-browser.json",
"urlCache": "test/utils/fixtures/urls"
"defaultSettings": "test/utils/fixtures/default-settings-browser.json"
},
"enableDeveloperExperiments": true
}
3 changes: 1 addition & 2 deletions ghost/core/core/shared/config/env/config.testing-mysql.json
Original file line number Diff line number Diff line change
Expand Up @@ -80,7 +80,6 @@
"useMinFiles": false,
"paths": {
"fixtures": "test/utils/fixtures/fixtures",
"defaultSettings": "test/utils/fixtures/default-settings.json",
"urlCache": "test/utils/fixtures/urls"
"defaultSettings": "test/utils/fixtures/default-settings.json"
}
}
3 changes: 1 addition & 2 deletions ghost/core/core/shared/config/env/config.testing.json
Original file line number Diff line number Diff line change
Expand Up @@ -76,7 +76,6 @@
"useMinFiles": false,
"paths": {
"fixtures": "test/utils/fixtures/fixtures",
"defaultSettings": "test/utils/fixtures/default-settings.json",
"urlCache": "test/utils/fixtures/urls"
"defaultSettings": "test/utils/fixtures/default-settings.json"
}
}
1 change: 0 additions & 1 deletion ghost/core/core/shared/labs.js
Original file line number Diff line number Diff line change
Expand Up @@ -44,7 +44,6 @@ const BETA_FEATURES = [

const ALPHA_FEATURES = [
'NestPlayground',
'urlCache',
'emailCustomization',
'mailEvents',
'collectionsCard',
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,6 @@ Object {
"postsX": true,
"stripeAutomaticTax": true,
"themeErrorsNotification": true,
"urlCache": true,
"webmentions": true,
},
"mail": "",
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -647,7 +647,7 @@ exports[`Pages API Convert can convert a mobiledoc page to lexical 2: [headers]
Object {
"access-control-allow-origin": "http://127.0.0.1:2369",
"cache-control": "no-cache, private, no-store, must-revalidate, max-stale=0, post-check=0, pre-check=0",
"content-length": "4063",
"content-length": "4069",
"content-type": "application/json; charset=utf-8",
"content-version": StringMatching /v\\\\d\\+\\\\\\.\\\\d\\+/,
"etag": StringMatching /\\(\\?:W\\\\/\\)\\?"\\(\\?:\\[ !#-\\\\x7E\\\\x80-\\\\xFF\\]\\*\\|\\\\r\\\\n\\[\\\\t \\]\\|\\\\\\\\\\.\\)\\*"/,
Expand Down Expand Up @@ -753,7 +753,7 @@ exports[`Pages API Convert can convert a mobiledoc page to lexical 4: [headers]
Object {
"access-control-allow-origin": "http://127.0.0.1:2369",
"cache-control": "no-cache, private, no-store, must-revalidate, max-stale=0, post-check=0, pre-check=0",
"content-length": "4063",
"content-length": "4069",
"content-type": "application/json; charset=utf-8",
"content-version": StringMatching /v\\\\d\\+\\\\\\.\\\\d\\+/,
"etag": StringMatching /\\(\\?:W\\\\/\\)\\?"\\(\\?:\\[ !#-\\\\x7E\\\\x80-\\\\xFF\\]\\*\\|\\\\r\\\\n\\[\\\\t \\]\\|\\\\\\\\\\.\\)\\*"/,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -874,7 +874,7 @@ exports[`Posts API Convert can convert a mobiledoc post to lexical 2: [headers]
Object {
"access-control-allow-origin": "http://127.0.0.1:2369",
"cache-control": "no-cache, private, no-store, must-revalidate, max-stale=0, post-check=0, pre-check=0",
"content-length": "4100",
"content-length": "4108",
"content-type": "application/json; charset=utf-8",
"content-version": StringMatching /v\\\\d\\+\\\\\\.\\\\d\\+/,
"etag": StringMatching /\\(\\?:W\\\\/\\)\\?"\\(\\?:\\[ !#-\\\\x7E\\\\x80-\\\\xFF\\]\\*\\|\\\\r\\\\n\\[\\\\t \\]\\|\\\\\\\\\\.\\)\\*"/,
Expand Down Expand Up @@ -983,7 +983,7 @@ exports[`Posts API Convert can convert a mobiledoc post to lexical 4: [headers]
Object {
"access-control-allow-origin": "http://127.0.0.1:2369",
"cache-control": "no-cache, private, no-store, must-revalidate, max-stale=0, post-check=0, pre-check=0",
"content-length": "4100",
"content-length": "4108",
"content-type": "application/json; charset=utf-8",
"content-version": StringMatching /v\\\\d\\+\\\\\\.\\\\d\\+/,
"etag": StringMatching /\\(\\?:W\\\\/\\)\\?"\\(\\?:\\[ !#-\\\\x7E\\\\x80-\\\\xFF\\]\\*\\|\\\\r\\\\n\\[\\\\t \\]\\|\\\\\\\\\\.\\)\\*"/,
Expand Down
2 changes: 2 additions & 0 deletions ghost/core/test/e2e-api/admin/sso.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,8 @@ describe('SSO API', function () {
let agent;

before(async function () {
models.init();

// Configure mock SSO adapter that always returns owner
const owner = await models.User.getOwnerUser();

Expand Down
7 changes: 1 addition & 6 deletions ghost/core/test/e2e-api/admin/utils.js
Original file line number Diff line number Diff line change
Expand Up @@ -243,11 +243,6 @@ module.exports = {
},

async startGhost(overrides = {}) {
const defaults = {
backend: true,
frontend: false
};

return await testUtils.startGhost(Object.assign(defaults, overrides));
return await testUtils.startGhost(overrides);
}
};
Original file line number Diff line number Diff line change
Expand Up @@ -1322,7 +1322,7 @@ Most successful subscription businesses publish a mix of free and paid posts to
"profile_image": null,
"slug": "slimer-mcectoplasm",
"twitter": null,
"url": "http://127.0.0.1:2369/404/",
"url": "http://127.0.0.1:2369/author/slimer-mcectoplasm/",
"website": null,
},
],
Expand Down Expand Up @@ -1616,7 +1616,7 @@ exports[`Posts Content API Can filter posts by authors 2: [headers] 1`] = `
Object {
"access-control-allow-origin": "*",
"cache-control": "public, max-age=0",
"content-length": "55184",
"content-length": "55206",
"content-type": "application/json; charset=utf-8",
"content-version": StringMatching /v\\\\d\\+\\\\\\.\\\\d\\+/,
"etag": StringMatching /\\(\\?:W\\\\/\\)\\?"\\(\\?:\\[ !#-\\\\x7E\\\\x80-\\\\xFF\\]\\*\\|\\\\r\\\\n\\[\\\\t \\]\\|\\\\\\\\\\.\\)\\*"/,
Expand Down Expand Up @@ -2878,7 +2878,7 @@ exports[`Posts Content API Can include relations 2: [headers] 1`] = `
Object {
"access-control-allow-origin": "*",
"cache-control": "public, max-age=0",
"content-length": "65513",
"content-length": "65535",
"content-type": "application/json; charset=utf-8",
"content-version": StringMatching /v\\\\d\\+\\\\\\.\\\\d\\+/,
"etag": StringMatching /\\(\\?:W\\\\/\\)\\?"\\(\\?:\\[ !#-\\\\x7E\\\\x80-\\\\xFF\\]\\*\\|\\\\r\\\\n\\[\\\\t \\]\\|\\\\\\\\\\.\\)\\*"/,
Expand Down
7 changes: 1 addition & 6 deletions ghost/core/test/e2e-api/content/utils.js
Original file line number Diff line number Diff line change
Expand Up @@ -97,11 +97,6 @@ module.exports = {
return testUtils.DataGenerator.Content.api_keys[1].secret;
},
async startGhost(overrides = {}) {
const defaults = {
backend: true,
frontend: false
};

return await testUtils.startGhost(Object.assign(defaults, overrides));
return await testUtils.startGhost(overrides);
}
};
4 changes: 2 additions & 2 deletions ghost/core/test/e2e-webhooks/__snapshots__/posts.test.js.snap
Original file line number Diff line number Diff line change
Expand Up @@ -511,7 +511,7 @@ Object {
"tour": null,
"twitter": null,
"updated_at": StringMatching /\\\\d\\{4\\}-\\\\d\\{2\\}-\\\\d\\{2\\}T\\\\d\\{2\\}:\\\\d\\{2\\}:\\\\d\\{2\\}\\\\\\.000Z/,
"url": "http://127.0.0.1:2369/author/joe-bloggs/",
"url": "http://127.0.0.1:2369/404/",
"website": null,
},
],
Expand Down Expand Up @@ -602,7 +602,7 @@ Header Level 3
"tour": null,
"twitter": null,
"updated_at": StringMatching /\\\\d\\{4\\}-\\\\d\\{2\\}-\\\\d\\{2\\}T\\\\d\\{2\\}:\\\\d\\{2\\}:\\\\d\\{2\\}\\\\\\.000Z/,
"url": "http://127.0.0.1:2369/author/joe-bloggs/",
"url": "http://127.0.0.1:2369/404/",
"website": null,
},
"primary_tag": null,
Expand Down
Loading

0 comments on commit db205ce

Please sign in to comment.