Provide document ID to Meteor.Collection callback.

docs/client/api.html

@@ -510,15 +510,18 @@ in the database, and return the ID.
 
 On the server, if you don't provide a callback, then `insert` blocks
 until the database acknowledges the write, or throws an exception if
-something went wrong.  If you do provide a callback, `insert` returns
-immediately.  Once the insert completes (or fails), the callback is
-called with error and result arguments, same as for
-[`methods`](#methods_header).
+something went wrong.  If you do provide a callback, `insert` still
+returns the ID immediately.  Once the insert completes (or fails), the
+callback is called with error and result arguments.  In an error case,
+`result` is undefined.  If the insert is successful, `error` is
+undefined and `result` is the new document ID.
 
 On the client, `insert` never blocks.  If you do not provide a callback
 and the insert fails on the server, then Meteor will log a warning to
 the console.  If you provide a callback, Meteor will call that function
-with the error or result of the server's insert.
+with `error` and `result` arguments.  In an error case, `result` is
+undefined.  If the insert is successful, `error` is undefined and
+`result` is the new document ID.
 
 Example:
 

docs/client/api.js

@@ -395,7 +395,7 @@ Template.api.insert = {
      descr: "The document to insert. Should not yet have an _id attribute."},
     {name: "callback",
      type: "Function",
-     descr: "Optional.  If present, called with an error object as the first argument and the _id as the second."}
+     descr: "Optional.  If present, called with an error object as the first argument and, if no error, the _id as the second."}
   ]
 };
 
@@ -415,7 +415,7 @@ Template.api.update = {
      descr: "Specifies how to modify the documents"},
     {name: "callback",
      type: "Function",
-     descr: "Optional.  If present, called with an error object as the first argument and the result as the second."}
+     descr: "Optional.  If present, called with an error object as its argument."}
   ],
   options: [
     {name: "multi",
@@ -436,7 +436,7 @@ Template.api.remove = {
      descr: "Specifies which documents to remove"},
     {name: "callback",
      type: "Function",
-     descr: "Optional.  If present, called with an error object as the first argument and the result as the second."}
+     descr: "Optional.  If present, called with an error object as its argument."}
   ]
 };
 

packages/mongo-livedata/collection.js

@@ -92,19 +92,19 @@ Meteor.Collection = function (name, manager, driver) {
     self._prefix = '/' + name + '/';
     m[self._prefix + 'insert'] = function (/* selector, options */) {
       self._maybe_snapshot();
-      // Allow exceptions to propagate
+      // insert returns nothing.  allow exceptions to propagate.
       self._collection.insert.apply(self._collection, _.toArray(arguments));
     };
 
     m[self._prefix + 'update'] = function (/* selector, mutator, options */) {
       self._maybe_snapshot();
-      // Allow exceptions to propagate
+      // update returns nothing.  allow exceptions to propagate.
       self._collection.update.apply(self._collection, _.toArray(arguments));
     };
 
     m[self._prefix + 'remove'] = function (/* selector */) {
       self._maybe_snapshot();
-      // Allow exceptions to propagate
+      // remove returns nothing.  allow exceptions to propagate.
       self._collection.remove.apply(self._collection, _.toArray(arguments));
     };
 
@@ -151,8 +151,10 @@ _.extend(Meteor.Collection.prototype, {
 // provided, they block until the operation is complete, and throw an
 // exception if it fails; if a callback is provided, then they don't
 // necessarily block, and they call the callback when they finish with
-// zero arguments on success, or one argument, an exception, on
-// failure; on the client, blocking is impossible, so if a callback
+// error and result arguments.  (The insert method provides the
+// document ID as its result; update and remove don't provide a result.)
+//
+// On the client, blocking is impossible, so if a callback
 // isn't provided, they just return immediately and any error
 // information is lost.
 //
@@ -169,9 +171,11 @@ _.each(["insert", "update", "remove"], function (name) {
   Meteor.Collection.prototype[name] = function (/* arguments */) {
     var self = this;
     var args = _.toArray(arguments);
+    var callback;
+    var ret;
 
     if (args.length && args[args.length - 1] instanceof Function)
-      var callback = args.pop();
+      callback = args.pop();
 
     if (Meteor.is_client && !callback)
       // Client can't block, so it can't report errors by exception,
@@ -191,33 +195,42 @@ _.each(["insert", "update", "remove"], function (name) {
       args[0] = _.extend({}, args[0]);
       if ('_id' in args[0])
         throw new Error("Do not pass an _id to insert. Meteor will generate the _id for you.");
-      var ret = args[0]._id = Meteor.uuid();
+      ret = args[0]._id = Meteor.uuid();
     }
 
     if (self._manager && self._manager !== Meteor.default_server) {
-      // NB: on failure, allow exception to propagate
-      self._manager.apply(self._prefix + name, args, callback);
-    }
-    else {
+      // just remote to another endpoint, propagate return value or
+      // exception.
+      if (callback)
+        // asynchronous: on success, callback should return ret
+        // (document ID for insert, undefined for update and
+        // remove), not the method's result.
+        self._manager.apply(self._prefix + name, args, function (error, result) {
+          callback(error, !error && ret);
+        });
+      else
+        // synchronous: propagate exception
+        self._manager.apply(self._prefix + name, args);
+
+    } else {
+      // it's my collection.  descend into the collection object
+      // and propagate any exception.
       try {
         self._collection[name].apply(self._collection, args);
       } catch (e) {
         if (callback) {
           callback(e);
-          return;
+          return null;
         }
-
-        // Note that on the client, this will never happen, because
-        // we will have been provided with a default callback. (This
-        // is nice because it matches the behavior of named
-        // collections, which on the client never throw exceptions
-        // directly.)
         throw e;
       }
 
-      callback && callback();
+      // on success, return *ret*, not the manager's return value.
+      callback && callback(null, ret);
     }
 
+    // both sync and async, unless we threw an exception, return ret
+    // (new document ID for insert, undefined otherwise).
     return ret;
   };
 });

packages/mongo-livedata/mongo_livedata_tests.js

@@ -9,7 +9,7 @@ testAsyncMulti("mongo-livedata - database failure reporting", [
   function (test, expect) {
     var ftc = Meteor._FailureTestCollection;
 
-    var exception = function (err) {
+    var exception = function (err, res) {
       test.instanceOf(err, Error);
     };