Skip to content

Commit

Permalink
Bug 1384278 - Move metadata tables back into browser.db (#2974)
Browse files Browse the repository at this point in the history 
* Bug 1384278 - Move metadata tables back into browser.db

* Consolidated some DB migrations between v21 and v27 as suggested by rnewman.
  • Loading branch information
@justindarc
justindarc committed Jul 27, 2017
1 parent cd0375b commit 0d11676
Show file tree
Hide file tree
Showing 18 changed files with 80 additions and 161 deletions.
1 change: 0 additions & 1 deletion ClientTests/MockProfile.swift
View file
Original file line number Diff line number Diff line change
Expand Up @@ -115,7 +115,6 @@ open class MockProfile: Profile {
syncManager = MockSyncManager()
logins = MockLogins(files: files)
db = BrowserDB(filename: "mock.db", files: files)
db.attachDB(filename: "metadata.db", as: AttachedDatabaseMetadata)
places = SQLiteHistory(db: self.db, prefs: MockProfilePrefs())
recommendations = places
history = places
Expand Down
1 change: 0 additions & 1 deletion Providers/Profile.swift
View file
Original file line number Diff line number Diff line change
Expand Up @@ -244,7 +244,6 @@ open class BrowserProfile: Profile {
self.syncManager = BrowserSyncManager(profile: self)

self.db.prepareSchema()
self.db.attachDB(filename: "metadata.db", as: AttachedDatabaseMetadata)

if syncDelegate != nil {
// We almost certainly don't want to be accessing the logins.db when in an extension, so let's avoid
Expand Down
49 changes: 1 addition & 48 deletions Storage/SQL/BrowserDB.swift
View file
Original file line number Diff line number Diff line change
Expand Up @@ -29,31 +29,6 @@ public enum DatabaseOpResult {
case closed
}

class AttachedDB {
public let filename: String
public let schemaName: String

init(filename: String, schemaName: String) {
self.filename = filename
self.schemaName = schemaName
}

func attach(to browserDB: BrowserDB) -> NSError? {
let file = URL(fileURLWithPath: (try! browserDB.files.getAndEnsureDirectory())).appendingPathComponent(filename).path
let command = "ATTACH DATABASE '\(file)' AS '\(schemaName)'"
return browserDB.db.withConnection(SwiftData.Flags.readWriteCreate, synchronous: true) { connection in
return connection.executeChange(command, withArgs: [])
}
}

func detach(from browserDB: BrowserDB) -> NSError? {
let command = "DETACH DATABASE '\(schemaName)'"
return browserDB.db.withConnection(SwiftData.Flags.readWriteCreate, synchronous: true) { connection in
return connection.executeChange(command, withArgs: [])
}
}
}

// Version 1 - Basic history table.
// Version 2 - Added a visits table, refactored the history table to be a GenericTable.
// Version 3 - Added a favicons table.
Expand All @@ -69,8 +44,7 @@ open class BrowserDB {
fileprivate let filename: String
fileprivate let secretKey: String?
fileprivate let schemaTable: SchemaTable

fileprivate var attachedDBs: [AttachedDB]

fileprivate var initialized = [String]()

// SQLITE_MAX_VARIABLE_NUMBER = 999 by default. This controls how many ?s can
Expand All @@ -83,7 +57,6 @@ open class BrowserDB {
self.filename = filename
self.schemaTable = SchemaTable()
self.secretKey = secretKey
self.attachedDBs = []

let file = URL(fileURLWithPath: (try! files.getAndEnsureDirectory())).appendingPathComponent(filename).path
self.db = SwiftData(filename: file, key: secretKey, prevKey: nil)
Expand Down Expand Up @@ -319,15 +292,6 @@ open class BrowserDB {
return success ? .success : .failure
}

open func attachDB(filename: String, as schemaName: String) {
let attachedDB = AttachedDB(filename: filename, schemaName: schemaName)
if let err = attachedDB.attach(to: self) {
log.error("Error attaching DB. \(err.localizedDescription)")
} else {
self.attachedDBs.append(attachedDB)
}
}

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 {
Expand Down Expand Up @@ -474,17 +438,6 @@ extension BrowserDB {
let wasClosed = db.closed

db.reopenIfClosed()

// Need to re-attach any previously-attached DBs if the DB was closed
if wasClosed {
for attachedDB in attachedDBs {
log.debug("Re-attaching DB \(attachedDB.filename) as \(attachedDB.schemaName).")

if let err = attachedDB.attach(to: self) {
log.error("Error re-attaching DB. \(err.localizedDescription)")
}
}
}
}
}

Expand Down
126 changes: 60 additions & 66 deletions Storage/SQL/BrowserTable.swift
View file
Original file line number Diff line number Diff line change
Expand Up @@ -32,9 +32,6 @@ let TableQueuedTabs = "queue"
let TableActivityStreamBlocklist = "activity_stream_blocklist"
let TablePageMetadata = "page_metadata"
let TableHighlights = "highlights"
public let AttachedDatabaseMetadata = "metadataDB" // Added in v22
let AttachedTablePageMetadata = AttachedDatabaseMetadata + "." + TablePageMetadata // Added in v22
let AttachedTableHighlights = AttachedDatabaseMetadata + "." + TableHighlights // Added in v23

let ViewBookmarksBufferOnMirror = "view_bookmarksBuffer_on_mirror"
let ViewBookmarksBufferStructureOnMirror = "view_bookmarksBufferStructure_on_mirror"
Expand All @@ -58,8 +55,6 @@ let IndexBookmarksBufferStructureParentIdx = "idx_bookmarksBufferStructure_paren
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 v21
let AttachedIndexPageMetadataCacheKey = AttachedDatabaseMetadata + "." + IndexPageMetadataCacheKey // Added in v22
let AttachedIndexPageMetadataSiteURL = AttachedDatabaseMetadata + "." + IndexPageMetadataSiteURL // Added in v22

private let AllTables: [String] = [
TableDomains,
Expand All @@ -79,8 +74,8 @@ private let AllTables: [String] = [
TableQueuedTabs,

TableActivityStreamBlocklist,
AttachedTablePageMetadata,
AttachedTableHighlights,
TablePageMetadata,
TableHighlights,
TablePinnedTopSites
]

Expand Down Expand Up @@ -118,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 = 26 // Bug 1370824.
static let DefaultVersion = 27 // Bug 1384278.

// TableInfo fields.
var name: String { return "BROWSER" }
Expand Down Expand Up @@ -283,22 +278,8 @@ open class BrowserTable: Table {
"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 attachedHighlightsCreate =
"CREATE TABLE IF NOT EXISTS \(AttachedTableHighlights) (" +
let highlightsCreate =
"CREATE TABLE IF NOT EXISTS \(TableHighlights) (" +
"historyID INTEGER PRIMARY KEY," +
"cache_key LONGVARCHAR," +
"url TEXT," +
Expand All @@ -315,12 +296,6 @@ open class BrowserTable: Table {
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"

Expand Down Expand Up @@ -632,11 +607,11 @@ open class BrowserTable: Table {
widestFavicons,
historyIDsWithIcon,
iconForURL,
attachedPageMetadataCreate,
pageMetadataCreate,
pinnedTopSitesTableCreate,
attachedHighlightsCreate,
attachedIndexPageMetadataSiteURLCreate,
attachedIndexPageMetadataCacheKeyCreate,
highlightsCreate,
indexPageMetadataSiteURLCreate,
indexPageMetadataCacheKeyCreate,
self.queueTableCreate,
self.topSitesTableCreate,
self.localBookmarksView,
Expand Down Expand Up @@ -903,46 +878,65 @@ open class BrowserTable: Table {
}
}

if from < 22 && to >= 22 {
if !self.run(db, queries: [
"DROP TABLE IF EXISTS \(TablePageMetadata)",
attachedPageMetadataCreate,
attachedIndexPageMetadataCacheKeyCreate,
attachedIndexPageMetadataSiteURLCreate]) {
return false
}
}

if from < 23 && to >= 23 {
if !self.run(db, queries: [
attachedHighlightsCreate]) {
return false
}
}

if from < 24 && to >= 24 {
if !self.run(db, queries: [
// We can safely drop the highlights cache table since it gets cleared on every invalidate anyways.
"DROP TABLE IF EXISTS \(AttachedTableHighlights)",
attachedHighlightsCreate
]) {
return false
}
}
// Someone upgrading from v21 will get these tables anyway.
// So, there's no need to create them only to be dropped and
// re-created at v27 anyway.
// if from < 22 && to >= 22 {
// if !self.run(db, queries: [
// "DROP TABLE IF EXISTS \(TablePageMetadata)",
// pageMetadataCreate,
// indexPageMetadataCacheKeyCreate,
// indexPageMetadataSiteURLCreate]) {
// return false
// }
// }
//
// if from < 23 && to >= 23 {
// if !self.run(db, queries: [
// highlightsCreate]) {
// return false
// }
// }
//
// if from < 24 && to >= 24 {
// if !self.run(db, queries: [
// // We can safely drop the highlights cache table since it gets cleared on every invalidate anyways.
// "DROP TABLE IF EXISTS \(TableHighlights)",
// highlightsCreate
// ]) {
// return false
// }
// }

// Someone upgrading from v21 will get this table anyway.
// So, there's no need to create it only to be dropped and
// re-created at v26 anyway.
// if from < 25 && to >= 25 {
// if !self.run(db, queries: [
// pinnedTopSitesTableCreate
// ]) {
// return false
// }
// }

if from < 25 && to >= 25 {
if from < 26 && to >= 26 {
if !self.run(db, queries: [
// The old pin table was never released so we can safely drop
"DROP TABLE IF EXISTS \(TablePinnedTopSites)",
pinnedTopSitesTableCreate
]) {
return false
}
}

if from < 26 && to >= 26 {
if from < 27 && to >= 27 {
if !self.run(db, queries: [
// The old pin table was never released so we can safely drop
"DROP TABLE IF EXISTS \(TablePinnedTopSites)",
pinnedTopSitesTableCreate
"DROP TABLE IF EXISTS \(TablePageMetadata)",
"DROP TABLE IF EXISTS \(TableHighlights)",
pageMetadataCreate,
indexPageMetadataCacheKeyCreate,
indexPageMetadataSiteURLCreate,
highlightsCreate
]) {
return false
}
Expand Down
2 changes: 1 addition & 1 deletion Storage/SQL/SQLiteHistory.swift
View file
Original file line number Diff line number Diff line change
Expand Up @@ -126,7 +126,7 @@ open class SQLiteHistory {
}
}

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 (?)"
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 (?)"

extension SQLiteHistory: BrowserHistory {
public func removeSiteFromTopSites(_ site: Site) -> Success {
Expand Down
26 changes: 13 additions & 13 deletions Storage/SQL/SQLiteHistoryRecommendations.swift
View file
Original file line number Diff line number Diff line change
Expand Up @@ -13,17 +13,17 @@ extension SQLiteHistory: HistoryRecommendations {
public func getHighlights() -> Deferred<Maybe<Cursor<Site>>> {
let highlightsProjection = [
"historyID",
"\(AttachedTableHighlights).cache_key AS cache_key",
"\(TableHighlights).cache_key AS cache_key",
"url",
"\(AttachedTableHighlights).title AS title",
"\(TableHighlights).title AS title",
"guid",
"visitCount",
"visitDate",
"is_bookmarked"
]
let faviconsProjection = ["iconID", "iconURL", "iconType", "iconDate", "iconWidth"]
let metadataProjections = [
"\(AttachedTablePageMetadata).title AS metadata_title",
"\(TablePageMetadata).title AS metadata_title",
"media_url",
"type",
"description",
Expand All @@ -33,7 +33,7 @@ extension SQLiteHistory: HistoryRecommendations {
let allProjection = highlightsProjection + faviconsProjection + metadataProjections

let highlightsHistoryIDs =
"SELECT historyID FROM \(AttachedTableHighlights)"
"SELECT historyID FROM \(TableHighlights)"

// Search the history/favicon view with our limited set of highlight IDs
// to avoid doing a full table scan on history
Expand All @@ -42,10 +42,10 @@ extension SQLiteHistory: HistoryRecommendations {

let sql =
"SELECT \(allProjection.joined(separator: ",")) " +
"FROM \(AttachedTableHighlights) " +
"FROM \(TableHighlights) " +
"LEFT JOIN (\(faviconSearch)) AS f1 ON f1.id = historyID " +
"LEFT OUTER JOIN \(AttachedTablePageMetadata) ON " +
"\(AttachedTablePageMetadata).cache_key = \(AttachedTableHighlights).cache_key"
"LEFT OUTER JOIN \(TablePageMetadata) ON " +
"\(TablePageMetadata).cache_key = \(TableHighlights).cache_key"

return self.db.runQuery(sql, args: nil, factory: SQLiteHistory.iconHistoryMetadataColumnFactory)
}
Expand All @@ -59,7 +59,7 @@ extension SQLiteHistory: HistoryRecommendations {
}

public func clearHighlights() -> Success {
return self.db.run("DELETE FROM \(AttachedTableHighlights)", withArgs: nil)
return self.db.run("DELETE FROM \(TableHighlights)", withArgs: nil)
}

private func populateHighlights() -> Success {
Expand Down Expand Up @@ -98,7 +98,7 @@ extension SQLiteHistory: HistoryRecommendations {
]

return self.db.bulkInsert(
AttachedTableHighlights,
TableHighlights,
op: .InsertOrReplace,
columns: highlightsProjection,
values: values
Expand Down Expand Up @@ -131,10 +131,10 @@ extension SQLiteHistory: HistoryRecommendations {

let siteProjection = subQuerySiteProjection.replacingOccurrences(of: "siteTitle", with: "siteTitle AS title")
let highlightsQuery =
"SELECT \(siteProjection), iconID, iconURL, iconType, iconDate, iconWidth, \(AttachedTablePageMetadata).title AS metadata_title, media_url, type, description, provider_name " +
"SELECT \(siteProjection), iconID, iconURL, iconType, iconDate, iconWidth, \(TablePageMetadata).title AS metadata_title, media_url, type, description, provider_name " +
"FROM (\(bookmarkHighlights) ) " +
"LEFT JOIN \(ViewHistoryIDsWithWidestFavicons) ON \(ViewHistoryIDsWithWidestFavicons).id = historyID " +
"LEFT OUTER JOIN \(AttachedTablePageMetadata) ON \(AttachedTablePageMetadata).site_url = url " +
"LEFT OUTER JOIN \(TablePageMetadata) ON \(TablePageMetadata).site_url = url " +
"GROUP BY url"
let args = [fiveDaysAgo, fiveDaysAgo] as Args
return self.db.runQuery(highlightsQuery, args: args, factory: SQLiteHistory.iconHistoryMetadataColumnFactory)
Expand Down Expand Up @@ -183,8 +183,8 @@ extension SQLiteHistory: HistoryRecommendations {
removeMultipleDomainsSubquery +
" LEFT OUTER JOIN \(ViewHistoryIDsWithWidestFavicons) ON" +
" \(ViewHistoryIDsWithWidestFavicons).id = \(TableHistory).id" +
" LEFT OUTER JOIN \(AttachedTablePageMetadata) ON" +
" \(AttachedTablePageMetadata).site_url = \(TableHistory).url" +
" LEFT OUTER JOIN \(TablePageMetadata) ON" +
" \(TablePageMetadata).site_url = \(TableHistory).url" +
" WHERE visitCount <= 3 AND \(TableHistory).title NOT NULL AND \(TableHistory).title != '' AND is_bookmarked == 0 AND url NOT IN" +
" (SELECT url FROM \(TableActivityStreamBlocklist))" +
" AND \(TableHistory).domain_id NOT IN ("
Expand Down
Loading

0 comments on commit 0d11676

Please sign in to comment.