Bug 11339824 - ATTACH metadata (#2533)

* Part 1 - Add ability to attach a database to a BrowserDB instance * Part 2 - Attach a DB called metadata.db to browser.db If the DB doesn't exist it will be created This cannot be done inside a transaction - this is a quirk of the `ATTACH` command, and because we reference the metadata table before we insert anything into it, through top sites and highlights, we cannot wait to create or attach the DB until we start to populate its tables, so we have to do it here. Also, as `logins.db` also implements `BrowserDB`, we cannot do it during the init of `browser.db` unless we want to attach `logins.db` too. Perhaps a follow up should be filed to separate Logins and Browser * Part 3 - Move all `page_metadata` table and index creation to create inside the attached DB. We are keeping all of the old SQL around for migration purposes. * Part 4 - update all tests to ensure they attach the metadata db before creating the `BrowserTable` Otherwise everything goes wrong when it cannot find the attached DB to create metadata
mozilla-mobile · Mar 22, 2017 · 8d08ece · 8d08ece
1 parent 2daecd5
commit 8d08ece
Show file tree

Hide file tree

Showing 18 changed files with 96 additions and 19 deletions.
diff --git a/ClientTests/MockProfile.swift b/ClientTests/MockProfile.swift
@@ -89,7 +89,9 @@ open class MockProfile: Profile {
     fileprivate var dbCreated = false
     lazy var db: BrowserDB = {
         self.dbCreated = true
-        return BrowserDB(filename: "mock.db", files: self.files)
+        let db = BrowserDB(filename: "mock.db", files: self.files)
+        db.attachDB(named: "metadata.db", as: AttachedDatabaseMetadata)
+        return db
     }()
 
     /**

diff --git a/Providers/Profile.swift b/Providers/Profile.swift
@@ -258,6 +258,7 @@ open class BrowserProfile: Profile {
         // Setup our database handles
         self.loginsDB = BrowserDB(filename: "logins.db", secretKey: BrowserProfile.loginsKey, files: files)
         self.db = BrowserDB(filename: "browser.db", files: files)
+        self.db.attachDB(named: "metadata.db", as: AttachedDatabaseMetadata)
 
         let notificationCenter = NotificationCenter.default
 

diff --git a/Storage/SQL/BrowserDB.swift b/Storage/SQL/BrowserDB.swift
@@ -265,6 +265,19 @@ open class BrowserDB {
         return success ? .success : .failure
     }
 
+    open func attachDB(named name: String, as alias: String) {
+        let file = URL(fileURLWithPath: (try! files.getAndEnsureDirectory())).appendingPathComponent(name).path
+        let command = "ATTACH DATABASE '\(file)' AS \(alias)"
+        let _ = db.withConnection(SwiftData.Flags.readWriteCreate, synchronous: true) { connection in
+            let err = connection.executeChange(command, withArgs: [])
+            if err != nil {
+                log.error("Error attaching DB. \(err?.localizedDescription)")
+                log.error("SQL was \(command)")
+            }
+            return err
+        }
+    }
+
     typealias IntCallback = (_ connection: SQLiteDBConnection, _ err: inout NSError?) -> Int
 
     func withConnection<T>(flags: SwiftData.Flags, err: inout NSError?, callback: @escaping (_ connection: SQLiteDBConnection, _ err: inout NSError?) -> T) -> T {

diff --git a/Storage/SQL/BrowserTable.swift b/Storage/SQL/BrowserTable.swift
@@ -30,6 +30,8 @@ let TableQueuedTabs = "queue"
 
 let TableActivityStreamBlocklist = "activity_stream_blocklist"
 let TablePageMetadata = "page_metadata"
+public let AttachedDatabaseMetadata = "metadataDB" // Added in v22
+let AttachedTablePageMetadata = AttachedDatabaseMetadata + "." + TablePageMetadata // Added in v22
 
 let ViewBookmarksBufferOnMirror = "view_bookmarksBuffer_on_mirror"
 let ViewBookmarksBufferStructureOnMirror = "view_bookmarksBufferStructure_on_mirror"
@@ -52,7 +54,9 @@ let IndexBookmarksLocalStructureParentIdx = "idx_bookmarksLocalStructure_parent_
 let IndexBookmarksBufferStructureParentIdx = "idx_bookmarksBufferStructure_parent_idx"   // Added in v12.
 let IndexBookmarksMirrorStructureChild = "idx_bookmarksMirrorStructure_child"            // Added in v14.
 let IndexPageMetadataCacheKey = "idx_page_metadata_cache_key_uniqueindex" // Added in v19
-let IndexPageMetadataSiteURL = "idx_page_metadata_site_url_uniqueindex" // Added in v19
+let IndexPageMetadataSiteURL = "idx_page_metadata_site_url_uniqueindex" // Added in v21
+let AttachedIndexPageMetadataCacheKey = AttachedDatabaseMetadata + "." + IndexPageMetadataCacheKey // Added in v22
+let AttachedIndexPageMetadataSiteURL = AttachedDatabaseMetadata + "." + IndexPageMetadataSiteURL // Added in v22
 
 private let AllTables: [String] = [
     TableDomains,
@@ -72,7 +76,7 @@ private let AllTables: [String] = [
     TableQueuedTabs,
 
     TableActivityStreamBlocklist,
-    TablePageMetadata,
+    AttachedTablePageMetadata,
 ]
 
 private let AllViews: [String] = [
@@ -109,7 +113,7 @@ private let log = Logger.syncLogger
  * We rely on SQLiteHistory having initialized the favicon table first.
  */
 open class BrowserTable: Table {
-    static let DefaultVersion = 21    // Bug 1253656.
+    static let DefaultVersion = 22    // Bug 1253656.
 
     // TableInfo fields.
     var name: String { return "BROWSER" }
@@ -262,14 +266,34 @@ open class BrowserTable: Table {
             "provider_name TEXT, " +
             "created_at DATETIME DEFAULT CURRENT_TIMESTAMP, " +
             "expired_at LONG" +
-        ") "
+    ") "
+
+    let attachedPageMetadataCreate =
+        "CREATE TABLE IF NOT EXISTS \(AttachedTablePageMetadata) (" +
+            "id INTEGER PRIMARY KEY, " +
+            "cache_key LONGVARCHAR UNIQUE, " +
+            "site_url TEXT, " +
+            "media_url LONGVARCHAR, " +
+            "title TEXT, " +
+            "type VARCHAR(32), " +
+            "description TEXT, " +
+            "provider_name TEXT, " +
+            "created_at DATETIME DEFAULT CURRENT_TIMESTAMP, " +
+            "expired_at LONG" +
+    ") "
 
     let indexPageMetadataCacheKeyCreate =
     "CREATE UNIQUE INDEX IF NOT EXISTS \(IndexPageMetadataCacheKey) ON page_metadata (cache_key)"
 
     let indexPageMetadataSiteURLCreate =
     "CREATE UNIQUE INDEX IF NOT EXISTS \(IndexPageMetadataSiteURL) ON page_metadata (site_url)"
 
+    let attachedIndexPageMetadataCacheKeyCreate =
+    "CREATE UNIQUE INDEX IF NOT EXISTS \(AttachedIndexPageMetadataCacheKey) ON \(TablePageMetadata) (cache_key)"
+
+    let attachedIndexPageMetadataSiteURLCreate =
+    "CREATE UNIQUE INDEX IF NOT EXISTS \(AttachedIndexPageMetadataSiteURL) ON \(TablePageMetadata) (site_url)"
+
     let iconColumns = ", faviconID INTEGER REFERENCES \(TableFavicons)(id) ON DELETE SET NULL"
     let mirrorColumns = ", is_overridden TINYINT NOT NULL DEFAULT 0"
 
@@ -581,9 +605,9 @@ open class BrowserTable: Table {
             widestFavicons,
             historyIDsWithIcon,
             iconForURL,
-            pageMetadataCreate,
-            indexPageMetadataCacheKeyCreate,
-            indexPageMetadataSiteURLCreate,
+            attachedPageMetadataCreate,
+            attachedIndexPageMetadataSiteURLCreate,
+            attachedIndexPageMetadataCacheKeyCreate,
             self.queueTableCreate,
             self.topSitesTableCreate,
             self.localBookmarksView,
@@ -828,8 +852,7 @@ open class BrowserTable: Table {
 
                 // Adds tables/indicies for metadata content
                 pageMetadataCreate,
-                indexPageMetadataCacheKeyCreate,
-                indexPageMetadataSiteURLCreate]) {
+                indexPageMetadataCacheKeyCreate]) {
                 return false
             }
         }
@@ -845,7 +868,18 @@ open class BrowserTable: Table {
         if from < 21 && to >= 21 {
             if !self.run(db, queries: [
                 "DROP VIEW IF EXISTS \(ViewHistoryVisits)",
-                self.historyVisitsView]) {
+                self.historyVisitsView,
+                indexPageMetadataSiteURLCreate]) {
+                return false
+            }
+        }
+
+        if from < 22 && to >= 22 {
+            if !self.run(db, queries: [
+                "DROP TABLE IF EXISTS \(TablePageMetadata)",
+                attachedPageMetadataCreate,
+                attachedIndexPageMetadataCacheKeyCreate,
+                attachedIndexPageMetadataSiteURLCreate]) {
                 return false
             }
         }

diff --git a/Storage/SQL/SQLiteHistory.swift b/Storage/SQL/SQLiteHistory.swift
@@ -126,7 +126,7 @@ open class SQLiteHistory {
     }
 }
 
-private let topSitesQuery = "SELECT \(TableCachedTopSites).*, \(TablePageMetadata).provider_name FROM \(TableCachedTopSites) LEFT OUTER JOIN \(TablePageMetadata) ON \(TableCachedTopSites).url = \(TablePageMetadata).site_url ORDER BY frecencies DESC LIMIT (?)"
+private let topSitesQuery = "SELECT \(TableCachedTopSites).*, \(AttachedTablePageMetadata).provider_name FROM \(TableCachedTopSites) LEFT OUTER JOIN \(AttachedTablePageMetadata) ON \(TableCachedTopSites).url = \(AttachedTablePageMetadata).site_url ORDER BY frecencies DESC LIMIT (?)"
 
 extension SQLiteHistory: BrowserHistory {
     public func removeSiteFromTopSites(_ site: Site) -> Success {

diff --git a/Storage/SQL/SQLiteHistoryRecommendations.swift b/Storage/SQL/SQLiteHistoryRecommendations.swift
@@ -80,10 +80,10 @@ extension SQLiteHistory: HistoryRecommendations {
 
         let siteProjection = subQuerySiteProjection.replacingOccurrences(of: "siteTitle", with: "siteTitle AS title")
         let highlightsQuery =
-            "SELECT \(siteProjection), iconID, iconURL, iconType, iconDate, iconWidth, \(TablePageMetadata).title AS metadata_title, media_url, type, description, provider_name " +
+            "SELECT \(siteProjection), iconID, iconURL, iconType, iconDate, iconWidth, \(AttachedTablePageMetadata).title AS metadata_title, media_url, type, description, provider_name " +
             "FROM ( \(nonRecentHistory) UNION ALL \(bookmarkHighlights) ) " +
             "LEFT JOIN \(ViewHistoryIDsWithWidestFavicons) ON \(ViewHistoryIDsWithWidestFavicons).id = historyID " +
-            "LEFT OUTER JOIN \(TablePageMetadata) ON \(TablePageMetadata).site_url = url " +
+            "LEFT OUTER JOIN \(AttachedTablePageMetadata) ON \(AttachedTablePageMetadata).site_url = url " +
             "GROUP BY url"
         let otherArgs = [threeDaysAgo, threeDaysAgo] as Args
         let args: Args = [thirtyMinutesAgo] + blacklistedHosts + otherArgs

diff --git a/Storage/SQL/SQLiteMetadata.swift b/Storage/SQL/SQLiteMetadata.swift
@@ -34,13 +34,13 @@ extension SQLiteMetadata: Metadata {
         }
 
         // Replace any matching cache_key entries if they exist
-        let selectUniqueCacheKey = "COALESCE((SELECT cache_key FROM \(TablePageMetadata) WHERE cache_key = ?), ?)"
+        let selectUniqueCacheKey = "COALESCE((SELECT cache_key FROM \(AttachedTablePageMetadata) WHERE cache_key = ?), ?)"
         let args: Args = [cacheKey, cacheKey, metadata.siteURL, metadata.mediaURL, metadata.title,
                           metadata.type, metadata.description, metadata.providerName,
                           expireAt]
 
         let insert =
-        "INSERT OR REPLACE INTO \(TablePageMetadata)" +
+        "INSERT OR REPLACE INTO \(AttachedTablePageMetadata)" +
         "(cache_key, site_url, media_url, title, type, description, provider_name, expired_at) " +
         "VALUES ( \(selectUniqueCacheKey), ?, ?, ?, ?, ?, ?, ?)"
 

diff --git a/StoragePerfTests/StoragePerfTests.swift b/StoragePerfTests/StoragePerfTests.swift
@@ -36,6 +36,7 @@ class TestSQLiteHistoryFrecencyPerf: XCTestCase {
     func testFrecencyPerf() {
         let files = MockFiles()
         let db = BrowserDB(filename: "browser.db", files: files)
+        db.attachDB(named: "metadata.db", as: AttachedDatabaseMetadata)
         let prefs = MockProfilePrefs()
         let history = SQLiteHistory(db: db, prefs: prefs)
 
@@ -57,6 +58,7 @@ class TestSQLiteHistoryTopSitesCachePref: XCTestCase {
     func testCachePerf() {
         let files = MockFiles()
         let db = BrowserDB(filename: "browser.db", files: files)
+        db.attachDB(named: "metadata.db", as: AttachedDatabaseMetadata)
         let prefs = MockProfilePrefs()
         let history = SQLiteHistory(db: db, prefs: prefs)
 

diff --git a/StorageTests/SyncCommandsTests.swift b/StorageTests/SyncCommandsTests.swift
@@ -35,6 +35,7 @@ class SyncCommandsTests: XCTestCase {
         } catch _ {
         }
         db = BrowserDB(filename: "browser.db", files: files)
+        db.attachDB(named: "metadata.db", as: AttachedDatabaseMetadata)
         // create clients
 
         let now = Date.now()

diff --git a/StorageTests/TestBrowserDB.swift b/StorageTests/TestBrowserDB.swift
@@ -58,6 +58,7 @@ class TestBrowserDB: XCTestCase {
 
     func testMovesDB() {
         let db = BrowserDB(filename: "foo.db", files: self.files)
+        db.attachDB(named: "metadata.db", as: AttachedDatabaseMetadata)
         XCTAssertTrue(db.createOrUpdate(BrowserTable()) == .success)
 
         db.run("CREATE TABLE foo (bar TEXT)").value      // Just so we have writes in the WAL.

diff --git a/StorageTests/TestFaviconsTable.swift b/StorageTests/TestFaviconsTable.swift
@@ -70,6 +70,7 @@ class TestFaviconsTable: XCTestCase {
     func testFaviconsTable() {
         let files = MockFiles()
         db = BrowserDB(filename: "test.db", files: files)
+        db.attachDB(named: "metadata.db", as: AttachedDatabaseMetadata)
         XCTAssertTrue(db.createOrUpdate(BrowserTable()) == .success)
         let f = FaviconsTable<Favicon>()
 

diff --git a/StorageTests/TestSQLiteBookmarks.swift b/StorageTests/TestSQLiteBookmarks.swift
@@ -10,6 +10,7 @@ import XCTest
 
 private func getBrowserDB(_ filename: String, files: FileAccessor) -> BrowserDB? {
     let db = BrowserDB(filename: filename, files: files)
+    db.attachDB(named: "metadata.db", as: AttachedDatabaseMetadata)
 
     // BrowserTable exists only to perform create/update etc. operations -- it's not
     // a queryable thing that needs to stick around.

diff --git a/StorageTests/TestSQLiteHistory.swift b/StorageTests/TestSQLiteHistory.swift
@@ -838,6 +838,7 @@ class TestSQLiteHistory: XCTestCase {
     // Test that our visit partitioning for frecency is correct.
     func testHistoryLocalAndRemoteVisits() {
         let db = BrowserDB(filename: "browser.db", files: files)
+        db.attachDB(named: "metadata.db", as: AttachedDatabaseMetadata)
         let prefs = MockProfilePrefs()
         let history = SQLiteHistory(db: db, prefs: prefs)
 
@@ -917,6 +918,7 @@ class TestSQLiteHistory: XCTestCase {
 
         for (version, table) in sources {
             let db = BrowserDB(filename: "browser-v\(version).db", files: files)
+            db.attachDB(named: "metadata.db", as: AttachedDatabaseMetadata)
             XCTAssertTrue(
                 db.runWithConnection { (conn, err) in
                     XCTAssertTrue(table.create(conn), "Creating browser table version \(version)")
@@ -931,6 +933,7 @@ class TestSQLiteHistory: XCTestCase {
 
     func testUpgradesWithData() {
         let db = BrowserDB(filename: "browser-v6-data.db", files: files)
+        db.attachDB(named: "metadata.db", as: AttachedDatabaseMetadata)
 
         XCTAssertTrue(db.createOrUpdate(BrowserTableV6()) == .success, "Creating browser table version 6")
 
@@ -960,6 +963,7 @@ class TestSQLiteHistory: XCTestCase {
 
     func testDomainUpgrade() {
         let db = BrowserDB(filename: "browser.db", files: files)
+        db.attachDB(named: "metadata.db", as: AttachedDatabaseMetadata)
         let prefs = MockProfilePrefs()
         let history = SQLiteHistory(db: db, prefs: prefs)
 
@@ -989,6 +993,7 @@ class TestSQLiteHistory: XCTestCase {
 
     func testDomains() {
         let db = BrowserDB(filename: "browser.db", files: files)
+        db.attachDB(named: "metadata.db", as: AttachedDatabaseMetadata)
         let prefs = MockProfilePrefs()
         let history = SQLiteHistory(db: db, prefs: prefs)
 
@@ -1026,6 +1031,7 @@ class TestSQLiteHistory: XCTestCase {
 
     func testHistoryIsSynced() {
         let db = BrowserDB(filename: "historysynced.db", files: files)
+        db.attachDB(named: "metadata.db", as: AttachedDatabaseMetadata)
         let prefs = MockProfilePrefs()
         let history = SQLiteHistory(db: db, prefs: prefs)
 
@@ -1043,6 +1049,7 @@ class TestSQLiteHistory: XCTestCase {
     // and then clears the database.
     func testHistoryTable() {
         let db = BrowserDB(filename: "browser.db", files: files)
+        db.attachDB(named: "metadata.db", as: AttachedDatabaseMetadata)
         let prefs = MockProfilePrefs()
         let history = SQLiteHistory(db: db, prefs: prefs)
         let bookmarks = SQLiteBookmarks(db: db)
@@ -1165,6 +1172,7 @@ class TestSQLiteHistory: XCTestCase {
 
     func testFaviconTable() {
         let db = BrowserDB(filename: "browser.db", files: files)
+        db.attachDB(named: "metadata.db", as: AttachedDatabaseMetadata)
         let prefs = MockProfilePrefs()
         let history = SQLiteHistory(db: db, prefs: prefs)
         let bookmarks = SQLiteBookmarks(db: db)
@@ -1228,6 +1236,7 @@ class TestSQLiteHistory: XCTestCase {
 
     func testTopSitesCache() {
         let db = BrowserDB(filename: "browser.db", files: files)
+        db.attachDB(named: "metadata.db", as: AttachedDatabaseMetadata)
         let prefs = MockProfilePrefs()
         let history = SQLiteHistory(db: db, prefs: prefs)
 
@@ -1315,6 +1324,7 @@ class TestSQLiteHistoryTransactionUpdate: XCTestCase {
     func testUpdateInTransaction() {
         let files = MockFiles()
         let db = BrowserDB(filename: "browser.db", files: files)
+        db.attachDB(named: "metadata.db", as: AttachedDatabaseMetadata)
         let prefs = MockProfilePrefs()
         let history = SQLiteHistory(db: db, prefs: prefs)
 
@@ -1334,6 +1344,7 @@ class TestSQLiteHistoryFilterSplitting: XCTestCase {
     let history: SQLiteHistory = {
         let files = MockFiles()
         let db = BrowserDB(filename: "browser.db", files: files)
+        db.attachDB(named: "metadata.db", as: AttachedDatabaseMetadata)
         let prefs = MockProfilePrefs()
         return SQLiteHistory(db: db, prefs: prefs)
     }()

diff --git a/StorageTests/TestSQLiteHistoryRecommendations.swift b/StorageTests/TestSQLiteHistoryRecommendations.swift
@@ -27,6 +27,7 @@ class TestSQLiteHistoryRecommendations: XCTestCase {
      */
     func testHistoryHighlights() {
         let db = BrowserDB(filename: "browser.db", files: files)
+        db.attachDB(named: "metadata.db", as: AttachedDatabaseMetadata)
         let prefs = MockProfilePrefs()
         let history = SQLiteHistory(db: db, prefs: prefs)
 
@@ -87,6 +88,7 @@ class TestSQLiteHistoryRecommendations: XCTestCase {
      */
     func testBookmarkHighlights() {
         let db = BrowserDB(filename: "browser.db", files: files)
+        db.attachDB(named: "metadata.db", as: AttachedDatabaseMetadata)
         let prefs = MockProfilePrefs()
         let history = SQLiteHistory(db: db, prefs: prefs)
         let bookmarks = SQLiteBookmarkBufferStorage(db: db)
@@ -142,6 +144,7 @@ class TestSQLiteHistoryRecommendations: XCTestCase {
      */
     func testBlacklistHighlights() {
         let db = BrowserDB(filename: "browser.db", files: files)
+        db.attachDB(named: "metadata.db", as: AttachedDatabaseMetadata)
         let prefs = MockProfilePrefs()
         let history = SQLiteHistory(db: db, prefs: prefs)
 
@@ -195,6 +198,7 @@ class TestSQLiteHistoryRecommendations: XCTestCase {
      */
     func testMostRecentUniqueDomainReturnedInHighlights() {
         let db = BrowserDB(filename: "browser.db", files: files)
+        db.attachDB(named: "metadata.db", as: AttachedDatabaseMetadata)
         let prefs = MockProfilePrefs()
         let history = SQLiteHistory(db: db, prefs: prefs)
 
@@ -227,6 +231,7 @@ class TestSQLiteHistoryRecommendations: XCTestCase {
 
     func testMetadataReturnedInHighlights() {
         let db = BrowserDB(filename: "browser.db", files: files)
+        db.attachDB(named: "metadata.db", as: AttachedDatabaseMetadata)
         let prefs = MockProfilePrefs()
         let history = SQLiteHistory(db: db, prefs: prefs)
 
@@ -273,7 +278,7 @@ class TestSQLiteHistoryRecommendations: XCTestCase {
             XCTAssertNotNil(highlight?.metadata?.mediaURL)
         }
 
-        db.run("DELETE FROM \(TablePageMetadata)").succeeded()
+        db.run("DELETE FROM \(AttachedTablePageMetadata)").succeeded()
         SDWebImageManager.shared().imageCache.clearDisk()
         SDWebImageManager.shared().imageCache.clearMemory()
     }
@@ -283,6 +288,7 @@ class TestSQLiteHistoryRecommendationsPerf: XCTestCase {
     func testRecommendationPref() {
         let files = MockFiles()
         let db = BrowserDB(filename: "browser.db", files: files)
+        db.attachDB(named: "metadata.db", as: AttachedDatabaseMetadata)
         let prefs = MockProfilePrefs()
         let history = SQLiteHistory(db: db, prefs: prefs)
         let bookmarks = SQLiteBookmarkBufferStorage(db: db)