Allow @ConfigurationProperties binding for immutable POJOs

[sdeleuze]

Currently, it seems we are forced to use Kotlin classes with mutable nullable properties and default constructor with @ConfigurationProperties while idiomatic Kotlin code would be using classes with immutable properties initialized via constructor. I think there is a way to supporting that by leveraging kotlin-reflect library like jackson-module-kotlin do.

More concretely, in MiXiT app I would like to be able to convert this MixitProperties class implementation to:

@ConfigurationProperties("mixit")
class MixitProperties(
    val baseUri: String,
    val admin: Credential,
    val drive: Drive
)

class Credential(
    val username: String,
    val password: String
)

class Drive(
    val fr: DriveDocuments,
    val en: DriveDocuments
)

class DriveDocuments(
    val sponsorform: String,
    val sponsor: String,
    val speaker: String,
    val press: String
)

We could imagine to support optional properties by using nullable types, like val sponsorform: String? for example.

I will be happy to help. I have also already worked with @apatrida who maintains Jackson Kotlin module, he may provide us some guidance I think.

[snicoll]

We currently use Spring Framework's binder (that's also used in the MVC world) and it does not support binding via constructor. I guess that's the problem you want to solve?

We're rewriting the binder for Boot 2.0, perhaps @philwebb has some plans about that?

[snicoll]

@jhoeller just pinged me to mention SPR-15199. Would be pretty cool if we could use that somehow but I guess the best course of action is our binder and 2.0 anyway...

Having said that, perhaps we can reuse some bits?

[apatrida]

There are two models that work with Kotlin. The first, is that a plugin model to the binder let's Kotlin describe the constructors and properties providing information about which can be used to construct the class (or a combination of constructor + properties set after). The second, is that a source of all settable things is made available and the decision is made by a Kotlin instantiator that does the work based on what it has available.

The best support considers all of:

• what properties are set in the constructor
• what properties are writeable after construction
• for any property if it is nullable
• for any property in the constructor has a default value that can be used if it is absent

Kotlin reflection has a callBy method on the constructor that can be used in the case that default values are being used, otherwise it should use call method.

So the binder should give the opportunity to a plugin to do the introspection and instantiation so that the plugin can do the best job for the language.

Binding examples for Kotlin:

Jackson Kotlin Module (pulls parameter name information, then hands it back along with actual parameters to a custom instantiator)

JDBI 3 (the Kotlin module handles all binding tasks when the target is a Kotlin class)

[apatrida]

If there is a branch to track for the new binder, I can look at it and see what needs to be done to support Kotlin, and add that support.

[philwebb]

The experimental binder code is here, but it's still very much a work in progress. The plan is to make a binder that's specifically designed for binding from our configuration properties. It'll allow us to do some nice things like track the line/column number that a property was loaded from.

The Binder class is pretty complete, and there are quite a few tests. It deals with Collections, Arrays, Maps and Scalar types directly. When it finds something it can't handle it delegates to BeanBinder implementations.

We currently have a JavaBeanBinder that works with mutable classes but the plan is to add a couple more. We'd like to have a ValueObjectBeanBinder that binds using constructor arguments (this should work well with Kotlin).

We've also got plans for an InterfaceBeanBinder. The idea would be that you can so something like this:

@ConfigurationProperties(prefix="mine")
public interface MyProperties {
 
    @DefaultValue("spring")
    public String getName();

    public int getAge();

}

Which would be used to generate a proxy class with the appropriate implementation.

The first step is to get the binder working with our existing mutable configuration property beans. We'll then look at extending the support.

[bclozel]

Quick update here: #8868 has been merged, the new binder is in master.

[sdeleuze]

@apatrida Since the new binder is now in master, any chance you could have a look to identify what needs to be done to provide first class support for immatable data classes in Kotlin?

[serandel]

Is this planned for the 2.0 release or for later in the roadmap? @ConfigurationProperties are a bit confusing without it.

[snicoll]

To summarize the discussion I just had with @sdeleuze we have interface binding on the pipe for 2.0 (see #1254). This issue is really about supporting constructor binding as well and it brings a number of challenges. One of them is that we need to upgrade the annotation processor to be able to detect and generate the metadata in such a case.

I bet Java developers will be interested by this feature as well and will use it with Lombok which will force us to detect all of these cases and I anticipate it is a lot of work. And supporting constructor binding without the metadata support is a no-go on my end.

I am flagging this one for team attention so that the rest of the team has a chance to review this.

[wilkinsona]

@serandel Can you please expand a bit on what you find confusing at the moment?

[serandel]

@wilkinsona Of course. :)

Compare this class from my project (please, ignore the missing ElasticsearchSource definition):

@Component
@ConfigurationProperties
class ElasticsearchConfiguration {
    var origin : ElasticsearchSource? = null
    var destination : ElasticsearchSource? = null

    override fun toString() = """
    |ElasticsearchConfiguration:
    |origin=$origin
    |destination=$destination
    """.trimMargin()
}

to the version I tried first:

@Component
@ConfigurationProperties
data class ElasticsearchConfiguration(val origin: ElasticsearchSource, val destination :ElasticsearchSource?)

Several features are dropped:

• properties have to be mutable
• properties have to be nullable or use lateinit
• equals, hashCode and toString are not included, and have to be written by hand (as I did in one case, just for debugging)
• copy and componentN are not included neither

From a Kotlin point of view, this type of configuration is obviously an immutable data class. There is some (very small, I concede) cognitive dissonance not being able to use a value type instead of a regular class for a limitation in the framework. Plus, the component is open to same rogue code altering a property.

I would really love to have this issue sorted out, as it would lead to beautiful, terse, expressive and idiomatic Kotlin data classes. :)

[serandel]

I just updated the previous comment. I thought that lateinit var would work... and it doesn't. :(

So, furthermore, we lose the distinction between optional and required properties. Perhaps it could be somewhat softened by using @required annotations and validating, I don't know, but it's not ideal.

[sdeleuze]

Comment updated

2 remarks on the provided use case:

• IMO we don't really care if that's a data class or not
• I don't think you should inject beans in such class

That said, I think what makes @ConfigurationProperties confusing in Kotlin is is that immutable classes initialized at constructor level are a super common pattern (and a best practice) in Kotlin and a perfect use case here.

Kotlin null-safety makes this even more needed because you can only support that correctly via constructor based injection. Otherwise you force developers to write code like properties.admin!!.username!! with additional null safety operators where just properties.admin.username should be needed.

A regular Java ValueObjectBeanBinder that binds using constructor arguments should support Kotlin immutable classes (including data classes) without additional effort. The main point to check will be how it behaves with null-safety. So such @ConfigurationProperties should work:

@ConfigurationProperties("mixit")
class MixitProperties(
    val baseUri: String,
    val admin: Credential
)

class Credential(
    val username: String,
    val password: String?
)

A more advanced use case that will require specific Kotlin support will be classes with optional parameters and default values like this one since from a bytecode POV 2 constructors are generated:

@ConfigurationProperties("mixit")
class MixitProperties(
    val baseUri: String = "http://localhost:8080",
    val admin: Credential
)

class Credential(
    val username: String = "root",
    val password: String?
)

I should be able to provide the relevant utils method as part of https://jira.spring.io/browse/SPR-15673 to deal with this use case, making possible for Spring Boot to leverage it.

[sdeleuze]

FYI Spring Framework 5.0 RC3 BeanUtils now fully supports Kotlin classes with immutable properties and optional parameters + default values while still providing API and implementation relevant for regular Java beans via the new BeanUtils#findPrimaryConstructor(Class) method and BeanUtils.instantiateClass(Constructor, Object...) which now supports Kotlin classes with optional parameters + default values.

Notice that Spring Data is about to add proper support for Kotlin constructor as well, so such support in Spring Boot for @ConfigurationProperties is the last missing bit to get proper support for typical Spring + Kotlin application based on immutable POJOs which are idiomatic in Kotlin (and a best practice worth to promote IMO).

For what I have understood from previous discussion, and since we now have the confirmation that Kotlin support for immutable POJOs for @ConfigurationProperties is mainly a small extension of the Java support for that, should we modify the title and scope of this issue to "Allow @ConfigurationProperties binding for immutable POJOs" in order to have an equivalent of #1254 but applied to POJO binding?

I know you have a huge todo for Spring Boot 2.0, but with the guidance of somebody aware of the new @ConfigurationProperties infrastructure, maybe I could help in contributing Java and Kotlin immutable POJO PR? I guess that could also be a way to make sure that current design is a good fit with immutable classes and eventually adapt it before 2.0 API freeze.

[philwebb]

I think the code changes for the binding won't be that tricky. The annotation processor that generates the meta-data might be a bit harder.

[snicoll]

There are more side effect to it. Like annotations we can only put on the getter that we'll have to accept on fields. Non scalar (nested) properties is also a challenge as soon as it's not an inner class.

[snicoll]

We discussed this one at our team call last week and it's still going to be a challenge in its current form. @ConfigurationProperties annotated types becomes beans ultimately. Either because the type was provided explicitly or because a bean is exposed manually.

Regardless, a bean has a lifecycle and the constructor is one of them. While we don't advocate for injecting anything in a @ConfigurationProperties object, there's nothing stopping you from doing that. Concretely, it means that we can't afford to use the constructor as a mean to pass the environment properties.

That's why I see interface-based binding as a very good way forward: we keep control over the object's lifecyle and things are more consistent with what we have now.

Of course, the other options would be to stop allowing a @ConfigurationProperties type to be a bean which is quite harsh and brings other challenges (binding of 3rd party object we build in an auto-config being the most obvious one).

[sdeleuze]

After discussing with @snicoll, I agree this feature requires to stop allowing a @ConfigurationProperties to be regular beans as well.

And in fact I see at least 3 advantages to remove such capability:

• It would enable @ConfigurationProperties binding for immutable POJOs (solve this issue) which is nice to have in Java but the most idiomatic way to support such feature in Kotlin
• It would make class and interface based @ConfigurationProperties bindings more consistent (in a sense interface based @ConfigurationProperties binding is a kind of workaround to avoid having the bean injection issue)
• It would stop to confuse users by providing a more error-prone and opinionated solution (I see so many people confused by these double properties + bean injection capabilities on Stack Overflow).

I may be biased and there would be side effects as explained by @snicoll, but I think that would be a good change even if a breaking one, so I vote 👍 for removing such support from Boot 2.0.

To move forward about the original need for Kotlin + Boot developers regardless of what Boot team decide on this issue, my point of view is that the most important point is to provide a solution to the current situation where both escaped @Value("\${foo}") and nullable mutable setter/field based @ConfigurationProperties are 2 far from ideal possible solutions .

This can be solved by this issue (ideal solution) but also by interface binding if Kotlin interfaces with val properties are supported (I will had a comment on this issue with more details about Kotlin bytecode generated in such case in order to evaluate if such support will be doable or not).

[sdeleuze]

Thanks a lot Boot team for fixing that!

[wilkinsona]

Re-opening so that we can update the reference docs. I’d also like to take another look at the occasional need for @Autowired although I am not sure there’s a better alternative.

[wilkinsona]

The documentation refers to @ConfigurationPropertyDefaultValue but the annotation was renamed to @DefaultValue.

[Azbesciak]

In the doc there is info that this is still not supported...

[snicoll]

That link is for the wrong version of Spring Boot.

This was updated properly: https://docs.spring.io/spring-boot/docs/2.2.0.BUILD-SNAPSHOT/reference/html/spring-boot-features.html#boot-features-kotlin-configuration-properties

[Azbesciak]

Ok, but this is first in the Google... and says that this is current.

[wilkinsona]

The current refers to the current, i.e. latest, Spring Boot release that is GA. Once 2.2 reaches that state, the current link will point to the documentation for 2.2.0.RELEASE.

[thekalinga]

@wilkinsona Does this work with Java aswell (or) it works just with kotlin?

I tried with the following configuration & it did not resolve properties. Not sure if I had any mistake

application.yml

config:
  username: alpha
  password: password
  timeoutInSecs: 2

ExternalSystemConfig.java

@Getter
@Builder
@AllArgsConstructor(onConstructor = @__(@JsonCreator))
@ToString
@ConfigurationProperties("config")
public class ExternalSystemConfig {
  private String username;
  private String password;
  private int timeoutInSecs;
}

DemoApplication.java

@Log4j2
@SpringBootApplication
@EnableConfigurationProperties(ExternalSystemConfig.class)
public class DemoApplication {

  public static void main(String[] args) {
    new SpringApplicationBuilder()
        .sources(DemoApplication.class)
        .web(WebApplicationType.NONE)
        .run(args);
  }

  @Bean
  InitializingBean configLogger(ExternalSystemConfig config) {
    return () -> log.info("Config: {}",  config);
  }

}

But the config object that gets injected have all values set to null

I'm using 2.2.0.BUILD-SNAPSHOT version for this

Any idea if I'm doing something wrong

Thanks!

[thekalinga]

If I make my object mutable, the binding works fine tho

[thekalinga]

This is how my (lombok) generated class looks like

  @ConstructorProperties({"username", "password", "timeoutInSecs"})
  @JsonCreator
  public ExternalSystemConfig(String username, String password, int timeoutInSecs) {
    this.username = username;
    this.password = password;
    this.timeoutInSecs = timeoutInSecs;
  }

[wilkinsona]

@thekalinga Immutable configuration property binding works with both Java and Kotlin. If you believe you've found a situation where it's not working as expected, please open a new issue with a complete and minimal sample that reproduces the problem. A project zipped and attached to the new issue or hosted in a separate GitHub repository is ideal.

[thekalinga]

@wilkinsona Just opened an issue with test project

#16928