Skip to content

Commit

Permalink
feat: Allow custom EventLoopGroup and Timeout (#78)
Browse files Browse the repository at this point in the history
- also set default EventLoopGroup to use one thread per available processor
  • Loading branch information
djones6 authored Nov 5, 2019
1 parent 6b12cda commit 34cf8de
Show file tree
Hide file tree
Showing 5 changed files with 142 additions and 13 deletions.
6 changes: 3 additions & 3 deletions .travis.yml
Original file line number Diff line number Diff line change
Expand Up @@ -15,17 +15,17 @@ matrix:
dist: xenial
sudo: required
services: docker
env: DOCKER_IMAGE=swift:5.0.3-xenial SWIFT_SNAPSHOT=5.0.3
env: DOCKER_IMAGE=swift:5.0.3-xenial SWIFT_SNAPSHOT=5.0.3 DOCKER_PACKAGES="zlib1g-dev"
- os: linux
dist: xenial
sudo: required
services: docker
env: DOCKER_IMAGE=swift:5.1
env: DOCKER_IMAGE=swift:5.1 DOCKER_PACKAGES="zlib1g-dev"
- os: linux
dist: xenial
sudo: required
services: docker
env: DOCKER_IMAGE=swift:5.1 SWIFT_SNAPSHOT=$SWIFT_DEVELOPMENT_SNAPSHOT
env: DOCKER_IMAGE=swift:5.1 SWIFT_SNAPSHOT=$SWIFT_DEVELOPMENT_SNAPSHOT DOCKER_PACKAGES="zlib1g-dev"
- os: osx
osx_image: xcode10.2
sudo: required
Expand Down
2 changes: 1 addition & 1 deletion Package.swift
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,7 @@ let package = Package(
dependencies: [
.package(url: "https://github.com/IBM-Swift/CircuitBreaker.git", from: "5.0.0"),
.package(url: "https://github.com/IBM-Swift/LoggerAPI.git", from: "1.8.0"),
.package(url: "https://github.com/swift-server/async-http-client.git", .exact("1.0.0-alpha.4")),
.package(url: "https://github.com/swift-server/async-http-client.git", from: "1.0.0"),
],
targets: [
.target(
Expand Down
26 changes: 17 additions & 9 deletions Sources/SwiftyRequest/RestRequest.swift
Original file line number Diff line number Diff line change
Expand Up @@ -22,8 +22,13 @@ import NIO
import NIOHTTP1
import NIOSSL

fileprivate let globalELG = MultiThreadedEventLoopGroup(numberOfThreads: 1)
/// The default EventLoopGroup used by all RestRequest instances: a `MultiThreadedEventLoopGroup`
/// with one thread per available processor.
fileprivate let globalELG = MultiThreadedEventLoopGroup(numberOfThreads: System.coreCount)

// An object to represent the parameters of a request that can be mutated prior to
// a request being issued. This acts as an adapter between SwiftyRequest's API and
// async-http-client's (immutable) `Request` type, and can vend new `Request`s.
fileprivate class MutableRequest {
/// Request HTTP method
var method: NIOHTTP1.HTTPMethod
Expand Down Expand Up @@ -365,9 +370,9 @@ public class RestRequest {
}
}

@available(*, deprecated, renamed: "init(method:url:insecure:clientCertificate:)")
public convenience init(method: HTTPMethod = .get, url: String, containsSelfSignedCert: Bool? = false, clientCertificate: ClientCertificate? = nil) {
self.init(method: method, url: url, insecure: containsSelfSignedCert ?? false, clientCertificate: clientCertificate)
@available(*, deprecated, renamed: "init(method:url:insecure:clientCertificate:timeout:eventLoopGroup:)")
public convenience init(method: HTTPMethod = .get, url: String, containsSelfSignedCert: Bool? = false, clientCertificate: ClientCertificate? = nil, timeout: HTTPClient.Configuration.Timeout? = nil, eventLoopGroup: EventLoopGroup? = nil) {
self.init(method: method, url: url, insecure: containsSelfSignedCert ?? false, clientCertificate: clientCertificate, timeout: timeout, eventLoopGroup: eventLoopGroup)
}

/// Initialize a `RestRequest` instance.
Expand All @@ -382,17 +387,19 @@ public class RestRequest {
/// - url: URL string to use for the network request.
/// - insecure: Pass `True` to accept invalid or self-signed certificates.
/// - clientCertificate: An optional `ClientCertificate` for client authentication.
public init(method: HTTPMethod = .get, url: String, insecure: Bool = false, clientCertificate: ClientCertificate? = nil) {
/// - timeout: An optional `HTTPClient.Configuration.Timeout` specifying how long to wait for connection or response from a remote service before timing out. Defaults to `nil`, which means no timeout.
/// - eventLoopGroup: An optional `EventLoopGroup` that should be used for requests, instead of the default `MultiThreadedEventLoopGroup` shared between all RestRequest instances.
public init(method: HTTPMethod = .get, url: String, insecure: Bool = false, clientCertificate: ClientCertificate? = nil, timeout: HTTPClient.Configuration.Timeout? = nil, eventLoopGroup: EventLoopGroup? = nil) {
self.mutableRequest = MutableRequest(method: method, url: url)
self.session = RestRequest.createHTTPClient(insecure: insecure, clientCertificate: clientCertificate)
self.session = RestRequest.createHTTPClient(insecure: insecure, clientCertificate: clientCertificate, timeout: timeout, eventLoopGroup: eventLoopGroup)

// Set initial headers
self.acceptType = "application/json"
self.contentType = "application/json"

}

private static func createHTTPClient(insecure: Bool, clientCertificate: ClientCertificate? = nil) -> HTTPClient {
private static func createHTTPClient(insecure: Bool, clientCertificate: ClientCertificate? = nil, timeout: HTTPClient.Configuration.Timeout?, eventLoopGroup: EventLoopGroup?) -> HTTPClient {
let chain: [NIOSSLCertificateSource]
let key: NIOSSLPrivateKeySource?
if let clientCertificate = clientCertificate {
Expand All @@ -405,8 +412,9 @@ public class RestRequest {
let tlsConfiguration = TLSConfiguration.forClient(
certificateVerification: (insecure ? .none : .fullVerification),
certificateChain: chain, privateKey: key)
let config = HTTPClient.Configuration(tlsConfiguration: tlsConfiguration)
return HTTPClient(eventLoopGroupProvider: .shared(globalELG), configuration: config)
let config = HTTPClient.Configuration(tlsConfiguration: tlsConfiguration,
timeout: timeout ?? HTTPClient.Configuration.Timeout())
return HTTPClient(eventLoopGroupProvider: .shared(eventLoopGroup ?? globalELG), configuration: config)
}

/// Convenience function to encode an `Encodable` type as the request body, using a
Expand Down
17 changes: 17 additions & 0 deletions TestServer/Sources/TestServer/main.swift
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@
* limitations under the License.
*/
import Kitura
import Socket
import HeliumLogger
import FileKit
import Foundation
Expand Down Expand Up @@ -112,6 +113,22 @@ router.get("/users/:id") { (id: Int, respondWith: (User?, RequestError?) -> Void
respondWith(userStore[id], nil)
}

// MARK: Timeout tests

router.get("/timeout") { request, response, next in
guard let param = request.queryParameters["delay"], let delay = Int(param) else {
return try response.status(.badRequest).end()
}
DispatchQueue.global().asyncAfter(deadline: .now() + .milliseconds(delay)) {
response.status(.OK)
next()
}
}

// A socket that listens but never accepts connections
let sleepyServerSocket = try Socket.create(family: .inet)
try sleepyServerSocket.listen(on: 8079, maxBacklogSize: 0, allowPortReuse: false)

// MARK: Start server

// Add an HTTP server and connect it to the router
Expand Down
104 changes: 104 additions & 0 deletions Tests/SwiftyRequestTests/SwiftyRequestTests.swift
Original file line number Diff line number Diff line change
@@ -1,6 +1,8 @@
import XCTest
import CircuitBreaker
import NIOSSL
import NIO
import AsyncHTTPClient
@testable import SwiftyRequest

#if swift(>=4.1)
Expand Down Expand Up @@ -71,6 +73,9 @@ class SwiftyRequestTests: XCTestCase {
("testBasicAuthenticationFails", testBasicAuthenticationFails),
("testTokenAuthentication", testTokenAuthentication),
("testHeaders", testHeaders),
("testEventLoopGroup", testEventLoopGroup),
("testRequestTimeout", testRequestTimeout),
// ("testConnectTimeout", testConnectTimeout),
]

// Enable logging output for tests
Expand Down Expand Up @@ -1092,4 +1097,103 @@ class SwiftyRequestTests: XCTestCase {
waitForExpectations(timeout: 10)
}

// MARK: Test configuration parameters

/// Tests that a custom EventLoopGroup can be specified and that RestRequest will use this
/// instead of the default group.
/// Because there is no way to compare an EventLoopGroup or inspect its properties to determine
/// whether it is the one we expected, we instead supply a custom group containing only one
/// thread, and then assert that multiple request handlers cannot run in parallel.
func testEventLoopGroup() {
let expectation = self.expectation(description: "All outstanding requests have completed")
let myELG = MultiThreadedEventLoopGroup(numberOfThreads: 1)
let request = RestRequest(method: .get, url: "http://localhost:8080/", eventLoopGroup: myELG)
let sema = DispatchSemaphore(value: 0)
let requests = DispatchSemaphore(value: 0)
request.responseVoid { _ in
requests.signal()
request.responseVoid { _ in
requests.signal()
expectation.fulfill()
}
// Intentionally block the EventLoop thread, so that we can tell that the second
// request has not been processed (detecting that our single-threaded group is
// in use).
XCTAssertEqual(sema.wait(timeout: .distantFuture), .success)
}

// Assert that only a single request has been executed by our single-threaded ELG
XCTAssertEqual(requests.wait(timeout: .now() + .seconds(1)), .success, "First request should have been issued")
XCTAssertEqual(requests.wait(timeout: .now() + .seconds(1)), .timedOut, "Second request should not have been issued")

// Unblock the EventLoop thread, allowing the queued request to complete
sema.signal()
waitForExpectations(timeout: 1)
}

// MARK: Timeout tests

/// Makes a request to a route that delays its response for longer than the configured timeout, causing a failure.
/// Then tests that a request with the same configuration succeeds if the route responds within the timeout.
func testRequestTimeout() {
let timeoutExpectation = self.expectation(description: "Request times out")
let successExpectation = self.expectation(description: "Request succeeds")

let request = RestRequest(method: .get, url: "http://localhost:8080/timeout", timeout: HTTPClient.Configuration.Timeout(connect: nil, read: .milliseconds(500)))

let delay1s = URLQueryItem(name: "delay", value: "1000")

request.responseVoid(queryItems: [delay1s]) { result in
switch result {
case .success(let response):
XCTFail("Request should have timed out, but status was \(response.status)")
case .failure(let error):
XCTAssertEqual(error, RestError.httpClientError(HTTPClientError.readTimeout))
}
timeoutExpectation.fulfill()
}

let delayNone = URLQueryItem(name: "delay", value: "100")

request.responseVoid(queryItems: [delayNone]) { result in
switch result {
case .success(let response):
XCTAssertEqual(response.status, .ok)
case .failure(let error):
XCTFail("Request should have succeeded, but produced: \(error)")
}
successExpectation.fulfill()
}

waitForExpectations(timeout: 3)
}

/**
* Note: Disabling this test because it seems unreliable in a CI environment
/// Connects to a socket that listens but never accepts a connection, and verifies that the client
/// times out with a failure after a specified connect timeout.
func testConnectTimeout() {
let timeoutExpectation = self.expectation(description: "Request times out")
let timeout: TimeAmount = .milliseconds(500)

let request = RestRequest(method: .get, url: "http://localhost:8079/", timeout: HTTPClient.Configuration.Timeout(connect: timeout, read: nil))

request.responseVoid { result in
switch result {
case .success(let response):
XCTFail("Connection should have timed out, but status was \(response.status)")
case .failure(let error):
XCTAssertEqual(error, RestError.otherError(NIO.ChannelError.connectTimeout(timeout)))
if let underlyingError = error.error, case let NIO.ChannelError.connectTimeout(timeAmount) = underlyingError {
XCTAssertEqual(timeAmount, timeout, "Timeout amount was incorrect")
} else {
XCTFail("Underlying error was not NIO.ChannelError.connectTimeout, it was: \(error.error ?? error)")
}
}
timeoutExpectation.fulfill()
}

waitForExpectations(timeout: 3)
}
*/
}

0 comments on commit 34cf8de

Please sign in to comment.