Skip to content

Commit

Permalink
Use ApiClient in ApiOperation to use retry-mechanism (#761)
Browse files Browse the repository at this point in the history
  • Loading branch information
michalrentka authored Sep 4, 2023
1 parent d2b6c5e commit 92bcf60
Show file tree
Hide file tree
Showing 5 changed files with 33 additions and 54 deletions.
5 changes: 0 additions & 5 deletions Zotero/Controllers/API/ApiClient.swift
Original file line number Diff line number Diff line change
Expand Up @@ -45,9 +45,4 @@ protocol ApiClient: AnyObject {
func upload(request: ApiRequest, queue: DispatchQueue, multipartFormData: @escaping (MultipartFormData) -> Void) -> Single<(Data?, HTTPURLResponse)>
func upload(request: ApiRequest, data: Data, queue: DispatchQueue) -> Single<(Data?, HTTPURLResponse)>
func upload(request: ApiRequest, fromFile file: File, queue: DispatchQueue) -> Single<(Data?, HTTPURLResponse)>
func operation(from request: ApiRequest, queue: DispatchQueue, completion: @escaping (Swift.Result<(Data?, HTTPURLResponse), Error>) -> Void) -> ApiOperation
}

protocol ApiRequestCreator: AnyObject {
func dataRequest(for request: ApiRequest) -> DataRequest
}
51 changes: 19 additions & 32 deletions Zotero/Controllers/API/ApiOperation.swift
Original file line number Diff line number Diff line change
Expand Up @@ -8,60 +8,47 @@

import Foundation

import Alamofire
import RxSwift

class ApiOperation: AsynchronousOperation {
private let apiRequest: ApiRequest
private let queue: DispatchQueue
private let responseQueue: DispatchQueue
private let completion: (Swift.Result<(Data?, HTTPURLResponse), Error>) -> Void
private unowned let requestCreator: ApiRequestCreator
private unowned let apiClient: ApiClient

private var request: DataRequest?
private var disposeBag: DisposeBag?

init(apiRequest: ApiRequest, requestCreator: ApiRequestCreator, queue: DispatchQueue, completion: @escaping (Swift.Result<(Data?, HTTPURLResponse), Error>) -> Void) {
init(apiRequest: ApiRequest, apiClient: ApiClient, responseQueue: DispatchQueue, completion: @escaping (Swift.Result<(Data?, HTTPURLResponse), Error>) -> Void) {
self.apiRequest = apiRequest
self.queue = queue
self.apiClient = apiClient
self.responseQueue = responseQueue
self.completion = completion
self.requestCreator = requestCreator

super.init()
}

override func main() {
super.main()

if let request = self.request {
request.resume()
return
if self.disposeBag != nil {
self.disposeBag = nil
}

var logData: ApiLogger.StartData?
let request = self.requestCreator.dataRequest(for: self.apiRequest)

request.onURLRequestCreation(on: self.queue) { request in
logData = ApiLogger.log(urlRequest: request, encoding: self.apiRequest.encoding, logParams: self.apiRequest.logParams)
}

request.response(queue: self.queue) { [weak self] response in
guard let self = self else { return }

switch response.logAndCreateResult(logData: logData) {
case .success(let data):
let disposeBag = DisposeBag()
self.apiClient.send(request: apiRequest, queue: self.responseQueue)
.subscribe(with: self, onSuccess: { `self`, data in
self.completion(.success(data))

case .failure(let error):
self.finish()
}, onFailure: { `self`, error in
self.completion(.failure(error))
}

self.finish()
}

request.resume()
self.request = request
self.finish()
})
.disposed(by: disposeBag)
self.disposeBag = disposeBag
}

override func cancel() {
super.cancel()
self.request?.cancel()
self.disposeBag = nil
}
}
12 changes: 0 additions & 12 deletions Zotero/Controllers/API/ZoteroApiClient.swift
Original file line number Diff line number Diff line change
Expand Up @@ -114,11 +114,6 @@ final class ZoteroApiClient: ApiClient {
}
}

/// Creates ApiOperation which performs request.
func operation(from request: ApiRequest, queue: DispatchQueue, completion: @escaping (Swift.Result<(Data?, HTTPURLResponse), Error>) -> Void) -> ApiOperation {
return ApiOperation(apiRequest: request, requestCreator: self, queue: queue, completion: completion)
}

/// Creates download request. Request needs to be started manually.
func download(request: ApiDownloadRequest, queue: DispatchQueue) -> Observable<DownloadRequest> {
let convertible = Convertible(request: request, baseUrl: self.url, token: self.token(for: request.endpoint), additionalHeaders: self.manager.sessionConfiguration.httpAdditionalHeaders)
Expand Down Expand Up @@ -202,13 +197,6 @@ final class ZoteroApiClient: ApiClient {
}
}

extension ZoteroApiClient: ApiRequestCreator {
func dataRequest(for request: ApiRequest) -> DataRequest {
let convertible = Convertible(request: request, baseUrl: self.url, token: self.token(for: request.endpoint), additionalHeaders: self.manager.sessionConfiguration.httpAdditionalHeaders)
return self.createRequest(for: request.endpoint) { $0.request(convertible).validate(acceptableStatusCodes: request.acceptableStatusCodes) }
}
}

extension ResponseHeaders {
var lastModifiedVersion: Int {
// Workaround for broken headers (stored in case-sensitive dictionary)
Expand Down
15 changes: 12 additions & 3 deletions Zotero/Controllers/Sync/SyncBatchProcessor.swift
Original file line number Diff line number Diff line change
Expand Up @@ -34,8 +34,17 @@ final class SyncBatchProcessor {

// MARK: - Lifecycle

init(batches: [DownloadBatch], userId: Int, apiClient: ApiClient, dbStorage: DbStorage, fileStorage: FileStorage, schemaController: SchemaController, dateParser: DateParser,
progress: @escaping (Int) -> Void, completion: @escaping (Result<SyncBatchResponse, Error>) -> Void) {
init(
batches: [DownloadBatch],
userId: Int,
apiClient: ApiClient,
dbStorage: DbStorage,
fileStorage: FileStorage,
schemaController: SchemaController,
dateParser: DateParser,
progress: @escaping (Int) -> Void,
completion: @escaping (Result<SyncBatchResponse, Error>) -> Void
) {
let queue = OperationQueue()
queue.maxConcurrentOperationCount = 4
queue.qualityOfService = .userInteractive
Expand Down Expand Up @@ -68,7 +77,7 @@ final class SyncBatchProcessor {
let operations = self.batches.map { batch -> ApiOperation in
let keysString = batch.keys.map({ "\($0)" }).joined(separator: ",")
let request = ObjectsRequest(libraryId: batch.libraryId, userId: self.userId, objectType: batch.object, keys: keysString)
return self.apiClient.operation(from: request, queue: self.storageQueue) { [weak self] result in
return ApiOperation(apiRequest: request, apiClient: self.apiClient, responseQueue: self.storageQueue) { [weak self] result in
self?.process(result: result, batch: batch)
}
}
Expand Down
4 changes: 2 additions & 2 deletions Zotero/Controllers/WebDAV/WebDavController.swift
Original file line number Diff line number Diff line change
Expand Up @@ -249,15 +249,15 @@ final class WebDavControllerImpl: WebDavController {

for key in keys {
let propRequest = WebDavDeleteRequest(url: url.appendingPathComponent(key + ".prop"))
let propOperation = self.apiClient.operation(from: propRequest, queue: queue) { result in
let propOperation = ApiOperation(apiRequest: propRequest, apiClient: self.apiClient, responseQueue: queue) { result in
queue.async(flags: .barrier) {
processResult(key, result)
}
}
operations.append(propOperation)

let zipRequest = WebDavDeleteRequest(url: url.appendingPathComponent(key + ".zip"))
let zipOperation = self.apiClient.operation(from: zipRequest, queue: queue) { result in
let zipOperation = ApiOperation(apiRequest: zipRequest, apiClient: self.apiClient, responseQueue: queue) { result in
queue.async(flags: .barrier) {
processResult(key, result)
}
Expand Down

0 comments on commit 92bcf60

Please sign in to comment.