From 39d3999a0d40be5f5caef83cc5c9198a214d0eac Mon Sep 17 00:00:00 2001 From: Kiel Gillard Date: Thu, 3 Oct 2019 21:23:49 +1000 Subject: [PATCH] --- .../Feedly/AccountFeedlySyncTest.swift | 3 ++- .../Account/AccountTests/TestTransport.swift | 18 ++++++++++++- .../Account/Feedly/FeedlyAPICaller.swift | 26 +++++++++++++++---- .../Feedly/FeedlyAccountDelegate.swift | 6 ++--- .../FeedlyGetCollectionStreamOperation.swift | 9 ++++--- .../FeedlyRequestStreamsOperation.swift | 12 +++++++-- .../Feedly/Refresh/FeedlySyncStrategy.swift | 22 +++++++++++++--- 7 files changed, 75 insertions(+), 21 deletions(-) diff --git a/Frameworks/Account/AccountTests/Feedly/AccountFeedlySyncTest.swift b/Frameworks/Account/AccountTests/Feedly/AccountFeedlySyncTest.swift index b196a8526..4c9183237 100644 --- a/Frameworks/Account/AccountTests/Feedly/AccountFeedlySyncTest.swift +++ b/Frameworks/Account/AccountTests/Feedly/AccountFeedlySyncTest.swift @@ -281,8 +281,9 @@ class AccountFeedlySyncTest: XCTestCase { } func set(testFiles: TestFiles, with transport: TestTransport) { + // TestTransport blacklists certain query items to make mocking responses easier. let endpoint = "https://sandbox7.feedly.com/v3" - let category = "\(endpoint)/streams/contents?unreadOnly=false&count=500&streamId=user/f2f031bd-f3e3-4893-a447-467a291c6d1e/category" + let category = "\(endpoint)/streams/contents?streamId=user/f2f031bd-f3e3-4893-a447-467a291c6d1e/category" switch testFiles { case .initial: diff --git a/Frameworks/Account/AccountTests/TestTransport.swift b/Frameworks/Account/AccountTests/TestTransport.swift index 1884c6256..9bd072dd5 100644 --- a/Frameworks/Account/AccountTests/TestTransport.swift +++ b/Frameworks/Account/AccountTests/TestTransport.swift @@ -18,6 +18,13 @@ final class TestTransport: Transport { var testFiles = [String: String]() var testStatusCodes = [String: Int]() + /// Allows tests to filter time sensitive state out to make matching against test data easier. + var blacklistedQueryItemNames = Set([ + "newerThan", // Feedly: Mock data has a fixed date. + "unreadOnly", // Feedly: Mock data is read/unread by test expectation. + "count", // Feedly: Mock data is limited by test expectation. + ]) + private func httpResponse(for request: URLRequest, statusCode: Int = 200) -> HTTPURLResponse { guard let url = request.url else { fatalError("Attempting to mock a http response for a request without a URL \(request).") @@ -27,7 +34,16 @@ final class TestTransport: Transport { func send(request: URLRequest, completion: @escaping (Result<(HTTPURLResponse, Data?), Error>) -> Void) { - guard let urlString = request.url?.absoluteString else { + guard let url = request.url, var components = URLComponents(url: url, resolvingAgainstBaseURL: false) else { + completion(.failure(TestTransportError.invalidState)) + return + } + + components.queryItems = components + .queryItems? + .filter { !blacklistedQueryItemNames.contains($0.name) } + + guard let urlString = components.url?.absoluteString else { completion(.failure(TestTransportError.invalidState)) return } diff --git a/Frameworks/Account/Feedly/FeedlyAPICaller.swift b/Frameworks/Account/Feedly/FeedlyAPICaller.swift index c30bd5dfb..6b7c1dd06 100644 --- a/Frameworks/Account/Feedly/FeedlyAPICaller.swift +++ b/Frameworks/Account/Feedly/FeedlyAPICaller.swift @@ -89,20 +89,36 @@ final class FeedlyAPICaller { } } - func getStream(for collection: FeedlyCollection, unreadOnly: Bool = false, completionHandler: @escaping (Result) -> ()) { + func getStream(for collection: FeedlyCollection, newerThan: Date? = nil, unreadOnly: Bool? = nil, completionHandler: @escaping (Result) -> ()) { guard let accessToken = credentials?.secret else { return DispatchQueue.main.async { completionHandler(.failure(CredentialsError.incompleteCredentials)) } } + var components = baseUrlComponents components.path = "/v3/streams/contents" // If you change these, check AccountFeedlySyncTest.set(testFiles:with:). - components.queryItems = [ - URLQueryItem(name: "unreadOnly", value: unreadOnly ? "true" : "false"), - URLQueryItem(name: "count", value: "500"), + var queryItems = [URLQueryItem]() + + if let date = newerThan { + let value = String(Int(date.timeIntervalSince1970 * 1000)) + let queryItem = URLQueryItem(name: "newerThan", value: value) + queryItems.append(queryItem) + } + + if let flag = unreadOnly { + let value = flag ? "true" : "false" + let queryItem = URLQueryItem(name: "unreadOnly", value: value) + queryItems.append(queryItem) + } + + queryItems.append(contentsOf: [ + URLQueryItem(name: "count", value: "1000"), URLQueryItem(name: "streamId", value: collection.id), - ] + ]) + + components.queryItems = queryItems guard let url = components.url else { fatalError("\(components) does not produce a valid URL.") diff --git a/Frameworks/Account/Feedly/FeedlyAccountDelegate.swift b/Frameworks/Account/Feedly/FeedlyAccountDelegate.swift index 996bdf287..1c6761bd1 100644 --- a/Frameworks/Account/Feedly/FeedlyAccountDelegate.swift +++ b/Frameworks/Account/Feedly/FeedlyAccountDelegate.swift @@ -86,10 +86,8 @@ final class FeedlyAccountDelegate: AccountDelegate { progress.addToNumberOfTasksAndRemaining(1) syncStrategy?.startSync { result in os_log(.debug, log: log, "Sync took %.3f seconds", -date.timeIntervalSinceNow) - DispatchQueue.main.async { - progress.completeTask() - completion(result) - } + progress.completeTask() + completion(result) } } diff --git a/Frameworks/Account/Feedly/Refresh/FeedlyGetCollectionStreamOperation.swift b/Frameworks/Account/Feedly/Refresh/FeedlyGetCollectionStreamOperation.swift index 60b69f274..7f67cc759 100644 --- a/Frameworks/Account/Feedly/Refresh/FeedlyGetCollectionStreamOperation.swift +++ b/Frameworks/Account/Feedly/Refresh/FeedlyGetCollectionStreamOperation.swift @@ -30,13 +30,15 @@ final class FeedlyGetCollectionStreamOperation: FeedlyOperation, FeedlyCollectio let account: Account let caller: FeedlyAPICaller - let unreadOnly: Bool + let unreadOnly: Bool? + let newerThan: Date? - init(account: Account, collection: FeedlyCollection, caller: FeedlyAPICaller, unreadOnly: Bool = false) { + init(account: Account, collection: FeedlyCollection, caller: FeedlyAPICaller, newerThan: Date?, unreadOnly: Bool? = nil) { self.account = account self.collection = collection self.caller = caller self.unreadOnly = unreadOnly + self.newerThan = newerThan } override func main() { @@ -45,8 +47,7 @@ final class FeedlyGetCollectionStreamOperation: FeedlyOperation, FeedlyCollectio return } - //TODO: Use account metadata to get articles newer than some date. - caller.getStream(for: collection, unreadOnly: unreadOnly) { result in + caller.getStream(for: collection, newerThan: newerThan, unreadOnly: unreadOnly) { result in switch result { case .success(let stream): self.storedStream = stream diff --git a/Frameworks/Account/Feedly/Refresh/FeedlyRequestStreamsOperation.swift b/Frameworks/Account/Feedly/Refresh/FeedlyRequestStreamsOperation.swift index b3e5cfde6..59311bace 100644 --- a/Frameworks/Account/Feedly/Refresh/FeedlyRequestStreamsOperation.swift +++ b/Frameworks/Account/Feedly/Refresh/FeedlyRequestStreamsOperation.swift @@ -23,11 +23,15 @@ final class FeedlyRequestStreamsOperation: FeedlyOperation { let caller: FeedlyAPICaller let account: Account let log: OSLog + let newerThan: Date? + let unreadOnly: Bool? - init(account: Account, collectionsProvider: FeedlyCollectionProviding, caller: FeedlyAPICaller, log: OSLog) { + init(account: Account, collectionsProvider: FeedlyCollectionProviding, newerThan: Date?, unreadOnly: Bool?, caller: FeedlyAPICaller, log: OSLog) { self.account = account self.caller = caller self.collectionsProvider = collectionsProvider + self.newerThan = newerThan + self.unreadOnly = unreadOnly self.log = log } @@ -41,7 +45,11 @@ final class FeedlyRequestStreamsOperation: FeedlyOperation { // TODO: Prioritise the must read collection/category before others so the most important content for the user loads first. for collection in collectionsProvider.collections { - let operation = FeedlyGetCollectionStreamOperation(account: account, collection: collection, caller: caller) + let operation = FeedlyGetCollectionStreamOperation(account: account, + collection: collection, + caller: caller, + newerThan: newerThan, + unreadOnly: unreadOnly) queueDelegate?.feedlyRequestStreamsOperation(self, enqueue: operation) } diff --git a/Frameworks/Account/Feedly/Refresh/FeedlySyncStrategy.swift b/Frameworks/Account/Feedly/Refresh/FeedlySyncStrategy.swift index f708faa28..ab62248c2 100644 --- a/Frameworks/Account/Feedly/Refresh/FeedlySyncStrategy.swift +++ b/Frameworks/Account/Feedly/Refresh/FeedlySyncStrategy.swift @@ -32,6 +32,14 @@ final class FeedlySyncStrategy { private var startSyncCompletionHandler: ((Result) -> ())? + private var newerThan: Date? { + if let date = account.metadata.lastArticleFetch { + return date + } else { + return Calendar.current.date(byAdding: .day, value: -31, to: Date()) + } + } + /// The truth is in the cloud. func startSync(completionHandler: @escaping (Result) -> ()) { guard operationQueue.operationCount == 0 else { @@ -63,6 +71,8 @@ final class FeedlySyncStrategy { // Get the streams for each Collection. It will call back to enqueue more operations. let getCollectionStreams = FeedlyRequestStreamsOperation(account: account, collectionsProvider: getCollections, + newerThan: newerThan, + unreadOnly: false, caller: caller, log: log) getCollectionStreams.delegate = self @@ -71,12 +81,16 @@ final class FeedlySyncStrategy { // Last operation to perform, which should be dependent on any other operation added to the queue. let syncId = UUID().uuidString + let lastArticleFetchDate = Date() let completionOperation = BlockOperation { [weak self] in - if let self = self { - os_log(.debug, log: self.log, "Sync completed: %@", syncId) - self.startSyncCompletionHandler = nil + DispatchQueue.main.async { + if let self = self { + self.account.metadata.lastArticleFetch = lastArticleFetchDate + os_log(.debug, log: self.log, "Sync completed: %@", syncId) + self.startSyncCompletionHandler = nil + } + completionHandler(.success(())) } - completionHandler(.success(())) } completionOperation.addDependency(getCollections)