allow both 'value' and 'text' params for setValue (#121)

* allow both 'value' and 'text' params for setValue

this introduces a 'makeArgs' option for the spec which allows handling of more complex scenarios.

* clarify some variable names and add fuller commentary

lib/mjsonwp/mjsonwp.js

@@ -67,7 +67,7 @@ function checkParams (paramSets, jsonObj) {
       optionalParams = paramSets.optional;
     }
 
-    // If a function was provided as the 'validate' key, it will here be called with 
+    // If a function was provided as the 'validate' key, it will here be called with
     // jsonObj as the param. If it returns something falsy, verification will be
     // considered to have passed. If it returns something else, that will be the
     // argument to an error which is thrown to the user
@@ -106,29 +106,60 @@ function checkParams (paramSets, jsonObj) {
   throw new errors.BadParametersError(paramSets, receivedParams);
 }
 
-function makeArgs (reqParams, jsonObj, payloadParams) {
-  // we want to pass the url parameters to the commands in reverse order
-  // since the command will sometimes want to ignore, say, the sessionId
-  let urlParams = _.keys(reqParams).reverse();
-
-  // there can be multiple sets of required params, so find the correct one
-  let realRequiredParams = payloadParams.required;
+/*
+ * This method takes 3 pieces of data: request parameters ('requestParams'),
+ * a request JSON body ('jsonObj'), and 'payloadParams', which is the section
+ * from the route definition for a particular endpoint which has instructions
+ * on handling parameters. This method returns an array of arguments which will
+ * be applied to a command.
+ */
+function makeArgs (requestParams, jsonObj, payloadParams) {
+  // We want to pass the "url" parameters to the commands in reverse order
+  // since the command will sometimes want to ignore, say, the sessionId.
+  // This has the effect of putting sessionId last, which means in JS we can
+  // omit it from the function signature if we're not going to use it.
+  let urlParams = _.keys(requestParams).reverse();
+
+  // In the simple case, the required parameters are a basic array in
+  // payloadParams.required, so start there. It's possible that there are
+  // multiple optional sets of required params, though, so handle that case
+  // too.
+  let requiredParams = payloadParams.required;
   if (_.isArray(_.first(payloadParams.required))) {
+    // If there are optional sets of required params, then we will have an
+    // array of arrays in payloadParams.required, so loop through each set and
+    // pick the one that matches which JSON params were actually sent. We've
+    // already been through validation so we're guaranteed to find a match.
     let keys = _.keys(jsonObj);
     for (let params of payloadParams.required) {
-      // check if all the required parameters are in the json object
       if (_.without(params, ...keys).length === 0) {
-        // we have all the parameters for this set
-        realRequiredParams = params;
+        requiredParams = params;
         break;
       }
     }
   }
-  let args = _.flatten(realRequiredParams).map((p) => jsonObj[p]);
-  if (payloadParams.optional) {
-    args = args.concat(_.flatten(payloadParams.optional).map((p) => jsonObj[p]));
+
+  // Now we construct our list of arguments which will be passed to the command
+  let args;
+  if (_.isFunction(payloadParams.makeArgs)) {
+    // In the route spec, a particular route might define a 'makeArgs' function
+    // if it wants full control over how to turn JSON parameters into command
+    // arguments. So we pass it the JSON parameters and it returns an array
+    // which will be applied to the handling command. For example if it returns
+    // [1, 2, 3], we will call `command(1, 2, 3, ...)` (url params are separate
+    // from JSON params and get concatenated below).
+    args = payloadParams.makeArgs(jsonObj);
+  } else {
+    // Otherwise, collect all the required and optional params and flatten them
+    // into an argument array
+    args = _.flatten(requiredParams).map((p) => jsonObj[p]);
+    if (payloadParams.optional) {
+      args = args.concat(_.flatten(payloadParams.optional).map((p) => jsonObj[p]));
+    }
   }
-  args = args.concat(urlParams.map((u) => reqParams[u]));
+  // Finally, get our url params (session id, element id, etc...) on the end of
+  // the list
+  args = args.concat(urlParams.map((u) => requestParams[u]));
   return args;
 }
 

lib/mjsonwp/routes.js

@@ -10,7 +10,7 @@ const METHOD_MAP = {
   },
   '/wd/hub/session': {
     POST: {command: 'createSession', payloadParams: {
-      validate: (jsonObj) => (!jsonObj.capabilities && !jsonObj.desiredCapabilities) && 'we require one of "desiredCapabilities" or "capabilities" object', 
+      validate: (jsonObj) => (!jsonObj.capabilities && !jsonObj.desiredCapabilities) && 'we require one of "desiredCapabilities" or "capabilities" object',
       optional: ['desiredCapabilities', 'requiredCapabilities', 'capabilities']}}
   },
   '/wd/hub/sessions': {
@@ -135,7 +135,21 @@ const METHOD_MAP = {
     GET: {command: 'getText'}
   },
   '/wd/hub/session/:sessionId/element/:elementId/value': {
-    POST: {command: 'setValue', payloadParams: {required: ['value']}}
+    POST: {command: 'setValue', payloadParams: {
+      validate: (jsonObj) => {
+        return (!jsonObj.value && !jsonObj.text) &&
+          'we require one of "text" or "value" params';
+      },
+      optional: ['value', 'text'],
+      makeArgs: (jsonObj) => {
+        // override the default argument constructor because of the special
+        // logic here. Basically we want to accept either a value (old JSONWP)
+        // or a text (new W3C) parameter, but only send one of them to the
+        // command (not both). Prefer 'value' since it's more
+        // backward-compatible.
+        return [jsonObj.value || jsonObj.text];
+      }
+    }}
   },
   '/wd/hub/session/:sessionId/keys': {
     POST: {command: 'keys', payloadParams: {required: ['value']}}

test/mjsonwp/fake-driver.js

@@ -95,8 +95,8 @@ class FakeDriver extends MobileJsonWireProtocol {
     return [attr, elementId, sessionId];
   }
 
-  async setValue (elementId, value) {
-    return value;
+  async setValue (value, elementId) {
+    return [value, elementId];
   }
 
   async performTouch (...args) {

test/mjsonwp/mjsonwp-specs.js

@@ -251,6 +251,36 @@ describe('MJSONWP', async () => {
       });
     });
 
+    describe('w3c sendkeys migration', () => {
+      it('should accept value for sendkeys', async () => {
+        let res = await request({
+          url: 'http://localhost:8181/wd/hub/session/foo/element/bar/value',
+          method: 'POST',
+          json: {value: "text to type"}
+        });
+        res.status.should.equal(0);
+        res.value.should.eql(["text to type", "bar"]);
+      });
+      it('should accept text for sendkeys', async () => {
+        let res = await request({
+          url: 'http://localhost:8181/wd/hub/session/foo/element/bar/value',
+          method: 'POST',
+          json: {text: "text to type"}
+        });
+        res.status.should.equal(0);
+        res.value.should.eql(["text to type", "bar"]);
+      });
+      it('should accept value and text for sendkeys, and use value', async () => {
+        let res = await request({
+          url: 'http://localhost:8181/wd/hub/session/foo/element/bar/value',
+          method: 'POST',
+          json: {value: "text to type", text: "text to ignore"}
+        });
+        res.status.should.equal(0);
+        res.value.should.eql(["text to type", "bar"]);
+      });
+    });
+
     describe('multiple sets of arguments', () => {
       describe('optional', () => {
         it('should allow moveto with element', async () => {
@@ -316,7 +346,7 @@ describe('MJSONWP', async () => {
 
     });
 
-    describe('optional sets of arguments', async () => {
+    describe('optional sets of arguments', () => {
       let desiredCapabilities = {a: 'b'};
       let requiredCapabilities = {c: 'd'};
       let capabilities = {e: 'f'};

test/mjsonwp/routes-specs.js

@@ -38,7 +38,7 @@ describe('MJSONWP', () => {
       }
       var hash = shasum.digest('hex').substring(0, 8);
       // Modify the hash whenever the protocol has intentionally been modified.
-      hash.should.equal('a1bab7af');
+      hash.should.equal('5165c012');
     });
   });