Infrastructure/ghost
0
Fork 0
mirror of https://github.com/TryGhost/Ghost.git synced 2025-03-25 02:31:59 -05:00
Code Issues Activity

Moved frontend data helpers into their own service

Browse source 
- Some of the helpers inside the routing service would be better suited to their own service
- These two helpers fetchData and entryLookup talk to the API to get data & so make a decent start for a data service
- The data service would be the single point of contact with the API for the frontend
- Doing this now cos I'm moving some files around ahead of deleting things for 5.0
This commit is contained in:
Hannah Wolfe 2022-04-05 13:12:31 +01:00
parent ac32944dc6
commit 4ee2fcd869
No known key found for this signature in database
GPG key ID: AB586C3B5AE5C037
16 changed files with 74 additions and 55 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

9
core/frontend/apps/amp/lib/router.js
View file

@ -7,6 +7,7 @@ const errors = require('@tryghost/errors');
// Dirty requires
const urlService = require('../../../../server/services/url');
const dataService = require('../../../services/data');
const helpers = require('../../../services/routing/helpers');
const templateName = 'amp';
@ -23,15 +24,15 @@ function _renderer(req, res, next) {
// Renderer begin
// Format data
let data = req.body || {};
let body = req.body || {};
// CASE: we only support amp pages for posts that are not static pages
if (!data.post || data.post.page) {
if (!body.post || body.post.page) {
return next(new errors.NotFoundError({message: tpl(messages.pageNotFound)}));
}
// Render Call
return helpers.renderer(req, res, data);
return helpers.renderer(req, res, body);
}
// This here is a controller.
@ -71,7 +72,7 @@ function getPostData(req, res, next) {
// @NOTE: amp is not supported for static pages
// @TODO: https://github.com/TryGhost/Ghost/issues/10548
helpers.entryLookup(urlWithoutSubdirectoryWithoutAmp, {permalinks, query: {controller: 'postsPublic', resource: 'posts'}}, res.locals)
dataService.entryLookup(urlWithoutSubdirectoryWithoutAmp, {permalinks, query: {controller: 'postsPublic', resource: 'posts'}}, res.locals)
.then((result) => {
if (result && result.entry) {
req.body.post = result.entry;

6
core/frontend/services/routing/helpers/entry-lookup.js → core/frontend/services/data/entry-lookup.js
View file

@ -1,11 +1,11 @@
const _ = require('lodash');
const Promise = require('bluebird');
const url = require('url');
const debug = require('@tryghost/debug')('services:routing:helpers:entry-lookup');
const debug = require('@tryghost/debug')('services:data:entry-lookup');
const routeMatch = require('path-match')();
/**
* @description Query API for a single entry/resource.
* Query API for a single entry/resource.
* @param {String} postUrl
* @param {Object} routerOptions
* @param {Object} locals
@ -14,7 +14,7 @@ const routeMatch = require('path-match')();
function entryLookup(postUrl, routerOptions, locals) {
debug(postUrl);
const api = require('../../proxy').api[locals.apiVersion];
const api = require('../proxy').api[locals.apiVersion];
const targetPath = url.parse(postUrl).path;
const permalinks = routerOptions.permalinks;
let isEditURL = false;

6
core/frontend/services/routing/helpers/fetch-data.js → core/frontend/services/data/fetch-data.js
View file

@ -34,7 +34,7 @@ const defaultPostQuery = _.cloneDeep(queryDefaults);
defaultPostQuery.options = defaultQueryOptions.options;
/**
* @description Process query request.
* Process query request.
*
* Takes a 'query' object, ensures that type, resource and options are set
* Replaces occurrences of `%s` in options with slugParam
@ -45,7 +45,7 @@ defaultPostQuery.options = defaultQueryOptions.options;
* @returns {Promise}
*/
function processQuery(query, slugParam, locals) {
const api = require('../../proxy').api[locals.apiVersion];
const api = require('../proxy').api[locals.apiVersion];
query = _.cloneDeep(query);
@ -62,7 +62,7 @@ function processQuery(query, slugParam, locals) {
}
/**
* @description Fetch data from API helper for controllers.
* Fetch data from API helper for controllers.
*
* Calls out to get posts per page, builds the final posts query & builds any additional queries
* Wraps the queries using Promise.props to ensure it gets named responses

4
core/frontend/services/data/index.js Normal file
View file

@ -0,0 +1,4 @@
module.exports = {
entryLookup: require('./entry-lookup'),
fetchData: require('./fetch-data')
};

3
core/frontend/services/routing/controllers/channel.js
View file

@ -4,6 +4,7 @@ const tpl = require('@tryghost/tpl');
const errors = require('@tryghost/errors');
const security = require('@tryghost/security');
const themeEngine = require('../../theme-engine');
const dataService = require('../../data');
const helpers = require('../helpers');
const messages = {
@ -50,7 +51,7 @@ module.exports = function channelController(req, res, next) {
}
}
return helpers.fetchData(pathOptions, res.routerOptions, res.locals)
return dataService.fetchData(pathOptions, res.routerOptions, res.locals)
.then(function handleResult(result) {
// CASE: requested page is greater than number of pages we have
if (pathOptions.page > result.meta.pagination.pages) {

3
core/frontend/services/routing/controllers/collection.js
View file

@ -6,6 +6,7 @@ const security = require('@tryghost/security');
const {routerManager} = require('../');
const themeEngine = require('../../theme-engine');
const helpers = require('../helpers');
const dataService = require('../../data');
const messages = {
pageNotFound: 'Page not found.'
@ -49,7 +50,7 @@ module.exports = function collectionController(req, res, next) {
}
debug('fetching data');
return helpers.fetchData(pathOptions, res.routerOptions, res.locals)
return dataService.fetchData(pathOptions, res.routerOptions, res.locals)
.then(function handleResult(result) {
// CASE: requested page is greater than number of pages we have
if (pathOptions.page > result.meta.pagination.pages) {

3
core/frontend/services/routing/controllers/entry.js
View file

@ -3,6 +3,7 @@ const url = require('url');
const config = require('../../../../shared/config');
const {routerManager} = require('../');
const urlUtils = require('../../../../shared/url-utils');
const dataService = require('../../data');
const helpers = require('../helpers');
/**
@ -15,7 +16,7 @@ const helpers = require('../helpers');
module.exports = function entryController(req, res, next) {
debug('entryController', res.routerOptions);
return helpers.entryLookup(req.path, res.routerOptions, res.locals)
return dataService.entryLookup(req.path, res.routerOptions, res.locals)
.then(function then(lookup) {
// Format data 1
const entry = lookup ? lookup.entry : false;

3
core/frontend/services/routing/controllers/rss.js
View file

@ -5,6 +5,7 @@ const security = require('@tryghost/security');
const settingsCache = require('../../../../shared/settings-cache');
const rssService = require('../../rss');
const helpers = require('../helpers');
const dataService = require('../../data');
// @TODO: is this really correct? Should we be using meta data title?
function getTitle(relatedData) {
@ -35,7 +36,7 @@ module.exports = function rssController(req, res, next) {
// @TODO: This belongs to the rss service O_o
const baseUrl = url.parse(req.originalUrl).pathname;
helpers.fetchData(pathOptions, res.routerOptions, res.locals)
dataService.fetchData(pathOptions, res.routerOptions, res.locals)
.then(function formatResult(result) {
const response = _.pick(result, ['posts', 'meta']);

8
core/frontend/services/routing/helpers/index.js
View file

@ -1,12 +1,4 @@
module.exports = {
get entryLookup() {
return require('./entry-lookup');
},
get fetchData() {
return require('./fetch-data');
},
get renderEntries() {
return require('./render-entries');
},

9
test/unit/frontend/apps/amp/router.test.js
View file

@ -5,6 +5,7 @@ const path = require('path');
const ampController = require('../../../../../core/frontend/apps/amp/lib/router');
const urlService = require('../../../../../core/server/services/url');
const helpers = require('../../../../../core/frontend/services/routing/helpers');
const dataService = require('../../../../../core/frontend/services/data');
const testUtils = require('../../../../utils');
const configUtils = require('../../../../utils/configUtils');
@ -107,7 +108,7 @@ describe('Unit - apps/amp/lib/router', function () {
entryLookupStub = sinon.stub();
sinon.stub(helpers, 'entryLookup').get(function () {
sinon.stub(dataService, 'entryLookup').get(function () {
return entryLookupStub;
});
@ -123,7 +124,7 @@ describe('Unit - apps/amp/lib/router', function () {
urlService.getPermalinkByUrl.withArgs('/welcome/').returns('/:slug/');
helpers.entryLookup.withArgs('/welcome/', {permalinks: '/:slug/', query: {controller: 'postsPublic', resource: 'posts'}})
dataService.entryLookup.withArgs('/welcome/', {permalinks: '/:slug/', query: {controller: 'postsPublic', resource: 'posts'}})
.resolves({
entry: post
});
@ -140,7 +141,7 @@ describe('Unit - apps/amp/lib/router', function () {
urlService.getPermalinkByUrl.withArgs('/welcome/').returns('/:slug/');
helpers.entryLookup.withArgs('/welcome/', {permalinks: '/:slug/', query: {controller: 'postsPublic', resource: 'posts'}}).resolves({
dataService.entryLookup.withArgs('/welcome/', {permalinks: '/:slug/', query: {controller: 'postsPublic', resource: 'posts'}}).resolves({
entry: post
});
@ -155,7 +156,7 @@ describe('Unit - apps/amp/lib/router', function () {
urlService.getPermalinkByUrl.withArgs('/welcome/').returns('/:slug/');
helpers.entryLookup.withArgs('/welcome/', {permalinks: '/:slug/', query: {controller: 'postsPublic', resource: 'posts'}})
dataService.entryLookup.withArgs('/welcome/', {permalinks: '/:slug/', query: {controller: 'postsPublic', resource: 'posts'}})
.rejects(new errors.NotFoundError());
ampController.getPostData(req, res, function (err) {

41
test/unit/frontend/services/routing/helpers/entry-lookup.test.js → test/unit/frontend/services/data/entry-lookup.test.js
View file

@ -1,11 +1,12 @@
const should = require('should');
const sinon = require('sinon');
const Promise = require('bluebird');
const testUtils = require('../../../../../utils');
const api = require('../../../../../../core/server/api');
const helpers = require('../../../../../../core/frontend/services/routing/helpers');
describe('Unit - services/routing/helpers/entry-lookup', function () {
const API_VERSION = 'canary';
const api = require('../../../../../core/server/api')[API_VERSION];
const data = require('../../../../../core/frontend/services/data');
const testUtils = require('../../../../utils');
describe('Unit - frontend/data/entry-lookup', function () {
let locals;
afterEach(function () {
@ -35,25 +36,25 @@ describe('Unit - services/routing/helpers/entry-lookup', function () {
pages: pages
});
sinon.stub(api.canary, 'posts').get(() => {
sinon.stub(api, 'posts').get(() => {
return {
read: postsReadStub
};
});
sinon.stub(api.canary, 'pagesPublic').get(() => {
sinon.stub(api, 'pagesPublic').get(() => {
return {
read: pagesReadStub
};
});
locals = {apiVersion: 'canary'};
locals = {apiVersion: API_VERSION};
});
it('ensure pages controller is triggered', function () {
const testUrl = 'http://127.0.0.1:2369' + pages[0].url;
return helpers.entryLookup(testUrl, routerOptions, locals).then(function (lookup) {
return data.entryLookup(testUrl, routerOptions, locals).then(function (lookup) {
postsReadStub.called.should.be.false();
pagesReadStub.calledOnce.should.be.true();
should.exist(lookup.entry);
@ -65,7 +66,7 @@ describe('Unit - services/routing/helpers/entry-lookup', function () {
describe('posts', function () {
const routerOptions = {
permalinks: '/:slug/',
permalinks: '/:slug/:options(edit)?/',
query: {controller: 'posts', resource: 'posts'}
};
@ -86,25 +87,25 @@ describe('Unit - services/routing/helpers/entry-lookup', function () {
posts: posts
});
sinon.stub(api.canary, 'posts').get(() => {
sinon.stub(api, 'posts').get(() => {
return {
read: postsReadStub
};
});
sinon.stub(api.canary, 'pagesPublic').get(() => {
sinon.stub(api, 'pagesPublic').get(() => {
return {
read: pagesReadStub
};
});
locals = {apiVersion: 'canary'};
locals = {apiVersion: API_VERSION};
});
it('ensure posts controller is triggered', function () {
const testUrl = 'http://127.0.0.1:2369' + posts[0].url;
return helpers.entryLookup(testUrl, routerOptions, locals).then(function (lookup) {
return data.entryLookup(testUrl, routerOptions, locals).then(function (lookup) {
postsReadStub.calledOnce.should.be.true();
pagesReadStub.called.should.be.false();
should.exist(lookup.entry);
@ -112,5 +113,17 @@ describe('Unit - services/routing/helpers/entry-lookup', function () {
lookup.isEditURL.should.be.false();
});
});
it('can handle an edit url', function () {
const testUrl = `http://127.0.0.1:2369${posts[0].url}edit/`;
return data.entryLookup(testUrl, routerOptions, locals).then(function (lookup) {
postsReadStub.calledOnce.should.be.true();
pagesReadStub.called.should.be.false();
should.exist(lookup.entry);
lookup.entry.should.have.property('url', posts[0].url);
lookup.isEditURL.should.be.true();
});
});
});
});

21
test/unit/frontend/services/routing/helpers/fetch-data.test.js → test/unit/frontend/services/data/fetch-data.test.js
View file

@ -1,11 +1,12 @@
const should = require('should');
const sinon = require('sinon');
const API_VERSION = 'canary';
const api = require('../../../../../../core/server/api')[API_VERSION];
const helpers = require('../../../../../../core/frontend/services/routing/helpers');
const testUtils = require('../../../../../utils');
describe('Unit - services/routing/helpers/fetch-data', function () {
const API_VERSION = 'canary';
const api = require('../../../../../core/server/api')[API_VERSION];
const data = require('../../../../../core/frontend/services/data');
const testUtils = require('../../../../utils');
describe('Unit - frontend/data/fetch-data', function () {
let posts;
let tags;
let locals;
@ -56,7 +57,7 @@ describe('Unit - services/routing/helpers/fetch-data', function () {
});
it('should handle no options', function (done) {
helpers.fetchData(null, null, locals).then(function (result) {
data.fetchData(null, null, locals).then(function (result) {
should.exist(result);
result.should.be.an.Object().with.properties('posts', 'meta');
result.should.not.have.property('data');
@ -71,7 +72,7 @@ describe('Unit - services/routing/helpers/fetch-data', function () {
});
it('should handle path options with page/limit', function (done) {
helpers.fetchData({page: 2, limit: 10}, null, locals).then(function (result) {
data.fetchData({page: 2, limit: 10}, null, locals).then(function (result) {
should.exist(result);
result.should.be.an.Object().with.properties('posts', 'meta');
result.should.not.have.property('data');
@ -104,7 +105,7 @@ describe('Unit - services/routing/helpers/fetch-data', function () {
}
};
helpers.fetchData(pathOptions, routerOptions, locals).then(function (result) {
data.fetchData(pathOptions, routerOptions, locals).then(function (result) {
should.exist(result);
result.should.be.an.Object().with.properties('posts', 'meta', 'data');
result.data.should.be.an.Object().with.properties('featured');
@ -135,7 +136,7 @@ describe('Unit - services/routing/helpers/fetch-data', function () {
}
};
helpers.fetchData(pathOptions, routerOptions, locals).then(function (result) {
data.fetchData(pathOptions, routerOptions, locals).then(function (result) {
should.exist(result);
result.should.be.an.Object().with.properties('posts', 'meta', 'data');
@ -170,7 +171,7 @@ describe('Unit - services/routing/helpers/fetch-data', function () {
}
};
helpers.fetchData(pathOptions, routerOptions, locals).then(function (result) {
data.fetchData(pathOptions, routerOptions, locals).then(function (result) {
should.exist(result);
result.should.be.an.Object().with.properties('posts', 'meta', 'data');
result.data.should.be.an.Object().with.properties('tag');

3
test/unit/frontend/services/routing/controllers/channel.test.js
View file

@ -6,6 +6,7 @@ const security = require('@tryghost/security');
const themeEngine = require('../../../../../../core/frontend/services/theme-engine');
const controllers = require('../../../../../../core/frontend/services/routing/controllers');
const helpers = require('../../../../../../core/frontend/services/routing/helpers');
const dataService = require('../../../../../../core/frontend/services/data');
function failTest(done) {
return function (err) {
@ -34,7 +35,7 @@ describe('Unit - services/routing/controllers/channel', function () {
fetchDataStub = sinon.stub();
renderStub = sinon.stub();
sinon.stub(helpers, 'fetchData').get(function () {
sinon.stub(dataService, 'fetchData').get(function () {
return fetchDataStub;
});

3
test/unit/frontend/services/routing/controllers/collection.test.js
View file

@ -7,6 +7,7 @@ const themeEngine = require('../../../../../../core/frontend/services/theme-engi
const routerManager = require('../../../../../../core/frontend/services/routing/').routerManager;
const controllers = require('../../../../../../core/frontend/services/routing/controllers');
const helpers = require('../../../../../../core/frontend/services/routing/helpers');
const dataService = require('../../../../../../core/frontend/services/data');
function failTest(done) {
return function (err) {
@ -36,7 +37,7 @@ describe('Unit - services/routing/controllers/collection', function () {
fetchDataStub = sinon.stub();
renderStub = sinon.stub();
sinon.stub(helpers, 'fetchData').get(function () {
sinon.stub(dataService, 'fetchData').get(function () {
return fetchDataStub;
});

3
test/unit/frontend/services/routing/controllers/entry.test.js
View file

@ -6,6 +6,7 @@ const urlUtils = require('../../../../../../core/shared/url-utils');
const routerManager = require('../../../../../../core/frontend/services/routing/').routerManager;
const controllers = require('../../../../../../core/frontend/services/routing/controllers');
const helpers = require('../../../../../../core/frontend/services/routing/helpers');
const dataService = require('../../../../../../core/frontend/services/data');
const EDITOR_URL = `/#/editor/post/`;
describe('Unit - services/routing/controllers/entry', function () {
@ -27,7 +28,7 @@ describe('Unit - services/routing/controllers/entry', function () {
entryLookUpStub = sinon.stub();
renderStub = sinon.stub();
sinon.stub(helpers, 'entryLookup').get(function () {
sinon.stub(dataService, 'entryLookup').get(function () {
return entryLookUpStub;
});

4
test/unit/frontend/services/routing/controllers/rss.test.js
View file

@ -3,7 +3,7 @@ const testUtils = require('../../../../../utils');
const security = require('@tryghost/security');
const settingsCache = require('../../../../../../core/shared/settings-cache');
const controllers = require('../../../../../../core/frontend/services/routing/controllers');
const helpers = require('../../../../../../core/frontend/services/routing/helpers');
const dataService = require('../../../../../../core/frontend/services/data');
const rssService = require('../../../../../../core/frontend/services/rss');
// Helper function to prevent unit tests
@ -43,7 +43,7 @@ describe('Unit - services/routing/controllers/rss', function () {
next = sinon.stub();
fetchDataStub = sinon.stub();
sinon.stub(helpers, 'fetchData').get(function () {
sinon.stub(dataService, 'fetchData').get(function () {
return fetchDataStub;
});