-
Notifications
You must be signed in to change notification settings - Fork 1.4k
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
Substitutable HTTP client #2455
Conversation
throws IOException { | ||
final List<HttpHeader> headers = | ||
Arrays.stream(apacheResponse.getHeaders()) | ||
.collect(groupingBy(NameValuePair::getName)) |
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.
I suspect this is case sensitive - perhaps lower case the name?
} | ||
|
||
protected ImmutableRequest( | ||
String absouteUrl, |
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.
typo, should be absoluteUrl
byte[] body, | ||
boolean multipart, | ||
boolean browserProxyRequest) { | ||
this.absouteUrl = absouteUrl; |
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.
Perhaps use Objects.requireNonNull
for all the fields that can't be null?
|
||
HttpUriRequest httpRequest = getHttpRequestFor(responseDefinition); |
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.
I think HttpClientFactory.getHttpRequestFor
is now unused
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.
Gonna leave this in case anybody depends on it I think
Minor comments, looks great otherwise |
…and made ProxyResponseRenderer use this instead of the Apache client directly. Still to do: make this configurable via Options/extension.
…bject and via an extension in the same manner has HttpServerFactory, allowing the client factory to be set via options or via an extension.
…this instead of its own hard-coded http client.
45c151a
to
80bb84b
Compare
…, fixed a typo, wrote a test to demonstrate correct header case handling
* Created an HttpClient interface with an Apache client implementation and made ProxyResponseRenderer use this instead of the Apache client directly. * Added an HttpClientFactory interface that's returned by the options object and via an extension in the same manner has HttpServerFactory, allowing the client factory to be set via options or via an extension. * Made HttpClient a service passed to extensions and made webhooks use this instead of its own hard-coded http client.
* Created an HttpClient interface with an Apache client implementation and made ProxyResponseRenderer use this instead of the Apache client directly. * Added an HttpClientFactory interface that's returned by the options object and via an extension in the same manner has HttpServerFactory, allowing the client factory to be set via options or via an extension. * Made HttpClient a service passed to extensions and made webhooks use this instead of its own hard-coded http client.
This change creates an
HttpClient
abstraction within WireMock with a default Apache backed implementation. This is used in the proxy renderer and webhooks extension in place of the original directly referenced Apache clients.The HTTP client factory and a default implementation are made available as services to extensions.
The factory for the client can be substituted via the
Options
object or via an extension, following the same pattern as for the HTTP server.Doing this enables a few useful things: