Quarkus config mapping migration

michael-simons commented 3 months ago

Hello @radcortez

I hope you have time to have a look again. I cannot find for the life of it the different in the 3rd extension now to the 2nd I migrated.

If you run the tests on this branch

./mvnw -DskipCoreTests -pl neo4j-migrations-quarkus-parent/runtime -pl neo4j-migrations-quarkus-parent/deployment -pl neo4j-migrations-quarkus-parent/integration-tests -am -Dcheckstyle.skip clean verify

like this, BlahTest fails with

[ERROR] ac.simons.neo4j.migrations.quarkus.test.BlahTest -- Time elapsed: 2.429 s <<< ERROR!
java.lang.ExceptionInInitializerError
    at io.quarkus.runner.ApplicationImpl.<clinit>(Unknown Source)
    at java.base/java.lang.Class.forName0(Native Method)
    at java.base/java.lang.Class.forName(Class.java:534)
    at java.base/java.lang.Class.forName(Class.java:513)
    at io.quarkus.runner.bootstrap.StartupActionImpl.run(StartupActionImpl.java:289)
    at io.quarkus.test.QuarkusUnitTest.beforeAll(QuarkusUnitTest.java:661)
    at java.base/java.util.ArrayList.forEach(ArrayList.java:1596)
    Suppressed: java.lang.NoClassDefFoundError: Could not initialize class io.quarkus.runtime.generated.Config
        at java.base/java.lang.Class.forName0(Native Method)
        at java.base/java.lang.Class.forName(Class.java:534)
        at java.base/java.lang.Class.forName(Class.java:513)
        at io.quarkus.runner.bootstrap.StartupActionImpl.run(StartupActionImpl.java:293)
        ... 2 more
    Caused by: java.lang.ExceptionInInitializerError: Exception io.smallrye.config.ConfigValidationException: Configuration validation failed:
    SRCFG00050: org.neo4j.migrations.external-locations in PropertiesConfigSource[source=file:/var/folders/_y/ms08hd653_n_qgljb3svns100000gq/T/quarkus-unit-test16212579664370349347/app-root/application.properties]:3 does not map to any root [in thread "main"]
        at io.smallrye.config.SmallRyeConfig.buildMappings(SmallRyeConfig.java:139)
        at io.smallrye.config.SmallRyeConfig.<init>(SmallRyeConfig.java:93)
        at io.smallrye.config.SmallRyeConfigBuilder.build(SmallRyeConfigBuilder.java:736)
        at io.quarkus.runtime.generated.Config.<clinit>(Unknown Source)
        at io.quarkus.runner.ApplicationImpl.<clinit>(Unknown Source)
        at java.base/java.lang.Class.forName0(Native Method)
        at java.base/java.lang.Class.forName(Class.java:534)
        at java.base/java.lang.Class.forName(Class.java:513)
        at io.quarkus.runner.bootstrap.StartupActionImpl.run(StartupActionImpl.java:289)
        ... 2 more
Caused by: io.smallrye.config.ConfigValidationException: Configuration validation failed:
    SRCFG00050: org.neo4j.migrations.external-locations in PropertiesConfigSource[source=file:/var/folders/_y/ms08hd653_n_qgljb3svns100000gq/T/quarkus-unit-test16212579664370349347/app-root/application.properties]:3 does not map to any root
    at io.smallrye.config.SmallRyeConfig.buildMappings(SmallRyeConfig.java:139)
    at io.smallrye.config.SmallRyeConfig.<init>(SmallRyeConfig.java:93)
    at io.smallrye.config.SmallRyeConfigBuilder.build(SmallRyeConfigBuilder.java:736)
    at io.quarkus.runtime.generated.Config.<clinit>(Unknown Source)
    ... 7 more

despite having this one in place

public class CustomConfigBuilder implements SmallRyeConfigBuilderCustomizer {
    @Override
    public void configBuilder(final SmallRyeConfigBuilder builder) {
        builder.withMappingIgnore("org.neo4j.migrations.**");
    }
}

and this time also with the correct names and getting invoked.

Any ideas? I am down already to debugging the config map (see F.java)

radcortez commented 3 months ago

Sure, let me have a look.

michael-simons commented 3 months ago

It’s quite a big project, but I added the maven incarnation that should focus on the extension alone, without having troubles with the rest. I can join a chat if you want and if you think it’s easier.

Things I tried: Moving tests between surefire / failsafe. Using the ShrinkWrap v withApplicationRoot Disable JaCoCo instrumentation Trying a sub package for the tests Adding WithName to the properties Various forms of the pattern

No luck.

Am 27.08.2024 um 12:58 schrieb Roberto Cortez @.***>:

Sure, let me have a look.

— Reply to this email directly, view it on GitHub https://github.com/michael-simons/neo4j-migrations/pull/1426#issuecomment-2312206324, or unsubscribe https://github.com/notifications/unsubscribe-auth/AAEAQL37FLG6PLG3GA5WU4TZTRLTVAVCNFSM6AAAAABNF2ZKHWVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMZDGMJSGIYDMMZSGQ. You are receiving this because you authored the thread.

radcortez commented 3 months ago

Thanks. Let me just have a look and I'll get back to you :)

radcortez commented 3 months ago

Please add @StaticInitSafe to the CustomConfigBuilder.

The reason why you need it, is because when you run Quarkus, you have two config phases (one for static init and another for runtime). For JVM mode, there is not much issue, but for native mode, static init runs during build.

By default, all config services (sources, converters, customizers) are only discovered and added to the runtime config. We do this, because these services may not be ready to work during static init, so we need the implementer to tell us if it is safe to add it. For instance, consider that someone is adding a database configuration source. Most likely, you don't want that source to be available during static init native image compilation. Since we cannot reliable determine such cases, we disable discovery for static init and the user has to let us know via @StaticInitSafe if the service can be added to that phase.

In this particular case, is really counter-intuitive, because the mapping ignore it is safe, but the custom builder has access to all config aspect, which we don't know if they are safe or not.

This as not been a big issue, because most extensions do use the quarkus namespace, which is always ignored by default. I can probably be smarter and implement something that checks extension mappings and add the path automatically so you don't need the customizer.

michael-simons commented 3 months ago

Excellent, @radcortez That works! I understand the explanation, but I am wondering why we didn't have to add this here:

https://github.com/neo4j/neo4j-ogm-quarkus/blob/fe8687626a265ffa0f6ed5cc9d7f5f7673683976/runtime/src/main/java/org/neo4j/ogm/quarkus/runtime/Neo4jOgmConfigCustomizer.java#L29

i.e. that one just worked after you fixed my typo in the pattern: https://github.com/neo4j/neo4j-ogm-quarkus/blob/main/deployment/src/test/java/org/neo4j/ogm/quarkus/test/ConfigurationTest.java

radcortez commented 3 months ago

Because the config classes are added to each config phase based on @ConfigRoot#phase, which can be BUILD_TIME, BUILD_AND_RUN_TIME_FIXED or RUN_TIME.

In the other project, the config class is marked as RUN_TIME, so it doesn't get added to the static init config, and since it is not there, there is no validation. It is added to the runtime config, where the customizer is also discovered by default, so things work.

In this project, the config class is marked as BUILD_AND_RUN_TIME_FIXED, so it gets added to both static init and runtime configs. The config phase name might be a bit misleading. Still, it is named like this because the configuration is used by the native image build and made available at runtime without further changes (this is also the case for JVM mode, but less noticeable because static init and runtime happen in the same process). Because of the config phase, the mapping becomes available at the static config, so the validation is also executed, needing that ignore piece we added with the customizer.

michael-simons commented 3 months ago

So it as actually sensible to add it to the OGM extension, too, as the other config class is build-time, too: https://github.com/neo4j/neo4j-ogm-quarkus/blob/fe8687626a265ffa0f6ed5cc9d7f5f7673683976/runtime/src/main/java/org/neo4j/ogm/quarkus/runtime/Neo4jOgmBuiltTimeProperties.java

Which didn't fell over until know as the config test doesn't test this.

Got it, thanks a lot, really valuable.

michael-simons commented 3 months ago

@all-contributors please add @radcortez for mentoring