diff --git a/lib/datastore/transaction.js b/lib/datastore/transaction.js index e30f9038017..9ba5bb12f85 100644 --- a/lib/datastore/transaction.js +++ b/lib/datastore/transaction.js @@ -21,8 +21,9 @@ 'use strict'; var arrify = require('arrify'); -var extend = require('extend'); +var flatten = require('lodash.flatten'); var nodeutil = require('util'); +var prop = require('propprop'); /** * @type {module:datastore/request} @@ -375,9 +376,11 @@ Transaction.prototype.commit_ = function(callback) { } }) - // Group entities together by action (delete or save). + // Group entities together by method: `save` mutations, then `delete`. Note: + // `save` mutations being first is required to maintain order when assigning + // IDs to incomplete keys. .sort(function(a, b) { - return a.method > b.method ? 1 : a.method < b.method ? -1 : 0; + return a.method < b.method ? 1 : a.method > b.method ? -1 : 0; }) // Group arguments together so that we only make one call to each method. @@ -418,10 +421,9 @@ Transaction.prototype.commit_ = function(callback) { // Take the `req` array built previously, and merge them into one request to // send as the final transactional commit. - var reqOpts = this.requests_.reduce(function(acc, req) { - return extend(true, acc, req); - }, {}); - + var reqOpts = { + mutations: flatten(this.requests_.map(prop('mutations'))) + }; this.request_(protoOpts, reqOpts, function(err, resp) { if (err) { diff --git a/package.json b/package.json index 0c974614cbb..5cea3b6dece 100644 --- a/package.json +++ b/package.json @@ -106,6 +106,7 @@ "grpc": "^0.14.1", "hash-stream-validation": "^0.1.0", "is": "^3.0.1", + "lodash.flatten": "^4.2.0", "methmeth": "^1.0.0", "mime-types": "^2.0.8", "modelo": "^4.2.0", diff --git a/system-test/datastore.js b/system-test/datastore.js index 958cad8fc3e..38630344361 100644 --- a/system-test/datastore.js +++ b/system-test/datastore.js @@ -690,48 +690,52 @@ describe('Datastore', function() { var key = datastore.key(['Company', 'Google']); var incompleteKey = datastore.key('Company'); - datastore.runInTransaction(function(t, tDone) { - t.delete(deleteKey); - - t.save([ - { - key: key, - data: { rating: 10 } - }, - { - key: incompleteKey, - data: { rating: 100 } - } - ]); - - tDone(); + datastore.save({ + key: deleteKey, + data: {} }, function(err) { assert.ifError(err); - // Incomplete key should have been given an ID. - assert.strictEqual(incompleteKey.path.length, 2); - - async.parallel([ - // The key queued for deletion should have been deleted. - function(done) { - datastore.get(deleteKey, function(err, entity) { - assert.ifError(err); - assert.strictEqual(typeof entity, 'undefined'); - done(); - }); - }, + datastore.runInTransaction(function(t, tDone) { + t.delete(deleteKey); + + t.save([ + { + key: key, + data: { rating: 10 } + }, + { + key: incompleteKey, + data: { rating: 100 } + } + ]); - // Data should have been updated on the key. - function(done) { - datastore.get(key, function(err, entity) { - assert.ifError(err); - assert.strictEqual(entity.data.rating, 10); - done(); - }); - } - ], function(err) { + tDone(); + }, function(err) { assert.ifError(err); - datastore.delete([key, incompleteKey], done); + + // Incomplete key should have been given an ID. + assert.strictEqual(incompleteKey.path.length, 2); + + async.parallel([ + // The key queued for deletion should have been deleted. + function(callback) { + datastore.get(deleteKey, function(err, entity) { + assert.ifError(err); + assert.strictEqual(typeof entity, 'undefined'); + callback(); + }); + }, + + // Data should have been updated on the key. + function(callback) { + datastore.get(key, function(err, entity) { + assert.ifError(err); + assert.strictEqual(entity.data.rating, 10); + callback(); + }); + } + ], done); }); }); }); diff --git a/test/datastore/transaction.js b/test/datastore/transaction.js index 8f7b8b24ebd..6e675b3a5ca 100644 --- a/test/datastore/transaction.js +++ b/test/datastore/transaction.js @@ -19,7 +19,6 @@ var arrify = require('arrify'); var assert = require('assert'); var entity = require('../../lib/datastore/entity.js'); -var extend = require('extend'); var mockery = require('mockery-next'); var util = require('../../lib/common/util.js'); @@ -334,9 +333,11 @@ describe('Transaction', function() { assert.equal(saveCalled, 1); assert.equal(args.length, 2); + + // Save arguments must come first. assert.deepEqual(args, [ - [deleteArg1, deleteArg2], - [saveArg1, saveArg2] + [saveArg1, saveArg2], + [deleteArg1, deleteArg2] ]); }); @@ -364,14 +365,29 @@ describe('Transaction', function() { it('should send the built request object', function(done) { transaction.requests_ = [ - { a: 'b', c: 'd' }, - { e: 'f', g: 'h' } + { + mutations: [ + { a: 'b' }, + { c: 'd' } + ] + }, + { + mutations: [ + { e: 'f' }, + { g: 'h' } + ] + } ]; transaction.request_ = function(protoOpts, reqOpts) { - var req1 = transaction.requests_[0]; - var req2 = transaction.requests_[1]; - assert.deepEqual(reqOpts, extend(req1, req2)); + assert.deepEqual(reqOpts, { + mutations: [ + { a: 'b' }, + { c: 'd' }, + { e: 'f' }, + { g: 'h' } + ] + }); done(); };