Quarkus: @ConfigProperty injection into application beans with package private/public fields are seen as null in injected proxy

[starksm64]

We have a mismatch between the encapsulation level we are promoting with injected fields and @ConfigProperty associated with those fields. I have updated the application-configuration quickstart to illustrate the problem. Apply the attached application-configuration.patch.txt and there is a new GreetingBean:

package org.acme.config;

import javax.annotation.PostConstruct;
import javax.enterprise.context.ApplicationScoped;
import javax.enterprise.context.RequestScoped;
import javax.inject.Inject;

import org.eclipse.microprofile.config.inject.ConfigProperty;

@ApplicationScoped
public class GreetingBean {
    // Duplicate injection of greeting.message in another bean
    @ConfigProperty(name = "greeting.message")
    String message;

    // Injection of same property as different types
    @ConfigProperty(name = "greeting.math-pi")
    Double decimalPi;
    @Inject
    @ConfigProperty(name = "greeting.math-pi")
    String stringPi;

    @Inject
    @ConfigProperty(name = "greeting.unique")
    String unique;

    @PostConstruct
    private void init() {
        System.out.println("GreetingBean.init: "+ toString());
    }

    public String getMessage() {
        return message;
    }

    public Double getDecimalPi() {
        return decimalPi;
    }

    public String getStringPi() {
        return stringPi;
    }

    public String getUnique() {
        return unique;
    }

    @Override
    public String toString() {
        return "GreetingBean{" +
                "message='" + message + '\'' +
                ", decimalPi=" + decimalPi +
                ", stringPi='" + stringPi + '\'' +
                ", unique='" + unique + '\'' +
                '}';
    }
}

and this GreetingResource#checkGreetingBean that illustrates the problem:

    @GET()
    @Path("check")
    @Produces(MediaType.TEXT_PLAIN)
    public String checkGreetingBean() {
        StringBuilder result = new StringBuilder();
        result.append("+++ GreetingBean Fields");
        result.append("\n...message: "+greetingBean.message);
        result.append("\n...decimalPi: "+greetingBean.decimalPi);
        result.append("\n...stringPi: "+greetingBean.stringPi);
        result.append("\n...unique: "+greetingBean.unique);
        result.append("\n\n+++ GreetingBean Accessors");
        result.append("\n...message: "+greetingBean.getMessage());
        result.append("\n...decimalPi: "+greetingBean.getDecimalPi());
        result.append("\n...stringPi: "+greetingBean.getStringPi());
        result.append("\n...unique: "+greetingBean.getUnique());
        result.append('\n');

        return result.toString();
    }

Calling the /greeting/check endpoint shows the discrepancy between the proxy field values and the getter values:

Scotts-iMacPro:tmp starksm$ curl http://localhost:8080/greeting/check
+++ GreetingBean Fields
...message: null
...decimalPi: null
...stringPi: null
...unique: null

+++ GreetingBean Accessors
...message: hello
...decimalPi: 3.141592653589793
...stringPi: 3.14159265358979323846
...unique: unique

If the fields are made private we generate this type of warning:

17:22:01,456 INFO  [io.qua.arc.pro.BeanProcessor] Found unrecommended usage of private members (use package-private instead) in application beans:
    - @Inject field org.acme.config.GreetingBean#decimalPi,
    - @Inject field org.acme.config.GreetingBean#message,
    - @Inject field org.acme.config.GreetingBean#stringPi,
    - @Inject field org.acme.config.GreetingBean#unique,
    - @PostConstruct callback org.acme.config.GreetingBean#init()

We can't be encouraging non-private fields and then not making them usable.

application-configuration.patch.txt

[mkouba]

This is a well known limitation of CDI client proxies - the only way to access the state of the underlying bean instance (contextual instance per the spec) is to _invoke a method_ of a client proxy (contextual reference for normal scoped beans). Only non-static _public_ fields are disallowed. You would get the same results with Weld.

I agree that it may not be understandable for new users but it's a trade-off between usability and required functionality.

[starksm64]

Yes, but we are not bound to CDI rules and since we are advocating public access due to native image constraints, why not generate a synthetic accessor method similar to what lombok is doing?

https://projectlombok.org/

[mkouba]

Hm, the problem is that we would have to redefine any existing class that reads the field, basically find and replace all getfield JVM instructions in user code which could be quite time consuming. But I'm definitely not an expert in this area.

[geoand]

I _think_ we already change the call sites as part of Panache. It would be interesting to know what kind of build time performance implications this has.
In any case this is a very interesting trade-off on which I personally don't have an educated opinion.

[FroMage]

I think we already change the call sites as part of Panache.

We do indeed.

[gsmet]

@mkouba do we plan to do something about that or should we close?

[mkouba]

-1 from me.

[rsvoboda]

@mkouba close then ?

[mkouba]

I'd rather keep this open but I'll change the labels.

[Akejelu-Nelu]

This issue still persists. Any idea what the fix is?

[mkouba]

I think that this is a low priority issue. The fix would involve a lot of bytecode transformation which is rather expensive in terms of performance. We would have to generate the missing getters (and instruct ArC to generate delegating methods on the client proxies because they're not visible in the jandex index) and transform all the classes that read the relevant fields and since we don't know which classes are these we would have to analyse all methods from all classes in the application.

-1 from me. However, I'd like to keep this issue open to track the existing problem.

[Akejelu-Nelu]

Okay. Great 👍