Bug 745410 - Remove microsummary support from Sync. r?kitcambridge draft
authorEdouard Oger <eoger@fastmail.com>
Fri, 10 Mar 2017 15:03:07 -0500
changeset 496921 ad004522bd79b2ecd82c0b7d2d0a2ef3c91b9d57
parent 496768 a8d497b09753c91783b68c5805c64f34a2f39629
child 548752 ce671a1e9f279e543e7a4d2b4e07536158a06ca4
push id48745
push userbmo:eoger@fastmail.com
push dateFri, 10 Mar 2017 22:14:03 +0000
reviewerskitcambridge
bugs745410
milestone55.0a1
Bug 745410 - Remove microsummary support from Sync. r?kitcambridge MozReview-Commit-ID: BdBgxHeHndj
services/sync/modules/engines/bookmarks.js
services/sync/tests/unit/test_bookmark_legacy_microsummaries_support.js
services/sync/tests/unit/xpcshell.ini
toolkit/components/places/PlacesSyncUtils.jsm
--- a/services/sync/modules/engines/bookmarks.js
+++ b/services/sync/modules/engines/bookmarks.js
@@ -69,17 +69,16 @@ function isSyncedRootNode(node) {
          node.root == "toolbarFolder" ||
          node.root == "mobileFolder";
 }
 
 // Returns the constructor for a bookmark record type.
 function getTypeObject(type) {
   switch (type) {
     case "bookmark":
-    case "microsummary":
       return Bookmark;
     case "query":
       return BookmarkQuery;
     case "folder":
       return BookmarkFolder;
     case "livemark":
       return Livemark;
     case "separator":
@@ -370,17 +369,16 @@ BookmarksEngine.prototype = {
         // hack should get them to dupe correctly.
         if (item.queryId) {
           key = "q" + item.queryId;
           altKey = "b" + item.bmkUri + ":" + (item.title || "");
           break;
         }
         // No queryID? Fall through to the regular bookmark case.
       case "bookmark":
-      case "microsummary":
         key = "b" + item.bmkUri + ":" + (item.title || "");
         break;
       case "folder":
       case "livemark":
         key = "f" + (item.title || "");
         break;
       case "separator":
         key = "s" + item.pos;
deleted file mode 100644
--- a/services/sync/tests/unit/test_bookmark_legacy_microsummaries_support.js
+++ /dev/null
@@ -1,99 +0,0 @@
-/* Any copyright is dedicated to the Public Domain.
-   http://creativecommons.org/publicdomain/zero/1.0/ */
-
-// Tests that Sync can correctly handle a legacy microsummary record
-Cu.import("resource://gre/modules/Services.jsm");
-Cu.import("resource://gre/modules/NetUtil.jsm");
-Cu.import("resource://gre/modules/PlacesUtils.jsm");
-
-Cu.import("resource://gre/modules/Log.jsm");
-Cu.import("resource://services-sync/engines.js");
-Cu.import("resource://services-sync/engines/bookmarks.js");
-Cu.import("resource://services-sync/record.js");
-Cu.import("resource://services-sync/service.js");
-Cu.import("resource://services-sync/util.js");
-
-const GENERATORURI_ANNO = "microsummary/generatorURI";
-const STATICTITLE_ANNO = "bookmarks/staticTitle";
-
-const TEST_URL = "http://micsum.mozilla.org/";
-const TEST_TITLE = "A microsummarized bookmark"
-const GENERATOR_URL = "http://generate.micsum/"
-const STATIC_TITLE = "Static title"
-
-function newMicrosummary(url, title) {
-  let id = PlacesUtils.bookmarks.insertBookmark(
-    PlacesUtils.unfiledBookmarksFolderId, NetUtil.newURI(url),
-    PlacesUtils.bookmarks.DEFAULT_INDEX, title
-  );
-  PlacesUtils.annotations.setItemAnnotation(id, GENERATORURI_ANNO,
-                                            GENERATOR_URL, 0,
-                                            PlacesUtils.annotations.EXPIRE_NEVER);
-  PlacesUtils.annotations.setItemAnnotation(id, STATICTITLE_ANNO,
-                                            "Static title", 0,
-                                            PlacesUtils.annotations.EXPIRE_NEVER);
-  return id;
-}
-
-function run_test() {
-
-  Service.engineManager.register(BookmarksEngine);
-  let engine = Service.engineManager.get("bookmarks");
-  let store = engine._store;
-
-  // Clean up.
-  store.wipe();
-
-  initTestLogging("Trace");
-  Log.repository.getLogger("Sync.Engine.Bookmarks").level = Log.Level.Trace;
-
-  _("Create a microsummarized bookmark.");
-  let id = newMicrosummary(TEST_URL, TEST_TITLE);
-  let guid = store.GUIDForId(id);
-  _("GUID: " + guid);
-  do_check_true(!!guid);
-
-  _("Create record object and verify that it's sane.");
-  let record = store.createRecord(guid);
-  do_check_true(record instanceof Bookmark);
-  do_check_eq(record.bmkUri, TEST_URL);
-
-  _("Make sure the new record does not carry the microsummaries annotations.");
-  do_check_false("staticTitle" in record);
-  do_check_false("generatorUri" in record);
-
-  _("Remove the bookmark from Places.");
-  PlacesUtils.bookmarks.removeItem(id);
-
-  _("Convert record to the old microsummaries one.");
-  record.staticTitle = STATIC_TITLE;
-  record.generatorUri = GENERATOR_URL;
-  record.type = "microsummary";
-
-  _("Apply the modified record as incoming data.");
-  store.applyIncoming(record);
-
-  _("Verify it has been created correctly as a simple Bookmark.");
-  id = store.idForGUID(record.id);
-  do_check_eq(store.GUIDForId(id), record.id);
-  do_check_eq(PlacesUtils.bookmarks.getItemType(id),
-              PlacesUtils.bookmarks.TYPE_BOOKMARK);
-  do_check_eq(PlacesUtils.bookmarks.getBookmarkURI(id).spec, TEST_URL);
-  do_check_eq(PlacesUtils.bookmarks.getItemTitle(id), TEST_TITLE);
-  do_check_eq(PlacesUtils.bookmarks.getFolderIdForItem(id),
-              PlacesUtils.unfiledBookmarksFolderId);
-  do_check_eq(PlacesUtils.bookmarks.getKeywordForBookmark(id), null);
-
-  do_check_throws(
-    () => PlacesUtils.annotations.getItemAnnotation(id, GENERATORURI_ANNO),
-    Cr.NS_ERROR_NOT_AVAILABLE
-  );
-
-  do_check_throws(
-    () => PlacesUtils.annotations.getItemAnnotation(id, STATICTITLE_ANNO),
-    Cr.NS_ERROR_NOT_AVAILABLE
-  );
-
-  // Clean up.
-  store.wipe();
-}
--- a/services/sync/tests/unit/xpcshell.ini
+++ b/services/sync/tests/unit/xpcshell.ini
@@ -125,17 +125,16 @@ tags = addons
 run-sequentially = Hardcoded port in static files.
 tags = addons
 [test_addons_tracker.js]
 tags = addons
 [test_bookmark_batch_fail.js]
 [test_bookmark_duping.js]
 [test_bookmark_engine.js]
 [test_bookmark_invalid.js]
-[test_bookmark_legacy_microsummaries_support.js]
 [test_bookmark_livemarks.js]
 [test_bookmark_order.js]
 [test_bookmark_places_query_rewriting.js]
 [test_bookmark_record.js]
 [test_bookmark_repair.js]
 [test_bookmark_repair_requestor.js]
 [test_bookmark_repair_responder.js]
 [test_bookmark_smart_bookmarks.js]
--- a/toolkit/components/places/PlacesSyncUtils.jsm
+++ b/toolkit/components/places/PlacesSyncUtils.jsm
@@ -77,20 +77,16 @@ const BookmarkSyncUtils = PlacesSyncUtil
   SMART_BOOKMARKS_ANNO: "Places/SmartBookmark",
   DESCRIPTION_ANNO: "bookmarkProperties/description",
   SIDEBAR_ANNO: "bookmarkProperties/loadInSidebar",
   SYNC_PARENT_ANNO: "sync/parent",
   SYNC_MOBILE_ROOT_ANNO: "mobile/bookmarksRoot",
 
   KINDS: {
     BOOKMARK: "bookmark",
-    // Microsummaries were removed from Places in bug 524091. For now, Sync
-    // treats them identically to bookmarks. Bug 745410 tracks removing them
-    // entirely.
-    MICROSUMMARY: "microsummary",
     QUERY: "query",
     FOLDER: "folder",
     LIVEMARK: "livemark",
     SEPARATOR: "separator",
   },
 
   get ROOTS() {
     return ROOTS;
@@ -661,17 +657,16 @@ const BookmarkSyncUtils = PlacesSyncUtil
     // Convert the Places bookmark object to a Sync bookmark and add
     // kind-specific properties. Titles are required for bookmarks,
     // folders, and livemarks; optional for queries, and omitted for
     // separators.
     let kind = yield getKindForItem(bookmarkItem);
     let item;
     switch (kind) {
       case BookmarkSyncUtils.KINDS.BOOKMARK:
-      case BookmarkSyncUtils.KINDS.MICROSUMMARY:
         item = yield fetchBookmarkItem(bookmarkItem);
         break;
 
       case BookmarkSyncUtils.KINDS.QUERY:
         item = yield fetchQueryItem(bookmarkItem);
         break;
 
       case BookmarkSyncUtils.KINDS.FOLDER:
@@ -1072,17 +1067,16 @@ var getKindForItem = Task.async(function
   return null;
 });
 
 // Returns the `nsINavBookmarksService` bookmark type constant for a Sync
 // record kind.
 function getTypeForKind(kind) {
   switch (kind) {
     case BookmarkSyncUtils.KINDS.BOOKMARK:
-    case BookmarkSyncUtils.KINDS.MICROSUMMARY:
     case BookmarkSyncUtils.KINDS.QUERY:
       return PlacesUtils.bookmarks.TYPE_BOOKMARK;
 
     case BookmarkSyncUtils.KINDS.FOLDER:
     case BookmarkSyncUtils.KINDS.LIVEMARK:
       return PlacesUtils.bookmarks.TYPE_FOLDER;
 
     case BookmarkSyncUtils.KINDS.SEPARATOR:
@@ -1286,42 +1280,35 @@ var updateBookmarkMetadata = Task.async(
   return newItem;
 });
 
 function validateNewBookmark(info) {
   let insertInfo = validateSyncBookmarkObject(info,
     { kind: { required: true }
     , syncId: { required: true }
     , url: { requiredIf: b => [ BookmarkSyncUtils.KINDS.BOOKMARK
-                              , BookmarkSyncUtils.KINDS.MICROSUMMARY
                               , BookmarkSyncUtils.KINDS.QUERY ].includes(b.kind)
            , validIf: b => [ BookmarkSyncUtils.KINDS.BOOKMARK
-                           , BookmarkSyncUtils.KINDS.MICROSUMMARY
                            , BookmarkSyncUtils.KINDS.QUERY ].includes(b.kind) }
     , parentSyncId: { required: true }
     , title: { validIf: b => [ BookmarkSyncUtils.KINDS.BOOKMARK
-                             , BookmarkSyncUtils.KINDS.MICROSUMMARY
                              , BookmarkSyncUtils.KINDS.QUERY
                              , BookmarkSyncUtils.KINDS.FOLDER
                              , BookmarkSyncUtils.KINDS.LIVEMARK ].includes(b.kind) }
     , query: { validIf: b => b.kind == BookmarkSyncUtils.KINDS.QUERY }
     , folder: { validIf: b => b.kind == BookmarkSyncUtils.KINDS.QUERY }
     , tags: { validIf: b => [ BookmarkSyncUtils.KINDS.BOOKMARK
-                            , BookmarkSyncUtils.KINDS.MICROSUMMARY
                             , BookmarkSyncUtils.KINDS.QUERY ].includes(b.kind) }
     , keyword: { validIf: b => [ BookmarkSyncUtils.KINDS.BOOKMARK
-                               , BookmarkSyncUtils.KINDS.MICROSUMMARY
                                , BookmarkSyncUtils.KINDS.QUERY ].includes(b.kind) }
     , description: { validIf: b => [ BookmarkSyncUtils.KINDS.BOOKMARK
-                                   , BookmarkSyncUtils.KINDS.MICROSUMMARY
                                    , BookmarkSyncUtils.KINDS.QUERY
                                    , BookmarkSyncUtils.KINDS.FOLDER
                                    , BookmarkSyncUtils.KINDS.LIVEMARK ].includes(b.kind) }
     , loadInSidebar: { validIf: b => [ BookmarkSyncUtils.KINDS.BOOKMARK
-                                     , BookmarkSyncUtils.KINDS.MICROSUMMARY
                                      , BookmarkSyncUtils.KINDS.QUERY ].includes(b.kind) }
     , feed: { validIf: b => b.kind == BookmarkSyncUtils.KINDS.LIVEMARK }
     , site: { validIf: b => b.kind == BookmarkSyncUtils.KINDS.LIVEMARK }
     });
 
   return insertInfo;
 }