Merge lp:~jamesh/bindwood/update-after-remove into lp:bindwood

[James Henstridge]

When processing changes from CouchDB, ignore the update if the local item has been removed. The record in CouchDB will be removed in the second half of the synchronisation.

When pulling changes from CouchDB, we check to see if there is an existing local item for the document ID.

If the item has been deleted locally in the time since the last sync, we will find the item ID via the GUID->ID cache. This was causing errors when we tried to update the deleted local item.

This branch just ignores the update, on the assumption that the second half of the synchronisation will delete the item in CouchDB.

[Eric Casteleijn (thisfred)]

review: Approve

[Samuele Pedroni (pedronis)]

makes sense, test pass

review: Approve

=== modified file 'modules/sync.jsm'
--- modules/sync.jsm	2011-03-11 07:56:59 +0000
+++ modules/sync.jsm	2011-03-11 07:56:59 +0000
@@ -404,6 +404,19 @@
     updateItem: function(item_id, doc) {
         Log.debug("Updating local item " + item_id + " from document " +
                   doc._id);
+        // If we've got this far, then the item ID existed and was
+        // mapped to the document's GUID.  But it is possible that the
+        // item has been removed locally and we only found the item ID
+        // from the cache.  In this case, we ignore the changes and
+        // assume the deletion will be pushed back later in this sync
+        // cycle.
+        try {
+            var item_type = bookmarksService.getItemType(item_id)
+        } catch (e) {
+            Log.debug("Item has been removed locally: ignoring changes.");
+            return;
+        }
+
         if (doc._deleted) {
             // Document has been deleted in Couch: propagate to local.
             Log.debug("Removing local item.");

=== modified file 'mozmill/tests/test_sync_from_couch.js'
--- mozmill/tests/test_sync_from_couch.js	2011-03-08 15:45:54 +0000
+++ mozmill/tests/test_sync_from_couch.js	2011-03-11 07:56:59 +0000
@@ -480,3 +480,42 @@
     jum.assertEquals(bm.bookmarksService.getFolderIdForItem(child2_id),
                      bm.bookmarksService.bookmarksMenuFolder);
 };
+
+var test_update_after_remove = function() {
+    // Create a bookmark.
+    var bookmark_doc = {
+        _id: '12345',
+        record_type: 'http://www.freedesktop.org/wiki/Specifications/desktopcouch/bookmark',
+        record_type_version: 2,
+        parent_guid: 'toolbar_profile_name',
+        title: 'Bookmark title',
+        uri: LOCAL_TEST_PAGE,
+        application_annotations: {
+            Firefox: {
+                profile: 'profile_name',
+                last_modified: 1,
+            }
+        }
+    };
+    synchroniser.processRecord(bookmark_doc);
+
+    // Remove the bookmark (as the user might do).
+    var bookmark_id = synchroniser.guid_to_id(bookmark_doc._id);
+    bm.bookmarksService.removeItem(bookmark_id);
+
+    // Process another change for the record from CouchDB.
+    synchroniser.processRecord(bookmark_doc);
+
+    // The change should have been ignored, and no new bookmark should
+    // have been created.
+    bookmark_id = synchroniser.guid_to_id(bookmark_doc._id);
+    if (bookmark_id) {
+        // Check to see whether the bookmark ID really exists.
+        try {
+            bm.bookmarksService.getItemType(bookmark_id)
+        } catch (e) {
+            bookmark_id = null;
+        }
+    }
+    jum.assertNull(bookmark_id);
+};