Skip to content

Commit

Permalink
Migrated schedules controller to v2
Browse files Browse the repository at this point in the history
closes TryGhost#10060

- Implemented scheduling for posts and pages
- Added cache invalidation when scheduling
- Refactored admin token eneration function to accept existing key as parameter in tests
- Added Ghost Scheduler Integration fixture
- Added fixture for permissions for post publish action
- Migrated getScheduled method to v2
- Did not add support for 'from' and 'to' parameters as they were not used by DefaultScheduler
- This method needs rethinking in a long run as it's an ugly hack and should rather become proper endpoint that returns JSON data instead of models
- Removed unused auth middleware from v2 routes
- Added internal scheduler role
- Implemetnted transactions in v2 frame
- This takes into account scenario mentioned in c93f03b
- Specifically:
>if two queries happening in a transaction we have to signalise
  knex/mysql that we select for an update
  otherwise the following case happens:
  you fetch posts for an update
  a user requests comes in and updates the post (e.g. sets title to "X")
  you update the fetched posts, title would get overriden to the old one
  • Loading branch information
naz committed Aug 7, 2019
1 parent 42c9904 commit 00f95e7
Show file tree
Hide file tree
Showing 16 changed files with 395 additions and 23 deletions.
3 changes: 2 additions & 1 deletion core/server/adapters/scheduling/post-scheduling/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@ _private.normalize = function normalize(options) {
const {model, apiUrl, client} = options;

return {
// NOTE: The scheduler expects a unix timestmap.
// NOTE: The scheduler expects a unix timestamp.
time: moment(model.get('published_at')).valueOf(),
// @TODO: We are still using API v0.1
url: `${urlUtils.urlJoin(apiUrl, 'schedules', 'posts', model.get('id'))}?client_id=${client.get('slug')}&client_secret=${client.get('secret')}`,
Expand All @@ -41,6 +41,7 @@ _private.loadClient = function loadClient() {
* @return {Promise}
*/
_private.loadScheduledPosts = function () {
// TODO: make this version aware?
const api = require('../../../api');
return api.schedules.getScheduledPosts()
.then((result) => {
Expand Down
5 changes: 5 additions & 0 deletions core/server/api/shared/validators/input/all.js
Original file line number Diff line number Diff line change
Expand Up @@ -197,5 +197,10 @@ module.exports = {
setup() {
debug('validate setup');
return this.add(...arguments);
},

publish() {
debug('validate schedule');
return this.browse(...arguments);
}
};
4 changes: 4 additions & 0 deletions core/server/api/v2/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,10 @@ module.exports = {
return require('./session');
},

get schedules() {
return shared.pipeline(require('./schedules'), localUtils);
},

get pages() {
return shared.pipeline(require('./pages'), localUtils);
},
Expand Down
10 changes: 8 additions & 2 deletions core/server/api/v2/pages.js
Original file line number Diff line number Diff line change
Expand Up @@ -43,7 +43,10 @@ module.exports = {
'fields',
'formats',
'debug',
'absolute_urls'
'absolute_urls',
// NOTE: only for internal context
'forUpdate',
'transacting'
],
data: [
'id',
Expand Down Expand Up @@ -113,7 +116,10 @@ module.exports = {
headers: {},
options: [
'include',
'id'
'id',
// NOTE: only for internal context
'forUpdate',
'transacting'
],
validation: {
options: {
Expand Down
10 changes: 8 additions & 2 deletions core/server/api/v2/posts.js
Original file line number Diff line number Diff line change
Expand Up @@ -42,7 +42,10 @@ module.exports = {
'fields',
'formats',
'debug',
'absolute_urls'
'absolute_urls',
// NOTE: only for internal context
'forUpdate',
'transacting'
],
data: [
'id',
Expand Down Expand Up @@ -115,7 +118,10 @@ module.exports = {
options: [
'include',
'id',
'source'
'source',
// NOTE: only for internal context
'forUpdate',
'transacting'
],
validation: {
options: {
Expand Down
130 changes: 130 additions & 0 deletions core/server/api/v2/schedules.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,130 @@
const _ = require('lodash');
const moment = require('moment');
const config = require('../../config');
const models = require('../../models');
const urlUtils = require('../../lib/url-utils');
const common = require('../../lib/common');
const api = require('./index');

module.exports = {
docName: 'schedules',
publish: {
headers: {},
options: [
'id',
'resource'
],
data: [
'force'
],
validation: {
options: {
id: {
required: true
},
resource: {
required: true,
values: ['posts', 'pages']
}
}
},
permissions: {
docName: 'posts'
},
query(frame) {
let resource;
const resourceType = frame.options.resource;
const publishAPostBySchedulerToleranceInMinutes = config.get('times').publishAPostBySchedulerToleranceInMinutes;

return models.Base.transaction((transacting) => {
const options = {
transacting: transacting,
status: 'scheduled',
forUpdate: true,
id: frame.options.id,
context: {
internal: true
}
};

return api[resourceType].read({id: frame.options.id}, options)
.then((result) => {
resource = result[resourceType][0];
const publishedAtMoment = moment(resource.published_at);

if (publishedAtMoment.diff(moment(), 'minutes') > publishAPostBySchedulerToleranceInMinutes) {
return Promise.reject(new common.errors.NotFoundError({message: common.i18n.t('errors.api.job.notFound')}));
}

if (publishedAtMoment.diff(moment(), 'minutes') < publishAPostBySchedulerToleranceInMinutes * -1 && frame.data.force !== true) {
return Promise.reject(new common.errors.NotFoundError({message: common.i18n.t('errors.api.job.publishInThePast')}));
}

const editedResource = {};
editedResource[resourceType] = [{
status: 'published',
updated_at: moment(resource.updated_at).toISOString(true)
}];

return api[resourceType].edit(
editedResource,
_.pick(options, ['context', 'id', 'transacting', 'forUpdate'])
);
})
.then((result) => {
const scheduledResource = result[resourceType][0];

if (
(scheduledResource.status === 'published' && resource.status !== 'published') ||
(scheduledResource.status === 'draft' && resource.status === 'published')
) {
this.headers.cacheInvalidate = true;
} else if (
(scheduledResource.status === 'draft' && resource.status !== 'published') ||
(scheduledResource.status === 'scheduled' && resource.status !== 'scheduled')
) {
this.headers.cacheInvalidate = {
value: urlUtils.urlFor({
relativeUrl: urlUtils.urlJoin('/p', scheduledResource.uuid, '/')
})
};
} else {
this.headers.cacheInvalidate = false;
}

return result;
});
});
}
},

getScheduled: {
// NOTE: this method is for internal use only by DefaultScheduler
// it is not exposed anywhere!
permissions: false,
validation: {
options: {
resource: {
required: true,
values: ['posts', 'pages']
}
}
},
query(frame) {
const resourceType = frame.options.resource;
const resourceModel = (resourceType === 'posts') ? 'Post' : 'Page';

const cleanOptions = {};
cleanOptions.filter = 'status:scheduled';
cleanOptions.columns = ['id', 'published_at', 'created_at'];

return models[resourceModel].findAll(cleanOptions)
.then((result) => {
let response = {};
response[resourceType] = result;

return response;
});
}
}
};
4 changes: 4 additions & 0 deletions core/server/api/v2/utils/serializers/output/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,10 @@ module.exports = {
return require('./slugs');
},

get schedules() {
return require('./schedules');
},

get webhooks() {
return require('./webhooks');
},
Expand Down
5 changes: 5 additions & 0 deletions core/server/api/v2/utils/serializers/output/schedules.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
module.exports = {
all(model, apiConfig, frame) {
frame.response = model;
}
};
19 changes: 19 additions & 0 deletions core/server/data/schema/fixtures/fixtures.json
Original file line number Diff line number Diff line change
Expand Up @@ -70,6 +70,10 @@
{
"name": "DB Backup Integration",
"description": "Internal DB Backup Client"
},
{
"name": "Scheduler Integration",
"description": "Internal Scheduler Client"
}
]
},
Expand Down Expand Up @@ -141,6 +145,11 @@
"action_type": "destroy",
"object_type": "post"
},
{
"name": "Publish posts",
"action_type": "publish",
"object_type": "post"
},
{
"name": "Browse settings",
"action_type": "browse",
Expand Down Expand Up @@ -584,6 +593,13 @@
"description": "Internal DB Backup integration",
"type": "internal",
"api_keys": [{"type": "admin", "role": "DB Backup Integration"}]
},
{
"slug": "ghost-scheduler",
"name": "Ghost Scheduler",
"description": "Internal Scheduler integration",
"type": "internal",
"api_keys": [{"type": "admin", "role": "Scheduler Integration"}]
}
]
}
Expand Down Expand Up @@ -624,6 +640,9 @@
"DB Backup Integration": {
"db": "all"
},
"Scheduler Integration": {
"post": "publish"
},
"Admin Integration": {
"mail": "all",
"notification": "all",
Expand Down
3 changes: 2 additions & 1 deletion core/server/web/api/v2/admin/middleware.js
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,8 @@ const notImplemented = function (req, res, next) {
themes: ['POST', 'PUT'],
subscribers: ['GET', 'PUT', 'DELETE', 'POST'],
config: ['GET'],
webhooks: ['POST', 'DELETE']
webhooks: ['POST', 'DELETE'],
schedules: ['PUT']
};

const match = req.url.match(/^\/(\w+)\/?/);
Expand Down
6 changes: 1 addition & 5 deletions core/server/web/api/v2/admin/routes.js
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,6 @@ const api = require('../../../../api');
const apiv2 = require('../../../../api/v2');
const mw = require('./middleware');

const auth = require('../../../../services/auth');
const shared = require('../../../shared');

// Handling uploads & imports
Expand Down Expand Up @@ -50,10 +49,7 @@ module.exports = function apiRoutes() {
router.del('/integrations/:id', mw.authAdminApi, http(apiv2.integrations.destroy));

// ## Schedules
router.put('/schedules/posts/:id', [
auth.authenticate.authenticateClient,
auth.authenticate.authenticateUser
], api.http(api.schedules.publishPost));
router.put('/schedules/:resource/:id', mw.authAdminApi, http(apiv2.schedules.publish));

// ## Settings
router.get('/settings/routes/yaml', mw.authAdminApi, http(apiv2.settings.download));
Expand Down
3 changes: 2 additions & 1 deletion core/test/acceptance/old/admin/roles_spec.js
Original file line number Diff line number Diff line change
Expand Up @@ -35,14 +35,15 @@ describe('Roles API', function () {
should.exist(response);
should.exist(response.roles);
localUtils.API.checkResponse(response, 'roles');
response.roles.should.have.length(7);
response.roles.should.have.length(8);
localUtils.API.checkResponse(response.roles[0], 'role');
localUtils.API.checkResponse(response.roles[1], 'role');
localUtils.API.checkResponse(response.roles[2], 'role');
localUtils.API.checkResponse(response.roles[3], 'role');
localUtils.API.checkResponse(response.roles[4], 'role');
localUtils.API.checkResponse(response.roles[5], 'role');
localUtils.API.checkResponse(response.roles[6], 'role');
localUtils.API.checkResponse(response.roles[7], 'role');

done();
});
Expand Down

0 comments on commit 00f95e7

Please sign in to comment.