Infrastructure/ghost
0
Fork 0
mirror of https://github.com/TryGhost/Ghost.git synced 2025-02-24 23:48:13 -05:00
Code Issues Activity

Refactored tags repository to be model-agnostic

Browse source 
refs https://github.com/TryGhost/Toolbox/issues/522

- The caching rules for Content API resources like Tags / Posts / Pages / Authors are the same, so it makes sense to make a common repository which would take in a model as a parameter and perform the queries on that level instead of implementing repository-per-model.
- With this refactor getting Posts Content API caching would be much simpler change.
This commit is contained in:
Naz 2023-02-17 15:55:49 +08:00
parent 5d00b64e27
commit c3d7104367
No known key found for this signature in database
12 changed files with 39 additions and 39 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

10
ghost/core/core/server/services/tags-public/service.js
View file

@ -6,7 +6,7 @@ class TagsPublicServiceWrapper {
}
// Wire up all the dependencies
const models = require('../../models');
const {TagPublic} = require('../../models');
const adapterManager = require('../adapter-manager');
const config = require('../../../shared/config');
@ -15,15 +15,15 @@ class TagsPublicServiceWrapper {
tagsCache = adapterManager.getAdapter('cache:tagsPublic');
}
const {TagsPublicRepository} = require('@tryghost/tags-public');
const {PublicResourcesRepository} = require('@tryghost/public-resource-repository');
this.linkRedirectRepository = new TagsPublicRepository({
Tag: models.TagPublic,
this.tagsPublicRepository = new PublicResourcesRepository({
Model: TagPublic,
cache: tagsCache
});
this.api = {
browse: this.linkRedirectRepository.getAll.bind(this.linkRedirectRepository)
browse: this.tagsPublicRepository.getAll.bind(this.tagsPublicRepository)
};
}
}

2
ghost/core/package.json
View file

@ -133,7 +133,7 @@
"@tryghost/staff-service": "0.0.0",
"@tryghost/stats-service": "0.0.0",
"@tryghost/string": "0.2.2",
"@tryghost/tags-public": "0.0.0",
"@tryghost/public-resource-repository": "0.0.0",
"@tryghost/tiers": "0.0.0",
"@tryghost/tpl": "0.1.21",
"@tryghost/update-check-service": "0.0.0",

0
ghost/tags-public/.eslintrc.js → ghost/public-resource-repository/.eslintrc.js
View file

2
ghost/tags-public/README.md → ghost/public-resource-repository/README.md
View file

@ -1,6 +1,6 @@
# Tags Public
services and repositories serving public tags endpoints
services and repositories serving public resource endpoints e.g. Tags/Posts/Authors Content API endpoints
## Usage

1
ghost/public-resource-repository/index.js Normal file
View file

@ -0,0 +1 @@
module.exports = require('./lib/resources-public');

18
ghost/tags-public/lib/TagsPublicRepository.js → ghost/public-resource-repository/lib/PublicResourcesRepository.js
View file

@ -1,33 +1,33 @@
const _ = require('lodash');
module.exports = class TagsPublicRepository {
module.exports = class PublicResourcesRepository {
/** @type {object} */
#Tag;
#Model;
/** @type {object} */
#cache;
/**
* @param {object} deps
* @param {object} deps.Tag Bookshelf Model instance of TagPublic
* @param {object} deps.Model Bookshelf Model instance of TagPublic/Post/Author etc.
* @param {object} deps.cache cache instance
*/
constructor(deps) {
this.#Tag = deps.Tag;
this.#Model = deps.Model;
this.#cache = deps.cache;
}
/**
* Queries the database for Tag records
* Queries the database for model records
* @param {Object} options model options
* @returns
*/
async #getAllDB(options) {
return this.#Tag.findPage(options);
return this.#Model.findPage(options);
}
/**
* Gets all tags and caches the returned results
* Retrieves all records from the storage (cache or database)
* @param {Object} options
* @returns
*/
@ -36,13 +36,13 @@ module.exports = class TagsPublicRepository {
if (this.#cache) {
// make the cache key smaller and don't include context
const permittedOptions = this.#Tag.permittedOptions('findPage')
const permittedOptions = this.#Model.permittedOptions('findPage')
.filter(option => (option !== 'context'));
const optionsForCacheKey = _.pick(options, permittedOptions);
// NOTE: can be more aggressive here with filtering options,
// for example, do we care make a distinction for logged
// in member on the tags level?
// in member on the repository level?
cacheKey = `get-all-${JSON.stringify(optionsForCacheKey)}`;
const cachedResult = await this.#cache.get(cacheKey);

1
ghost/public-resource-repository/lib/resources-public.js Normal file
View file

@ -0,0 +1 @@
module.exports.PublicResourcesRepository = require('./PublicResourcesRepository');

2
ghost/tags-public/package.json → ghost/public-resource-repository/package.json
View file

@ -1,5 +1,5 @@
{
"name": "@tryghost/tags-public",
"name": "@tryghost/public-resource-repository",
"version": "0.0.0",
"repository": "https://github.com/TryGhost/Ghost/tree/main/packages/tags-public",
"author": "Ghost Foundation",

0
ghost/tags-public/test/.eslintrc.js → ghost/public-resource-repository/test/.eslintrc.js
View file

40
ghost/tags-public/test/TagsPublicRepository.test.js → ghost/public-resource-repository/test/PublicResourcesRepository.test.js
View file

@ -1,24 +1,24 @@
const assert = require('assert');
const sinon = require('sinon');
const {TagsPublicRepository} = require('../index');
const {PublicResourcesRepository} = require('../index');
// @NOTE: This is a dirty import from the Ghost "core"!
// extract it to it's own package and import here as require('@tryghost/adapter-base-cache-memory');
const MemoryCache = require('../../core/core/server/adapters/cache/Memory');
describe('TagsPublicRepository', function () {
describe('PublicResourcesRepository', function () {
afterEach(function () {
sinon.restore();
});
describe('getAll', function () {
it('calls findPage on the model multiple times when cache NOT present', async function () {
const tagStub = {
const tagModelStub = {
findPage: sinon.stub().resolves(),
permittedOptions: sinon.stub()
};
const repo = new TagsPublicRepository({
Tag: tagStub
const repo = new PublicResourcesRepository({
Model: tagModelStub
});
// first call
@ -30,12 +30,12 @@ describe('TagsPublicRepository', function () {
limit: 'all'
});
assert.equal(tagStub.findPage.callCount, 2, 'should be called same amount of times as getAll');
assert.ok(tagStub.findPage.calledWith({limit: 'all'}));
assert.equal(tagModelStub.findPage.callCount, 2, 'should be called same amount of times as getAll');
assert.ok(tagModelStub.findPage.calledWith({limit: 'all'}));
});
it('calls findPage once and uses the cached value on subsequent calls', async function () {
const tagStub = {
const tagModelStub = {
findPage: sinon.stub().resolves({
data: [{
get(key) {
@ -46,8 +46,8 @@ describe('TagsPublicRepository', function () {
}),
permittedOptions: sinon.stub().returns(['limit'])
};
const repo = new TagsPublicRepository({
Tag: tagStub,
const repo = new PublicResourcesRepository({
Model: tagModelStub,
cache: new MemoryCache()
});
@ -60,14 +60,14 @@ describe('TagsPublicRepository', function () {
limit: 'all'
});
assert.equal(tagStub.findPage.callCount, 1, 'should be called once when cache is present');
assert.ok(tagStub.findPage.calledWith({limit: 'all'}));
assert.equal(tagModelStub.findPage.callCount, 1, 'should be called once when cache is present');
assert.ok(tagModelStub.findPage.calledWith({limit: 'all'}));
assert.equal(dbTags, cacheTags, 'should return the same value from the cache');
});
it('calls findPage multiple times if the record is not present in the cache', async function () {
const tagStub = {
const tagModelStub = {
findPage: sinon.stub().resolves({
data: [{
get(key) {
@ -79,8 +79,8 @@ describe('TagsPublicRepository', function () {
permittedOptions: sinon.stub().returns(['limit'])
};
const cache = new MemoryCache();
const repo = new TagsPublicRepository({
Tag: tagStub,
const repo = new PublicResourcesRepository({
Model: tagModelStub,
cache: cache
});
@ -97,12 +97,12 @@ describe('TagsPublicRepository', function () {
limit: 'all'
});
assert.equal(tagStub.findPage.callCount, 2, 'should be called every time the item is not in the cache');
assert.ok(tagStub.findPage.calledWith({limit: 'all'}));
assert.equal(tagModelStub.findPage.callCount, 2, 'should be called every time the item is not in the cache');
assert.ok(tagModelStub.findPage.calledWith({limit: 'all'}));
});
it('works with a cache that has an asynchronous interface', async function () {
const tagStub = {
const tagModelStub = {
findPage: sinon.stub().resolves({
data: [{
get(key) {
@ -119,8 +119,8 @@ describe('TagsPublicRepository', function () {
set: sinon.stub().resolves()
};
const repo = new TagsPublicRepository({
Tag: tagStub,
const repo = new PublicResourcesRepository({
Model: tagModelStub,
cache: asyncMemoryCache
});

1
ghost/tags-public/index.js
View file

@ -1 +0,0 @@
module.exports = require('./lib/tags-public');

1
ghost/tags-public/lib/tags-public.js
View file

@ -1 +0,0 @@
module.exports.TagsPublicRepository = require('./TagsPublicRepository');