Allow @ConfigurationProperties binding on interfaces

[philwebb]

It would be useful to allow configuration binding to work directly on fields if the user doesn't want to expose public setters.

[snicoll]

That would completely compromise work on #1001 that rely on the presence of a standard javabean property.

I also find this useful but we need to chose our camp. If we support it, we need a way to exclude unwanted fields/properties through annotations.

We also could add a mode to the annotations that would provide a hint to the processor. In that mode only the field would be exposed (same for metadata harvesting)

[philwebb]

What about if we support field based binding if there is a getter but no setter. Or another easier option might be to allow protected setters. What I wanted was a quick way to expose configuration properties in a read-only form.

Yet another option might be to allow @ConfigurationProperties on an interface, generating an implementation proxy class.

@ConfigurationProperties
public interface SomeProperties {
    String getDateToolAttribute();
    String getNumberToolAttribute();
    getProperties();
    String getResourceLoaderPath();
    String getToolboxConfigLocation();
}

[wilkinsona]

RJo on StackOverflow is interested in this functionality:

If the setters were not visible, the configuration properties would be unmodifiable without resorting to violence :)

[snicoll]

I am fine either way but we have to understand the consequence for the annotation processor. We can go with any crazy option as long as it's explicit (i.e. you have to enable a flag on the @ConfigurationProperties annotation). If we don't, then we'll have to update the algorithm auto-detection and potentially expose thing we don't want in the metadata

(reminder: we have no way to exclude a property)

[fromi]

I would be happy to be able to use @ConfigurationProperties with constructor injection, so I can create immutable and encapsulated classes.

[mwisnicki]

I, for one, would like to have it working with plain fields.

[mwisnicki]

Maybe use transient or @Transient to exclude properties from serialization/metadata ?

[mwisnicki]

BTW spring-boot documentation has an example that shows binding to field which I guess is a mistake.

[philwebb]

@mwisnicki Good spot on the docs. I've raised #2813 to address this.

[michaeltecourt]

In order to have an "immutable" configuration properties object, I currently use a workaround with an interface, whose implementation is a "Java Bean" annotated with @ConfigurationProperties. Any shortcut would be appreciated. By the way, enabling field based injection would make a lot of sense, as the behavior would be more consistent with @Value. Having to add both getters/setters can be counter-intuitive the first time around.

[jordanjennings]

@michaeltecourt Thanks for the suggestion. Doing this in combination with Lombok's @DaTa annotation on the implementation class makes this fairly painless.

[snicoll]

@michaeltecourt that's not the same thing at all. With @Value you're giving the full id in a regular Spring Framework annotation (i.e. that you can put on field, setter or constructor argument). @ConfigurationProperties is a strongly-typed binder on a regular class where properties are discovered.

If we were enabling field access by default, all the fields of your class would be exposed which isn't what you want. The presence of accessors defines if it should be exposed or not.

I think our best shot at it is to add a flag to enable field support explicitly. In that case, you're at least aware they are all exposed.

[michaeltecourt]

@snicoll By "all the fields of your class would be exposed" do you mean "injectable/bindable" ?

We mostly use @ConfigurationProperties like a bulk @Value.
The semantic and precise use cases might be different, but isn't the purpose the same? binding properties from the Spring context to Spring beans in a type-safe manner?

In the end, what I find slightly confusing as a user is that you can do this :

/** Works on fields, constructor, etc */
@Component
class Props {
  @Value("${cfg.props.a}") String a;
  @Value("${cfg.props.b}") BigDecimal b;

  public Props(a, b) { this.a = a; this.b = b; }
  public String getA() { return this.a; }
  public BigDecimal getB() { return this.b; }
 }

and not this :

@ConfigurationProperties("cfg.props")
class Props {
  String a;
  BigDecimal b;

  public Props(a, b) { this.a = a; this.b = b; }
  public String getA() { return this.a; }
  public BigDecimal getB() { return this.b; }
  // SETTERS NEEDED
}

[snicoll]

Yes, that is what I mean.

I think we already covered why it is confusing (see my previous comment). I am not arguing we shouldn't do it, I am arguing it must be opt-in.

[michaeltecourt]

oh just realized ConfigurationProperties are also exposed through the /configprops actuator endpoint while "regular" properties are not.

From a humble user standpoint, I know that Spring has implicit write access to my class attributes as soon as it's annotated with @ConfigurationProperties, I wouldn't mind if field binding became the default, with some kind of exclusion mechanism as described by @mwisnicki, like the vast majority of annotation based binding frameworks (JAXB, Jackson, JPA...).
I know the use case is different, as config properties are often directly injected to beans whose purpose is far more complex than holding values, so any additional option over getters+setters would be cool :)

@ConfigurationProperties("cfg.props")
 class Props {

      /** requires accessors by default */
      String a; 

      /** Enable field access explicitely */
      @ConfigurationProperty BigDecimal b;

      /** Explicit exclusion ? */
      @Transient Integer c;
 }

@philwebb 's proxy based solution looks a lot like the "old" (not pejorative) property binder framework : http://pholser.github.io/property-binder/examples.html

[AndersDJohnson]

+1 I'd like a @ConfigurationProperty annotation valid on fields, which could accept an optional value parameter to specify the property path rather than reflecting on the field name, as well as other applicable parameters from @ConfigurationProperties.

[laxika]

+1 from me. Setters are entirely useless for my user case and still, I should provide them. Having an immutable configuration would be a lot better. I'm not exactly sure how it can by supported sadly but it would be a lot better.

[sta-szek]

👍 getters and setters on all my property classes... 😢

[MHarris021]

+1

[victorherraiz]

I'd love to have that feature. Annotated constructor parameters should be another option, something like @JsonCreator annotation, to build actual inmutable objects.

[tgianos]

+1 Also like the idea of mimicking how Jackson uses @JsonCreator

[krzyk]

This could be achieved by using java 8 -parameters so Spring would know which property should be set at which constructor argument.

[thinkbigthings]

+1 for immutable configuration.

[philwebb]

I think we should hold off on this one until we see how immutable constructor based properties work out.

[philwebb]

We've decided not to investigate this for now for the reason mentioned above.

[thekalinga]

immutable constructor based properties

Is there a problem with this?

[snicoll]

@thekalinga no, that's why we've decided to close this one. We recommend using immutable configuration properties and we feel interface binding is very similar and not worth investigating any further at this point.

[kiriakos]

@snicoll
please help me understand this, if I read the docs correctly the immutable config props proposal is doing something like this:

@ConstructorBinding
@ConfigurationProperties("acme")
public class AcmeProperties {
    private final boolean enabled;
    public AcmeProperties(boolean enabled) {
        this.enabled = enabled;
    }
    public isEnabled(){
        return enabled;
    }
}

(simplified from example)
how does this compare to simply declaring

@ConfigurationProperties("acme")
public interface AcmeProperties {
    public isEnabled()
}

The one is imperative and wordy the other is a succinct declaration. Are there any non apparent drawbacks that I'm missing here?

Thanks

EDIT: whitespace

[philwebb]

Are there any non apparent drawbacks that I'm missing here?

The drawbacks are not really technical but more to do with where we should invest our time. Our experience with immutable configuration properties has shown that there are quite a few things that we need to consider when changing the configuration properties model (and these are often quite time consuming). For example, one problem that's often overlooked is the generation of the meta-data file that's used to provide IDE support.

Closing this issue doesn't mean that interface based properties are something that we'll never consider. We may well decide to re-open this one again in the future. For now though, we think that it's best if we see how useful people find constructor binder and how many problems the new code causes.

[dragneelfps]

Can this be reopened it as we already have support for constructor based immutable properties? (at least in Kotlin afaik)

[laxika]

@dragneelfps

You can already do this add the following annotation to your application class:

@ConfigurationPropertiesScan(basePackages = "com.github.loa")

Then you can create a configuration properties class with final values like this:

package com.github.loa.vault.configuration;

import lombok.Getter;
import lombok.RequiredArgsConstructor;
import org.springframework.boot.context.properties.ConfigurationProperties;
import org.springframework.boot.context.properties.ConstructorBinding;

/**
 * Holds the configuration properties to the vault.
 */
@Getter
@ConstructorBinding
@RequiredArgsConstructor
@ConfigurationProperties("loa.vault")
public class VaultConfigurationProperties {

    /**
     * The unique name of the vault. This name will be saved when archiving new documents and will be used to identify
     * the vault that holds a given document. Do not change this after creating the vault because the documents
     * previously archived by this vault are not going to be accessible.
     */
    private final String name;

    /**
     * If this vault should be able to modify documents (e.g. remove) after they are archived. If your vault is available
     * publicly on the internet then set this to false!
     */
    private final boolean modificationEnabled;

    /**
     * If this vault should archive new documents.
     */
    private final boolean archiving;

    /**
     * This version number will be saved to the database for every document that has been archived. It will be used
     * later on if it's necessary to run cleanup or fixing tasks that are specific to a given version.
     */
    private final int versionNumber;
}

Why is this not enough for your usecase? You want to set the values via reflection, without constructors?

[snicoll]

@dragneelfps if you're willing us to reconsider, you'd have to provide a bit more context and justification. Immutable configuration properties are available for both Java and Kotlin and this comment still stands.

[krzyk]

@dragneelfps You can also use records for that (or just pure immutable class like shown above). I'm not sure why you would prefer interface.

[dragneelfps]

So, I have this scenario where there is supposed to be some base config and its different child which may override and add new properties to that config, as follows

http-base: &httpBase
  connect-timeout: 5s
  
connectors:
  clientA:
    <<: *httpBase
    clientId: foobar

One way to solve is using composition instead of inheritance, but lets talk about inheritance for a bit
One possible way would be:

open class Base { lateinit var connectTimeout: Duration }
class ClientA(val clientId: String): Base()

The issue with above would be that the property check will throw error at runtime instead of start time when connectTimeout is missing. Also, it makes connectTimeout mutable, which I do not want.

Second way would be:

open class Base(val connectTimeout: Duration)
class ClientA(val clientId: String, connectTimeout: Duration): Base(connectTimeout)

Now, I do not want to write connectTimeout redundantly in ClientX classes.
Same issue if I make Base an interface then it becomes

interface Base { val connectTImeout: Duration }
class ClientA(val clientId: String, override val connectTimeout: Duration): Base

Now, IMO, following would be best suited for this kind of usecase

interface Base { val connectTimeout: Duration }
interface Client { val clientId: String } : Base

It is clean and provides immutability and makes sense.

PS:
A solution with a composition approach would be something like

class Base(val connectTimeout: Duration)
class ClientA(val clientId: String, val baseConfig: Base)

This has its own quirks, but it seems best solution compared to other currently possible ones.

PS-2:
I would be more than happy if someone can suggest a cleaner solution other than the ones I mentioned.