Merge branch 'master' into accent-color-experimental

This commit is contained in:
Maurice Parker 2020-04-11 12:31:26 -05:00
commit de5c087fa6
18 changed files with 375 additions and 127 deletions

View File

@ -715,7 +715,7 @@ public final class Account: DisplayNameProvider, UnreadCountProvider, Container,
webFeedDictionariesNeedUpdate = true webFeedDictionariesNeedUpdate = true
} }
func update(_ webFeed: WebFeed, with parsedFeed: ParsedFeed, _ completion: @escaping DatabaseCompletionBlock) { func update(_ webFeed: WebFeed, with parsedFeed: ParsedFeed, _ completion: @escaping UpdateArticlesCompletionBlock) {
// Used only by an On My Mac or iCloud account. // Used only by an On My Mac or iCloud account.
precondition(Thread.isMainThread) precondition(Thread.isMainThread)
precondition(type == .onMyMac || type == .cloudKit) precondition(type == .onMyMac || type == .cloudKit)
@ -723,14 +723,14 @@ public final class Account: DisplayNameProvider, UnreadCountProvider, Container,
webFeed.takeSettings(from: parsedFeed) webFeed.takeSettings(from: parsedFeed)
let parsedItems = parsedFeed.items let parsedItems = parsedFeed.items
guard !parsedItems.isEmpty else { guard !parsedItems.isEmpty else {
completion(nil) completion(.success(NewAndUpdatedArticles()))
return return
} }
update(webFeed.webFeedID, with: parsedItems, completion: completion) update(webFeed.webFeedID, with: parsedItems, completion: completion)
} }
func update(_ webFeedID: String, with parsedItems: Set<ParsedItem>, completion: @escaping DatabaseCompletionBlock) { func update(_ webFeedID: String, with parsedItems: Set<ParsedItem>, completion: @escaping UpdateArticlesCompletionBlock) {
// Used only by an On My Mac or iCloud account. // Used only by an On My Mac or iCloud account.
precondition(Thread.isMainThread) precondition(Thread.isMainThread)
precondition(type == .onMyMac || type == .cloudKit) precondition(type == .onMyMac || type == .cloudKit)
@ -739,9 +739,9 @@ public final class Account: DisplayNameProvider, UnreadCountProvider, Container,
switch updateArticlesResult { switch updateArticlesResult {
case .success(let newAndUpdatedArticles): case .success(let newAndUpdatedArticles):
self.sendNotificationAbout(newAndUpdatedArticles) self.sendNotificationAbout(newAndUpdatedArticles)
completion(nil) completion(.success(newAndUpdatedArticles))
case .failure(let databaseError): case .failure(let databaseError):
completion(databaseError) completion(.failure(databaseError))
} }
} }
} }
@ -800,39 +800,45 @@ public final class Account: DisplayNameProvider, UnreadCountProvider, Container,
/// Mark articleIDs statuses based on statusKey and flag. /// Mark articleIDs statuses based on statusKey and flag.
/// Will create statuses in the database and in memory as needed. Sends a .StatusesDidChange notification. /// Will create statuses in the database and in memory as needed. Sends a .StatusesDidChange notification.
func mark(articleIDs: Set<String>, statusKey: ArticleStatus.Key, flag: Bool, completion: DatabaseCompletionBlock? = nil) { /// Returns a set of new article statuses.
func markAndFetchNew(articleIDs: Set<String>, statusKey: ArticleStatus.Key, flag: Bool, completion: ArticleIDsCompletionBlock? = nil) {
guard !articleIDs.isEmpty else { guard !articleIDs.isEmpty else {
completion?(nil) completion?(.success(Set<String>()))
return
}
database.mark(articleIDs: articleIDs, statusKey: statusKey, flag: flag) { error in
if let error = error {
completion?(error)
return return
} }
database.markAndFetchNew(articleIDs: articleIDs, statusKey: statusKey, flag: flag) { result in
switch result {
case .success(let newArticleStatusIDs):
self.noteStatusesForArticleIDsDidChange(articleIDs) self.noteStatusesForArticleIDsDidChange(articleIDs)
completion?(nil) completion?(.success(newArticleStatusIDs))
case .failure(let databaseError):
completion?(.failure(databaseError))
}
} }
} }
/// Mark articleIDs as read. Will create statuses in the database and in memory as needed. Sends a .StatusesDidChange notification. /// Mark articleIDs as read. Will create statuses in the database and in memory as needed. Sends a .StatusesDidChange notification.
func markAsRead(_ articleIDs: Set<String>, completion: DatabaseCompletionBlock? = nil) { /// Returns a set of new article statuses.
mark(articleIDs: articleIDs, statusKey: .read, flag: true, completion: completion) func markAsRead(_ articleIDs: Set<String>, completion: ArticleIDsCompletionBlock? = nil) {
markAndFetchNew(articleIDs: articleIDs, statusKey: .read, flag: true, completion: completion)
} }
/// Mark articleIDs as unread. Will create statuses in the database and in memory as needed. Sends a .StatusesDidChange notification. /// Mark articleIDs as unread. Will create statuses in the database and in memory as needed. Sends a .StatusesDidChange notification.
func markAsUnread(_ articleIDs: Set<String>, completion: DatabaseCompletionBlock? = nil) { /// Returns a set of new article statuses.
mark(articleIDs: articleIDs, statusKey: .read, flag: false, completion: completion) func markAsUnread(_ articleIDs: Set<String>, completion: ArticleIDsCompletionBlock? = nil) {
markAndFetchNew(articleIDs: articleIDs, statusKey: .read, flag: false, completion: completion)
} }
/// Mark articleIDs as starred. Will create statuses in the database and in memory as needed. Sends a .StatusesDidChange notification. /// Mark articleIDs as starred. Will create statuses in the database and in memory as needed. Sends a .StatusesDidChange notification.
func markAsStarred(_ articleIDs: Set<String>, completion: DatabaseCompletionBlock? = nil) { /// Returns a set of new article statuses.
mark(articleIDs: articleIDs, statusKey: .starred, flag: true, completion: completion) func markAsStarred(_ articleIDs: Set<String>, completion: ArticleIDsCompletionBlock? = nil) {
markAndFetchNew(articleIDs: articleIDs, statusKey: .starred, flag: true, completion: completion)
} }
/// Mark articleIDs as unstarred. Will create statuses in the database and in memory as needed. Sends a .StatusesDidChange notification. /// Mark articleIDs as unstarred. Will create statuses in the database and in memory as needed. Sends a .StatusesDidChange notification.
func markAsUnstarred(_ articleIDs: Set<String>, completion: DatabaseCompletionBlock? = nil) { /// Returns a set of new article statuses.
mark(articleIDs: articleIDs, statusKey: .starred, flag: false, completion: completion) func markAsUnstarred(_ articleIDs: Set<String>, completion: ArticleIDsCompletionBlock? = nil) {
markAndFetchNew(articleIDs: articleIDs, statusKey: .starred, flag: false, completion: completion)
} }
/// Empty caches that can reasonably be emptied. Call when the app goes in the background, for instance. /// Empty caches that can reasonably be emptied. Call when the app goes in the background, for instance.
@ -887,7 +893,7 @@ public final class Account: DisplayNameProvider, UnreadCountProvider, Container,
public func debugDropConditionalGetInfo() { public func debugDropConditionalGetInfo() {
#if DEBUG #if DEBUG
flattenedWebFeeds().forEach{ $0.debugDropConditionalGetInfo() } flattenedWebFeeds().forEach{ $0.dropConditionalGetInfo() }
#endif #endif
} }

View File

@ -13,6 +13,7 @@ import SyncDatabase
import RSCore import RSCore
import RSParser import RSParser
import Articles import Articles
import ArticlesDatabase
import RSWeb import RSWeb
public enum CloudKitAccountDelegateError: String, Error { public enum CloudKitAccountDelegateError: String, Error {
@ -34,7 +35,13 @@ final class CloudKitAccountDelegate: AccountDelegate {
private let accountZone: CloudKitAccountZone private let accountZone: CloudKitAccountZone
private let articlesZone: CloudKitArticlesZone private let articlesZone: CloudKitArticlesZone
private let refresher = LocalAccountRefresher() weak var account: Account?
private lazy var refresher: LocalAccountRefresher = {
let refresher = LocalAccountRefresher()
refresher.delegate = self
return refresher
}()
let behaviors: AccountBehaviors = [] let behaviors: AccountBehaviors = []
let isOPMLImportInProgress = false let isOPMLImportInProgress = false
@ -76,6 +83,7 @@ final class CloudKitAccountDelegate: AccountDelegate {
completion(.success(())) completion(.success(()))
return return
} }
refreshAll(for: account, downloadFeeds: true, completion: completion) refreshAll(for: account, downloadFeeds: true, completion: completion)
} }
@ -90,12 +98,12 @@ final class CloudKitAccountDelegate: AccountDelegate {
return return
} }
let starredArticleIDs = syncStatuses.filter({ $0.key == .starred && $0.flag == true }).map({ $0.articleID }) let articleIDs = syncStatuses.map({ $0.articleID })
account.fetchArticlesAsync(.articleIDs(Set(starredArticleIDs))) { result in account.fetchArticlesAsync(.articleIDs(Set(articleIDs))) { result in
func processWithArticles(_ starredArticles: Set<Article>) { func processWithArticles(_ articles: Set<Article>) {
self.articlesZone.sendArticleStatus(syncStatuses, starredArticles: starredArticles) { result in self.articlesZone.sendArticleStatus(syncStatuses, articles: articles) { result in
switch result { switch result {
case .success: case .success:
self.database.deleteSelectedForProcessing(syncStatuses.map({ $0.articleID }) ) self.database.deleteSelectedForProcessing(syncStatuses.map({ $0.articleID }) )
@ -111,8 +119,8 @@ final class CloudKitAccountDelegate: AccountDelegate {
} }
switch result { switch result {
case .success(let starredArticles): case .success(let articles):
processWithArticles(starredArticles) processWithArticles(articles)
case .failure(let databaseError): case .failure(let databaseError):
completion(.failure(databaseError)) completion(.failure(databaseError))
} }
@ -146,6 +154,11 @@ final class CloudKitAccountDelegate: AccountDelegate {
} }
func importOPML(for account:Account, opmlFile: URL, completion: @escaping (Result<Void, Error>) -> Void) { func importOPML(for account:Account, opmlFile: URL, completion: @escaping (Result<Void, Error>) -> Void) {
guard refreshProgress.isComplete else {
completion(.success(()))
return
}
var fileData: Data? var fileData: Data?
do { do {
@ -463,8 +476,13 @@ final class CloudKitAccountDelegate: AccountDelegate {
} }
func accountDidInitialize(_ account: Account) { func accountDidInitialize(_ account: Account) {
self.account = account
accountZone.delegate = CloudKitAcountZoneDelegate(account: account, refreshProgress: refreshProgress) accountZone.delegate = CloudKitAcountZoneDelegate(account: account, refreshProgress: refreshProgress)
articlesZone.delegate = CloudKitArticlesZoneDelegate(account: account, database: database, articlesZone: articlesZone) articlesZone.delegate = CloudKitArticlesZoneDelegate(account: account,
database: database,
articlesZone: articlesZone,
refreshProgress: refreshProgress)
// Check to see if this is a new account and initialize anything we need // Check to see if this is a new account and initialize anything we need
if account.externalID == nil { if account.externalID == nil {
@ -472,11 +490,7 @@ final class CloudKitAccountDelegate: AccountDelegate {
switch result { switch result {
case .success(let externalID): case .success(let externalID):
account.externalID = externalID account.externalID = externalID
self.refreshAll(for: account, downloadFeeds: false) { result in self.refreshAll(for: account, downloadFeeds: false) { _ in }
if case .failure(let error) = result {
os_log(.error, log: self.log, "Error while doing intial refresh: %@", error.localizedDescription)
}
}
case .failure(let error): case .failure(let error):
os_log(.error, log: self.log, "Error adding account container: %@", error.localizedDescription) os_log(.error, log: self.log, "Error adding account container: %@", error.localizedDescription)
} }
@ -521,6 +535,12 @@ private extension CloudKitAccountDelegate {
let intialWebFeedsCount = downloadFeeds ? account.flattenedWebFeeds().count : 0 let intialWebFeedsCount = downloadFeeds ? account.flattenedWebFeeds().count : 0
refreshProgress.addToNumberOfTasksAndRemaining(3 + intialWebFeedsCount) refreshProgress.addToNumberOfTasksAndRemaining(3 + intialWebFeedsCount)
func fail(_ error: Error) {
self.processAccountError(account, error)
self.refreshProgress.clear()
completion(.failure(error))
}
BatchUpdate.shared.start() BatchUpdate.shared.start()
accountZone.fetchChangesInZone() { result in accountZone.fetchChangesInZone() { result in
BatchUpdate.shared.end() BatchUpdate.shared.end()
@ -549,31 +569,34 @@ private extension CloudKitAccountDelegate {
return return
} }
self.refresher.refreshFeeds(webFeeds, feedCompletionBlock: { _ in self.refreshProgress.completeTask() }) { self.refresher.refreshFeeds(webFeeds) {
account.metadata.lastArticleFetchEndTime = Date() account.metadata.lastArticleFetchEndTime = Date()
self.refreshProgress.clear()
self.sendArticleStatus(for: account) { result in
switch result {
case .success:
completion(.success(())) completion(.success(()))
}
case .failure(let error): case .failure(let error):
self.processAccountError(account, error) fail(error)
self.refreshProgress.clear()
completion(.failure(error))
} }
} }
case .failure(let error):
self.processAccountError(account, error)
self.refreshProgress.clear()
completion(.failure(error))
}
} }
case .failure(let error): case .failure(let error):
self.processAccountError(account, error) fail(error)
self.refreshProgress.clear() }
completion(.failure(error)) }
case .failure(let error):
fail(error)
}
}
case .failure(let error):
fail(error)
} }
} }
} }
@ -588,3 +611,24 @@ private extension CloudKitAccountDelegate {
} }
} }
extension CloudKitAccountDelegate: LocalAccountRefresherDelegate {
func localAccountRefresher(_ refresher: LocalAccountRefresher, didProcess newAndUpdatedArticles: NewAndUpdatedArticles) {
if let newArticles = newAndUpdatedArticles.newArticles {
let syncStatuses = newArticles.map { article in
return SyncStatus(articleID: article.articleID, key: .read, flag: false)
}
database.insertStatuses(syncStatuses)
}
}
func localAccountRefresher(_ refresher: LocalAccountRefresher, requestCompletedFor: WebFeed) {
refreshProgress.completeTask()
}
func localAccountRefresherDidFinish(_ refresher: LocalAccountRefresher) {
refreshProgress.clear()
}
}

View File

@ -48,6 +48,7 @@ final class CloudKitArticlesZone: CloudKitZone {
struct CloudKitArticleStatus { struct CloudKitArticleStatus {
static let recordType = "ArticleStatus" static let recordType = "ArticleStatus"
struct Fields { struct Fields {
static let webFeedExternalID = "webFeedExternalID"
static let read = "read" static let read = "read"
static let starred = "starred" static let starred = "starred"
static let userDeleted = "userDeleted" static let userDeleted = "userDeleted"
@ -81,8 +82,11 @@ final class CloudKitArticlesZone: CloudKitZone {
} }
} }
func sendArticleStatus(_ syncStatuses: [SyncStatus], starredArticles: Set<Article>, completion: @escaping ((Result<Void, Error>) -> Void)) { func sendArticleStatus(_ syncStatuses: [SyncStatus], articles: Set<Article>, completion: @escaping ((Result<Void, Error>) -> Void)) {
var records = makeStatusRecords(syncStatuses)
var records = makeStatusRecords(syncStatuses, articles)
let starredArticles = articles.filter({ $0.status.starred == true })
makeArticleRecordsIfNecessary(starredArticles) { result in makeArticleRecordsIfNecessary(starredArticles) { result in
switch result { switch result {
case .success(let articleRecords): case .success(let articleRecords):
@ -92,11 +96,11 @@ final class CloudKitArticlesZone: CloudKitZone {
case .success: case .success:
completion(.success(())) completion(.success(()))
case .failure(let error): case .failure(let error):
self.handleSendArticleStatusError(error, syncStatuses: syncStatuses, starredArticles: starredArticles, completion: completion) self.handleSendArticleStatusError(error, syncStatuses: syncStatuses, starredArticles: articles, completion: completion)
} }
} }
case .failure(let error): case .failure(let error):
self.handleSendArticleStatusError(error, syncStatuses: syncStatuses, starredArticles: starredArticles, completion: completion) self.handleSendArticleStatusError(error, syncStatuses: syncStatuses, starredArticles: articles, completion: completion)
} }
} }
} }
@ -106,7 +110,7 @@ final class CloudKitArticlesZone: CloudKitZone {
self.createZoneRecord() { result in self.createZoneRecord() { result in
switch result { switch result {
case .success: case .success:
self.sendArticleStatus(syncStatuses, starredArticles: starredArticles, completion: completion) self.sendArticleStatus(syncStatuses, articles: starredArticles, completion: completion)
case .failure(let error): case .failure(let error):
completion(.failure(error)) completion(.failure(error))
} }
@ -120,7 +124,13 @@ final class CloudKitArticlesZone: CloudKitZone {
private extension CloudKitArticlesZone { private extension CloudKitArticlesZone {
func makeStatusRecords(_ syncStatuses: [SyncStatus]) -> [CKRecord] { func makeStatusRecords(_ syncStatuses: [SyncStatus], _ articles: Set<Article>) -> [CKRecord] {
var articleDict = [String: Article]()
for article in articles {
articleDict[article.articleID] = article
}
var records = [String: CKRecord]() var records = [String: CKRecord]()
for status in syncStatuses { for status in syncStatuses {
@ -132,6 +142,10 @@ private extension CloudKitArticlesZone {
records[status.articleID] = record records[status.articleID] = record
} }
if let webFeedExternalID = articleDict[status.articleID]?.webFeed?.externalID {
record![CloudKitArticleStatus.Fields.webFeedExternalID] = webFeedExternalID
}
switch status.key { switch status.key {
case .read: case .read:
record![CloudKitArticleStatus.Fields.read] = status.flag ? "1" : "0" record![CloudKitArticleStatus.Fields.read] = status.flag ? "1" : "0"
@ -147,7 +161,6 @@ private extension CloudKitArticlesZone {
func makeArticleRecordsIfNecessary(_ articles: Set<Article>, completion: @escaping ((Result<[CKRecord], Error>) -> Void)) { func makeArticleRecordsIfNecessary(_ articles: Set<Article>, completion: @escaping ((Result<[CKRecord], Error>) -> Void)) {
let group = DispatchGroup() let group = DispatchGroup()
var errorOccurred = false
var records = [CKRecord]() var records = [CKRecord]()
for article in articles { for article in articles {
@ -164,9 +177,8 @@ private extension CloudKitArticlesZone {
if !recordFound { if !recordFound {
records.append(contentsOf: self.makeArticleRecords(article)) records.append(contentsOf: self.makeArticleRecords(article))
} }
case .failure(let error): case .failure:
errorOccurred = true records.append(contentsOf: self.makeArticleRecords(article))
os_log(.error, log: self.log, "Error occurred while checking for existing articles: %@", error.localizedDescription)
} }
group.leave() group.leave()
} }
@ -174,13 +186,9 @@ private extension CloudKitArticlesZone {
} }
group.notify(queue: DispatchQueue.main) { group.notify(queue: DispatchQueue.main) {
if errorOccurred {
completion(.failure(CloudKitZoneError.unknown))
} else {
completion(.success(records)) completion(.success(records))
} }
} }
}
func makeArticleRecords(_ article: Article) -> [CKRecord] { func makeArticleRecords(_ article: Article) -> [CKRecord] {
var records = [CKRecord]() var records = [CKRecord]()

View File

@ -9,8 +9,11 @@
import Foundation import Foundation
import os.log import os.log
import RSParser import RSParser
import RSWeb
import CloudKit import CloudKit
import SyncDatabase import SyncDatabase
import Articles
import ArticlesDatabase
class CloudKitArticlesZoneDelegate: CloudKitZoneDelegate { class CloudKitArticlesZoneDelegate: CloudKitZoneDelegate {
@ -19,11 +22,19 @@ class CloudKitArticlesZoneDelegate: CloudKitZoneDelegate {
weak var account: Account? weak var account: Account?
var database: SyncDatabase var database: SyncDatabase
weak var articlesZone: CloudKitArticlesZone? weak var articlesZone: CloudKitArticlesZone?
weak var refreshProgress: DownloadProgress?
init(account: Account, database: SyncDatabase, articlesZone: CloudKitArticlesZone) { private lazy var refresher: LocalAccountRefresher = {
let refresher = LocalAccountRefresher()
refresher.delegate = self
return refresher
}()
init(account: Account, database: SyncDatabase, articlesZone: CloudKitArticlesZone, refreshProgress: DownloadProgress?) {
self.account = account self.account = account
self.database = database self.database = database
self.articlesZone = articlesZone self.articlesZone = articlesZone
self.refreshProgress = refreshProgress
} }
func cloudKitDidChange(record: CKRecord) { func cloudKitDidChange(record: CKRecord) {
@ -82,8 +93,40 @@ private extension CloudKitArticlesZoneDelegate {
let group = DispatchGroup() let group = DispatchGroup()
group.enter() group.enter()
account?.markAsUnread(updateableUnreadArticleIDs) { _ in account?.markAsUnread(updateableUnreadArticleIDs) { result in
switch result {
case .success(let newArticleStatusIDs):
if newArticleStatusIDs.isEmpty {
group.leave() group.leave()
} else {
var webFeedExternalIDDict = [String: String]()
for record in records {
if let webFeedExternalID = record[CloudKitArticlesZone.CloudKitArticleStatus.Fields.webFeedExternalID] as? String {
webFeedExternalIDDict[record.externalID] = webFeedExternalID
}
}
var webFeeds = Set<WebFeed>()
for newArticleStatusID in newArticleStatusIDs {
if let webFeedExternalID = webFeedExternalIDDict[newArticleStatusID],
let webFeed = self.account?.existingWebFeed(withExternalID: webFeedExternalID) {
webFeeds.insert(webFeed)
}
}
webFeeds.forEach { $0.dropConditionalGetInfo() }
self.refreshProgress?.addToNumberOfTasksAndRemaining(webFeeds.count)
self.refresher.refreshFeeds(webFeeds) {
group.leave()
}
}
case .failure:
group.leave()
}
} }
group.enter() group.enter()
@ -104,9 +147,9 @@ private extension CloudKitArticlesZoneDelegate {
for receivedStarredArticle in receivedStarredArticles { for receivedStarredArticle in receivedStarredArticles {
if let parsedItem = makeParsedItem(receivedStarredArticle) { if let parsedItem = makeParsedItem(receivedStarredArticle) {
group.enter() group.enter()
self.account?.update(parsedItem.feedURL, with: Set([parsedItem])) { databaseError in self.account?.update(parsedItem.feedURL, with: Set([parsedItem])) { result in
group.leave() group.leave()
if let databaseError = databaseError { if case .failure(let databaseError) = result {
os_log(.error, log: self.log, "Error occurred while storing starred items: %@", databaseError.localizedDescription) os_log(.error, log: self.log, "Error occurred while storing starred items: %@", databaseError.localizedDescription)
} }
} }
@ -119,7 +162,6 @@ private extension CloudKitArticlesZoneDelegate {
} }
func makeParsedItem(_ articleRecord: CKRecord) -> ParsedItem? { func makeParsedItem(_ articleRecord: CKRecord) -> ParsedItem? {
var parsedAuthors = Set<ParsedAuthor>() var parsedAuthors = Set<ParsedAuthor>()
@ -160,3 +202,17 @@ private extension CloudKitArticlesZoneDelegate {
} }
} }
extension CloudKitArticlesZoneDelegate: LocalAccountRefresherDelegate {
func localAccountRefresher(_ refresher: LocalAccountRefresher, didProcess newAndUpdatedArticles: NewAndUpdatedArticles) {
}
func localAccountRefresher(_ refresher: LocalAccountRefresher, requestCompletedFor: WebFeed) {
refreshProgress?.completeTask()
}
func localAccountRefresherDidFinish(_ refresher: LocalAccountRefresher) {
}
}

View File

@ -14,7 +14,7 @@ import RSCore
class FeedFinder { class FeedFinder {
static func find(url: URL, completion: @escaping (Result<Set<FeedSpecifier>, Error>) -> Void) { static func find(url: URL, completion: @escaping (Result<Set<FeedSpecifier>, Error>) -> Void) {
downloadUsingCache(url) { (data, response, error) in downloadAddingToCache(url) { (data, response, error) in
if response?.forcedStatusCode == 404 { if response?.forcedStatusCode == 404 {
completion(.failure(AccountError.createErrorNotFound)) completion(.failure(AccountError.createErrorNotFound))
return return

View File

@ -124,15 +124,19 @@ final class FeedlyIngestStarredArticleIdsOperation: FeedlyOperation {
let results = StarredStatusResults() let results = StarredStatusResults()
group.enter() group.enter()
account.markAsStarred(remoteStarredArticleIDs) { error in account.markAsStarred(remoteStarredArticleIDs) { result in
if case .failure(let error) = result {
results.markAsStarredError = error results.markAsStarredError = error
}
group.leave() group.leave()
} }
let deltaUnstarredArticleIDs = localStarredArticleIDs.subtracting(remoteStarredArticleIDs) let deltaUnstarredArticleIDs = localStarredArticleIDs.subtracting(remoteStarredArticleIDs)
group.enter() group.enter()
account.markAsUnstarred(deltaUnstarredArticleIDs) { error in account.markAsUnstarred(deltaUnstarredArticleIDs) { result in
if case .failure(let error) = result {
results.markAsUnstarredError = error results.markAsUnstarredError = error
}
group.leave() group.leave()
} }

View File

@ -124,15 +124,19 @@ final class FeedlyIngestUnreadArticleIdsOperation: FeedlyOperation {
let results = ReadStatusResults() let results = ReadStatusResults()
group.enter() group.enter()
account.markAsUnread(remoteUnreadArticleIDs) { error in account.markAsUnread(remoteUnreadArticleIDs) { result in
if case .failure(let error) = result {
results.markAsUnreadError = error results.markAsUnreadError = error
}
group.leave() group.leave()
} }
let articleIDsToMarkRead = localUnreadArticleIDs.subtracting(remoteUnreadArticleIDs) let articleIDsToMarkRead = localUnreadArticleIDs.subtracting(remoteUnreadArticleIDs)
group.enter() group.enter()
account.markAsRead(articleIDsToMarkRead) { error in account.markAsRead(articleIDsToMarkRead) { result in
if case .failure(let error) = result {
results.markAsReadError = error results.markAsReadError = error
}
group.leave() group.leave()
} }

View File

@ -10,6 +10,7 @@ import Foundation
import RSCore import RSCore
import RSParser import RSParser
import Articles import Articles
import ArticlesDatabase
import RSWeb import RSWeb
public enum LocalAccountDelegateError: String, Error { public enum LocalAccountDelegateError: String, Error {
@ -18,7 +19,13 @@ public enum LocalAccountDelegateError: String, Error {
final class LocalAccountDelegate: AccountDelegate { final class LocalAccountDelegate: AccountDelegate {
private let refresher = LocalAccountRefresher() weak var account: Account?
private lazy var refresher: LocalAccountRefresher? = {
let refresher = LocalAccountRefresher()
refresher.delegate = self
return refresher
}()
let behaviors: AccountBehaviors = [] let behaviors: AccountBehaviors = []
let isOPMLImportInProgress = false let isOPMLImportInProgress = false
@ -28,19 +35,23 @@ final class LocalAccountDelegate: AccountDelegate {
var accountMetadata: AccountMetadata? var accountMetadata: AccountMetadata?
let refreshProgress = DownloadProgress(numberOfTasks: 0) let refreshProgress = DownloadProgress(numberOfTasks: 0)
var refreshAllCompletion: ((Result<Void, Error>) -> Void)? = nil
func receiveRemoteNotification(for account: Account, userInfo: [AnyHashable : Any], completion: @escaping () -> Void) { func receiveRemoteNotification(for account: Account, userInfo: [AnyHashable : Any], completion: @escaping () -> Void) {
completion() completion()
} }
func refreshAll(for account: Account, completion: @escaping (Result<Void, Error>) -> Void) { func refreshAll(for account: Account, completion: @escaping (Result<Void, Error>) -> Void) {
guard refreshAllCompletion == nil else {
completion(.success(()))
return
}
refreshAllCompletion = completion
let webFeeds = account.flattenedWebFeeds() let webFeeds = account.flattenedWebFeeds()
refreshProgress.addToNumberOfTasksAndRemaining(webFeeds.count) refreshProgress.addToNumberOfTasksAndRemaining(webFeeds.count)
refresher.refreshFeeds(webFeeds, feedCompletionBlock: { _ in self.refreshProgress.completeTask() }) { refresher?.refreshFeeds(webFeeds)
self.refreshProgress.clear()
account.metadata.lastArticleFetchEndTime = Date()
completion(.success(()))
}
} }
func sendArticleStatus(for account: Account, completion: @escaping ((Result<Void, Error>) -> Void)) { func sendArticleStatus(for account: Account, completion: @escaping ((Result<Void, Error>) -> Void)) {
@ -196,6 +207,7 @@ final class LocalAccountDelegate: AccountDelegate {
} }
func accountDidInitialize(_ account: Account) { func accountDidInitialize(_ account: Account) {
self.account = account
} }
func accountWillBeDeleted(_ account: Account) { func accountWillBeDeleted(_ account: Account) {
@ -208,7 +220,7 @@ final class LocalAccountDelegate: AccountDelegate {
// MARK: Suspend and Resume (for iOS) // MARK: Suspend and Resume (for iOS)
func suspendNetwork() { func suspendNetwork() {
refresher.suspend() refresher?.suspend()
} }
func suspendDatabase() { func suspendDatabase() {
@ -216,6 +228,24 @@ final class LocalAccountDelegate: AccountDelegate {
} }
func resume() { func resume() {
refresher.resume() refresher?.resume()
} }
} }
extension LocalAccountDelegate: LocalAccountRefresherDelegate {
func localAccountRefresher(_ refresher: LocalAccountRefresher, didProcess newAndUpdatedArticles: NewAndUpdatedArticles) {
}
func localAccountRefresher(_ refresher: LocalAccountRefresher, requestCompletedFor: WebFeed) {
refreshProgress.completeTask()
}
func localAccountRefresherDidFinish(_ refresher: LocalAccountRefresher) {
self.refreshProgress.clear()
account?.metadata.lastArticleFetchEndTime = Date()
refreshAllCompletion?(.success(()))
refreshAllCompletion = nil
}
}

View File

@ -11,20 +11,28 @@ import RSCore
import RSParser import RSParser
import RSWeb import RSWeb
import Articles import Articles
import ArticlesDatabase
protocol LocalAccountRefresherDelegate {
func localAccountRefresher(_ refresher: LocalAccountRefresher, didProcess: NewAndUpdatedArticles)
func localAccountRefresher(_ refresher: LocalAccountRefresher, requestCompletedFor: WebFeed)
func localAccountRefresherDidFinish(_ refresher: LocalAccountRefresher)
}
final class LocalAccountRefresher { final class LocalAccountRefresher {
private var feedCompletionBlock: ((WebFeed) -> Void)? private var completions = [() -> Void]()
private var completion: (() -> Void)?
private var isSuspended = false private var isSuspended = false
var delegate: LocalAccountRefresherDelegate?
private lazy var downloadSession: DownloadSession = { private lazy var downloadSession: DownloadSession = {
return DownloadSession(delegate: self) return DownloadSession(delegate: self)
}() }()
public func refreshFeeds(_ feeds: Set<WebFeed>, feedCompletionBlock: @escaping (WebFeed) -> Void, completion: @escaping () -> Void) { public func refreshFeeds(_ feeds: Set<WebFeed>, completion: (() -> Void)? = nil) {
self.feedCompletionBlock = feedCompletionBlock if let completion = completion {
self.completion = completion completions.append(completion)
}
downloadSession.downloadObjects(feeds as NSSet) downloadSession.downloadObjects(feeds as NSSet)
} }
@ -64,21 +72,21 @@ extension LocalAccountRefresher: DownloadSessionDelegate {
guard !data.isEmpty, !isSuspended else { guard !data.isEmpty, !isSuspended else {
completion() completion()
feedCompletionBlock?(feed) delegate?.localAccountRefresher(self, requestCompletedFor: feed)
return return
} }
if let error = error { if let error = error {
print("Error downloading \(feed.url) - \(error)") print("Error downloading \(feed.url) - \(error)")
completion() completion()
feedCompletionBlock?(feed) delegate?.localAccountRefresher(self, requestCompletedFor: feed)
return return
} }
let dataHash = data.md5String let dataHash = data.md5String
if dataHash == feed.contentHash { if dataHash == feed.contentHash {
completion() completion()
feedCompletionBlock?(feed) delegate?.localAccountRefresher(self, requestCompletedFor: feed)
return return
} }
@ -87,20 +95,20 @@ extension LocalAccountRefresher: DownloadSessionDelegate {
guard let account = feed.account, let parsedFeed = parsedFeed, error == nil else { guard let account = feed.account, let parsedFeed = parsedFeed, error == nil else {
completion() completion()
self.feedCompletionBlock?(feed) self.delegate?.localAccountRefresher(self, requestCompletedFor: feed)
return return
} }
account.update(feed, with: parsedFeed) { error in account.update(feed, with: parsedFeed) { result in
if error == nil { if case .success(let newAndUpdatedArticles) = result {
self.delegate?.localAccountRefresher(self, didProcess: newAndUpdatedArticles)
if let httpResponse = response as? HTTPURLResponse { if let httpResponse = response as? HTTPURLResponse {
feed.conditionalGetInfo = HTTPConditionalGetInfo(urlResponse: httpResponse) feed.conditionalGetInfo = HTTPConditionalGetInfo(urlResponse: httpResponse)
} }
feed.contentHash = dataHash feed.contentHash = dataHash
} }
completion() completion()
self.feedCompletionBlock?(feed) self.delegate?.localAccountRefresher(self, requestCompletedFor: feed)
} }
} }
@ -109,7 +117,7 @@ extension LocalAccountRefresher: DownloadSessionDelegate {
func downloadSession(_ downloadSession: DownloadSession, shouldContinueAfterReceivingData data: Data, representedObject: AnyObject) -> Bool { func downloadSession(_ downloadSession: DownloadSession, shouldContinueAfterReceivingData data: Data, representedObject: AnyObject) -> Bool {
let feed = representedObject as! WebFeed let feed = representedObject as! WebFeed
guard !isSuspended else { guard !isSuspended else {
feedCompletionBlock?(feed) delegate?.localAccountRefresher(self, requestCompletedFor: feed)
return false return false
} }
@ -118,7 +126,7 @@ extension LocalAccountRefresher: DownloadSessionDelegate {
} }
if data.isDefinitelyNotFeed() { if data.isDefinitelyNotFeed() {
feedCompletionBlock?(feed) delegate?.localAccountRefresher(self, requestCompletedFor: feed)
return false return false
} }
@ -127,7 +135,7 @@ extension LocalAccountRefresher: DownloadSessionDelegate {
if FeedParser.mightBeAbleToParseBasedOnPartialData(parserData) { if FeedParser.mightBeAbleToParseBasedOnPartialData(parserData) {
return true return true
} else { } else {
feedCompletionBlock?(feed) delegate?.localAccountRefresher(self, requestCompletedFor: feed)
return false return false
} }
} }
@ -137,17 +145,23 @@ extension LocalAccountRefresher: DownloadSessionDelegate {
func downloadSession(_ downloadSession: DownloadSession, didReceiveUnexpectedResponse response: URLResponse, representedObject: AnyObject) { func downloadSession(_ downloadSession: DownloadSession, didReceiveUnexpectedResponse response: URLResponse, representedObject: AnyObject) {
let feed = representedObject as! WebFeed let feed = representedObject as! WebFeed
feedCompletionBlock?(feed) delegate?.localAccountRefresher(self, requestCompletedFor: feed)
} }
func downloadSession(_ downloadSession: DownloadSession, didReceiveNotModifiedResponse: URLResponse, representedObject: AnyObject) { func downloadSession(_ downloadSession: DownloadSession, didReceiveNotModifiedResponse: URLResponse, representedObject: AnyObject) {
let feed = representedObject as! WebFeed let feed = representedObject as! WebFeed
feedCompletionBlock?(feed) delegate?.localAccountRefresher(self, requestCompletedFor: feed)
}
func downloadSession(_ downloadSession: DownloadSession, didDiscardDuplicateRepresentedObject representedObject: AnyObject) {
let feed = representedObject as! WebFeed
delegate?.localAccountRefresher(self, requestCompletedFor: feed)
} }
func downloadSessionDidCompleteDownloadObjects(_ downloadSession: DownloadSession) { func downloadSessionDidCompleteDownloadObjects(_ downloadSession: DownloadSession) {
completion?() completions.forEach({ $0() })
completion = nil completions = [() -> Void]()
delegate?.localAccountRefresherDidFinish(self)
} }
} }

View File

@ -221,9 +221,9 @@ public final class WebFeed: Feed, Renamable, Hashable {
self.metadata = metadata self.metadata = metadata
} }
// MARK: - Debug // MARK: - API
public func debugDropConditionalGetInfo() { public func dropConditionalGetInfo() {
conditionalGetInfo = nil conditionalGetInfo = nil
contentHash = nil contentHash = nil
} }

View File

@ -27,6 +27,16 @@ public typealias SingleUnreadCountCompletionBlock = (SingleUnreadCountResult) ->
public struct NewAndUpdatedArticles { public struct NewAndUpdatedArticles {
public let newArticles: Set<Article>? public let newArticles: Set<Article>?
public let updatedArticles: Set<Article>? public let updatedArticles: Set<Article>?
public init() {
self.newArticles = Set<Article>()
self.updatedArticles = Set<Article>()
}
public init(newArticles: Set<Article>?, updatedArticles: Set<Article>?) {
self.newArticles = newArticles
self.updatedArticles = updatedArticles
}
} }
public typealias UpdateArticlesResult = Result<NewAndUpdatedArticles, DatabaseError> public typealias UpdateArticlesResult = Result<NewAndUpdatedArticles, DatabaseError>
@ -222,8 +232,8 @@ public final class ArticlesDatabase {
return try articlesTable.mark(articles, statusKey, flag) return try articlesTable.mark(articles, statusKey, flag)
} }
public func mark(articleIDs: Set<String>, statusKey: ArticleStatus.Key, flag: Bool, completion: @escaping DatabaseCompletionBlock) { public func markAndFetchNew(articleIDs: Set<String>, statusKey: ArticleStatus.Key, flag: Bool, completion: @escaping ArticleIDsCompletionBlock) {
articlesTable.mark(articleIDs, statusKey, flag, completion) articlesTable.markAndFetchNew(articleIDs, statusKey, flag, completion)
} }
/// Create statuses for specified articleIDs. For existing statuses, dont do anything. /// Create statuses for specified articleIDs. For existing statuses, dont do anything.

View File

@ -194,7 +194,7 @@ final class ArticlesTable: DatabaseTable {
func makeDatabaseCalls(_ database: FMDatabase) { func makeDatabaseCalls(_ database: FMDatabase) {
let articleIDs = parsedItems.articleIDs() let articleIDs = parsedItems.articleIDs()
let statusesDictionary = self.statusesTable.ensureStatusesForArticleIDs(articleIDs, false, database) //1 let (statusesDictionary, _) = self.statusesTable.ensureStatusesForArticleIDs(articleIDs, false, database) //1
assert(statusesDictionary.count == articleIDs.count) assert(statusesDictionary.count == articleIDs.count)
let allIncomingArticles = Article.articlesWithParsedItems(parsedItems, webFeedID, self.accountID, statusesDictionary) //2 let allIncomingArticles = Article.articlesWithParsedItems(parsedItems, webFeedID, self.accountID, statusesDictionary) //2
@ -266,7 +266,7 @@ final class ArticlesTable: DatabaseTable {
articleIDs.formUnion(parsedItems.articleIDs()) articleIDs.formUnion(parsedItems.articleIDs())
} }
let statusesDictionary = self.statusesTable.ensureStatusesForArticleIDs(articleIDs, read, database) //1 let (statusesDictionary, _) = self.statusesTable.ensureStatusesForArticleIDs(articleIDs, read, database) //1
assert(statusesDictionary.count == articleIDs.count) assert(statusesDictionary.count == articleIDs.count)
let allIncomingArticles = Article.articlesWithWebFeedIDsAndItems(webFeedIDsAndItems, self.accountID, statusesDictionary) //2 let allIncomingArticles = Article.articlesWithWebFeedIDsAndItems(webFeedIDsAndItems, self.accountID, statusesDictionary) //2
@ -418,17 +418,17 @@ final class ArticlesTable: DatabaseTable {
return statuses return statuses
} }
func mark(_ articleIDs: Set<String>, _ statusKey: ArticleStatus.Key, _ flag: Bool, _ completion: @escaping DatabaseCompletionBlock) { func markAndFetchNew(_ articleIDs: Set<String>, _ statusKey: ArticleStatus.Key, _ flag: Bool, _ completion: @escaping ArticleIDsCompletionBlock) {
queue.runInTransaction { databaseResult in queue.runInTransaction { databaseResult in
switch databaseResult { switch databaseResult {
case .success(let database): case .success(let database):
self.statusesTable.mark(articleIDs, statusKey, flag, database) let newStatusIDs = self.statusesTable.markAndFetchNew(articleIDs, statusKey, flag, database)
DispatchQueue.main.async { DispatchQueue.main.async {
completion(nil) completion(.success(newStatusIDs))
} }
case .failure(let databaseError): case .failure(let databaseError):
DispatchQueue.main.async { DispatchQueue.main.async {
completion(databaseError) completion(.failure(databaseError))
} }
} }
} }

View File

@ -27,11 +27,11 @@ final class StatusesTable: DatabaseTable {
// MARK: - Creating/Updating // MARK: - Creating/Updating
func ensureStatusesForArticleIDs(_ articleIDs: Set<String>, _ read: Bool, _ database: FMDatabase) -> [String: ArticleStatus] { func ensureStatusesForArticleIDs(_ articleIDs: Set<String>, _ read: Bool, _ database: FMDatabase) -> ([String: ArticleStatus], Set<String>) {
// Check cache. // Check cache.
let articleIDsMissingCachedStatus = articleIDsWithNoCachedStatus(articleIDs) let articleIDsMissingCachedStatus = articleIDsWithNoCachedStatus(articleIDs)
if articleIDsMissingCachedStatus.isEmpty { if articleIDsMissingCachedStatus.isEmpty {
return statusesDictionary(articleIDs) return (statusesDictionary(articleIDs), Set<String>())
} }
// Check database. // Check database.
@ -43,7 +43,7 @@ final class StatusesTable: DatabaseTable {
self.createAndSaveStatusesForArticleIDs(articleIDsNeedingStatus, read, database) self.createAndSaveStatusesForArticleIDs(articleIDsNeedingStatus, read, database)
} }
return statusesDictionary(articleIDs) return (statusesDictionary(articleIDs), articleIDsNeedingStatus)
} }
func existingStatusesForArticleIDs(_ articleIDs: Set<String>, _ database: FMDatabase) -> [String: ArticleStatus] { func existingStatusesForArticleIDs(_ articleIDs: Set<String>, _ database: FMDatabase) -> [String: ArticleStatus] {
@ -85,10 +85,11 @@ final class StatusesTable: DatabaseTable {
return updatedStatuses return updatedStatuses
} }
func mark(_ articleIDs: Set<String>, _ statusKey: ArticleStatus.Key, _ flag: Bool, _ database: FMDatabase) { func markAndFetchNew(_ articleIDs: Set<String>, _ statusKey: ArticleStatus.Key, _ flag: Bool, _ database: FMDatabase) -> Set<String> {
let statusesDictionary = ensureStatusesForArticleIDs(articleIDs, flag, database) let (statusesDictionary, newStatusIDs) = ensureStatusesForArticleIDs(articleIDs, flag, database)
let statuses = Set(statusesDictionary.values) let statuses = Set(statusesDictionary.values)
mark(statuses, statusKey, flag, database) mark(statuses, statusKey, flag, database)
return newStatusIDs
} }
// MARK: - Fetching // MARK: - Fetching

View File

@ -19,7 +19,7 @@ class AccountsAddViewController: NSViewController {
#if DEBUG #if DEBUG
private var addableAccountTypes: [AccountType] = [.onMyMac, .feedbin, .feedly, .feedWrangler, .freshRSS, .cloudKit, .newsBlur] private var addableAccountTypes: [AccountType] = [.onMyMac, .feedbin, .feedly, .feedWrangler, .freshRSS, .cloudKit, .newsBlur]
#else #else
private var addableAccountTypes: [AccountType] = [.onMyMac, .feedbin, .feedly] private var addableAccountTypes: [AccountType] = [.onMyMac, .feedbin, .feedly, .cloudKit, .newsBlur]
#endif #endif
init() { init() {

View File

@ -33,8 +33,7 @@ final class AccountsDetailViewController: NSViewController, NSTextFieldDelegate
return true return true
} }
switch account.type { switch account.type {
case .onMyMac, case .onMyMac, .cloudKit, .feedly:
.feedly:
return true return true
default: default:
return false return false

View File

@ -0,0 +1,72 @@
# Retention Policy
This is a user interface issue, primarily — what articles should be displayed to the user?
This article answers that question, and it describes how the decisions are implemented.
And, at the end, theres a special note about why we have limits at all.
## Web-Based Sync Systems
When used with Feedbin, Feedly, and other syncing systems, NetNewsWire should show the same unread articles that the user would see in the browser-based version. (The unread counts would necessarily be the same in NetNewsWire and on the web.)
It should also show the exact same starred items.
It does *not* have to show the exact same read items. Instead, it will show read items that arrived locally in the last 90 days.
### Database Queries
Most queries for articles should include this logic:
* If an article is userDeleted, dont show it
* If an article is starred or unread, show it no matter what
* If an article is read, and status.dateArrived < 90 days ago, then show it
### Database Cleanup
Database cleanups to do at startup:
* Delete articles from feeds no-longer-subscribed-to, unless starred
* Delete read/not-starred articles where status.dateArrived > 90 days go (because these wouldnt be shown in the UI)
* Delete statuses where status is read, not starred, and not userDeleted, and dateArrived > 180 days ago, and the associated article no longer exists in the articles table.
We keep statuses a bit longer than articles, in case an article comes back. But we dont keep most statuses forever.
## Local and iCloud Accounts
NetNewsWire should show articles that are currently in the feed. When an article drops off the feed, it no longer displays in the UI.
The one exception is starred articles: as with sync systems, starred articles are kept forever.
### Database Queries
Most queries for articles should include this logic:
* If an article is userDeleted, dont show it
* If an article is starred, show it no matter what
### Database Cleanup
Database cleanups to do while running:
* When processing a feed, delete articles that no longer appear in the feed — unless a feed comes back empty (with zero articles); do nothing in that case
Database cleanups to do at startup:
* Delete articles from feeds no-longer-subscribed-to, unless starred
* Delete statuses where not starred, not userDeleted, and dateArrived > 30 days ago, and the associated article no longer exists in the articles table.
We keep statuses a bit longer than articles, in case an article comes back. (An article could come back when, for example, a publisher reconfigures their feed so that it includes more items. This could bring back articles that had previously fallen off the feed.)
## Why Do We Have Limits At All?
Most people dont want NetNewsWire to just keep holding on to everything forever, but a few people do.
And thats understandable. Its pretty cool to have a personal backup of your favorite parts of the web. Its great for researchers, journalists, and bloggers.
But the more articles we keep, the larger the database gets. Its already not unusual for a database to become 1GB in size — but we cant let it grow to many times that, because it will:
* Make NetNewsWire unacceptably slow
* Take up an inordinate amount of disk space
So we need to have limits. The point of NetNewsWire is to keep up with whats new: its *not* an archiving system. So weve defined “whats new” expansively, but not so expansively that we dont have a definition for “whats old.”

View File

@ -19,7 +19,7 @@ class AddAccountViewController: UITableViewController, AddAccountDismissDelegate
#if DEBUG #if DEBUG
private var addableAccountTypes: [AccountType] = [.onMyMac, .feedbin, .feedly, .feedWrangler, .cloudKit, .newsBlur] private var addableAccountTypes: [AccountType] = [.onMyMac, .feedbin, .feedly, .feedWrangler, .cloudKit, .newsBlur]
#else #else
private var addableAccountTypes: [AccountType] = [.onMyMac, .feedbin, .feedly] private var addableAccountTypes: [AccountType] = [.onMyMac, .feedbin, .feedly, .cloudKit, .newsBlur]
#endif #endif
override func viewDidLoad() { override func viewDidLoad() {

@ -1 +1 @@
Subproject commit 88d634f5fd42aab203b6e53c7b551a92b03ffc97 Subproject commit c524ce9145dfe093500325b1c758ea83f82cc090