Bug 1436215 - Remove left joins on the Sync merge states table when recording bookmark observer notifications. r?markh draft
authorKit Cambridge <kit@yakshaving.ninja>
Tue, 06 Feb 2018 15:33:33 -0800
changeset 751882 d12603822ce65e58c91996a54d286f4eff22a895
parent 751815 1489e164a5e724462790388dd6358644a3ca06f1
child 751883 00d018753b331995eb424247041064c8330fbb0a
push id98084
push userbmo:kit@mozilla.com
push dateWed, 07 Feb 2018 01:53:52 +0000
reviewersmarkh
bugs1436215
milestone60.0a1
Bug 1436215 - Remove left joins on the Sync merge states table when recording bookmark observer notifications. r?markh MozReview-Commit-ID: KIp6jRshqY4
toolkit/components/places/SyncedBookmarksMirror.jsm
--- a/toolkit/components/places/SyncedBookmarksMirror.jsm
+++ b/toolkit/components/places/SyncedBookmarksMirror.jsm
@@ -1208,45 +1208,40 @@ class SyncedBookmarksMirror {
 
     MirrorLog.debug("Recording observer notifications for changed GUIDs");
     let changedGuidRows = await this.db.execute(`
       SELECT b.id, b.lastModified, b.type, b.guid AS newGuid,
              c.oldGuid, p.id AS parentId, p.guid AS parentGuid
       FROM guidsChanged c
       JOIN moz_bookmarks b ON b.id = c.itemId
       JOIN moz_bookmarks p ON p.id = b.parent
-      JOIN mergeStates r ON r.mergedGuid = b.guid
-      ORDER BY r.level, p.id, b.position`);
+      ORDER BY c.level, p.id, b.position`);
     for (let row of changedGuidRows) {
       let info = {
         id: row.getResultByName("id"),
         lastModified: row.getResultByName("lastModified"),
         type: row.getResultByName("type"),
         newGuid: row.getResultByName("newGuid"),
         oldGuid: row.getResultByName("oldGuid"),
         parentId: row.getResultByName("parentId"),
         parentGuid: row.getResultByName("parentGuid"),
       };
       observersToNotify.noteGuidChanged(info);
     }
 
     MirrorLog.debug("Recording observer notifications for new items");
-    // We `LEFT JOIN` to `mergeStates` because `itemsAdded` may include tag
-    // folders and entries, which are not part of the merged tree structure, and
-    // so don't exist in `mergeStates`.
     let newItemRows = await this.db.execute(`
       SELECT b.id, p.id AS parentId, b.position, b.type, h.url,
              IFNULL(b.title, "") AS title, b.dateAdded, b.guid,
              p.guid AS parentGuid, n.isTagging
       FROM itemsAdded n
       JOIN moz_bookmarks b ON b.guid = n.guid
       JOIN moz_bookmarks p ON p.id = b.parent
       LEFT JOIN moz_places h ON h.id = b.fk
-      LEFT JOIN mergeStates r ON r.mergedGuid = b.guid
-      ORDER BY r.level, p.id, b.position`);
+      ORDER BY n.level, p.id, b.position`);
     for (let row of newItemRows) {
       let info = {
         id: row.getResultByName("id"),
         parentId: row.getResultByName("parentId"),
         position: row.getResultByName("position"),
         type: row.getResultByName("type"),
         urlHref: row.getResultByName("url"),
         title: row.getResultByName("title"),
@@ -1261,18 +1256,17 @@ class SyncedBookmarksMirror {
     MirrorLog.debug("Recording observer notifications for moved items");
     let movedItemRows = await this.db.execute(`
       SELECT b.id, b.guid, b.type, p.id AS newParentId, c.oldParentId,
              p.guid AS newParentGuid, c.oldParentGuid,
              b.position AS newPosition, c.oldPosition
       FROM itemsMoved c
       JOIN moz_bookmarks b ON b.id = c.itemId
       JOIN moz_bookmarks p ON p.id = b.parent
-      JOIN mergeStates r ON r.mergedGuid = b.guid
-      ORDER BY r.level, newParentId, newPosition`);
+      ORDER BY c.level, newParentId, newPosition`);
     for (let row of movedItemRows) {
       let info = {
         id: row.getResultByName("id"),
         guid: row.getResultByName("guid"),
         type: row.getResultByName("type"),
         newParentId: row.getResultByName("newParentId"),
         oldParentId: row.getResultByName("oldParentId"),
         newParentGuid: row.getResultByName("newParentGuid"),
@@ -1288,20 +1282,19 @@ class SyncedBookmarksMirror {
       SELECT b.id, b.guid, b.lastModified, b.type,
              IFNULL(b.title, "") AS newTitle,
              IFNULL(c.oldTitle, "") AS oldTitle,
              h.url AS newURL, i.url AS oldURL,
              p.id AS parentId, p.guid AS parentGuid
       FROM itemsChanged c
       JOIN moz_bookmarks b ON b.id = c.itemId
       JOIN moz_bookmarks p ON p.id = b.parent
-      JOIN mergeStates r ON r.mergedGuid = b.guid
       LEFT JOIN moz_places h ON h.id = b.fk
       LEFT JOIN moz_places i ON i.id = c.oldPlaceId
-      ORDER BY r.level, p.id, b.position`);
+      ORDER BY c.level, p.id, b.position`);
     for (let row of changedItemRows) {
       let info = {
         id: row.getResultByName("id"),
         guid: row.getResultByName("guid"),
         lastModified: row.getResultByName("lastModified"),
         type: row.getResultByName("type"),
         newTitle: row.getResultByName("newTitle"),
         oldTitle: row.getResultByName("oldTitle"),
@@ -1902,22 +1895,22 @@ async function initializeTempMirrorEntit
   ) WITHOUT ROWID`);
 
   // A view of the value states for all bookmarks in the mirror. We use triggers
   // on this view to update Places. Note that we can't just `REPLACE INTO
   // moz_bookmarks`, because `REPLACE` doesn't fire the `AFTER DELETE` triggers
   // that Places uses to maintain schema coherency.
   await db.execute(`
     CREATE TEMP VIEW newRemoteItems(localId, remoteId, localGuid, mergedGuid,
-                                    needsUpdate, type, dateAdded, title,
-                                    oldPlaceId, newPlaceId, newKeyword,
+                                    mergedLevel, needsUpdate, type, dateAdded,
+                                    title, oldPlaceId, newPlaceId, newKeyword,
                                     description, loadInSidebar,
                                     smartBookmarkName, feedURL, siteURL,
                                     syncChangeCounter) AS
-    SELECT b.id, v.id, r.localGuid, r.mergedGuid,
+    SELECT b.id, v.id, r.localGuid, r.mergedGuid, r.level,
            r.valueState = ${BookmarkMergeState.TYPE.REMOTE},
            (CASE WHEN v.kind IN (${[
                         SyncedBookmarksMirror.KIND.BOOKMARK,
                         SyncedBookmarksMirror.KIND.QUERY,
                       ].join(",")}) THEN ${PlacesUtils.bookmarks.TYPE_BOOKMARK}
                  WHEN v.kind IN (${[
                         SyncedBookmarksMirror.KIND.FOLDER,
                         SyncedBookmarksMirror.KIND.LIVEMARK,
@@ -1952,18 +1945,18 @@ async function initializeTempMirrorEntit
          trigger, because deduped items where we're keeping the local value
          state won't have "needsMerge" set. */
       UPDATE moz_bookmarks SET
         guid = OLD.mergedGuid
       WHERE OLD.localGuid <> OLD.mergedGuid AND
             guid = OLD.localGuid;
 
       /* Record item changed notifications for the updated GUIDs. */
-      INSERT INTO guidsChanged(itemId, oldGuid)
-      SELECT OLD.localId, OLD.localGuid
+      INSERT INTO guidsChanged(itemId, oldGuid, level)
+      SELECT OLD.localId, OLD.localGuid, OLD.mergedLevel
       WHERE OLD.localGuid <> OLD.mergedGuid;
 
       DELETE FROM moz_bookmarks_deleted WHERE guid = OLD.mergedGuid;
 
       /* Flag the remote item as merged. */
       UPDATE items SET
         needsMerge = 0
       WHERE needsMerge AND
@@ -2015,18 +2008,18 @@ async function initializeTempMirrorEntit
                                 dateAdded, lastModified, syncStatus,
                                 syncChangeCounter)
       VALUES(OLD.mergedGuid, -1, -1, OLD.type, OLD.newPlaceId, OLD.title,
              OLD.dateAdded, STRFTIME('%s', 'now', 'localtime', 'utc') * 1000000,
              ${PlacesUtils.bookmarks.SYNC_STATUS.NORMAL},
              OLD.syncChangeCounter);
 
       /* Record an item added notification for the new item. */
-      INSERT INTO itemsAdded(guid)
-      VALUES(OLD.mergedGuid);
+      INSERT INTO itemsAdded(guid, level)
+      VALUES(OLD.mergedGuid, OLD.mergedLevel);
 
       /* Insert new keywords after the item, so that "noteKeywordAdded" can find
          the new item by Place ID. */
       INSERT INTO moz_keywords(keyword, place_id)
       SELECT OLD.newKeyword, OLD.newPlaceId
       WHERE OLD.newKeyword NOT NULL;
 
       /* Record item changed notifications for the new keyword. */
@@ -2071,18 +2064,18 @@ async function initializeTempMirrorEntit
 
   // Updates existing items with new values from the mirror.
   await db.execute(`
     CREATE TEMP TRIGGER updateExistingLocalItems
     INSTEAD OF DELETE ON newRemoteItems WHEN OLD.needsUpdate AND
                                              OLD.localId NOT NULL
     BEGIN
       /* Record item changed notifications for the title and URL. */
-      INSERT INTO itemsChanged(itemId, oldTitle, oldPlaceId)
-      SELECT id, title, OLD.oldPlaceId FROM moz_bookmarks
+      INSERT INTO itemsChanged(itemId, oldTitle, oldPlaceId, level)
+      SELECT id, title, OLD.oldPlaceId, OLD.mergedLevel FROM moz_bookmarks
       WHERE id = OLD.localId;
 
       UPDATE moz_bookmarks SET
         title = OLD.title,
         dateAdded = OLD.dateAdded,
         lastModified = STRFTIME('%s', 'now', 'localtime', 'utc') * 1000000,
         syncStatus = ${PlacesUtils.bookmarks.SYNC_STATUS.NORMAL},
         syncChangeCounter = OLD.syncChangeCounter
@@ -2189,18 +2182,18 @@ async function initializeTempMirrorEntit
   // stores structure info on children. Unlike iOS, we can't simply check the
   // parent's merge state to know if its children changed. This is because our
   // merged tree might diverge from the mirror if we're missing children, or if
   // we temporarily reparented children without parents to "unfiled". In that
   // case, we want to keep syncing, but *don't* want to reupload the new local
   // structure to the server.
   await db.execute(`
     CREATE TEMP VIEW newRemoteStructure(localId, oldParentId, newParentId,
-                                        oldPosition, newPosition) AS
-    SELECT b.id, b.parent, p.id, b.position, r.position
+                                        oldPosition, newPosition, newLevel) AS
+    SELECT b.id, b.parent, p.id, b.position, r.position, r.level
     FROM moz_bookmarks b
     JOIN mergeStates r ON r.mergedGuid = b.guid
     JOIN moz_bookmarks p ON p.guid = r.parentGuid
     WHERE r.parentGuid <> '${PlacesUtils.bookmarks.rootGuid}'`);
 
   // Updates all parents and positions to reflect the merged tree.
   await db.execute(`
     CREATE TEMP TRIGGER updateLocalStructure
@@ -2214,18 +2207,20 @@ async function initializeTempMirrorEntit
       UPDATE moz_bookmarks SET
         position = OLD.newPosition
       WHERE id = OLD.localId AND
             position <> OLD.newPosition;
 
       /* Record observer notifications for moved items. We ignore items that
          didn't move, and items with placeholder parents and positions of "-1",
          since they're new. */
-      INSERT INTO itemsMoved(itemId, oldParentId, oldParentGuid, oldPosition)
-      SELECT OLD.localId, OLD.oldParentId, p.guid, OLD.oldPosition
+      INSERT INTO itemsMoved(itemId, oldParentId, oldParentGuid, oldPosition,
+                             level)
+      SELECT OLD.localId, OLD.oldParentId, p.guid, OLD.oldPosition,
+             OLD.newLevel
       FROM moz_bookmarks p
       WHERE p.id = OLD.oldParentId AND
             -1 NOT IN (OLD.oldParentId, OLD.oldPosition) AND
             (OLD.oldParentId <> OLD.newParentId OR
              OLD.oldPosition <> OLD.newPosition);
     END`);
 
   // A view of local bookmark tags. Tags, like keywords, are associated with
@@ -2329,36 +2324,40 @@ async function initializeTempMirrorEntit
             p.title = NEW.tag AND
             r.guid = '${PlacesUtils.bookmarks.tagsGuid}';
     END`);
 
   // Stores properties to pass to `onItem{Added, Changed, Moved, Removed}`
   // bookmark observers for new, updated, moved, and deleted items.
   await db.execute(`CREATE TEMP TABLE itemsAdded(
     guid TEXT PRIMARY KEY,
-    isTagging BOOLEAN NOT NULL DEFAULT 0
+    isTagging BOOLEAN NOT NULL DEFAULT 0,
+    level INTEGER NOT NULL DEFAULT -1
   ) WITHOUT ROWID`);
 
   await db.execute(`CREATE TEMP TABLE guidsChanged(
     itemId INTEGER NOT NULL,
     oldGuid TEXT NOT NULL,
+    level INTEGER NOT NULL DEFAULT -1,
     PRIMARY KEY(itemId, oldGuid)
   ) WITHOUT ROWID`);
 
   await db.execute(`CREATE TEMP TABLE itemsChanged(
     itemId INTEGER PRIMARY KEY,
     oldTitle TEXT,
-    oldPlaceId INTEGER
+    oldPlaceId INTEGER,
+    level INTEGER NOT NULL DEFAULT -1
   )`);
 
   await db.execute(`CREATE TEMP TABLE itemsMoved(
     itemId INTEGER PRIMARY KEY,
     oldParentId INTEGER NOT NULL,
     oldParentGuid TEXT NOT NULL,
-    oldPosition INTEGER NOT NULL
+    oldPosition INTEGER NOT NULL,
+    level INTEGER NOT NULL DEFAULT -1
   )`);
 
   await db.execute(`CREATE TEMP TABLE itemsRemoved(
     guid TEXT PRIMARY KEY,
     itemId INTEGER NOT NULL,
     parentId INTEGER NOT NULL,
     position INTEGER NOT NULL,
     type INTEGER NOT NULL,