Bug 584927 - Factor GUIDForUri and setGUID into HistoryStore [r=mconnor]

services/sync/modules/engines/history.js

@@ -50,31 +50,6 @@ Cu.import("resource://services-sync/type_records/history.js");
 Cu.import("resource://services-sync/util.js");
 Cu.import("resource://services-sync/log4moz.js");
 
-// Create some helper functions to handle GUIDs
-function setGUID(uri, guid) {
-  if (arguments.length == 1)
-    guid = Utils.makeGUID();
-
-  try {
-    Utils.anno(uri, GUID_ANNO, guid, "WITH_HISTORY");
-  } catch (ex) {
-    let log = Log4Moz.repository.getLogger("Engine.History");
-    log.warn("Couldn't annotate URI " + uri + ": " + ex);
-  }
-  return guid;
-}
-function GUIDForUri(uri, create) {
-  try {
-    // Use the existing GUID if it exists
-    return Utils.anno(uri, GUID_ANNO);
-  }
-  catch (ex) {
-    // Give the uri a GUID if it doesn't have one
-    if (create)
-      return setGUID(uri);
-  }
-}
-
 function HistoryEngine() {
   SyncEngine.call(this, "History");
 }
@@ -87,7 +62,7 @@ HistoryEngine.prototype = {
   _sync: Utils.batchSync("History", SyncEngine),
 
   _findDupe: function _findDupe(item) {
-    return GUIDForUri(item.histUri);
+    return this._store.GUIDForUri(item.histUri);
   }
 };
 
@@ -142,6 +117,31 @@ HistoryStore.prototype = {
     return this.__haveTempTables;
   },
 
+  // Some helper functions to handle GUIDs
+  setGUID: function setGUID(uri, guid) {
+    if (arguments.length == 1)
+      guid = Utils.makeGUID();
+
+    try {
+      Utils.anno(uri, GUID_ANNO, guid, "WITH_HISTORY");
+    } catch (ex) {
+      let log = Log4Moz.repository.getLogger("Engine.History");
+      log.warn("Couldn't annotate URI " + uri + ": " + ex);
+    }
+    return guid;
+  },
+
+  GUIDForUri: function GUIDForUri(uri, create) {
+    try {
+      // Use the existing GUID if it exists
+      return Utils.anno(uri, GUID_ANNO);
+    } catch (ex) {
+      // Give the uri a GUID if it doesn't have one
+      if (create)
+        return this.setGUID(uri);
+    }
+  },
+
   get _visitStm() {
     // Gecko <2.0
     if (this._haveTempTables) {
@@ -219,7 +219,7 @@ HistoryStore.prototype = {
   },
 
   changeItemID: function HStore_changeItemID(oldID, newID) {
-    setGUID(this._findURLByGUID(oldID).url, newID);
+    this.setGUID(this._findURLByGUID(oldID).url, newID);
   },
 
 
@@ -229,8 +229,9 @@ HistoryStore.prototype = {
     this._allUrlStm.params.max_results = 5000;
 
     let urls = Utils.queryAsync(this._allUrlStm, "url");
+    let self = this;
     return urls.reduce(function(ids, item) {
-      ids[GUIDForUri(item.url, true)] = item.url;
+      ids[self.GUIDForUri(item.url, true)] = item.url;
       return ids;
     }, {});
   },
@@ -238,7 +239,7 @@ HistoryStore.prototype = {
   create: function HistStore_create(record) {
     // Add the url and set the GUID
     this.update(record);
-    setGUID(record.histUri, record.id);
+    this.setGUID(record.histUri, record.id);
   },
 
   remove: function HistStore_remove(record) {
@@ -332,6 +333,11 @@ HistoryTracker.prototype = {
     }
   },
 
+  _GUIDForUri: function _GUIDForUri(uri, create) {
+    // Isn't indirection fun...
+    return Engines.get("history")._store.GUIDForUri(uri, create);
+  },
+
   QueryInterface: XPCOMUtils.generateQI([
     Ci.nsINavHistoryObserver,
     Ci.nsINavHistoryObserver_MOZILLA_1_9_1_ADDITIONS,
@@ -354,7 +360,7 @@ HistoryTracker.prototype = {
     if (this.ignoreAll)
       return;
     this._log.trace("onVisit: " + uri.spec);
-    if (this.addChangedID(GUIDForUri(uri, true)))
+    if (this.addChangedID(this._GUIDForUri(uri, true)))
       this._upScore();
   },
   onDeleteVisits: function onDeleteVisits() {
@@ -365,7 +371,7 @@ HistoryTracker.prototype = {
     if (this.ignoreAll)
       return;
     this._log.trace("onBeforeDeleteURI: " + uri.spec);
-    if (this.addChangedID(GUIDForUri(uri, true)))
+    if (this.addChangedID(this._GUIDForUri(uri, true)))
       this._upScore();
   },
   onDeleteURI: function HT_onDeleteURI(uri) {

services/sync/tests/unit/test_history_store.js

@@ -132,7 +132,8 @@ function run_test() {
   }, function (next) {
 
     _("Make sure we handle invalid URLs in places databases gracefully.");
-    let query = "INSERT INTO moz_places "
+    let table = store._haveTempTables ? "moz_places_temp" : "moz_places";
+    let query = "INSERT INTO " + table + " "
       + "(url, title, rev_host, visit_count, last_visit_date) "
       + "VALUES ('invalid-uri', 'Invalid URI', '.', 1, " + TIMESTAMP3 + ")";
     let stmt = Utils.createStatement(Svc.History.DBConnection, query);

services/sync/tests/unit/test_history_tracker.js

@@ -1,10 +1,13 @@
 Cu.import("resource://gre/modules/XPCOMUtils.jsm");
+Cu.import("resource://services-sync/engines.js");
 Cu.import("resource://services-sync/engines/history.js");
 Cu.import("resource://services-sync/util.js");
 
 function run_test() {
+  Engines.register(HistoryEngine);
+  let tracker = Engines.get("history")._tracker;
+
   _("Verify we've got an empty tracker to work with.");
-  let tracker = new HistoryEngine()._tracker;
   do_check_eq([id for (id in tracker.changedIDs)].length, 0);
 
   let _counter = 0;