Support constructor binding on 3rd party classes

[tomfi]

I use kotlin, and I want to use data classes to describe my properties/options. The data classes themselves sit in core modules that do not depend on spring, and should have minimal 3rd party dependencies.

On top of the modules mentioned above, I built spring auto-configuration modules that create beans, load properties, and utilize spring's features to expose the core module as proper spring modules.

It would be great if I could create a data class in my core module, e.g.

data class FooProperties(
  val bar: String
)

And then in the spring auto-configuration module somehow load it with constructor binding, e.g.

@Configuration
@EnableConfigurationProperties(value = [FooProperties::class], constructorBinding: true)
class FooAutoConfiguration

Or maybe some way to do it programmatically, e.g.

@Bean
fun fooProperties(binder: SpringConstructorBinder, environment: Environment): FooProperties {
  return binder.bind(environment, FooProperties::class)
}

I saw the following issues: #18935, #19011.
If i understand correctly i can create a bean that wraps my data class and the constructor binding will work for my inner class, but it will be nice to have a better way to define the properties without additional wrappers.

[philwebb]

Adding an option to @EnableConfigurationProperties is an interesting idea, but I'm a bit worried that there isn't a place to put the prefix that's usually in @ConfigurationProperties. Perhaps a dedicated annotation might be better? Something like:

@RegisterConfigurationPropertiesBean(value=FooProperties.class, prefix="myproperties")

As things stand, I think you can almost do what you want by using the Binder directly. The only problem is that ConfigurationPropertiesBean.getAll won't find them so the /configprops actuator endpoint won't be correct.

@Bean
public FooProperties fooProperties(Environment env) {
    return Binder.get(env).bindOrCreate("my.foo", FooProperties.class);
}

[tomfi]

@philwebb - yes a dedicated annotation will be perfect, I forgot about the prefix. It will make it easier to register the data classes with the correct prefix and to enable constructor binding.

I will try the 2nd option as a temporary solution.

Another thing, in case there will be a dedicated annotation, will it also be picked by spring-boot-configuration-processor?
I guess that by creating a bean programmatically it won't be picked by the annotation processor so there won't be auto-complete with spring's plugin in the IDE.

[snicoll]

If we'd created a separate annotation we'd have to update the annotation processor to handle it I guess. The contract for third party classes so far was to expose them as a @Bean with a @ConfigurationProperties prefix. My vote is to keep doing that and offering some sort of contract that you can invoke programmatically. A FactoryBean perhaps?

[tomfi]

@snicoll the problem comes when you want to use kotlin + data-class + constructor-binding. You can't initialize the class in a Bean unless you use the Binder like @philwebb mentioned.

And in both cases I think the annotation processor won't catch them in order to create the meta-data for the yaml editors which is very important for people like me that create libraries for other developers.

I think there should be an easy way to register immutable data classes (with constructor binding) of 3rd party classes with all the benefits of normal classes (e.g. annotation processing, actuator, etc)

[snicoll]

@tomfi I read the issue and I am aware of that, thank you.

[Davio]

Maybe I'm not interpreting this correctly, but it looks like @ConfigurationProperties is an annotation that can be put on a @Bean method, but @ConstructorBinding is an annotation which needs to be put on the target type or constructor. For domain model classes which are kept isolated from Spring, this is not ideal.

How about adding a parameter, such as bindingMode to @ConfigurationProperties which defaults to using setters, but can be set to use the constructor.

Example:

@Bean
@ConfigurationProperties(prefix = "my.foo", bindingMode = ConfigurationProperties.BindingMode.CONSTRUCTOR)
public FooProperties fooProperties() {}

[tomfi]

@Davio looks nice, but what will you put in the actual method body?

@Bean
@ConfigurationProperties(prefix = "my.foo", bindingMode = ConfigurationProperties.BindingMode.CONSTRUCTOR)
public FooProperties fooProperties() {
    // compilation error, must provide parameters to the constructor
    return new FooProperties();
}

And if you use Binder.get(env).bindOrCreate("my.foo", FooProperties.class);, then the annotation does not do anything.

[Davio]

Oh of course the method body, it must return an actual instance of the thing.

So if you want to do it without a method body. you basically need the annotation to be processed entirely by the annotation processor and you get the suggested @RegisterConfigurationPropertiesBean(value=FooProperties.class, prefix="myproperties")

And if you go with the existing route, you need to fill in the body, so that's why the factory bean option whas proposed.

I guess a workaround would be to just use the Kotlin no-arg compiler plugin and annotate your data classes with some special custom annotation, like @com.myname.foo.Properties so you can use them as regular property classes from a Java / Spring point of view. It's not ideal, but it would work. It might be tempting to add such an annotation to Spring, but that would make the domain model depend on Spring and we're trying to avoid that.

[philwebb]

I tried a few different approaches and landed on a new @ImportConfigurationPropertiesBean annotation. The FactoryBean approach unfortunately didn't work out since it needed to be aware of the annotation on the factory method that created it.

The new annotation is conceptually a mix of @Import and @ConfigurationProperties. It's repeatable and you can use it on any @Configuration class. There are still a few things that you can't do if you don't annotate the source class directly. Specifically, you can't have multiple constructors and you can't have default-values. For those cases you'll need a subclass that adds @ConstructorBinding and @DefaultValue annotations.

The original feature request can now be implemented as follows:

data class FooProperties(
  val bar: String
)

@Configuration
@ImportConfigurationPropertiesBean(type = [FooProperties::class], prefix = "foo")
class FooAutoConfiguration

[philwebb]

Oh, I almost forgot. The annotation processor has also been updated to generate meta-data if it finds @ImportConfigurationPropertiesBean.

[tomfi]

@philwebb amazing!!

Thank you very much :)

[tomfi]

I tried a few different approaches and landed on a new @ImportConfigurationPropertiesBean annotation. The FactoryBean approach unfortunately didn't work out since it needed to be aware of the annotation on the factory method that created it.

The new annotation is conceptually a mix of @Import and @ConfigurationProperties. It's repeatable and you can use it on any @Configuration class. There are still a few things that you can't do if you don't annotate the source class directly. Specifically, you can't have multiple constructors and you can't have default-values. For those cases you'll need a subclass that adds @ConstructorBinding and @DefaultValue annotations.

The original feature request can now be implemented as follows:

data class FooProperties(
  val bar: String
)

@Configuration
@ImportConfigurationPropertiesBean(type = [FooProperties::class], prefix = "foo")
class FooAutoConfiguration

@philwebb you wrote that default values are not supported. If the data class is as the following:

data class FooProperties(
  val a: String,
  val b: String = "default"

Will it honor the default value of the data class and succeed in creating the bean?

[philwebb]

@tomfi I'm not too sure what bytecode Kotlin with generate for default values. If it's a primary constructor that accepts null then it should bind just fine. One thing I don't think you'll get is the value in your metadata JSON file.

[philwebb]

Reopening to rename the annotation to @ImportAsConfigurationProperties following a team discussion.

[philwebb]

I've renamed it to @ConfigurationPropertiesImport for now, we'll need to try it for a bit and see if that feels natural.

[tomfi]

@tomfi I'm not too sure what bytecode Kotlin with generate for default values. If it's a primary constructor that accepts null then it should bind just fine. One thing I don't think you'll get is the value in your metadata JSON file.

Yes it is a single constructor, it works perfectly fine with @ConstructorBinding so I guess it will work here as well.

[wilkinsona]

@philwebb What made you move away from @ImportAsConfigurationProperties that we agreed upon yesterday? To me, @ConfigurationPropertiesImport still suggests that what is being imported is @ConfigurationProperties. I still like the as in the annotation name as it provides a hint that the target of the import isn't already @ConfigurationProperties. I think that's important given the intended usage of the annotation.

[philwebb]

I was super frustrated yesterday trying to come up with a name for the container annotation, so I wanted to try flipping the terms to see if it made any difference. I agree, it didn't really work :(

We originally were thinking @ImportAsConfigurationProperties/@ImportAllAsConfigurationProperties but @ImportAllAsConfigurationProperties feels really weird, especially as @ImportAsConfigurationProperties already takes an array of types. I'd really like to find a name that doesn't end with s so that we can use the standard convention for the container.

Perhaps we should add Bean back as a suffix?

@ImportAsConfigurationPropertiesBean / @ImportAllAsConfigurationPropertiesBeans

Either that or I should just forget about finding a good name for the container and we go with:

@ImportAsConfigurationProperties / @ImportAllAsConfigurationPropertiesContainer

[Davio]

My only remaining question is: if there are multiple constructors, how does it deduce which one to use? The one with the most (matched) arguments or something like that?

[tomfi]

@Davio as @philwebb mentioned above:

you can't have multiple constructors and you can't have default-values

So the answer is, this will work only if there is a single constructor - which is perfect, at least for my kotlin data-class use case.

[Davio]

Okay, for your use case it will work, but if you have a non-data Kotlin class with default parameters, it won't work. Maybe that's an acceptable tradeoff as there might not be a sensible way to determine the 'primary' constructor without it having a special annotation or something like that.

[snicoll]

FTR we've decided to revert this feature as it brings a number of inconsistencies. Please follow-up on #23607