Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Cached api controller pipelines #19880

Merged
merged 2 commits into from Mar 18, 2024
Merged

Cached api controller pipelines #19880

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
9b4eb36
Cached api controller pipelines
allouis Mar 18, 2024
2c6a719
fixup! Cached api controller pipelines
allouis Mar 18, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
12 changes: 11 additions & 1 deletion ghost/api-framework/lib/pipeline.js
View file
Open in desktop
Expand Up @@ -160,6 +160,8 @@ const STAGES = {
}
};

const controllerMap = new Map();

/**
* @description The pipeline runs the request through all stages (validation, serialisation, permissions).
*
Expand All @@ -180,12 +182,16 @@ const STAGES = {
* @return {Object}
*/
const pipeline = (apiController, apiUtils, apiType) => {
if (controllerMap.has(apiController)) {
return controllerMap.get(apiController);
}

const keys = Object.keys(apiController);
const docName = apiController.docName;

// CASE: api controllers are objects with configuration.
// We have to ensure that we expose a functional interface e.g. `api.posts.add` has to be available.
return keys.reduce((obj, method) => {
const result = keys.reduce((obj, method) => {
const apiImpl = _.cloneDeep(apiController)[method];

Object.freeze(apiImpl.headers);
Expand Down Expand Up @@ -267,6 +273,10 @@ const pipeline = (apiController, apiUtils, apiType) => {
Object.assign(obj[method], apiImpl);
return obj;
}, {});

controllerMap.set(apiController, result);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Would it be worth using docName as the key?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

docName isn't unique - so no! 😝

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

wtf

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

One of the things docName is used for is to determine which permissions to use - so the pages endpoint has a docName of 'posts' - as does the postsPublic endpoint

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

unbelievable


return result;
};

module.exports = pipeline;
Expand Down
20 changes: 9 additions & 11 deletions ghost/core/test/regression/mock-express-style/api-vs-frontend.test.js
View file
Open in desktop
Expand Up @@ -33,8 +33,8 @@ describe('Frontend behavior tests', function () {

beforeEach(function () {
sinon.stub(themeEngine.getActive(), 'config').withArgs('posts_per_page').returns(2);
const postsAPI = require('../../../core/server/api/endpoints/posts-public');
postSpy = sinon.spy(postsAPI.browse, 'query');
const postsAPI = require('../../../core/server/api/endpoints').postsPublic;
postSpy = sinon.spy(postsAPI, 'browse');
});

afterEach(function () {
Expand Down Expand Up @@ -143,22 +143,20 @@ describe('Frontend behavior tests', function () {
});
});

it('serve tag', function () {
it('serve tag', async function () {
const req = {
method: 'GET',
url: '/tag/bacon/',
host: 'example.com'
};

return localUtils.mockExpress.invoke(app, req)
.then(function (response) {
response.statusCode.should.eql(200);
response.template.should.eql('tag');
const response = await localUtils.mockExpress.invoke(app, req);
response.statusCode.should.eql(200);
response.template.should.eql('tag');

postSpy.args[0][0].options.filter.should.eql('(tags:\'bacon\'+tags.visibility:public)+type:post');
postSpy.args[0][0].options.page.should.eql(1);
postSpy.args[0][0].options.limit.should.eql(2);
});
postSpy.args[0][0].filter.should.eql('tags:\'bacon\'+tags.visibility:public');
postSpy.args[0][0].page.should.eql(1);
postSpy.args[0][0].limit.should.eql(2);
});

it('serve tag rss', function () {
Expand Down