Merge lp:~jamesh/bindwood/sync-all into lp:bindwood

[James Henstridge]

Tie together the new synchroniser code to provide an interface for doing regular two-way synchronisation.

Pull together the recent work to perform a two way sync.

The basic algorithm is as follows:

 * pull changes from CouchDB, and create or update local items. If we're updating items, discard the change if the local item's modification time is greater than or equal to the CouchDB record.
 * Determine the list of local items that have been changed (and clear the change list in the observer in the process).
 * For each changed local item, push changes back to CouchDB. Don't actually save to couch for null-changes (as may be the case for items changed in the first step).

In addition to this, there is some first time behaviour:
 * If we've never synced with Couch, treat every record as changed. We'll keep track of this over multiple runs through a preference as current Bindwood does.
 * On start up, treat all local items as changed.

[lp:~jamesh/bindwood/sync-all]

42. By James Henstridge on 2011-02-25

Add missing onItemRemoved callback to observer.

43. By James Henstridge on 2011-02-25

Add a test demonstrating problem when importing a bookmark from CouchDB.

We create the bookmark as normal, and then annotate it with the CouchDB
record ID. Unfortunately, the bookmarks observer picks up the new
bookmark and assigns an ID too. Normally this wouldn't be a problem,
but our GUID->ID cache keeps the bogus GUID alive causing us to export
the bookmark twice.

44. By James Henstridge on 2011-02-25

When setting a GUID on an item, first check if it has already been annotated.

If there is an existing annotation, clear it from our GUID->ID cache
since it will soon be out of date.

[Eric Casteleijn (thisfred)]

I wonder if we can actually safely do this:

 * pull changes from CouchDB, and create or update local items. If we're updating items, discard the change if the local item's modification time is greater than or equal to the CouchDB record.

Say a user modifies bookmarks on their laptop while offline, and only connects that machine *after* working (and possibly modifying the same bookmark) on another machine?

In general, relying on timestamps is extremely tricky in the face of couchdb replication as well, as timestamps may actually decrease on replication.

Not sure if it's a (big) problem in this case, but in general, document revision numbers are safer to rely on as always increasing on updates (at least in the current implementation.) I hope that doesn't mean we need to store a mapping of timestamps to revision numbers somewhere, but I've had to do something similar for the funambol integration...

Anyway, approving, with the above caveat.

review: Approve

[James Henstridge (jamesh)]

One of the problems with relying on timestamps is that it opens us up to problems when there is clock skew between two systems.

If the user modifies the same bookmark on two systems at once, I think it is fine if one of the edits wins out completely, rather than e.g. picking the title from one and the url from the other.

With the new schema, we don't rely on the children property on folders to locate children (only to order them), so having one winner is acceptable in the case where two systems add a bookmark to the same folder too.

Combine this with the fact that we're using the changes feed to decide which documents to sync, and I think we should be fine.

[Manuel de la Peña (mandel)]

Tests pass and code looks ok

review: Approve

=== modified file 'modules/sync.jsm'
--- modules/sync.jsm	2011-02-21 15:04:37 +0000
+++ modules/sync.jsm	2011-02-25 07:36:30 +0000
@@ -16,6 +16,13 @@
 
 const EXPORTED_SYMBOLS = ["Synchroniser", "BookmarksObserver"];
 
+const Cc = Components.classes;
+const Ci = Components.interfaces;
+const Cr = Components.results;
+const Cu = Components.utils;
+
+Cu.import("resource://bindwood/logging.jsm");
+
 const GUID_ANNOTATION = "bindwood/uuid";
 const PARENT_ANNOTATION = "bindwood/parent";
 
@@ -26,10 +33,6 @@
 const TYPE_FEED = RECORD_PREFIX + "feed";
 const TYPE_SEPARATOR = RECORD_PREFIX + "separator";
 
-const Cc = Components.classes;
-const Ci = Components.interfaces;
-const Cr = Components.results;
-
 var bookmarksService = Cc["@mozilla.org/browser/nav-bookmarks-service;1"]
     .getService(Ci.nsINavBookmarksService);
 var livemarkService = Cc["@mozilla.org/browser/livemark-service;2"]
@@ -50,16 +53,34 @@
     this.last_seq = 0;
     this.guid_item_map = {};
     this.folders_to_reorder = {};
+    this.first_push = true;
+    this.observer = null;
 }
 
 Synchroniser.prototype = {
+    init: function() {
+        Log.debug("Initialising synchroniser.");
+        this.ensureCouchViews();
+        this.first_push = true;
+        this.observer = new BookmarksObserver(this);
+        bookmarksService.addObserver(this.observer, false);
+    },
+
+    uninit: function() {
+        Log.debug("Uninitialising synchroniser.");
+        if (this.observer) {
+            bookmarksService.removeObserver(this.observer);
+            this.observer = null;
+        }
+    },
+
     ensureCouchViews: function() {
         var design_doc_id = "_design/bindwood", doc;
 
         try {
             doc = this.couch.open(design_doc_id);
         } catch (e) {
-            dump("Problem retrieving view view:" + e + "\n");
+            Log.exception("Problem retrieving view view");
             return;
         }
         if (!doc) {
@@ -87,14 +108,26 @@
             "            req.query.profile);\n" +
             "  } catch (e) { /* ignore error */ }\n" +
             "}")
+        // XXX: should we check to see whether we've changed the
+        // document before saving it?
         try {
             this.couch.save(doc);
         } catch (e) {
-            dump("Problem saving view:" + e + "\n");
+            Log.exception("Problem saving view");
         }
     },
 
     set_guid: function(item_id, guid) {
+        // If the item has already been annotated with a GUID, clear
+        // the old GUID mapping from our cache.
+        try {
+            var old_guid = annotationService.getItemAnnotation(
+                item_id, GUID_ANNOTATION);
+            delete this.guid_item_map[old_guid];
+        } catch (e) {
+            // Ignore errors.
+        }
+
         if (!guid) {
             guid = uuidService.generateUUID().toString();
         }
@@ -108,7 +141,9 @@
 
     guid_from_id: function(item_id) {
         // Handle special items.
-        if (item_id == bookmarksService.toolbarFolder)
+        if (item_id == bookmarksService.placesRoot)
+            return "root_" + this.profile;
+        else if (item_id == bookmarksService.toolbarFolder)
             return "toolbar_" + this.profile;
         else if (item_id == bookmarksService.bookmarksMenuFolder)
             return "menu_" + this.profile;
@@ -132,7 +167,9 @@
 
     guid_to_id: function(guid) {
         // Handle special items.
-        if (guid == "toolbar_" + this.profile)
+        if (guid == "root_" + this.profile)
+            return bookmarksService.placesRoot;
+        else if (guid == "toolbar_" + this.profile)
             return  bookmarksService.toolbarFolder;
         else if (guid == "menu_" + this.profile)
             return  bookmarksService.bookmarksMenuFolder;
@@ -159,15 +196,71 @@
         return null;
     },
 
+    _get_folder_root: function(folder_id) {
+        var query = historyService.getNewQuery();
+        var options = historyService.getNewQueryOptions();
+        query.setFolders([folder_id], 1);
+        return historyService.executeQuery(query, options).root;
+    },
+
+    get_folder_children: function(parent_id) {
+        if (parent_id == bookmarksService.placesRoot) {
+            // Special case the root folder, returning the three trees
+            // we're interested in.
+            return [
+                bookmarksService.toolbarFolder,
+                bookmarksService.bookmarksMenuFolder,
+                bookmarksService.unfiledBookmarksFolder];
+        }
+        var query = historyService.getNewQuery();
+        var options = historyService.getNewQueryOptions();
+        query.setFolders([parent_id], 1);
+        var root = this._get_folder_root(parent_id);
+
+        var children = [];
+        root.containerOpen = true;
+        for (var i = 0; i < root.childCount; i++) {
+            var child = root.getChild(i);
+            children.push(child.itemId);
+        }
+        return children;
+    },
+
+    get_all_bookmarks: function() {
+        var bookmarks = {}
+        bookmarks[this.guid_from_id(bookmarksService.placesRoot)] = true;
+        var sync = this;
+        var add_bookmarks = function(node) {
+            bookmarks[sync.guid_from_id(node.itemId)] = true;
+
+            if (node.type != node.RESULT_TYPE_FOLDER ||
+                livemarkService.isLivemark(node.itemId))
+                return;
+            node.QueryInterface(Ci.nsINavHistoryQueryResultNode);
+            node.containerOpen = true;
+            for (var i = 0; i < node.childCount; i++) {
+                add_bookmarks(node.getChild(i));
+            }
+        };
+
+        add_bookmarks(this._get_folder_root(
+            bookmarksService.toolbarFolder));
+        add_bookmarks(this._get_folder_root(
+            bookmarksService.bookmarksMenuFolder));
+        add_bookmarks(this._get_folder_root(
+            bookmarksService.unfiledBookmarksFolder));
+
+        return bookmarks;
+    },
+
     sync: function() {
-        //this.recordChangedIds();
+        Log.debug("Starting two-way synchronisation.");
         this.pullChanges();
-        //this.pushChanges();
+        this.pushChanges();
         //this.cleanup();
     },
 
     pullChanges: function() {
-        var new_last_seq;
         var info = this.couch.info();
         if (this.last_seq <= info.purge_seq) {
             // We haven't pulled any records before, or the database
@@ -202,10 +295,15 @@
                 this.processRecord(row.doc);
             }
         }
+        this.reorder_children();
     },
 
     processRecord: function (doc) {
         var item_id = this.guid_to_id(doc._id);
+        if (item_id == bookmarksService.placesRoot) {
+            Log.debug("Ignoring change to places root.\n");
+            return;
+        }
         // XXX: We should perform duplicate detection here if item_id
         // is null.
         if (item_id != null) {
@@ -266,12 +364,23 @@
     },
 
     updateItem: function(item_id, doc) {
+        Log.debug("Updating local item " + item_id + " from document " +
+                  doc._id);
         if (doc._deleted) {
             // Document has been deleted in Couch: propagate to local.
+            Log.debug("Removing local item.");
             bookmarksService.removeItem(item_id);
             return;
         }
 
+        // If the local item is newer than the version in Couch,
+        // ignore.
+        if (bookmarksService.getItemLastModified(item_id) >=
+            doc.application_annotations.Firefox.last_modified) {
+            Log.debug("Local item is newer than CouchDB version.");
+            return;
+        }
+
         var parent_id = this.guid_to_id(doc.parent_guid);
         if (parent_id == null) {
             this.set_orphan(item_id, doc.parent_guid);
@@ -333,21 +442,6 @@
         }
     },
 
-    get_folder_children: function(parent_id) {
-        var query = historyService.getNewQuery();
-        var options = historyService.getNewQueryOptions();
-        query.setFolders([parent_id], 1);
-        var root = historyService.executeQuery(query, options).root;
-
-        var children = [];
-        root.containerOpen = true;
-        for (var i = 0; i < root.childCount; i++) {
-            var child = root.getChild(i);
-            children.push(child.itemId);
-        }
-        return children;
-    },
-
     reorder_children: function() {
         for (var parent_guid in this.folders_to_reorder) {
             var parent_id = this.guid_to_id(parent_guid);
@@ -377,8 +471,23 @@
         this.folders_to_reorder = {}
     },
 
+    pushChanges: function() {
+        Log.debug("Pushing changes.  first_push = " + this.first_push);
+        var changed_guids = this.observer.clear_changes();
+        if (this.first_push) {
+            // First time: process all bookmarks.
+            changed_guids = this.get_all_bookmarks();
+            this.first_push = false;
+        }
+        for (var item_guid in changed_guids) {
+            this.exportItem(item_guid);
+        }
+    },
+
     exportItem: function(item_guid) {
         var item_id = this.guid_to_id(item_guid);
+        Log.debug("Exporting item " + item_guid +
+                  " (local ID " + item_id + ")");
         var item_type = null, deleted = false;
         if (item_id) {
             // Get the item type, which also checks whether the item exists.
@@ -457,11 +566,13 @@
             _setattr(doc, "record_type", TYPE_SEPARATOR);
             break;
         default:
-            dump("Can not handle item " + item_id + " of type " +
-                 item_type + "\n");
+            Log.error("Can not handle item " + item_id + " of type " +
+                      item_type);
             return;
         }
         if (changed) {
+            doc.application_annotations.Firefox.last_modified = (
+                bookmarksService.getItemLastModified(item_id));
             this.couch.save(doc);
         }
     },
@@ -503,7 +614,9 @@
     },
 
     clear_changes: function() {
+        var changes = this.changed_guids;
         this.changed_guids = {};
+        return changes;
     },
 
     onItemAdded: function(item_id, folder_id, child_index) {
@@ -555,5 +668,6 @@
     // Currently unhandled
     onBeginUpdateBatch: function() {},
     onEndUpdateBatch: function() {},
+    onItemRemoved: function(item_id, folder_id, index) {},
     onItemVisited: function(aBookmarkId, aVisitID, time) {},
 };

=== added file 'mozmill/tests/test_sync_all.js'
--- mozmill/tests/test_sync_all.js	1970-01-01 00:00:00 +0000
+++ mozmill/tests/test_sync_all.js	2011-02-25 07:36:30 +0000
@@ -0,0 +1,195 @@
+var bm = require("../shared-modules/bookmarks");
+
+const TIMEOUT = 5000;
+
+const LOCAL_TEST_FOLDER = collector.addHttpResource('../test-files/');
+const LOCAL_TEST_PAGE = LOCAL_TEST_FOLDER + 'test.html';
+const LOCAL_TEST_FEED = LOCAL_TEST_FOLDER + 'feed.atom';
+
+var setupModule = function(module) {
+    module.controller = mozmill.getBrowserController();
+    module.jum = {};
+    module.desktopcouch = {};
+    module.sync = {};
+    Cu.import("resource://mozmill/modules/jum.js", module.jum);
+    Cu.import("resource://bindwood/desktopcouch.jsm", module.desktopcouch);
+    Cu.import("resource://bindwood/sync.jsm", module.sync);
+    bm.clearBookmarks();
+    module.couch = null;
+    module.synchroniser = null;
+};
+
+
+var setupTest = function(test) {
+    var done = false;
+    desktopcouch.connect_desktopcouch("test_bookmarks", function(db) {
+            couch = db;
+            done = true;
+        }, function (message) {});
+    controller.waitFor(
+        function() { return done; }, "Could not connect to CouchDB", TIMEOUT);
+    jum.assertNotEquals(couch, null);
+
+    try {
+        couch.createDb();
+    } catch (e) {
+        if (e.error != 'file_exists')
+            throw(e);
+    }
+};
+
+
+var teardownTest = function(test) {
+    if (synchroniser) {
+        synchroniser.uninit();
+        synchroniser = null;
+    }
+    bm.clearBookmarks();
+    couch.deleteDb();
+};
+
+
+var test_sync_local = function() {
+    var item_id = bm.bookmarksService.insertBookmark(
+        bm.bookmarksService.toolbarFolder, bm.createURI(LOCAL_TEST_PAGE),
+        bm.bookmarksService.DEFAULT_INDEX, "Bookmark title");
+
+    // Perform synchronisation.
+    synchroniser = new sync.Synchroniser(couch, "profile_name");
+    synchroniser.init();
+    synchroniser.sync();
+
+    // The root and toolbar have been synchronised.
+    var root_guid = synchroniser.guid_from_id(
+        bm.bookmarksService.placesRoot);
+    var toolbar_guid = synchroniser.guid_from_id(
+        bm.bookmarksService.toolbarFolder)
+    var doc = couch.open(root_guid);
+    jum.assertNotNull(doc);
+    var old_root_rev = doc._rev;
+
+    doc = couch.open(toolbar_guid);
+    jum.assertNotNull(doc);
+    var old_toolbar_rev = doc._rev;
+
+    // Our bookmark has also been synchronised.
+    var item_guid = synchroniser.guid_from_id(item_id);
+    doc = couch.open(item_guid);
+    jum.assertNotNull(doc);
+    jum.assertEquals(doc.title, "Bookmark title");
+
+    // Making a local change to the bookmark.
+    bm.bookmarksService.setItemTitle(item_id, "New title");
+    synchroniser.sync();
+
+    // The parent folders should be unchanged.
+    doc = couch.open(root_guid);
+    jum.assertEquals(doc._rev, old_root_rev);
+    doc = couch.open(toolbar_guid);
+    jum.assertEquals(doc._rev, old_toolbar_rev);
+
+    // Our bookmark should have been updated.
+    doc = couch.open(item_guid);
+    jum.assertEquals(doc.title, "New title");
+    var old_item_rev = doc._rev;
+
+    // Performing another synchronisation should leave the item unchanged.
+    synchroniser.sync();
+    doc = couch.open(item_guid);
+    jum.assertEquals(doc._rev, old_item_rev);
+};
+
+var test_sync_remote = function() {
+    var item_guid = '12345';
+    var doc = {
+        _id: item_guid,
+        record_type: 'http://www.freedesktop.org/wiki/Specifications/desktopcouch/bookmark',
+        parent_guid: 'toolbar_profile_name',
+        title: 'Bookmark title',
+        uri: LOCAL_TEST_PAGE,
+        application_annotations: {
+            Firefox: {
+                profile: 'profile_name',
+                last_modified: 1,
+            }
+        }
+    };
+    couch.save(doc);
+    var old_item_rev = doc._rev;
+
+    // Perform synchronisation.
+    synchroniser = new sync.Synchroniser(couch, "profile_name");
+    synchroniser.init();
+    synchroniser.sync();
+
+    // Item has been created locally.
+    var item_id = synchroniser.guid_to_id(item_guid);
+    jum.assertEquals(
+        bm.bookmarksService.getItemTitle(item_id), 'Bookmark title');
+
+    // CouchDB record has not been modified.
+    doc = couch.open(item_guid);
+    jum.assertEquals(doc._rev, old_item_rev);
+
+    // Modify the item and save it back.
+    doc.title = 'New title';
+    doc.application_annotations.Firefox.last_modified = (
+        bm.bookmarksService.getItemLastModified(item_id) + 1);
+    couch.save(doc);
+    old_item_rev = doc._rev;
+
+    // After synchronisation, the local bookmark has changed.
+    synchroniser.sync();
+    jum.assertEquals(
+        bm.bookmarksService.getItemTitle(item_id), 'New title');
+
+    // Again, the CouchDB record hasn't been modified by the sync.
+    doc = couch.open(item_guid);
+    jum.assertEquals(doc._rev, old_item_rev);
+};
+
+function get_bookmark_ids(profile) {
+    var result = couch.view("bindwood/bookmarks", {key: profile});
+    return [row.value for each (row in result.rows)];
+}
+
+var test_sync_remote_add_no_duplicate = function() {
+    // Perform synchronisation.
+    synchroniser = new sync.Synchroniser(couch, "profile_name");
+    synchroniser.init();
+    synchroniser.sync();
+
+    var expected_bookmarks = [
+        synchroniser.guid_from_id(bm.bookmarksService.placesRoot),
+        synchroniser.guid_from_id(bm.bookmarksService.toolbarFolder),
+        synchroniser.guid_from_id(bm.bookmarksService.bookmarksMenuFolder),
+        synchroniser.guid_from_id(bm.bookmarksService.unfiledBookmarksFolder),
+    ];
+    var actual_bookmarks = get_bookmark_ids("profile_name");
+    jum.assertEquals(expected_bookmarks.sort().join("\n"),
+                     actual_bookmarks.sort().join("\n"))
+
+    var item_guid = '12345';
+    var doc = {
+        _id: item_guid,
+        record_type: 'http://www.freedesktop.org/wiki/Specifications/desktopcouch/bookmark',
+        parent_guid: 'toolbar_profile_name',
+        title: 'Bookmark title',
+        uri: LOCAL_TEST_PAGE,
+        application_annotations: {
+            Firefox: {
+                profile: 'profile_name',
+                last_modified: 1,
+            }
+        }
+    };
+    couch.save(doc);
+
+    // After synchronisation, we expect to only find the one added
+    // bookmark.
+    synchroniser.sync();
+    expected_bookmarks.push(item_guid);
+    var actual_bookmarks = get_bookmark_ids("profile_name");
+    jum.assertEquals(expected_bookmarks.sort().join("\n"),
+                     actual_bookmarks.sort().join("\n"))
+};

=== modified file 'mozmill/tests/test_sync_from_couch.js'
--- mozmill/tests/test_sync_from_couch.js	2011-02-22 11:02:10 +0000
+++ mozmill/tests/test_sync_from_couch.js	2011-02-25 07:36:30 +0000
@@ -32,6 +32,12 @@
         parent_guid: 'toolbar_profile_name',
         title: 'Bookmark title',
         uri: LOCAL_TEST_PAGE,
+        application_annotations: {
+            Firefox: {
+                profile: 'profile_name',
+                last_modified: 1,
+            }
+        }
     });
     var bookmark_id = synchroniser.guid_to_id('12345');
     jum.assertEquals(bm.bookmarksService.getItemType(bookmark_id),
@@ -51,13 +57,21 @@
         parent_guid: 'toolbar_profile_name',
         title: 'Bookmark title',
         uri: LOCAL_TEST_PAGE,
+        application_annotations: {
+            Firefox: {
+                profile: 'profile_name',
+                last_modified: 1,
+            }
+        }
     };
     synchroniser.processRecord(doc);
+    var bookmark_id = synchroniser.guid_to_id('12345');
     doc.title = 'New title';
     doc.uri = LOCAL_TEST_PAGE + "#updated";
+    doc.application_annotations.Firefox.last_modified = (
+        bm.bookmarksService.getItemLastModified(bookmark_id) + 1);
     synchroniser.processRecord(doc);
 
-    var bookmark_id = synchroniser.guid_to_id('12345');
     jum.assertEquals(bm.bookmarksService.getItemTitle(bookmark_id),
                      'New title');
     jum.assertEquals(bm.bookmarksService.getBookmarkURI(bookmark_id).spec,
@@ -71,6 +85,12 @@
         parent_guid: 'toolbar_profile_name',
         title: 'Folder',
         children: [],
+        application_annotations: {
+            Firefox: {
+                profile: 'profile_name',
+                last_modified: 1,
+            }
+        }
     });
     var bookmark_id = synchroniser.guid_to_id('12345');
     jum.assertEquals(bm.bookmarksService.getItemType(bookmark_id),
@@ -90,12 +110,20 @@
         parent_guid: 'toolbar_profile_name',
         title: 'Folder',
         children: [],
+        application_annotations: {
+            Firefox: {
+                profile: 'profile_name',
+                last_modified: 1,
+            }
+        }
     };
     synchroniser.processRecord(doc);
+    var bookmark_id = synchroniser.guid_to_id('12345');
     doc.title = 'Updated folder';
+    doc.application_annotations.Firefox.last_modified = (
+        bm.bookmarksService.getItemLastModified(bookmark_id) + 1);
     synchroniser.processRecord(doc);
 
-    var bookmark_id = synchroniser.guid_to_id('12345');
     jum.assertEquals(bm.bookmarksService.getItemTitle(bookmark_id),
                      'Updated folder');
     // Reordering of folder's children has been scheduled.
@@ -110,6 +138,12 @@
         title: 'Livemark',
         site_uri: "http://www.example.com/",
         feed_uri: LOCAL_TEST_FEED,
+        application_annotations: {
+            Firefox: {
+                profile: 'profile_name',
+                last_modified: 1,
+            }
+        }
     });
     var bookmark_id = synchroniser.guid_to_id('12345');
     jum.assertTrue(bm.livemarkService.isLivemark(bookmark_id))
@@ -131,13 +165,21 @@
         title: 'Livemark',
         site_uri: "http://www.example.com/",
         feed_uri: LOCAL_TEST_FEED,
+        application_annotations: {
+            Firefox: {
+                profile: 'profile_name',
+                last_modified: 1,
+            }
+        }
     };
     synchroniser.processRecord(doc);
+    var bookmark_id = synchroniser.guid_to_id('12345');
     doc.title = 'New title';
     doc.feed_uri = LOCAL_TEST_FEED + "#updated";
+    doc.application_annotations.Firefox.last_modified = (
+        bm.bookmarksService.getItemLastModified(bookmark_id) + 1);
     synchroniser.processRecord(doc);
 
-    var bookmark_id = synchroniser.guid_to_id('12345');
     jum.assertEquals(bm.bookmarksService.getItemTitle(bookmark_id),
                      'New title');
     jum.assertEquals(bm.livemarkService.getFeedURI(bookmark_id).spec,
@@ -149,6 +191,12 @@
         _id: '12345',
         record_type: 'http://www.freedesktop.org/wiki/Specifications/desktopcouch/separator',
         parent_guid: 'toolbar_profile_name',
+        application_annotations: {
+            Firefox: {
+                profile: 'profile_name',
+                last_modified: 1,
+            }
+        }
     });
     var bookmark_id = synchroniser.guid_to_id('12345');
     jum.assertEquals(bm.bookmarksService.getItemType(bookmark_id),
@@ -162,12 +210,21 @@
         _id: '12345',
         record_type: 'http://www.freedesktop.org/wiki/Specifications/desktopcouch/separator',
         parent_guid: 'toolbar_profile_name',
+        application_annotations: {
+            Firefox: {
+                profile: 'profile_name',
+                last_modified: 1,
+            }
+        }
     };
     synchroniser.processRecord(doc);
+    var bookmark_id = synchroniser.guid_to_id('12345');
     // No properties to change on the separator, but verify that the
     // update code path runs anyway.
+    doc.application_annotations.Firefox.last_modified = (
+        bm.bookmarksService.getItemLastModified(bookmark_id) + 1);
     synchroniser.processRecord(doc);
-    var bookmark_id = synchroniser.guid_to_id('12345');
+
     jum.assertEquals(bm.bookmarksService.getItemType(bookmark_id),
                      bm.bookmarksService.TYPE_SEPARATOR);
 };
@@ -179,6 +236,12 @@
         parent_guid: 'toolbar_profile_name',
         title: 'Bookmark title',
         uri: LOCAL_TEST_PAGE,
+        application_annotations: {
+            Firefox: {
+                profile: 'profile_name',
+                last_modified: 1,
+            }
+        }
     };
     synchroniser.processRecord(doc);
     var bookmark_id = synchroniser.guid_to_id('12345');
@@ -187,7 +250,10 @@
 
     // Move to a new parent folder.
     doc.parent_guid = "menu_profile_name";
+    doc.application_annotations.Firefox.last_modified = (
+        bm.bookmarksService.getItemLastModified(bookmark_id) + 1);
     synchroniser.processRecord(doc);
+
     jum.assertEquals(bm.bookmarksService.getFolderIdForItem(bookmark_id),
                      bm.bookmarksService.bookmarksMenuFolder);
 };
@@ -199,6 +265,12 @@
         parent_guid: 'toolbar_profile_name',
         title: 'Folder',
         children: ['67890'],
+        application_annotations: {
+            Firefox: {
+                profile: 'profile_name',
+                last_modified: 1,
+            }
+        }
     });
     var parent_id = synchroniser.guid_to_id('12345');
     jum.assertEquals(bm.bookmarksService.getFolderIdForItem(parent_id),
@@ -212,6 +284,12 @@
         parent_guid: '12345',
         title: 'Bookmark title',
         uri: LOCAL_TEST_PAGE,
+        application_annotations: {
+            Firefox: {
+                profile: 'profile_name',
+                last_modified: 1,
+            }
+        }
     });
     var child_id = synchroniser.guid_to_id('67890');
     jum.assertEquals(bm.bookmarksService.getFolderIdForItem(child_id),
@@ -225,6 +303,12 @@
         parent_guid: '12345',
         title: 'Bookmark title',
         uri: LOCAL_TEST_PAGE,
+        application_annotations: {
+            Firefox: {
+                profile: 'profile_name',
+                last_modified: 1,
+            }
+        }
     });
     var child_id = synchroniser.guid_to_id('67890');
     // The bookmark is temporarily stashed in unfiled bookmarks.
@@ -237,6 +321,12 @@
         parent_guid: 'toolbar_profile_name',
         title: 'Folder',
         children: ['67890'],
+        application_annotations: {
+            Firefox: {
+                profile: 'profile_name',
+                last_modified: 1,
+            }
+        }
     });
     var parent_id = synchroniser.guid_to_id('12345');
     jum.assertEquals(bm.bookmarksService.getFolderIdForItem(parent_id),
@@ -250,12 +340,18 @@
 
 var test_reorder_children = function() {
     synchroniser.processRecord({
-            _id: 'parent',
-            record_type: 'http://www.freedesktop.org/wiki/Specifications/desktopcouch/folder',
-            parent_guid: 'toolbar_profile_name',
-            title: 'Folder',
-            children: ['child1', 'child2', 'child3', 'missing_child'],
-        });
+        _id: 'parent',
+        record_type: 'http://www.freedesktop.org/wiki/Specifications/desktopcouch/folder',
+        parent_guid: 'toolbar_profile_name',
+        title: 'Folder',
+        children: ['child1', 'child2', 'child3', 'missing_child'],
+        application_annotations: {
+            Firefox: {
+                profile: 'profile_name',
+                last_modified: 1,
+            }
+        }
+    });
     var parent_id = synchroniser.guid_to_id('parent');
     // Add an untracked bookmark to the folder.
     var untracked_child = bm.bookmarksService.insertBookmark(
@@ -265,26 +361,44 @@
 
     // Add expected children.
     synchroniser.processRecord({
-            _id: 'child1',
-            record_type: 'http://www.freedesktop.org/wiki/Specifications/desktopcouch/bookmark',
-            parent_guid: 'parent',
-            title: 'Child 1',
-            uri: LOCAL_TEST_PAGE + "#child1",
-        });
-    synchroniser.processRecord({
-            _id: 'child3',
-            record_type: 'http://www.freedesktop.org/wiki/Specifications/desktopcouch/bookmark',
-            parent_guid: 'parent',
-            title: 'Child 3',
-            uri: LOCAL_TEST_PAGE + "#child1",
-        });
-    synchroniser.processRecord({
-            _id: 'child2',
-            record_type: 'http://www.freedesktop.org/wiki/Specifications/desktopcouch/bookmark',
-            parent_guid: 'parent',
-            title: 'Child 2',
-            uri: LOCAL_TEST_PAGE + "#child1",
-        });
+        _id: 'child1',
+        record_type: 'http://www.freedesktop.org/wiki/Specifications/desktopcouch/bookmark',
+        parent_guid: 'parent',
+        title: 'Child 1',
+        uri: LOCAL_TEST_PAGE + "#child1",
+        application_annotations: {
+            Firefox: {
+                profile: 'profile_name',
+                last_modified: 1,
+            }
+        }
+    });
+    synchroniser.processRecord({
+        _id: 'child3',
+        record_type: 'http://www.freedesktop.org/wiki/Specifications/desktopcouch/bookmark',
+        parent_guid: 'parent',
+        title: 'Child 3',
+        uri: LOCAL_TEST_PAGE + "#child1",
+        application_annotations: {
+            Firefox: {
+                profile: 'profile_name',
+                last_modified: 1,
+            }
+        }
+    });
+    synchroniser.processRecord({
+        _id: 'child2',
+        record_type: 'http://www.freedesktop.org/wiki/Specifications/desktopcouch/bookmark',
+        parent_guid: 'parent',
+        title: 'Child 2',
+        uri: LOCAL_TEST_PAGE + "#child1",
+        application_annotations: {
+            Firefox: {
+                profile: 'profile_name',
+                last_modified: 1,
+            }
+        }
+    });
 
     // The parent has been scheduled for reordering, so trigger
     // reordering.

=== modified file 'mozmill/tests/test_sync_guid_map.js'
--- mozmill/tests/test_sync_guid_map.js	2011-02-15 09:37:54 +0000
+++ mozmill/tests/test_sync_guid_map.js	2011-02-25 07:36:30 +0000
@@ -25,12 +25,15 @@
 };
 
 var test_special_ids = function() {
+    var placesRoot = bm.bookmarksService.placesRoot;
     var toolbarFolder = bm.bookmarksService.toolbarFolder;
     var bookmarksMenuFolder = bm.bookmarksService.bookmarksMenuFolder;
     var unfiledBookmarksFolder = bm.bookmarksService.unfiledBookmarksFolder;
 
     // Special folders get mapped to fixed GUIDs
     jum.assertEquals(
+        synchroniser.guid_from_id(placesRoot), "root_profile_name");
+    jum.assertEquals(
         synchroniser.guid_from_id(toolbarFolder), "toolbar_profile_name");
     jum.assertEquals(
         synchroniser.guid_from_id(bookmarksMenuFolder), "menu_profile_name");
@@ -40,6 +43,8 @@
 
     // Special fixed GUIDs get mapped to special folders
     jum.assertEquals(
+        synchroniser.guid_to_id("root_profile_name"), placesRoot);
+    jum.assertEquals(
         synchroniser.guid_to_id("toolbar_profile_name"), toolbarFolder);
     jum.assertEquals(
         synchroniser.guid_to_id("menu_profile_name"), bookmarksMenuFolder);
@@ -79,3 +84,34 @@
     // Non-existent GUIDs result in a null result.
     jum.assertEquals(synchroniser.guid_to_id("non-existent"), null);
 };
+
+var test_get_all_bookmarks = function() {
+    var first_id = bm.bookmarksService.insertBookmark(
+        bm.bookmarksService.toolbarFolder,
+        bm.createURI(LOCAL_TEST_PAGE + "#one"),
+        bm.bookmarksService.DEFAULT_INDEX,
+        "Bookmark 1");
+    var second_id = bm.bookmarksService.createFolder(
+        bm.bookmarksService.bookmarksMenuFolder,
+        "Folder", bm.bookmarksService.DEFAULT_INDEX);
+    var third_id = bm.bookmarksService.insertBookmark(
+        second_id,
+        bm.createURI(LOCAL_TEST_PAGE + "#three"),
+        bm.bookmarksService.DEFAULT_INDEX,
+        "Bookmark 3");
+    var fourth_id = bm.bookmarksService.insertBookmark(
+        bm.bookmarksService.unfiledBookmarksFolder,
+        bm.createURI(LOCAL_TEST_PAGE + "#four"),
+        bm.bookmarksService.DEFAULT_INDEX,
+        "Bookmark 4");
+
+    var bookmarks = synchroniser.get_all_bookmarks();
+    jum.assertNotUndefined(bookmarks["root_profile_name"]);
+    jum.assertNotUndefined(bookmarks["toolbar_profile_name"]);
+    jum.assertNotUndefined(bookmarks[synchroniser.guid_from_id(first_id)]);
+    jum.assertNotUndefined(bookmarks["menu_profile_name"]);
+    jum.assertNotUndefined(bookmarks[synchroniser.guid_from_id(second_id)]);
+    jum.assertNotUndefined(bookmarks[synchroniser.guid_from_id(third_id)]);
+    jum.assertNotUndefined(bookmarks["unfiled_profile_name"]);
+    jum.assertNotUndefined(bookmarks[synchroniser.guid_from_id(fourth_id)]);
+};

=== modified file 'mozmill/tests/test_sync_to_couch.js'
--- mozmill/tests/test_sync_to_couch.js	2011-02-22 11:09:37 +0000
+++ mozmill/tests/test_sync_to_couch.js	2011-02-25 07:36:30 +0000
@@ -183,3 +183,64 @@
     var doc = couch.open(item_guid);
     jum.assertNull(doc);
 };
+
+var test_export_root = function() {
+    var root_guid = synchroniser.guid_from_id(bm.bookmarksService.placesRoot);
+    synchroniser.exportItem(root_guid);
+
+    var doc = couch.open(root_guid);
+    jum.assertEquals(doc.record_type, "http://www.freedesktop.org/wiki/" +
+                     "Specifications/desktopcouch/folder");
+    jum.assertUndefined(doc.parent_guid);
+    jum.assertEquals(doc.children.length, 3);
+    jum.assertEquals(doc.children[0], synchroniser.guid_from_id(
+        bm.bookmarksService.toolbarFolder));
+    jum.assertEquals(doc.children[1], synchroniser.guid_from_id(
+        bm.bookmarksService.bookmarksMenuFolder));
+    jum.assertEquals(doc.children[2], synchroniser.guid_from_id(
+        bm.bookmarksService.unfiledBookmarksFolder));
+};
+
+var test_push_changes = function() {
+    var item_id = bm.bookmarksService.insertBookmark(
+        bm.bookmarksService.toolbarFolder, bm.createURI(LOCAL_TEST_PAGE),
+        bm.bookmarksService.DEFAULT_INDEX, "Bookmark title");
+
+    // Trap calls to exportItem, to see what bookmarks are exported.
+    var bookmarks = []
+    synchroniser.exportItem = function(item_id) {
+        bookmarks.push(item_id);
+    }
+
+    // Fake observer that returns a fixed set of changes.
+    synchroniser.observer = {
+        clear_changes: function() {
+            var changes = {}
+            changes[synchroniser.guid_from_id(
+                bm.bookmarksService.toolbarFolder)] = true;
+            changes[synchroniser.guid_from_id(item_id)] = true;
+            return changes;
+        }
+    };
+
+    // First call to pushItems() will export everything.
+    synchroniser.pushChanges();
+    bookmarks.sort();
+    jum.assertEquals(bookmarks.length, 5);
+    // This relies on the fact that the genrated IDs '{...}' sort
+    // after the special ones.  If we change the generated IDs, we'll
+    // need to change how this test works.
+    jum.assertEquals(bookmarks[0], "menu_profile_name");
+    jum.assertEquals(bookmarks[1], "root_profile_name");
+    jum.assertEquals(bookmarks[2], "toolbar_profile_name");
+    jum.assertEquals(bookmarks[3], "unfiled_profile_name");
+    jum.assertEquals(bookmarks[4], synchroniser.guid_from_id(item_id));
+
+    // Subsequent calls get the ID list from the bookmarks observer.
+    bookmarks = [];
+    synchroniser.pushChanges();
+    bookmarks.sort()
+    jum.assertEquals(bookmarks.length, 2);
+    jum.assertEquals(bookmarks[0], "toolbar_profile_name");
+    jum.assertEquals(bookmarks[1], synchroniser.guid_from_id(item_id));
+};