diff --git a/Frameworks/Data/Article.swift b/Frameworks/Data/Article.swift index cf2c7cffd..5d587b5fe 100644 --- a/Frameworks/Data/Article.swift +++ b/Frameworks/Data/Article.swift @@ -29,7 +29,6 @@ public struct Article: Hashable { public let attachments: Set? public let hashValue: Int - public init(accountID: String, articleID: String?, feedID: String, uniqueID: String, title: String?, contentHTML: String?, contentText: String?, url: String?, externalURL: String?, summary: String?, imageURL: String?, bannerImageURL: String?, datePublished: Date?, dateModified: Date?, authors: Set?, tags: Set?, attachments: Set?, accountInfo: AccountInfo?) { self.accountID = accountID @@ -68,31 +67,32 @@ public struct Article: Hashable { public extension Article { + // MARK: Main-thread only accessors. + public var logicalDatePublished: Date? { get { + assert(Thread.isMainThread) return (datePublished ?? dateModified) ?? status?.dateArrived } } - // MARK: Main-thread only accessors. - public var account: Account? { get { - assert(Thread.isMainThread, "article.account is main-thread-only.") + assert(Thread.isMainThread) return Account.account(with: accountID) } } public var feed: Feed? { get { - assert(Thread.isMainThread, "article.feed is main-thread-only.") + assert(Thread.isMainThread) return account?.existingFeed(with: feedID) } } public var status: ArticleStatus? { get { - assert(Thread.isMainThread, "article.status is main-thread-only.") + assert(Thread.isMainThread) return account?.status(with: articleID) } } diff --git a/Frameworks/Database/ArticlesTable.swift b/Frameworks/Database/ArticlesTable.swift index bf728c7e0..c41d85b9a 100644 --- a/Frameworks/Database/ArticlesTable.swift +++ b/Frameworks/Database/ArticlesTable.swift @@ -15,9 +15,9 @@ import Data final class ArticlesTable: DatabaseTable { let name: String - private weak var account: Account? + private let accountID: String private let queue: RSDatabaseQueue - private let statusesTable = StatusesTable() + private let statusesTable: StatusesTable private let authorsLookupTable: DatabaseLookupTable private let attachmentsLookupTable: DatabaseLookupTable private let tagsLookupTable: DatabaseLookupTable @@ -26,12 +26,14 @@ final class ArticlesTable: DatabaseTable { private var articleCutoffDate = NSDate.rs_dateWithNumberOfDays(inThePast: 3 * 31)! private var maximumArticleCutoffDate = NSDate.rs_dateWithNumberOfDays(inThePast: 4 * 31)! - init(name: String, account: Account, queue: RSDatabaseQueue) { + init(name: String, accountID: String, queue: RSDatabaseQueue) { self.name = name - self.account = account + self.accountID = accountID self.queue = queue + let statusesTable = StatusesTable(queue: queue) + let authorsTable = AuthorsTable(name: DatabaseTableName.authors) self.authorsLookupTable = DatabaseLookupTable(name: DatabaseTableName.authorsLookup, objectIDKey: DatabaseKey.articleID, relatedObjectIDKey: DatabaseKey.authorID, relatedTable: authorsTable, relationshipName: RelationshipName.authors) @@ -84,12 +86,37 @@ final class ArticlesTable: DatabaseTable { return } - queue.update { (database) in + // 1. Ensure statuses for all the parsedItems. + // 2. Ignore parsedItems that are userDeleted || (!starred and really old) + // 3. Fetch all articles for the feed. + // 4. Create Articles with parsedItems. + // 5. + let feedID = feed.feedID + let parsedItemArticleIDs = Set(parsedFeed.items.map { $0.databaseIdentifierWithFeed(feed) }) + let parsedItemsDictionary = parsedFeed.itemsDictionary(with: feed) + statusesTable.ensureStatusesForArticleIDs(parsedItemArticleIDs) { + + let filteredParsedItems = self.filterParsedItems(parsedItemsDictionary) + if filteredParsedItems.isEmpty { + completion() + return + } + + queue.fetch{ (database) in + + let fetchedArticles = self.fetchArticlesForFeedID(feedID, withLimits: false, database: database) + + let incomingArticles = Article.articlesWithParsedItems(parsedFeed.items, accountID, feedID) + + + + } + } - // 1. Ensure statuses for all the parsedItems. - // 2. Fetch all articles for the feed. + + // 3. For each parsedItem: // - if userDeleted || (!starred && status.dateArrived < cutoff), then ignore // - if matches existing article, then update database with changes between the two diff --git a/Frameworks/Database/Extensions/Article+Database.swift b/Frameworks/Database/Extensions/Article+Database.swift index 57926d942..9191f2cab 100644 --- a/Frameworks/Database/Extensions/Article+Database.swift +++ b/Frameworks/Database/Extensions/Article+Database.swift @@ -13,7 +13,7 @@ import RSParser extension Article { - convenience init?(row: FMResultSet, account: Account) { + convenience init?(row: FMResultSet, authors: Set, attachments: Set, tags: Set, accountID: String) { guard let feedID = row.string(forColumn: DatabaseKey.feedID) else { return nil @@ -35,12 +35,10 @@ extension Article { let dateModified = row.date(forColumn: DatabaseKey.dateModified) let accountInfo: [String: Any]? = nil // TODO - // authors, tags, and attachments are fetched from related tables, after init. - - self.init(account: account, articleID: articleID, feedID: feedID, uniqueID: uniqueID, title: title, contentHTML: contentHTML, contentText: contentText, url: url, externalURL: externalURL, summary: summary, imageURL: imageURL, bannerImageURL: bannerImageURL, datePublished: datePublished, dateModified: dateModified, authors: nil, tags: nil, attachments: nil, accountInfo: accountInfo) + self.init(account: account, articleID: articleID, feedID: feedID, uniqueID: uniqueID, title: title, contentHTML: contentHTML, contentText: contentText, url: url, externalURL: externalURL, summary: summary, imageURL: imageURL, bannerImageURL: bannerImageURL, datePublished: datePublished, dateModified: dateModified, authors: authors, tags: tags, attachments: attachments, accountInfo: accountInfo) } - convenience init(parsedItem: ParsedItem, feedID: String, account: Account) { + convenience init(parsedItem: ParsedItem, accountID: String, feedID: String) { let authors = Author.authorsWithParsedAuthors(parsedItem.authors) let attachments = Attachment.attachmentsWithParsedAttachments(parsedItem.attachments) @@ -73,92 +71,97 @@ extension Article { return d.copy() as! NSDictionary } + static func articlesWithParsedItems(_ parsedItems: [ParsedItem], _ accountID: String, _ feedID: String) -> Set
{ + + return parsedItems.map{ Article(parsedItem: $0, accountID: accountID, feedID: feedID) } + } + // MARK: Updating with ParsedItem - func updateTagsWithParsedTags(_ parsedTags: [String]?) -> Bool { - - // Return true if there's a change. - - let currentTags = tags - - if parsedTags == nil && currentTags == nil { - return false - } - if parsedTags != nil && currentTags == nil { - tags = Set(parsedItemTags!) - return true - } - if parsedTags == nil && currentTags != nil { - tags = nil - return true - } - let parsedTagSet = Set(parsedTags!) - if parsedTagSet == tags! { - return false - } - tags = parsedTagSet - return true - } - - func updateAttachmentsWithParsedAttachments(_ parsedAttachments: [ParsedAttachment]?) -> Bool { - - // Return true if there's a change. - - let currentAttachments = attachments - let updatedAttachments = Attachment.attachmentsWithParsedAttachments(parsedAttachments) - - if updatedAttachments == nil && currentAttachments == nil { - return false - } - if updatedAttachments != nil && currentAttachments == nil { - attachments = updatedAttachments - return true - } - if updatedAttachments == nil && currentAttachments != nil { - attachments = nil - return true - } - - guard let currentAttachments = currentAttachments, let updatedAttachments = updatedAttachments else { - assertionFailure("currentAttachments and updatedAttachments must both be non-nil.") - return false - } - if currentAttachments != updatedAttachments { - attachments = updatedAttachments - return true - } - return false - } - - func updateAuthorsWithParsedAuthors(_ parsedAuthors: [ParsedAuthor]?) -> Bool { - - // Return true if there's a change. - - let currentAuthors = authors - let updatedAuthors = Author.authorsWithParsedAuthors(parsedAuthors) - - if updatedAuthors == nil && currentAuthors == nil { - return false - } - if updatedAuthors != nil && currentAuthors == nil { - authors = updatedAuthors - return true - } - if updatedAuthors == nil && currentAuthors != nil { - authors = nil - return true - } - - guard let currentAuthors = currentAuthors, let updatedAuthors = updatedAuthors else { - assertionFailure("currentAuthors and updatedAuthors must both be non-nil.") - return false - } - if currentAuthors != updatedAuthors { - authors = updatedAuthors - return true - } - return false - } +// func updateTagsWithParsedTags(_ parsedTags: [String]?) -> Bool { +// +// // Return true if there's a change. +// +// let currentTags = tags +// +// if parsedTags == nil && currentTags == nil { +// return false +// } +// if parsedTags != nil && currentTags == nil { +// tags = Set(parsedItemTags!) +// return true +// } +// if parsedTags == nil && currentTags != nil { +// tags = nil +// return true +// } +// let parsedTagSet = Set(parsedTags!) +// if parsedTagSet == tags! { +// return false +// } +// tags = parsedTagSet +// return true +// } +// +// func updateAttachmentsWithParsedAttachments(_ parsedAttachments: [ParsedAttachment]?) -> Bool { +// +// // Return true if there's a change. +// +// let currentAttachments = attachments +// let updatedAttachments = Attachment.attachmentsWithParsedAttachments(parsedAttachments) +// +// if updatedAttachments == nil && currentAttachments == nil { +// return false +// } +// if updatedAttachments != nil && currentAttachments == nil { +// attachments = updatedAttachments +// return true +// } +// if updatedAttachments == nil && currentAttachments != nil { +// attachments = nil +// return true +// } +// +// guard let currentAttachments = currentAttachments, let updatedAttachments = updatedAttachments else { +// assertionFailure("currentAttachments and updatedAttachments must both be non-nil.") +// return false +// } +// if currentAttachments != updatedAttachments { +// attachments = updatedAttachments +// return true +// } +// return false +// } +// +// func updateAuthorsWithParsedAuthors(_ parsedAuthors: [ParsedAuthor]?) -> Bool { +// +// // Return true if there's a change. +// +// let currentAuthors = authors +// let updatedAuthors = Author.authorsWithParsedAuthors(parsedAuthors) +// +// if updatedAuthors == nil && currentAuthors == nil { +// return false +// } +// if updatedAuthors != nil && currentAuthors == nil { +// authors = updatedAuthors +// return true +// } +// if updatedAuthors == nil && currentAuthors != nil { +// authors = nil +// return true +// } +// +// guard let currentAuthors = currentAuthors, let updatedAuthors = updatedAuthors else { +// assertionFailure("currentAuthors and updatedAuthors must both be non-nil.") +// return false +// } +// if currentAuthors != updatedAuthors { +// authors = updatedAuthors +// return true +// } +// return false +// } } extension Article: DatabaseObject { diff --git a/Frameworks/Database/Extensions/Author+Database.swift b/Frameworks/Database/Extensions/Author+Database.swift index 7840002f0..326ffa009 100644 --- a/Frameworks/Database/Extensions/Author+Database.swift +++ b/Frameworks/Database/Extensions/Author+Database.swift @@ -35,7 +35,7 @@ extension Author { } let authors = parsedAuthors.flatMap { Author(parsedAuthor: $0) } - return authors.isEmpty ? nil : Set(authors) + return authors.isEmpty ? nil : authors } } diff --git a/Frameworks/Database/StatusesTable.swift b/Frameworks/Database/StatusesTable.swift index 564701732..2efbcd925 100644 --- a/Frameworks/Database/StatusesTable.swift +++ b/Frameworks/Database/StatusesTable.swift @@ -19,45 +19,53 @@ final class StatusesTable: DatabaseTable { let name = DatabaseTableName.statuses private let cache = StatusCache() + private let queue: RSDatabaseQueue + + init(queue: RSDatabaseQueue) { + + self.queue = queue + } + + func cachedStatus(for articleID: String) -> ArticleStatus? { - func existingStatus(for articleID: String) -> ArticleStatus? { - - cache.lock() - defer { cache.unlock() } + assert(Thread.isMainThread) + assert(cache[articleID] != nil) return cache[articleID] } // MARK: Creating/Updating - func ensureStatusesForArticleIDs(_ articleIDs: Set, _ database: FMDatabase) { - - cache.lock() - defer { cache.unlock() } - + func ensureStatusesForArticleIDs(_ articleIDs: Set, _ completion: @escaping RSVoidCompletionBlock) { + + // Adds them to the cache if not cached. + + assert(Thread.isMainThread) + // Check cache. let articleIDsMissingCachedStatus = articleIDsWithNoCachedStatus(articleIDs) if articleIDsMissingCachedStatus.isEmpty { + completion() return } - + // Check database. - fetchAndCacheStatusesForArticleIDs(articleIDsMissingCachedStatus, database) - let articleIDsNeedingStatus = articleIDsWithNoCachedStatus(articleIDs) - if articleIDsNeedingStatus.isEmpty { - return + fetchAndCacheStatusesForArticleIDs(articleIDsMissingCachedStatus) { + + let articleIDsNeedingStatus = articleIDsWithNoCachedStatus(articleIDs) + if articleIDsNeedingStatus.isEmpty { + completion() + return + } + + // Create new statuses. + createAndSaveStatusesForArticleIDs(articleIDsNeedingStatus, completion) } - - // Create new statuses. - 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) @@ -70,54 +78,21 @@ 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. - - var d = [String: ArticleStatus]() - var articleIDsMissingCachedStatus = Set() - - for articleID in articleIDs { - if let cachedStatus = cache[articleID] as? ArticleStatus { - d[articleID] = cachedStatus - } - else { - articleIDsMissingCachedStatus.insert(articleID) - } - } - - if articleIDsMissingCachedStatus.isEmpty { - return d - } - - fetchAndCacheStatusesForArticleIDs(articleIDsMissingCachedStatus, database) - for articleID in articleIDsMissingCachedStatus { - if let cachedStatus = cache[articleID] as? ArticleStatus { - d[articleID] = cachedStatus - } - } - - return d - } - 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 } + // MARK: Cache + func articleIDsWithNoCachedStatus(_ articleIDs: Set) -> Set { return Set(articleIDs.filter { cache[$0] == nil }) @@ -125,92 +100,80 @@ private extension StatusesTable { // MARK: Creating - func saveStatuses(_ statuses: Set, _ database: FMDatabase) { + func saveStatuses(_ statuses: Set) { - let statusArray = statuses.map { $0.databaseDictionary() } - insertRows(statusArray, insertType: .orIgnore, in: database) + queue.update { (database) in + let statusArray = statuses.map { $0.databaseDictionary() } + insertRows(statusArray, insertType: .orIgnore, in: database) + } } - func createAndSaveStatusesForArticles(_ articles: Set
, _ database: FMDatabase) { - - let articleIDs = Set(articles.map { $0.articleID }) - createAndSaveStatusesForArticleIDs(articleIDs, database) - } - - func createAndSaveStatusesForArticleIDs(_ articleIDs: Set, _ database: FMDatabase) { + func createAndSaveStatusesForArticleIDs(_ articleIDs: Set, _ completion: @escaping RSVoidCompletionBlock) { + assert(Thread.isMainThread) + let now = Date() let statuses = Set(articleIDs.map { ArticleStatus(articleID: $0, dateArrived: now) }) - - cache.add(statuses) - - saveStatuses(statuses, database) + cache.addIfNotCached(statuses) + + // No need to wait for database to return before calling completion, + // since the new statuses have been cached at this point. + + completion() + saveStatuses(statuses) } - func fetchAndCacheStatusesForArticleIDs(_ articleIDs: Set, _ database: FMDatabase) { - - guard let resultSet = selectRowsWhere(key: DatabaseKey.articleID, inValues: Array(articleIDs), in: database) else { - return + func fetchAndCacheStatusesForArticleIDs(_ articleIDs: Set, _ completion: @escaping RSVoidCompletionBlock) { + + queue.fetch { (database) in + guard let resultSet = selectRowsWhere(key: DatabaseKey.articleID, inValues: Array(articleIDs), in: database) else { + completion() + return + } + + let statuses = resultSet.mapToSet(statusWithRow) + + DispatchQueue.main.async { + cache.addIfNotCached(statuses) + completion() + } } - - let statuses = resultSet.mapToSet(statusWithRow) - cache.add(statuses) } } private final class StatusCache { - // Locking is left to the caller. Use the provided lock methods. + // Main thread only. - 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 + func addIfNotCached(_ statuses: Set) { + + // Does not replace already cached statuses. + + for status in statuses { + let articleID = status.articleID + if let _ = self[articleID] { + continue } + self[articleID] = status } - - return d } - + subscript(_ articleID: String) -> ArticleStatus { get { - assert(isLocked) return self[articleID] } set { - assert(isLocked) self[articleID] = newValue } }