Skip to content

Commit

Permalink
Refactor Request Event Management (Alamofire#2796)
Browse files Browse the repository at this point in the history
* Update Request event management and add tests.

* DRY up state switch.

* Add additional test for actions from multiple queues.
  • Loading branch information
jshier authored and cnoon committed Apr 12, 2019
1 parent 6e3dec3 commit 5bedc2a
Show file tree
Hide file tree
Showing 11 changed files with 413 additions and 144 deletions.
4 changes: 0 additions & 4 deletions Alamofire.xcodeproj/project.pbxproj
Original file line number Diff line number Diff line change
Expand Up @@ -372,7 +372,6 @@
319917A4209CDAC400103A19 /* RequestTaskMap.swift */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.swift; path = RequestTaskMap.swift; sourceTree = "<group>"; };
319917A9209CDCB000103A19 /* HTTPHeaders.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = HTTPHeaders.swift; sourceTree = "<group>"; };
319917B8209CE53A00103A19 /* OperationQueue+Alamofire.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = "OperationQueue+Alamofire.swift"; sourceTree = "<group>"; };
31B2CA9421AA24F5005B371A /* [email protected] */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = "[email protected]"; sourceTree = "<group>"; };
31B2CA9521AA25CD005B371A /* Package.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = Package.swift; sourceTree = "<group>"; };
31D83FCD20D5C29300D93E47 /* URLConvertible+URLRequestConvertible.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = "URLConvertible+URLRequestConvertible.swift"; sourceTree = "<group>"; };
31DADDFA224811ED0051390F /* AlamofireExtended.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = AlamofireExtended.swift; sourceTree = "<group>"; };
Expand Down Expand Up @@ -431,7 +430,6 @@
4CE292321EF4A393008DA555 /* CHANGELOG.md */ = {isa = PBXFileReference; lastKnownFileType = net.daringfireball.markdown; path = CHANGELOG.md; sourceTree = "<group>"; };
4CE292331EF4A393008DA555 /* CONTRIBUTING.md */ = {isa = PBXFileReference; lastKnownFileType = net.daringfireball.markdown; path = CONTRIBUTING.md; sourceTree = "<group>"; };
4CE292391EF4B12B008DA555 /* Alamofire.podspec */ = {isa = PBXFileReference; lastKnownFileType = text; path = Alamofire.podspec; sourceTree = "<group>"; };
4CE2923A1EF4B12B008DA555 /* [email protected] */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = "[email protected]"; sourceTree = "<group>"; };
4CF3B4281F5FC7900075BE59 /* LICENSE */ = {isa = PBXFileReference; lastKnownFileType = text; path = LICENSE; sourceTree = "<group>"; };
4CF626EF1BA7CB3E0011A099 /* Alamofire.framework */ = {isa = PBXFileReference; explicitFileType = wrapper.framework; includeInIndex = 0; path = Alamofire.framework; sourceTree = BUILT_PRODUCTS_DIR; };
4CF626F81BA7CB3E0011A099 /* Alamofire tvOS Tests.xctest */ = {isa = PBXFileReference; explicitFileType = wrapper.cfbundle; includeInIndex = 0; path = "Alamofire tvOS Tests.xctest"; sourceTree = BUILT_PRODUCTS_DIR; };
Expand Down Expand Up @@ -762,8 +760,6 @@
4CE292391EF4B12B008DA555 /* Alamofire.podspec */,
4CF3B4281F5FC7900075BE59 /* LICENSE */,
31B2CA9521AA25CD005B371A /* Package.swift */,
31B2CA9421AA24F5005B371A /* [email protected] */,
4CE2923A1EF4B12B008DA555 /* [email protected] */,
);
name = Deployment;
sourceTree = "<group>";
Expand Down
28 changes: 0 additions & 28 deletions [email protected]

This file was deleted.

41 changes: 0 additions & 41 deletions [email protected]

This file was deleted.

41 changes: 0 additions & 41 deletions [email protected]

This file was deleted.

45 changes: 45 additions & 0 deletions Source/EventMonitor.swift
Original file line number Diff line number Diff line change
Expand Up @@ -130,12 +130,21 @@ public protocol EventMonitor {
/// Event called when a `Request` receives a `resume` call.
func requestDidResume(_ request: Request)

/// Event called when a `Request`'s associated `URLSessionTask` is resumed.
func request(_ request: Request, didResumeTask task: URLSessionTask)

/// Event called when a `Request` receives a `suspend` call.
func requestDidSuspend(_ request: Request)

/// Event called when a `Request`'s associated `URLSessionTask` is suspended.
func request(_ request: Request, didSuspendTask task: URLSessionTask)

/// Event called when a `Request` receives a `cancel` call.
func requestDidCancel(_ request: Request)

/// Event called when a `Request`'s associated `URLSessionTask` is cancelled.
func request(_ request: Request, didCancelTask task: URLSessionTask)

// MARK: DataRequest Events

/// Event called when a `DataRequest` calls a `Validation`.
Expand Down Expand Up @@ -242,8 +251,11 @@ extension EventMonitor {
public func requestIsRetrying(_ request: Request) { }
public func requestDidFinish(_ request: Request) { }
public func requestDidResume(_ request: Request) { }
public func request(_ request: Request, didResumeTask task: URLSessionTask) { }
public func requestDidSuspend(_ request: Request) { }
public func request(_ request: Request, didSuspendTask task: URLSessionTask) { }
public func requestDidCancel(_ request: Request) { }
public func request(_ request: Request, didCancelTask task: URLSessionTask) { }
public func request(_ request: DataRequest,
didValidateRequest urlRequest: URLRequest?,
response: HTTPURLResponse,
Expand Down Expand Up @@ -424,14 +436,26 @@ public final class CompositeEventMonitor: EventMonitor {
performEvent { $0.requestDidResume(request) }
}

public func request(_ request: Request, didResumeTask task: URLSessionTask) {
performEvent { $0.request(request, didResumeTask: task) }
}

public func requestDidSuspend(_ request: Request) {
performEvent { $0.requestDidSuspend(request) }
}

public func request(_ request: Request, didSuspendTask task: URLSessionTask) {
performEvent { $0.request(request, didSuspendTask: task) }
}

public func requestDidCancel(_ request: Request) {
performEvent { $0.requestDidCancel(request) }
}

public func request(_ request: Request, didCancelTask task: URLSessionTask) {
performEvent { $0.request(request, didCancelTask: task) }
}

public func request(_ request: DataRequest,
didValidateRequest urlRequest: URLRequest?,
response: HTTPURLResponse,
Expand Down Expand Up @@ -571,12 +595,21 @@ open class ClosureEventMonitor: EventMonitor {
/// Closure called on the `requestDidResume(_:)` event.
open var requestDidResume: ((Request) -> Void)?

/// Closure called on the `request(_:didResumeTask:)` event.
open var requestDidResumeTask: ((Request, URLSessionTask) -> Void)?

/// Closure called on the `requestDidSuspend(_:)` event.
open var requestDidSuspend: ((Request) -> Void)?

/// Closure called on the `request(_:didSuspendTask:)` event.
open var requestDidSuspendTask: ((Request, URLSessionTask) -> Void)?

/// Closure called on the `requestDidCancel(_:)` event.
open var requestDidCancel: ((Request) -> Void)?

/// Closure called on the `request(_:didCancelTask:)` event.
open var requestDidCancelTask: ((Request, URLSessionTask) -> Void)?

/// Closure called on the `request(_:didValidateRequest:response:data:withResult:)` event.
open var requestDidValidateRequestResponseDataWithResult: ((DataRequest, URLRequest?, HTTPURLResponse, Data?, Request.ValidationResult) -> Void)?

Expand Down Expand Up @@ -722,14 +755,26 @@ open class ClosureEventMonitor: EventMonitor {
requestDidResume?(request)
}

public func request(_ request: Request, didResumeTask task: URLSessionTask) {
requestDidResumeTask?(request, task)
}

open func requestDidSuspend(_ request: Request) {
requestDidSuspend?(request)
}

public func request(_ request: Request, didSuspendTask task: URLSessionTask) {
requestDidSuspendTask?(request, task)
}

open func requestDidCancel(_ request: Request) {
requestDidCancel?(request)
}

public func request(_ request: Request, didCancelTask task: URLSessionTask) {
requestDidCancelTask?(request, task)
}

open func request(_ request: DataRequest,
didValidateRequest urlRequest: URLRequest?,
response: HTTPURLResponse,
Expand Down
16 changes: 16 additions & 0 deletions Source/Protector.swift
Original file line number Diff line number Diff line change
Expand Up @@ -128,3 +128,19 @@ extension Protector where T == Data? {
}
}
}

extension Protector where T == Request.MutableState {
/// Attempts to transition to the passed `State`.
///
/// - Parameter state: The `State` to attempt transition to.
/// - Returns: Whether the transtion occured.
func attemptToTransitionTo(_ state: Request.State) -> Bool {
return lock.around {
guard value.state.canTransitionTo(state) else { return false }

value.state = state

return true
}
}
}
35 changes: 22 additions & 13 deletions Source/Request.swift
Original file line number Diff line number Diff line change
Expand Up @@ -66,12 +66,12 @@ open class Request {
/// The `Request`'s interceptor.
public let interceptor: RequestInterceptor?
/// The `Request`'s delegate.
public weak var delegate: RequestDelegate?
public private(set) weak var delegate: RequestDelegate?

// MARK: - Updated State

/// Type encapsulating all mutable state that may need to be accessed from anything other than the `underlyingQueue`.
private struct MutableState {
struct MutableState {
/// State of the `Request`.
var state: State = .initialized
/// `ProgressHandler` and `DispatchQueue` provided for upload progress callbacks.
Expand Down Expand Up @@ -303,18 +303,33 @@ open class Request {
eventMonitor?.requestDidResume(self)
}

/// Called when a `URLSessionTask` is resumed on behalf of the instance.
func didResumeTask(_ task: URLSessionTask) {
eventMonitor?.request(self, didResumeTask: task)
}

/// Called when suspension is completed.
func didSuspend() {
eventMonitor?.requestDidSuspend(self)
}

/// Callend when a `URLSessionTask` is suspended on behalf of the instance.
func didSuspendTask(_ task: URLSessionTask) {
eventMonitor?.request(self, didSuspendTask: task)
}

/// Called when cancellation is completed, sets `error` to `AFError.explicitlyCancelled`.
func didCancel() {
error = AFError.explicitlyCancelled

eventMonitor?.requestDidCancel(self)
}

/// Called when a `URLSessionTask` is cancelled on behalf of the instance.
func didCancelTask(_ task: URLSessionTask) {
eventMonitor?.request(self, didCancelTask: task)
}

/// Called when a `URLSessionTaskMetrics` value is gathered on behalf of the `Request`.
func didGatherMetrics(_ metrics: URLSessionTaskMetrics) {
protectedMutableState.write { $0.metrics.append(metrics) }
Expand Down Expand Up @@ -465,9 +480,7 @@ open class Request {
/// - Returns: The `Request`.
@discardableResult
open func cancel() -> Self {
guard state.canTransitionTo(.cancelled) else { return self }

state = .cancelled
guard protectedMutableState.attemptToTransitionTo(.cancelled) else { return self }

delegate?.cancelRequest(self)

Expand All @@ -479,9 +492,7 @@ open class Request {
/// - Returns: The `Request`.
@discardableResult
open func suspend() -> Self {
guard state.canTransitionTo(.suspended) else { return self }

state = .suspended
guard protectedMutableState.attemptToTransitionTo(.suspended) else { return self }

delegate?.suspendRequest(self)

Expand All @@ -494,9 +505,7 @@ open class Request {
/// - Returns: The `Request`.
@discardableResult
open func resume() -> Self {
guard state.canTransitionTo(.resumed) else { return self }

state = .resumed
guard protectedMutableState.attemptToTransitionTo(.resumed) else { return self }

delegate?.resumeRequest(self)

Expand Down Expand Up @@ -877,12 +886,12 @@ open class DownloadRequest: Request {

// MARK: Updated State

private struct MutableState {
private struct DownloadRequestMutableState {
var resumeData: Data?
var fileURL: URL?
}

private let protectedMutableState: Protector<MutableState> = Protector(MutableState())
private let protectedMutableState: Protector<DownloadRequestMutableState> = Protector(DownloadRequestMutableState())

public var resumeData: Data? { return protectedMutableState.directValue.resumeData }
public var fileURL: URL? { return protectedMutableState.directValue.fileURL }
Expand Down
Loading

0 comments on commit 5bedc2a

Please sign in to comment.