Skip to content

Commit d1ffddc

Browse files
tomerdtomerd
committed
make API non-throwing
motivation: cleaner API for users changes: * make request initializer optional instead of throwing * http "sugar" methods (get, post, etc) no longer need to be throwing * fix schema logic to be non-case sensitive * made request object version, method and url immutable * made request object scheme and host internal * adjusted redirect ahndler implementation to stricter request immutabllity * adjust and add tests
1 parent 8fa6c6f commit d1ffddc

File tree

4 files changed

+88
-97
lines changed

4 files changed

+88
-97
lines changed

‎Sources/AsyncHTTPClient/HTTPClient.swift

Lines changed: 15 additions & 25 deletions
Original file line numberDiff line numberDiff line change
@@ -59,48 +59,38 @@ public class HTTPClient {
5959
}
6060

6161
public func get(url: String, deadline: NIODeadline? = nil) -> EventLoopFuture<Response> {
62-
do {
63-
let request = try Request(url: url, method: .GET)
64-
return self.execute(request: request, deadline: deadline)
65-
} catch {
66-
return self.eventLoopGroup.next().makeFailedFuture(error)
62+
guard let request = Request(url: url, method: .GET) else {
63+
return self.eventLoopGroup.next().makeFailedFuture(HTTPClientError.invalidURL)
6764
}
65+
return self.execute(request: request, deadline: deadline)
6866
}
6967

7068
public func post(url: String, body: Body? = nil, deadline: NIODeadline? = nil) -> EventLoopFuture<Response> {
71-
do {
72-
let request = try HTTPClient.Request(url: url, method: .POST, body: body)
73-
return self.execute(request: request, deadline: deadline)
74-
} catch {
75-
return self.eventLoopGroup.next().makeFailedFuture(error)
69+
guard let request = Request(url: url, method: .POST, body: body) else {
70+
return self.eventLoopGroup.next().makeFailedFuture(HTTPClientError.invalidURL)
7671
}
72+
return self.execute(request: request, deadline: deadline)
7773
}
7874

7975
public func patch(url: String, body: Body? = nil, deadline: NIODeadline? = nil) -> EventLoopFuture<Response> {
80-
do {
81-
let request = try HTTPClient.Request(url: url, method: .PATCH, body: body)
82-
return self.execute(request: request, deadline: deadline)
83-
} catch {
84-
return self.eventLoopGroup.next().makeFailedFuture(error)
76+
guard let request = Request(url: url, method: .PATCH, body: body) else {
77+
return self.eventLoopGroup.next().makeFailedFuture(HTTPClientError.invalidURL)
8578
}
79+
return self.execute(request: request, deadline: deadline)
8680
}
8781

8882
public func put(url: String, body: Body? = nil, deadline: NIODeadline? = nil) -> EventLoopFuture<Response> {
89-
do {
90-
let request = try HTTPClient.Request(url: url, method: .PUT, body: body)
91-
return self.execute(request: request, deadline: deadline)
92-
} catch {
93-
return self.eventLoopGroup.next().makeFailedFuture(error)
83+
guard let request = Request(url: url, method: .PUT, body: body) else {
84+
return self.eventLoopGroup.next().makeFailedFuture(HTTPClientError.invalidURL)
9485
}
86+
return self.execute(request: request, deadline: deadline)
9587
}
9688

9789
public func delete(url: String, deadline: NIODeadline? = nil) -> EventLoopFuture<Response> {
98-
do {
99-
let request = try Request(url: url, method: .DELETE)
100-
return self.execute(request: request, deadline: deadline)
101-
} catch {
102-
return self.eventLoopGroup.next().makeFailedFuture(error)
90+
guard let request = Request(url: url, method: .DELETE) else {
91+
return self.eventLoopGroup.next().makeFailedFuture(HTTPClientError.invalidURL)
10392
}
93+
return self.execute(request: request, deadline: deadline)
10494
}
10595

10696
public func execute(request: Request, deadline: NIODeadline? = nil) -> EventLoopFuture<Response> {

‎Sources/AsyncHTTPClient/HTTPHandler.swift

Lines changed: 36 additions & 43 deletions
Original file line numberDiff line numberDiff line change
@@ -60,33 +60,34 @@ extension HTTPClient {
6060
}
6161

6262
public struct Request {
63-
public var version: HTTPVersion
64-
public var method: HTTPMethod
65-
public var url: URL
66-
public var scheme: String
67-
public var host: String
63+
public let version: HTTPVersion
64+
public let method: HTTPMethod
65+
public let url: URL
6866
public var headers: HTTPHeaders
6967
public var body: Body?
68+
69+
internal let scheme: String
70+
internal let host: String
7071

71-
public init(url: String, version: HTTPVersion = HTTPVersion(major: 1, minor: 1), method: HTTPMethod = .GET, headers: HTTPHeaders = HTTPHeaders(), body: Body? = nil) throws {
72+
public init?(url: String, version: HTTPVersion = HTTPVersion(major: 1, minor: 1), method: HTTPMethod = .GET, headers: HTTPHeaders = HTTPHeaders(), body: Body? = nil) {
7273
guard let url = URL(string: url) else {
73-
throw HTTPClientError.invalidURL
74+
return nil
7475
}
7576

76-
try self.init(url: url, version: version, method: method, headers: headers, body: body)
77+
self.init(url: url, version: version, method: method, headers: headers, body: body)
7778
}
7879

79-
public init(url: URL, version: HTTPVersion = HTTPVersion(major: 1, minor: 1), method: HTTPMethod = .GET, headers: HTTPHeaders = HTTPHeaders(), body: Body? = nil) throws {
80-
guard let scheme = url.scheme else {
81-
throw HTTPClientError.emptyScheme
80+
public init?(url: URL, version: HTTPVersion = HTTPVersion(major: 1, minor: 1), method: HTTPMethod = .GET, headers: HTTPHeaders = HTTPHeaders(), body: Body? = nil) {
81+
guard let scheme = url.scheme?.lowercased() else {
82+
return nil
8283
}
8384

8485
guard Request.isSchemeSupported(scheme: scheme) else {
85-
throw HTTPClientError.unsupportedScheme(scheme)
86+
return nil
8687
}
8788

8889
guard let host = url.host else {
89-
throw HTTPClientError.emptyHost
90+
return nil
9091
}
9192

9293
self.version = version
@@ -99,14 +100,14 @@ extension HTTPClient {
99100
}
100101

101102
public var useTLS: Bool {
102-
return self.url.scheme == "https"
103+
return self.scheme == "https"
103104
}
104105

105106
public var port: Int {
106107
return self.url.port ?? (self.useTLS ? 443 : 80)
107108
}
108109

109-
static func isSchemeSupported(scheme: String?) -> Bool {
110+
static func isSchemeSupported(scheme: String) -> Bool {
110111
return scheme == "http" || scheme == "https"
111112
}
112113
}
@@ -514,7 +515,7 @@ internal struct RedirectHandler<T> {
514515
return nil
515516
}
516517

517-
guard HTTPClient.Request.isSchemeSupported(scheme: url.scheme) else {
518+
guard HTTPClient.Request.isSchemeSupported(scheme: request.scheme) else {
518519
return nil
519520
}
520521

@@ -526,44 +527,36 @@ internal struct RedirectHandler<T> {
526527
}
527528

528529
func redirect(status: HTTPResponseStatus, to redirectURL: URL, promise: EventLoopPromise<T>) {
529-
let originalURL = self.request.url
530-
531-
var request = self.request
532-
request.url = redirectURL
533-
534-
if let redirectHost = redirectURL.host {
535-
request.host = redirectHost
536-
} else {
537-
preconditionFailure("redirectURL doesn't contain a host")
538-
}
539-
540-
if let redirectScheme = redirectURL.scheme {
541-
request.scheme = redirectScheme
542-
} else {
543-
preconditionFailure("redirectURL doesn't contain a scheme")
544-
}
530+
let originalRequest = self.request
545531

546532
var convertToGet = false
547533
if status == .seeOther, request.method != .HEAD {
548534
convertToGet = true
549535
} else if status == .movedPermanently || status == .found, request.method == .POST {
550536
convertToGet = true
551537
}
552-
538+
539+
var method = originalRequest.method
540+
var headers = originalRequest.headers
541+
var body = originalRequest.body
542+
553543
if convertToGet {
554-
request.method = .GET
555-
request.body = nil
556-
request.headers.remove(name: "Content-Length")
557-
request.headers.remove(name: "Content-Type")
544+
headers.remove(name: "Content-Length")
545+
headers.remove(name: "Content-Type")
546+
method = .GET
547+
body = nil
558548
}
559549

560-
if !originalURL.hasTheSameOrigin(as: redirectURL) {
561-
request.headers.remove(name: "Origin")
562-
request.headers.remove(name: "Cookie")
563-
request.headers.remove(name: "Authorization")
564-
request.headers.remove(name: "Proxy-Authorization")
550+
if !originalRequest.url.hasTheSameOrigin(as: redirectURL) {
551+
headers.remove(name: "Origin")
552+
headers.remove(name: "Cookie")
553+
headers.remove(name: "Authorization")
554+
headers.remove(name: "Proxy-Authorization")
555+
}
556+
557+
guard let request = HTTPClient.Request(url: redirectURL, version: originalRequest.version, method: method, headers: headers, body: body) else {
558+
return promise.fail(HTTPClientError.invalidURL)
565559
}
566-
567560
return self.execute(request).futureResult.cascade(to: promise)
568561
}
569562
}

‎Tests/AsyncHTTPClientTests/HTTPClientInternalTests.swift

Lines changed: 12 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -30,7 +30,7 @@ class HTTPClientInternalTests: XCTestCase {
3030
try channel.pipeline.addHandler(recorder).wait()
3131
try channel.pipeline.addHandler(TaskHandler(task: task, delegate: TestHTTPDelegate(), redirectHandler: nil)).wait()
3232

33-
var request = try Request(url: "http://localhost/get")
33+
var request = Request(url: "http://localhost/get")!
3434
request.headers.add(name: "X-Test-Header", value: "X-Test-Value")
3535
request.body = .string("1234")
3636

@@ -82,17 +82,13 @@ class HTTPClientInternalTests: XCTestCase {
8282
}
8383

8484
let body: HTTPClient.Body = .stream(length: 50) { writer in
85-
do {
86-
var request = try Request(url: "http://localhost:\(httpBin.port)/events/10/1")
87-
request.headers.add(name: "Accept", value: "text/event-stream")
85+
var request = Request(url: "http://localhost:\(httpBin.port)/events/10/1")!
86+
request.headers.add(name: "Accept", value: "text/event-stream")
8887

89-
let delegate = HTTPClientCopyingDelegate { part in
90-
writer.write(.byteBuffer(part))
91-
}
92-
return httpClient.execute(request: request, delegate: delegate).futureResult
93-
} catch {
94-
return httpClient.eventLoopGroup.next().makeFailedFuture(error)
88+
let delegate = HTTPClientCopyingDelegate { part in
89+
writer.write(.byteBuffer(part))
9590
}
91+
return httpClient.execute(request: request, delegate: delegate).futureResult
9692
}
9793

9894
let upload = try! httpClient.post(url: "http://localhost:\(httpBin.port)/post", body: body).wait()
@@ -118,17 +114,13 @@ class HTTPClientInternalTests: XCTestCase {
118114
XCTAssertThrowsError(try httpClient.post(url: "http://localhost:\(httpBin.port)/post", body: body).wait())
119115

120116
body = .stream(length: 50) { _ in
121-
do {
122-
var request = try Request(url: "http://localhost:\(httpBin.port)/events/10/1")
123-
request.headers.add(name: "Accept", value: "text/event-stream")
117+
var request = Request(url: "http://localhost:\(httpBin.port)/events/10/1")!
118+
request.headers.add(name: "Accept", value: "text/event-stream")
124119

125-
let delegate = HTTPClientCopyingDelegate { _ in
126-
httpClient.eventLoopGroup.next().makeFailedFuture(HTTPClientError.invalidProxyResponse)
127-
}
128-
return httpClient.execute(request: request, delegate: delegate).futureResult
129-
} catch {
130-
return httpClient.eventLoopGroup.next().makeFailedFuture(error)
120+
let delegate = HTTPClientCopyingDelegate { _ in
121+
httpClient.eventLoopGroup.next().makeFailedFuture(HTTPClientError.invalidProxyResponse)
131122
}
123+
return httpClient.execute(request: request, delegate: delegate).futureResult
132124
}
133125

134126
XCTAssertThrowsError(try httpClient.post(url: "http://localhost:\(httpBin.port)/post", body: body).wait())
@@ -172,7 +164,7 @@ class HTTPClientInternalTests: XCTestCase {
172164
httpBin.shutdown()
173165
}
174166

175-
let request = try Request(url: "http://localhost:\(httpBin.port)/custom")
167+
let request = Request(url: "http://localhost:\(httpBin.port)/custom")!
176168
let delegate = BackpressureTestDelegate(promise: httpClient.eventLoopGroup.next().makePromise())
177169
let future = httpClient.execute(request: request, delegate: delegate).futureResult
178170

‎Tests/AsyncHTTPClientTests/HTTPClientTests.swift

Lines changed: 25 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -12,7 +12,7 @@
1212
//
1313
//===----------------------------------------------------------------------===//
1414

15-
import AsyncHTTPClient
15+
@testable import AsyncHTTPClient
1616
import NIO
1717
import NIOFoundationCompat
1818
import NIOHTTP1
@@ -22,17 +22,34 @@ class HTTPClientTests: XCTestCase {
2222
typealias Request = HTTPClient.Request
2323

2424
func testRequestURI() throws {
25-
let request1 = try Request(url: "https://someserver.com:8888/some/path?foo=bar")
25+
let request1 = Request(url: "https://someserver.com:8888/some/path?foo=bar")!
2626
XCTAssertEqual(request1.host, "someserver.com")
2727
XCTAssertEqual(request1.url.path, "/some/path")
2828
XCTAssertEqual(request1.url.query!, "foo=bar")
2929
XCTAssertEqual(request1.port, 8888)
3030
XCTAssertTrue(request1.useTLS)
3131

32-
let request2 = try Request(url: "https://someserver.com")
32+
let request2 = Request(url: "https://someserver.com")!
3333
XCTAssertEqual(request2.url.path, "")
3434
}
3535

36+
func testBadRequestURI() throws {
37+
let request1 = Request(url: "file://somewhere/some/path?foo=bar")
38+
XCTAssertNil(request1)
39+
40+
let url = URL(string: "file://somewhere/some/path?foo=bar")!
41+
let request2 = Request(url: url)
42+
XCTAssertNil(request2)
43+
}
44+
45+
func testSchemaCasing() throws {
46+
let request1 = Request(url: "https://someserver.com:8888/some/path?foo=bar")!
47+
XCTAssertNotNil(request1)
48+
49+
let request2 = Request(url: "hTTpS://someserver.com:8888/some/path?foo=bar")!
50+
XCTAssertNotNil(request2)
51+
}
52+
3653
func testGet() throws {
3754
let httpBin = HttpBin()
3855
let httpClient = HTTPClient(eventLoopGroupProvider: .createNew)
@@ -55,7 +72,7 @@ class HTTPClientTests: XCTestCase {
5572
}
5673

5774
let delegate = TestHTTPDelegate()
58-
let request = try HTTPClient.Request(url: "http://localhost:\(httpBin.port)/events/10/1")
75+
let request = HTTPClient.Request(url: "http://localhost:\(httpBin.port)/events/10/1")!
5976
let task = httpClient.execute(request: request, delegate: delegate)
6077
let expectedEventLoop = task.eventLoop
6178
task.futureResult.whenComplete { (_) in
@@ -102,8 +119,7 @@ class HTTPClientTests: XCTestCase {
102119
httpBin.shutdown()
103120
}
104121

105-
let request = try Request(url: "https://localhost:\(httpBin.port)/post", method: .POST, body: .string("1234"))
106-
122+
let request = Request(url: "https://localhost:\(httpBin.port)/post", method: .POST, body: .string("1234"))!
107123
let response = try httpClient.execute(request: request).wait()
108124
let bytes = response.body.flatMap { $0.getData(at: 0, length: $0.readableBytes) }
109125
let data = try JSONDecoder().decode(RequestInfo.self, from: bytes!)
@@ -181,7 +197,7 @@ class HTTPClientTests: XCTestCase {
181197

182198
var headers = HTTPHeaders()
183199
headers.add(name: "Content-Length", value: "12")
184-
let request = try Request(url: "http://localhost:\(httpBin.port)/post", method: .POST, headers: headers, body: .byteBuffer(body))
200+
let request = Request(url: "http://localhost:\(httpBin.port)/post", method: .POST, headers: headers, body: .byteBuffer(body))!
185201
let response = try httpClient.execute(request: request).wait()
186202
// if the library adds another content length header we'll get a bad request error.
187203
XCTAssertEqual(.ok, response.status)
@@ -195,7 +211,7 @@ class HTTPClientTests: XCTestCase {
195211
httpBin.shutdown()
196212
}
197213

198-
var request = try Request(url: "http://localhost:\(httpBin.port)/events/10/1")
214+
var request = Request(url: "http://localhost:\(httpBin.port)/events/10/1")!
199215
request.headers.add(name: "Accept", value: "text/event-stream")
200216

201217
let delegate = CountingDelegate()
@@ -262,7 +278,7 @@ class HTTPClientTests: XCTestCase {
262278
}
263279

264280
let queue = DispatchQueue(label: "nio-test")
265-
let request = try Request(url: "http://localhost:\(httpBin.port)/wait")
281+
let request = Request(url: "http://localhost:\(httpBin.port)/wait")!
266282
let task = httpClient.execute(request: request, delegate: TestHTTPDelegate())
267283

268284
queue.asyncAfter(deadline: .now() + .milliseconds(100)) {

0 commit comments

Comments
 (0)