Ensure YAML configuration is taken into account for DynamicFeatures

[essobedo]

fixes #18237

Motivation

Value of properties defined into a YAML file cannot be used anymore since Quarkus 2.0 in case of JAX-RS DynamicFeatures

Modifications:

• Adds a test case showing the problem
• Proposes a potential fix to define the ConfigPropertyProvider of the yaml extension as StaticInitConfigSourceProviderBuildItem

[geoand]

This seems perfectly reasonable to me

[essobedo]

@geoand

This seems perfectly reasonable to me

Not for @radcortez as he prefers a solution that would work with all ConfigPropertyProviders. See the discussion in the original ticket for more details #18237 (comment)

@radcortez Do I close this PR?

[geoand]

Of course Roberto is the subject matter expert here, so it's up to him

[radcortez]

@geoand

This seems perfectly reasonable to me

Not for @radcortez as he prefers a solution that would work with all ConfigPropertyProviders. See the discussion in the original ticket for more details #18237 (comment)

@radcortez Do I close this PR?

If you can work around with my proposal, yes :) Did that work?

[essobedo]

@radcortez If your workaround is to initialize "manually" the Bean that I inject into my DynamicFeature as next:

My class file

@Singleton
public class MyBean {

    String property;

    @Inject
    void init(Config config) {
        this.property = config.getValue("property.value", String.class);
    }
}

My file config/application.yml

property:
   value: foo

The answer is no, it doesn't work as it still uses a Config instance created during the static init phase which does use the "discovered" config sources like the Yaml config sources.

My current workaround is to somehow fork the config-yaml extension with this fix, I could not found any better way for now 😞

[radcortez]

No, that was not what I was thinking.

You need to do the lookup in the filter method with ConfigProvider.getConfig and retrieve the value there. Yes, this adds some extra calls on each call, but it should be ok. An alternative is to just lazy load the field in an AtomicReference on the first filter call and then you avoid additional calls to Config.

[essobedo]

No, that was not what I was thinking.

You need to do the lookup in the filter method with ConfigProvider.getConfig and retrieve the value there. Yes, this adds some extra calls on each call, but it should be ok. An alternative is to just lazy load the field in an AtomicReference on the first filter call and then you avoid additional calls to Config.

Ah I see, your idea is to delay the initialization to make it happen during at worse the startup time but in my use case I cannot do that, I need my bean to be initialized when calling the configure method of my DynamicFeature which happens during the static init phase.

[radcortez]

Not exactly. You let the bean initialize without any value (or a default). Then inside the filter method, you do a lookup of the right configuration value (this will only be called when the filter is called and way beyond startup).

Something like:

@Priority(1001)
 public class GreetingFilter implements ContainerResponseFilter {
     private AtomicReference<String> message = new AtomicReference();
     private CountDownLatch init = new CountDownLatch(1);

     @Override
     public void filter(ContainerRequestContext containerRequestContext, ContainerResponseContext containerResponseContext)
             throws IOException {

        ensureInit();

         containerResponseContext.getHeaders().add("X-MESSAGE", message);
     }

     private void ensureInit( {
            if (message.compareAndSet(null, "some-default"))) {
                ConfigProvider.getConfig().getOptionalValue("my.property", String.class).ifPresent(message::set);
                init.countDown();
            } else {
                try {
                    init.await();
                } catch (final InterruptedException e) {
                    // log
                }
            }
        }
 }

I did by memory, so I hope I didn't forget anything, but I hope you get the idea :)

[essobedo]

@radcortez Yeah it is what I understood from your previous comment but unfortunately my real use case is not exactly what I put in the PR, I added a filter just to be able to easily test the value injected into the DynamicFeature.

My real use case is more:

@Provider
public class MyDynamicFeature implements DynamicFeature {

    @Inject
    MyBean bean;

    @Override
    public void configure(ResourceInfo resourceInfo, FeatureContext featureContext) {
        if ("foo".equals(bean.property)) {
            // Do something in case of foo
        } else if ("bar".equals(bean.property)) {
            // Do something in case of bar
        }
    }
}

I'm afraid that your workaround is not applicable in my use case since the bean needs to be initialized when calling the configure method and the configure method is called during the static init phase 😢

[radcortez]

Ok, let's work on this PR and try to solve this use case while we think of another solution for the overall problem.

[quarkus-bot]

This workflow status is outdated as a new workflow run has been triggered.

🚫 This workflow run has been cancelled.

Failing Jobs - Building 4c88cde

⚠️ Artifacts of the workflow run were not available thus the report misses some details.

Status	Name	Step	Test failures	Logs	Raw logs
⌛	Initial JDK 11 Build	Set up JDK 11	⚠️ Check →	Logs	Raw logs

[essobedo]

@radcortez remarks addressed, please check again

[radcortez]

Better. Thank you. Let's wait for CI now.

[essobedo]

@radcortez the CI seems to be happy, is something missing before being merged?

[radcortez]

Yes. This one should go first: #17483 and then this one rebased on.

[gsmet]

@radcortez I merged #17483 . It appears this one still applies cleanly. Should we merge or it requires additional adjustments?

[radcortez]

The used AdditionalStaticInitConfigSourceProviderBuildItem is now deprecated in favour of StaticInitConfigSourceProviderBuildItem. If @essobedo wants to update it it :)

[essobedo]

The used AdditionalStaticInitConfigSourceProviderBuildItem is now deprecated in favour of StaticInitConfigSourceProviderBuildItem. If @essobedo wants to update it it :)

No PBR, I will

[radcortez]

The used AdditionalStaticInitConfigSourceProviderBuildItem is now deprecated in favour of StaticInitConfigSourceProviderBuildItem. If @essobedo wants to update it it :)

No PBR, I will

Thanks!

[essobedo]

Should be done, let's see what the CI says

[gsmet]

CI looks happy :). @radcortez I let you check that everything is OK and merge? Thanks!