-
Notifications
You must be signed in to change notification settings - Fork 422
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Interceptors content negotiation #1078
Merged
Merged
Changes from 26 commits
Commits
Show all changes
36 commits
Select commit
Hold shift + click to select a range
d4d00e6
validation of status code mappings
kubinio123 334ac9e
test for open api multiple media types to single status code
kubinio123 69a711a
test case mapping one status to multiple schemas in multiple formats
kubinio123 e45f6d4
select output based on content-type in client interpreters
kubinio123 92aee61
xml module added
kubinio123 f4e3755
test fixing
kubinio123 bdfd456
refactor
kubinio123 9cbdd63
filter.find -> collectFirst
kubinio123 b905b1d
improvement
kubinio123 4ceacad
initial ContentNegotiation impl
kubinio123 6d202f4
refactor
kubinio123 2392a87
removed validate from mapOut methods
kubinio123 3893a79
content negotiation introduced in logic + tests
kubinio123 5eba867
Merge branch 'feature/248' into interceptors-content-negotiation
kubinio123 f739155
merged changes from feature/248 (client side content negotiation adju…
kubinio123 ff07b9f
Merge branch 'interceptors' into interceptors-content-negotiation
kubinio123 e6aed63
described extract method
kubinio123 bbec3a3
charset extraction from StreamBody before picking best content
kubinio123 a7b545a
test case added, static imports
kubinio123 b7750ca
open api model adjustement
kubinio123 c73b232
decoding response adjustment, handle case with no content specified a…
kubinio123 615f4c1
default charset only for text type
kubinio123 4b9769e
Merge branch 'master' into interceptors-content-negotiation
kubinio123 da09172
new sttp-model adjustment
kubinio123 2f05eba
compilation fix
kubinio123 d8aaa3d
handled case when there's no body mappings
kubinio123 0d89ce3
traverse outputs, handle no content case with body and non body mappings
kubinio123 44befae
Merge branch 'master' into interceptors-content-negotiation
kubinio123 28396e5
compilation fix + merge
kubinio123 4933584
Merge branch 'master' into interceptors-content-negotiation
kubinio123 cf19df4
ContentTypeInterceptor added
kubinio123 a6afdc1
decoding client result unified
kubinio123 cc244b2
Merge branch 'master' into interceptors-content-negotiation
kubinio123 987ddde
removed commented code
kubinio123 cd725aa
Bring back ranges to ServerRequest
adamw 8c531b7
Simplify
adamw File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,15 +1,16 @@ | ||
package sttp.tapir.client.sttp | ||
|
||
import java.io.ByteArrayInputStream | ||
import java.nio.ByteBuffer | ||
import sttp.capabilities.Streams | ||
import sttp.client3._ | ||
import sttp.model.{HeaderNames, Method, Part, ResponseMetadata, StatusCode, Uri} | ||
import sttp.model._ | ||
import sttp.tapir.Codec.PlainCodec | ||
import sttp.tapir._ | ||
import sttp.tapir.internal._ | ||
import sttp.ws.WebSocket | ||
|
||
import java.io.ByteArrayInputStream | ||
import java.nio.ByteBuffer | ||
|
||
private[sttp] class EndpointToSttpClient[R](clientOptions: SttpClientOptions, wsToPipe: WebSocketToPipe[R]) { | ||
def toSttpRequest[O, E, I](e: Endpoint[I, E, O, R], baseUri: Option[Uri]): I => Request[DecodeResult[Either[E, O]], R] = { params => | ||
val (uri, req1) = | ||
|
@@ -62,15 +63,45 @@ private[sttp] class EndpointToSttpClient[R](clientOptions: SttpClientOptions, ws | |
case EndpointIO.FixedHeader(_, codec, _) => codec.decode(()) | ||
case EndpointIO.Empty(codec, _) => codec.decode(()) | ||
case EndpointOutput.OneOf(mappings, codec) => | ||
mappings | ||
.find(mapping => mapping.statusCode.isEmpty || mapping.statusCode.contains(meta.code)) match { | ||
case Some(mapping) => | ||
getOutputParams(mapping.output, body, meta).flatMap(p => codec.decode(p.asAny)) | ||
val mappingsForStatus = mappings collect { | ||
case m if m.statusCode.isEmpty || m.statusCode.contains(meta.code) => m | ||
} | ||
|
||
val contentType = meta | ||
.header(HeaderNames.ContentType) | ||
.map(MediaType.parse) | ||
|
||
def applyMapping(m: EndpointOutput.StatusMapping[_]) = getOutputParams(m.output, body, meta).flatMap(p => codec.decode(p.asAny)) | ||
|
||
contentType match { | ||
case None => | ||
DecodeResult.Error( | ||
meta.statusText, | ||
new IllegalArgumentException(s"Cannot find mapping for status code ${meta.code} in outputs $output") | ||
) | ||
mappingsForStatus.headOption | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. shouldn't we give priority to mappings without a body in such case? e.g. for a mapping
in case we get back an empty response, the second should be picked |
||
.map(applyMapping) | ||
.getOrElse( | ||
DecodeResult.Error( | ||
meta.statusText, | ||
new IllegalArgumentException(s"Cannot find mapping for status code ${meta.code} in outputs $output") | ||
) | ||
) | ||
|
||
case Some(Right(content)) => | ||
val bodyMappingForStatus = mappingsForStatus collectFirst { | ||
case m if m.output.hasBodyMatchingContent(content) => m | ||
} | ||
|
||
bodyMappingForStatus | ||
.orElse(mappingsForStatus.headOption) | ||
.map(applyMapping) | ||
.getOrElse( | ||
DecodeResult.Error( | ||
meta.statusText, | ||
new IllegalArgumentException( | ||
s"Cannot find mapping for status code ${meta.code} and content $content in outputs $output" | ||
) | ||
) | ||
) | ||
|
||
case Some(Left(_)) => DecodeResult.Error(meta.statusText, new IllegalArgumentException("Unable to parse Content-Type header")) | ||
} | ||
|
||
case EndpointIO.MappedPair(wrapped, codec) => getOutputParams(wrapped, body, meta).flatMap(p => codec.decode(p.asAny)) | ||
|
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
do we have default arguments in other places as well? would be keep to stay consistent in the model
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
no, adjusted