0
Fork 0
mirror of https://github.com/TryGhost/Ghost.git synced 2025-02-10 23:36:14 -05:00

Added Location header to API's POST request responses (#12186)

refs #2635

- Adds 'Location' header to endpoints which create new resources and have corresponding `GET` endpoint as speced in JSON API - https://jsonapi.org/format/#crud-creating-responses-201. Specifically:
    /posts/
    /pages/
    /integrations/
    /tags/
    /members/
    /labels/
    /notifications/
    /invites/

- Adding the header should allow for better resource discoverability and improved logging readability
- Added `url` property to the frame constructor. Data in `url` should give enough information  to later build up the `Location` header URL for created resource.
- Added Location header to headers handler. The Location value is built up from a combination of request URL and the id that is present in the response for the resource. The header is automatically added to requests coming to `add` controller methods which return `id` property in the frame result
- Excluded Webhooks API  as there is no "GET" endpoint available to fetch the resource
This commit is contained in:
naz 2020-09-14 22:33:37 +12:00 committed by GitHub
parent 50436656a7
commit cbdc91ce48
No known key found for this signature in database
GPG key ID: 4AEE18F83AFDEB23
16 changed files with 173 additions and 14 deletions

View file

@ -7,7 +7,10 @@ module.exports = {
add: {
statusCode: 201,
headers: {},
headers: {
// NOTE: remove if there is ever a 'read' method
location: false
},
options: [],
data: [],
permissions: true,

View file

@ -1,3 +1,4 @@
const url = require('url');
const debug = require('ghost-ignition').debug('api:shared:headers');
const Promise = require('bluebird');
const INVALIDATE_ALL = '/*';
@ -98,28 +99,53 @@ module.exports = {
* @description Get header based on ctrl configuration.
*
* @param {Object} result - API response
* @param {Object} apiConfig
* @param {Object} apiConfigHeaders
* @param {Object} frame
* @return {Promise}
*/
async get(result, apiConfig = {}) {
async get(result, apiConfigHeaders = {}, frame) {
let headers = {};
if (apiConfig.disposition) {
const dispositionHeader = await disposition[apiConfig.disposition.type](result, apiConfig.disposition);
if (apiConfigHeaders.disposition) {
const dispositionHeader = await disposition[apiConfigHeaders.disposition.type](result, apiConfigHeaders.disposition);
if (dispositionHeader) {
Object.assign(headers, dispositionHeader);
}
}
if (apiConfig.cacheInvalidate) {
const cacheInvalidationHeader = cacheInvalidate(result, apiConfig.cacheInvalidate);
if (apiConfigHeaders.cacheInvalidate) {
const cacheInvalidationHeader = cacheInvalidate(result, apiConfigHeaders.cacheInvalidate);
if (cacheInvalidationHeader) {
Object.assign(headers, cacheInvalidationHeader);
}
}
const locationHeaderDisabled = apiConfigHeaders && apiConfigHeaders.location === false;
const hasFrameData = frame
&& (frame.method === 'add')
&& result[frame.docName]
&& result[frame.docName][0]
&& result[frame.docName][0].id;
if (!locationHeaderDisabled && hasFrameData) {
const protocol = (frame.original.url.secure === false) ? 'http://' : 'https://';
const resourceId = result[frame.docName][0].id;
let locationURL = url.resolve(`${protocol}${frame.original.url.host}`,frame.original.url.pathname);
if (!locationURL.endsWith('/')) {
locationURL += '/';
}
locationURL += `${resourceId}/`;
const locationHeader = {
Location: locationURL
};
Object.assign(headers, locationHeader);
}
debug(headers);
return headers;
}

View file

@ -1,3 +1,4 @@
const url = require('url');
const debug = require('ghost-ignition').debug('api:shared:http');
const shared = require('../shared');
const models = require('../../models');
@ -41,6 +42,11 @@ const http = (apiImpl) => {
params: req.params,
user: req.user,
session: req.session,
url: {
host: req.vhost ? req.vhost.host : req.get('host'),
pathname: url.parse(req.originalUrl || req.url).pathname,
secure: req.secure
},
context: {
api_key: apiKey,
user: user,

View file

@ -7,7 +7,10 @@ module.exports = {
add: {
statusCode: 201,
headers: {},
headers: {
// NOTE: remove if there is ever a 'read' method
location: false
},
options: [],
data: [],
validation: {

View file

@ -65,14 +65,14 @@ describe('Integrations API', function () {
.expect('Content-Type', /json/)
.expect('Cache-Control', testUtils.cacheRules.private)
.expect(201)
.end(function (err, {body}) {
.end(function (err, res) {
if (err) {
return done(err);
}
should.equal(body.integrations.length, 1);
should.equal(res.body.integrations.length, 1);
const [integration] = body.integrations;
const [integration] = res.body.integrations;
should.equal(integration.name, 'Dis-Integrate!!');
should.equal(integration.api_keys.length, 2);
@ -91,6 +91,9 @@ describe('Integrations API', function () {
should.exist(secret);
secret.length.should.equal(64);
should.exist(res.headers.location);
res.headers.location.should.equal(`http://127.0.0.1:2369${localUtils.API.getApiQuery('integrations/')}${res.body.integrations[0].id}/`);
done();
});
});
@ -110,14 +113,14 @@ describe('Integrations API', function () {
.expect('Content-Type', /json/)
.expect('Cache-Control', testUtils.cacheRules.private)
.expect(201)
.end(function (err, {body}) {
.end(function (err, res) {
if (err) {
return done(err);
}
should.equal(body.integrations.length, 1);
should.equal(res.body.integrations.length, 1);
const [integration] = body.integrations;
const [integration] = res.body.integrations;
should.equal(integration.name, 'Integratatron4000');
should.equal(integration.webhooks.length, 1);
@ -125,6 +128,9 @@ describe('Integrations API', function () {
const webhook = integration.webhooks[0];
should.equal(webhook.integration_id, integration.id);
should.exist(res.headers.location);
res.headers.location.should.equal(`http://127.0.0.1:2369${localUtils.API.getApiQuery('integrations/')}${res.body.integrations[0].id}/`);
done();
});
});

View file

@ -116,6 +116,9 @@ describe('Invites API', function () {
mailService.GhostMailer.prototype.send.called.should.be.true();
should.exist(res.headers.location);
res.headers.location.should.equal(`http://127.0.0.1:2369${localUtils.API.getApiQuery('invites/')}${res.body.invites[0].id}/`);
done();
});
});

View file

@ -46,6 +46,9 @@ describe('Labels API', function () {
jsonResponse.labels.should.have.length(1);
jsonResponse.labels[0].name.should.equal(label.name);
jsonResponse.labels[0].slug.should.equal(label.name);
should.exist(res.headers.location);
res.headers.location.should.equal(`http://127.0.0.1:2369${localUtils.API.getApiQuery('labels/')}${res.body.labels[0].id}/`);
});
});
});

View file

@ -165,6 +165,9 @@ describe('Members API', function () {
jsonResponse.members[0].labels.length.should.equal(1);
jsonResponse.members[0].labels[0].name.should.equal('test-label');
should.exist(res.headers.location);
res.headers.location.should.equal(`http://127.0.0.1:2369${localUtils.API.getApiQuery('members/')}${res.body.members[0].id}/`);
})
.then(() => {
return request
@ -206,6 +209,9 @@ describe('Members API', function () {
should.exist(jsonResponse.members);
jsonResponse.members.should.have.length(1);
should.exist(res.headers.location);
res.headers.location.should.equal(`http://127.0.0.1:2369${localUtils.API.getApiQuery('members/')}${res.body.members[0].id}/`);
return jsonResponse.members[0];
})
.then((newMember) => {

View file

@ -49,6 +49,9 @@ describe('Notifications API', function () {
should.exist(jsonResponse.notifications[0].location);
jsonResponse.notifications[0].location.should.equal('bottom');
jsonResponse.notifications[0].id.should.be.a.String();
should.exist(res.headers.location);
res.headers.location.should.equal(`http://127.0.0.1:2369${localUtils.API.getApiQuery('notifications/')}${res.body.notifications[0].id}/`);
});
});

View file

@ -75,6 +75,9 @@ describe('Pages API', function () {
localUtils.API.checkResponse(res.body.pages[0], 'page');
should.exist(res.headers['x-cache-invalidate']);
should.exist(res.headers.location);
res.headers.location.should.equal(`http://127.0.0.1:2369${localUtils.API.getApiQuery('pages/')}${res.body.pages[0].id}/`);
return models.Post.findOne({
id: res.body.pages[0].id
}, testUtils.context.internal);

View file

@ -291,6 +291,9 @@ describe('Posts API', function () {
res.body.posts[0].url.should.match(new RegExp(`${config.get('url')}/p/[0-9a-f]{8}-[0-9a-f]{4}-[1-5][0-9a-f]{3}-[89ab][0-9a-f]{3}-[0-9a-f]{12}`));
should.not.exist(res.headers['x-cache-invalidate']);
should.exist(res.headers.location);
res.headers.location.should.equal(`http://127.0.0.1:2369${localUtils.API.getApiQuery('posts/')}${res.body.posts[0].id}/`);
return models.Post.findOne({
id: res.body.posts[0].id,
status: 'draft'

View file

@ -107,6 +107,9 @@ describe('Tag API', function () {
localUtils.API.checkResponse(jsonResponse.tags[0], 'tag', ['url']);
testUtils.API.isISO8601(jsonResponse.tags[0].created_at).should.be.true();
should.exist(res.headers.location);
res.headers.location.should.equal(`http://127.0.0.1:2369${localUtils.API.getApiQuery('tags/')}${res.body.tags[0].id}/`);
});
});
@ -132,6 +135,9 @@ describe('Tag API', function () {
jsonResponse.tags[0].visibility.should.eql('internal');
jsonResponse.tags[0].name.should.eql('#test');
jsonResponse.tags[0].slug.should.eql('hash-test');
should.exist(res.headers.location);
res.headers.location.should.equal(`http://127.0.0.1:2369${localUtils.API.getApiQuery('tags/')}${res.body.tags[0].id}/`);
});
});

View file

@ -50,6 +50,8 @@ describe('Webhooks API', function () {
jsonResponse.webhooks[0].name.should.equal(webhookData.name);
jsonResponse.webhooks[0].api_version.should.equal(webhookData.api_version);
jsonResponse.webhooks[0].integration_id.should.equal(webhookData.integration_id);
should.not.exist(res.headers.location);
});
});

View file

@ -140,6 +140,9 @@ describe('Posts API', function () {
should.exist(res.body.posts);
should.exist(res.body.posts[0].title);
res.body.posts[0].title.should.equal('(Untitled)');
should.exist(res.headers.location);
res.headers.location.should.equal(`http://127.0.0.1:2369${localUtils.API.getApiQuery('posts/')}${res.body.posts[0].id}/`);
});
});
});

View file

@ -61,4 +61,83 @@ describe('Unit: api/shared/headers', function () {
});
});
});
describe('location header', function () {
it('adds header when all needed data is present', function () {
const apiResult = {
posts: [{
id: 'id_value'
}]
};
const apiConfigHeaders = {};
const frame = {
docName: 'posts',
method: 'add',
original: {
url: {
host: 'example.com',
pathname: `/api/canary/posts/`
}
}
};
return shared.headers.get(apiResult, apiConfigHeaders, frame)
.then((result) => {
result.should.eql({
// NOTE: the backslash in the end is important to avoid unecessary 301s using the header
Location: 'https://example.com/api/canary/posts/id_value/'
});
});
});
it('adds and resolves header to correct url when pathname does not contain backslash in the end', function () {
const apiResult = {
posts: [{
id: 'id_value'
}]
};
const apiConfigHeaders = {};
const frame = {
docName: 'posts',
method: 'add',
original: {
url: {
host: 'example.com',
pathname: `/api/canary/posts`
}
}
};
return shared.headers.get(apiResult, apiConfigHeaders, frame)
.then((result) => {
result.should.eql({
// NOTE: the backslash in the end is important to avoid unecessary 301s using the header
Location: 'https://example.com/api/canary/posts/id_value/'
});
});
});
it('does not add header when missing result values', function () {
const apiResult = {};
const apiConfigHeaders = {};
const frame = {
docName: 'posts',
method: 'add',
original: {
url: {
host: 'example.com',
pathname: `/api/canary/posts/`
}
}
};
return shared.headers.get(apiResult, apiConfigHeaders, frame)
.then((result) => {
result.should.eql({});
});
});
});
});

View file

@ -15,6 +15,10 @@ describe('Unit: api/shared/http', function () {
req.body = {
a: 'a'
};
req.vhost = {
host: 'example.com'
};
req.url = 'https://example.com/ghost/api/canary/',
res.status = sinon.stub();
res.json = sinon.stub();