From 94e80960882d5654f267ec7bb944e4ad6f216d57 Mon Sep 17 00:00:00 2001 From: Michael Rebello Date: Mon, 19 Aug 2019 16:59:59 -0700 Subject: [PATCH] ios: add request unary interfaces (#347) Adds unary interfaces (related to https://github.com/lyft/envoy-mobile/issues/118) which will call into the streaming interfaces under the hood. These are to be used as convenience convenience accessors for the common case of non-streaming requests, and will be added on Android as part of https://github.com/lyft/envoy-mobile/issues/119. This also removes `data` and `trailers` from `Request`, as the unary function will take care of this for unary requests, and streams will be expected to handle this through the `StreamEmitter`. The underlying implementation will look something like this: ```swift public func sendUnary(_ request: Request, body: Data?, trailers: [String: [String]] = [:], handler: ResponseHandler) { let emitter = self.startStream(with: request, handler: handler) if let body = body { emitter.sendData(body) } emitter.close(trailers: trailers) } ``` Signed-off-by: Michael Rebello Signed-off-by: JP Simard --- mobile/library/swift/src/Client.swift | 21 +++++- mobile/library/swift/src/Request.swift | 11 --- mobile/library/swift/src/RequestBuilder.swift | 39 ---------- mobile/library/swift/src/StreamEmitter.swift | 2 + .../swift/test/RequestBuilderTests.swift | 73 ------------------- 5 files changed, 21 insertions(+), 125 deletions(-) diff --git a/mobile/library/swift/src/Client.swift b/mobile/library/swift/src/Client.swift index 1f5d41a238e8..3ef269099704 100644 --- a/mobile/library/swift/src/Client.swift +++ b/mobile/library/swift/src/Client.swift @@ -9,6 +9,23 @@ public protocol Client { /// - parameter handler: Handler for receiving stream events. /// /// - returns: Emitter for sending streaming data outward. - func startStream(request: Request, handler: ResponseHandler) - -> StreamEmitter + func startStream(with request: Request, handler: ResponseHandler) -> StreamEmitter + + /// Convenience function for sending a unary request. + /// + /// - parameter request: The request to send. + /// - parameter body: Serialized data to send as the body of the request. + /// - parameter trailers: Trailers to send with the request. + func sendUnary(_ request: Request, body: Data?, + trailers: [String: [String]], handler: ResponseHandler) +} + +extension Client { + /// Convenience function for sending a unary request without trailers. + /// + /// - parameter request: The request to send. + /// - parameter body: Serialized data to send as the body of the request. + public func sendUnary(_ request: Request, body: Data?, handler: ResponseHandler) { + self.sendUnary(request, body: body, trailers: [:], handler: handler) + } } diff --git a/mobile/library/swift/src/Request.swift b/mobile/library/swift/src/Request.swift index 1886177485c3..49c124c30b49 100644 --- a/mobile/library/swift/src/Request.swift +++ b/mobile/library/swift/src/Request.swift @@ -14,11 +14,6 @@ public final class Request: NSObject { /// Headers to send with the request. /// Multiple values for a given name are valid, and will be sent as comma-separated values. public let headers: [String: [String]] - /// Trailers to send with the request. - /// Multiple values for a given name are valid, and will be sent as comma-separated values. - public let trailers: [String: [String]] - // Serialized data to send as the body of the request. - public let body: Data? // Retry policy to use for this request. public let retryPolicy: RetryPolicy? @@ -35,8 +30,6 @@ public final class Request: NSObject { authority: String, path: String, headers: [String: [String]] = [:], - trailers: [String: [String]] = [:], - body: Data?, retryPolicy: RetryPolicy?) { self.method = method @@ -44,8 +37,6 @@ public final class Request: NSObject { self.authority = authority self.path = path self.headers = headers - self.trailers = trailers - self.body = body self.retryPolicy = retryPolicy } } @@ -63,8 +54,6 @@ extension Request { && self.authority == other.authority && self.path == other.path && self.headers == other.headers - && self.trailers == other.trailers - && self.body == other.body && self.retryPolicy == other.retryPolicy } } diff --git a/mobile/library/swift/src/RequestBuilder.swift b/mobile/library/swift/src/RequestBuilder.swift index b5715b299b88..8efb049c3db3 100644 --- a/mobile/library/swift/src/RequestBuilder.swift +++ b/mobile/library/swift/src/RequestBuilder.swift @@ -14,11 +14,6 @@ public final class RequestBuilder: NSObject { /// Headers to send with the request. /// Multiple values for a given name are valid, and will be sent as comma-separated values. public private(set) var headers: [String: [String]] = [:] - /// Trailers to send with the request. - /// Multiple values for a given name are valid, and will be sent as comma-separated values. - public private(set) var trailers: [String: [String]] = [:] - // Serialized data to send as the body of the request. - public private(set) var body: Data? // Retry policy to use for this request. public private(set) var retryPolicy: RetryPolicy? @@ -31,8 +26,6 @@ public final class RequestBuilder: NSObject { self.authority = request.authority self.path = request.path self.headers = request.headers - self.trailers = request.trailers - self.body = request.body self.retryPolicy = request.retryPolicy } @@ -68,34 +61,6 @@ public final class RequestBuilder: NSObject { return self } - @discardableResult - public func addTrailer(name: String, value: String) -> RequestBuilder { - self.trailers[name, default: []].append(value) - return self - } - - @discardableResult - public func removeTrailers(name: String) -> RequestBuilder { - self.trailers.removeValue(forKey: name) - return self - } - - @discardableResult - public func removeTrailer(name: String, value: String) -> RequestBuilder { - self.trailers[name]?.removeAll(where: { $0 == value }) - if self.trailers[name]?.isEmpty == true { - self.trailers.removeValue(forKey: name) - } - - return self - } - - @discardableResult - public func addBody(_ body: Data?) -> RequestBuilder { - self.body = body - return self - } - @discardableResult public func addRetryPolicy(_ retryPolicy: RetryPolicy) -> RequestBuilder { self.retryPolicy = retryPolicy @@ -108,8 +73,6 @@ public final class RequestBuilder: NSObject { authority: self.authority, path: self.path, headers: self.headers, - trailers: self.trailers, - body: self.body, retryPolicy: self.retryPolicy) } } @@ -122,9 +85,7 @@ extension Request { /// For example: /// /// Request *req = [Request withMethod:RequestMethodGet (...) build:^(RequestBuilder *builder) { - /// [builder addBody:bodyData]; /// [builder addHeaderWithName:@"x-some-header" value:@"foo"]; - /// [builder addTrailerWithName:@"x-some-trailer" value:@"foo"]; /// }]; @objc public static func with(method: RequestMethod, diff --git a/mobile/library/swift/src/StreamEmitter.swift b/mobile/library/swift/src/StreamEmitter.swift index 833a0f9fe050..8aea4d3c62e6 100644 --- a/mobile/library/swift/src/StreamEmitter.swift +++ b/mobile/library/swift/src/StreamEmitter.swift @@ -16,6 +16,7 @@ public protocol StreamEmitter { /// - throws: `Envoy.Error` when the stream is inactive or data can't be sent. /// /// - returns: The stream emitter, for chaining syntax. + @discardableResult func sendData(_ data: Data) throws -> StreamEmitter /// Send metadata over the associated stream. @@ -25,6 +26,7 @@ public protocol StreamEmitter { /// - throws: `Envoy.Error` when the stream is inactive or data can't be sent. /// /// - returns: The stream emitter, for chaining syntax. + @discardableResult func sendMetadata(_ metadata: [String: [String]]) throws -> StreamEmitter /// End the stream after sending any provided trailers. diff --git a/mobile/library/swift/test/RequestBuilderTests.swift b/mobile/library/swift/test/RequestBuilderTests.swift index ec22ff064a2b..c1dcdb97137f 100644 --- a/mobile/library/swift/test/RequestBuilderTests.swift +++ b/mobile/library/swift/test/RequestBuilderTests.swift @@ -2,7 +2,6 @@ import Envoy import Foundation import XCTest -private let kBodyData = Data([1, 2, 3, 4]) private let kRetryPolicy = RetryPolicy(maxRetryCount: 123, retryOn: [.connectFailure, .status5xx], perRetryTimeoutMS: 9000) @@ -28,23 +27,6 @@ final class RequestBuilderTests: XCTestCase { XCTAssertEqual("/foo", request.path) } - // MARK: - Body data - - func testAddingRequestDataHasBodyPresentInRequest() { - let request = RequestBuilder(method: .post, scheme: "https", - authority: "api.foo.com", path: "/foo") - .addBody(kBodyData) - .build() - XCTAssertEqual(kBodyData, request.body) - } - - func testNotAddingRequestDataHasNilBodyInRequest() { - let request = RequestBuilder(method: .post, scheme: "https", - authority: "api.foo.com", path: "/foo") - .build() - XCTAssertNil(request.body) - } - // MARK: - Retry policy func testAddingRetryPolicyHasRetryPolicyInRequest() { @@ -114,58 +96,6 @@ final class RequestBuilderTests: XCTestCase { XCTAssertNil(request.headers["foo"]) } - // MARK: - Trailers - - func testAddingNewTrailerAppendsToListOfTrailerKeys() { - let request = RequestBuilder(method: .post, scheme: "https", - authority: "api.foo.com", path: "/foo") - .addTrailer(name: "foo", value: "bar") - .build() - XCTAssertEqual(["bar"], request.trailers["foo"]) - } - - func testRemovingSpecificTrailerKeyRemovesAllOfItsValuesFromRequest() { - let request = RequestBuilder(method: .post, scheme: "https", - authority: "api.foo.com", path: "/foo") - .addTrailer(name: "foo", value: "1") - .addTrailer(name: "foo", value: "2") - .removeTrailers(name: "foo") - .build() - XCTAssertNil(request.trailers["foo"]) - } - - func testRemovingSpecificTrailerKeyDoesNotRemoveOtherKeysFromRequest() { - let request = RequestBuilder(method: .post, scheme: "https", - authority: "api.foo.com", path: "/foo") - .addTrailer(name: "foo", value: "1") - .addTrailer(name: "bar", value: "2") - .removeTrailers(name: "foo") - .build() - XCTAssertEqual(["bar": ["2"]], request.trailers) - } - - func testRemovingSpecificTrailerValueRemovesItFromRequest() { - let request = RequestBuilder(method: .post, scheme: "https", - authority: "api.foo.com", path: "/foo") - .addTrailer(name: "foo", value: "1") - .addTrailer(name: "foo", value: "2") - .addTrailer(name: "foo", value: "3") - .removeTrailer(name: "foo", value: "2") - .build() - XCTAssertEqual(["1", "3"], request.trailers["foo"]) - } - - func testRemovingAllTrailerValuesRemovesKeyFromRequest() { - let request = RequestBuilder(method: .post, scheme: "https", - authority: "api.foo.com", path: "/foo") - .addTrailer(name: "foo", value: "1") - .addTrailer(name: "foo", value: "2") - .removeTrailer(name: "foo", value: "1") - .removeTrailer(name: "foo", value: "2") - .build() - XCTAssertNil(request.trailers["foo"]) - } - // MARK: - Request conversion func testRequestsAreEqualWhenPropertiesAreEqual() { @@ -190,12 +120,9 @@ final class RequestBuilderTests: XCTestCase { private func newRequestBuilder() -> RequestBuilder { return RequestBuilder(method: .post, scheme: "https", authority: "api.foo.com", path: "/foo") - .addBody(kBodyData) .addRetryPolicy(kRetryPolicy) .addHeader(name: "foo", value: "1") .addHeader(name: "foo", value: "2") .addHeader(name: "bar", value: "3") - .addTrailer(name: "baz", value: "4") - .addTrailer(name: "baz", value: "5") } }