From aeaeac61fceb6413885bfd0efc5c753358123e38 Mon Sep 17 00:00:00 2001 From: Brent Simmons Date: Fri, 27 Dec 2019 22:47:02 -0800 Subject: [PATCH] Get rid of DatabaseArticle entirely. Cache Article objects. This will make fetches faster *and* save memory. --- .../project.pbxproj | 12 +- .../ArticlesDatabase/ArticlesTable.swift | 105 +++++++----------- .../ArticlesDatabase/DatabaseArticle.swift | 44 -------- .../Extensions/Article+Database.swift | 37 +++++- 4 files changed, 76 insertions(+), 122 deletions(-) delete mode 100644 Frameworks/ArticlesDatabase/DatabaseArticle.swift diff --git a/Frameworks/ArticlesDatabase/ArticlesDatabase.xcodeproj/project.pbxproj b/Frameworks/ArticlesDatabase/ArticlesDatabase.xcodeproj/project.pbxproj index f3241666f..a93a2be9d 100644 --- a/Frameworks/ArticlesDatabase/ArticlesDatabase.xcodeproj/project.pbxproj +++ b/Frameworks/ArticlesDatabase/ArticlesDatabase.xcodeproj/project.pbxproj @@ -11,7 +11,6 @@ 841D4D742106B59F00DD04E6 /* Articles.framework in Frameworks */ = {isa = PBXBuildFile; fileRef = 841D4D732106B59F00DD04E6 /* Articles.framework */; }; 84288A001F6A3C4400395871 /* DatabaseObject+Database.swift in Sources */ = {isa = PBXBuildFile; fileRef = 842889FF1F6A3C4400395871 /* DatabaseObject+Database.swift */; }; 84288A021F6A3D8000395871 /* RelatedObjectsMap+Database.swift in Sources */ = {isa = PBXBuildFile; fileRef = 84288A011F6A3D8000395871 /* RelatedObjectsMap+Database.swift */; }; - 843577161F744FC800F460AE /* DatabaseArticle.swift in Sources */ = {isa = PBXBuildFile; fileRef = 843577151F744FC800F460AE /* DatabaseArticle.swift */; }; 843577221F749C6200F460AE /* ArticleChangesTests.swift in Sources */ = {isa = PBXBuildFile; fileRef = 843577211F749C6200F460AE /* ArticleChangesTests.swift */; }; 843702C31F70D15D00B18807 /* ParsedArticle+Database.swift in Sources */ = {isa = PBXBuildFile; fileRef = 843702C21F70D15D00B18807 /* ParsedArticle+Database.swift */; }; 843CB9961F34174100EE6581 /* Author+Database.swift in Sources */ = {isa = PBXBuildFile; fileRef = 84F20F901F1810DD00D8E682 /* Author+Database.swift */; }; @@ -115,7 +114,6 @@ 841D4D732106B59F00DD04E6 /* Articles.framework */ = {isa = PBXFileReference; explicitFileType = wrapper.framework; path = Articles.framework; sourceTree = BUILT_PRODUCTS_DIR; }; 842889FF1F6A3C4400395871 /* DatabaseObject+Database.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = "DatabaseObject+Database.swift"; sourceTree = ""; }; 84288A011F6A3D8000395871 /* RelatedObjectsMap+Database.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = "RelatedObjectsMap+Database.swift"; sourceTree = ""; }; - 843577151F744FC800F460AE /* DatabaseArticle.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = DatabaseArticle.swift; sourceTree = ""; }; 843577211F749C6200F460AE /* ArticleChangesTests.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = ArticleChangesTests.swift; sourceTree = ""; }; 843702C21F70D15D00B18807 /* ParsedArticle+Database.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; name = "ParsedArticle+Database.swift"; path = "Extensions/ParsedArticle+Database.swift"; sourceTree = ""; }; 844BEE371F0AB3AA004AB7CD /* ArticlesDatabase.framework */ = {isa = PBXFileReference; explicitFileType = wrapper.framework; includeInIndex = 0; path = ArticlesDatabase.framework; sourceTree = BUILT_PRODUCTS_DIR; }; @@ -176,7 +174,6 @@ 845580661F0AEBCD003CCFA1 /* Constants.swift */, 84E156EB1F0AB80E00F8CC05 /* ArticlesTable.swift */, 8477ACBB2221E76F00DF7F37 /* SearchTable.swift */, - 843577151F744FC800F460AE /* DatabaseArticle.swift */, 84E156ED1F0AB81400F8CC05 /* StatusesTable.swift */, 84F20F8E1F180D8700D8E682 /* AuthorsTable.swift */, 8461462A1F0AC44100870CB3 /* Extensions */, @@ -350,14 +347,14 @@ TargetAttributes = { 844BEE361F0AB3AA004AB7CD = { CreatedOnToolsVersion = 8.3.2; - DevelopmentTeam = SHJK2V3AJG; + DevelopmentTeam = M8L2WTLA8W; LastSwiftMigration = 0830; - ProvisioningStyle = Automatic; + ProvisioningStyle = Manual; }; 844BEE3F1F0AB3AB004AB7CD = { CreatedOnToolsVersion = 8.3.2; - DevelopmentTeam = SHJK2V3AJG; - ProvisioningStyle = Automatic; + DevelopmentTeam = M8L2WTLA8W; + ProvisioningStyle = Manual; }; }; }; @@ -522,7 +519,6 @@ 84F20F8F1F180D8700D8E682 /* AuthorsTable.swift in Sources */, 84288A001F6A3C4400395871 /* DatabaseObject+Database.swift in Sources */, 8477ACBC2221E76F00DF7F37 /* SearchTable.swift in Sources */, - 843577161F744FC800F460AE /* DatabaseArticle.swift in Sources */, 843702C31F70D15D00B18807 /* ParsedArticle+Database.swift in Sources */, 84E156EC1F0AB80E00F8CC05 /* ArticlesTable.swift in Sources */, 84E156EE1F0AB81400F8CC05 /* StatusesTable.swift in Sources */, diff --git a/Frameworks/ArticlesDatabase/ArticlesTable.swift b/Frameworks/ArticlesDatabase/ArticlesTable.swift index 877b1b0b6..81ebe47f2 100644 --- a/Frameworks/ArticlesDatabase/ArticlesTable.swift +++ b/Frameworks/ArticlesDatabase/ArticlesTable.swift @@ -19,7 +19,7 @@ final class ArticlesTable: DatabaseTable { private let queue: DatabaseQueue private let statusesTable: StatusesTable private let authorsLookupTable: DatabaseLookupTable - private var databaseArticlesCache = [String: DatabaseArticle]() + private var articlesCache = [String: Article]() private lazy var searchTable: SearchTable = { return SearchTable(queue: queue, articlesTable: self) @@ -449,7 +449,7 @@ final class ArticlesTable: DatabaseTable { func emptyCaches() { queue.runInDatabase { _ in - self.databaseArticlesCache = [String: DatabaseArticle]() + self.articlesCache = [String: Article]() } } @@ -527,86 +527,57 @@ private extension ArticlesTable { } func articlesWithResultSet(_ resultSet: FMResultSet, _ database: FMDatabase) -> Set
{ - // 1. Create DatabaseArticles without related objects. - // 2. Then fetch the related objects, given the set of articleIDs. - // 3. Then create set of Articles with DatabaseArticles and related objects and return it. + var cachedArticles = Set
() + var fetchedArticles = Set
() - // 1. Create databaseArticles (intermediate representations). + while resultSet.next() { - let databaseArticles = makeDatabaseArticles(with: resultSet) - if databaseArticles.isEmpty { - return Set
() - } - - let articleIDs = databaseArticles.articleIDs() - - // 2. Fetch related objects. - - let authorsMap = authorsLookupTable.fetchRelatedObjects(for: articleIDs, in: database) - - // 3. Create articles with related objects. - - let articles = databaseArticles.map { (databaseArticle) -> Article in - return articleWithDatabaseArticle(databaseArticle, authorsMap) - } - - return Set(articles) - } - - func articleWithDatabaseArticle(_ databaseArticle: DatabaseArticle, _ authorsMap: RelatedObjectsMap?) -> Article { - - let articleID = databaseArticle.articleID - let authors = authorsMap?.authors(for: articleID) - - return Article(databaseArticle: databaseArticle, accountID: accountID, authors: authors) - } - - func makeDatabaseArticles(with resultSet: FMResultSet) -> Set { - let articles = resultSet.mapToSet { (row) -> DatabaseArticle? in - - guard let articleID = row.string(forColumn: DatabaseKey.articleID) else { + guard let articleID = resultSet.string(forColumn: DatabaseKey.articleID) else { assertionFailure("Expected articleID.") - return nil + continue } // Articles are removed from the cache when they’re updated. // See saveUpdatedArticles. - if let databaseArticle = databaseArticlesCache[articleID] { - return databaseArticle + if let article = articlesCache[articleID] { + cachedArticles.insert(article) + continue } // The resultSet is a result of a JOIN query with the statuses table, // so we can get the statuses at the same time and avoid additional database lookups. guard let status = statusesTable.statusWithRow(resultSet, articleID: articleID) else { assertionFailure("Expected status.") - return nil - } - guard let webFeedID = row.string(forColumn: DatabaseKey.feedID) else { - assertionFailure("Expected feedID.") - return nil - } - guard let uniqueID = row.string(forColumn: DatabaseKey.uniqueID) else { - assertionFailure("Expected uniqueID.") - return nil + continue } - let title = row.string(forColumn: DatabaseKey.title) - let contentHTML = row.string(forColumn: DatabaseKey.contentHTML) - let contentText = row.string(forColumn: DatabaseKey.contentText) - let url = row.string(forColumn: DatabaseKey.url) - let externalURL = row.string(forColumn: DatabaseKey.externalURL) - let summary = row.string(forColumn: DatabaseKey.summary) - let imageURL = row.string(forColumn: DatabaseKey.imageURL) - let bannerImageURL = row.string(forColumn: DatabaseKey.bannerImageURL) - let datePublished = row.date(forColumn: DatabaseKey.datePublished) - let dateModified = row.date(forColumn: DatabaseKey.dateModified) + guard let article = Article(accountID: accountID, row: resultSet, status: status) else { + continue + } + fetchedArticles.insert(article) + } + resultSet.close() - let databaseArticle = DatabaseArticle(articleID: articleID, webFeedID: webFeedID, uniqueID: uniqueID, title: title, contentHTML: contentHTML, contentText: contentText, url: url, externalURL: externalURL, summary: summary, imageURL: imageURL, bannerImageURL: bannerImageURL, datePublished: datePublished, dateModified: dateModified, status: status) - databaseArticlesCache[articleID] = databaseArticle - return databaseArticle + if fetchedArticles.isEmpty { + return cachedArticles } - return articles + // Fetch authors for non-cached articles. (Articles from the cache already have authors.) + let fetchedArticleIDs = fetchedArticles.articleIDs() + let authorsMap = authorsLookupTable.fetchRelatedObjects(for: fetchedArticleIDs, in: database) + let articlesWithFetchedAuthors = fetchedArticles.map { (article) -> Article in + if let authors = authorsMap?.authors(for: article.articleID) { + return article.byAdding(authors) + } + return article + } + + // Add fetchedArticles to cache, now that they have attached authors. + for article in articlesWithFetchedAuthors { + articlesCache[article.articleID] = article + } + + return cachedArticles.union(articlesWithFetchedAuthors) } func fetchArticlesWithWhereClause(_ database: FMDatabase, whereClause: String, parameters: [AnyObject], withLimits: Bool) -> Set
{ @@ -872,7 +843,7 @@ private extension ArticlesTable { func saveUpdatedArticles(_ updatedArticles: Set
, _ fetchedArticles: [String: Article], _ database: FMDatabase) { - removeArticlesFromDatabaseArticlesCache(updatedArticles) + removeArticlesFromArticlesCache(updatedArticles) saveUpdatedRelatedObjects(updatedArticles, fetchedArticles, database) for updatedArticle in updatedArticles { @@ -897,10 +868,10 @@ private extension ArticlesTable { updateRowsWithDictionary(changesDictionary, whereKey: DatabaseKey.articleID, matches: updatedArticle.articleID, database: database) } - func removeArticlesFromDatabaseArticlesCache(_ updatedArticles: Set
) { + func removeArticlesFromArticlesCache(_ updatedArticles: Set
) { let articleIDs = updatedArticles.articleIDs() for articleID in articleIDs { - databaseArticlesCache[articleID] = nil + articlesCache[articleID] = nil } } diff --git a/Frameworks/ArticlesDatabase/DatabaseArticle.swift b/Frameworks/ArticlesDatabase/DatabaseArticle.swift deleted file mode 100644 index d029f7113..000000000 --- a/Frameworks/ArticlesDatabase/DatabaseArticle.swift +++ /dev/null @@ -1,44 +0,0 @@ -// -// DatabaseArticle.swift -// NetNewsWire -// -// Created by Brent Simmons on 9/21/17. -// Copyright © 2017 Ranchero Software. All rights reserved. -// - -import Foundation -import Articles - -// Intermediate representation of an Article. Doesn’t include related objects. -// Used by ArticlesTable as part of fetching articles. - -struct DatabaseArticle: Hashable { - - let articleID: String - let webFeedID: String - let uniqueID: String - let title: String? - let contentHTML: String? - let contentText: String? - let url: String? - let externalURL: String? - let summary: String? - let imageURL: String? - let bannerImageURL: String? - let datePublished: Date? - let dateModified: Date? - let status: ArticleStatus - - // MARK: - Hashable - - public func hash(into hasher: inout Hasher) { - hasher.combine(articleID) - } -} - -extension Set where Element == DatabaseArticle { - - func articleIDs() -> Set { - return Set(map { $0.articleID }) - } -} diff --git a/Frameworks/ArticlesDatabase/Extensions/Article+Database.swift b/Frameworks/ArticlesDatabase/Extensions/Article+Database.swift index 058922dfb..d0e8add42 100644 --- a/Frameworks/ArticlesDatabase/Extensions/Article+Database.swift +++ b/Frameworks/ArticlesDatabase/Extensions/Article+Database.swift @@ -13,8 +13,32 @@ import RSParser extension Article { - init(databaseArticle: DatabaseArticle, accountID: String, authors: Set?) { - self.init(accountID: accountID, articleID: databaseArticle.articleID, webFeedID: databaseArticle.webFeedID, uniqueID: databaseArticle.uniqueID, title: databaseArticle.title, contentHTML: databaseArticle.contentHTML, contentText: databaseArticle.contentText, url: databaseArticle.url, externalURL: databaseArticle.externalURL, summary: databaseArticle.summary, imageURL: databaseArticle.imageURL, bannerImageURL: databaseArticle.bannerImageURL, datePublished: databaseArticle.datePublished, dateModified: databaseArticle.dateModified, authors: authors, status: databaseArticle.status) + init?(accountID: String, row: FMResultSet, status: ArticleStatus) { + guard let articleID = row.string(forColumn: DatabaseKey.articleID) else { + assertionFailure("Expected articleID.") + return nil + } + guard let webFeedID = row.string(forColumn: DatabaseKey.feedID) else { + assertionFailure("Expected feedID.") + return nil + } + guard let uniqueID = row.string(forColumn: DatabaseKey.uniqueID) else { + assertionFailure("Expected uniqueID.") + return nil + } + + let title = row.string(forColumn: DatabaseKey.title) + let contentHTML = row.string(forColumn: DatabaseKey.contentHTML) + let contentText = row.string(forColumn: DatabaseKey.contentText) + let url = row.string(forColumn: DatabaseKey.url) + let externalURL = row.string(forColumn: DatabaseKey.externalURL) + let summary = row.string(forColumn: DatabaseKey.summary) + let imageURL = row.string(forColumn: DatabaseKey.imageURL) + let bannerImageURL = row.string(forColumn: DatabaseKey.bannerImageURL) + let datePublished = row.date(forColumn: DatabaseKey.datePublished) + let dateModified = row.date(forColumn: DatabaseKey.dateModified) + + self.init(accountID: accountID, articleID: articleID, webFeedID: webFeedID, uniqueID: uniqueID, title: title, contentHTML: contentHTML, contentText: contentText, url: url, externalURL: externalURL, summary: summary, imageURL: imageURL, bannerImageURL: bannerImageURL, datePublished: datePublished, dateModified: dateModified, authors: nil, status: status) } init(parsedItem: ParsedItem, maximumDateAllowed: Date, accountID: String, webFeedID: String, status: ArticleStatus) { @@ -42,7 +66,14 @@ extension Article { dictionary[key] = self[keyPath: comparisonKeyPath] ?? "" } } - + + func byAdding(_ authors: Set) -> Article { + if authors.isEmpty { + return self + } + return Article(accountID: self.accountID, articleID: self.articleID, webFeedID: self.webFeedID, uniqueID: self.uniqueID, title: self.title, contentHTML: self.contentHTML, contentText: self.contentText, url: self.url, externalURL: self.externalURL, summary: self.summary, imageURL: self.imageURL, bannerImageURL: self.bannerImageURL, datePublished: self.datePublished, dateModified: self.dateModified, authors: authors, status: self.status) + } + func changesFrom(_ existingArticle: Article) -> DatabaseDictionary? { if self == existingArticle { return nil