-
Notifications
You must be signed in to change notification settings - Fork 75
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
Path normalization breaks decoding data classes inside maps and sets incorrect map keys for property sources #443
Comments
@sksamuel Would appreciate a review and merge of this asap, because current master is a bit broken. |
I think that with the new version 2.8.0, another problem was introduced in the same area. Map keys from CommandLine was fixed but, now, if it comes from environment variables, the key ends up being |
@osoykan The key is lowercase because all keys are normalized to lowercase now due to the addition of the path normalizer by default, but it should still set the config correctly. Does it? Also FYI, there is possibly related issue #410 which is fixed by PR #414. |
@rocketraman I've reproduced the case in the commit: https://github.com/osoykan/hoplite/commit/d858187a057f58e9066ef0ca9b63ff2fedbe07cc When the key is lowercase, then my access strategy needs to be changed, too, which is breaking change. Could be fixed with your MR, indeed, if you could give it a try with the test-case I've added that would be perfect! |
@osoykan Oh that's interesting -- the test is fine if I will look into it. |
The implementations of
AbstractUnnormalizedKeysDecoders
which was created in PR #413 "restore" the unnormalized node keys.Unfortunately, when a decoder that requires normalization is further down in the tree, such as a data class decoder, it sees the unnormalized values and can therefore fail to read the config correctly.
The solution is to be less heavy-handed about de-normalization for map decoders and the Hikara data source decoder, and have them internally decode nodes only when they need to.
Additionally, the sourceKey was being set incorrectly to null for props-based sources like the
CommandLinePropertySource
.The text was updated successfully, but these errors were encountered: