feat: add interceptors interfaces #685
Conversation
milesziemer
force-pushed
the
interceptors-interfaces
branch
2 times, most recently
from
b92019e
to
ef8709a
Compare
Tests/ClientRuntimeTests/InterceptorTests/InterceptorTests.swift
Outdated
Show resolved
Hide resolved
Tests/ClientRuntimeTests/InterceptorTests/InterceptorTests.swift
Outdated
Show resolved
Hide resolved
milesziemer
force-pushed
the
interceptors-interfaces
branch
from
27b8941
to
ee234ab
Compare
syall
force-pushed
the
interceptors-interfaces
branch
from
ee234ab
to
7be5f31
Compare
| associatedtype AttributesType: HasAttributes | ||
|
|
||
| /// - Returns: The input for the operation being invoked. | ||
| func getInput() -> Any |
There was a problem hiding this comment.
Isn't the type of the input RequestType? Not sure why this return value is untyped.
There was a problem hiding this comment.
input and output are the types of the modeled operation input and output, respectively. RequestType and ResponseType are the transport message types, like SdkHttpRequest, HttpResponse
There was a problem hiding this comment.
Should the modeled input type be an associated type of this protocol like the RequestType & ResponseType then? That way you can return the actual modeled input type, and you don't need the func getInput<T>() -> T? method below.
| } | ||
|
|
||
| public func modifyBeforeTransmit(context: inout some MutableRequest<Self.RequestType, Self.AttributesType>) async throws { | ||
| let input: TestInput = context.getInput()! |
There was a problem hiding this comment.
Force unwrapping during a test will crash the entire test suite if the unwrapped value is nil.
Instead, use XCTUnwrap which will throw a Swift error and just fail the test being run:
let input: TestInput = try XCTUnwrap(context.getInput())| try await self.readBeforeExecution(context) | ||
| } | ||
|
|
||
| internal func modifyBeforeSerialization(context: inout InterceptorContextType) async throws { |
There was a problem hiding this comment.
What is the purpose for making the context param inout in this & the other interceptor hooks?
InterceptorContextType is a typealias for DefaultInterceptorContext. DefaultInterceptorContext is a class, which is a Swift reference type.
The inout modifier means that the reference to context can be modified by the called method to point to a different context, which isn't what I think you want. (inout would be necessary if the passed context was a Swift value type such as struct and you wanted to change the struct's properties, but that is not the case here.)
There was a problem hiding this comment.
No purpose. Working on the orchestrator PR I realized it and just made InterceptorContext AnyObject, so there's no more inouts anywhere. Need to update this PR
milesziemer
force-pushed
the
interceptors-interfaces
branch
from
7be5f31
to
73fd7a7
Compare
| associatedtype AttributesType: HasAttributes | ||
|
|
||
| /// - Returns: The input for the operation being invoked. | ||
| func getInput() -> Any |
There was a problem hiding this comment.
Should the modeled input type be an associated type of this protocol like the RequestType & ResponseType then? That way you can return the actual modeled input type, and you don't need the func getInput<T>() -> T? method below.
| func getAttributes() -> AttributesType | ||
| } | ||
|
|
||
| public extension InterceptorContext { |
There was a problem hiding this comment.
See my comment above, we should make the modeled input type an associated type of InterceptorContext so this conditional cast is unneeded
Add interceptors interfaces to client libraries. Interceptors defines hooks that can be used to inject functionality into different phases of the operation execution pipeline.
Also use XCTUnwrap in tests
milesziemer
force-pushed
the
interceptors-interfaces
branch
from
10c3660
to
d534cc6
Compare
Add interceptors interfaces to client libraries.
Interceptors defines hooks that can be used to inject functionality into different phases of the operation execution pipeline.
Description of changes
Interceptorprotocol, which has methods defining the hooks into the operation execution pipeline. Protocol implementations can specify concrete types for the transport request/response types (i.e.SdkHttpRequest/HttpResponse), as well as a type that providesAttributesthrough the newHasAttributesprotocol. This allows 'less specific' interceptors that don't care about the transport request/response types, or the available attributes, to be used with more specific interceptors. See an example in InterceptorTestsHttpInterceptor, which inherits fromInterceptorand specializes the request, response, and attributes types to be http specific. Interceptor implementations can use this protocol so they don't have to defineRequestType,ResponseType, andAttributesTypeif they need all http specific stuff.InterceptorContextprotocol and various child protocols that define which properties of the interceptor context can be accessed at different points in the execution pipeline. Each interceptor hook accepts one of these child protocols as input. The context protocols have the same associated types asInterceptor, so implementations can define the types they will have in the context.HasAttributesprotocol, which is basically a protocol version of theAttributesstruct, and implements it forHttpContext. This lets us wrapHttpContextinto interceptor context objects, and use it in interceptor implementations, much like how it is used in existing middleware.Scope
By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.