Skip to content

Commit 06a9183

Browse files
tomerdtomerd
committed
some refactoring
motivation: better code structure changes: * abstract errors as enum instead of nested structs * make configurationa nested class
1 parent a6c9f86 commit 06a9183

File tree

4 files changed

+69
-93
lines changed

4 files changed

+69
-93
lines changed

‎Sources/NIOHTTPClient/HTTPHandler.swift

Lines changed: 7 additions & 29 deletions
Original file line numberDiff line numberDiff line change
@@ -18,28 +18,6 @@ import NIOConcurrencyHelpers
1818
import NIOHTTP1
1919
import NIOSSL
2020

21-
protocol HTTPClientError: Error {}
22-
23-
public struct HTTPClientErrors {
24-
public struct InvalidURLError: HTTPClientError {}
25-
26-
public struct EmptyHostError: HTTPClientError {}
27-
28-
public struct AlreadyShutdown: HTTPClientError {}
29-
30-
public struct EmptySchemeError: HTTPClientError {}
31-
32-
public struct UnsupportedSchemeError: HTTPClientError {
33-
var scheme: String
34-
}
35-
36-
public struct ReadTimeoutError: HTTPClientError {}
37-
38-
public struct RemoteConnectionClosedError: HTTPClientError {}
39-
40-
public struct CancelledError: HTTPClientError {}
41-
}
42-
4321
public enum HTTPBody: Equatable {
4422
case byteBuffer(ByteBuffer)
4523
case data(Data)
@@ -68,23 +46,23 @@ public struct HTTPRequest: Equatable {
6846

6947
public init(url: String, version: HTTPVersion = HTTPVersion(major: 1, minor: 1), method: HTTPMethod = .GET, headers: HTTPHeaders = HTTPHeaders(), body: HTTPBody? = nil) throws {
7048
guard let url = URL(string: url) else {
71-
throw HTTPClientErrors.InvalidURLError()
49+
throw HTTPClientError.invalidURL
7250
}
7351

7452
try self.init(url: url, version: version, method: method, headers: headers, body: body)
7553
}
7654

7755
public init(url: URL, version: HTTPVersion, method: HTTPMethod = .GET, headers: HTTPHeaders = HTTPHeaders(), body: HTTPBody? = nil) throws {
7856
guard let scheme = url.scheme else {
79-
throw HTTPClientErrors.EmptySchemeError()
57+
throw HTTPClientError.emptyScheme
8058
}
8159

8260
guard HTTPRequest.isSchemeSupported(scheme: scheme) else {
83-
throw HTTPClientErrors.UnsupportedSchemeError(scheme: scheme)
61+
throw HTTPClientError.unsupportedScheme(scheme)
8462
}
8563

8664
guard let host = url.host else {
87-
throw HTTPClientErrors.EmptyHostError()
65+
throw HTTPClientError.emptyHost
8866
}
8967

9068
self.version = version
@@ -378,12 +356,12 @@ class HTTPTaskHandler<T: HTTPResponseDelegate>: ChannelInboundHandler, ChannelOu
378356
func userInboundEventTriggered(context: ChannelHandlerContext, event: Any) {
379357
if (event as? IdleStateHandler.IdleStateEvent) == .read {
380358
self.state = .end
381-
let error = HTTPClientErrors.ReadTimeoutError()
359+
let error = HTTPClientError.readTimeout
382360
delegate.didReceiveError(task: self.task, error)
383361
promise.fail(error)
384362
} else if (event as? CancelEvent) != nil {
385363
self.state = .end
386-
let error = HTTPClientErrors.CancelledError()
364+
let error = HTTPClientError.cancelled
387365
delegate.didReceiveError(task: self.task, error)
388366
promise.fail(error)
389367
} else {
@@ -397,7 +375,7 @@ class HTTPTaskHandler<T: HTTPResponseDelegate>: ChannelInboundHandler, ChannelOu
397375
break
398376
default:
399377
self.state = .end
400-
let error = HTTPClientErrors.RemoteConnectionClosedError()
378+
let error = HTTPClientError.remoteConnectionClosed
401379
delegate.didReceiveError(task: self.task, error)
402380
promise.fail(error)
403381
}

‎Sources/NIOHTTPClient/RequestValidation.swift

Lines changed: 2 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -15,12 +15,6 @@
1515
import NIO
1616
import NIOHTTP1
1717

18-
extension HTTPClientErrors {
19-
public struct IdentityCodingIncorrectlyPresentError: HTTPClientError {}
20-
21-
public struct ChunkedSpecifiedMultipleTimesError: HTTPClientError {}
22-
}
23-
2418
extension HTTPHeaders {
2519
mutating func validate(body: HTTPBody?) throws {
2620
// validate transfer encoding and content length (https://tools.ietf.org/html/rfc7230#section-3.3.1)
@@ -29,15 +23,15 @@ extension HTTPHeaders {
2923
let encodings = self[canonicalForm: "Transfer-Encoding"].map { $0.lowercased() }
3024

3125
guard !encodings.contains("identity") else {
32-
throw HTTPClientErrors.IdentityCodingIncorrectlyPresentError()
26+
throw HTTPClientError.identityCodingIncorrectlyPresent
3327
}
3428

3529
self.remove(name: "Transfer-Encoding")
3630
self.remove(name: "Content-Length")
3731

3832
if let body = body {
3933
guard (encodings.filter { $0 == "chunked" }.count <= 1) else {
40-
throw HTTPClientErrors.ChunkedSpecifiedMultipleTimesError()
34+
throw HTTPClientError.chunkedSpecifiedMultipleTimes
4135
}
4236

4337
if encodings.isEmpty {

‎Sources/NIOHTTPClient/SwiftNIOHTTP.swift

Lines changed: 44 additions & 31 deletions
Original file line numberDiff line numberDiff line change
@@ -23,42 +23,14 @@ public enum EventLoopGroupProvider {
2323
case createNew
2424
}
2525

26-
public struct Timeout {
27-
public var connect: TimeAmount?
28-
public var read: TimeAmount?
29-
30-
public init(connectTimeout: TimeAmount? = nil, readTimeout: TimeAmount? = nil) {
31-
self.connect = connectTimeout
32-
self.read = readTimeout
33-
}
34-
}
35-
36-
public struct HTTPClientConfiguration {
37-
public var tlsConfiguration: TLSConfiguration?
38-
public var followRedirects: Bool
39-
public var timeout: Timeout
40-
41-
public init(tlsConfiguration: TLSConfiguration? = nil, followRedirects: Bool = false, timeout: Timeout = Timeout()) {
42-
self.tlsConfiguration = tlsConfiguration
43-
self.followRedirects = followRedirects
44-
self.timeout = timeout
45-
}
46-
47-
public init(certificateVerification: CertificateVerification, followRedirects: Bool = false, timeout: Timeout = Timeout()) {
48-
self.tlsConfiguration = TLSConfiguration.forClient(certificateVerification: certificateVerification)
49-
self.followRedirects = followRedirects
50-
self.timeout = timeout
51-
}
52-
}
53-
5426
public class HTTPClient {
5527
let eventLoopGroupProvider: EventLoopGroupProvider
5628
let group: EventLoopGroup
57-
let configuration: HTTPClientConfiguration
29+
let configuration: Configuration
5830

5931
let isShutdown = Atomic<Bool>(value: false)
6032

61-
public init(eventLoopGroupProvider: EventLoopGroupProvider, configuration: HTTPClientConfiguration = HTTPClientConfiguration()) {
33+
public init(eventLoopGroupProvider: EventLoopGroupProvider, configuration: Configuration = Configuration()) {
6234
self.eventLoopGroupProvider = eventLoopGroupProvider
6335
switch self.eventLoopGroupProvider {
6436
case .shared(let group):
@@ -87,7 +59,7 @@ public class HTTPClient {
8759
if self.isShutdown.compareAndExchange(expected: false, desired: true) {
8860
try self.group.syncShutdownGracefully()
8961
} else {
90-
throw HTTPClientErrors.AlreadyShutdown()
62+
throw HTTPClientError.alreadyShutdown
9163
}
9264
}
9365
}
@@ -197,4 +169,45 @@ public class HTTPClient {
197169
return channel.eventLoop.makeSucceededFuture(())
198170
}
199171
}
172+
173+
public struct Configuration {
174+
public var tlsConfiguration: TLSConfiguration?
175+
public var followRedirects: Bool
176+
public var timeout: Timeout
177+
178+
public init(tlsConfiguration: TLSConfiguration? = nil, followRedirects: Bool = false, timeout: Timeout = Timeout()) {
179+
self.tlsConfiguration = tlsConfiguration
180+
self.followRedirects = followRedirects
181+
self.timeout = timeout
182+
}
183+
184+
public init(certificateVerification: CertificateVerification, followRedirects: Bool = false, timeout: Timeout = Timeout()) {
185+
self.tlsConfiguration = TLSConfiguration.forClient(certificateVerification: certificateVerification)
186+
self.followRedirects = followRedirects
187+
self.timeout = timeout
188+
}
189+
}
190+
191+
public struct Timeout {
192+
public var connect: TimeAmount?
193+
public var read: TimeAmount?
194+
195+
public init(connect: TimeAmount? = nil, read: TimeAmount? = nil) {
196+
self.connect = connect
197+
self.read = read
198+
}
199+
}
200+
}
201+
202+
public enum HTTPClientError: Error {
203+
case invalidURL
204+
case emptyHost
205+
case alreadyShutdown
206+
case emptyScheme
207+
case unsupportedScheme(String)
208+
case readTimeout
209+
case remoteConnectionClosed
210+
case cancelled
211+
case identityCodingIncorrectlyPresent
212+
case chunkedSpecifiedMultipleTimes
200213
}

‎Tests/NIOHTTPClientTests/SwiftNIOHTTPTests.swift

Lines changed: 16 additions & 25 deletions
Original file line numberDiff line numberDiff line change
@@ -117,7 +117,7 @@ class SwiftHTTPTests: XCTestCase {
117117
func testGetHttps() throws {
118118
let httpBin = HttpBin(ssl: true)
119119
let httpClient = HTTPClient(eventLoopGroupProvider: .createNew,
120-
configuration: HTTPClientConfiguration(certificateVerification: .none))
120+
configuration: HTTPClient.Configuration(certificateVerification: .none))
121121
defer {
122122
try! httpClient.syncShutdown()
123123
httpBin.shutdown()
@@ -130,7 +130,7 @@ class SwiftHTTPTests: XCTestCase {
130130
func testPostHttps() throws {
131131
let httpBin = HttpBin(ssl: true)
132132
let httpClient = HTTPClient(eventLoopGroupProvider: .createNew,
133-
configuration: HTTPClientConfiguration(certificateVerification: .none))
133+
configuration: HTTPClient.Configuration(certificateVerification: .none))
134134
defer {
135135
try! httpClient.syncShutdown()
136136
httpBin.shutdown()
@@ -152,7 +152,7 @@ class SwiftHTTPTests: XCTestCase {
152152
let httpBin = HttpBin(ssl: false)
153153
let httpsBin = HttpBin(ssl: true)
154154
let httpClient = HTTPClient(eventLoopGroupProvider: .createNew,
155-
configuration: HTTPClientConfiguration(certificateVerification: .none, followRedirects: true))
155+
configuration: HTTPClient.Configuration(certificateVerification: .none, followRedirects: true))
156156

157157
defer {
158158
try! httpClient.syncShutdown()
@@ -215,32 +215,26 @@ class SwiftHTTPTests: XCTestCase {
215215
httpBin.shutdown()
216216
}
217217

218-
do {
219-
_ = try httpClient.get(url: "http://localhost:\(httpBin.port)/close").wait()
220-
XCTFail("Should fail with RemoteConnectionClosedError")
221-
} catch _ as HTTPClientErrors.RemoteConnectionClosedError {
222-
// ok
223-
} catch {
224-
XCTFail("Unexpected error: \(error)")
218+
XCTAssertThrowsError(try httpClient.get(url: "http://localhost:\(httpBin.port)/close").wait(), "Should fail") { error in
219+
guard case HTTPClientError.remoteConnectionClosed = error else {
220+
return XCTFail("Should fail with remoteConnectionClosed")
221+
}
225222
}
226223
}
227224

228225
func testReadTimeout() throws {
229226
let httpBin = HttpBin()
230-
let httpClient = HTTPClient(eventLoopGroupProvider: .createNew, configuration: HTTPClientConfiguration(timeout: Timeout(readTimeout: .milliseconds(150))))
227+
let httpClient = HTTPClient(eventLoopGroupProvider: .createNew, configuration: HTTPClient.Configuration(timeout: HTTPClient.Timeout(read: .milliseconds(150))))
231228

232229
defer {
233230
try! httpClient.syncShutdown()
234231
httpBin.shutdown()
235232
}
236233

237-
do {
238-
_ = try httpClient.get(url: "http://localhost:\(httpBin.port)/wait").wait()
239-
XCTFail("Should fail with: ReadTimeoutError")
240-
} catch _ as HTTPClientErrors.ReadTimeoutError {
241-
// ok
242-
} catch {
243-
XCTFail("Unexpected error: \(error)")
234+
XCTAssertThrowsError(try httpClient.get(url: "http://localhost:\(httpBin.port)/wait").wait(), "Should fail") { error in
235+
guard case HTTPClientError.readTimeout = error else {
236+
return XCTFail("Should fail with readTimeout")
237+
}
244238
}
245239
}
246240

@@ -261,13 +255,10 @@ class SwiftHTTPTests: XCTestCase {
261255
task.cancel()
262256
}
263257

264-
do {
265-
_ = try task.wait()
266-
XCTFail("Should fail with: CancelledError")
267-
} catch _ as HTTPClientErrors.CancelledError {
268-
// ok
269-
} catch {
270-
XCTFail("Unexpected error: \(error)")
258+
XCTAssertThrowsError(try task.wait(), "Should fail") { error in
259+
guard case HTTPClientError.cancelled = error else {
260+
return XCTFail("Should fail with cancelled")
261+
}
271262
}
272263
}
273264
}

0 commit comments

Comments
 (0)