From d262968b1c327dc94d889777e90d059734fff0a9 Mon Sep 17 00:00:00 2001
From: Stephen Sawchuk <sawchuk@gmail.com>
Date: Tue, 22 Sep 2015 09:40:52 -0400
Subject: [PATCH] datastore: throw if an unrecognized commit method is given

---
 lib/datastore/request.js  | 109 ++++++++++++++++++--------------------
 test/datastore/request.js |  12 +++++
 2 files changed, 64 insertions(+), 57 deletions(-)

diff --git a/lib/datastore/request.js b/lib/datastore/request.js
index 986cebb9bb4..eb73093e0f8 100644
--- a/lib/datastore/request.js
+++ b/lib/datastore/request.js
@@ -227,10 +227,12 @@ DatastoreRequest.prototype.insert = function(entities, callback) {
  *
  * @borrows {module:datastore/transaction#save} as save
  *
+ * @throws {Error} If an unrecognized method is provided.
+ *
  * @param {object|object[]} entities - Datastore key object(s).
  * @param {Key} entities.key - Datastore key object.
  * @param {string=} entities.method - Optional method to explicity use for save.
- *     The choices include 'insert', 'update', 'upsert' and 'auto_insert_id'.
+ *     The choices include 'insert', 'update', 'upsert' and 'insert_auto_id'.
  * @param {object|object[]} entities.data - Data to save with the provided key.
  *     If you provide an array of objects, you must use the explicit syntax:
  *     `name` for the name of the property and `value` for its value. You may
@@ -382,75 +384,68 @@ DatastoreRequest.prototype.save = function(entities, callback) {
 
   var insertIndexes = [];
 
-  var req = {
-    mutation: entities.reduce(function(acc, entityObject, index) {
-      entityObject = extend(true, {}, entityObject);
+  var mutation = {
+    insert: [],
+    update: [],
+    upsert: [],
+    insert_auto_id: []
+  };
 
-      var ent = {};
-      var method = entityObject.method;
-      delete entityObject.method;
+  // Iterate over the entity objects, build a proto from all keys and values,
+  // then place in the correct mutation array (insert, update, etc).
+  entities.forEach(function(entityObject, index) {
+    entityObject = extend(true, {}, entityObject);
 
-      if (Array.isArray(entityObject.data)) {
-        ent.property = entityObject.data.map(function(data) {
-          data.value = entity.valueToProperty(data.value);
+    var entityProto = {};
+    var method = entityObject.method;
 
-          if (is.boolean(data.excludeFromIndexes)) {
-            var indexed = !data.excludeFromIndexes;
+    if (is.array(entityObject.data)) {
+      entityProto.property = entityObject.data.map(function(data) {
+        data.value = entity.valueToProperty(data.value);
 
-            if (is.array(data.value.list_value)) {
-              data.value.list_value =
-                data.value.list_value.map(propAssign('indexed', indexed));
-            } else {
-              data.value.indexed = indexed;
-            }
+        if (is.boolean(data.excludeFromIndexes)) {
+          var indexed = !data.excludeFromIndexes;
 
-            delete data.excludeFromIndexes;
+          if (is.array(data.value.list_value)) {
+            data.value.list_value =
+              data.value.list_value.map(propAssign('indexed', indexed));
+          } else {
+            data.value.indexed = indexed;
           }
 
-          return data;
-        });
-      } else {
-        ent = entity.entityToEntityProto(entityObject.data);
-      }
+          delete data.excludeFromIndexes;
+        }
 
-      ent.key = entity.keyToKeyProto(entityObject.key);
+        return data;
+      });
+    } else {
+      entityProto = entity.entityToEntityProto(entityObject.data);
+    }
 
-      if (method) {
-        switch (method) {
-          case 'insert': {
-            acc.insert.push(ent);
-            break;
-          }
-          case 'update': {
-            acc.update.push(ent);
-            break;
-          }
-          case 'upsert': {
-            acc.upsert.push(ent);
-            break;
-          }
-          case 'insert_auto_id': {
-            insertIndexes.push(index);
-            acc.insert_auto_id.push(ent);
-            break;
-          }
-        }
-      } else {
-        if (entity.isKeyComplete(entityObject.key)) {
-          acc.upsert.push(ent);
-        } else {
+    entityProto.key = entity.keyToKeyProto(entityObject.key);
+
+    if (method) {
+      if (mutation[method]) {
+        mutation[method].push(entityProto);
+
+        if (method === 'insert_auto_id') {
           insertIndexes.push(index);
-          acc.insert_auto_id.push(ent);
         }
+      } else {
+        throw new Error('Method ' + method + ' not recognized.');
       }
+    } else {
+      if (entity.isKeyComplete(entityObject.key)) {
+        mutation.upsert.push(entityProto);
+      } else {
+        insertIndexes.push(index);
+        mutation.insert_auto_id.push(entityProto);
+      }
+    }
+  });
 
-      return acc;
-    }, {
-      insert: [],
-      update: [],
-      upsert: [],
-      insert_auto_id: []
-    })
+  var req = {
+    mutation: mutation
   };
 
   if (this.id) {
diff --git a/test/datastore/request.js b/test/datastore/request.js
index 431c1560816..88786e9d5a8 100644
--- a/test/datastore/request.js
+++ b/test/datastore/request.js
@@ -455,6 +455,18 @@ describe('Request', function() {
       ], done);
     });
 
+    it('should throw if a given method is not recognized', function() {
+      assert.throws(function() {
+        request.save({
+          key: key,
+          method: 'auto_insert_id',
+          data: {
+            k: 'v'
+          }
+        }, assert.ifError);
+      }, /Method auto_insert_id not recognized/);
+    });
+
     it('should not alter the provided data object', function(done) {
       var entities = [
         {