Merge lp:~jamesh/bindwood/recording-observer into lp:bindwood

Proposed by James Henstridge
Status: Merged
Approved by: dobey
Approved revision: 35
Merged at revision: 29
Proposed branch: lp:~jamesh/bindwood/recording-observer
Merge into: lp:bindwood
Prerequisite: lp:~jamesh/bindwood/refactor-desktopcouch-access
Diff against target: 312 lines (+288/-0)
4 files modified
modules/sync.jsm (+119/-0)
mozmill/shared-modules/bookmarks.js (+1/-0)
mozmill/test-files/feed.atom (+18/-0)
mozmill/tests/test_sync_observer.js (+150/-0)
To merge this branch: bzr merge lp:~jamesh/bindwood/recording-observer
Reviewer Review Type Date Requested Status
Zachery Bir (community) Approve
Eric Casteleijn (community) Approve
Review via email: mp+49335@code.launchpad.net

Commit message

Add a bookmark observer implementation that just records the GUIDs of changed items.

Description of the change

This is part of the refactoring work on Bindwood. Rather than directly updating CouchDB as bookmarks change, I want to delay those updates so they can be done together with the main synchronisation process.

This branch includes a new bookmarks observer implementation that simply records the GUIDs of changed items as the bookmarks change. Unlike the existing Bindwood schema, I want to store parent GUID references with bookmarks, so reparenting a bookmark should imply that the bookmark has changed, rather than just its parent.

The observer depends on another object to handle the local ID -> GUID mapping, which is not included in this branch: that will come later. For now, I've just worked with a fake implementation in the tests that derives GUIDs from local IDs in a simple way.

To post a comment you must log in.
Revision history for this message
Eric Casteleijn (thisfred) :
review: Approve
Revision history for this message
Zachery Bir (urbanape) wrote :

This looks great

review: Approve
Revision history for this message
Ubuntu One Auto Pilot (otto-pilot) wrote :

The prerequisite lp:~jamesh/bindwood/refactor-desktopcouch-access has not yet been merged into lp:bindwood.

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
=== added file 'modules/sync.jsm'
--- modules/sync.jsm 1970-01-01 00:00:00 +0000
+++ modules/sync.jsm 2011-02-11 03:41:22 +0000
@@ -0,0 +1,119 @@
1/*
2 * Copyright 2009-2011 Canonical Ltd.
3 *
4 * This program is free software: you can redistribute it and/or modify it
5 * under the terms of the GNU General Public License version 3, as published
6 * by the Free Software Foundation.
7 *
8 * This program is distributed in the hope that it will be useful, but
9 * WITHOUT ANY WARRANTY; without even the implied warranties of
10 * MERCHANTABILITY, SATISFACTORY QUALITY, or FITNESS FOR A PARTICULAR
11 * PURPOSE. See the GNU General Public License for more details.
12 *
13 * You should have received a copy of the GNU General Public License along
14 * with this program. If not, see <http://www.gnu.org/licenses/>.
15 */
16
17const EXPORTED_SYMBOLS = ["BookmarksObserver"];
18
19const Cc = Components.classes;
20const Ci = Components.interfaces;
21const Cr = Components.results;
22
23var bookmarksService = Cc["@mozilla.org/browser/nav-bookmarks-service;1"]
24 .getService(Ci.nsINavBookmarksService);
25var livemarkService = Cc["@mozilla.org/browser/livemark-service;2"]
26 .getService(Ci.nsILivemarkService);
27var annotationService = Cc["@mozilla.org/browser/annotation-service;1"]
28 .getService(Ci.nsIAnnotationService);
29
30
31function BookmarksObserver(sync) {
32 this.sync = sync;
33 this.changed_guids = {}
34}
35
36BookmarksObserver.prototype = {
37 QueryInterface: function(iid) {
38 if (iid.equals(Ci.nsINavBookmarkObserver) ||
39 iid.equals(Ci.nsINavBookmarkObserver_MOZILLA_1_9_1_ADDITIONS) ||
40 iid.equals(Ci.nsISupports)) {
41 return this;
42 }
43 throw Cr.NS_ERROR_NO_INTERFACE;
44 },
45
46 should_ignore: function(item_id, folder_id) {
47 if (arguments.length < 2) {
48 folder_id = bookmarksService.getFolderIdForItem(item_id);
49 }
50 // Ignore children of Livemarks, since they are automatically
51 // created.
52 if (livemarkService.isLivemark(folder_id)) {
53 return true;
54 }
55 // XXX: Should also verify that the item is a child of a
56 // folder we care about.
57 return false;
58 },
59
60 record_change: function(item_id) {
61 var guid = this.sync.guid_from_id(item_id);
62 this.changed_guids[guid] = true;
63 },
64
65 clear_changes: function() {
66 this.changed_guids = {};
67 },
68
69 onItemAdded: function(item_id, folder_id, child_index) {
70 if (this.should_ignore(item_id, folder_id))
71 return;
72
73 // The parent folder is modified, since its child list has
74 // changed.
75 this.record_change(item_id);
76 this.record_change(folder_id);
77 },
78
79 onBeforeItemRemoved: function(item_id) {
80 folder_id = bookmarksService.getFolderIdForItem(item_id);
81 if (this.should_ignore(item_id, folder_id))
82 return;
83 this.record_change(item_id);
84 this.record_change(folder_id);
85 },
86
87 onItemChanged: function(item_id, property, is_annotation, value) {
88 if (this.should_ignore(item_id))
89 return;
90
91 // XXX: We could optimise the list of property changes we
92 // respond to like Firefox Sync here.
93
94 // Ignore icon changes.
95 if (property == "favicon")
96 return;
97
98 this.record_change(item_id);
99 },
100
101 onItemMoved: function(item_id, old_parent, old_index,
102 new_parent, new_index) {
103 if (this.should_ignore(item_id, new_parent))
104 return;
105
106 this.record_change(old_parent);
107 if (old_parent != new_parent) {
108 // The item has been moved to a different folder, rather
109 // than just to a new position within the same folder.
110 this.record_change(item_id);
111 this.record_change(new_parent);
112 }
113 },
114
115 // Currently unhandled
116 onBeginUpdateBatch: function() {},
117 onEndUpdateBatch: function() {},
118 onItemVisited: function(aBookmarkId, aVisitID, time) {},
119};
0120
=== modified file 'mozmill/shared-modules/bookmarks.js'
--- mozmill/shared-modules/bookmarks.js 2011-02-11 03:41:22 +0000
+++ mozmill/shared-modules/bookmarks.js 2011-02-11 03:41:22 +0000
@@ -42,5 +42,6 @@
42}42}
4343
44exports.bookmarksService = bookmarksService;44exports.bookmarksService = bookmarksService;
45exports.livemarkService = livemarkService;
45exports.createURI = createURI;46exports.createURI = createURI;
46exports.clearBookmarks = clearBookmarks;47exports.clearBookmarks = clearBookmarks;
4748
=== added file 'mozmill/test-files/feed.atom'
--- mozmill/test-files/feed.atom 1970-01-01 00:00:00 +0000
+++ mozmill/test-files/feed.atom 2011-02-11 03:41:22 +0000
@@ -0,0 +1,18 @@
1<?xml version="1.0" encoding="UTF-8"?>
2<feed xmlns="http://www.w3.org/2005/Atom">
3 <id>urn:uuid:518ac9fe-646b-42de-8e80-4d7372f15d00</id>
4 <title>Example feed</title>
5 <link href="http://www.example.com/"/>
6 <updated>2011-02-11T02:30:00Z</updated>
7 <author>
8 <name>Joe Smith</name>
9 </author>
10
11 <entry>
12 <id>urn:uuid:314128f7-542d-44b8-91cd-4f03ad5bddf1</id>
13 <title>Feed item</title>
14 <link href="test.html"/>
15 <updated>2011-02-11T02:30:00Z</updated>
16 <summary>Feed item description</summary>
17 </entry>
18</feed>
019
=== added file 'mozmill/tests/test_sync_observer.js'
--- mozmill/tests/test_sync_observer.js 1970-01-01 00:00:00 +0000
+++ mozmill/tests/test_sync_observer.js 2011-02-11 03:41:22 +0000
@@ -0,0 +1,150 @@
1var bm = require("../shared-modules/bookmarks");
2
3const TIMEOUT = 5000;
4
5const LOCAL_TEST_FOLDER = collector.addHttpResource('../test-files/');
6const LOCAL_TEST_PAGE = LOCAL_TEST_FOLDER + 'test.html';
7const LOCAL_TEST_FEED = LOCAL_TEST_FOLDER + 'feed.atom';
8
9var setupModule = function(module) {
10 module.controller = mozmill.getBrowserController();
11 module.jum = {}
12 module.sync = {}
13 module.fake_synchroniser = {
14 guid_from_id: function (item_id) {
15 return "guid:" + item_id;
16 }
17 }
18 module.observer = null;
19 Cu.import("resource://mozmill/modules/jum.js", module.jum);
20 Cu.import("resource://bindwood/sync.jsm", module.sync);
21};
22
23
24var setupTest = function(test) {
25 observer = new sync.BookmarksObserver(fake_synchroniser);
26 bm.bookmarksService.addObserver(observer, false);
27};
28
29var teardownTest = function(test) {
30 bm.bookmarksService.removeObserver(observer);
31 bm.clearBookmarks();
32};
33
34var testQueryInterface = function() {
35 jum.assertEquals(
36 observer.QueryInterface(Ci.nsISupports), observer);
37 jum.assertEquals(
38 observer.QueryInterface(Ci.nsINavBookmarkObserver), observer);
39 if (Ci.nsINavBookmarkObserver_MOZILLA_1_9_1_ADDITIONS) {
40 jum.assertEquals(
41 observer.QueryInterface(Ci.nsINavBookmarkObserver_MOZILLA_1_9_1_ADDITIONS), observer);
42 }
43};
44
45var assertChanged = function (item_id) {
46 var guid = fake_synchroniser.guid_from_id(item_id);
47 return observer.changed_guids[guid] != null;
48};
49
50var assertNotChanged = function (item_id) {
51 var guid = fake_synchroniser.guid_from_id(item_id);
52 return observer.changed_guids[guid] == null;
53};
54
55var testClearChanges = function() {
56 var dummy_id = 42;
57 observer.record_change(dummy_id);
58 assertChanged(dummy_id);
59 observer.clear_changes(dummy_id);
60 assertNotChanged(dummy_id);
61};
62
63var testAddBookmark = function() {
64 var bookmark_id = bm.bookmarksService.insertBookmark(
65 bm.bookmarksService.toolbarFolder,
66 bm.createURI(LOCAL_TEST_PAGE),
67 bm.bookmarksService.DEFAULT_INDEX,
68 "Bookmark title");
69 assertChanged(bookmark_id);
70 assertChanged(bm.bookmarksService.toolbarFolder);
71};
72
73var testRemoveBookmark = function() {
74 var bookmark_id = bm.bookmarksService.insertBookmark(
75 bm.bookmarksService.toolbarFolder,
76 bm.createURI(LOCAL_TEST_PAGE),
77 bm.bookmarksService.DEFAULT_INDEX,
78 "Bookmark title");
79 observer.clear_changes();
80
81 bm.bookmarksService.removeItem(bookmark_id);
82 assertChanged(bookmark_id);
83 assertChanged(bm.bookmarksService.toolbarFolder);
84};
85
86var testMoveBookmarkSameFolder = function() {
87 var folder_id = bm.bookmarksService.createFolder(
88 bm.bookmarksService.toolbarFolder,
89 "Folder", bm.bookmarksService.DEFAULT_INDEX);
90 var bookmark1_id = bm.bookmarksService.insertBookmark(
91 folder_id, bm.createURI(LOCAL_TEST_PAGE + "#one"),
92 bm.bookmarksService.DEFAULT_INDEX, "Bookmark 1");
93 var bookmark2_id = bm.bookmarksService.insertBookmark(
94 folder_id, bm.createURI(LOCAL_TEST_PAGE + "#two"),
95 bm.bookmarksService.DEFAULT_INDEX, "Bookmark 2");
96 observer.clear_changes();
97
98 // If we move a bookmark within the same folder (i.e. no parent
99 // change), then only the parent folder has changed.
100 bm.bookmarksService.moveItem(bookmark2_id, folder_id, 0);
101 assertChanged(folder_id);
102 assertNotChanged(bookmark2_id);
103};
104
105var testMoveBookmarkDifferentFolder = function() {
106 var folder1_id = bm.bookmarksService.createFolder(
107 bm.bookmarksService.toolbarFolder,
108 "Folder 1", bm.bookmarksService.DEFAULT_INDEX);
109 var folder2_id = bm.bookmarksService.createFolder(
110 bm.bookmarksService.toolbarFolder,
111 "Folder 2", bm.bookmarksService.DEFAULT_INDEX);
112 var bookmark_id = bm.bookmarksService.insertBookmark(
113 folder1_id, bm.createURI(LOCAL_TEST_PAGE),
114 bm.bookmarksService.DEFAULT_INDEX, "Bookmark");
115 observer.clear_changes();
116
117 bm.bookmarksService.moveItem(
118 bookmark_id, folder2_id, bm.bookmarksService.DEFAULT_INDEX);
119 // The item is changed because it has a new parent. The two
120 // folders have changed because their child lists have changed.
121 assertChanged(bookmark_id);
122 assertChanged(folder1_id);
123 assertChanged(folder2_id);
124};
125
126var testLivemarkChildrenIgnored = function() {
127 var livemark_id = bm.livemarkService.createLivemark(
128 bm.bookmarksService.toolbarFolder,
129 "Test Feed",
130 bm.createURI("http://www.example.com/"),
131 bm.createURI(LOCAL_TEST_FEED),
132 bm.bookmarksService.DEFAULT_INDEX);
133
134 // Wait for the livemark to load, then check that it has been
135 // loaded correctly.
136 var livemark_child_id;
137 controller.waitFor(function() {
138 livemark_child_id = bm.bookmarksService.getIdForItemAt(
139 livemark_id, 0);
140 return livemark_child_id >= 0;
141 }, TIMEOUT);
142 var livemark_child_uri = bm.bookmarksService.getBookmarkURI(
143 livemark_child_id);
144 jum.assertEquals(livemark_child_uri.spec, LOCAL_TEST_PAGE);
145
146 // Only the livemark itself is recorded as changed: the children
147 // are ignored.
148 assertChanged(livemark_id);
149 assertNotChanged(livemark_child_id);
150}

Subscribers

People subscribed via source and target branches