From 3bd4d0989ae6bc65576684c71ab5a649b03377c7 Mon Sep 17 00:00:00 2001 From: Hannah Wolfe Date: Mon, 21 Mar 2022 11:46:41 +0000 Subject: [PATCH] Added default serializer + handling refs: https://github.com/TryGhost/Toolbox/issues/245 - Added a serializer called default to the canary API - Ideally, this would be part of the shared framework, but this would change v2/v3 and we're about to get rid of them - Therefore, we change just canary for now, and we can refactor again later. - Added wiring to handler that uses the default serializer, if there is a default, and isn't an explicit serializer for the endpoint - Removed the invites serializer, so that one endpoint now uses the default Note: previous commits have added explicit serializers to every endpoint, this is the first step towards paring that back so that we have less serializers overall, not more! --- .../utils/serializers/output/default.js | 35 ++++ .../canary/utils/serializers/output/index.js | 8 +- .../utils/serializers/output/invites.js | 24 --- core/server/api/shared/serializers/handle.js | 19 +- .../utils/serializers/output/default.test.js | 192 ++++++++++++++++++ .../api/shared/serializers/handle.test.js | 44 ++++ 6 files changed, 292 insertions(+), 30 deletions(-) create mode 100644 core/server/api/canary/utils/serializers/output/default.js delete mode 100644 core/server/api/canary/utils/serializers/output/invites.js create mode 100644 test/unit/api/canary/utils/serializers/output/default.test.js diff --git a/core/server/api/canary/utils/serializers/output/default.js b/core/server/api/canary/utils/serializers/output/default.js new file mode 100644 index 0000000000..cf4e34012b --- /dev/null +++ b/core/server/api/canary/utils/serializers/output/default.js @@ -0,0 +1,35 @@ +const debug = require('@tryghost/debug')('api:canary:utils:serializers:output:default'); +const mappers = require('./mappers'); + +const mapResponse = (docName, mappable, frame) => { + if (mappers[docName]) { + return mappers[docName](mappable, frame); + } else if (mappable.toJSON) { + return mappable.toJSON(frame.options); + } + + return mappable; +}; + +module.exports = { + all(response, apiConfig, frame) { + const {docName, method} = apiConfig; + debug('serializing', docName, method); + + if (!response) { + return; + } + + frame.response = {}; + + if (response.data) { + frame.response[docName] = response.data.map(model => mapResponse(docName, model, frame)); + } else { + frame.response[docName] = [mapResponse(docName, response, frame)]; + } + + if (response.meta) { + frame.response.meta = response.meta; + } + } +}; diff --git a/core/server/api/canary/utils/serializers/output/index.js b/core/server/api/canary/utils/serializers/output/index.js index 4dd7123bbd..47d30d120e 100644 --- a/core/server/api/canary/utils/serializers/output/index.js +++ b/core/server/api/canary/utils/serializers/output/index.js @@ -9,6 +9,10 @@ module.exports = { return require('./all'); }, + get default() { + return require('./default'); + }, + get authentication() { return require('./authentication'); }, @@ -49,10 +53,6 @@ module.exports = { return require('./posts'); }, - get invites() { - return require('./invites'); - }, - get settings() { return require('./settings'); }, diff --git a/core/server/api/canary/utils/serializers/output/invites.js b/core/server/api/canary/utils/serializers/output/invites.js deleted file mode 100644 index 6eba16d383..0000000000 --- a/core/server/api/canary/utils/serializers/output/invites.js +++ /dev/null @@ -1,24 +0,0 @@ -const debug = require('@tryghost/debug')('api:canary:utils:serializers:output:invites'); - -module.exports = { - all(models, apiConfig, frame) { - debug('all'); - - if (!models) { - return; - } - - if (models.meta) { - frame.response = { - invites: models.data.map(model => model.toJSON(frame.options)), - meta: models.meta - }; - - return; - } - - frame.response = { - invites: [models.toJSON(frame.options)] - }; - } -}; diff --git a/core/server/api/shared/serializers/handle.js b/core/server/api/shared/serializers/handle.js index 77fb7f782e..9583c7d338 100644 --- a/core/server/api/shared/serializers/handle.js +++ b/core/server/api/shared/serializers/handle.js @@ -101,18 +101,33 @@ module.exports.output = (response = {}, apiConfig, apiSerializers, frame) => { }); } + // CASE: custom serializer exists if (apiSerializers[apiConfig.docName]) { if (apiSerializers[apiConfig.docName].all) { - tasks.push(function serializeOptionsShared() { + tasks.push(function serialiseCustomAll() { return apiSerializers[apiConfig.docName].all(response, apiConfig, frame); }); } if (apiSerializers[apiConfig.docName][apiConfig.method]) { - tasks.push(function serializeOptionsShared() { + tasks.push(function serialiseCustomMethod() { return apiSerializers[apiConfig.docName][apiConfig.method](response, apiConfig, frame); }); } + + // CASE: Fall back to default serializer + } else if (apiSerializers.default) { + if (apiSerializers.default.all) { + tasks.push(function serializeDefaultAll() { + return apiSerializers.default.all(response, apiConfig, frame); + }); + } + + if (apiSerializers.default[apiConfig.method]) { + tasks.push(function serializeDefaultMethod() { + return apiSerializers.default[apiConfig.method](response, apiConfig, frame); + }); + } } if (apiSerializers.all && apiSerializers.all.after) { diff --git a/test/unit/api/canary/utils/serializers/output/default.test.js b/test/unit/api/canary/utils/serializers/output/default.test.js new file mode 100644 index 0000000000..6fd3210f77 --- /dev/null +++ b/test/unit/api/canary/utils/serializers/output/default.test.js @@ -0,0 +1,192 @@ +const should = require('should'); +const sinon = require('sinon'); +const serializers = require('../../../../../../../core/server/api/canary/utils/serializers'); +const mappers = require('../../../../../../../core/server/api/canary/utils/serializers/output/mappers'); + +describe('Unit: canary/utils/serializers/output/default', function () { + let toJSONStub; + beforeEach(function () { + toJSONStub = sinon.stub().callsFake(function () { + return {title: this.title, hello: 'world'}; + }); + }); + + afterEach(function () { + sinon.restore(); + }); + + it('.all() can map a pojo with one object', function () { + const response = { + title: 'foo' + }; + + const apiConfig = { + docName: 'stuffs' + }; + const frame = {}; + + serializers.output.default.all(response, apiConfig, frame); + + frame.response.should.eql({ + stuffs: [response] + }); + }); + + it('.all() can map a pojo of many objects', function () { + const response = { + data: [ + { + title: 'foo' + }, + { + title: 'bar' + } + ], + meta: { + total: 2 + } + }; + + const apiConfig = { + docName: 'stuffs' + }; + const frame = {}; + + serializers.output.default.all(response, apiConfig, frame); + + frame.response.should.eql({ + stuffs: response.data, + meta: response.meta + }); + }); + + it('.all() can map a single Bookshelf model', function () { + const response = { + toJSON: toJSONStub, + title: 'foo' + }; + + const apiConfig = { + docName: 'stuffs' + }; + const frame = {}; + + serializers.output.default.all(response, apiConfig, frame); + + frame.response.should.eql({ + stuffs: [ + {title: 'foo', hello: 'world'} + ] + }); + + sinon.assert.calledOnce(toJSONStub); + }); + + it('.all() can map a Bookshelf collection', function () { + const response = { + data: [ + { + toJSON: toJSONStub, + title: 'foo' + }, + { + toJSON: toJSONStub, + title: 'bar' + } + ], + meta: { + total: 2 + } + }; + + const apiConfig = { + docName: 'stuffs' + }; + const frame = {}; + + serializers.output.default.all(response, apiConfig, frame); + + frame.response.should.eql({ + stuffs: [ + {title: 'foo', hello: 'world'}, + {title: 'bar', hello: 'world'} + ], + meta: response.meta + }); + + sinon.assert.calledTwice(toJSONStub); + }); + + it('.all() can map a single Bookshelf model with custom mapper', function () { + mappers.stuffs = sinon.stub().callsFake(function (res) { + return { + title: res.title, + custom: 'thing' + }; + }); + + const response = { + toJSON: toJSONStub, + title: 'foo' + }; + + const apiConfig = { + docName: 'stuffs' + }; + const frame = {}; + + serializers.output.default.all(response, apiConfig, frame); + + frame.response.should.eql({ + stuffs: [ + {title: 'foo', custom: 'thing'} + ] + }); + + sinon.assert.calledOnce(mappers.stuffs); + sinon.assert.notCalled(toJSONStub); + }); + + it('.all() can map a Bookshelf collection with custom mapper', function () { + mappers.stuffs = sinon.stub().callsFake(function (res) { + return { + title: res.title, + custom: 'thing' + }; + }); + + const response = { + data: [ + { + toJSON: toJSONStub, + title: 'foo' + }, + { + toJSON: toJSONStub, + title: 'bar' + } + ], + meta: { + total: 2 + } + }; + + const apiConfig = { + docName: 'stuffs' + }; + const frame = {}; + + serializers.output.default.all(response, apiConfig, frame); + + frame.response.should.eql({ + stuffs: [ + {title: 'foo', custom: 'thing'}, + {title: 'bar', custom: 'thing'} + ], + meta: response.meta + }); + + sinon.assert.calledTwice(mappers.stuffs); + sinon.assert.notCalled(toJSONStub); + }); +}); diff --git a/test/unit/api/shared/serializers/handle.test.js b/test/unit/api/shared/serializers/handle.test.js index 1df53cdfd2..8bbad43e50 100644 --- a/test/unit/api/shared/serializers/handle.test.js +++ b/test/unit/api/shared/serializers/handle.test.js @@ -142,6 +142,11 @@ describe('Unit: api/shared/serializers/handle', function () { after: sinon.stub().resolves(), before: sinon.stub().resolves() + }, + default: { + add: sinon.stub().resolves(), + all: sinon.stub().resolves() + }, posts: { add: sinon.stub().resolves(), @@ -169,6 +174,45 @@ describe('Unit: api/shared/serializers/handle', function () { sinon.assert.calledOnceWithExactly(apiSerializers.all.after, apiConfig, frame); sinon.assert.callOrder(apiSerializers.all.before, apiSerializers.posts.all, apiSerializers.posts.add, apiSerializers.all.after); + + sinon.assert.notCalled(apiSerializers.default.add); + sinon.assert.notCalled(apiSerializers.default.all); + }); + }); + + it('correctly calls default serializer when no custom one is set', function () { + const apiSerializers = { + all: { + after: sinon.stub().resolves(), + before: sinon.stub().resolves() + + }, + default: { + add: sinon.stub().resolves(), + all: sinon.stub().resolves() + } + }; + + const response = []; + const apiConfig = {docName: 'posts', method: 'add'}; + const frame = {}; + + const stubsToCheck = [ + apiSerializers.all.before, + apiSerializers.default.all, + apiSerializers.default.add + ]; + + return shared.serializers.handle.output(response, apiConfig, apiSerializers, frame) + .then(() => { + stubsToCheck.forEach((stub) => { + sinon.assert.calledOnceWithExactly(stub, response, apiConfig, frame); + }); + + // After has a different call signature... is this a intentional? + sinon.assert.calledOnceWithExactly(apiSerializers.all.after, apiConfig, frame); + + sinon.assert.callOrder(apiSerializers.all.before, apiSerializers.default.all, apiSerializers.default.add, apiSerializers.all.after); }); }); });