fix method ordering & system test

stephenplusplus · stephenplusplus · commit 77cbb3aa4137 · 2016-05-26T11:23:37.000-04:00
diff --git a/lib/datastore/transaction.js b/lib/datastore/transaction.js
@@ -376,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.
diff --git 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
@@ -333,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]
       ]);
     });
 
