datastore: accept entity object to mutation methods (#1804)

Author: stephenplusplus

handwritten/datastore/src/index.js

@@ -281,10 +281,7 @@ var Transaction = require('./transaction.js');
  *
  *     entity.symbol = 'GOOG';
  *
- *     transaction.save({
- *       key: key,
- *       data: entity
- *     });
+ *     transaction.save(entity);
  *
  *     transaction.commit(function(err) {
  *       if (!err) {

handwritten/datastore/src/request.js

@@ -67,6 +67,42 @@ var CONSISTENCY_PROTO_CODE = {
  */
 function DatastoreRequest() {}
 
+/**
+ * Format a user's input to mutation methods. This will create a deep clone of
+ * the input, as well as allow users to pass an object in the format of an
+ * entity.
+ *
+ * Both of the following formats can be supplied supported:
+ *
+ *     datastore.save({
+ *       key: datastore.key('Kind'),
+ *       data: { foo: 'bar' }
+ *     }, function(err) {})
+ *
+ *     var entity = { foo: 'bar' }
+ *     entity[datastore.KEY] = datastore.key('Kind')
+ *     datastore.save(entity, function(err) {})
+ *
+ * @private
+ *
+ * @resource [#1803]{@link https://github.com/GoogleCloudPlatform/google-cloud-node/issues/1803}
+ *
+ * @param {object} obj - The user's input object.
+ */
+DatastoreRequest.prepareEntityObject_ = function(obj) {
+  var entityObject = extend(true, {}, obj);
+
+  // Entity objects are also supported.
+  if (obj[entity.KEY_SYMBOL]) {
+    return {
+      key: obj[entity.KEY_SYMBOL],
+      data: entityObject
+    };
+  }
+
+  return entityObject;
+};
+
 /**
  * Generate IDs without creating entities.
  *
@@ -426,7 +462,10 @@ DatastoreRequest.prototype.get = function(keys, options, callback) {
  * Maps to {module:datastore#save}, forcing the method to be `insert`.
  */
 DatastoreRequest.prototype.insert = function(entities, callback) {
-  entities = arrify(entities).map(propAssign('method', 'insert'));
+  entities = arrify(entities)
+    .map(DatastoreRequest.prepareEntityObject_)
+    .map(propAssign('method', 'insert'));
+
   this.save(entities, callback);
 };
 
@@ -863,53 +902,53 @@ DatastoreRequest.prototype.save = function(entities, callback) {
 
   // 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);
-
-    var mutation = {};
-    var entityProto = {};
-    var method = 'upsert';
-
-    if (entityObject.method) {
-      if (methods[entityObject.method]) {
-        method = entityObject.method;
-      } else {
-        throw new Error('Method ' + entityObject.method + ' not recognized.');
+  entities
+    .map(DatastoreRequest.prepareEntityObject_)
+    .forEach(function(entityObject, index) {
+      var mutation = {};
+      var entityProto = {};
+      var method = 'upsert';
+
+      if (entityObject.method) {
+        if (methods[entityObject.method]) {
+          method = entityObject.method;
+        } else {
+          throw new Error('Method ' + entityObject.method + ' not recognized.');
+        }
       }
-    }
 
-    if (!entity.isKeyComplete(entityObject.key)) {
-      insertIndexes[index] = true;
-    }
+      if (!entity.isKeyComplete(entityObject.key)) {
+        insertIndexes[index] = true;
+      }
 
-    if (is.array(entityObject.data)) {
-      entityProto.properties = entityObject.data.reduce(function(acc, data) {
-        var value = entity.encodeValue(data.value);
+      if (is.array(entityObject.data)) {
+        entityProto.properties = entityObject.data.reduce(function(acc, data) {
+          var value = entity.encodeValue(data.value);
 
-        if (is.boolean(data.excludeFromIndexes)) {
-          var excluded = data.excludeFromIndexes;
-          var values = value.arrayValue && value.arrayValue.values;
+          if (is.boolean(data.excludeFromIndexes)) {
+            var excluded = data.excludeFromIndexes;
+            var values = value.arrayValue && value.arrayValue.values;
 
-          if (values) {
-            values = values.map(propAssign('excludeFromIndexes', excluded));
-          } else {
-            value.excludeFromIndexes = data.excludeFromIndexes;
+            if (values) {
+              values = values.map(propAssign('excludeFromIndexes', excluded));
+            } else {
+              value.excludeFromIndexes = data.excludeFromIndexes;
+            }
           }
-        }
 
-        acc[data.name] = value;
+          acc[data.name] = value;
 
-        return acc;
-      }, {});
-    } else {
-      entityProto = entity.entityToEntityProto(entityObject.data);
-    }
+          return acc;
+        }, {});
+      } else {
+        entityProto = entity.entityToEntityProto(entityObject.data);
+      }
 
-    entityProto.key = entity.keyToKeyProto(entityObject.key);
+      entityProto.key = entity.keyToKeyProto(entityObject.key);
 
-    mutation[method] = entityProto;
-    mutations.push(mutation);
-  });
+      mutation[method] = entityProto;
+      mutations.push(mutation);
+    });
 
   var protoOpts = {
     service: 'Datastore',
@@ -953,15 +992,21 @@ DatastoreRequest.prototype.save = function(entities, callback) {
  * Maps to {module:datastore#save}, forcing the method to be `update`.
  */
 DatastoreRequest.prototype.update = function(entities, callback) {
-  entities = arrify(entities).map(propAssign('method', 'update'));
+  entities = arrify(entities)
+    .map(DatastoreRequest.prepareEntityObject_)
+    .map(propAssign('method', 'update'));
+
   this.save(entities, callback);
 };
 
 /**
  * Maps to {module:datastore#save}, forcing the method to be `upsert`.
  */
 DatastoreRequest.prototype.upsert = function(entities, callback) {
-  entities = arrify(entities).map(propAssign('method', 'upsert'));
+  entities = arrify(entities)
+    .map(DatastoreRequest.prepareEntityObject_)
+    .map(propAssign('method', 'upsert'));
+
   this.save(entities, callback);
 };
 

handwritten/datastore/system-test/datastore.js

@@ -26,7 +26,6 @@ var env = require('../../../system-test/env.js');
 describe('Datastore', function() {
   var testKinds = [];
   var datastore = new Datastore(env);
-
   // Override the Key method so we can track what keys are created during the
   // tests. They are then deleted in the `after` hook.
   var key = datastore.key;
@@ -156,6 +155,32 @@ describe('Datastore', function() {
       });
     });
 
+    it('should save/get/update', function(done) {
+      var postKey = datastore.key('Post');
+
+      datastore.save({ key: postKey, data: post }, function(err) {
+        assert.ifError(err);
+
+        datastore.get(postKey, function(err, entity) {
+          assert.ifError(err);
+
+          assert.strictEqual(entity.title, post.title);
+
+          entity.title = 'Updated';
+
+          datastore.save(entity, function(err) {
+            assert.ifError(err);
+
+            datastore.get(postKey, function(err, entity) {
+              assert.ifError(err);
+              assert.strictEqual(entity.title, 'Updated');
+              datastore.delete(postKey, done);
+            });
+          });
+        });
+      });
+    });
+
     it('should fail explicitly set second insert on save', function(done) {
       var postKey = datastore.key('Post');
 

handwritten/datastore/test/request.js

@@ -88,6 +88,8 @@ describe('Request', function() {
       './entity.js': entity,
       './query.js': FakeQuery
     });
+
+    override('Request', Request);
   });
 
   after(function() {
@@ -110,6 +112,42 @@ describe('Request', function() {
     });
   });
 
+  describe('prepareEntityObject_', function() {
+    it('should clone an object', function() {
+      var obj = {
+        data: {
+          nested: {
+            obj: true
+          }
+        },
+        method: 'insert'
+      };
+      var expectedPreparedEntityObject = extend(true, {}, obj);
+
+      var preparedEntityObject = Request.prepareEntityObject_(obj);
+
+      assert.notStrictEqual(preparedEntityObject, obj);
+
+      assert.notStrictEqual(
+        preparedEntityObject.data.nested,
+        obj.data.nested
+      );
+
+      assert.deepEqual(preparedEntityObject, expectedPreparedEntityObject);
+    });
+
+    it('should format an entity', function() {
+      var key = {};
+      var entityObject = { data: true };
+      entityObject[entity.KEY_SYMBOL] = key;
+
+      var preparedEntityObject = Request.prepareEntityObject_(entityObject);
+
+      assert.strictEqual(preparedEntityObject.key, key);
+      assert.deepEqual(preparedEntityObject.data, entityObject);
+    });
+  });
+
   describe('allocateIds', function() {
     var incompleteKey;
     var apiResponse = {
@@ -597,6 +635,26 @@ describe('Request', function() {
   });
 
   describe('insert', function() {
+    it('should prepare entity objects', function(done) {
+      var entityObject = {};
+      var preparedEntityObject = { prepared: true };
+      var expectedEntityObject = extend({}, preparedEntityObject, {
+        method: 'insert'
+      });
+
+      overrides.Request.prepareEntityObject_ = function(obj) {
+        assert.strictEqual(obj, entityObject);
+        return preparedEntityObject;
+      };
+
+      request.save = function(entities) {
+        assert.deepEqual(entities[0], expectedEntityObject);
+        done();
+      };
+
+      request.insert(entityObject, assert.ifError);
+    });
+
     it('should pass the correct arguments to save', function(done) {
       request.save = function(entities, callback) {
         assert.deepEqual(entities, [{
@@ -1134,6 +1192,17 @@ describe('Request', function() {
       ], done);
     });
 
+    it('should prepare entity objects', function(done) {
+      var entityObject = {};
+
+      overrides.Request.prepareEntityObject_ = function(obj) {
+        assert.strictEqual(obj, entityObject);
+        done();
+      };
+
+      request.save(entityObject, assert.ifError);
+    });
+
     it('should save with specific method', function(done) {
       request.request_ = function(protoOpts, reqOpts, callback) {
         assert.equal(reqOpts.mutations.length, 3);
@@ -1324,6 +1393,26 @@ describe('Request', function() {
   });
 
   describe('update', function() {
+    it('should prepare entity objects', function(done) {
+      var entityObject = {};
+      var preparedEntityObject = { prepared: true };
+      var expectedEntityObject = extend({}, preparedEntityObject, {
+        method: 'update'
+      });
+
+      overrides.Request.prepareEntityObject_ = function(obj) {
+        assert.strictEqual(obj, entityObject);
+        return preparedEntityObject;
+      };
+
+      request.save = function(entities) {
+        assert.deepEqual(entities[0], expectedEntityObject);
+        done();
+      };
+
+      request.update(entityObject, assert.ifError);
+    });
+
     it('should pass the correct arguments to save', function(done) {
       request.save = function(entities, callback) {
         assert.deepEqual(entities, [{
@@ -1345,6 +1434,26 @@ describe('Request', function() {
   });
 
   describe('upsert', function() {
+    it('should prepare entity objects', function(done) {
+      var entityObject = {};
+      var preparedEntityObject = { prepared: true };
+      var expectedEntityObject = extend({}, preparedEntityObject, {
+        method: 'upsert'
+      });
+
+      overrides.Request.prepareEntityObject_ = function(obj) {
+        assert.strictEqual(obj, entityObject);
+        return preparedEntityObject;
+      };
+
+      request.save = function(entities) {
+        assert.deepEqual(entities[0], expectedEntityObject);
+        done();
+      };
+
+      request.upsert(entityObject, assert.ifError);
+    });
+
     it('should pass the correct arguments to save', function(done) {
       request.save = function(entities, callback) {
         assert.deepEqual(entities, [{