From 5636ca0a015d9e1cd636fabac62929a45a64e489 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?F=C3=A1bio=20Oliveira?= Date: Tue, 9 Feb 2021 14:13:24 +0100 Subject: [PATCH 1/3] Add authors names to article search index Add authors to search table; Add authors names to ArticleSearchInfo and modified code to retrieve it correctly. Needs some cleaning up before being ready to merge. --- .../ArticlesDatabase/ArticlesDatabase.swift | 2 +- .../ArticlesDatabase/ArticlesTable.swift | 21 +++++++++++++++++-- .../ArticlesDatabase/SearchTable.swift | 8 ++++--- 3 files changed, 25 insertions(+), 6 deletions(-) diff --git a/ArticlesDatabase/Sources/ArticlesDatabase/ArticlesDatabase.swift b/ArticlesDatabase/Sources/ArticlesDatabase/ArticlesDatabase.swift index ddb576ba9..e2a62b8d0 100644 --- a/ArticlesDatabase/Sources/ArticlesDatabase/ArticlesDatabase.swift +++ b/ArticlesDatabase/Sources/ArticlesDatabase/ArticlesDatabase.swift @@ -332,7 +332,7 @@ private extension ArticlesDatabase { CREATE INDEX if not EXISTS statuses_starred_index on statuses (starred); - CREATE VIRTUAL TABLE if not EXISTS search using fts4(title, body); + CREATE VIRTUAL TABLE if not EXISTS search using fts4(title, body, authors); CREATE TRIGGER if not EXISTS articles_after_delete_trigger_delete_search_text after delete on articles begin delete from search where rowid = OLD.searchRowID; end; """ diff --git a/ArticlesDatabase/Sources/ArticlesDatabase/ArticlesTable.swift b/ArticlesDatabase/Sources/ArticlesDatabase/ArticlesTable.swift index 61c2845ef..0f9533c74 100644 --- a/ArticlesDatabase/Sources/ArticlesDatabase/ArticlesTable.swift +++ b/ArticlesDatabase/Sources/ArticlesDatabase/ArticlesTable.swift @@ -149,7 +149,23 @@ final class ArticlesTable: DatabaseTable { func fetchArticleSearchInfos(_ articleIDs: Set, in database: FMDatabase) -> Set? { let parameters = articleIDs.map { $0 as AnyObject } let placeholders = NSString.rs_SQLValueList(withPlaceholders: UInt(articleIDs.count))! - let sql = "select articleID, title, contentHTML, contentText, summary, searchRowID from articles where articleID in \(placeholders);"; + // Aggregating authors names in SQL using a subquery with GROUP_CONCAT + let sql = """ + SELECT + art.articleID, + art.title, + art.contentHTML, + art.contentText, + art.summary, + art.searchRowID, + (SELECT GROUP_CONCAT(name SEPARATOR ' ') + FROM authorLookup as autL + JOIN authors as aut ON autL.authorID = aut.authorID + WHERE art.articleID = autL.articleID + GROUP BY autl.articleID) as authors + FROM articles as art + WHERE articleID in \(placeholders); + """; if let resultSet = database.executeQuery(sql, withArgumentsIn: parameters) { return resultSet.mapToSet { (row) -> ArticleSearchInfo? in @@ -158,6 +174,7 @@ final class ArticlesTable: DatabaseTable { let contentHTML = row.string(forColumn: DatabaseKey.contentHTML) let contentText = row.string(forColumn: DatabaseKey.contentText) let summary = row.string(forColumn: DatabaseKey.summary) + let authors = row.string(forColumn: DatabaseKey.authors) let searchRowIDObject = row.object(forColumnName: DatabaseKey.searchRowID) var searchRowID: Int? = nil @@ -165,7 +182,7 @@ final class ArticlesTable: DatabaseTable { searchRowID = Int(row.longLongInt(forColumn: DatabaseKey.searchRowID)) } - return ArticleSearchInfo(articleID: articleID, title: title, contentHTML: contentHTML, contentText: contentText, summary: summary, searchRowID: searchRowID) + return ArticleSearchInfo(articleID: articleID, title: title, authorsNames: authors, contentHTML: contentHTML, contentText: contentText, summary: summary, searchRowID: searchRowID) } } return nil diff --git a/ArticlesDatabase/Sources/ArticlesDatabase/SearchTable.swift b/ArticlesDatabase/Sources/ArticlesDatabase/SearchTable.swift index 9f97a6d69..788ac6418 100644 --- a/ArticlesDatabase/Sources/ArticlesDatabase/SearchTable.swift +++ b/ArticlesDatabase/Sources/ArticlesDatabase/SearchTable.swift @@ -17,6 +17,7 @@ final class ArticleSearchInfo: Hashable { let articleID: String let title: String? + let authorsNames: String? let contentHTML: String? let contentText: String? let summary: String? @@ -37,9 +38,10 @@ final class ArticleSearchInfo: Hashable { return s.strippingHTML().collapsingWhitespace }() - init(articleID: String, title: String?, contentHTML: String?, contentText: String?, summary: String?, searchRowID: Int?) { + init(articleID: String, title: String?, authorsNames: String?, contentHTML: String?, contentText: String?, summary: String?, searchRowID: Int?) { self.articleID = articleID self.title = title + self.authorsNames = authorsNames self.contentHTML = contentHTML self.contentText = contentText self.summary = summary @@ -47,7 +49,7 @@ final class ArticleSearchInfo: Hashable { } convenience init(article: Article) { - self.init(articleID: article.articleID, title: article.title, contentHTML: article.contentHTML, contentText: article.contentText, summary: article.summary, searchRowID: nil) + self.init(articleID: article.articleID, title: article.title, authorsNames: article.authors?.map({ $0.name }).reduce("", { $0.appending("").appending($1 ?? "") }), contentHTML: article.contentHTML, contentText: article.contentText, summary: article.summary, searchRowID: nil) } // MARK: Hashable @@ -127,7 +129,7 @@ private extension SearchTable { } func insert(_ article: ArticleSearchInfo, _ database: FMDatabase) -> Int { - let rowDictionary: DatabaseDictionary = [DatabaseKey.body: article.bodyForIndex, DatabaseKey.title: article.title ?? ""] + let rowDictionary: DatabaseDictionary = [DatabaseKey.body: article.bodyForIndex, DatabaseKey.title: article.title ?? "", DatabaseKey.authors: article.authorsNames ?? ""] insertRow(rowDictionary, insertType: .normal, in: database) return Int(database.lastInsertRowId()) } From cf1be330447a5f20202cc70e32bcc52677056dec Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?F=C3=A1bio=20Oliveira?= Date: Wed, 10 Feb 2021 19:46:16 +0100 Subject: [PATCH 2/3] Add code to migrate database model Add code to migrate database model when `authors` column is not present in the index This recreates `search` table and resets the articles index Added missing or code in error --- .../Sources/ArticlesDatabase/ArticlesDatabase.swift | 5 +++++ .../Sources/ArticlesDatabase/ArticlesTable.swift | 4 ++-- ArticlesDatabase/Sources/ArticlesDatabase/SearchTable.swift | 4 ++-- 3 files changed, 9 insertions(+), 4 deletions(-) diff --git a/ArticlesDatabase/Sources/ArticlesDatabase/ArticlesDatabase.swift b/ArticlesDatabase/Sources/ArticlesDatabase/ArticlesDatabase.swift index e2a62b8d0..800cb0f4c 100644 --- a/ArticlesDatabase/Sources/ArticlesDatabase/ArticlesDatabase.swift +++ b/ArticlesDatabase/Sources/ArticlesDatabase/ArticlesDatabase.swift @@ -63,6 +63,7 @@ public final class ArticlesDatabase { } private let articlesTable: ArticlesTable + private let searchTable: SearchTable private let queue: DatabaseQueue private let operationQueue = MainThreadOperationQueue() private let retentionStyle: RetentionStyle @@ -71,6 +72,7 @@ public final class ArticlesDatabase { let queue = DatabaseQueue(databasePath: databaseFilePath) self.queue = queue self.articlesTable = ArticlesTable(name: DatabaseTableName.articles, accountID: accountID, queue: queue, retentionStyle: retentionStyle) + self.searchTable = SearchTable(queue: queue, articlesTable: self.articlesTable) self.retentionStyle = retentionStyle try! queue.runCreateStatements(ArticlesDatabase.tableCreationStatements) @@ -81,6 +83,9 @@ public final class ArticlesDatabase { } database.executeStatements("CREATE INDEX if not EXISTS articles_searchRowID on articles(searchRowID);") database.executeStatements("DROP TABLE if EXISTS tags;DROP INDEX if EXISTS tags_tagName_index;DROP INDEX if EXISTS articles_feedID_index;DROP INDEX if EXISTS statuses_read_index;DROP TABLE if EXISTS attachments;DROP TABLE if EXISTS attachmentsLookup;") + if !self.searchTable.containsColumn("authors", in: database) { + database.executeStatements("DROP TABLE if EXISTS search;CREATE VIRTUAL TABLE if not EXISTS search using fts4(title, body, authors);UPDATE articles SET searchRowID = null;") + } } DispatchQueue.main.async { diff --git a/ArticlesDatabase/Sources/ArticlesDatabase/ArticlesTable.swift b/ArticlesDatabase/Sources/ArticlesDatabase/ArticlesTable.swift index 0f9533c74..2a925612e 100644 --- a/ArticlesDatabase/Sources/ArticlesDatabase/ArticlesTable.swift +++ b/ArticlesDatabase/Sources/ArticlesDatabase/ArticlesTable.swift @@ -158,8 +158,8 @@ final class ArticlesTable: DatabaseTable { art.contentText, art.summary, art.searchRowID, - (SELECT GROUP_CONCAT(name SEPARATOR ' ') - FROM authorLookup as autL + (SELECT GROUP_CONCAT(name, ' ') + FROM authorsLookup as autL JOIN authors as aut ON autL.authorID = aut.authorID WHERE art.articleID = autL.articleID GROUP BY autl.articleID) as authors diff --git a/ArticlesDatabase/Sources/ArticlesDatabase/SearchTable.swift b/ArticlesDatabase/Sources/ArticlesDatabase/SearchTable.swift index 788ac6418..acb6a2e64 100644 --- a/ArticlesDatabase/Sources/ArticlesDatabase/SearchTable.swift +++ b/ArticlesDatabase/Sources/ArticlesDatabase/SearchTable.swift @@ -61,7 +61,7 @@ final class ArticleSearchInfo: Hashable { // MARK: Equatable static func == (lhs: ArticleSearchInfo, rhs: ArticleSearchInfo) -> Bool { - return lhs.articleID == rhs.articleID && lhs.title == rhs.title && lhs.contentHTML == rhs.contentHTML && lhs.contentText == rhs.contentText && lhs.summary == rhs.summary && lhs.searchRowID == rhs.searchRowID + return lhs.articleID == rhs.articleID && lhs.title == rhs.title && lhs.authorsNames == rhs.authorsNames && lhs.contentHTML == rhs.contentHTML && lhs.contentText == rhs.contentText && lhs.summary == rhs.summary && lhs.searchRowID == rhs.searchRowID } } @@ -203,7 +203,7 @@ private extension SearchTable { return nil } let placeholders = NSString.rs_SQLValueList(withPlaceholders: UInt(searchRowIDs.count))! - let sql = "select rowid, title, body from \(name) where rowid in \(placeholders);" + let sql = "select rowid, title, body, authors from \(name) where rowid in \(placeholders);" guard let resultSet = database.executeQuery(sql, withArgumentsIn: searchRowIDs) else { return nil } From 408e5d0ba092d1e992b0e68547df607c3bc9286e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?F=C3=A1bio=20Oliveira?= Date: Wed, 10 Feb 2021 20:25:21 +0100 Subject: [PATCH 3/3] Clean code - Extract statements to create `search` table so it can be reused during database migration - Rename `authors` as `authorsNames` for consistency - Extract code to improve readability --- .../ArticlesDatabase/ArticlesDatabase.swift | 14 ++++++++----- .../ArticlesDatabase/ArticlesTable.swift | 20 +++++++++---------- .../ArticlesDatabase/SearchTable.swift | 3 ++- 3 files changed, 21 insertions(+), 16 deletions(-) diff --git a/ArticlesDatabase/Sources/ArticlesDatabase/ArticlesDatabase.swift b/ArticlesDatabase/Sources/ArticlesDatabase/ArticlesDatabase.swift index 800cb0f4c..fa01b7a3d 100644 --- a/ArticlesDatabase/Sources/ArticlesDatabase/ArticlesDatabase.swift +++ b/ArticlesDatabase/Sources/ArticlesDatabase/ArticlesDatabase.swift @@ -76,6 +76,7 @@ public final class ArticlesDatabase { self.retentionStyle = retentionStyle try! queue.runCreateStatements(ArticlesDatabase.tableCreationStatements) + try! queue.runCreateStatements(ArticlesDatabase.searchTableCreationStatements) queue.runInDatabase { databaseResult in let database = databaseResult.database! if !self.articlesTable.containsColumn("searchRowID", in: database) { @@ -84,7 +85,8 @@ public final class ArticlesDatabase { database.executeStatements("CREATE INDEX if not EXISTS articles_searchRowID on articles(searchRowID);") database.executeStatements("DROP TABLE if EXISTS tags;DROP INDEX if EXISTS tags_tagName_index;DROP INDEX if EXISTS articles_feedID_index;DROP INDEX if EXISTS statuses_read_index;DROP TABLE if EXISTS attachments;DROP TABLE if EXISTS attachmentsLookup;") if !self.searchTable.containsColumn("authors", in: database) { - database.executeStatements("DROP TABLE if EXISTS search;CREATE VIRTUAL TABLE if not EXISTS search using fts4(title, body, authors);UPDATE articles SET searchRowID = null;") + database.executeStatements("DROP TABLE if EXISTS search;UPDATE articles SET searchRowID = null;") + database.executeStatements(ArticlesDatabase.searchTableCreationStatements) } } @@ -336,12 +338,14 @@ private extension ArticlesDatabase { CREATE INDEX if not EXISTS articles_feedID_datePublished_articleID on articles (feedID, datePublished, articleID); CREATE INDEX if not EXISTS statuses_starred_index on statuses (starred); - - CREATE VIRTUAL TABLE if not EXISTS search using fts4(title, body, authors); - - CREATE TRIGGER if not EXISTS articles_after_delete_trigger_delete_search_text after delete on articles begin delete from search where rowid = OLD.searchRowID; end; """ + static let searchTableCreationStatements = """ + CREATE VIRTUAL TABLE if not EXISTS search using fts4(title, body, authors); + + CREATE TRIGGER if not EXISTS articles_after_delete_trigger_delete_search_text after delete on articles begin delete from search where rowid = OLD.searchRowID; end; + """ + func todayCutoffDate() -> Date { // 24 hours previous. This is used by the Today smart feed, which should not actually empty out at midnight. return Date(timeIntervalSinceNow: -(60 * 60 * 24)) // This does not need to be more precise. diff --git a/ArticlesDatabase/Sources/ArticlesDatabase/ArticlesTable.swift b/ArticlesDatabase/Sources/ArticlesDatabase/ArticlesTable.swift index 2a925612e..099f626a1 100644 --- a/ArticlesDatabase/Sources/ArticlesDatabase/ArticlesTable.swift +++ b/ArticlesDatabase/Sources/ArticlesDatabase/ArticlesTable.swift @@ -145,12 +145,8 @@ final class ArticlesTable: DatabaseTable { } // MARK: - Fetching Articles for Indexer - - func fetchArticleSearchInfos(_ articleIDs: Set, in database: FMDatabase) -> Set? { - let parameters = articleIDs.map { $0 as AnyObject } - let placeholders = NSString.rs_SQLValueList(withPlaceholders: UInt(articleIDs.count))! - // Aggregating authors names in SQL using a subquery with GROUP_CONCAT - let sql = """ + private func articleSearchInfosQuery(with placeholders: String) -> String { + return """ SELECT art.articleID, art.title, @@ -165,16 +161,20 @@ final class ArticlesTable: DatabaseTable { GROUP BY autl.articleID) as authors FROM articles as art WHERE articleID in \(placeholders); - """; + """ + } - if let resultSet = database.executeQuery(sql, withArgumentsIn: parameters) { + func fetchArticleSearchInfos(_ articleIDs: Set, in database: FMDatabase) -> Set? { + let parameters = articleIDs.map { $0 as AnyObject } + let placeholders = NSString.rs_SQLValueList(withPlaceholders: UInt(articleIDs.count))! + if let resultSet = database.executeQuery(self.articleSearchInfosQuery(with: placeholders), withArgumentsIn: parameters) { return resultSet.mapToSet { (row) -> ArticleSearchInfo? in let articleID = row.string(forColumn: DatabaseKey.articleID)! let title = row.string(forColumn: DatabaseKey.title) let contentHTML = row.string(forColumn: DatabaseKey.contentHTML) let contentText = row.string(forColumn: DatabaseKey.contentText) let summary = row.string(forColumn: DatabaseKey.summary) - let authors = row.string(forColumn: DatabaseKey.authors) + let authorsNames = row.string(forColumn: DatabaseKey.authors) let searchRowIDObject = row.object(forColumnName: DatabaseKey.searchRowID) var searchRowID: Int? = nil @@ -182,7 +182,7 @@ final class ArticlesTable: DatabaseTable { searchRowID = Int(row.longLongInt(forColumn: DatabaseKey.searchRowID)) } - return ArticleSearchInfo(articleID: articleID, title: title, authorsNames: authors, contentHTML: contentHTML, contentText: contentText, summary: summary, searchRowID: searchRowID) + return ArticleSearchInfo(articleID: articleID, title: title, authorsNames: authorsNames, contentHTML: contentHTML, contentText: contentText, summary: summary, searchRowID: searchRowID) } } return nil diff --git a/ArticlesDatabase/Sources/ArticlesDatabase/SearchTable.swift b/ArticlesDatabase/Sources/ArticlesDatabase/SearchTable.swift index acb6a2e64..7aca4ae5a 100644 --- a/ArticlesDatabase/Sources/ArticlesDatabase/SearchTable.swift +++ b/ArticlesDatabase/Sources/ArticlesDatabase/SearchTable.swift @@ -49,7 +49,8 @@ final class ArticleSearchInfo: Hashable { } convenience init(article: Article) { - self.init(articleID: article.articleID, title: article.title, authorsNames: article.authors?.map({ $0.name }).reduce("", { $0.appending("").appending($1 ?? "") }), contentHTML: article.contentHTML, contentText: article.contentText, summary: article.summary, searchRowID: nil) + let authorsNames = article.authors?.map({ $0.name }).reduce("", { $0.appending("").appending($1 ?? "") }) + self.init(articleID: article.articleID, title: article.title, authorsNames: authorsNames, contentHTML: article.contentHTML, contentText: article.contentText, summary: article.summary, searchRowID: nil) } // MARK: Hashable