Merge pull request #1095 from kielgillard/1077-improve-content-consistentcy

Improve Feedly content consistency

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:

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
 		}

Frameworks/Account/Feedly/FeedlyAPICaller.swift

@@ -89,20 +89,36 @@ final class FeedlyAPICaller {
 		}
 	}
 	
-	func getStream(for collection: FeedlyCollection, unreadOnly: Bool = false, completionHandler: @escaping (Result<FeedlyStream, Error>) -> ()) {
+	func getStream(for collection: FeedlyCollection, newerThan: Date? = nil, unreadOnly: Bool? = nil, completionHandler: @escaping (Result<FeedlyStream, Error>) -> ()) {
 		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.")

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)
 		}
 	}
 	

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

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)
 		}
 		

Frameworks/Account/Feedly/Refresh/FeedlySyncStrategy.swift

@@ -32,6 +32,14 @@ final class FeedlySyncStrategy {
 	
 	private var startSyncCompletionHandler: ((Result<Void, Error>) -> ())?
 	
+	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<Void, Error>) -> ()) {
 		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)