Skip to content

Commit

Permalink
Remove JSONP support from blueprints, and clean up files for JSHint
Browse files Browse the repository at this point in the history
  • Loading branch information
@sgress454
sgress454 committed Nov 1, 2016
1 parent 820d1eb commit effd6c3
Show file tree
Hide file tree
Showing 10 changed files with 61 additions and 89 deletions.
26 changes: 3 additions & 23 deletions lib/hooks/blueprints/actionUtil.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -7,13 +7,6 @@ var _ = require('lodash');
var mergeDefaults = require('merge-defaults');
var WLCriteria = require('waterline-criteria');


// Parameter used for jsonp callback is constant, as far as
// blueprints are concerned (for now.)
var JSONP_CALLBACK_PARAM = 'callback';



/**
* Utility methods used in built-in blueprint actions.
*
Expand Down Expand Up @@ -244,12 +237,6 @@ var actionUtil = {
if (_.isUndefined(p)) {return true;}
});

// Omit jsonp callback param (but only if jsonp is enabled)
var jsonpOpts = req.options.jsonp && !req.isSocket;
jsonpOpts = _.isObject(jsonpOpts) ? jsonpOpts : { callback: JSONP_CALLBACK_PARAM };
if (jsonpOpts) {
where = _.omit(where, [jsonpOpts.callback]);
}
}

// Merge w/ req.options.where.
Expand Down Expand Up @@ -316,13 +303,6 @@ var actionUtil = {
// Otherwaise grab the first (and only) value from valuesArray
values = valuesArray[0];

// Omit jsonp callback param (but only if jsonp is enabled)
var jsonpOpts = req.options.jsonp && !req.isSocket;
jsonpOpts = _.isObject(jsonpOpts) ? jsonpOpts : { callback: JSONP_CALLBACK_PARAM };
if (jsonpOpts) {
values = _.omit(values, [jsonpOpts.callback]);
}

return values;
},

Expand All @@ -337,10 +317,10 @@ var actionUtil = {

// Ensure a model can be deduced from the request options.
var model = req.options.model || req.options.controller;
if (!model) throw new Error(util.format('No "model" specified in route options.'));
if (!model) { throw new Error(util.format('No "model" specified in route options.')); }

var Model = req._sails.models[model];
if ( !Model ) throw new Error(util.format('Invalid route option, "model".\nI don\'t know about any models named: `%s`',model));
if ( !Model ) { throw new Error(util.format('Invalid route option, "model".\nI don\'t know about any models named: `%s`',model)); }

return Model;
},
Expand Down Expand Up @@ -403,7 +383,7 @@ var actionUtil = {
// If JSON is falsey, return null
// (this is so that it will be ignored if not specified)
function tryToParseJSON (json) {
if (!_.isString(json)) return null;
if (!_.isString(json)) { return null; }
try {
return JSON.parse(json);
}
Expand Down
28 changes: 13 additions & 15 deletions lib/hooks/blueprints/actions/add.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -8,8 +8,8 @@ var async = require('async');
/**
* Add Record To Collection
*
* post /:modelIdentity/:id/:collectionAttr/:childid

This comment has been minimized.

Sign in to view

Copy link
@mikermcneil

mikermcneil Nov 1, 2016

Member

@sgress454 in the docs, we use ":id", ":association", and ":fk", so should just change it to that here......

....or much, much better, let's just include a link so this stays up to date:

http://sailsjs.com/docs/reference/blueprint-api/add-to

This comment has been minimized.

Sign in to view

Copy link
@mikermcneil

mikermcneil Nov 1, 2016

Member

(and same deal for the other ones below)
* * /:modelIdentity/:id/:collectionAttr/add/:childid
* post /:modelIdentity/:parentid/:collectionAttr/:childid
* * /:modelIdentity/:parentid/:collectionAttr/add/:childid
*
* Associate one record with the collection attribute of another.
* e.g. add a Horse named "Jimmy" to a Farm's "animals".
Expand All @@ -24,8 +24,6 @@ var async = require('async');
* to this endpoint to create a new child record, then associate
* it with the parent.
*
* @option {String} model - the identity of the model
* @option {String} alias - the name of the association attribute (aka "alias")
*/

module.exports = function addToCollection (req, res) {
Expand Down Expand Up @@ -75,10 +73,10 @@ module.exports = function addToCollection (req, res) {
// Look up the parent record
parent: function (cb) {
Model.findOne(parentPk).exec(function foundParent(err, parentRecord) {
if (err) return cb(err);
if (!parentRecord) return cb({status: 404});
if (!parentRecord[relation]) return cb({status: 404});
cb(null, parentRecord);
if (err) { return cb(err); }
if (!parentRecord) { return cb({status: 404}); }
if (!parentRecord[relation]) { return cb({status: 404}); }
return cb(null, parentRecord);
});
},

Expand All @@ -95,7 +93,7 @@ module.exports = function addToCollection (req, res) {
// it before we create it.
if (child[childPkAttr]) {
ChildModel.findOne(child[childPkAttr]).exec(function foundChild(err, childRecord) {
if (err) return cb(err);
if (err) { return cb(err); }
// Didn't find it? Then try creating it.
if (!childRecord) {return createChild();}
// Otherwise use the one we found.
Expand All @@ -110,7 +108,7 @@ module.exports = function addToCollection (req, res) {
// Create a new instance and send out any required pubsub messages.
function createChild() {
ChildModel.create(child).exec(function createdNewChild (err, newChildRecord){
if (err) return cb(err);
if (err) { return cb(err); }
if (req._sails.hooks.pubsub) {
if (req.isSocket) {
ChildModel.subscribe(req, [newChildRecord[ChildModel.primaryKey]]);
Expand Down Expand Up @@ -155,13 +153,13 @@ module.exports = function addToCollection (req, res) {
// Save the parent record
function readyToSave (err, async_data) {

if (err) return res.negotiate(err);
if (err) { return res.negotiate(err); }
async_data.parent.save(function saved(err) {

// Ignore `insert` errors for duplicate adds
// (but keep in mind, we should not _publishAdd if this is the case...)
var isDuplicateInsertError = (err && typeof err === 'object' && err.length && err[0] && err[0].type === 'insert');
if (err && !isDuplicateInsertError) return res.negotiate(err);
if (err && !isDuplicateInsertError) { return res.negotiate(err); }

// Only broadcast an update if this isn't a duplicate `add`
// (otherwise connected clients will see duplicates)
Expand All @@ -176,9 +174,9 @@ module.exports = function addToCollection (req, res) {
// Finally, look up the parent record again and populate the relevant collection.
// TODO: populateRequest
Model.findOne(parentPk).populate(relation).exec(function(err, matchingRecord) {
if (err) return res.serverError(err);
if (!matchingRecord) return res.serverError();
if (!matchingRecord[relation]) return res.serverError();
if (err) { return res.serverError(err); }
if (!matchingRecord) { return res.serverError(); }
if (!matchingRecord[relation]) { return res.serverError(); }
return res.ok(matchingRecord);
});
});
Expand Down
14 changes: 6 additions & 8 deletions lib/hooks/blueprints/actions/create.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -13,26 +13,24 @@ var _ = require('lodash');
* using the specified criteria. If an id was specified, just the instance with
* that unique id will be returned.
*
* Optional:
* @param {String} callback - default jsonp callback param (i.e. the name of the js function returned)
* @param {*} * - other params will be used as `values` in the create
*/
module.exports = function createRecord (req, res) {

var Model = actionUtil.parseModel(req);

// Create data object (monolithic combination of all parameters)
// Omit the blacklisted params (like JSONP callback param, etc.)
// Create data object (monolithic combination of all parameters),
// omitting any blacklisted params.
var data = actionUtil.parseValues(req);


// Create new instance of model using data from params
Model.create(data).exec(function created (err, newInstance) {

// Differentiate between waterline-originated validation errors
// and serious underlying issues. Respond with badRequest if a
// validation error is encountered, w/ validation info.
if (err) return res.negotiate(err);
if (err) {
return res.negotiate(err);
}

// If we have the pubsub hook, use the model class's publish method
// to notify all subscribers about the created item
Expand All @@ -48,7 +46,7 @@ module.exports = function createRecord (req, res) {
Model._publishCreate(publishData, !req.options.mirror && req);
}

// Send JSONP-friendly response if it's supported
// Send response
res.ok(newInstance);
});
};
8 changes: 3 additions & 5 deletions lib/hooks/blueprints/actions/destroy.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -15,8 +15,6 @@ var actionUtil = require('../actionUtil');
* Required:
* @param {Integer|String} id - the unique id of the particular instance you'd like to delete
*
* Optional:
* @param {String} callback - default jsonp callback param (i.e. the name of the js function returned)
*/
module.exports = function destroyOneRecord (req, res) {

Expand All @@ -26,11 +24,11 @@ module.exports = function destroyOneRecord (req, res) {
var query = Model.findOne(pk);
query = actionUtil.populateRequest(query, req);
query.exec(function foundRecord (err, record) {
if (err) return res.serverError(err);
if(!record) return res.notFound('No record found with the specified `id`.');
if (err) { return res.serverError(err); }
if(!record) { return res.notFound('No record found with the specified `id`.'); }

Model.destroy(pk).exec(function destroyedRecord (err) {
if (err) return res.negotiate(err);
if (err) { return res.negotiate(err); }

if (req._sails.hooks.pubsub) {
Model._publishDestroy(pk, !req._sails.config.blueprints.mirror && req, {previous: record});
Expand Down
5 changes: 2 additions & 3 deletions lib/hooks/blueprints/actions/find.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -14,12 +14,11 @@ var actionUtil = require('../actionUtil'),
* using the specified criteria. If an id was specified, just the instance
* with that unique id will be returned.
*
* Optional:
* Optional query params:
* @param {Object} where - the find criteria (passed directly to the ORM)
* @param {Integer} limit - the maximum number of records to send back (useful for pagination)
* @param {Integer} skip - the number of records to skip (useful for pagination)
* @param {String} sort - the order of returned records, e.g. `name ASC` or `age DESC`
* @param {String} callback - default jsonp callback param (i.e. the name of the js function returned)
*/

module.exports = function findRecords (req, res) {
Expand All @@ -44,7 +43,7 @@ module.exports = function findRecords (req, res) {
.sort( actionUtil.parseSort(req) );
query = actionUtil.populateRequest(query, req);
query.exec(function found(err, matchingRecords) {
if (err) return res.serverError(err);
if (err) { return res.serverError(err); }

// Only `._watch()` for new instances of the model if
// `autoWatch` is enabled.
Expand Down
6 changes: 2 additions & 4 deletions lib/hooks/blueprints/actions/findOne.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -14,8 +14,6 @@ var actionUtil = require('../actionUtil');
* Required:
* @param {Integer|String} id - the unique id of the particular instance you'd like to look up *
*
* Optional:
* @param {String} callback - default jsonp callback param (i.e. the name of the js function returned)
*/

module.exports = function findOneRecord (req, res) {
Expand All @@ -26,8 +24,8 @@ module.exports = function findOneRecord (req, res) {
var query = Model.findOne(pk);
query = actionUtil.populateRequest(query, req);
query.exec(function found(err, matchingRecord) {
if (err) return res.serverError(err);
if(!matchingRecord) return res.notFound('No record found with the specified `id`.');
if (err) { return res.serverError(err); }
if(!matchingRecord) { return res.notFound('No record found with the specified `id`.'); }

if (req._sails.hooks.pubsub && req.isSocket) {
Model.subscribe(req, [matchingRecord[Model.primaryKey]]);
Expand Down
19 changes: 10 additions & 9 deletions lib/hooks/blueprints/actions/populate.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -8,25 +8,26 @@ var util = require('util'),
/**
* Populate (or "expand") an association
*
* get /model/:parentid/relation
* get /model/:parentid/relation/:id
* get /model/:parentid/:alias
* get /model/:parentid/:alias/:id
*
* @param {Integer|String} parentid - the unique id of the parent instance
* @param {String} alias - the association to populate
* @param {Integer|String} id - the unique id of the particular child instance you'd like to look up within this relation
*
* Optional query params:
* @param {Object} where - the find criteria (passed directly to the ORM)
* @param {Integer} limit - the maximum number of records to send back (useful for pagination)
* @param {Integer} skip - the number of records to skip (useful for pagination)
* @param {String} sort - the order of returned records, e.g. `name ASC` or `age DESC`
*
* @option {String} model - the identity of the model
* @option {String} alias - the name of the association attribute (aka "alias")
*/

module.exports = function expand(req, res) {

var Model = actionUtil.parseModel(req);
var relation = req.options.alias;
if (!relation || !Model) return res.serverError();
if (!relation || !Model) { return res.serverError(); }

// Allow customizable blacklist for params.
req.options.criteria = req.options.criteria || {};
Expand All @@ -41,7 +42,7 @@ module.exports = function expand(req, res) {

// Coerce the child PK to an integer if necessary
if (childPk) {
if (Model.attributes[Model.primaryKey].type == 'integer') {
if (Model.attributes[Model.primaryKey].type === 'integer') {
childPk = +childPk || 0;
}
}
Expand All @@ -59,9 +60,9 @@ module.exports = function expand(req, res) {
.findOne(parentPk)
.populate(relation, populate)
.exec(function found(err, matchingRecord) {
if (err) return res.serverError(err);
if (!matchingRecord) return res.notFound('No record found with the specified id.');
if (!matchingRecord[relation]) return res.notFound(util.format('Specified record (%s) is missing relation `%s`', parentPk, relation));
if (err) { return res.serverError(err); }
if (!matchingRecord) { return res.notFound('No record found with the specified id.'); }
if (!matchingRecord[relation]) { return res.notFound(util.format('Specified record (%s) is missing relation `%s`', parentPk, relation)); }

// Subcribe to instance, if relevant
// TODO: only subscribe to populated attribute- not the entire model
Expand Down
21 changes: 11 additions & 10 deletions lib/hooks/blueprints/actions/remove.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -8,11 +8,12 @@ var _ = require('lodash');
/**
* Remove a member from an association
*
* delete /:modelIdentity/:parentid/:collectionAttr/:childid
* * /:modelIdentity/:parentid/:collectionAttr/remove/:childid
*
* @param {Integer|String} parentid - the unique id of the parent record
* @param {Integer|String} id - the unique id of the child record to remove
*
* @option {String} model - the identity of the model
* @option {String} alias - the name of the association attribute (aka "alias")
*/

module.exports = function remove(req, res) {
Expand All @@ -37,22 +38,22 @@ module.exports = function remove(req, res) {

Model
.findOne(parentPk).exec(function found(err, parentRecord) {
if (err) return res.serverError(err);
if (!parentRecord) return res.notFound();
if (!parentRecord[relation]) return res.notFound();
if (err) { return res.serverError(err); }
if (!parentRecord) { return res.notFound(); }
if (!parentRecord[relation]) { return res.notFound(); }

parentRecord[relation].remove(childPk);
parentRecord.save(function(err) {
if (err) return res.negotiate(err);
if (err) { return res.negotiate(err); }

Model.findOne(parentPk)
.populate(relation)
// TODO: use populateRequest util instead
.exec(function found(err, parentRecord) {
if (err) return res.serverError(err);
if (!parentRecord) return res.serverError();
if (!parentRecord[relation]) return res.serverError();
if (!parentRecord[Model.primaryKey]) return res.serverError();
if (err) { return res.serverError(err); }
if (!parentRecord) { return res.serverError(); }
if (!parentRecord[relation]) { return res.serverError(); }
if (!parentRecord[Model.primaryKey]) { return res.serverError(); }

// If we have the pubsub hook, use the model class's publish method
// to notify all subscribers about the removed item
Expand Down
Loading

0 comments on commit effd6c3

Please sign in to comment.