Skip to content

Add no-internet connection monitoring #52

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 10 commits into
base: develop
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 4 additions & 1 deletion Package.swift
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,10 @@ let package = Package(
// Targets are the basic building blocks of a package. A target can define a module or a test suite.
// Targets can depend on other targets in this package, and on products in packages which this package depends on.
.target(
name: "Jetworking"
name: "Jetworking",
linkerSettings: [
.linkedFramework("SystemConfiguration")
]
),
.testTarget(
name: "JetworkingTests",
Expand Down
37 changes: 29 additions & 8 deletions Sources/Jetworking/Client/Client.swift
Original file line number Diff line number Diff line change
Expand Up @@ -9,11 +9,21 @@ enum APIError: Error {

public final class Client {
public typealias RequestCompletion<ResponseType> = (HTTPURLResponse?, Result<ResponseType, Error>) -> Void

internal enum Task {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do we really need the additional task abstraction?

case dataTask(request: URLRequest, completion: ((Data?, URLResponse?, Error?) -> Void))
case downloadTask(request: URLRequest)
case uploadDataTask(request: URLRequest, data: Data)
case uploadFileTask(request: URLRequest, fileURL: URL)
}

// MARK: - Properties
private let configuration: Configuration

private lazy var session: URLSession = .init(configuration: .default)

private let taskExecutor: ClientTaskExecutor = .default

private lazy var requestExecutor: RequestExecutor = {
switch configuration.requestExecutorType {
case .sync:
Expand Down Expand Up @@ -93,7 +103,7 @@ public final class Client {
public func get<ResponseType>(endpoint: Endpoint<ResponseType>, _ completion: @escaping RequestCompletion<ResponseType>) -> CancellableRequest? {
do {
let request: URLRequest = try createRequest(forHttpMethod: .GET, and: endpoint)
return requestExecutor.send(request: request) { [weak self] data, urlResponse, error in
let task: Task = .dataTask(request: request) { [weak self] data, urlResponse, error in
self?.handleResponse(
data: data,
urlResponse: urlResponse,
Expand All @@ -102,6 +112,7 @@ public final class Client {
completion: completion
)
}
return try taskExecutor.perform(task, on: requestExecutor)
} catch {
enqueue(completion(nil, .failure(error)))
}
Expand All @@ -114,7 +125,7 @@ public final class Client {
do {
let bodyData: Data = try configuration.encoder.encode(body)
let request: URLRequest = try createRequest(forHttpMethod: .POST, and: endpoint, and: bodyData)
return requestExecutor.send(request: request) { [weak self] data, urlResponse, error in
let task: Task = .dataTask(request: request) { [weak self] data, urlResponse, error in
self?.handleResponse(
data: data,
urlResponse: urlResponse,
Expand All @@ -123,6 +134,7 @@ public final class Client {
completion: completion
)
}
return try taskExecutor.perform(task, on: requestExecutor)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm not sure if this was the intention of @SimonNumberTwo that the client is throwing an error if there is no internet connection. I guess he meant that the completion of the function should be called with an error.

@SimonNumberTwo can you give some more details about the usage in our apps? What should be the expected behavior?

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes exactly. I would prefer to have an error case for no internet and let the app itself using Jetworking decide what to do with it rather than implicitly throwing an error.

} catch {
enqueue(completion(nil, .failure(error)))
}
Expand All @@ -135,7 +147,7 @@ public final class Client {
do {
let bodyData: Data = try configuration.encoder.encode(body)
let request: URLRequest = try createRequest(forHttpMethod: .PUT, and: endpoint, and: bodyData)
return requestExecutor.send(request: request) { [weak self] data, urlResponse, error in
let task: Task = .dataTask(request: request) { [weak self] data, urlResponse, error in
self?.handleResponse(
data: data,
urlResponse: urlResponse,
Expand All @@ -144,6 +156,7 @@ public final class Client {
completion: completion
)
}
return try taskExecutor.perform(task, on: requestExecutor)
} catch {
enqueue(completion(nil, .failure(error)))
}
Expand All @@ -160,7 +173,7 @@ public final class Client {
do {
let bodyData: Data = try configuration.encoder.encode(body)
let request: URLRequest = try createRequest(forHttpMethod: .PATCH, and: endpoint, and: bodyData)
return requestExecutor.send(request: request) { [weak self] data, urlResponse, error in
let task: Task = .dataTask(request: request) { [weak self] data, urlResponse, error in
self?.handleResponse(
data: data,
urlResponse: urlResponse,
Expand All @@ -169,6 +182,7 @@ public final class Client {
completion: completion
)
}
return try taskExecutor.perform(task, on: requestExecutor)
} catch {
enqueue(completion(nil, .failure(error)))
}
Expand All @@ -180,7 +194,7 @@ public final class Client {
public func delete<ResponseType>(endpoint: Endpoint<ResponseType>, parameter: [String: Any] = [:], _ completion: @escaping RequestCompletion<ResponseType>) -> CancellableRequest? {
do {
let request: URLRequest = try createRequest(forHttpMethod: .DELETE, and: endpoint)
return requestExecutor.send(request: request) { [weak self] data, urlResponse, error in
let task: Task = .dataTask(request: request) { [weak self] data, urlResponse, error in
self?.handleResponse(
data: data,
urlResponse: urlResponse,
Expand All @@ -189,6 +203,7 @@ public final class Client {
completion: completion
)
}
return try taskExecutor.perform(task, on: requestExecutor)
} catch {
enqueue(completion(nil, .failure(error)))
}
Expand All @@ -206,7 +221,7 @@ public final class Client {
guard checkForValidDownloadURL(url) else { return nil }

let request: URLRequest = .init(url: url)
let task = downloadExecutor.download(request: request)
let task = try? taskExecutor.perform(.downloadTask(request: request), on: downloadExecutor)
task.flatMap {
executingDownloads[$0.identifier] = DownloadHandler(
progressHandler: progressHandler,
Expand All @@ -224,7 +239,10 @@ public final class Client {
_ completion: @escaping UploadHandler.CompletionHandler
) -> CancellableRequest? {
let request: URLRequest = .init(url: url, httpMethod: .POST)
let task = uploadExecutor.upload(request: request, fromFile: fileURL)
let task = try? taskExecutor.perform(
.uploadFileTask(request: request, fileURL: fileURL),
on: uploadExecutor
)
task.flatMap {
executingUploads[$0.identifier] = UploadHandler(
progressHandler: progressHandler,
Expand Down Expand Up @@ -259,7 +277,10 @@ public final class Client {
// TODO: Extract into constants
request.setValue("\(multipartType.rawValue); boundary=\(boundary)", forHTTPHeaderField: "Content-Type")

let task = uploadExecutor.upload(request: request, from: multipartData)
let task = try? taskExecutor.perform(
.uploadDataTask(request: request, data: multipartData),
on: uploadExecutor
)
task.flatMap {
executingUploads[$0.identifier] = UploadHandler(
progressHandler: progressHandler,
Expand Down
51 changes: 51 additions & 0 deletions Sources/Jetworking/Client/Executor/ClientTaskExecutor.swift
Original file line number Diff line number Diff line change
@@ -0,0 +1,51 @@
import Foundation

enum ClientTaskError: Error {
case unexpectedTaskExecution

static let connectionUnavailable = NSError(
domain: URLError.errorDomain,
code: URLError.Code.notConnectedToInternet.rawValue,
userInfo: nil
)
}

final class ClientTaskExecutor: NSObject {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do we really need another Executer? The Client is now working Client.post -> Executer -> Executer. Maybe it is possible to integrate the reachability into the RequestExecuter by composition to reduce the complexity?

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Eventually you forget UploadExecuter and DownloadExecuter. I just didn't want to add the same code in those components.

internal static let `default` = ClientTaskExecutor()
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Type definitions should be on the left Side of the declaration.


private var reachabilityManager: NetworkReachabilityMonitor?

init(reachabilityMonitor: NetworkReachabilityMonitor? = NetworkReachabilityManager.default) {
self.reachabilityManager = reachabilityMonitor
super.init()

do {
try self.reachabilityManager?.startListening(on: .main) { _ in }
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Whats happening in the completion? Do we need a log here as well?

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The closure is not about completion but callback on state update. We do not really observe the reachability all the time. In this implementation we just check that a connection can be established before we perform a network request.

} catch {
NSLog("[WARNING] Faild to start network monitor. Error: \(error)")
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We do have a Logger implementation in the framework.

}
}

func perform<T>(_ task: Client.Task, on executor: T) throws -> CancellableRequest? {
guard reachabilityManager == nil || reachabilityManager?.isReachable == true else {
throw ClientTaskError.connectionUnavailable
}

switch (executor, task) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If we use composition and we add this functionality to the request executer we do not need the cast.

case (is RequestExecutor, let .dataTask(request, completionHandler)) :
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

For casting in switch cases please take a look at the example of the swift documentation:

 switch thing {
    case 0 as Int:
        print("zero as an Int")
    case 0 as Double:
        print("zero as a Double")
    case let someInt as Int:
        print("an integer value of \(someInt)")
    case let someDouble as Double where someDouble > 0:
        print("a positive double value of \(someDouble)")
    case is Double:
        print("some other double value that I don't want to print")
    case let someString as String:
        print("a string value of \"\(someString)\"")
    case let (x, y) as (Double, Double):
        print("an (x, y) point at \(x), \(y)")
    case let movie as Movie:
        print("a movie called \(movie.name), dir. \(movie.director)")
    case let stringConverter as (String) -> String:
        print(stringConverter("Michael"))
    default:
        print("something else")
    }

source: https://docs.swift.org/swift-book/LanguageGuide/TypeCasting.html

return (executor as! RequestExecutor).send(request: request, completionHandler)

case (is DownloadExecutor, let .downloadTask(request)):
return (executor as! DownloadExecutor).download(request: request)

case (is UploadExecutor, let .uploadDataTask(request, data)):
return (executor as! UploadExecutor).upload(request: request, from: data)

case (is UploadExecutor, let .uploadFileTask(request, fileURL)):
return (executor as! UploadExecutor).upload(request: request, fromFile: fileURL)

default:
throw ClientTaskError.unexpectedTaskExecution
}
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,25 @@
import Foundation
import SystemConfiguration

extension SCNetworkReachabilityFlags {
var isReachableViaCellular: Bool {
#if os(iOS)
return contains(.isWWAN)
#else
return false
#endif
}

var isReachableViaNetworkInterface: Bool {
contains(.reachable) &&
(!contains(.connectionRequired) || canEstablishConnectionAutomatically)
}

private var canEstablishConnection: Bool {
!intersection([.connectionOnTraffic, .connectionOnDemand]).isEmpty
}

private var canEstablishConnectionAutomatically: Bool {
canEstablishConnection && !contains(.interventionRequired)
}
}
43 changes: 43 additions & 0 deletions Sources/Jetworking/Utility/Reachability/NetworkReachability.swift
Original file line number Diff line number Diff line change
@@ -0,0 +1,43 @@
import Foundation
import SystemConfiguration

/// Typealias for the state of network reachability.
public typealias NetworkReachabilityState = NetworkReachability.State

/// A container for all enumerations related to network state.
public enum NetworkReachability {
/// Defines the various connection types detected by reachability flags.
public enum ConnectionInterface: Equatable {
/// Wired ethernet or WiFi.
case wiredOrWirelessLAN

/// Cellular connection.
case cellular
}

/// Defines the various states of network reachability.
public enum State: Equatable {
/// It could not be determined whether the network is reachable.
case notDetermined

/// The network is not reachable.
case unreachable

/// The network is reachable over an interface `ConnectionInterface`.
case reachable(ConnectionInterface)

init(_ flags: SCNetworkReachabilityFlags) {
guard flags.isReachableViaNetworkInterface else {
self = .unreachable
return
}

var networkStatus: Self = .reachable(.wiredOrWirelessLAN)
if flags.isReachableViaCellular {
networkStatus = .reachable(.cellular)
}

self = networkStatus
}
}
}
Loading