archaius
archaius copied to clipboard
Netflix
2.x Question: Decoder
This issue is to discuss enhancements to the current 2.x Decoder class.
Currently there is a single decoder within each Config instance often with that decoder needing to be shared between multiple configurations. When you need to add multiple types that are candidates for decoding, the current decoder design starts to break down. In order to extend a Decoder's functionality it must be extended and the new types added with the fallback on the super class.
I believe that a better implementation would be to follow a paradigm similar to that of the Jackson Module. Having a Decoder decode exactly one type and a configuration selects the decoder appropriate for decoding that type explicitly.
Creating a Decoder that behaves as you stated is possible with the current design. A sort of composite Decoder. My initial thought when I created it was being able to plug in Spring's conversion service. I don't want to have to register each type individually when I have a system that does that already.
I can get behind not wanting to have to register each type individually. Is there a composite decoder implementation that I missed somewhere or is that a challenge left up to the developer?
The Spring conversion service is an interesting concept. Do you have any thoughts on what that would look like?
No composite decoder yet. In general it might look like this
class CompositeDecoder implements Decoder {
List<MyDecoders> decoders;
CompositeDecoder(List<MyDecoders> decoders) {
this.decoders = decoders;
}
<T> T decode(Class<T> type, String encoded) {
for (MyDecoder decoder : decoders) {
if (decoder.canDecode(type)) {
return decoder.decode(type, encoded)
}
}
return null; // or throw exception or use default
}
}
I plug spring into feign here: https://github.com/spring-cloud/spring-cloud-netflix/blob/master/spring-cloud-netflix-core/src/main/java/org/springframework/cloud/netflix/feign/support/SpringDecoder.java It's http specific, but there is a generic ConversionService that isn't.
It's probably best to make the composite nature of Decoder plugin an integral part of the core implementation and not a customization of a single Decoder object. So, we would roll the above CompositeDecoder into the core implementation.
Regarding the above implementation would we have each Decoder handle each primitive type (ie. Integer) or would we have one Decoder that can handles all primitive types? The implementation for one decoder for all primitive types might look ugly because it would have duplicate type checking code in canDecode() and decode() which may be error prone. I suppose we could use the Decoder contract only for custom decoders and fallback to a single DefaultDecoder. For example,
for (Decoder decoder : decoders) {
if (decoder.canDecode(type)) {
return decoder.decode(type, encoded);
}
}
return defaultDecoder.decode(type, encoded);
The defaultDecoder will return null or throw the unsupported type exception.
I think that, as far as separation of concerns goes and for consistency, it would be best to have a decoder for each primitive type. However, I think we would need to come up with a mechanism to solve @spencergibb original problem of not wanting to have to register each type individually.
Regarding separation of concerns, I noticed AbstractConfig is also doing some type conversion work when using the type specific gettes. Is the goal that these methods should take advantage of the provided Decoder? Or are they expected to handle conversion?
