Require transform functions to return objects.

Specifically, only those passed into `Meteor.Collection`.

packages/mongo-livedata/allow_tests.js

@@ -335,7 +335,7 @@ if (Meteor.isClient) {
         },
         function (test, expect) {
           test.equal(
-            restrictedCollectionWithTransform.findOne({"a.bar": "bar"}),
+            _.omit(restrictedCollectionWithTransform.findOne({"a.bar": "bar"}), '_id'),
             {foo: "foo", bar: "bar", baz: "baz"});
           restrictedCollectionWithTransform.remove(item1, expect(function (e, res) {
             test.isFalse(e);

packages/mongo-livedata/collection.js

@@ -38,10 +38,11 @@ Meteor.Collection = function (name, options) {
     break;
   }
 
-  if (options.transform)
-    self._transform = Deps._makeNonreactive(options.transform);
-  else
+  if (options.transform) {
+    self._transform = Deps._makeNonreactive(transformTransform(options.transform));
+  } else {
     self._transform = null;
+  }
 
   if (!name && (name !== null)) {
     Meteor._debug("Warning: creating anonymous collection. It will not be " +
@@ -184,6 +185,39 @@ Meteor.Collection = function (name, options) {
   }
 };
 
+// Transform a transform function to return objects that have the
+// original _id field. Used by transform functions passed into `new
+// Meteor.Collection`. This ensures that subsystems such as the
+// observe-sequence package that call `observe` can keep track of the
+// documents identities.
+//
+// - Require that it returns objects
+// - If the return value has an _id field, verify that it matches the
+//   original _id field
+// - If the return value doesn't have an _id field, add it back.
+var transformTransform = function(transform) {
+  return function (doc) {
+    var id = doc._id;
+    var transformed = transform(doc);
+    if (typeof transformed !== 'object' ||
+        // Even though fine technically, don't let Mongo ObjectIDs
+        // through. It would suck to think your app works until
+        // you insert the first document using Meteor.
+        transformed instanceof Meteor.Collection.ObjectID) {
+      throw new Error("transform must return object");
+    }
+
+    if (transformed._id) {
+      if (transformed._id !== id) {
+        throw new Error("transformed document can't have different _id");
+      }
+    } else {
+      transformed._id = id;
+    }
+    return transformed;
+  };
+};
+
 ///
 /// Main collection API
 ///

packages/mongo-livedata/mongo_livedata_tests.js

@@ -789,6 +789,36 @@ testAsyncMulti('mongo-livedata - document with a date, ' + idGeneration, [
   }
 ]);
 
+testAsyncMulti('mongo-livedata - transform must return an object, ' + idGeneration, [
+  function (test, expect) {
+    var self = this;
+    var justId = function (doc) {
+      return doc._id;
+    };
+    TRANSFORMS["justId"] = justId;
+    var collectionOptions = {
+      idGeneration: idGeneration,
+      transform: justId,
+      transformName: "justId"
+    };
+    var collectionName = Random.id();
+    if (Meteor.isClient) {
+      Meteor.call('createInsecureCollection', collectionName, collectionOptions);
+      Meteor.subscribe('c-' + collectionName);
+    }
+    self.coll = new Meteor.Collection(collectionName, collectionOptions);
+    self.coll.insert({}, expect(function (err, id) {
+      test.isFalse(err);
+      test.isTrue(id);
+      test.throws(function () {
+        self.coll.findOne();
+      });
+      // you can still override the transform though.
+      test.equal(self.coll.findOne({}, {transform: null})._id, id);
+    }));
+  }
+]);
+
 testAsyncMulti('mongo-livedata - document goes through a transform, ' + idGeneration, [
   function (test, expect) {
     var self = this;
@@ -847,15 +877,63 @@ testAsyncMulti('mongo-livedata - document goes through a transform, ' + idGenera
       test.isTrue(id);
       self.id2 = id;
     }));
-  },
+  }
+]);
+
+testAsyncMulti('mongo-livedata - transformed object can\'t have conflicting _id, ' + idGeneration, [
+  function (test, expect) {
+    var self = this;
+    var justId = function (doc) {
+      doc._id = "foo";
+      return doc;
+    };
+    TRANSFORMS["justId"] = justId;
+    var collectionOptions = {
+      idGeneration: idGeneration,
+      transform: justId,
+      transformName: "justId"
+    };
+    var collectionName = Random.id();
+    if (Meteor.isClient) {
+      Meteor.call('createInsecureCollection', collectionName, collectionOptions);
+      Meteor.subscribe('c-' + collectionName);
+    }
+    self.coll = new Meteor.Collection(collectionName, collectionOptions);
+    self.coll.insert({}, expect(function (err, id) {
+      test.isFalse(err);
+      test.isTrue(id);
+      test.throws(function () {
+        self.coll.findOne();
+      });
+      // you can still override the transform though.
+      test.equal(self.coll.findOne({}, {transform: null})._id, id);
+    }));
+  }
+]);
+
+testAsyncMulti('mongo-livedata - transform sets _id if not present, ' + idGeneration, [
   function (test, expect) {
     var self = this;
-    // Test that a transform that returns something other than a document with
-    // an _id (eg, a number) works. Regression test for #974.
-    test.equal(self.coll.find({}, {
-      transform: function (doc) { return doc.d.getSeconds(); },
-      sort: {d: 1}
-    }).fetch(), [50, 51]);
+    var justId = function (doc) {
+      return _.omit(doc, '_id');
+    };
+    TRANSFORMS["justId"] = justId;
+    var collectionOptions = {
+      idGeneration: idGeneration,
+      transform: justId,
+      transformName: "justId"
+    };
+    var collectionName = Random.id();
+    if (Meteor.isClient) {
+      Meteor.call('createInsecureCollection', collectionName, collectionOptions);
+      Meteor.subscribe('c-' + collectionName);
+    }
+    self.coll = new Meteor.Collection(collectionName, collectionOptions);
+    self.coll.insert({}, expect(function (err, id) {
+      test.isFalse(err);
+      test.isTrue(id);
+      test.equal(self.coll.findOne()._id, id);
+    }));
   }
 ]);