Merge pull request #151 from strongloop/feature/improve-remoting-metadata

Improve remoting metadata: register exported models using singular names

Author: bajtos

lib/application.js

@@ -4,6 +4,7 @@
 
 var DataSource = require('loopback-datasource-juggler').DataSource
   , ModelBuilder = require('loopback-datasource-juggler').ModelBuilder
+  , compat = require('./compat')
   , assert = require('assert')
   , fs = require('fs')
   , RemoteObjects = require('strong-remoting')
@@ -97,8 +98,9 @@ app.disuse = function (route) {
 app.model = function (Model, config) {
   if(arguments.length === 1) {
     assert(typeof Model === 'function', 'app.model(Model) => Model must be a function / constructor');
-    assert(Model.pluralModelName, 'Model must have a "pluralModelName" property');
-    this.remotes().exports[Model.pluralModelName] = Model;
+    assert(Model.modelName, 'Model must have a "modelName" property');
+    var remotingClassName = compat.getClassNameForRemoting(Model);
+    this.remotes().exports[remotingClassName] = Model;
     this.models().push(Model);
     Model.shared = true;
     Model.app = this;
@@ -203,7 +205,7 @@ app.remoteObjects = function () {
   models.forEach(function (ModelCtor) {
     // only add shared models
     if(ModelCtor.shared && typeof ModelCtor.sharedCtor === 'function') {
-      result[ModelCtor.pluralModelName] = ModelCtor;
+      result[compat.getClassNameForRemoting(ModelCtor)] = ModelCtor;
     }
   });
 

lib/compat.js

@@ -0,0 +1,56 @@
+var assert = require('assert');
+
+/**
+ * Compatibility layer allowing applications based on an older LoopBack version
+ * to work with newer versions with minimum changes involved.
+ *
+ * You should not use it unless migrating from an older version of LoopBack.
+ */
+
+var compat = exports;
+
+/**
+ * LoopBack versions pre-1.6 use plural model names when registering shared
+ * classes with strong-remoting. As the result, strong-remoting use method names
+ * like `Users.create` for the javascript methods like `User.create`.
+ * This has been fixed in v1.6, LoopBack consistently uses the singular
+ * form now.
+ *
+ * Turn this option on to enable the old behaviour.
+ *
+ *   - `app.remotes()` and `app.remoteObjects()` will be indexed using
+ *      plural names (Users instead of User).
+ *
+ *   - Remote hooks must use plural names for the class name, i.e
+ *     `Users.create` instead of `User.create`. This is transparently
+ *     handled by `Model.beforeRemote()` and `Model.afterRemote()`.
+ *
+ * @type {boolean}
+ * @deprecated Your application should not depend on the way how loopback models
+ *   and strong-remoting are wired together. It if does, you should update
+ *   it to use singular model names.
+ */
+
+compat.usePluralNamesForRemoting = false;
+
+/**
+ * Get the class name to use with strong-remoting.
+ * @param {function} Ctor Model class (constructor), e.g. `User`
+ * @return {string} Singular or plural name, depending on the value
+ *   of `compat.usePluralNamesForRemoting`
+ * @internal
+ */
+
+compat.getClassNameForRemoting = function(Ctor) {
+  assert(
+    typeof(Ctor) === 'function',
+    'compat.getClassNameForRemoting expects a constructor as the argument');
+
+  if (compat.usePluralNamesForRemoting) {
+    assert(Ctor.pluralModelName,
+      'Model must have a "pluralModelName" property in compat mode');
+    return Ctor.pluralModelName;
+  }
+
+  return Ctor.modelName;
+};

lib/loopback.js

@@ -40,6 +40,11 @@ loopback.version = require('../package.json').version;
 
 loopback.mime = express.mime;
 
+/*!
+ * Compatibility layer, intentionally left undocumented.
+ */
+loopback.compat = require('./compat');
+
 /**
  * Create an loopback application.
  *

lib/models/model.js

@@ -2,6 +2,7 @@
  * Module Dependencies.
  */
 var loopback = require('../loopback');
+var compat = require('../compat');
 var ModelBuilder = require('loopback-datasource-juggler').ModelBuilder;
 var modeler = new ModelBuilder();
 var assert = require('assert');
@@ -69,7 +70,8 @@ Model.setup = function () {
     var self = this;
     if(this.app) {
       var remotes = this.app.remotes();
-      remotes.before(self.pluralModelName + '.' + name, function (ctx, next) {
+      var className = compat.getClassNameForRemoting(self);
+      remotes.before(className + '.' + name, function (ctx, next) {
         fn(ctx, ctx.result, next);
       });
     } else {
@@ -85,7 +87,8 @@ Model.setup = function () {
     var self = this;
     if(this.app) {
       var remotes = this.app.remotes();
-      remotes.after(self.pluralModelName + '.' + name, function (ctx, next) {
+      var className = compat.getClassNameForRemoting(self);
+      remotes.after(className + '.' + name, function (ctx, next) {
         fn(ctx, ctx.result, next);
       });
     } else {

lib/models/user.js

@@ -240,7 +240,7 @@ User.prototype.verify = function (options, fn) {
                        options.protocol
                        + '://'
                        + options.host
-                       + (User.sharedCtor.http.path || '/' + User.pluralModelName)
+                       + User.http.path
                        + User.confirm.http.path;
 
 

package.json

@@ -16,7 +16,7 @@
   "dependencies": {
     "debug": "~0.7.2",
     "express": "~3.4.0",
-    "strong-remoting": "~1.1.0",
+    "strong-remoting": "~1.2.1",
     "inflection": "~1.2.5",
     "passport": "~0.1.17",
     "passport-local": "~0.1.6",
@@ -29,10 +29,10 @@
     "async": "~0.2.9"
   },
   "peerDependencies": {
-    "loopback-datasource-juggler": "~1.2.11"
+    "loopback-datasource-juggler": "~1.2.13"
   },
   "devDependencies": {
-    "loopback-datasource-juggler": "~1.2.11",
+    "loopback-datasource-juggler": "~1.2.13",
     "mocha": "~1.14.0",
     "strong-task-emitter": "0.0.x",
     "supertest": "~0.8.1",

test/app.test.js

@@ -4,12 +4,52 @@ var SIMPLE_APP = path.join(__dirname, 'fixtures', 'simple-app');
 describe('app', function() {
 
   describe('app.model(Model)', function() {
+    var app, db;
+    beforeEach(function() {
+      app = loopback();
+      db = loopback.createDataSource({connector: loopback.Memory});
+    });
+
     it("Expose a `Model` to remote clients", function() {
-      var app = loopback();
-      var memory = loopback.createDataSource({connector: loopback.Memory});
-      var Color = memory.createModel('color', {name: String});
+      var Color = db.createModel('color', {name: String});
+      app.model(Color);
+
+      expect(app.models()).to.eql([Color]);
+    });
+
+    it('uses singlar name as app.remoteObjects() key', function() {
+      var Color = db.createModel('color', {name: String});
+      app.model(Color);
+      expect(app.remoteObjects()).to.eql({ color: Color });
+    });
+
+    it('uses singular name as shared class name', function() {
+      var Color = db.createModel('color', {name: String});
       app.model(Color);
-      assert.equal(app.models().length, 1);
+      expect(app.remotes().exports).to.eql({ color: Color });
+    });
+
+    describe('in compat mode', function() {
+      before(function() {
+        loopback.compat.usePluralNamesForRemoting = true;
+      });
+      after(function() {
+        loopback.compat.usePluralNamesForRemoting = false;
+      });
+
+      it('uses plural name as shared class name', function() {
+        loopback.compat.usePluralNamesForRemoting = true;
+        var Color = db.createModel('color', {name: String});
+        app.model(Color);
+        expect(app.remotes().exports).to.eql({ colors: Color });
+      });
+
+      it('uses plural name as app.remoteObjects() key', function() {
+        var Color = db.createModel('color', {name: String});
+        app.model(Color);
+        expect(app.remoteObjects()).to.eql({ colors: Color });
+      });
+;
     });
   });
 

test/model.test.js

@@ -426,6 +426,37 @@ describe('Model', function() {
         });
       });
     })
+
+    describe('in compat mode', function() {
+      before(function() {
+        loopback.compat.usePluralNamesForRemoting = true;
+      });
+      after(function() {
+        loopback.compat.usePluralNamesForRemoting = false;
+      });
+
+      it('correctly install before/after hooks', function(done) {
+        var hooksCalled = [];
+
+        User.beforeRemote('**', function(ctx, user, next) {
+          hooksCalled.push('beforeRemote');
+          next();
+        });
+
+        User.afterRemote('**', function(ctx, user, next) {
+          hooksCalled.push('afterRemote');
+          next();
+        });
+
+        request(app).get('/users')
+          .expect(200, function(err, res) {
+            if (err) return done(err);
+            expect(hooksCalled, 'hooks called')
+              .to.eql(['beforeRemote', 'afterRemote']);
+            done();
+          });
+      });
+    });
   });
 
   describe('Model.hasMany(Model)', function() {