From 3328306c4452f8910929146666db2df6a683386e Mon Sep 17 00:00:00 2001 From: Justin Mazzocchi <2831158+jzzocc@users.noreply.github.com> Date: Wed, 23 Sep 2020 18:33:13 -0700 Subject: [PATCH] Use response headers for pagination --- HTTP/Sources/HTTP/HTTPClient.swift | 68 ++++++++----------- HTTP/Sources/HTTP/Target.swift | 1 + .../Sources/MastodonAPI/Endpoints/Paged.swift | 18 +++-- .../MastodonAPI/MastodonAPIClient.swift | 67 ++++++++++++++++-- .../Services/StatusListService.swift | 22 +++--- .../CollectionViewController.swift | 5 +- .../ViewModels/CollectionViewModel.swift | 2 +- .../ViewModels/StatusListViewModel.swift | 7 +- 8 files changed, 124 insertions(+), 66 deletions(-) diff --git a/HTTP/Sources/HTTP/HTTPClient.swift b/HTTP/Sources/HTTP/HTTPClient.swift index e108273..dc98d8e 100644 --- a/HTTP/Sources/HTTP/HTTPClient.swift +++ b/HTTP/Sources/HTTP/HTTPClient.swift @@ -4,63 +4,51 @@ import Combine import Foundation public enum HTTPError: Error { - case invalidStatusCode(HTTPURLResponse) + case nonHTTPURLResponse(data: Data, response: URLResponse) + case invalidStatusCode(data: Data, response: HTTPURLResponse) } open class HTTPClient { + public let decoder: JSONDecoder + private let session: URLSession - private let decoder: JSONDecoder public init(session: URLSession, decoder: JSONDecoder) { self.session = session self.decoder = decoder } + open func dataTaskPublisher( + _ target: T) -> AnyPublisher<(data: Data, response: HTTPURLResponse), Error> { + if let protocolClasses = session.configuration.protocolClasses { + for protocolClass in protocolClasses { + (protocolClass as? TargetProcessing.Type)?.process(target: target) + } + } + + return session.dataTaskPublisher(for: target.urlRequest()) + .tryMap { data, response in + guard let httpResponse = response as? HTTPURLResponse else { + throw HTTPError.nonHTTPURLResponse(data: data, response: response) + } + + guard Self.validStatusCodes.contains(httpResponse.statusCode) else { + throw HTTPError.invalidStatusCode(data: data, response: httpResponse) + } + + return (data, httpResponse) + } + .eraseToAnyPublisher() + } + open func request(_ target: T) -> AnyPublisher { dataTaskPublisher(target) .map(\.data) .decode(type: T.ResultType.self, decoder: decoder) .eraseToAnyPublisher() } - - public func request( - _ target: T, - decodeErrorsAs errorType: E.Type) -> AnyPublisher { - let decoder = self.decoder - - return dataTaskPublisher(target) - .tryMap { result -> Data in - if - let response = result.response as? HTTPURLResponse, - !Self.validStatusCodes.contains(response.statusCode) { - - if let decodedError = try? decoder.decode(E.self, from: result.data) { - throw decodedError - } else { - throw HTTPError.invalidStatusCode(response) - } - } - - return result.data - } - .decode(type: T.ResultType.self, decoder: decoder) - .eraseToAnyPublisher() - } } -private extension HTTPClient { +public extension HTTPClient { static let validStatusCodes = 200..<300 - func dataTaskPublisher(_ target: T) -> URLSession.DataTaskPublisher { - if let protocolClasses = session.configuration.protocolClasses { - for protocolClass in protocolClasses { - (protocolClass as? TargetProcessing.Type)?.process(target: target) - } - } - - return session.dataTaskPublisher(for: target.urlRequest()) - -// return session.request(target.urlRequest()) -// .validate() -// .publishDecodable(type: T.ResultType.self, queue: session.rootQueue, decoder: decoder) - } } diff --git a/HTTP/Sources/HTTP/Target.swift b/HTTP/Sources/HTTP/Target.swift index e88ccd2..efe943c 100644 --- a/HTTP/Sources/HTTP/Target.swift +++ b/HTTP/Sources/HTTP/Target.swift @@ -35,6 +35,7 @@ public extension Target { if let jsonBody = jsonBody { urlRequest.httpBody = try? JSONSerialization.data(withJSONObject: jsonBody) + urlRequest.setValue("application/json; charset=utf-8", forHTTPHeaderField: "Content-Type") } return urlRequest diff --git a/MastodonAPI/Sources/MastodonAPI/Endpoints/Paged.swift b/MastodonAPI/Sources/MastodonAPI/Endpoints/Paged.swift index af36661..e6c1013 100644 --- a/MastodonAPI/Sources/MastodonAPI/Endpoints/Paged.swift +++ b/MastodonAPI/Sources/MastodonAPI/Endpoints/Paged.swift @@ -21,8 +21,7 @@ public struct Paged { } extension Paged: Endpoint { - public typealias ResultType = T.ResultType - // public typealias ResultType = PagedResult + public typealias ResultType = PagedResult public var APIVersion: String { endpoint.APIVersion } @@ -49,8 +48,13 @@ extension Paged: Endpoint { public var headers: [String: String]? { endpoint.headers } } -//public struct PagedResult: Decodable { -// public let result: T -// public let maxID: String? -// public let sinceID: String? -//} +public struct PagedResult: Decodable { + public struct Info: Decodable { + public let maxID: String? + public let minID: String? + public let sinceID: String? + } + + public let result: T + public let info: Info +} diff --git a/MastodonAPI/Sources/MastodonAPI/MastodonAPIClient.swift b/MastodonAPI/Sources/MastodonAPI/MastodonAPIClient.swift index ed453d2..35cb0c8 100644 --- a/MastodonAPI/Sources/MastodonAPI/MastodonAPIClient.swift +++ b/MastodonAPI/Sources/MastodonAPI/MastodonAPIClient.swift @@ -14,15 +14,72 @@ public final class MastodonAPIClient: HTTPClient { super.init(session: session, decoder: MastodonDecoder()) } - public override func request(_ target: T) -> AnyPublisher { - super.request(target, decodeErrorsAs: APIError.self) + public override func dataTaskPublisher( + _ target: T) -> AnyPublisher<(data: Data, response: HTTPURLResponse), Error> { + super.dataTaskPublisher(target) + .mapError { [weak self] error -> Error in + if case let HTTPError.invalidStatusCode(data, _) = error, + let apiError = try? self?.decoder.decode(APIError.self, from: data) { + return apiError + } + + return error + } + .eraseToAnyPublisher() } } extension MastodonAPIClient { public func request(_ endpoint: E) -> AnyPublisher { - super.request( - MastodonAPITarget(baseURL: instanceURL, endpoint: endpoint, accessToken: accessToken), - decodeErrorsAs: APIError.self) + dataTaskPublisher(target(endpoint: endpoint)) + .map(\.data) + .decode(type: E.ResultType.self, decoder: decoder) + .eraseToAnyPublisher() + } + + public func pagedRequest( + _ endpoint: E, + maxID: String? = nil, + minID: String? = nil, + sinceID: String? = nil, + limit: Int? = nil) -> AnyPublisher, Error> { + let pagedTarget = target(endpoint: Paged(endpoint, maxID: maxID, minID: minID, sinceID: sinceID, limit: limit)) + let dataTask = dataTaskPublisher(pagedTarget).share() + let decoded = dataTask.map(\.data).decode(type: E.ResultType.self, decoder: decoder) + let info = dataTask.map { _, response -> PagedResult.Info in + var maxID: String? + var minID: String? + var sinceID: String? + + if let links = response.value(forHTTPHeaderField: "Link") { + let queryItems = Self.linkDataDetector.matches( + in: links, + range: .init(links.startIndex.. [URLQueryItem]? in + guard let url = match.url else { return nil } + + return URLComponents(url: url, resolvingAgainstBaseURL: true)?.queryItems + } + .reduce([], +) + + maxID = queryItems.first { $0.name == "max_id" }?.value + minID = queryItems.first { $0.name == "min_id" }?.value + sinceID = queryItems.first { $0.name == "since_id" }?.value + } + + return PagedResult.Info(maxID: maxID, minID: minID, sinceID: sinceID) + } + + return decoded.zip(info).map(PagedResult.init(result:info:)).eraseToAnyPublisher() + } +} + +private extension MastodonAPIClient { + // swiftlint:disable force_try + static let linkDataDetector = try! NSDataDetector(types: NSTextCheckingResult.CheckingType.link.rawValue) + // swiftlint:enable force_try + + func target(endpoint: E) -> MastodonAPITarget { + MastodonAPITarget(baseURL: instanceURL, endpoint: endpoint, accessToken: accessToken) } } diff --git a/ServiceLayer/Sources/ServiceLayer/Services/StatusListService.swift b/ServiceLayer/Sources/ServiceLayer/Services/StatusListService.swift index deb6b14..07a1c0b 100644 --- a/ServiceLayer/Sources/ServiceLayer/Services/StatusListService.swift +++ b/ServiceLayer/Sources/ServiceLayer/Services/StatusListService.swift @@ -8,7 +8,7 @@ import MastodonAPI public struct StatusListService { public let statusSections: AnyPublisher<[[Status]], Error> - public let paginates: Bool + public let nextPageMaxIDs: AnyPublisher public let contextParentID: String? public let title: String? @@ -35,15 +35,18 @@ extension StatusListService { title = "#".appending(tag) } + let nextPageMaxIDsSubject = PassthroughSubject() + self.init(statusSections: contentDatabase.statusesObservation(timeline: timeline), - paginates: true, + nextPageMaxIDs: nextPageMaxIDsSubject.eraseToAnyPublisher(), contextParentID: nil, title: title, filterContext: filterContext, mastodonAPIClient: mastodonAPIClient, contentDatabase: contentDatabase) { maxID, minID in - mastodonAPIClient.request(Paged(timeline.endpoint, maxID: maxID, minID: minID)) - .flatMap { contentDatabase.insert(statuses: $0, timeline: timeline) } + mastodonAPIClient.pagedRequest(timeline.endpoint, maxID: maxID, minID: minID) + .handleEvents(receiveOutput: { nextPageMaxIDsSubject.send($0.info.maxID) }) + .flatMap { contentDatabase.insert(statuses: $0.result, timeline: timeline) } .eraseToAnyPublisher() } } @@ -53,11 +56,13 @@ extension StatusListService { collection: CurrentValueSubject, mastodonAPIClient: MastodonAPIClient, contentDatabase: ContentDatabase) { + let nextPageMaxIDsSubject = PassthroughSubject() + self.init( statusSections: collection .flatMap { contentDatabase.statusesObservation(accountID: accountID, collection: $0) } .eraseToAnyPublisher(), - paginates: true, + nextPageMaxIDs: nextPageMaxIDsSubject.eraseToAnyPublisher(), contextParentID: nil, title: nil, filterContext: .account, @@ -83,8 +88,9 @@ extension StatusListService { excludeReplies: excludeReplies, onlyMedia: onlyMedia, pinned: false) - return mastodonAPIClient.request(Paged(endpoint, maxID: maxID, minID: minID)) - .flatMap { contentDatabase.insert(statuses: $0, accountID: accountID, collection: collection.value) } + return mastodonAPIClient.pagedRequest(endpoint, maxID: maxID, minID: minID) + .handleEvents(receiveOutput: { nextPageMaxIDsSubject.send($0.info.maxID) }) + .flatMap { contentDatabase.insert(statuses: $0.result, accountID: accountID, collection: collection.value) } .eraseToAnyPublisher() } } @@ -113,7 +119,7 @@ public extension StatusListService { func contextService(statusID: String) -> Self { Self(statusSections: contentDatabase.contextObservation(parentID: statusID), - paginates: false, + nextPageMaxIDs: Empty().eraseToAnyPublisher(), contextParentID: statusID, title: nil, filterContext: .thread, diff --git a/View Controllers/CollectionViewController.swift b/View Controllers/CollectionViewController.swift index 4aebf14..4564c03 100644 --- a/View Controllers/CollectionViewController.swift +++ b/View Controllers/CollectionViewController.swift @@ -147,11 +147,10 @@ class CollectionViewController: UITableViewController { extension CollectionViewController: UITableViewDataSourcePrefetching { func tableView(_ tableView: UITableView, prefetchRowsAt indexPaths: [IndexPath]) { guard - viewModel.paginates, + let maxID = viewModel.nextPageMaxID, let indexPath = indexPaths.last, indexPath.section == dataSource.numberOfSections(in: tableView) - 1, - indexPath.row == dataSource.tableView(tableView, numberOfRowsInSection: indexPath.section) - 1, - let maxID = dataSource.itemIdentifier(for: indexPath)?.id + indexPath.row == dataSource.tableView(tableView, numberOfRowsInSection: indexPath.section) - 1 else { return } viewModel.request(maxID: maxID, minID: nil) diff --git a/ViewModels/Sources/ViewModels/CollectionViewModel.swift b/ViewModels/Sources/ViewModels/CollectionViewModel.swift index 5465e70..5aad1f2 100644 --- a/ViewModels/Sources/ViewModels/CollectionViewModel.swift +++ b/ViewModels/Sources/ViewModels/CollectionViewModel.swift @@ -9,7 +9,7 @@ public protocol CollectionViewModel { var alertItems: AnyPublisher { get } var loading: AnyPublisher { get } var navigationEvents: AnyPublisher { get } - var paginates: Bool { get } + var nextPageMaxID: String? { get } var maintainScrollPositionOfItem: CollectionItem? { get } func request(maxID: String?, minID: String?) func itemSelected(_ item: CollectionItem) diff --git a/ViewModels/Sources/ViewModels/StatusListViewModel.swift b/ViewModels/Sources/ViewModels/StatusListViewModel.swift index b1b21ae..f76f67b 100644 --- a/ViewModels/Sources/ViewModels/StatusListViewModel.swift +++ b/ViewModels/Sources/ViewModels/StatusListViewModel.swift @@ -9,6 +9,7 @@ public class StatusListViewModel: ObservableObject { @Published public private(set) var items = [[CollectionItem]]() @Published public var alertItem: AlertItem? public let navigationEvents: AnyPublisher + public private(set) var nextPageMaxID: String? public private(set) var maintainScrollPositionOfItem: CollectionItem? private var statuses = [String: Status]() @@ -36,6 +37,10 @@ public class StatusListViewModel: ObservableObject { .assignErrorsToAlertItem(to: \.alertItem, on: self) .map { $0.map { $0.map { CollectionItem(id: $0.id, kind: .status) } } } .assign(to: &$items) + + statusListService.nextPageMaxIDs + .sink { [weak self] in self?.nextPageMaxID = $0 } + .store(in: &cancellables) } public var title: AnyPublisher { Just(statusListService.title).eraseToAnyPublisher() } @@ -96,8 +101,6 @@ extension StatusListViewModel: CollectionViewModel { } public extension StatusListViewModel { - var paginates: Bool { statusListService.paginates } - var contextParentID: String? { statusListService.contextParentID } func statusViewModel(id: String) -> StatusViewModel? {