Bug 1455906 - Handle zero dates added and last modified times in Places maintenance. r?Standard8 draft
authorKit Cambridge <kit@yakshaving.ninja>
Mon, 23 Apr 2018 09:36:58 -0700
changeset 789095 db37d4450ba22398a157466fb772cd62da70ce86
parent 789020 d2d518b1f8730eb61554df7179ef9a2aeed4d843
push id108174
push userbmo:kit@mozilla.com
push dateFri, 27 Apr 2018 15:52:47 +0000
reviewersStandard8
bugs1455906
milestone61.0a1
Bug 1455906 - Handle zero dates added and last modified times in Places maintenance. r?Standard8 MozReview-Commit-ID: 7eulrNyKh29
toolkit/components/places/PlacesDBUtils.jsm
toolkit/components/places/tests/unit/test_preventive_maintenance.js
--- a/toolkit/components/places/PlacesDBUtils.jsm
+++ b/toolkit/components/places/PlacesDBUtils.jsm
@@ -691,26 +691,33 @@ var PlacesDBUtils = {
       { query:
         `DELETE FROM moz_bookmarks_deleted
         WHERE guid IN (SELECT guid FROM moz_bookmarks)`,
       },
 
       // S.3 set missing added and last modified dates.
       { query:
         `UPDATE moz_bookmarks
-        SET dateAdded = COALESCE(dateAdded, lastModified, (
+        SET dateAdded = COALESCE(NULLIF(dateAdded, 0), NULLIF(lastModified, 0), NULLIF((
               SELECT MIN(visit_date) FROM moz_historyvisits
               WHERE place_id = fk
-            ), STRFTIME('%s', 'now', 'localtime', 'utc') * 1000000),
-            lastModified = COALESCE(lastModified, dateAdded, (
+            ), 0), STRFTIME('%s', 'now', 'localtime', 'utc') * 1000000),
+            lastModified = COALESCE(NULLIF(lastModified, 0), NULLIF(dateAdded, 0), NULLIF((
               SELECT MAX(visit_date) FROM moz_historyvisits
               WHERE place_id = fk
-            ), STRFTIME('%s', 'now', 'localtime', 'utc') * 1000000)
-        WHERE dateAdded IS NULL OR
-              lastModified IS NULL`,
+            ), 0), STRFTIME('%s', 'now', 'localtime', 'utc') * 1000000)
+        WHERE NULLIF(dateAdded, 0) IS NULL OR
+              NULLIF(lastModified, 0) IS NULL`,
+      },
+
+      // S.4 reset added dates that are ahead of last modified dates.
+      { query:
+        `UPDATE moz_bookmarks
+         SET dateAdded = lastModified
+         WHERE dateAdded > lastModified`,
       },
     ];
 
     // Create triggers for updating Sync metadata. The "sync change" trigger
     // bumps the parent's change counter when we update a GUID or move an item
     // to a different folder, since Sync stores the list of child GUIDs on the
     // parent. The "sync tombstone" trigger inserts tombstones for deleted
     // synced bookmarks.
--- a/toolkit/components/places/tests/unit/test_preventive_maintenance.js
+++ b/toolkit/components/places/tests/unit/test_preventive_maintenance.js
@@ -1480,22 +1480,26 @@ tests.push({
 tests.push({
   name: "S.3",
   desc: "set missing added and last modified dates",
   _placeVisits: [],
   _bookmarksWithDates: [],
 
   async setup() {
     let placeIdWithVisits = addPlace();
+    let placeIdWithZeroVisit = addPlace();
     this._placeVisits.push({
       placeId: placeIdWithVisits,
       visitDate: PlacesUtils.toPRTime(new Date(2017, 9, 4)),
     }, {
       placeId: placeIdWithVisits,
       visitDate: PlacesUtils.toPRTime(new Date(2017, 9, 8)),
+    }, {
+      placeId: placeIdWithZeroVisit,
+      visitDate: 0,
     });
 
     this._bookmarksWithDates.push({
       guid: "bookmarkAAAA",
       placeId: null,
       parentId: bs.bookmarksMenuFolder,
       dateAdded: null,
       lastModified: PlacesUtils.toPRTime(new Date(2017, 9, 1)),
@@ -1518,43 +1522,57 @@ tests.push({
       dateAdded: null,
       lastModified: null,
     }, {
       guid: "bookmarkEEEE",
       placeId: placeIdWithVisits,
       parentId: bs.unfiledBookmarksFolder,
       dateAdded: PlacesUtils.toPRTime(new Date(2017, 9, 3)),
       lastModified: PlacesUtils.toPRTime(new Date(2017, 9, 6)),
+    }, {
+      guid: "bookmarkFFFF",
+      placeId: placeIdWithZeroVisit,
+      parentId: bs.unfiledBookmarksFolder,
+      dateAdded: 0,
+      lastModified: 0,
     });
 
     await PlacesUtils.withConnectionWrapper(
-      "Insert bookmarks and visits with dates",
+      "S.3: Insert bookmarks and visits",
       db => db.executeTransaction(async () => {
-        await db.executeCached(`
+        await db.execute(`
           INSERT INTO moz_historyvisits(place_id, visit_date)
           VALUES(:placeId, :visitDate)`,
           this._placeVisits);
 
-        await db.executeCached(`
+        await db.execute(`
           INSERT INTO moz_bookmarks(fk, type, parent, guid, dateAdded,
                                     lastModified)
           VALUES(:placeId, 1, :parentId, :guid, :dateAdded,
                  :lastModified)`,
           this._bookmarksWithDates);
+
+        await db.execute(`
+          UPDATE moz_bookmarks SET
+            dateAdded = 0,
+            lastModified = NULL
+          WHERE id = :toolbarFolderId`,
+          { toolbarFolderId: bs.toolbarFolder });
       })
     );
   },
 
   async check() {
     let db = await PlacesUtils.promiseDBConnection();
-    let updatedRows = await db.executeCached(`
+    let updatedRows = await db.execute(`
       SELECT guid, dateAdded, lastModified
       FROM moz_bookmarks
-      WHERE guid IN (?, ?, ?, ?, ?)`,
-      this._bookmarksWithDates.map(info => info.guid));
+      WHERE guid = :guid`,
+      [{ guid: bs.toolbarGuid },
+       ...this._bookmarksWithDates.map(({ guid }) => ({ guid }))]);
 
     for (let row of updatedRows) {
       let guid = row.getResultByName("guid");
 
       let dateAdded = row.getResultByName("dateAdded");
       Assert.ok(Number.isInteger(dateAdded));
 
       let lastModified = row.getResultByName("lastModified");
@@ -1572,20 +1590,24 @@ tests.push({
         // Date added exists, so we should use it for last modified date.
         case "bookmarkBBBB": {
           let expectedInfo = this._bookmarksWithDates[1];
           Assert.equal(dateAdded, expectedInfo.dateAdded);
           Assert.equal(lastModified, expectedInfo.dateAdded);
           break;
         }
 
-        // Neither date added nor last modified exists, and no visits, so we
-        // should fall back to the current time for both.
-        case "bookmarkCCCC": {
+        // C has no visits, date added, or last modified time, F has zeros for
+        // all, and the toolbar has a zero date added and no last modified time.
+        // In all cases, we should fall back to the current time.
+        case "bookmarkCCCC":
+        case "bookmarkFFFF":
+        case bs.toolbarGuid: {
           let nowAsPRTime = PlacesUtils.toPRTime(new Date());
+          Assert.greater(dateAdded, 0);
           Assert.equal(dateAdded, lastModified);
           Assert.ok(dateAdded <= nowAsPRTime);
           break;
         }
 
         // Neither date added nor last modified exists, but we have two
         // visits, so we should fall back to the earliest and latest visit
         // dates.
@@ -1600,16 +1622,73 @@ tests.push({
         // We have two visits, but both date added and last modified exist,
         // so we shouldn't update them.
         case "bookmarkEEEE": {
           let expectedInfo = this._bookmarksWithDates[4];
           Assert.equal(dateAdded, expectedInfo.dateAdded);
           Assert.equal(lastModified, expectedInfo.lastModified);
           break;
         }
+
+        default:
+          throw new Error(`Unexpected row for bookmark ${guid}`);
+      }
+    }
+  },
+});
+
+// ------------------------------------------------------------------------------
+
+tests.push({
+  name: "S.4",
+  desc: "reset added dates that are ahead of last modified dates",
+  _bookmarksWithDates: [],
+
+  async setup() {
+    this._bookmarksWithDates.push({
+      guid: "bookmarkGGGG",
+      parentId: bs.unfiledBookmarksFolder,
+      dateAdded: PlacesUtils.toPRTime(new Date(2017, 9, 6)),
+      lastModified: PlacesUtils.toPRTime(new Date(2017, 9, 3)),
+    });
+
+    await PlacesUtils.withConnectionWrapper(
+      "S.4: Insert bookmarks and visits",
+      db => db.executeTransaction(async () => {
+        await db.execute(`
+          INSERT INTO moz_bookmarks(type, parent, guid, dateAdded,
+                                    lastModified)
+          VALUES(1, :parentId, :guid, :dateAdded, :lastModified)`,
+          this._bookmarksWithDates);
+      })
+    );
+  },
+
+  async check() {
+    let db = await PlacesUtils.promiseDBConnection();
+    let updatedRows = await db.execute(`
+      SELECT guid, dateAdded, lastModified
+      FROM moz_bookmarks
+      WHERE guid = :guid`,
+      this._bookmarksWithDates.map(({ guid }) => ({ guid })));
+
+    for (let row of updatedRows) {
+      let guid = row.getResultByName("guid");
+      let dateAdded = row.getResultByName("dateAdded");
+      let lastModified = row.getResultByName("lastModified");
+      switch (guid) {
+        case "bookmarkGGGG": {
+          let expectedInfo = this._bookmarksWithDates[0];
+          Assert.equal(dateAdded, expectedInfo.lastModified);
+          Assert.equal(lastModified, expectedInfo.lastModified);
+          break;
+        }
+
+        default:
+          throw new Error(`Unexpected row for bookmark ${guid}`);
       }
     }
   },
 });
 
 // ------------------------------------------------------------------------------
 
 tests.push({