From fa356ec797d45eb6265f254a82aac877e2e59e17 Mon Sep 17 00:00:00 2001
From: Avital Oliver <avital@thewe.net>
Date: Fri, 10 Jan 2014 15:19:59 -0500
Subject: [PATCH] observe-sequence: fix non-string ids

---
 packages/observe-sequence/observe_sequence.js | 38 +++++++++++++------
 .../observe_sequence_tests.js                 | 18 +++++++++
 2 files changed, 45 insertions(+), 11 deletions(-)

diff --git a/packages/observe-sequence/observe_sequence.js b/packages/observe-sequence/observe_sequence.js
index f9e68eef50..4d94f59b61 100644
--- a/packages/observe-sequence/observe_sequence.js
+++ b/packages/observe-sequence/observe_sequence.js
@@ -12,6 +12,9 @@ if (typeof console !== 'undefined' && console.warn) {
   warn = function () {};
 }
 
+var idStringify = LocalCollection._idStringify;
+var idParse = LocalCollection._idParse;
+
 ObserveSequence = {
   _suppressWarnings: 0,
   _loggedWarnings: 0,
@@ -109,7 +112,7 @@ ObserveSequence = {
             else
               throw new Error("unsupported type in {{#each}}: " + typeof item);
 
-            var idString = LocalCollection._idStringify(id);
+            var idString = idStringify(id);
             if (idsUsed[idString]) {
               warn("duplicate id " + id + " in", seq);
               id = Random.id();
@@ -119,6 +122,7 @@ ObserveSequence = {
 
             return { _id: id, item: item };
           });
+
           diffArray(lastSeqArray, seqArray, callbacks);
         } else if (isMinimongoCursor(seq)) {
           var cursor = seq;
@@ -202,16 +206,16 @@ var diffArray = function (lastSeqArray, seqArray, callbacks) {
   var diffFn = Package.minimongo.LocalCollection._diffQueryOrderedChanges;
   var oldIdObjects = [];
   var newIdObjects = [];
-  var posOld = {};
-  var posNew = {};
+  var posOld = {}; // maps from idStringify'd ids
+  var posNew = {}; // ditto
 
   _.each(seqArray, function (doc, i) {
     newIdObjects.push(_.pick(doc, '_id'));
-    posNew[doc._id] = i;
+    posNew[idStringify(doc._id)] = i;
   });
   _.each(lastSeqArray, function (doc, i) {
     oldIdObjects.push(_.pick(doc, '_id'));
-    posOld[doc._id] = i;
+    posOld[idStringify(doc._id)] = i;
   });
 
   // Arrays can contain arbitrary objects. We don't diff the
@@ -220,18 +224,30 @@ var diffArray = function (lastSeqArray, seqArray, callbacks) {
   // it appropriately.
   diffFn(oldIdObjects, newIdObjects, {
     addedBefore: function (id, doc, before) {
-      callbacks.addedAt(id, seqArray[posNew[id]].item, posNew[id], before);
+      callbacks.addedAt(
+        id,
+        seqArray[posNew[idStringify(id)]].item,
+        posNew[idStringify(id)],
+        before);
     },
     movedBefore: function (id, before) {
-      callbacks.movedTo(id, seqArray[posNew[id]].item, posOld[id], posNew[id], before);
+      callbacks.movedTo(
+        id,
+        seqArray[posNew[idStringify(id)]].item,
+        posOld[idStringify(id)],
+        posNew[idStringify(id)],
+        before);
     },
     removed: function (id) {
-      callbacks.removed(id, lastSeqArray[posOld[id]].item);
+      callbacks.removed(
+        id,
+        lastSeqArray[posOld[idStringify(id)]].item);
     }
   });
 
-  _.each(posNew, function (pos, id) {
-    if (_.has(posOld, id)) {
+  _.each(posNew, function (pos, idString) {
+    var id = idParse(idString);
+    if (_.has(posOld, idString)) {
       // specifically for primitive types, compare equality before
       // firing the changed callback. otherwise, always fire it
       // because doing a deep EJSON comparison is not guaranteed to
@@ -240,7 +256,7 @@ var diffArray = function (lastSeqArray, seqArray, callbacks) {
       // necessarily the most efficient (if only a specific subfield
       // of the object is later accessed).
       var newItem = seqArray[pos].item;
-      var oldItem = lastSeqArray[posOld[id]].item;
+      var oldItem = lastSeqArray[posOld[idString]].item;
 
       if (typeof newItem === 'object' || newItem !== oldItem)
         callbacks.changed(id, newItem, oldItem);
diff --git a/packages/observe-sequence/observe_sequence_tests.js b/packages/observe-sequence/observe_sequence_tests.js
index 993a4e8534..78da06c8a0 100644
--- a/packages/observe-sequence/observe_sequence_tests.js
+++ b/packages/observe-sequence/observe_sequence_tests.js
@@ -154,6 +154,24 @@ Tinytest.add('observe sequence - array to other array', function (test) {
   ]);
 });
 
+Tinytest.add('observe sequence - array to other array without ids', function (test) {
+  var dep = new Deps.Dependency;
+  var seq = [{foo: 1}, {bar: 2}];
+
+  runOneObserveSequenceTestCase(test, function () {
+    dep.depend();
+    return seq;
+  }, function () {
+    seq = [{foo: 2}];
+    dep.changed();
+  }, [
+    {addedAt: [0, {foo: 1}, 0, null]},
+    {addedAt: [1, {bar: 2}, 1, null]},
+    {removed: [1, {bar: 2}]},
+    {changed: [0, {foo: 2}, {foo: 1}]}
+  ]);
+});
+
 Tinytest.add('observe sequence - array to other array, changes', function (test) {
   var dep = new Deps.Dependency;
   var seq = [{_id: "13", foo: 1}, {_id: "37", bar: 2}, {_id: "42", baz: 42}];