From ec697051dc3bae13c240cb4fcadd98263a9921a4 Mon Sep 17 00:00:00 2001 From: Fabien O'Carroll Date: Tue, 27 Feb 2024 16:15:24 -0500 Subject: [PATCH] =?UTF-8?q?=F0=9F=90=9B=20Fixed=20cache=20invalidation=20h?= =?UTF-8?q?eader=20race=20conditions?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit refs https://linear.app/tryghost/issue/ENG-674/ This ensures that all of our dynamic cache invalidation header logic is applied on a per-request basis! --- ghost/core/core/server/api/endpoints/collections.js | 11 +---------- ghost/core/core/server/api/endpoints/labels.js | 4 +--- ghost/core/core/server/api/endpoints/pages.js | 10 ++++++++-- ghost/core/core/server/api/endpoints/posts.js | 13 ++++++++----- ghost/core/core/server/api/endpoints/schedules.js | 9 +++++++-- ghost/core/core/server/api/endpoints/settings.js | 8 +++----- ghost/core/core/server/api/endpoints/tags.js | 4 +--- ghost/core/core/server/api/endpoints/themes.js | 5 ++--- ghost/core/core/server/api/endpoints/users.js | 4 +++- .../services/posts/post-scheduling-service.js | 2 +- 10 files changed, 35 insertions(+), 35 deletions(-) diff --git a/ghost/core/core/server/api/endpoints/collections.js b/ghost/core/core/server/api/endpoints/collections.js index 2bf31facbe..7885775bb1 100644 --- a/ghost/core/core/server/api/endpoints/collections.js +++ b/ghost/core/core/server/api/endpoints/collections.js @@ -87,7 +87,7 @@ module.exports = { edit: { headers: { - cacheInvalidate: false + cacheInvalidate: true }, options: [ 'id' @@ -111,15 +111,6 @@ module.exports = { }); } - // @NOTE: cache invalidation has to be done manually for now - // because the model's wasChanged is not returned from - // the service using in-memory repository layer - // if (model.wasChanged()) { - this.headers.cacheInvalidate = true; - // } else { - // this.headers.cacheInvalidate = false; - // } - return model; } }, diff --git a/ghost/core/core/server/api/endpoints/labels.js b/ghost/core/core/server/api/endpoints/labels.js index 80f97b3425..56b10f7637 100644 --- a/ghost/core/core/server/api/endpoints/labels.js +++ b/ghost/core/core/server/api/endpoints/labels.js @@ -129,9 +129,7 @@ module.exports = { } if (model.wasChanged()) { - this.headers.cacheInvalidate = true; - } else { - this.headers.cacheInvalidate = false; + frame.setHeader('X-Cache-Invalidate', '/*'); } return model; diff --git a/ghost/core/core/server/api/endpoints/pages.js b/ghost/core/core/server/api/endpoints/pages.js index bd5661a93b..38c7d29ee4 100644 --- a/ghost/core/core/server/api/endpoints/pages.js +++ b/ghost/core/core/server/api/endpoints/pages.js @@ -122,7 +122,7 @@ module.exports = { return models.Post.add(frame.data.pages[0], frame.options) .then((model) => { if (model.get('status') === 'published') { - this.headers.cacheInvalidate = true; + frame.setHeader('X-Cache-Invalidate', '/*'); } return model; @@ -166,7 +166,13 @@ module.exports = { async query(frame) { const model = await models.Post.edit(frame.data.pages[0], frame.options); - this.headers.cacheInvalidate = postsService.handleCacheInvalidation(model); + const cacheInvalidation = postsService.handleCacheInvalidation(model); + + if (cacheInvalidation === true) { + frame.setHeader('X-Cache-Invalidate', '/*'); + } else if (cacheInvalidation.value) { + frame.setHeader('X-Cache-Invalidate', cacheInvalidation.value); + } return model; } diff --git a/ghost/core/core/server/api/endpoints/posts.js b/ghost/core/core/server/api/endpoints/posts.js index cc71f529e8..6ec3174aa3 100644 --- a/ghost/core/core/server/api/endpoints/posts.js +++ b/ghost/core/core/server/api/endpoints/posts.js @@ -167,10 +167,8 @@ module.exports = { query(frame) { return models.Post.add(frame.data.posts[0], frame.options) .then((model) => { - if (model.get('status') !== 'published') { - this.headers.cacheInvalidate = false; - } else { - this.headers.cacheInvalidate = true; + if (model.get('status') === 'published') { + frame.setHeader('X-Cache-Invalidate', '/*'); } return model; @@ -216,7 +214,12 @@ module.exports = { async query(frame) { let model = await postsService.editPost(frame, { eventHandler: (event, dto) => { - this.headers.cacheInvalidate = getCacheHeaderFromEventString(event, dto); + const cacheInvalidate = getCacheHeaderFromEventString(event, dto); + if (cacheInvalidate === true) { + frame.setHeader('X-Cache-Invalidate', '/*'); + } else if (cacheInvalidate?.value) { + frame.setHeader('X-Cache-Invalidate', cacheInvalidate.value); + } } }); diff --git a/ghost/core/core/server/api/endpoints/schedules.js b/ghost/core/core/server/api/endpoints/schedules.js index 52a856bc92..de57ba6a7d 100644 --- a/ghost/core/core/server/api/endpoints/schedules.js +++ b/ghost/core/core/server/api/endpoints/schedules.js @@ -40,8 +40,13 @@ module.exports = { }; const {scheduledResource, preScheduledResource} = await postSchedulingService.publish(resourceType, frame.options.id, frame.data.force, options); - const cacheInvalidate = postSchedulingService.handleCacheInvalidation(scheduledResource, preScheduledResource); - this.headers.cacheInvalidate = cacheInvalidate; + const cacheInvalidation = postSchedulingService.handleCacheInvalidation(scheduledResource, preScheduledResource); + + if (cacheInvalidation === true) { + frame.setHeader('X-Cache-Invalidate', '/*'); + } else if (cacheInvalidation.value) { + frame.setHeader('X-Cache-Invalidate', cacheInvalidation.value); + } const response = {}; response[resourceType] = [scheduledResource]; diff --git a/ghost/core/core/server/api/endpoints/settings.js b/ghost/core/core/server/api/endpoints/settings.js index b664748d47..a4ad985569 100644 --- a/ghost/core/core/server/api/endpoints/settings.js +++ b/ghost/core/core/server/api/endpoints/settings.js @@ -122,7 +122,7 @@ module.exports = { edit: { headers: { - cacheInvalidate: true + cacheInvalidate: false }, permissions: { unsafeAttrsObject(frame) { @@ -134,10 +134,8 @@ module.exports = { let result = await settingsBREADService.edit(frame.data.settings, frame.options, stripeConnectData); - if (_.isEmpty(result)) { - this.headers.cacheInvalidate = false; - } else { - this.headers.cacheInvalidate = true; + if (!_.isEmpty(result)) { + frame.setHeader('X-Cache-Invalidate', '/*'); } // We need to return all settings here, because we have calculated settings that might change diff --git a/ghost/core/core/server/api/endpoints/tags.js b/ghost/core/core/server/api/endpoints/tags.js index 97ef22f787..9d4485979a 100644 --- a/ghost/core/core/server/api/endpoints/tags.js +++ b/ghost/core/core/server/api/endpoints/tags.js @@ -124,9 +124,7 @@ module.exports = { } if (model.wasChanged()) { - this.headers.cacheInvalidate = true; - } else { - this.headers.cacheInvalidate = false; + frame.setHeader('X-Cache-Invalidate', '/*'); } return model; diff --git a/ghost/core/core/server/api/endpoints/themes.js b/ghost/core/core/server/api/endpoints/themes.js index 5cdfa7ab7c..e2ca3b5844 100644 --- a/ghost/core/core/server/api/endpoints/themes.js +++ b/ghost/core/core/server/api/endpoints/themes.js @@ -91,7 +91,7 @@ module.exports = { const {theme, themeOverridden} = await themeService.api.installFromGithub(frame.options.ref); if (themeOverridden) { - this.headers.cacheInvalidate = true; + frame.setHeader('X-Cache-Invalidate', '/*'); } events.emit('theme.uploaded', {name: theme.name}); @@ -125,8 +125,7 @@ module.exports = { return themeService.api.setFromZip(zip) .then(({theme, themeOverridden}) => { if (themeOverridden) { - // CASE: clear cache - this.headers.cacheInvalidate = true; + frame.setHeader('X-Cache-Invalidate', '/*'); } events.emit('theme.uploaded', {name: theme.name}); return theme; diff --git a/ghost/core/core/server/api/endpoints/users.js b/ghost/core/core/server/api/endpoints/users.js index 5fd7625d3e..4f6d68dad1 100644 --- a/ghost/core/core/server/api/endpoints/users.js +++ b/ghost/core/core/server/api/endpoints/users.js @@ -170,7 +170,9 @@ module.exports = { })); } - this.headers.cacheInvalidate = shouldInvalidateCacheAfterChange(model); + if (shouldInvalidateCacheAfterChange(model)) { + frame.setHeader('X-Cache-Invalidate', '/*'); + } return model; }); diff --git a/ghost/core/core/server/services/posts/post-scheduling-service.js b/ghost/core/core/server/services/posts/post-scheduling-service.js index accfe367d5..46f7b8dff0 100644 --- a/ghost/core/core/server/services/posts/post-scheduling-service.js +++ b/ghost/core/core/server/services/posts/post-scheduling-service.js @@ -55,7 +55,7 @@ class PostSchedulingService { * * @param {Object} scheduledResource post or page resource object * @param {Object} preScheduledResource post or page resource object in state before publishing - * @returns {Boolean|Object} + * @returns {boolean|{value: string}} */ handleCacheInvalidation(scheduledResource, preScheduledResource) { if (