Refactored the API layer: do not handle API response after pipelining

no issue

- this has a big underlying problem
- each task in the pipeline can modify the options
- e.g. add a proper permission context
- if we chain after the pipeline, we don't have access to the modified options object
- and then we pass the wrong options into the `toJSON` function of a model
- the toJSON function decides what to return based on options
- this is the easiest solution for now, but i am going to write a spec if we can solve this problem differently
This commit is contained in:
kirrg001 2017-09-27 10:31:41 +02:00 committed by Hannah Wolfe
parent a6d57d6324
commit 1e2beface1
9 changed files with 224 additions and 154 deletions

View File

@ -35,7 +35,19 @@ clients = {
function doQuery(options) {
// only User Agent (type = `ua`) clients are available at the moment.
options.data = _.extend(options.data, {type: 'ua'});
return models.Client.findOne(options.data, _.omit(options, ['data']));
return models.Client.findOne(options.data, _.omit(options, ['data']))
.then(function onModelResponse(model) {
if (!model) {
return Promise.reject(new errors.NotFoundError({
message: i18n.t('common.api.clients.clientNotFound')
}));
}
return {
clients: [model.toJSON(options)]
};
});
}
// Push all of our tasks into a `tasks` array in the correct order
@ -47,13 +59,7 @@ clients = {
];
// Pipeline calls each task passing the result of one to be the arguments for the next
return pipeline(tasks, options).then(function formatResponse(result) {
if (result) {
return {clients: [result.toJSON(options)]};
}
return Promise.reject(new errors.NotFoundError({message: i18n.t('common.api.clients.clientNotFound')}));
});
return pipeline(tasks, options);
}
};

View File

@ -37,7 +37,18 @@ invites = {
tasks;
function modelQuery(options) {
return models.Invite.findOne(options.data, _.omit(options, ['data']));
return models.Invite.findOne(options.data, _.omit(options, ['data']))
.then(function onModelResponse(model) {
if (!model) {
return Promise.reject(new errors.NotFoundError({
message: i18n.t('errors.api.invites.inviteNotFound')
}));
}
return {
invites: [model.toJSON(options)]
};
});
}
tasks = [
@ -47,14 +58,7 @@ invites = {
modelQuery
];
return pipeline(tasks, options)
.then(function formatResponse(result) {
if (result) {
return {invites: [result.toJSON(options)]};
}
return Promise.reject(new errors.NotFoundError({message: i18n.t('errors.api.invites.inviteNotFound')}));
});
return pipeline(tasks, options);
},
destroy: function destroy(options) {
@ -131,7 +135,10 @@ invites = {
}).then(function () {
invite.set('status', 'sent');
var inviteAsJSON = invite.toJSON();
return {invites: [inviteAsJSON]};
return {
invites: [inviteAsJSON]
};
}).catch(function (error) {
if (error && error.errorType === 'EmailError') {
error.message = i18n.t('errors.api.invites.errorSendingEmail.error', {message: error.message}) + ' ' +

View File

@ -103,7 +103,9 @@ notifications = {
}
});
return addedNotifications;
return {
notifications: addedNotifications
};
}
tasks = [
@ -112,9 +114,7 @@ notifications = {
saveNotifications
];
return pipeline(tasks, object, options).then(function formatResponse(result) {
return {notifications: result};
});
return pipeline(tasks, object, options);
},
/**

View File

@ -89,7 +89,18 @@ posts = {
* @returns {Object} options
*/
function modelQuery(options) {
return models.Post.findOne(options.data, _.omit(options, ['data']));
return models.Post.findOne(options.data, _.omit(options, ['data']))
.then(function onModelResponse(model) {
if (!model) {
return Promise.reject(new errors.NotFoundError({
message: i18n.t('errors.api.posts.postNotFound')
}));
}
return {
posts: [model.toJSON(options)]
};
});
}
// Push all of our tasks into a `tasks` array in the correct order
@ -101,14 +112,7 @@ posts = {
];
// Pipeline calls each task passing the result of one to be the arguments for the next
return pipeline(tasks, options).then(function formatResponse(result) {
// @TODO make this a formatResponse task?
if (result) {
return {posts: [result.toJSON(options)]};
}
return Promise.reject(new errors.NotFoundError({message: i18n.t('errors.api.posts.postNotFound')}));
});
return pipeline(tasks, options);
},
/**
@ -130,7 +134,27 @@ posts = {
* @returns {Object} options
*/
function modelQuery(options) {
return models.Post.edit(options.data.posts[0], _.omit(options, ['data']));
return models.Post.edit(options.data.posts[0], _.omit(options, ['data']))
.then(function onModelResponse(model) {
if (!model) {
return Promise.reject(new errors.NotFoundError({
message: i18n.t('errors.api.posts.postNotFound')
}));
}
var post = model.toJSON(options);
// If previously was not published and now is (or vice versa), signal the change
// @TODO: `statusChanged` get's added for the API headers only. Reconsider this.
post.statusChanged = false;
if (model.updated('status') !== model.get('status')) {
post.statusChanged = true;
}
return {
posts: [post]
};
});
}
// Push all of our tasks into a `tasks` array in the correct order
@ -142,20 +166,7 @@ posts = {
];
// Pipeline calls each task passing the result of one to be the arguments for the next
return pipeline(tasks, object, options).then(function formatResponse(result) {
if (result) {
var post = result.toJSON(options);
// If previously was not published and now is (or vice versa), signal the change
post.statusChanged = false;
if (result.updated('status') !== result.get('status')) {
post.statusChanged = true;
}
return {posts: [post]};
}
return Promise.reject(new errors.NotFoundError({message: i18n.t('errors.api.posts.postNotFound')}));
});
return pipeline(tasks, object, options);
},
/**
@ -177,7 +188,17 @@ posts = {
* @returns {Object} options
*/
function modelQuery(options) {
return models.Post.add(options.data.posts[0], _.omit(options, ['data']));
return models.Post.add(options.data.posts[0], _.omit(options, ['data']))
.then(function onModelResponse(model) {
var post = model.toJSON(options);
if (post.status === 'published') {
// When creating a new post that is published right now, signal the change
post.statusChanged = true;
}
return {posts: [post]};
});
}
// Push all of our tasks into a `tasks` array in the correct order
@ -189,15 +210,7 @@ posts = {
];
// Pipeline calls each task passing the result of one to be the arguments for the next
return pipeline(tasks, object, options).then(function formatResponse(result) {
var post = result.toJSON(options);
if (post.status === 'published') {
// When creating a new post that is published right now, signal the change
post.statusChanged = true;
}
return {posts: [post]};
});
return pipeline(tasks, object, options);
},
/**

View File

@ -39,20 +39,10 @@ roles = {
* @returns {Object} options
*/
function modelQuery(options) {
return models.Role.findAll(options);
}
// Push all of our tasks into a `tasks` array in the correct order
tasks = [
apiUtils.validate(docName, {opts: permittedOptions}),
apiUtils.handlePermissions(docName, 'browse'),
modelQuery
];
// Pipeline calls each task passing the result of one to be the arguments for the next
return pipeline(tasks, options).then(function formatResponse(results) {
var roles = results.map(function (r) {
return r.toJSON();
return models.Role.findAll(options)
.then(function onModelResponse(models) {
var roles = models.map(function (role) {
return role.toJSON();
});
if (options.permissions !== 'assign') {
@ -66,10 +56,23 @@ roles = {
}), function (value) {
return value && value.name !== 'Owner';
}).then(function (roles) {
return {roles: roles};
return {
roles: roles
};
});
});
}
// Push all of our tasks into a `tasks` array in the correct order
tasks = [
apiUtils.validate(docName, {opts: permittedOptions}),
apiUtils.handlePermissions(docName, 'browse'),
modelQuery
];
// Pipeline calls each task passing the result of one to be the arguments for the next
return pipeline(tasks, options);
}
};
module.exports = roles;

View File

@ -57,7 +57,18 @@ slugs = {
* @returns {Object} options
*/
function modelQuery(options) {
return models.Base.Model.generateSlug(allowedTypes[options.type], options.data.name, {status: 'all'});
return models.Base.Model.generateSlug(allowedTypes[options.type], options.data.name, {status: 'all'})
.then(function onModelResponse(slug) {
if (!slug) {
return Promise.reject(new errors.GhostError({
message: i18n.t('errors.api.slugs.couldNotGenerateSlug')
}));
}
return {
slugs: [{slug: slug}]
};
});
}
// Push all of our tasks into a `tasks` array in the correct order
@ -69,13 +80,7 @@ slugs = {
];
// Pipeline calls each task passing the result of one to be the arguments for the next
return pipeline(tasks, options).then(function (slug) {
if (!slug) {
return Promise.reject(new errors.GhostError({message: i18n.t('errors.api.slugs.couldNotGenerateSlug')}));
}
return {slugs: [{slug: slug}]};
});
return pipeline(tasks, options);
}
};

View File

@ -63,7 +63,18 @@ subscribers = {
* @returns {Object} options
*/
function doQuery(options) {
return models.Subscriber.findOne(options.data, _.omit(options, ['data']));
return models.Subscriber.findOne(options.data, _.omit(options, ['data']))
.then(function onModelResponse(model) {
if (!model) {
return Promise.reject(new errors.NotFoundError({
message: i18n.t('errors.api.subscribers.subscriberNotFound')
}));
}
return {
subscribers: [model.toJSON(options)]
};
});
}
// Push all of our tasks into a `tasks` array in the correct order
@ -74,13 +85,7 @@ subscribers = {
];
// Pipeline calls each task passing the result of one to be the arguments for the next
return pipeline(tasks, options).then(function formatResponse(result) {
if (result) {
return {subscribers: [result.toJSON(options)]};
}
return Promise.reject(new errors.NotFoundError({message: i18n.t('errors.api.subscribers.subscriberNotFound')}));
});
return pipeline(tasks, options);
},
/**
@ -114,6 +119,11 @@ subscribers = {
return Promise.reject(error);
});
})
.then(function onModelResponse(model) {
return {
subscribers: [model.toJSON(options)]
};
});
}
@ -125,10 +135,7 @@ subscribers = {
];
// Pipeline calls each task passing the result of one to be the arguments for the next
return pipeline(tasks, object, options).then(function formatResponse(result) {
var subscriber = result.toJSON(options);
return {subscribers: [subscriber]};
});
return pipeline(tasks, object, options);
},
/**
@ -148,7 +155,18 @@ subscribers = {
* @returns {Object} options
*/
function doQuery(options) {
return models.Subscriber.edit(options.data.subscribers[0], _.omit(options, ['data']));
return models.Subscriber.edit(options.data.subscribers[0], _.omit(options, ['data']))
.then(function onModelResponse(model) {
if (!model) {
return Promise.reject(new errors.NotFoundError({
message: i18n.t('errors.api.subscribers.subscriberNotFound')
}));
}
return {
subscribers: [model.toJSON(options)]
};
});
}
// Push all of our tasks into a `tasks` array in the correct order
@ -159,15 +177,7 @@ subscribers = {
];
// Pipeline calls each task passing the result of one to be the arguments for the next
return pipeline(tasks, object, options).then(function formatResponse(result) {
if (result) {
var subscriber = result.toJSON(options);
return {subscribers: [subscriber]};
}
return Promise.reject(new errors.NotFoundError({message: i18n.t('errors.api.subscribers.subscriberNotFound')}));
});
return pipeline(tasks, object, options);
},
/**

View File

@ -63,7 +63,18 @@ tags = {
* @returns {Object} options
*/
function doQuery(options) {
return models.Tag.findOne(options.data, _.omit(options, ['data']));
return models.Tag.findOne(options.data, _.omit(options, ['data']))
.then(function onModelResponse(model) {
if (!model) {
return Promise.reject(new errors.NotFoundError({
message: i18n.t('errors.api.tags.tagNotFound')
}));
}
return {
tags: [model.toJSON(options)]
};
});
}
// Push all of our tasks into a `tasks` array in the correct order
@ -75,13 +86,7 @@ tags = {
];
// Pipeline calls each task passing the result of one to be the arguments for the next
return pipeline(tasks, options).then(function formatResponse(result) {
if (result) {
return {tags: [result.toJSON(options)]};
}
return Promise.reject(new errors.NotFoundError({message: i18n.t('errors.api.tags.tagNotFound')}));
});
return pipeline(tasks, options);
},
/**
@ -99,7 +104,12 @@ tags = {
* @returns {Object} options
*/
function doQuery(options) {
return models.Tag.add(options.data.tags[0], _.omit(options, ['data']));
return models.Tag.add(options.data.tags[0], _.omit(options, ['data']))
.then(function onModelResponse(model) {
return {
tags: [model.toJSON(options)]
};
});
}
// Push all of our tasks into a `tasks` array in the correct order
@ -111,11 +121,7 @@ tags = {
];
// Pipeline calls each task passing the result of one to be the arguments for the next
return pipeline(tasks, object, options).then(function formatResponse(result) {
var tag = result.toJSON(options);
return {tags: [tag]};
});
return pipeline(tasks, object, options);
},
/**
@ -135,7 +141,18 @@ tags = {
* @returns {Object} options
*/
function doQuery(options) {
return models.Tag.edit(options.data.tags[0], _.omit(options, ['data']));
return models.Tag.edit(options.data.tags[0], _.omit(options, ['data']))
.then(function onModelResponse(model) {
if (!model) {
return Promise.reject(new errors.NotFoundError({
message: i18n.t('errors.api.tags.tagNotFound')
}));
}
return {
tags: [model.toJSON(options)]
};
});
}
// Push all of our tasks into a `tasks` array in the correct order
@ -147,15 +164,7 @@ tags = {
];
// Pipeline calls each task passing the result of one to be the arguments for the next
return pipeline(tasks, object, options).then(function formatResponse(result) {
if (result) {
var tag = result.toJSON(options);
return {tags: [tag]};
}
return Promise.reject(new errors.NotFoundError({message: i18n.t('errors.api.tags.tagNotFound')}));
});
return pipeline(tasks, object, options);
},
/**

View File

@ -73,7 +73,18 @@ users = {
* @returns {Object} options
*/
function doQuery(options) {
return models.User.findOne(options.data, _.omit(options, ['data']));
return models.User.findOne(options.data, _.omit(options, ['data']))
.then(function onModelResponse(model) {
if (!model) {
return Promise.reject(new errors.NotFoundError({
message: i18n.t('errors.api.users.userNotFound')
}));
}
return {
users: [model.toJSON(options)]
};
});
}
// Push all of our tasks into a `tasks` array in the correct order
@ -85,13 +96,7 @@ users = {
];
// Pipeline calls each task passing the result of one to be the arguments for the next
return pipeline(tasks, options).then(function formatResponse(result) {
if (result) {
return {users: [result.toJSON(options)]};
}
return Promise.reject(new errors.NotFoundError({message: i18n.t('errors.api.users.userNotFound')}));
});
return pipeline(tasks, options);
},
/**
@ -192,7 +197,18 @@ users = {
* @returns {Object} options
*/
function doQuery(options) {
return models.User.edit(options.data.users[0], _.omit(options, ['data']));
return models.User.edit(options.data.users[0], _.omit(options, ['data']))
.then(function onModelResponse(model) {
if (!model) {
return Promise.reject(new errors.NotFoundError({
message: i18n.t('errors.api.users.userNotFound')
}));
}
return {
users: [model.toJSON(options)]
};
});
}
// Push all of our tasks into a `tasks` array in the correct order
@ -203,13 +219,7 @@ users = {
doQuery
];
return pipeline(tasks, object, options).then(function formatResponse(result) {
if (result) {
return {users: [result.toJSON(options)]};
}
return Promise.reject(new errors.NotFoundError({message: i18n.t('errors.api.users.userNotFound')}));
});
return pipeline(tasks, object, options);
},
/**
@ -324,7 +334,11 @@ users = {
return models.User.changePassword(
options.data.password[0],
_.omit(options, ['data'])
);
).then(function onModelResponse() {
return Promise.resolve({
password: [{message: i18n.t('notices.api.users.pwdChangedSuccessfully')}]
});
});
}
// Push all of our tasks into a `tasks` array in the correct order
@ -336,9 +350,7 @@ users = {
];
// Pipeline calls each task passing the result of one to be the arguments for the next
return pipeline(tasks, object, options).then(function formatResponse() {
return Promise.resolve({password: [{message: i18n.t('notices.api.users.pwdChangedSuccessfully')}]});
});
return pipeline(tasks, object, options);
},
/**
@ -371,7 +383,14 @@ users = {
* @returns {Object} options
*/
function doQuery(options) {
return models.User.transferOwnership(options.data.owner[0], _.omit(options, ['data']));
return models.User.transferOwnership(options.data.owner[0], _.omit(options, ['data']))
.then(function onModelResponse(model) {
// NOTE: model returns json object already
// @TODO: why?
return {
users: model
};
});
}
// Push all of our tasks into a `tasks` array in the correct order
@ -383,9 +402,7 @@ users = {
];
// Pipeline calls each task passing the result of one to be the arguments for the next
return pipeline(tasks, object, options).then(function formatResult(result) {
return Promise.resolve({users: result});
});
return pipeline(tasks, object, options);
}
};