Merge pull request #746 from jodal/feature/js-calling-convention

Add by-name calling convention to Mopidy.js

docs/api/js.rst

@@ -115,6 +115,22 @@ When creating an instance, you can specify the following settings:
     The maximum number of milliseconds to wait after a connection error before
     we try to reconnect. Defaults to 64000.
 
+``callingConvention``
+    Which calling convention to use when calling methods.
+
+    If set to "by-position-only", methods expect to be called with positional
+    arguments, like ``mopidy.foo.bar(null, true, 2)``.
+
+    If set to "by-position-or-by-name", methods expect to be called either with
+    an array of position arguments, like ``mopidy.foo.bar([null, true, 2])``,
+    or with an object of named arguments, like ``mopidy.foo.bar({id: 2})``. The
+    advantage of the "by-position-or-by-name" calling convention is that
+    arguments with default values can be left out of the named argument object.
+    Using named arguments also makes the code more readable, and more resistent
+    to future API changes.
+
+    For backwards compatibility, the default is "by-position-only".
+
 ``webSocket``
     An existing WebSocket object to be used instead of creating a new
     WebSocket. Defaults to undefined.

docs/changelog.rst

@@ -92,6 +92,10 @@ Feature release.
   ``Mopidy.ServerError``, and connection related errors are of the type
   ``Mopidy.ConnectionError``.
 
+- Add support for method calls with by-name arguments. The old calling
+  convention, by-position-only, is still the default. See the :ref:`mopidy-js`
+  docs for details.
+
 **MPD frontend**
 
 - Proper command tokenization for MPD requests. This replaces the old regex

js/README.md

@@ -80,6 +80,11 @@ To run other [grunt](http://gruntjs.com/) targets which isn't predefined in
 Changelog
 ---------
 
+### 0.4.0 (UNRELEASED)
+
+- Add support for method calls with by-name arguments. The old calling
+  convention, by-position-only, is still the default. See the docs for details.
+
 ### 0.3.0 (2014-06-16)
 
 - Upgrade to when.js 3, which brings great performance improvements and better

js/src/mopidy.js

@@ -53,6 +53,9 @@ Mopidy.prototype._configure = function (settings) {
     settings.backoffDelayMin = settings.backoffDelayMin || 1000;
     settings.backoffDelayMax = settings.backoffDelayMax || 64000;
 
+    settings.callingConvention = (
+        settings.callingConvention || "by-position-only");
+
     return settings;
 };
 
@@ -250,13 +253,31 @@ Mopidy.prototype._getApiSpec = function () {
 };
 
 Mopidy.prototype._createApi = function (methods) {
+    var byPositionOrByName = (
+        this._settings.callingConvention === "by-position-or-by-name");
+
     var caller = function (method) {
         return function () {
-            var params = Array.prototype.slice.call(arguments);
-            return this._send({
-                method: method,
-                params: params
-            });
+            var message = {method: method};
+            if (arguments.length === 0) {
+                return this._send(message);
+            }
+            if (!byPositionOrByName) {
+                message.params = Array.prototype.slice.call(arguments);
+                return this._send(message);
+            }
+            if (arguments.length > 1) {
+                return when.reject(new Error(
+                    "Expected zero arguments, a single array, " +
+                    "or a single object."));
+            }
+            if (!Array.isArray(arguments[0]) &&
+                arguments[0] !== Object(arguments[0])) {
+                return when.reject(new TypeError(
+                    "Expected an array or an object."));
+            }
+            message.params = arguments[0];
+            return this._send(message);
         }.bind(this);
     }.bind(this);
 

js/test/mopidy-test.js

@@ -766,20 +766,137 @@ buster.testCase("Mopidy", {
             assert.calledOnceWith(spy);
         },
 
-        "creates methods that sends correct messages": function () {
-            var sendStub = this.stub(this.mopidy, "_send");
-            this.mopidy._createApi({
-                foo: {
-                    params: ["bar", "baz"]
-                }
-            });
+        "by-position calling convention": {
+            setUp: function () {
+                this.mopidy._createApi({
+                    foo: {
+                        params: ["bar", "baz"]
+                    }
+                });
+                this.sendStub = this.stub(this.mopidy, "_send");
 
-            this.mopidy.foo(31, 97);
+            },
 
-            assert.calledOnceWith(sendStub, {
-                method: "foo",
-                params: [31, 97]
-            });
+            "is the default": function () {
+                assert.equals(
+                    this.mopidy._settings.callingConvention,
+                    "by-position-only");
+            },
+
+            "sends no params if no arguments passed to function": function () {
+                this.mopidy.foo();
+
+                assert.calledOnceWith(this.sendStub, {method: "foo"});
+            },
+
+            "sends messages with function arguments unchanged": function () {
+                this.mopidy.foo(31, 97);
+
+                assert.calledOnceWith(this.sendStub, {
+                    method: "foo",
+                    params: [31, 97]
+                });
+            },
+        },
+
+        "by-position-or-by-name calling convention": {
+            setUp: function () {
+                this.mopidy = new Mopidy({
+                    webSocket: this.webSocket,
+                    callingConvention: "by-position-or-by-name"
+                });
+                this.mopidy._createApi({
+                    foo: {
+                        params: ["bar", "baz"]
+                    }
+                });
+                this.sendStub = this.stub(this.mopidy, "_send");
+            },
+
+            "must be turned on manually": function () {
+                assert.equals(
+                    this.mopidy._settings.callingConvention,
+                    "by-position-or-by-name");
+            },
+
+            "sends no params if no arguments passed to function": function () {
+                this.mopidy.foo();
+
+                assert.calledOnceWith(this.sendStub, {method: "foo"});
+            },
+
+            "sends by-position if argument is a list": function () {
+                this.mopidy.foo([31, 97]);
+
+                assert.calledOnceWith(this.sendStub, {
+                    method: "foo",
+                    params: [31, 97]
+                });
+            },
+
+            "sends by-name if argument is an object": function () {
+                this.mopidy.foo({bar: 31, baz: 97});
+
+                assert.calledOnceWith(this.sendStub, {
+                    method: "foo",
+                    params: {bar: 31, baz: 97}
+                });
+            },
+
+            "rejects with error if more than one argument": function (done) {
+                var promise = this.mopidy.foo([1, 2], {c: 3, d: 4});
+
+                refute.called(this.sendStub);
+
+                promise.done(
+                    done(function () {
+                        assert(false);
+                    }),
+                    done(function (error) {
+                        assert(error instanceof Error);
+                        assert.equals(
+                            error.message,
+                            "Expected zero arguments, a single array, " +
+                            "or a single object.");
+                    })
+                );
+            },
+
+            "rejects with error if string": function (done) {
+                var promise = this.mopidy.foo("hello");
+
+                refute.called(this.sendStub);
+
+                promise.done(
+                    done(function () {
+                        assert(false);
+                    }),
+                    done(function (error) {
+                        assert(error instanceof Error);
+                        assert(error instanceof TypeError);
+                        assert.equals(
+                            error.message, "Expected an array or an object.");
+                    })
+                );
+            },
+
+            "rejects with error if number": function (done) {
+                var promise = this.mopidy.foo(1337);
+
+                refute.called(this.sendStub);
+
+                promise.done(
+                    done(function () {
+                        assert(false);
+                    }),
+                    done(function (error) {
+                        assert(error instanceof Error);
+                        assert(error instanceof TypeError);
+                        assert.equals(
+                            error.message, "Expected an array or an object.");
+                    })
+                );
+            }
         }
     }
 });