Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Drop the requirement for no-args constructors in @ConfigProperties beans #760

Open
1 of 4 tasks
JHahnHRO opened this issue Nov 26, 2022 · 3 comments
Open
1 of 4 tasks
Labels
use case 💡 An issue which illustrates a desired use case for the specification
Milestone

Comments

@JHahnHRO
Copy link

Description

As a:

  • Application user/user of the configuration itself
  • API user (application developer)
  • SPI user (container or runtime developer)
  • Specification implementer

...I need to be able to:

Aggregate config properties in beans with non-trivial constructors in particular record classes.

More generally, I want to remove all restriction @ConfigProperties beans. They should just be (almost) normal class beans and support every CDI feature like interception & decoration, non-trivial bean constructors, PostConstruct & @PreDestroy methods...

Currently, the smallrye implementation imposes some (unspecified) restrictions, e.g. @ConfigProperties beans cannot have @PostConstruct methods or producer methods, because smallrye vetoes the original and adds a synthetic bean which loses everything but the information about field injection points.

...which enables me to:

a.) write more concise code with records, because it is no longer necessary to have injection point fields and write getter for all of them. Everything could in principle be done in the canonical constructor of a record if only I were able to write (taking the example from the spec)

@ConfigProperties(prefix="server")
@Dependent
public record Details(String host,
                      int port,
                      String endpoint,
                      @ConfigProperty(name="old.location") String location) {}

This can work for records, but not other classes, because the names of the parameters of the canonical constructor are always retained in the class file as the names of the record components. So the problem that constructor- and method-parameter names need not be present for reflection, does not appear with records.

Records also have the nice benefit that they provide a canonical place to do validation of the aggregated properties.

b.) do non-trivial validation in @PostConstruct, e.g.

@ConfigProperties(prefix="my.app")
@Dependent
class Range {
   int from;
   int to;

   @PostConstruct
   void validate(){
      if (from > to){
         throw new IllegalStateException("Non-sensical config values");
      }
   }
}

or again stealing from the example in the spec:

@ConfigProperties(prefix="server")
@Dependent
public class Details {
    public String host; // the value of the configuration property server.host
    public int port;   // the value of the configuration property server.port
    private String endpoint; //the value of the configuration property server.endpoint
    public @ConfigProperty(name="old.location")
    String location; //the value of the configuration property server.old.location
    
   @PostConstruct
   void validate(){
      if (port <= 0) {
         throw new IllegalStateExeption("Check the config. Some idiot configured negative ports");
      }
   }
}

Some obvious validations like this one can be done with the smallrye implementation by using Jakarta Bean Validation's @Min annotation, but for something like the Range example that needs to correlate multiple config properties, a single annotation is not sufficient, but writing a custom Validator seems like overkill. Also it's certainly a non-trivial leap for a developer to use Bean Validation.

c.) support computed default values, e.g.

@ConfigProperties(prefix="my.app")
@Dependent
class Server {
   @ConfigProperty(defaultValue="localhost")
   String host;
   @ConfigProperty(defaultValue="8080")
   int basePort;
   @ConfigProperty(defaultValue="0")
   int portShift;
   @ConfigProperty(defaultValue="-1")
   int port1;
   @ConfigProperty(defaultValue="-1")
   int port2;
   @ConfigProperty(defaultValue="-1")
   int port3;

   @PostConstruct
   void calculatedDefaults(){
      if (port1 < 0){
         port1 = basePort + portShift + 1;
      }
      if (port2 < 0){
         port2 = basePort + portShift + 3;
      }
      if (port3 < 0){
         port3 = basePort + portShift + 3;
      }
   }
}

At the moment this would need to be done in custom getter methods for example.

d.) use producer methods directly in @ConfigProperties to provide simplified/transformed views of the config, e.g.

@ConfigProperties(prefix="my.app")
@Dependent
class Dates {
   @ConfigProperty(defaultValue="1970-01-01")
   LocalDate date;
   LocalTime time;


   @Produces
   @Dependent
   CustomDateFormat custom(){
      return new CustomDateFormat(date.getYear(), date.getMonthValue(), date.getDayOfMonth());
   }
}

record CustomDateFormat(int year, int month, int day){}

At the moment, this does not work with smallrye, because of how @ConfigProperties is implemented. One needs a separate bean class for the producer methods.

Implementation details

The requirement of a no-args constructor seems like an implementation detail to me. It took some fiddling, but I have a proof of concept that shows that this detail is not necessary and one can in fact integrate with a CDI container without relying on this restriction.

@JHahnHRO JHahnHRO added the use case 💡 An issue which illustrates a desired use case for the specification label Nov 26, 2022
@Emily-Jiang
Copy link
Member

@JHahnHRO thanks for raising this issue with thorough description! Sorry for the slow response! From my understanding, it looks like this issue is about the implementation of SmallRye config as this spec does not enforce the bean with the annotation @ConfigProperites has a zero arg constructor. Please correct me if I got this wrong. @radcortez can you take a look to see whether you can incorporate the proposal from @JHahnHRO in SmallRye impl? thanks

@radcortez
Copy link
Contributor

At the moment, we don't have the bandwidth to accommodate such changes. PR's are welcomed, of course :)

And if I understood correctly, @JHahnHRO even has a POC for it, so it should be fairly easy to propose that to SmallRye Config and I'm happy to review it.

@JHahnHRO
Copy link
Author

JHahnHRO commented Jul 24, 2023

It is not just a SmallRye-Issue. The requirement for a no-args constructor is part of the Spec: section 3.5.3 https://download.eclipse.org/microprofile/microprofile-config-3.0/microprofile-config-spec-3.0.html#_configproperties_bean_class_validation

Last I checked (months ago) my PoC currently fails the TCK, because it would change the behaviour of @RequestScoped beans. That's where #764 becomes relevant to this.

@Emily-Jiang Emily-Jiang added this to the Config 4.0 milestone Jan 16, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
use case 💡 An issue which illustrates a desired use case for the specification
Projects
None yet
Development

No branches or pull requests

3 participants