Improve the way runtime configuration is injected into CDI producers

[geoand]

Description
Currently the way injection of Runtime Configuration into CDI producers works by utilizing a Quarkus Recorder (that "records" the configuration object) and sets a field of the producer class (see for example: AbstractDataSourceProducer and AgroalRecorder).

This only works if the bean being produced by the producer method is not produced eagerly. If the bean needs to be produced eagerly the runtime configuration is null and things break.
However this is not evident to extension authors.

Implementation ideas
One way around the problem that would be independent of the how the laziness of the produced bean, would be have each extension generate a producer (most likely extending an abstract) that basically creates the runtime config object in bytecode. This is obviously not trivial to do, but it would be lift all the haze around the interaction between configuration and CDI producers.
We could perhaps even provide some kind of utility - most likely utilizing code from the config infrastructure.

cc @Sanne @mkouba

[mkouba]

Could you add some pseudocode example? I'm not sure I'm following... a real example would be even better ;-).

CC @manovotn

[geoand]

He goes a very simple attempt:

Let's say that in the runtime module you have:

protected abstract class AbstractMyProducer {

    // do whatever

   protected abstract SomeRuntimeConfiguration getRuntimeConfiguration(); 
}

Then in the build-time module an implementation would be generated simply extends AbstractMyProducer and simply generated an implementation for getRuntimeConfiguration.
Obviously it would be great to make generating this method as easy as possible.

Does that make sense if you compare it say to the links I have added in the description?

[mkouba]

I'm sorry I missed the links ;-). Hm, I don't see a problem here - you could set the configuration on a bean in a RUNTIME_INIT build step (lazy/eager init should not matter here). In other words, the AgroalRuntimeConfig should contain the values...

[mkouba]

Well, actually I'm not sure whether BytecodeRecorderImpl supports serialization of a RuntimeConfig... probably not and that's the real problem, right? You would have to pass e.g. AgroalRuntimeConfig.namedDataSources directly.

[geoand]

The bytecode recorder can "serialize" and deserialize the runtime config (since it's just a pojo), which is why it's being used via the pattern you saw in the links.
The pattern however is problematic, so we just need to find a better one that doesn't make extension author's live hard :).

[geoand]

I'm sorry I missed the links ;-). Hm, I don't see a problem here - you could set the configuration on a bean in a RUNTIME_INIT build step (lazy/eager init should not matter here). In other words, the AgroalRuntimeConfig should contain the values...

Now I am the one that doesn't follow 😆

[manovotn]

The recorder uses Arc.container().instance(AbstractDataSourceProducer.class).get() which means it either creates the bean if it didn't exist yet, or it retrieves one from bean store (I am assuming we are talking singleton/app scoped bean here).

If it is the former, then you shouldn't have issues, you are the first one to touch the bean and you immediately provide config.
If it is the latter, someone else might have tried to use the bean while the config was null; but the only case where this can happen if from another recorder that gets invoked before yours did - is that the issue you mean @geoand ?

[geoand]

@manovotn I am assuming that things currently work because of the former scenario you describe.

However, if the producer where to eagerly produce a bean because some other bean needed to inject it, then wouldn't the fact that the runtime configuration is injected into the producer after it's created be sort of a race condition? I think that is what people have been experiencing.
So what I am proposing is to eliminate this race condition, by baking the runtime to configuration into the producer bean.

Does that make sense?

[mkouba]

then wouldn't the fact that the runtime configuration is injected into the producer after it's created be sort of a race condition?

So the bean that needs some kind of initialization should react appropriately and the client bean should not use it until it's fully initialized. If an extension needs to use bean A that needs some init there should be a build item that signals that bean A was initialized -> this would ensure the build step will be executed after A is fully initialized. WDYT?

by baking the runtime to configuration into the producer bean.

I don't like this very much. It's harder to debug/understand.

[geoand]

then wouldn't the fact that the runtime configuration is injected into the producer after it's created be sort of a race condition?

So the bean that needs some kind of initialization should react appropriately and the client bean should not use it until it's fully initialized. If an extension needs to use bean A that needs some init there should be a build item that signals that bean A was initialized -> this would ensure the build step will be executed after A is fully initialized. WDYT?

I think I understand what you mean, however I sense a mismatch here between the CDI world and the Quarkus world.
Ideally as an extension author all I want to do is be able to easily create a CDI producer bean that can utilize runtime configuration and then let Arc figure out the initialization order.

by baking the runtime to configuration into the producer bean.

I don't like this very much. It's harder to debug/understand.

I agree that it's both harder to understand and debug. But if ask @Sanne I assume that he would say that current state of things has made it difficult for him to debug as well :)

[mkouba]

by baking the runtime to configuration into the producer bean.

I don't like this very much. It's harder to debug/understand.

I agree that it's both harder to understand and debug. But if ask @Sanne I assume that he would say that current state of things has made it difficult for him to debug as well :)

So I think you could use the RuntimeBeanBuildItem to achieve similar results, i.e. return the initialized bean instance supplier from the recorder.

[geoand]

I'll need to RuntimeBeanBuildItem out. I was not aware of it.

[mkouba]

Well, it's not documented because we don't feel it's stable/mature enough.

[geoand]

Understood, thanks. I'll play around with it when are past 1.0 and see if it fits the bill for what we want here.

[mkouba]

FYI the RuntimeBeanBuildItem was deprecated and removed. We now have the SyntheticBeanBuildItem that can be used for both the STATIC_INIT and RUNTIME_INIT. See io.quarkus.arc.deployment.SyntheticBeanBuildItem.ExtendedBeanConfigurator.setRuntimeInit() for more info.

@geoand Pls could you review this issue and let us know if it fits your needs?

[geoand]

@mkouba will do, thanks for the heads up!

[geoand]

@mkouba I have to hand it to you! SyntheticBeanBuildItem is awesome!
So I will close this issue since it's covered by SyntheticBeanBuildItem perfectly.