Skip to content

Commit 63f3aac

Browse files
daniellockyerdaniellockyer
committed
Deprecated models.init pattern for initializing models
refs https://ghost.slack.com/archives/C02G9E68C/p1740997729615089 ref https://linear.app/ghost/issue/ENG-2071/remove-modelsinit-pattern - `models.init` will dynamically require all the files in the folder and re-export them - this is useful in that you can add a model, but the downside is that it breaks all editor autocomplete because we don't know the exports until runtime - more generally, this pattern is super annoying and we always have to remember to do `models.init` in unit tests - to fix that, we can deprecate the use of this (there are some other places outside of this codebase we need to remove it from too) and explicit export all the files - this means you have to add your new model to this file, but that's better than not having any types available
1 parent 49b6a64 commit 63f3aac

File tree

67 files changed

+87
-279
lines changed

Some content is hidden

Large Commits have some content hidden by default. Use the searchbox below for content that may be hidden.

67 files changed

+87
-279
lines changed

‎ghost/core/core/boot.js

-6
Original file line numberDiff line numberDiff line change
@@ -91,12 +91,6 @@ async function initCore({ghostServer, config, frontend}) {
9191
require('./shared/url-utils');
9292
debug('End: Load urlUtils');
9393

94-
// Models are the heart of Ghost - this is a syncronous operation
95-
debug('Begin: models');
96-
const models = require('./server/models');
97-
models.init();
98-
debug('End: models');
99-
10094
// Settings are a core concept we use settings to store key-value pairs used in critical pathways as well as public data like the site title
10195
debug('Begin: settings');
10296
const settings = require('./server/services/settings/settings-service');

‎ghost/core/core/cli/generate-data.js

-2
Original file line numberDiff line numberDiff line change
@@ -21,8 +21,6 @@ module.exports = class DataGeneratorCommand extends Command {
2121
const models = require('../server/models');
2222
const knex = require('../server/data/db/connection');
2323

24-
models.init();
25-
2624
context.models = models;
2725
context.m = models;
2826
context.knex = knex;

‎ghost/core/core/cli/repl.js

-2
Original file line numberDiff line numberDiff line change
@@ -12,8 +12,6 @@ module.exports = class REPL extends Command {
1212
const models = require('../server/models');
1313
const knex = require('../server/data/db/connection');
1414

15-
models.init();
16-
1715
context.models = models;
1816
context.m = models;
1917
context.knex = knex;

‎ghost/core/core/server/data/migrations/hooks/init/before.js

-3
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,3 @@
1-
const models = require('../../../../models');
2-
31
module.exports = function before() {
4-
models.init();
52
return Promise.resolve();
63
};

‎ghost/core/core/server/models/base/listeners.js

+8-5
Original file line numberDiff line numberDiff line change
@@ -1,10 +1,12 @@
11
const moment = require('moment-timezone');
22
const _ = require('lodash');
3-
const models = require('../../models');
43
const logging = require('@tryghost/logging');
54
const errors = require('@tryghost/errors');
65
const {sequence} = require('@tryghost/promise');
76

7+
const {Post: PostModel} = require('../post');
8+
const {Settings: SettingsModel} = require('../settings');
9+
810
// Listen to settings.timezone.edited and settings.notifications.edited to bind extra logic to settings, similar to the bridge and member service
911
const events = require('../../lib/common/events');
1012

@@ -36,12 +38,13 @@ events.on('settings.timezone.edited', function (settingModel, options) {
3638
* We lock the target row on fetch by using the `forUpdate` option.
3739
* Read more in models/post.js - `onFetching`
3840
*/
39-
return models.Base.transaction(async function (transacting) {
41+
42+
return PostModel.transaction(async function (transacting) {
4043
options.transacting = transacting;
4144
options.forUpdate = true;
4245

4346
try {
44-
const results = await models.Post.findAll(_.merge({filter: 'status:scheduled'}, options));
47+
const results = await PostModel.findAll(_.merge({filter: 'status:scheduled'}, options));
4548
if (!results.length) {
4649
return;
4750
}
@@ -67,7 +70,7 @@ events.on('settings.timezone.edited', function (settingModel, options) {
6770
}
6871

6972
try {
70-
await models.Post.edit(post.toJSON(), _.merge({id: post.id}, options));
73+
await PostModel.edit(post.toJSON(), _.merge({id: post.id}, options));
7174
} catch (err) {
7275
logging.error(new errors.InternalServerError({
7376
err
@@ -111,7 +114,7 @@ events.on('settings.notifications.edited', function (settingModel) {
111114
return;
112115
}
113116

114-
return models.Settings.edit({
117+
return SettingsModel.edit({
115118
key: 'notifications',
116119
value: JSON.stringify(allNotifications)
117120
}, options).catch(function (err) {

‎ghost/core/core/server/models/index.js

+76-24
Original file line numberDiff line numberDiff line change
@@ -1,35 +1,87 @@
1-
/**
2-
* Dependencies
3-
*/
4-
5-
const _ = require('lodash');
6-
const debug = require('@tryghost/debug')('models');
7-
const glob = require('glob');
1+
/* eslint-disable max-lines */
82

93
// enable event listeners
104
require('./base/listeners');
115

126
/**
137
* Expose all models
148
*/
15-
exports = module.exports;
9+
module.exports = {
10+
// `base` file does not export a Base model
11+
Base: require('./base'),
1612

17-
function init() {
18-
const baseNow = Date.now();
19-
exports.Base = require('./base');
20-
debug(`${Date.now() - baseNow}ms - Base.js require`);
21-
22-
let modelsFiles = glob.sync('!(index).js', {cwd: __dirname});
23-
modelsFiles.forEach((model) => {
24-
const name = model.replace(/.js$/, '');
25-
const modelNow = Date.now();
26-
_.extend(exports, require('./' + name));
27-
debug(`${Date.now() - modelNow}ms - ${model} require`);
28-
});
29-
}
13+
...require('./action'),
14+
...require('./author'),
15+
...require('./api-key'),
16+
...require('./benefit'),
17+
...require('./collection-post'),
18+
...require('./collection'),
19+
...require('./comment-like'),
20+
...require('./comment-report'),
21+
...require('./comment'),
22+
...require('./custom-theme-setting'),
23+
...require('./donation-payment-event'),
24+
...require('./email-batch'),
25+
...require('./email-recipient-failure'),
26+
...require('./email-recipient'),
27+
...require('./email-spam-complaint-event'),
28+
...require('./email'),
29+
...require('./integration'),
30+
...require('./invite'),
31+
...require('./job'),
32+
...require('./label'),
33+
...require('./mail-event'),
34+
...require('./member-cancel-event'),
35+
...require('./member-click-event'),
36+
...require('./member-created-event'),
37+
...require('./member-email-change-event'),
38+
...require('./member-feedback'),
39+
...require('./member-login-event'),
40+
...require('./member-newsletter'),
41+
...require('./member-paid-subscription-event'),
42+
...require('./member-payment-event'),
43+
...require('./member-product-event'),
44+
...require('./member-status-event'),
45+
...require('./member-stripe-customer'),
46+
...require('./member-subscribe-event'),
47+
...require('./member'),
48+
...require('./mention'),
49+
...require('./milestone'),
50+
...require('./mobiledoc-revision'),
51+
...require('./newsletter'),
52+
...require('./offer-redemption'),
53+
...require('./offer'),
54+
...require('./permission'),
55+
...require('./post-revision'),
56+
...require('./post'),
57+
...require('./posts-meta'),
58+
...require('./product'),
59+
...require('./recommendation-click-event'),
60+
...require('./recommendation-subscribe-event'),
61+
...require('./recommendation'),
62+
...require('./redirect'),
63+
...require('./role'),
64+
...require('./session'),
65+
...require('./settings'),
66+
...require('./single-use-token'),
67+
...require('./snippet'),
68+
...require('./stripe-customer-subscription'),
69+
...require('./stripe-price'),
70+
...require('./stripe-product'),
71+
...require('./subscription-created-event'),
72+
...require('./suppression'),
73+
...require('./tag-public'),
74+
...require('./tag'),
75+
...require('./user'),
76+
...require('./webhook')
77+
};
3078

3179
/**
32-
* Expose `init`
80+
* @deprecated: remove this once we've removed it from everywhere
3381
*/
34-
35-
exports.init = init;
82+
module.exports.init = function init() {
83+
if (process.env.NODE_ENV !== 'production') {
84+
// eslint-disable-next-line no-console
85+
console.warn('@deprecated: models.init() is deprecated. Models are now automatically required.');
86+
}
87+
};

‎ghost/core/core/server/models/user.js

+1-1
Original file line numberDiff line numberDiff line change
@@ -9,7 +9,6 @@ const errors = require('@tryghost/errors');
99
const security = require('@tryghost/security');
1010
const {pipeline} = require('@tryghost/promise');
1111
const validatePassword = require('../lib/validate-password');
12-
const permissions = require('../services/permissions');
1312
const urlUtils = require('../../shared/url-utils');
1413
const activeStates = ['active', 'warn-1', 'warn-2', 'warn-3', 'warn-4'];
1514
const ASSIGNABLE_ROLES = ['Administrator', 'Editor', 'Author', 'Contributor'];
@@ -905,6 +904,7 @@ User = ghostBookshelf.Model.extend({
905904
// @NOTE: your role is not the same than the role you try to change (!)
906905
// e.g. admin can assign admin role to a user, but not owner
907906

907+
const permissions = require('../services/permissions');
908908
return permissions.canThis(context).assign.role(role)
909909
.then(() => {
910910
if (hasUserPermission && hasApiKeyPermission) {

‎ghost/core/core/server/services/update-check/run-update-check.js

-4
Original file line numberDiff line numberDiff line change
@@ -31,10 +31,6 @@ if (parentPort) {
3131
(async () => {
3232
const updateCheck = require('./');
3333

34-
// INIT required services
35-
const models = require('../../models');
36-
models.init();
37-
3834
const permissions = require('../permissions');
3935
await permissions.init();
4036

‎ghost/core/core/server/web/api/testmode/jobs/graceful-job.js

-2
Original file line numberDiff line numberDiff line change
@@ -25,8 +25,6 @@ const internalContext = {context: {internal: true}};
2525
(async () => {
2626
const models = require('../../../../models');
2727

28-
await models.init();
29-
3028
postParentPortMessage(`Fetching tags`);
3129
const tags = await models.Tag.findPage(internalContext);
3230

‎ghost/core/test/integration/url_service.test.js

-5
Original file line numberDiff line numberDiff line change
@@ -2,16 +2,11 @@ const should = require('should');
22
const sinon = require('sinon');
33
const testUtils = require('../utils');
44
const configUtils = require('../utils/configUtils');
5-
const models = require('../../core/server/models');
65
const UrlService = require('../../core/server/services/url/UrlService');
76

87
describe('Integration: services/url/UrlService', function () {
98
let urlService;
109

11-
before(function () {
12-
models.init();
13-
});
14-
1510
before(testUtils.teardownDb);
1611
before(testUtils.setup('users:roles', 'posts'));
1712
after(testUtils.teardownDb);

‎ghost/core/test/regression/api/admin/schedules.test.js

-2
Original file line numberDiff line numberDiff line change
@@ -15,8 +15,6 @@ describe('Schedules API', function () {
1515
let request;
1616

1717
before(function () {
18-
models.init();
19-
2018
// @NOTE: mock the post scheduler, otherwise it will auto publish the post
2119
sinon.stub(SchedulingDefault.prototype, '_pingUrl').resolves();
2220
});

‎ghost/core/test/regression/models/model_settings.test.js

-1
Original file line numberDiff line numberDiff line change
@@ -8,7 +8,6 @@ const models = require('../../../core/server/models');
88
const SETTINGS_LENGTH = 94;
99

1010
describe('Settings Model', function () {
11-
before(models.init);
1211
afterEach(testUtils.teardownDb);
1312

1413
describe('defaults', function () {

‎ghost/core/test/unit/api/cache-invalidation.test.js

-7
Original file line numberDiff line numberDiff line change
@@ -3,15 +3,8 @@ const path = require('path');
33

44
const glob = require('glob');
55

6-
const models = require('../../../core/server/models');
7-
86
describe('API', function () {
97
describe('Cache Invalidation', function () {
10-
before(async function () {
11-
// Initialise models - Utilised by various endpoints to reference static fields (i.e models.Post.allowedFormats) when required in
12-
models.init();
13-
});
14-
158
it('Controller actions explicitly declare cacheInvalidate header', async function () {
169
const controllersRootPath = path.join(__dirname, '../../../core/server/api/endpoints');
1710
const controllerPaths = glob.sync('*.js', {

‎ghost/core/test/unit/api/canary/session.test.js

-4
Original file line numberDiff line numberDiff line change
@@ -8,10 +8,6 @@ const sessionController = require('../../../../core/server/api/endpoints/session
88
const sessionServiceMiddleware = require('../../../../core/server/services/auth/session');
99

1010
describe('Session controller', function () {
11-
before(function () {
12-
models.init();
13-
});
14-
1511
afterEach(function () {
1612
sinon.restore();
1713
});

‎ghost/core/test/unit/api/canary/utils/validators/input/pages.test.js

-4
Original file line numberDiff line numberDiff line change
@@ -5,10 +5,6 @@ const validators = require('../../../../../../../core/server/api/endpoints/utils
55
const models = require('../../../../../../../core/server/models');
66

77
describe('Unit: endpoints/utils/validators/input/pages', function () {
8-
before(function () {
9-
return models.init();
10-
});
11-
128
beforeEach(function () {
139
const memberFindPageStub = sinon.stub(models.Member, 'findPage').returns(Promise.reject());
1410
memberFindPageStub.withArgs({filter: 'label:vip', limit: 1}).returns(Promise.resolve());

‎ghost/core/test/unit/api/canary/utils/validators/input/posts.test.js

-4
Original file line numberDiff line numberDiff line change
@@ -5,10 +5,6 @@ const validators = require('../../../../../../../core/server/api/endpoints/utils
55
const models = require('../../../../../../../core/server/models');
66

77
describe('Unit: endpoints/utils/validators/input/posts', function () {
8-
before(function () {
9-
models.init();
10-
});
11-
128
beforeEach(function () {
139
const memberFindPageStub = sinon.stub(models.Member, 'findPage').returns(Promise.reject());
1410
memberFindPageStub.withArgs({filter: 'label:vip', limit: 1}).returns(Promise.resolve());

‎ghost/core/test/unit/frontend/helpers/authors.test.js

-5
Original file line numberDiff line numberDiff line change
@@ -2,14 +2,9 @@ const should = require('should');
22
const sinon = require('sinon');
33
const urlService = require('../../../../core/server/services/url');
44
const authorsHelper = require('../../../../core/frontend/helpers/authors');
5-
const models = require('../../../../core/server/models');
65
const testUtils = require('../../../utils');
76

87
describe('{{authors}} helper', function () {
9-
before(function () {
10-
models.init();
11-
});
12-
138
beforeEach(function () {
149
sinon.stub(urlService, 'getUrlByResourceId');
1510
});

‎ghost/core/test/unit/frontend/helpers/get.test.js

-5
Original file line numberDiff line numberDiff line change
@@ -7,7 +7,6 @@ const loggingLib = require('@tryghost/logging');
77

88
// Stuff we are testing
99
const get = require('../../../../core/frontend/helpers/get');
10-
const models = require('../../../../core/server/models');
1110
const proxy = require('../../../../core/frontend/services/proxy');
1211
const api = require('../../../../core/server/api').endpoints;
1312

@@ -17,10 +16,6 @@ describe('{{#get}} helper', function () {
1716
let locals = {};
1817
let logging;
1918

20-
before(function () {
21-
models.init();
22-
});
23-
2419
beforeEach(function () {
2520
fn = sinon.spy();
2621
inverse = sinon.spy();

‎ghost/core/test/unit/frontend/helpers/ghost_head.test.js

+1-5
Original file line numberDiff line numberDiff line change
@@ -6,7 +6,6 @@ const _ = require('lodash');
66
const moment = require('moment');
77
const testUtils = require('../../../utils');
88
const configUtils = require('../../../utils/configUtils');
9-
const models = require('../../../../core/server/models');
109
const imageLib = require('../../../../core/server/lib/image');
1110
const routing = require('../../../../core/frontend/services/routing');
1211
const urlService = require('../../../../core/server/services/url');
@@ -358,9 +357,6 @@ describe('{{ghost_head}} helper', function () {
358357
};
359358

360359
before(function () {
361-
// @TODO: remove when visibility is refactored out of models
362-
models.init();
363-
364360
keyStub = sinon.stub().resolves('xyz');
365361
const dataService = {
366362
getFrontendKey: keyStub
@@ -1547,7 +1543,7 @@ describe('{{ghost_head}} helper', function () {
15471543
}));
15481544

15491545
rendered.should.match(/data-datasource="analytics_events_json_v1"/);
1550-
});
1546+
});
15511547
});
15521548
describe('respects values from excludes: ', function () {
15531549
it('when excludes is empty', async function () {

‎ghost/core/test/unit/frontend/helpers/recommendations.test.js

-3
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,5 @@
11
const should = require('should');
22
const sinon = require('sinon');
3-
const models = require('../../../../core/server/models');
43
const api = require('../../../../core/server/api').endpoints;
54
const hbs = require('../../../../core/frontend/services/theme-engine/engine');
65
const configUtils = require('../../../utils/configUtils');
@@ -21,8 +20,6 @@ describe('{{#recommendations}} helper', function () {
2120
let logging;
2221

2322
before(function () {
24-
models.init();
25-
2623
hbs.express4({
2724
partialsDir: [configUtils.config.get('paths').helperTemplates]
2825
});

0 commit comments

Comments
 (0)