diff --git a/Frameworks/Database/ArticlesTable.swift b/Frameworks/Database/ArticlesTable.swift index 479b87ddf..bf728c7e0 100644 --- a/Frameworks/Database/ArticlesTable.swift +++ b/Frameworks/Database/ArticlesTable.swift @@ -21,8 +21,7 @@ final class ArticlesTable: DatabaseTable { private let authorsLookupTable: DatabaseLookupTable private let attachmentsLookupTable: DatabaseLookupTable private let tagsLookupTable: DatabaseLookupTable - private let articleCache = ArticleCache() - + // TODO: update articleCutoffDate as time passes and based on user preferences. private var articleCutoffDate = NSDate.rs_dateWithNumberOfDays(inThePast: 3 * 31)! private var maximumArticleCutoffDate = NSDate.rs_dateWithNumberOfDays(inThePast: 4 * 31)! @@ -50,23 +49,22 @@ final class ArticlesTable: DatabaseTable { let feedID = feed.feedID var articles = Set
() - queue.fetchSync { (database: FMDatabase!) -> Void in + queue.fetchSync { (database) in articles = self.fetchArticlesForFeedID(feedID, withLimits: true, database: database) } - return articleCache.uniquedArticles(articles) + return articles } func fetchArticlesAsync(_ feed: Feed, withLimits: Bool, _ resultBlock: @escaping ArticleResultBlock) { let feedID = feed.feedID - queue.fetch { (database: FMDatabase!) -> Void in + queue.fetch { (database) in - let fetchedArticles = self.fetchArticlesForFeedID(feedID, withLimits: withLimits, database: database) + let articles = self.fetchArticlesForFeedID(feedID, withLimits: withLimits, database: database) DispatchQueue.main.async { - let articles = self.articleCache.uniquedArticles(fetchedArticles) resultBlock(articles) } } @@ -86,6 +84,10 @@ final class ArticlesTable: DatabaseTable { return } + queue.update { (database) in + + + } // 1. Ensure statuses for all the parsedItems. // 2. Fetch all articles for the feed. // 3. For each parsedItem: @@ -516,48 +518,3 @@ private extension ArticlesTable { } } -// MARK: - - -private struct ArticleCache { - - // Main thread only — unlike the other object caches. - // The cache contains a given article only until all outside references are gone. - // Cache key is articleID. - - private let articlesMapTable: NSMapTable = NSMapTable.weakToWeakObjects() - - func uniquedArticles(_ articles: Set
) -> Set
{ - - var articlesToReturn = Set
() - - for article in articles { - let articleID = article.articleID - if let cachedArticle = cachedArticle(for: articleID) { - articlesToReturn.insert(cachedArticle) - } - else { - articlesToReturn.insert(article) - addToCache(article) - } - } - - // At this point, every Article must have an attached Status. - assert(articlesToReturn.eachHasAStatus()) - - return articlesToReturn - } - - private func cachedArticle(for articleID: String) -> Article? { - - return articlesMapTable.object(forKey: articleID as NSString) - } - - private func addToCache(_ article: Article) { - - articlesMapTable.setObject(article, forKey: article.articleID as NSString) - } -} - - - - diff --git a/Frameworks/Database/StatusesTable.swift b/Frameworks/Database/StatusesTable.swift index ca5243798..564701732 100644 --- a/Frameworks/Database/StatusesTable.swift +++ b/Frameworks/Database/StatusesTable.swift @@ -18,46 +18,22 @@ import Data final class StatusesTable: DatabaseTable { let name = DatabaseTableName.statuses - private let cache = DatabaseObjectCache() + private let cache = StatusCache() - // MARK: Fetching + func existingStatus(for articleID: String) -> ArticleStatus? { - func statusWithRow(_ row: FMResultSet) -> ArticleStatus? { - - guard let articleID = row.string(forColumn: DatabaseKey.articleID) else { - return nil - } - if let cachedStatus = cache[articleID] as? ArticleStatus { - return cachedStatus - } - - guard let dateArrived = row.date(forColumn: DatabaseKey.dateArrived) else { - return nil - } - - let articleStatus = ArticleStatus(articleID: articleID, dateArrived: dateArrived, row: row) - cache[articleID] = articleStatus - return articleStatus + cache.lock() + defer { cache.unlock() } + return cache[articleID] } - + // MARK: Creating/Updating - func ensureStatusesForArticles(_ articles: Set
, _ database: FMDatabase) { - - let articlesNeedingStatuses = articles.missingStatuses() - if articlesNeedingStatuses.isEmpty { - return - } - - let articleIDs = articlesNeedingStatuses.articleIDs() - ensureStatusesForArticleIDs(articleIDs, database) - - attachCachedStatuses(articlesNeedingStatuses) - assert(articles.eachHasAStatus()) - } - func ensureStatusesForArticleIDs(_ articleIDs: Set, _ database: FMDatabase) { + cache.lock() + defer { cache.unlock() } + // Check cache. let articleIDsMissingCachedStatus = articleIDsWithNoCachedStatus(articleIDs) if articleIDsMissingCachedStatus.isEmpty { @@ -75,6 +51,25 @@ final class StatusesTable: DatabaseTable { createAndSaveStatusesForArticleIDs(articleIDsNeedingStatus, database) } + // MARK: Marking + + func markArticleIDs(_ articleIDs: Set, _ statusKey: String, _ flag: Bool, _ database: FMDatabase) { + + cache.lock() + defer { cache.unlock() } + + // TODO: replace statuses in cache. + + updateRowsWithValue(NSNumber(value: flag), valueKey: statusKey, whereKey: DatabaseKey.articleID, matches: Array(articleIDs), database: database) + } +} + +// MARK: - Private + +private extension StatusesTable { + + // MARK: Fetching + func fetchStatusesForArticleIDs(_ articleIDs: Set, _ database: FMDatabase) -> [String: ArticleStatus] { // Does not create statuses. Checks cache first, then database only if needed. @@ -105,23 +100,22 @@ final class StatusesTable: DatabaseTable { return d } - // MARK: Marking - - func markArticleIDs(_ articleIDs: Set, _ statusKey: String, _ flag: Bool, _ database: FMDatabase) { - - updateRowsWithValue(NSNumber(value: flag), valueKey: statusKey, whereKey: DatabaseKey.articleID, matches: Array(articleIDs), database: database) - } -} + func statusWithRow(_ row: FMResultSet) -> ArticleStatus? { -private extension StatusesTable { - - func attachCachedStatuses(_ articles: Set
) { - - for article in articles { - if let cachedStatus = cache[article.articleID] as? ArticleStatus { - article.status = cachedStatus - } + guard let articleID = row.string(forColumn: DatabaseKey.articleID) else { + return nil } + if let cachedStatus = cache[articleID] as? ArticleStatus { + return cachedStatus + } + + guard let dateArrived = row.date(forColumn: DatabaseKey.dateArrived) else { + return nil + } + + let articleStatus = ArticleStatus(articleID: articleID, dateArrived: dateArrived, row: row) + cache[articleID] = articleStatus + return articleStatus } func articleIDsWithNoCachedStatus(_ articleIDs: Set) -> Set { @@ -137,12 +131,6 @@ private extension StatusesTable { insertRows(statusArray, insertType: .orIgnore, in: database) } - func cacheStatuses(_ statuses: Set) { - - let databaseObjects = statuses.map { $0 as DatabaseObject } - cache.addObjectsNotCached(databaseObjects) - } - func createAndSaveStatusesForArticles(_ articles: Set
, _ database: FMDatabase) { let articleIDs = Set(articles.map { $0.articleID }) @@ -154,7 +142,8 @@ private extension StatusesTable { let now = Date() let statuses = Set(articleIDs.map { ArticleStatus(articleID: $0, dateArrived: now) }) - cacheStatuses(statuses) + cache.add(statuses) + saveStatuses(statuses, database) } @@ -165,6 +154,66 @@ private extension StatusesTable { } let statuses = resultSet.mapToSet(statusWithRow) - cacheStatuses(statuses) + cache.add(statuses) } } + +private final class StatusCache { + + // Locking is left to the caller. Use the provided lock methods. + + private let lock = NSLock() + private var isLocked = false + var dictionary = [String: ArticleStatus]() + + func lock() { + + assert(!isLocked) + lock.lock() + isLocked = true + } + + func unlock() { + + assert(isLocked) + lock.unlock() + isLocked = false + } + + func add(_ statuses: Set) { + + // Replaces any cached statuses. + + assert(isLocked) + for status in statuses { + self[status.articleID] = status + } + } + + func statuses(for articleIDs: Set) -> [String: ArticleStatus] { + + assert(isLocked) + + var d = [String: ArticleStatus]() + for articleID in articleIDs { + if let cachedStatus = self[articleID] { + d[articleID] = cachedStatus + } + } + + return d + } + + subscript(_ articleID: String) -> ArticleStatus { + get { + assert(isLocked) + return self[articleID] + } + set { + assert(isLocked) + self[articleID] = newValue + } + } +} + +