Moved content-version middleware onto api app

closes: https://github.com/TryGhost/Toolbox/issues/319

- at the moment, content-version is only set if one of our endpoints touches the request
  - this was demonstrated in the e2e tests, where many of the tests that set accept-version did not receive accept-version
- by moving the middleware out of the http module and onto the api app we ensure it's always done
- I put the code in the api-version-compatibility service to keep it all co-located
- ideally we will refactor that service slightly so it only exposes middleware
This commit is contained in:
Hannah Wolfe 2022-05-02 17:01:54 +01:00
parent 55ce208ebb
commit c6ae3c30d8
6 changed files with 32 additions and 33 deletions

View File

@ -82,9 +82,6 @@ const http = (apiImpl) => {
res.status(statusCode);
// CASE: generate headers based on the api ctrl configuration
if (req && req.headers && req.headers['accept-version'] && res.locals) {
headers['Content-Version'] = `v${res.locals.safeVersion}`;
}
res.set(headers);
const send = (format) => {

View File

@ -5,6 +5,7 @@ const versionMismatchHandler = require('@tryghost/mw-api-version-mismatch');
const settingsService = require('../../services/settings');
const models = require('../../models');
const logging = require('@tryghost/logging');
const ghostVersion = require('@tryghost/version');
let serviceInstance;
@ -31,4 +32,12 @@ const init = () => {
module.exports.errorHandler = (req, res, next) => {
return versionMismatchHandler(serviceInstance)(req, res, next);
};
module.exports.contentVersion = (req, res, next) => {
if (req.header('accept-version')) {
res.header('Content-Version', `v${ghostVersion.safe}`);
}
next();
};
module.exports.init = init;

View File

@ -26,6 +26,8 @@ module.exports = function setupApiApp() {
res.redirect(307, versionlessURL);
});
apiApp.use(APIVersionCompatibilityService.contentVersion);
apiApp.lazyUse('/content/', require('./canary/content/app'));
apiApp.lazyUse('/admin/', require('./canary/admin/app'));

View File

@ -56,6 +56,7 @@ Object {
"cache-control": "no-cache, private, no-store, must-revalidate, max-stale=0, post-check=0, pre-check=0",
"content-length": "224",
"content-type": "application/json; charset=utf-8",
"content-version": StringMatching /v\\\\d\\+\\\\\\.\\\\d\\+/,
"etag": StringMatching /\\(\\?:W\\\\/\\)\\?"\\(\\?:\\[ !#-\\\\x7E\\\\x80-\\\\xFF\\]\\*\\|\\\\r\\\\n\\[\\\\t \\]\\|\\\\\\\\\\.\\)\\*"/,
"vary": "Origin, Accept-Encoding",
"x-powered-by": "Express",
@ -84,6 +85,7 @@ Object {
"cache-control": "no-cache, private, no-store, must-revalidate, max-stale=0, post-check=0, pre-check=0",
"content-length": "347",
"content-type": "application/json; charset=utf-8",
"content-version": StringMatching /v\\\\d\\+\\\\\\.\\\\d\\+/,
"etag": StringMatching /\\(\\?:W\\\\/\\)\\?"\\(\\?:\\[ !#-\\\\x7E\\\\x80-\\\\xFF\\]\\*\\|\\\\r\\\\n\\[\\\\t \\]\\|\\\\\\\\\\.\\)\\*"/,
"vary": "Accept-Encoding",
"x-powered-by": "Express",
@ -112,6 +114,7 @@ Object {
"cache-control": "no-cache, private, no-store, must-revalidate, max-stale=0, post-check=0, pre-check=0",
"content-length": "354",
"content-type": "application/json; charset=utf-8",
"content-version": StringMatching /v\\\\d\\+\\\\\\.\\\\d\\+/,
"etag": StringMatching /\\(\\?:W\\\\/\\)\\?"\\(\\?:\\[ !#-\\\\x7E\\\\x80-\\\\xFF\\]\\*\\|\\\\r\\\\n\\[\\\\t \\]\\|\\\\\\\\\\.\\)\\*"/,
"vary": "Accept-Encoding",
"x-powered-by": "Express",
@ -195,6 +198,7 @@ Object {
"cache-control": "no-cache, private, no-store, must-revalidate, max-stale=0, post-check=0, pre-check=0",
"content-length": "354",
"content-type": "application/json; charset=utf-8",
"content-version": StringMatching /v\\\\d\\+\\\\\\.\\\\d\\+/,
"etag": StringMatching /\\(\\?:W\\\\/\\)\\?"\\(\\?:\\[ !#-\\\\x7E\\\\x80-\\\\xFF\\]\\*\\|\\\\r\\\\n\\[\\\\t \\]\\|\\\\\\\\\\.\\)\\*"/,
"vary": "Origin, Accept-Encoding",
"x-powered-by": "Express",
@ -224,6 +228,7 @@ Object {
"cache-control": "no-cache, private, no-store, must-revalidate, max-stale=0, post-check=0, pre-check=0",
"content-length": "354",
"content-type": "application/json; charset=utf-8",
"content-version": StringMatching /v\\\\d\\+\\\\\\.\\\\d\\+/,
"etag": StringMatching /\\(\\?:W\\\\/\\)\\?"\\(\\?:\\[ !#-\\\\x7E\\\\x80-\\\\xFF\\]\\*\\|\\\\r\\\\n\\[\\\\t \\]\\|\\\\\\\\\\.\\)\\*"/,
"vary": "Origin, Accept-Encoding",
"x-powered-by": "Express",
@ -253,6 +258,7 @@ Object {
"cache-control": "no-cache, private, no-store, must-revalidate, max-stale=0, post-check=0, pre-check=0",
"content-length": "348",
"content-type": "application/json; charset=utf-8",
"content-version": StringMatching /v\\\\d\\+\\\\\\.\\\\d\\+/,
"etag": StringMatching /\\(\\?:W\\\\/\\)\\?"\\(\\?:\\[ !#-\\\\x7E\\\\x80-\\\\xFF\\]\\*\\|\\\\r\\\\n\\[\\\\t \\]\\|\\\\\\\\\\.\\)\\*"/,
"vary": "Origin, Accept-Encoding",
"x-powered-by": "Express",
@ -282,6 +288,7 @@ Object {
"cache-control": "no-cache, private, no-store, must-revalidate, max-stale=0, post-check=0, pre-check=0",
"content-length": "354",
"content-type": "application/json; charset=utf-8",
"content-version": StringMatching /v\\\\d\\+\\\\\\.\\\\d\\+/,
"etag": StringMatching /\\(\\?:W\\\\/\\)\\?"\\(\\?:\\[ !#-\\\\x7E\\\\x80-\\\\xFF\\]\\*\\|\\\\r\\\\n\\[\\\\t \\]\\|\\\\\\\\\\.\\)\\*"/,
"vary": "Origin, Accept-Encoding",
"x-powered-by": "Express",

View File

@ -72,7 +72,8 @@ describe('API Versioning', function () {
.header('Accept-Version', 'v999.1')
.expectStatus(406)
.matchHeaderSnapshot({
etag: anyEtag
etag: anyEtag,
'content-version': stringMatching(/v\d+\.\d+/)
})
.matchBodySnapshot({
errors: [{
@ -90,7 +91,8 @@ describe('API Versioning', function () {
.header('User-Agent', 'Zapier 1.3')
.expectStatus(406)
.matchHeaderSnapshot({
etag: anyEtag
etag: anyEtag,
'content-version': stringMatching(/v\d+\.\d+/)
})
.matchBodySnapshot({
errors: [{
@ -114,7 +116,8 @@ describe('API Versioning', function () {
.header('User-Agent', 'Zapier 1.4')
.expectStatus(406)
.matchHeaderSnapshot({
etag: anyEtag
etag: anyEtag,
'content-version': stringMatching(/v\d+\.\d+/)
})
.matchBodySnapshot({
errors: [{
@ -136,7 +139,8 @@ describe('API Versioning', function () {
.header('User-Agent', 'Zapier 1.4')
.expectStatus(406)
.matchHeaderSnapshot({
etag: anyEtag
etag: anyEtag,
'content-version': stringMatching(/v\d+\.\d+/)
})
.matchBodySnapshot({
errors: [{
@ -155,7 +159,8 @@ describe('API Versioning', function () {
.header('Accept-Version', 'v4.1')
.expectStatus(404)
.matchHeaderSnapshot({
etag: anyEtag
etag: anyEtag,
'content-version': stringMatching(/v\d+\.\d+/)
})
.matchBodySnapshot({
errors: [{
@ -190,7 +195,8 @@ describe('API Versioning', function () {
.header('Accept-Version', 'v99.0')
.expectStatus(406)
.matchHeaderSnapshot({
etag: anyEtag
etag: anyEtag,
'content-version': stringMatching(/v\d+\.\d+/)
})
.matchBodySnapshot({errors: [{
context: stringMatching(/Provided client version v99\.0 is ahead of current Ghost instance version v\d+\.\d+/),
@ -204,7 +210,8 @@ describe('API Versioning', function () {
.header('Accept-Version', 'v1.0')
.expectStatus(406)
.matchHeaderSnapshot({
etag: anyEtag
etag: anyEtag,
'content-version': stringMatching(/v\d+\.\d+/)
})
.matchBodySnapshot({errors: [{
context: stringMatching(/Provided client version v1\.0 is outdated and is behind current Ghost version v\d+\.\d+/),

View File

@ -87,27 +87,4 @@ describe('Unit: api/shared/http', function () {
shared.http(apiImpl)(req, res, next);
});
it('adds content-version header to the response when accept-version header is present in the request', function (done) {
const apiImpl = sinon.stub().resolves('data');
req.headers = {
'accept-version': 'v5.1'
};
apiImpl.headers = {
'Content-Type': 'application/json'
};
res.locals = {
safeVersion: '5.4'
};
next.callsFake(done);
res.json.callsFake(function () {
shared.headers.get.calledOnce.should.be.true();
res.status.calledOnce.should.be.true();
res.headers['Content-Version'].should.equal('v5.4');
done();
});
shared.http(apiImpl)(req, res, next);
});
});