Bug 608757 - Make API and tests for Utils.json{Load|Save} async. r=mconnor

Utils.jsonLoad() already has an async API. Utils.jsonSave() had an ill-named 'callback' parameter which s renamed to 'obj'; an optional callback parameter (in the async sense) is added.

services/sync/modules/engines.js

@@ -115,7 +115,9 @@ Tracker.prototype = {
 
   loadChangedIDs: function T_loadChangedIDs() {
     Utils.jsonLoad("changes/" + this.file, this, function(json) {
-      this.changedIDs = json;
+      if (json) {
+        this.changedIDs = json;
+      }
     });
   },
 
@@ -523,7 +525,9 @@ SyncEngine.prototype = {
     // Initialize to empty if there's no file
     this._toFetch = [];
     Utils.jsonLoad("toFetch/" + this.name, this, function(toFetch) {
-      this._toFetch = toFetch;
+      if (toFetch) {
+        this._toFetch = toFetch;
+      }
     });
   },
 

services/sync/modules/util.js

@@ -1062,19 +1062,22 @@ let Utils = {
       that._log.trace("Loading json from disk: " + filePath);
 
     let file = Utils.getProfileFile(filePath);
-    if (!file.exists())
+    if (!file.exists()) {
+      callback.call(that);
       return;
+    }
 
+    let json;
     try {
       let [is] = Utils.open(file, "<");
-      let json = Utils.readStream(is);
+      json = JSON.parse(Utils.readStream(is));
       is.close();
-      callback.call(that, JSON.parse(json));
     }
     catch (ex) {
       if (that._log)
         that._log.debug("Failed to load json: " + Utils.exceptionStr(ex));
     }
+    callback.call(that, json);
   },
 
   /**
@@ -1084,21 +1087,26 @@ let Utils = {
    *        Json file path save to weave/[filePath].json
    * @param that
    *        Object to use for logging and "this" for callback
-   * @param callback
+   * @param obj
    *        Function to provide json-able object to save. If this isn't a
    *        function, it'll be used as the object to make a json string.
+   * @param callback
+   *        Function called when the write has been performed. Optional.
    */
-  jsonSave: function Utils_jsonSave(filePath, that, callback) {
+  jsonSave: function Utils_jsonSave(filePath, that, obj, callback) {
     filePath = "weave/" + filePath + ".json";
     if (that._log)
       that._log.trace("Saving json to disk: " + filePath);
 
     let file = Utils.getProfileFile({ autoCreate: true, path: filePath });
-    let json = typeof callback == "function" ? callback.call(that) : callback;
+    let json = typeof obj == "function" ? obj.call(that) : obj;
     let out = JSON.stringify(json);
     let [fos] = Utils.open(file, ">");
     fos.writeString(out);
     fos.close();
+    if (typeof callback == "function") {
+      callback.call(that);
+    }
   },
 
   /**

services/sync/tests/unit/head_helpers.js

@@ -378,6 +378,21 @@ function SyncTestingInfrastructure(engineFactory) {
   };
 }
 
+
+/*
+ * Ensure exceptions from inside callbacks leads to test failures.
+ */
+function ensureThrows(func) {
+  return function() {
+    try {
+      func.apply(this, arguments);
+    } catch (ex) {
+      do_throw(ex);
+    }
+  };
+}
+
+
 /**
  * Print some debug message to the console. All arguments will be printed,
  * separated by spaces.

services/sync/tests/unit/test_resource_async.js

@@ -166,17 +166,6 @@ function run_test() {
                 function (subject) quotaValue = subject);
 
 
-  // Ensure exceptions from inside callbacks leads to test failures.
-  function ensureThrows(func) {
-    return function() {
-      try {
-        func.apply(this, arguments);
-      } catch (ex) {
-        do_throw(ex);
-      }
-    };
-  }
-
   let res_upload = new AsyncResource("http://localhost:8080/upload");
   let res_headers = new AsyncResource("http://localhost:8080/headers");
 

services/sync/tests/unit/test_utils_json.js

@@ -2,59 +2,86 @@ _("Make sure json saves and loads from disk");
 Cu.import("resource://services-sync/util.js");
 
 function run_test() {
-  _("Do a simple write of an array to json and read");
-  let foo;
-  Utils.jsonSave("foo", {}, ["v1", "v2"]);
-  Utils.jsonLoad("foo", {}, function(val) {
-    foo = val;
-  });
-  do_check_eq(typeof foo, "object");
-  do_check_eq(foo.length, 2);
-  do_check_eq(foo[0], "v1");
-  do_check_eq(foo[1], "v2");
+  do_test_pending();
+  Utils.asyncChain(function (next) {
 
-  _("Use the function callback version of jsonSave");
-  let bar;
-  Utils.jsonSave("bar", {}, function() ["v1", "v2"]);
-  Utils.jsonLoad("bar", {}, function(val) {
-    bar = val;
-  });
-  do_check_eq(typeof bar, "object");
-  do_check_eq(bar.length, 2);
-  do_check_eq(bar[0], "v1");
-  do_check_eq(bar[1], "v2");
+    _("Do a simple write of an array to json and read");
+    Utils.jsonSave("foo", {}, ["v1", "v2"], ensureThrows(function() {
+      Utils.jsonLoad("foo", {}, ensureThrows(function(val) {
+        let foo = val;
+        do_check_eq(typeof foo, "object");
+        do_check_eq(foo.length, 2);
+        do_check_eq(foo[0], "v1");
+        do_check_eq(foo[1], "v2");
+        next();
+      }));
+    }));
 
-  _("Try saving simple strings");
-  let str;
-  Utils.jsonSave("str", {}, "hi");
-  Utils.jsonLoad("str", {}, function(val) {
-    str = val;
-  });
-  do_check_eq(typeof str, "string");
-  do_check_eq(str.length, 2);
-  do_check_eq(str[0], "h");
-  do_check_eq(str[1], "i");
+  }, function (next) {
 
-  _("Try saving a number");
-  let num;
-  Utils.jsonSave("num", {}, function() 42);
-  Utils.jsonLoad("num", {}, function(val) {
-    num = val;
-  });
-  do_check_eq(typeof num, "number");
-  do_check_eq(num, 42);
+    _("Try saving simple strings");
+    Utils.jsonSave("str", {}, "hi", ensureThrows(function() {
+      Utils.jsonLoad("str", {}, ensureThrows(function(val) {
+        let str = val;
+        do_check_eq(typeof str, "string");
+        do_check_eq(str.length, 2);
+        do_check_eq(str[0], "h");
+        do_check_eq(str[1], "i");
+        next();
+      }));
+    }));
+
+  }, function (next) {
+
+    _("Try saving a number");
+    Utils.jsonSave("num", {}, 42, ensureThrows(function() {
+      Utils.jsonLoad("num", {}, ensureThrows(function(val) {
+        let num = val;
+        do_check_eq(typeof num, "number");
+        do_check_eq(num, 42);
+        next();
+      }));
+    }));
+
+  }, function (next) {
+
+    _("Try loading a non-existent file.");
+    Utils.jsonLoad("non-existent", {}, ensureThrows(function(val) {
+      do_check_eq(val, undefined);
+      next();
+    }));
+    
+  }, function (next) {
+
+    _("Verify that writes are logged.");
+    let trace;
+    Utils.jsonSave("log", {_log: {trace: function(msg) { trace = msg; }}},
+                   "hi", ensureThrows(function () {
+      do_check_true(!!trace);
+      next();
+    }));
+
+  }, function (next) {
+
+    _("Verify that reads and read errors are logged.");
+
+    // Write a file with some invalid JSON
+    let file = Utils.getProfileFile({ autoCreate: true,
+                                      path: "weave/log.json" });
+    let [fos] = Utils.open(file, ">");
+    fos.writeString("invalid json!");
+    fos.close();
+
+    let trace, debug;
+    Utils.jsonLoad("log",
+                   {_log: {trace: function(msg) { trace = msg; },
+                           debug: function(msg) { debug = msg; }}},
+                   ensureThrows(function(val) {
+      do_check_true(!!trace);
+      do_check_true(!!debug);
+      next();
+    }));
+
+  }, do_test_finished)();
 
-  _("Verify that things get logged");
-  let trace, debug;
-  Utils.jsonSave("str",
-                 {_log: {trace: function(msg) { trace = msg; }}},
-                 "hi");
-  do_check_true(!!trace);
-  trace = undefined;
-  Utils.jsonLoad("str",
-                 {_log: {trace: function(msg) { trace = msg; },
-                         debug: function(msg) { debug = msg; }}},
-                 function(val) { throw "exception"; });
-  do_check_true(!!trace);
-  do_check_true(!!debug);
 }