-
Notifications
You must be signed in to change notification settings - Fork 567
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
Bug: ConfigProperty annotation on Map<String, String> field doesn't filter the configuration values, but populates all configuration entries in Map #4414
Comments
MIcroProfile Config does not specify injection of |
As Helidon, we have these options:
Both 1. and 2. suffer from possible performance impact and also from the case that not all sources can provide full set of their properties (so the map may not be complete). |
@tomas-langer Thanks for the details. I see a similar discussion in this MicroProfile issue as well. Last, but not least, I noticed that the implementation behaviour has changed from filtering a map to the current behaviour in Helidon MP. as part of this PR. I am curious now, wasn't this a breaking / backward incompatible behaviour? |
As this is not tested, we did not catch the difference in behavior. This was quite a big change (we had to switch away from using Helidon SE Config to using a full implementation of MP Config due to differences in handling environment variables), I have missed it when implementing it. |
In the PR:
|
For a map configuration field like below, i expect the map to be populated with all configuration entries under configKey section, but the map is populated with all possible configuration entries - without any filtering.
Is this behaviour expected??? Is there a provision to filter out the configuration entries that just match to the name mentioned in the annotation?
If i understand correctly, filtration happens properly for List and Set field types (at-least as per the code for those branches). Why the behaviour is different for Map alone???
Environment Details
The text was updated successfully, but these errors were encountered: