Merge pull request #2174 from strongloop/change_remote_method

Remove constraint making isStatic required

Author: 0candy

3.0-RELEASE-NOTES.md

@@ -32,4 +32,34 @@ via `global.Promise`,
 you will have to check all places where you are using non-standard promise API
 and update them to use Bluebird API instead.
 
-Please see [Related code change](https://github.com/strongloop/loopback/pull/1896) here.
+Please see [Related code change](https://github.com/strongloop/loopback/pull/1896) here.
+
+## new method of defining remoting metadata
+
+In 2.0, remote methods were defined as:
+```
+methods: {
+  staticMethod: {
+    isStatic: true,
+    http: { path: '/static' }
+  },
+  instanceMethod: {
+    isStatic: false,
+    http: { path: '/instance' }
+  }
+}
+```
+
+For 3.0, the isStatic flag is no longer required and will be determined from the method name.
+Method name starting with "prototype." will be the same as having isStatic flag set to false.
+```
+methods: {
+  staticMethod: {
+    http: { path: '/static' }
+  },
+  'prototype.instanceMethod': {
+  http: { path: '/instance' }
+}
+```
+
+Please see [related code change](https://github.com/strongloop/loopback/pull/2174) here.

lib/model.js

@@ -415,7 +415,9 @@ module.exports = function(registry) {
 
   Model.remoteMethod = function(name, options) {
     if (options.isStatic === undefined) {
-      options.isStatic = true;
+      var m = name.match(/^prototype\.(.*)$/);
+      options.isStatic = !m;
+      name = options.isStatic ? name : m[1];
     }
     this.sharedClass.defineMethod(name, options);
   };

lib/registry.js

@@ -4,6 +4,7 @@ var juggler = require('loopback-datasource-juggler');
 var debug = require('debug')('loopback:registry');
 var DataSource = juggler.DataSource;
 var ModelBuilder = juggler.ModelBuilder;
+var deprecated = require('depd')('strong-remoting');
 
 module.exports = Registry;
 
@@ -258,12 +259,19 @@ Registry.prototype._defineRemoteMethods = function(ModelCtor, methods) {
 
   Object.keys(methods).forEach(function(key) {
     var meta = methods[key];
+    var m = key.match(/^prototype\.(.*)$/);
+    var isStatic = !m;
+
     if (typeof meta.isStatic !== 'boolean') {
-      console.warn('Remoting metadata for "%s.%s" is missing "isStatic" ' +
-        'flag, the method is registered as an instance method.',
-        ModelCtor.modelName,
-        key);
-      console.warn('This behaviour may change in the next major version.');
+      key = isStatic ? key : m[1];
+      meta.isStatic = isStatic;
+    } else if (meta.isStatic && m) {
+      throw new Error('Remoting metadata for ' + ModelCtor.modelName + '.' +
+      key + ' "isStatic" does not match new method name-based style.');
+    } else {
+      key = isStatic ? key : m[1];
+      deprecated('Remoting metadata "isStatic" is deprecated. Please ' +
+      'specify "prototype.name" in method name instead for isStatic=false.');
     }
     ModelCtor.remoteMethod(key, meta);
   });

test/loopback.test.js

@@ -631,4 +631,92 @@ describe('loopback', function() {
       });
     });
   });
+
+  describe('new remote method configuration', function() {
+    function getAllMethodNamesWithoutClassName(TestModel) {
+      return TestModel.sharedClass.methods().map(function(m) {
+        return m.stringName.replace(/^[^.]+\./, ''); // drop the class name
+      });
+    }
+
+    it('treats method names that don\'t start with "prototype." as "isStatic:true"', function() {
+      var TestModel = loopback.createModel(uniqueModelName);
+      loopback.configureModel(TestModel, {
+        dataSource: null,
+        methods: {
+          staticMethod: {
+            http: { path: '/static' }
+          }
+        }
+      });
+
+      var methodNames = getAllMethodNamesWithoutClassName(TestModel);
+
+      expect(methodNames).to.include('staticMethod');
+    });
+
+    it('treats method names starting with "prototype." as "isStatic:false"', function() {
+      var TestModel = loopback.createModel(uniqueModelName);
+      loopback.configureModel(TestModel, {
+        dataSource: null,
+        methods: {
+          'prototype.instanceMethod': {
+            http: { path: '/instance' }
+          }
+        }
+      });
+
+      var methodNames = getAllMethodNamesWithoutClassName(TestModel);
+
+      expect(methodNames).to.include('prototype.instanceMethod');
+    });
+
+    it('throws an error when "isStatic:true" and method name starts with "prototype."', function() {
+      var TestModel = loopback.createModel(uniqueModelName);
+      expect(function() { loopback.configureModel(TestModel, {
+        dataSource: null,
+        methods: {
+          'prototype.instanceMethod': {
+            isStatic: true,
+            http: { path: '/instance' }
+          }
+        }
+      });}).to.throw(Error, new Error('Remoting metadata for' + TestModel.modelName +
+      ' "isStatic" does not match new method name-based style.'));
+    });
+
+    it('use "isStatic:true" if method name does not start with "prototype."', function() {
+      var TestModel = loopback.createModel(uniqueModelName);
+      loopback.configureModel(TestModel, {
+        dataSource: null,
+        methods: {
+          staticMethod: {
+            isStatic: true,
+            http: { path: '/static' }
+          }
+        }
+      });
+
+      var methodNames = getAllMethodNamesWithoutClassName(TestModel);
+
+      expect(methodNames).to.include('staticMethod');
+    });
+
+    it('use "isStatic:false" if method name starts with "prototype."', function() {
+      var TestModel = loopback.createModel(uniqueModelName);
+      loopback.configureModel(TestModel, {
+        dataSource: null,
+        methods: {
+          'prototype.instanceMethod': {
+            isStatic: false,
+            http: { path: '/instance' }
+          }
+        }
+      });
+
+      var methodNames = getAllMethodNamesWithoutClassName(TestModel);
+
+      expect(methodNames).to.include('prototype.instanceMethod');
+    });
+  });
 });