ConsoleNotifyingWiremockNetworkTrafficListener accepts custom encoding #2139
Conversation
Allows specification of a custom `Charset` for on `ConsoleNotifyingWiremockNetworkTrafficListener` allowing flexibility for service mocks that include non UTF-8 characters in their responses. Additionally: - Adds test coverage for `ConsoleNotifyingWiremockNetworkTrafficListener`. - Changes behavior of `incoming` and `outgoing` so that if a ByteBuffer with a Charset other than the one specified by `ConsoleNotifyingWiremockNetworkTrafficListener` is provided, a succinct message is logged. This replaces the previous behavior that dumped the entire CharacterCodingException. This was extremely noisy and did not provide useful additional diagnostics. Fixes wiremock#1541
| private final Charset charset; | ||
| private final CharsetDecoder charsetDecoder; | ||
|
|
||
| ConsoleNotifyingWiremockNetworkTrafficListener(Notifier notifier, Charset charset) { |
There was a problem hiding this comment.
Maybe public too? Can be done later if needed
There was a problem hiding this comment.
This choice was made around the fact that this type is named ConsoleNotifying*. It seemed off to allow for instantiating it with a custom Notifier that could notify to places other than the console. So per that consideration and Effective Java Item 13, I'm intentionally minimizing accessibility.
I do think there's potentially an opportunity here to create a simple or skeletal type that would expose the ability to create a WiremockNetworkTrafficListener with an arbitrary Notifier/ Charset combo.
I'd probably have that be structured as:
public final class WiremockNetworkTrafficListeners {
private WiremockNetworkTrafficListeners() {}
public static create(Notifier notifier, CharSet charset) {
return new SimpleWiremockNetworkTrafficListener(notifier, charset);
}
public static createConsoleNotifying() {
return new SimpleWiremockNetworkTrafficListener(DEFAULT_CONSOLE_NOTIFIER, DEFAULT_CHARSET)
}
public static createConsoleNotifying(CharSet charset) {
return new SimpleWiremockNetworkTrafficListener(DEFAULT_CONSOLE_NOTIFIER, charset)
}
}
class SimpleWiremockNetworkTrafficListener implements WiremockNetworkTrafficListener {
// Basically all the implementation that currently exists in this pull request ConsoleNotifying...
}
In Wiremock 2 you'd probably keep a shim ConsoleNotifyingNetworkTrafficListener type for backwards compatibility that would simply call through to WiremockNetworkTrafficListeners, but in 3 I'd drop the ConsoleNotifyingNetworkTrafficListener type altogether and exclusively rely on instantiation through the WiremockNetworkTrafficListeners factory pattern per EJ1.
Let me know what you think. But in any case, as long as it's okay with you let's do it as part of a separate PR?
There was a problem hiding this comment.
I think it would be a good improvement for WireMock's API extensibility!
If you are open to submitting a separate PR for that, indeed it would be the best approach
…-listener-enhancements
…k-traffic-listener-enhancements
|
Fixed the Spotless issues @oleg-nenashev so hopefully with your approval and once CI runs we can get this in 🙌 . |
…k-traffic-listener-enhancements
|
Not sure why, but I unless I'm going crazy, I had to run Spotless a second time to completely address fix identified issues. So apologies for not clearing them all on last commit I think we should be good now. |
No problems at all. Commits are free, we can always squash-merge if needed 😄 |
| private final Charset charset; | ||
| private final CharsetDecoder charsetDecoder; | ||
|
|
||
| ConsoleNotifyingWiremockNetworkTrafficListener(Notifier notifier, Charset charset) { |
There was a problem hiding this comment.
I think it would be a good improvement for WireMock's API extensibility!
If you are open to submitting a separate PR for that, indeed it would be the best approach
Allows specification of a custom
Charsetfor onConsoleNotifyingWiremockNetworkTrafficListenerallowing flexibility for service mocks that include non UTF-8 characters in their responses.Additionally:
ConsoleNotifyingWiremockNetworkTrafficListener.incomingandoutgoingso that if a ByteBuffer with a Charset other than the one specified byConsoleNotifyingWiremockNetworkTrafficListeneris provided, a succinct message is logged. This replaces the previous behavior that dumped the entire CharacterCodingException. This was extremely noisy and did not provide useful additional diagnostics.Fixes #1541