Bug 1760155 - Change Snapshot Groups title to NULLable. r=Standard8

Differential Revision: https://phabricator.services.mozilla.com/D142215

toolkit/components/places/Database.cpp

@@ -1258,7 +1258,12 @@ nsresult Database::InitSchema(bool* aDatabaseMigrated) {
         NS_ENSURE_SUCCESS(rv, rv);
       }
 
-      // Firefox 100 uses schema version 65
+      if (currentSchemaVersion < 66) {
+        rv = MigrateV66Up();
+        NS_ENSURE_SUCCESS(rv, rv);
+      }
+
+      // Firefox 100 uses schema version 66
 
       // Schema Upgrades must add migration code here.
       // >>> IMPORTANT! <<<
@@ -2485,6 +2490,37 @@ nsresult Database::MigrateV65Up() {
   return NS_OK;
 }
 
+nsresult Database::MigrateV66Up() {
+  // Let the title column in snapshots groups be NULL.
+  nsCOMPtr<mozIStorageStatement> stmt;
+  nsresult rv = mMainConn->CreateStatement(
+      "SELECT 1 "
+      "FROM sqlite_master "
+      "WHERE name = 'moz_places_metadata_snapshots_groups' AND "
+      "INSTR(sql, 'title TEXT NOT NULL') > 0"_ns,
+      getter_AddRefs(stmt));
+  NS_ENSURE_SUCCESS(rv, rv);
+  bool hasMore = false;
+  if (NS_SUCCEEDED(stmt->ExecuteStep(&hasMore)) && hasMore) {
+    // Migrate non-empty titles to the builder_data object, drop the column
+    // and recreate a nullable one.
+    rv = mMainConn->ExecuteSimpleSQL(
+        "UPDATE moz_places_metadata_snapshots_groups "
+        "SET builder_data = json_set(IFNULL(builder_data, json_object()), "
+        "'$.title', title) "
+        "WHERE title <> '' AND builder_data->>'title' IS NULL"_ns);
+    NS_ENSURE_SUCCESS(rv, rv);
+    rv = mMainConn->ExecuteSimpleSQL(
+        "ALTER TABLE moz_places_metadata_snapshots_groups DROP COLUMN title"_ns);
+    NS_ENSURE_SUCCESS(rv, rv);
+    rv = mMainConn->ExecuteSimpleSQL(
+        "ALTER TABLE moz_places_metadata_snapshots_groups "
+        "ADD COLUMN title TEXT"_ns);
+    NS_ENSURE_SUCCESS(rv, rv);
+  }
+  return NS_OK;
+}
+
 nsresult Database::ConvertOldStyleQuery(nsCString& aURL) {
   AutoTArray<QueryKeyValuePair, 8> tokens;
   nsresult rv = TokenizeQueryString(aURL, &tokens);

toolkit/components/places/Database.h

@@ -18,7 +18,7 @@
 
 // This is the schema version. Update it at any schema change and add a
 // corresponding migrateVxx method below.
-#define DATABASE_SCHEMA_VERSION 65
+#define DATABASE_SCHEMA_VERSION 66
 
 // Fired after Places inited.
 #define TOPIC_PLACES_INIT_COMPLETE "places-init-complete"
@@ -338,6 +338,7 @@ class Database final : public nsIObserver, public nsSupportsWeakReference {
   nsresult MigrateV63Up();
   nsresult MigrateV64Up();
   nsresult MigrateV65Up();
+  nsresult MigrateV66Up();
 
   void MigrateV52OriginFrecencies();
 

toolkit/components/places/nsPlacesTables.h

@@ -335,7 +335,7 @@
   nsLiteralCString(                                                        \
       "CREATE TABLE IF NOT EXISTS moz_places_metadata_snapshots_groups ( " \
       "  id INTEGER PRIMARY KEY, "                                         \
-      "  title TEXT NOT NULL, "                                            \
+      "  title TEXT, "                                                     \
       "  hidden INTEGER DEFAULT 0 NOT NULL, "                              \
       "  builder TEXT NOT NULL, "                                          \
       "  builder_data TEXT "                                               \

toolkit/components/places/tests/migration/head_migration.js

@@ -15,7 +15,7 @@ var { Services } = ChromeUtils.import("resource://gre/modules/Services.jsm");
 
 // Put any other stuff relative to this test folder below.
 
-const CURRENT_SCHEMA_VERSION = 65;
+const CURRENT_SCHEMA_VERSION = 66;
 const FIRST_UPGRADABLE_SCHEMA_VERSION = 43;
 
 async function assertAnnotationsRemoved(db, expectedAnnos) {

toolkit/components/places/tests/migration/test_current_from_v65.js

@@ -0,0 +1,51 @@
+/* Any copyright is dedicated to the Public Domain.
+ * http://creativecommons.org/publicdomain/zero/1.0/ */
+
+"use strict";
+
+add_task(async function setup() {
+  let path = await setupPlacesDatabase("places_v65.sqlite");
+  // Since this migration doesn't affect favicons.sqlite, we can reuse v41.
+  await setupPlacesDatabase("favicons_v41.sqlite", "favicons.sqlite");
+  let db = await Sqlite.openConnection({ path });
+  await db.execute(`
+    INSERT INTO moz_places_metadata_snapshots_groups (title, builder, builder_data)
+    VALUES
+      ('', 'test1', json_object('fluentTitle', 'test1')),
+      ('test2', 'test2', NULL),
+      ('test3', 'test3', json_object())
+  `);
+  await db.close();
+});
+
+add_task(async function database_is_valid() {
+  // Accessing the database for the first time triggers migration.
+  Assert.equal(
+    PlacesUtils.history.databaseStatus,
+    PlacesUtils.history.DATABASE_STATUS_UPGRADED
+  );
+
+  let db = await PlacesUtils.promiseDBConnection();
+  Assert.equal(await db.getSchemaVersion(), CURRENT_SCHEMA_VERSION);
+});
+
+add_task(async function snapshot_groups_nullable_title_field() {
+  await PlacesUtils.withConnectionWrapper("test_sqlite_migration", async db => {
+    await db.execute(
+      `INSERT INTO moz_places_metadata_snapshots_groups (title, builder, builder_data)
+        VALUES (NULL, "test", json_object('title', 'test'))`
+    );
+    let rows = await db.execute(
+      `SELECT * FROM moz_places_metadata_snapshots_groups`
+    );
+    for (let row of rows) {
+      let title = row.getResultByName("title");
+      Assert.strictEqual(title, null);
+      let builder_data = JSON.parse(row.getResultByName("builder_data"));
+      Assert.ok(
+        builder_data.fluentTitle || builder_data.title,
+        "Check there's always a title or fluentTitle in the data"
+      );
+    }
+  });
+});

toolkit/components/places/tests/migration/xpcshell.ini

@@ -8,6 +8,7 @@ support-files =
   places_v54.sqlite
   places_v61.sqlite
   places_v65.sqlite
+  places_v66.sqlite
 
 [test_current_from_downgraded.js]
 [test_current_from_outdated.js]
@@ -20,3 +21,4 @@ support-files =
 [test_current_from_v53.js]
 [test_current_from_v54.js]
 [test_current_from_v61.js]
+[test_current_from_v65.js]