Removed theme engines API versioning concept

refs: https://github.com/TryGhost/Toolbox/issues/228

- we are getting rid of the concept of api versions from Ghost
- this means getting rid of them from the frontend as well, and from themes
This commit is contained in:
Hannah Wolfe 2022-04-06 11:40:28 +01:00 committed by Daniel Lockyer
parent 67c5395a7f
commit 4332546a56
No known key found for this signature in database
GPG Key ID: D21186F0B47295AD
19 changed files with 18 additions and 349 deletions

View File

@ -228,12 +228,10 @@ async function initDynamicRouting() {
const bridge = require('./bridge');
bridge.init();
// We pass the frontend API version + the dynamic routes here, so that the frontend services are slightly less tightly-coupled
const apiVersion = bridge.getFrontendApiVersion();
// We pass the dynamic routes here, so that the frontend services are slightly less tightly-coupled
const routeSettings = await routeSettingsService.loadRouteSettings();
debug(`Frontend API Version: ${apiVersion}`);
routing.routerManager.start(apiVersion, routeSettings);
routing.routerManager.start(routeSettings);
const getRoutesHash = () => routeSettingsService.api.getCurrentHash();
const settings = require('./server/services/settings');

View File

@ -56,19 +56,7 @@ class Bridge {
// no need to check the score, activation should be used in combination with validate.check
// Use the two theme objects to set the current active theme
try {
let previousGhostAPI;
if (this.getActiveTheme()) {
previousGhostAPI = this.getActiveTheme().engine('ghost-api');
}
themeEngine.setActive(settings, loadedTheme, checkedTheme);
const currentGhostAPI = this.getActiveTheme().engine('ghost-api');
if (previousGhostAPI !== undefined && (previousGhostAPI !== currentGhostAPI)) {
events.emit('services.themes.api.changed');
await this.reloadFrontend();
}
const cardAssetConfig = this.getCardAssetConfig();
debug('reload card assets config', cardAssetConfig);
@ -81,11 +69,6 @@ class Bridge {
}
}
getFrontendApiVersion() {
// @NOTE: we've pinned the frontend to canary in a step towards removing API versions
return 'canary';
}
getCardAssetConfig() {
if (this.getActiveTheme()) {
return this.getActiveTheme().config('card_assets');
@ -95,11 +78,9 @@ class Bridge {
}
async reloadFrontend() {
const apiVersion = this.getFrontendApiVersion();
debug('reload frontend', apiVersion);
debug('reload frontend');
const siteApp = require('./frontend/web/site');
await siteApp.reload({apiVersion});
await siteApp.reload();
}
}

View File

@ -13,9 +13,8 @@ const UnsubscribeRouter = require('./UnsubscribeRouter');
const events = require('../../../server/lib/common/events');
class RouterManager {
constructor({registry, defaultApiVersion = 'v4'}) {
constructor({registry}) {
this.registry = registry;
this.defaultApiVersion = defaultApiVersion;
this.siteRouter = null;
this.urlService = null;
}
@ -62,14 +61,13 @@ class RouterManager {
*
* @param {Object} options
* @param {Boolean} [options.start] - flag controlling if the frontend Routes should be reinitialized
* @param {String} options.apiVersion - API version frontend Routes should communicate through
* @param {Object} options.routerSettings - JSON configuration to build frontend Routes
* @param {Object} options.urlService - service providing resource URL utility functions such as owns, getUrlByResourceId, and getResourceById
* @returns {ExpressRouter}
*/
init({start = false, routerSettings, apiVersion, urlService}) {
init({start = false, routerSettings, urlService}) {
this.urlService = urlService;
debug('routing init', start, apiVersion, routerSettings);
debug('routing init', start, routerSettings);
this.registry.resetAllRouters();
this.registry.resetAllRoutes();
@ -82,8 +80,7 @@ class RouterManager {
this.registry.setRouter('siteRouter', this.siteRouter);
if (start) {
apiVersion = apiVersion || this.defaultApiVersion;
this.start(apiVersion, routerSettings);
this.start(routerSettings);
}
return this.siteRouter.router();
@ -102,12 +99,11 @@ class RouterManager {
* 5. Static Pages: Weaker than collections, because we first try to find a post slug and fallback to lookup a static page.
* 6. Internal Apps: Weakest
*
* @param {string} apiVersion
* @param {object} routerSettings
*/
start(apiVersion, routerSettings) {
debug('routing start', apiVersion, routerSettings);
const RESOURCE_CONFIG = require(`./config/${apiVersion}`);
start(routerSettings) {
debug('routing start', routerSettings);
const RESOURCE_CONFIG = require(`./config/canary`);
const unsubscribeRouter = new UnsubscribeRouter();
this.siteRouter.mountRouter(unsubscribeRouter.router());

View File

@ -15,7 +15,6 @@ const join = require('path').join;
const _ = require('lodash');
const themeConfig = require('./config');
const themeEngines = require('./engines');
const config = require('../../../shared/config');
const engine = require('./engine');
const themeI18n = require('./i18n');
@ -53,9 +52,6 @@ class ActiveTheme {
// Create a theme config object
this._config = themeConfig.create(this._packageInfo);
// Create a theme engines object
this._engines = themeEngines.create(this._packageInfo);
this.initI18n();
}
@ -95,10 +91,6 @@ class ActiveTheme {
return this._config[key];
}
engine(key) {
return this._engines[key];
}
/**
*
* @param {object} options

View File

@ -1,45 +0,0 @@
const semver = require('semver');
const config = require('../../../../shared/config');
/**
* Valid definitions for "ghost-api":
*
* ^2
* ^2.0.0
* 2.0.0
* v4
* v3
* v2
* canary
*
* Goal: Extract major version from input.
*
* @param packageJson
* @returns {*}
*/
module.exports = (packageJson) => {
let themeEngines = {'ghost-api': config.get('api:versions:default')};
if (packageJson && Object.prototype.hasOwnProperty.call(packageJson, 'engines')) {
// CASE: validate
if (packageJson.engines['ghost-api']) {
const availableApiVersions = {};
config.get('api:versions:all').forEach((version) => {
if (version === 'canary') {
availableApiVersions.canary = version;
} else {
availableApiVersions[semver.major(semver.coerce(version).version)] = version;
}
});
const apiVersion = packageJson.engines['ghost-api'];
const apiVersionMajor = apiVersion === 'canary' ? 'canary' : semver.major(semver.coerce(apiVersion).version);
if (availableApiVersions[apiVersionMajor]) {
themeEngines['ghost-api'] = availableApiVersions[apiVersionMajor];
}
}
}
return themeEngines;
};

View File

@ -1,5 +0,0 @@
module.exports = {
get create() {
return require('./create');
}
};

View File

@ -158,9 +158,9 @@ module.exports = function setupSiteApp(options = {}) {
return siteApp;
};
module.exports.reload = ({apiVersion}) => {
module.exports.reload = () => {
// https://github.com/expressjs/express/issues/2596
router = siteRoutes({start: true, apiVersion});
router = siteRoutes({start: true});
Object.setPrototypeOf(SiteRouter, router);
// re-initialize apps (register app routers, because we have re-initialized the site routers)

View File

@ -1,8 +1,6 @@
const _ = require('lodash');
const debug = require('@tryghost/debug')('frontend:services:settings:validate');
const tpl = require('@tryghost/tpl');
const errors = require('@tryghost/errors');
const bridge = require('../../../bridge');
const _private = {};
let RESOURCE_CONFIG;
@ -430,12 +428,8 @@ module.exports = function validate(object) {
object.taxonomies = {};
}
const apiVersion = bridge.getFrontendApiVersion();
debug('api version', apiVersion);
// TODO: extract this config outta here! the config should be passed into this module
RESOURCE_CONFIG = require(`../../../frontend/services/routing/config/${apiVersion}`);
RESOURCE_CONFIG = require('../../../frontend/services/routing/config/canary');
object.routes = _private.validateRoutes(object.routes);
object.collections = _private.validateCollections(object.collections);

View File

@ -59,9 +59,7 @@ class Resources {
return;
}
const bridge = require('../../../bridge');
const resourcesAPIVersion = bridge.getFrontendApiVersion();
this.resourcesConfig = require(`./configs/${resourcesAPIVersion}`);
this.resourcesConfig = require('./configs/canary');
}
/**

View File

@ -9,9 +9,6 @@ const Urls = require('./Urls');
const Resources = require('./Resources');
const urlUtils = require('../../../shared/url-utils');
// This listens to services.themes.api.changed, routing events, and it's own queue events
const events = require('../../lib/common/events');
/**
* The url service class holds all instances in a centralized place.
* It's the public API you can talk to.
@ -49,9 +46,6 @@ class UrlService {
* @private
*/
_listeners() {
this._onThemeChangedListener = this._onThemeChangedListener.bind(this);
events.on('services.themes.api.changed', this._onThemeChangedListener);
this._onQueueStartedListener = this._onQueueStarted.bind(this);
this.queue.addListener('started', this._onQueueStartedListener);
@ -120,15 +114,6 @@ class UrlService {
generator.regenerateResources();
}
/**
* @description If the API version in the theme config changes, we have to reset urls and resources.
* @private
*/
_onThemeChangedListener() {
this.reset({keepListeners: true});
this.init();
}
/**
* @description Get Resource by url.
*
@ -375,7 +360,6 @@ class UrlService {
if (!options.keepListeners) {
this._onQueueStartedListener && this.queue.removeListener('started', this._onQueueStartedListener);
this._onQueueEndedListener && this.queue.removeListener('ended', this._onQueueEndedListener);
this._onThemeChangedListener && events.removeListener('services.themes.api.changed', this._onThemeChangedListener);
}
}

View File

@ -1,5 +1,4 @@
const ghostVersion = require('@tryghost/version');
const bridge = require('../../../../bridge');
// ### GhostLocals Middleware
// Expose the standard locals that every request will need to have available
@ -12,8 +11,8 @@ module.exports = function ghostLocals(req, res, next) {
res.locals.safeVersion = ghostVersion.safe;
// relative path from the URL
res.locals.relativeUrl = req.path;
// make ghost api version available for the theme + routing
res.locals.apiVersion = bridge.getFrontendApiVersion();
// @TODO: remove
res.locals.apiVersion = 'canary';
next();
};

View File

@ -16,9 +16,6 @@ describe('Frontend Routing: Email Routes', function () {
let emailPosts;
before(async function () {
sinon.stub(bridge, 'getFrontendApiVersion')
.returns('v4');
await testUtils.startGhost({forceStart: true});
request = supertest.agent(config.get('url'));

View File

@ -286,11 +286,8 @@ describe('{{ghost_head}} helper', function () {
// @TODO: this is a LOT of mocking :/
sinon.stub(routing.registry, 'getRssUrl').returns('http://localhost:65530/rss/');
sinon.stub(imageLib.imageSize, 'getImageSizeFromUrl').resolves();
sinon.stub(themeEngine, 'getActive').returns({
engine: () => 'canary'
});
sinon.stub(settingsCache, 'get');
settingsCache.get.withArgs('title').returns('Ghost');
settingsCache.get.withArgs('description').returns('site description');
settingsCache.get.withArgs('cover_image').returns('/content/images/site-cover.png');

View File

@ -1,176 +0,0 @@
const should = require('should');
const sinon = require('sinon');
const themeEngines = require('../../../../../../core/frontend/services/theme-engine/engines');
/**
* @NOTE
*
* If this test fails for you, you are probably modifying supported theme engines.
*
* When you make a change, please test that:
*
* 1. Please check that uploading a theme with newly added/modified engine version works
* 2. Check other places that potentially need updates (e.g.: frontends resource cache config, )
* 3. Add the a protective test for when next verstion (v6?) is planned it has to be changed again
*/
describe('Themes: engines', function () {
// NOTE: This is deliberately hard-coded so that when the default version is upgraded this test fails and needs reading!
const DEFAULT_ENGINE_VERSION = 'v4';
afterEach(function () {
sinon.restore();
});
it('no engines', function () {
const engines = themeEngines.create();
engines.should.eql({
'ghost-api': DEFAULT_ENGINE_VERSION
});
});
describe('ghost-api', function () {
it('unknown version falls back to default version', function () {
const engines = themeEngines.create({
engines: {
'ghost-api': 'v100'
}
});
engines.should.eql({
'ghost-api': DEFAULT_ENGINE_VERSION
});
});
it('deprecated and not supported version falls back to default version', function () {
const engines = themeEngines.create({
engines: {
'ghost-api': '^0.1'
}
});
engines.should.eql({
'ghost-api': DEFAULT_ENGINE_VERSION
});
});
it('not supported upcoming version falls back to default version', function () {
const engines = themeEngines.create({
engines: {
'ghost-api': 'v6'
}
});
engines.should.eql({
'ghost-api': DEFAULT_ENGINE_VERSION
});
});
it('v2', function () {
const engines = themeEngines.create({
engines: {
'ghost-api': 'v2'
}
});
engines.should.eql({
'ghost-api': 'v2'
});
});
it('^2', function () {
const engines = themeEngines.create({
engines: {
'ghost-api': '^2'
}
});
engines.should.eql({
'ghost-api': 'v2'
});
});
it('2.0.0', function () {
const engines = themeEngines.create({
engines: {
'ghost-api': '2.0.0'
}
});
engines.should.eql({
'ghost-api': 'v2'
});
});
it('2.17.0', function () {
const engines = themeEngines.create({
engines: {
'ghost-api': '2.17.0'
}
});
engines.should.eql({
'ghost-api': 'v2'
});
});
it('canary', function () {
const engines = themeEngines.create({
engines: {
'ghost-api': 'canary'
}
});
engines.should.eql({
'ghost-api': 'canary'
});
});
it('3', function () {
const engines = themeEngines.create({
engines: {
'ghost-api': '3'
}
});
engines.should.eql({
'ghost-api': 'v3'
});
});
it('v3', function () {
const engines = themeEngines.create({
engines: {
'ghost-api': 'v3'
}
});
engines.should.eql({
'ghost-api': 'v3'
});
});
it('4', function () {
const engines = themeEngines.create({
engines: {
'ghost-api': '4'
}
});
engines.should.eql({
'ghost-api': DEFAULT_ENGINE_VERSION
});
});
it('v4', function () {
const engines = themeEngines.create({
engines: {
'ghost-api': 'v4'
}
});
engines.should.eql({
'ghost-api': DEFAULT_ENGINE_VERSION
});
});
});
});

View File

@ -1,28 +1,10 @@
const should = require('should');
const sinon = require('sinon');
const errors = require('@tryghost/errors');
const themeEngine = require('../../../../../core/frontend/services/theme-engine');
const validate = require('../../../../../core/server/services/route-settings/validate');
should.equal(true, true);
describe('UNIT: services/settings/validate', function () {
let apiVersion;
before(function () {
apiVersion = 'canary';
sinon.stub(themeEngine, 'getActive').returns({
engine: () => {
return apiVersion;
}
});
});
after(function () {
sinon.restore();
});
it('no type definitions / empty yaml file', function () {
const object = validate({});

View File

@ -1,10 +1,7 @@
const errors = require('@tryghost/errors');
const _ = require('lodash');
const Promise = require('bluebird');
const rewire = require('rewire');
const should = require('should');
const sinon = require('sinon');
const events = require('../../../../../core/server/lib/common/events');
const Queue = require('../../../../../core/server/services/url/Queue');
const Resources = require('../../../../../core/server/services/url/Resources');
const UrlGenerator = require('../../../../../core/server/services/url/UrlGenerator');
@ -36,8 +33,6 @@ describe('Unit: services/url/UrlService', function () {
UrlService.__set__('Urls', UrlsStub);
UrlService.__set__('UrlGenerator', UrlGeneratorStub);
sinon.stub(events, 'on');
urlService = new UrlService();
});
@ -57,9 +52,6 @@ describe('Unit: services/url/UrlService', function () {
urlService.queue.addListener.calledTwice.should.be.true();
urlService.queue.addListener.args[0][0].should.eql('started');
urlService.queue.addListener.args[1][0].should.eql('ended');
events.on.calledOnce.should.be.true();
events.on.args[0][0].should.eql('services.themes.api.changed');
});
it('fn: _onQueueStarted', function () {

View File

@ -1,7 +1,6 @@
const should = require('should');
const sinon = require('sinon');
const ghostLocals = require('../../../../../../core/server/web/parent/middleware/ghost-locals');
const bridge = require('../../../../../../core/bridge');
describe('Theme Handler', function () {
let req;
@ -12,14 +11,6 @@ describe('Theme Handler', function () {
req = sinon.spy();
res = sinon.spy();
next = sinon.spy();
sinon.stub(bridge, 'getActiveTheme').callsFake(() => {
return {
engine() {
return 'canary';
}
};
});
});
afterEach(function () {

View File

@ -2,9 +2,6 @@
"name": "locale-theme",
"description": "A fake theme for testing locales.",
"version": "1.0.0",
"engines": {
"ghost-api": "canary"
},
"license": "MIT",
"config": {
"posts_per_page": 5

View File

@ -2,9 +2,6 @@
"name": "members-test-theme",
"description": "A minimal theme for testing members behaviour",
"version": "1.0.0",
"engines": {
"ghost-api": "canary"
},
"license": "MIT",
"config": {
"posts_per_page": 25