mirror of
https://github.com/TryGhost/Ghost.git
synced 2025-03-11 02:12:21 -05:00
Limited the API surface of the UpdateCheckService
refs https://github.com/TryGhost/Team/issues/728 - This is continuation of the previous commit. TLDR: Passing only the necessary API endpoint function makes it easier to reason about what dependencies the UpdateCheckService has to deal with - There are 8 different configs that NotificationService depends upon it will need some further investigation around which ones are even needed anymore and the naming is not the best. To keep the time cap at bay leaving it at what it is.
This commit is contained in:
Naz
parent
bdab32d30a
commit
2e7d0a4e26
3 changed files with 47 additions and 25 deletions
|
|
@ -29,6 +29,15 @@ class UpdateCheckService {
|
|||
* @param {Function} options.api.posts.browse - method allowing to read Ghost's posts
|
||||
* @param {Object} options.api.users - Users API methods
|
||||
* @param {Function} options.api.users.browse - method allowing to read Ghost's users
|
||||
* @param {Object} options.config
|
||||
* @param {Object} options.config.mail
|
||||
* @param {string} options.config.env
|
||||
* @param {string} options.config.databaseType
|
||||
* @param {string} options.config.checkEndpoint - update check service URL
|
||||
* @param {boolean} [options.config.isPrivacyDisabled]
|
||||
* @param {string[]} [options.config.notificationGroups] - example values ["migration", "something"]
|
||||
* @param {string} options.config.siteUrl - Ghost instance URL
|
||||
* @param {boolean} [options.config.forceUpdate]
|
||||
*/
|
||||
constructor({api, config, i18n, logging, urlUtils, request, ghostVersion, ghostMailer}) {
|
||||
this.api = api;
|
||||
|
|
@ -78,12 +87,12 @@ class UpdateCheckService {
|
|||
*/
|
||||
async updateCheckData() {
|
||||
let data = {};
|
||||
let mailConfig = this.config.get('mail');
|
||||
let mailConfig = this.config.mail;
|
||||
|
||||
data.ghost_version = this.ghostVersion.original;
|
||||
data.node_version = process.versions.node;
|
||||
data.env = this.config.get('env');
|
||||
data.database_type = this.config.get('database').client;
|
||||
data.env = this.config.env;
|
||||
data.database_type = this.config.databaseType;
|
||||
data.email_transport = mailConfig &&
|
||||
(mailConfig.options && mailConfig.options.service ?
|
||||
mailConfig.options.service :
|
||||
|
|
@ -131,8 +140,8 @@ class UpdateCheckService {
|
|||
headers: {}
|
||||
};
|
||||
|
||||
let checkEndpoint = this.config.get('updateCheck:url');
|
||||
let checkMethod = this.config.isPrivacyDisabled('useUpdateCheck') ? 'GET' : 'POST';
|
||||
let checkEndpoint = this.config.checkEndpoint;
|
||||
let checkMethod = this.config.isPrivacyDisabled ? 'GET' : 'POST';
|
||||
|
||||
// CASE: Expose stats and do a check-in
|
||||
if (checkMethod === 'POST') {
|
||||
|
|
@ -210,7 +219,7 @@ class UpdateCheckService {
|
|||
*/
|
||||
async updateCheckResponse(response) {
|
||||
let notifications = [];
|
||||
let notificationGroups = (this.config.get('notificationGroups') || []).concat(['all']);
|
||||
let notificationGroups = (this.config.notificationGroups || []).concat(['all']);
|
||||
|
||||
debug('Notification Groups', notificationGroups);
|
||||
debug('Response Update Check Service', response);
|
||||
|
|
@ -284,7 +293,7 @@ class UpdateCheckService {
|
|||
.filter(user => ['Owner', 'Administrator'].includes(user.roles[0].name))
|
||||
.map(user => user.email);
|
||||
|
||||
const siteUrl = this.config.get('url');
|
||||
const siteUrl = this.config.siteUrl;
|
||||
|
||||
for (const message of notification.messages) {
|
||||
const toAdd = {
|
||||
|
|
@ -333,7 +342,7 @@ class UpdateCheckService {
|
|||
|
||||
// CASE: Next update check should happen now?
|
||||
// @NOTE: You can skip this check by adding a config value. This is helpful for developing.
|
||||
if (!this.config.get('updateCheck:forceUpdate') && nextUpdateCheck && nextUpdateCheck.value && nextUpdateCheck.value > moment().unix()) {
|
||||
if (!this.config.forceUpdate && nextUpdateCheck && nextUpdateCheck.value && nextUpdateCheck.value > moment().unix()) {
|
||||
return;
|
||||
}
|
||||
|
||||
|
|
|
|||
|
|
@ -36,7 +36,16 @@ module.exports = () => {
|
|||
browse: api.users.browse
|
||||
}
|
||||
},
|
||||
config,
|
||||
config: {
|
||||
mail: config.get('mail'),
|
||||
env: config.get('env'),
|
||||
databaseType: config.get('database').client,
|
||||
checkEndpoint: config.get('updateCheck:url'),
|
||||
isPrivacyDisabled: config.isPrivacyDisabled('useUpdateCheck'),
|
||||
notificationGroups: this.config.get('notificationGroups'),
|
||||
siteUrl: config.get('url'),
|
||||
forceUpdate: config.get('updateCheck:forceUpdate')
|
||||
},
|
||||
i18n,
|
||||
logging,
|
||||
urlUtils,
|
||||
|
|
|
|||
|
|
@ -2,7 +2,6 @@ const should = require('should');
|
|||
const sinon = require('sinon');
|
||||
const moment = require('moment');
|
||||
const uuid = require('uuid');
|
||||
const configUtils = require('../../utils/configUtils');
|
||||
const urlUtils = require('../../utils/urlUtils');
|
||||
const packageInfo = require('../../../package.json');
|
||||
const ghostVersion = require('../../../core/server/lib/ghost-version');
|
||||
|
|
@ -47,7 +46,6 @@ describe('Update Check', function () {
|
|||
afterEach(function () {
|
||||
sinon.restore();
|
||||
urlUtils.restore();
|
||||
configUtils.restore();
|
||||
});
|
||||
|
||||
describe('UpdateCheck execution', function () {
|
||||
|
|
@ -65,7 +63,10 @@ describe('Update Check', function () {
|
|||
browse: sinon.stub().resolves()
|
||||
}
|
||||
},
|
||||
config: configUtils.config,
|
||||
config: {
|
||||
checkEndpoint: 'https://updates.ghost.org',
|
||||
isPrivacyDisabled: true
|
||||
},
|
||||
i18n: i18nStub,
|
||||
logging: loggingStub,
|
||||
urlUtils: urlUtilsStub,
|
||||
|
|
@ -97,7 +98,8 @@ describe('Update Check', function () {
|
|||
read: lateSettingStub
|
||||
}
|
||||
},
|
||||
config: configUtils.config
|
||||
config: {},
|
||||
request: requestStub
|
||||
});
|
||||
|
||||
await updateCheckService.check();
|
||||
|
|
@ -125,7 +127,10 @@ describe('Update Check', function () {
|
|||
browse: sinon.stub().resolves()
|
||||
}
|
||||
},
|
||||
config: configUtils.config,
|
||||
config: {
|
||||
checkEndpoint: 'https://updates.ghost.org',
|
||||
isPrivacyDisabled: true
|
||||
},
|
||||
i18n: i18nStub,
|
||||
logging: loggingStub,
|
||||
urlUtils: urlUtilsStub,
|
||||
|
|
@ -146,14 +151,6 @@ describe('Update Check', function () {
|
|||
});
|
||||
|
||||
describe('Data sent with the POST request', function () {
|
||||
before(function () {
|
||||
configUtils.set('privacy:useUpdateCheck', true);
|
||||
});
|
||||
|
||||
after(function () {
|
||||
configUtils.restore();
|
||||
});
|
||||
|
||||
it('should report the correct data', async function () {
|
||||
const updateCheckService = new UpdateCheckService({
|
||||
api: {
|
||||
|
|
@ -178,7 +175,12 @@ describe('Update Check', function () {
|
|||
})
|
||||
}
|
||||
},
|
||||
config: configUtils.config,
|
||||
config: {
|
||||
checkEndpoint: 'https://updates.ghost.org',
|
||||
isPrivacyDisabled: false,
|
||||
env: process.env.NODE_ENV,
|
||||
databaseType: 'mysql'
|
||||
},
|
||||
i18n: i18nStub,
|
||||
logging: loggingStub,
|
||||
urlUtils: urlUtilsStub,
|
||||
|
|
@ -249,7 +251,7 @@ describe('Update Check', function () {
|
|||
add: notificationsAPIAddStub
|
||||
}
|
||||
},
|
||||
config: configUtils.config,
|
||||
config: {},
|
||||
i18n: i18nStub,
|
||||
logging: loggingStub,
|
||||
urlUtils: urlUtilsStub,
|
||||
|
|
@ -318,7 +320,9 @@ describe('Update Check', function () {
|
|||
add: notificationsAPIAddStub
|
||||
}
|
||||
},
|
||||
config: configUtils.config,
|
||||
config: {
|
||||
siteUrl: 'http://127.0.0.1:2369'
|
||||
},
|
||||
i18n: i18nStub,
|
||||
logging: loggingStub,
|
||||
urlUtils: urlUtilsStub,
|
||||
|
|
|
|||
Loading…
Add table
Reference in a new issue