Merge pull request #23 from digitalsadhu/document_code

Cleanup and commenting code

Author: digitalsadhu

lib/create.js

@@ -1,19 +1,33 @@
 var url = require('url');
 
 module.exports = function (app, options) {
+  //get remote methods.
+  //set strong-remoting for more information
+  //https://github.com/strongloop/strong-remoting
   var remotes = app.remotes();
 
+  //register after remote method hook on all methods
   remotes.after('**', function (ctx, next) {
+
+    //in this case we are only interested in handling create operations.
     if (ctx.method.name === 'create') {
+
+      //JSON API specifies that created resources should have the
+      //http status code of 201
       ctx.res.statusCode = 201;
 
+      //build the location url for the created resource.
       var location = url.format({
         protocol: ctx.req.protocol,
         host: ctx.req.get('host'),
         pathname: ctx.req.baseUrl + '/' + ctx.result.data.id
       });
 
+      //we don't need the links property so just delete it.
       delete ctx.result.links;
+
+      //JSON API specifies that when creating a resource, there should be a
+      //location header set with the url of the created resource as the value
       ctx.res.append('Location', location);
     }
     next();

lib/deserialize.js

@@ -3,17 +3,28 @@
 module.exports = function (app) {
   var remotes = app.remotes();
 
+  //register a handler to run before all remote methods so that we can
+  //transform JSON API structured JSON payload into something loopback
+  //can work with.
   remotes.before('**', function (ctx, next) {
+
+    //JSON api doesn't support PUT so we only need to apply our changes to
+    //POST and PATCH operations.
     if (ctx.req.method === 'POST' || ctx.req.method === 'PATCH') {
+
+      //set the JSON API Content-Type response header
       ctx.res.set({'Content-Type': 'application/vnd.api+json'});
+
+      //check the incoming payload data to ensure it is valid according to the
+      //JSON API spec.
       var data = ctx.args.data;
       if (!data.data) return next(new Error('JSON API resource object must contain `data` property'));
       if (!data.data.type) return next(new Error('JSON API resource object must contain `data.type` property'));
-
       if (ctx.req.method === 'PATCH') {
         if (!data.data.id) return next(new Error('JSON API resource object must contain `data.id` property'));
       }
 
+      //transform the payload
       ctx.args.data = data.data.attributes || {};
     }
 

lib/headers.js

@@ -5,6 +5,8 @@ var bodyParser = require('body-parser');
 module.exports = function (app, options) {
 
   //registered at the beginning of the routes phase due to usage of
-  //express app.use instead of app.middleware
+  //express app.use instead of app.middleware.
+  //We need to do this as bodyParser doesn't support
+  //JSON APIs application/vnd.api+json format by default
   app.use(bodyParser.json({ type: 'application/vnd.api+json' }));
 };

lib/patch.js

@@ -1,3 +1,6 @@
+//copied in its entirity from loopbacks Model class.
+//it was necessary to do so as this function is used
+//in the other code below
 function convertNullToNotFoundError (toModelName, ctx, cb) {
   if (ctx.result !== null) return cb();
 
@@ -9,12 +12,30 @@ function convertNullToNotFoundError (toModelName, ctx, cb) {
   cb(error);
 }
 
-function fixHttpMethod (fn, name) {
-  if (fn.http && fn.http.verb && fn.http.verb.toLowerCase() === 'put') fn.http.verb = 'patch';
-}
-
 module.exports = function (app, options) {
+
+  //iterate over all loopback models. This gives us a constructor function
+  //and allows us to overwrite the relationship methods:
+  // - belongsToRemoting
+  // - hasOneRemoting
+  // - hasManyRemoting
+  // - scopeRemoting
+  // - etc
+  //
+  //Note: This does not seem ideal.
+  //We are copying a tonne of code out of loopbacks Model class
+  //and then modifying little bits of it below.
+  // see: https://github.com/strongloop/loopback/blob/master/lib/model.js#L462-L661
+  //Most likely this will be fragile and
+  //require us to keep up to date with any chances introduced in loopback.
+  //
+  //It would be good to have a discussion with the strongloop guys and see
+  //if there is a better way this can be done.
   app.models().forEach(function (ctor) {
+
+    //sets up remote relationship method belongsTo on any model that
+    //has a belongsTo relationship defined in its .json file (or via the
+    //node API)
     ctor.belongsToRemoting = function (relationName, relation, define) {
       var modelName = relation.modelTo && relation.modelTo.modelName;
       modelName = modelName || 'PersistedModel';
@@ -83,6 +104,10 @@ module.exports = function (app, options) {
       }, findHasManyRelationshipsFunc);
 
       var createRelationshipFunc = function (cb) {
+        //TODO: implement this
+        //this is where we need to implement
+        // POST /:model/:id/relationships/:relatedModel
+        //
         // this['__get__' + pathName](cb);
       };
       define('__createRelationships__' + relationName, {
@@ -95,6 +120,10 @@ module.exports = function (app, options) {
       }, createRelationshipFunc);
 
       var updateRelationshipsFunc = function (cb) {
+        //TODO: implement this
+        //this is where we need to implement
+        // PATCH /:model/:id/relationships/:relatedModel
+        //
         // this['__get__' + pathName](cb);
       };
       define('__updateRelationships__' + relationName, {
@@ -107,6 +136,10 @@ module.exports = function (app, options) {
       }, updateRelationshipsFunc);
 
       var deleteRelationshipsFunc = function (cb) {
+        //TODO: implement this
+        //this is where we need to implement
+        // DELETE /:model/:id/relationships/:relatedModel
+        //
         // this['__get__' + pathName](cb);
       };
       define('__deleteRelationships__' + relationName, {
@@ -156,5 +189,12 @@ module.exports = function (app, options) {
     };
   });
 
+  //copied from loopbacks Model class.
+  function fixHttpMethod (fn, name) {
+    if (fn.http && fn.http.verb && fn.http.verb.toLowerCase() === 'put') fn.http.verb = 'patch';
+  }
+
+  //iterate through all remote methods and swap PUTs to PATCHs
+  //as PUT is not supported by JSON API.
   app.remotes().methods().forEach(fixHttpMethod);
 };

lib/removeRemoteMethods.js

@@ -1,6 +1,8 @@
 module.exports = function (app, options) {
   var models = app.models;
 
+  //use loopbacks standard API to disable all methods that JSON API
+  //does not support.
   var isStatic = true;
   Object.keys(models).forEach(function (model) {
     models[model].disableRemoteMethod('upsert', isStatic);

lib/serialize.js

@@ -44,21 +44,6 @@ function pluralForModel (model) {
   return inflection.pluralize(model.sharedClass.name);
 }
 
-//TODO: Not used!!
-// function modelNameForPlural (models, plural) {
-//   return Object.keys(models).filter(function (name) {
-//     if (models[name] && models[name].definition) {
-//       return models[name].definition.settings.plural === plural;
-//     }
-//     return false;
-//   })[0];
-// }
-
-//TODO: Not used!!
-// function modelForPlural (models, plural) {
-//   return models[modelNameForPlural(models, plural)];
-// }
-
 function attributesForModel (model) {
   return Object.keys(model.definition.properties);
 }
@@ -75,15 +60,6 @@ function clone (object) {
   return JSON.parse(JSON.stringify(object));
 }
 
-//TODO: Never used
-// function filterFromContext (context) {
-//   try {
-//     return JSON.parse(context.args.filter);
-//   } catch (e) {
-//     return false;
-//   }
-// }
-
 function modelNameFromContext (context) {
   return context.method.sharedClass.name;
 }
@@ -92,11 +68,6 @@ function urlFromContext (context) {
   return context.req.protocol + '://' + context.req.get('host') + context.req.originalUrl;
 }
 
-//TODO: Never used!!
-// function primaryKeyFromModel(model) {
-//   console.log(model.definition.properties)
-// }
-
 function buildModelUrl (protocol, host, apiRoot, modelName, id) {
   var result;
   try {
@@ -165,8 +136,6 @@ module.exports = function (app, options) {
       dataLinks: {
         self: function (item) {
           if (relatedModelPlural) {
-            //TODO: fix url building. Use url module.
-            //currently doesnt take into account if /api/ is in the url etc.
             return buildModelUrl(ctx.req.protocol, ctx.req.get('host'), options.restApiRoot, relatedModelPlural, item.id);
           }
           return buildModelUrl(ctx.req.protocol, ctx.req.get('host'), options.restApiRoot, pluralForModel(app.models[modelName]), item.id);

lib/update.js

@@ -3,8 +3,8 @@ module.exports = function (app, options) {
 
   remotes.after('**', function (ctx, next) {
     if (ctx.method.name === 'updateAttributes') {
-      // ctx.res.statusCode = 201;
 
+      //remote links object from resource response
       delete ctx.result.links;
     }
     next();

package.json

@@ -4,7 +4,8 @@
   "description": "JSONAPI support for loopback",
   "main": "lib/index.js",
   "scripts": {
-    "test": "mocha --reporter=spec ./test/**/*"
+    "test": "npm run lint && mocha --reporter=spec ./test/**/*",
+    "lint": "eslint ."
   },
   "repository": {
     "type": "git",

test/hasMany.test.js

@@ -30,7 +30,7 @@ describe('loopback json api hasMany relationships', function () {
     JSONAPIComponent(app, {restApiRoot: '/'});
   });
 
-  describe('Post with no comments', function (done) {
+  describe('Fetch a post with no comments', function (done) {
     beforeEach(function (done) {
       Post.create({
         title: 'my post without comments',
@@ -82,7 +82,7 @@ describe('loopback json api hasMany relationships', function () {
     });
   });
 
-  describe('Post with a comment', function (done) {
+  describe('Fetch a post with a comment', function (done) {
     beforeEach(function (done) {
       Post.create({
         title: 'my post',