avaje / avaje-config

Application configuration / properties loading for JVM applications
https://avaje.io/config
Apache License 2.0
47 stars 7 forks source link

Extracting property source. #139

Closed agentgt closed 2 months ago

agentgt commented 2 months ago

While working on Rainbow Gum's generic agnostic property mapping validation and error message I noticed there is no way to get a CoreEntry (or analog).

That is I need the CoreEntry to get the source so that I can build better error messages.

The only way now to get the source information (e.g. like line number) is to use getAs which runs your conversion code as a Function.

Unfortunately I ended up designing Rainbow Gum where that is not really desirable as it creates very confusing call stacks.

What it instead would be easier to some wrapper object like the concrete CoreEntry so that you can do something like:

Optional<PropertyEntry> getOptionalEntry(String key). (and the non optional equivalent if you like). You can bike shed the name PropertyEntry but hopefully you get the idea.

In lightbend aka typesafe config the above analog is called ConfigValue: https://lightbend.github.io/config/latest/api/com/typesafe/config/ConfigValue.html

rob-bygrave commented 2 months ago

Hi @agentgt, if you get a moment can you check that PR to check it satisfies the requirement?

Note that currently in the PR theOptional<Configuration.Entry> configuration.entry(key) method is on Configuration and I haven't added it to Config yet as I don't think it will be "frequent use" per se. So people needing it via Config can access it via Config.asConfiguration().entry(key) if they need it which I think is ok.

Cheers, Rob.

xabolcs commented 2 months ago

... can you check that PR to ...

For the record: that PR is the #140

agentgt commented 2 months ago

@rob-bygrave I believe so. What I will do is pull the PR and try it against rainbowgum.

As minor critique of the PR I would like to see a unit test access Configuration.Entry#source() to confirm the goods are there :)

rbygrave commented 2 months ago

see a unit test access Configuration.Entry#source() to confirm the goods are there :)

Ha ha, nice catch. There was such a test but I changed entry() to filter out entries that represent null and yeah looks like I managed to remove the only assert on that when I made that change.

agentgt commented 2 months ago

@rbygrave it appears to work albeit it does not have line number for file based properties like typesafe config.

I'm OK with that not being present given avaje-config is for simple properties and that would probably just add bloat.

If you don't mind ping me if you guys release this so I can merge the experimental rainbowgum branch into main.

Cheers!

agentgt commented 2 months ago

For fun here is an example of rainbowgum working with config PR:

application-test.properties:

logging.level.my=TRACE
logging.global.change=true
logging.change.my=true
logging.appenders=stuff
logging.appender.stuff.output=blah # <- this is bad

The error messaging is a work in progress but you see here it cannot convert the property to an output (like a logback appender).

io.jstach.rainbowgum.LogProperty$PropertyConvertException: Error for property. key: 'logging.appender.stuff.output' from AVAJE(resource:application-test.properties)[logging.appender.stuff.output], java.lang.RuntimeException Output type for URI: 'blah:///' not found.
Tried: 'logging.appender.stuff.output' from AVAJE(resource:application-test.properties)[logging.appender.stuff.output]
    at io.jstach.rainbowgum/io.jstach.rainbowgum.LogProperty$Result$Error.value(LogProperty.java:482)
    at io.jstach.rainbowgum/io.jstach.rainbowgum.DefaultAppenderRegistry.appender(LogAppenderRegistry.java:150)
    at io.jstach.rainbowgum/io.jstach.rainbowgum.DefaultAppenderRegistry.appender(LogAppenderRegistry.java:87)
    at io.jstach.rainbowgum/io.jstach.rainbowgum.DefaultAppenderRegistry.defaultAppenders(LogAppenderRegistry.java:62)
    at io.jstach.rainbowgum/io.jstach.rainbowgum.LogRouter$Router$Builder.build(LogRouter.java:371)
    at io.jstach.rainbowgum/io.jstach.rainbowgum.LogRouter$Router$Builder.build(LogRouter.java:311)
    at io.jstach.rainbowgum/io.jstach.rainbowgum.RainbowGum$Builder.build(RainbowGum.java:268)
    at io.jstach.rainbowgum/io.jstach.rainbowgum.RainbowGum$Builder.build(RainbowGum.java:249)
    at io.jstach.rainbowgum.avaje/io.jstach.rainbowgum.avaje.AvajePropertiesProviderTest.lambda$0(AvajePropertiesProviderTest.java:19)
    at io.jstach.rainbowgum/io.jstach.rainbowgum.RainbowGumHolder.get(RainbowGum.java:369)
    at io.jstach.rainbowgum/io.jstach.rainbowgum.RainbowGum.of(RainbowGum.java:80)
    at io.jstach.rainbowgum.avaje/io.jstach.rainbowgum.avaje.AvajePropertiesProviderTest.test(AvajePropertiesProviderTest.java:20)
    at java.base/java.lang.reflect.Method.invoke(Method.java:580)
    at java.base/java.util.ArrayList.forEach(ArrayList.java:1596)
    at java.base/java.util.ArrayList.forEach(ArrayList.java:1596)
Caused by: java.lang.RuntimeException: Output type for URI: 'blah:///' not found.
    at io.jstach.rainbowgum/io.jstach.rainbowgum.DefaultOutputRegistry.provide(LogOutputRegistry.java:132)
    at io.jstach.rainbowgum/io.jstach.rainbowgum.LogOutput.lambda$0(LogOutput.java:95)
    at io.jstach.rainbowgum/io.jstach.rainbowgum.DefaultAppenderRegistry.lambda$7(LogAppenderRegistry.java:189)
    at io.jstach.rainbowgum/io.jstach.rainbowgum.LogProperty$PropertyFunction.apply(LogProperty.java:55)
    at io.jstach.rainbowgum/io.jstach.rainbowgum.LogProperty$PropertyGetter.lambda$0(LogProperty.java:1041)
    at io.jstach.rainbowgum/io.jstach.rainbowgum.ResultFuncGetter.get(LogProperty.java:1233)
    at io.jstach.rainbowgum/io.jstach.rainbowgum.LogProperty$PropertyGetter.get(LogProperty.java:665)
    at io.jstach.rainbowgum/io.jstach.rainbowgum.DefaultProperty.get(LogProperty.java:1102)
    ... 14 more