Spring-boot: Configuration Processor should use the constructor as a source only with @ConstructorBinding

Created on 31 May 2019  路  4Comments  路  Source: spring-projects/spring-boot

Starting with 2.2.0.M3 spring-boot-configuration-processor generates an incorrect spring-configuration-metadata.json when configuration property class contains another autowired ConfigurationProperties.

For example:

    @Autowired
    public BrokenConfigProperties(@Autowired(required = false) NestedConfigProperties nested) {
        this.nested = nested;
    }

This works in 2.1.5.RELEASE and 2.2.0.M1.
Full example: https://github.com/bdemers/spring-config-problem

Running this example with M2 or M3 creates metadata:

{
  "groups": [
    {
      "name": "another.namespace",
      "type": "com.example.spring.configproblem.NestedConfigProperties",
      "sourceType": "com.example.spring.configproblem.NestedConfigProperties"
    },
    {
      "name": "example.broken",
      "type": "com.example.spring.configproblem.BrokenConfigProperties",
      "sourceType": "com.example.spring.configproblem.BrokenConfigProperties"
    }
  ],
  "properties": [
    {
      "name": "another.namespace.other-property",
      "type": "java.lang.String",
      "sourceType": "com.example.spring.configproblem.NestedConfigProperties"
    },
    {
      "name": "example.broken.nested",
      "type": "com.example.spring.configproblem.NestedConfigProperties",
      "sourceType": "com.example.spring.configproblem.BrokenConfigProperties"
    }
  ],
  "hints": []
}

expected (and 2.1.5.RELEASE output):

{
  "groups": [
    {
      "name": "another.namespace",
      "type": "com.example.spring.configproblem.NestedConfigProperties",
      "sourceType": "com.example.spring.configproblem.NestedConfigProperties"
    },
    {
      "name": "example.broken",
      "type": "com.example.spring.configproblem.BrokenConfigProperties",
      "sourceType": "com.example.spring.configproblem.BrokenConfigProperties"
    }
  ],
  "properties": [
    {
      "name": "another.namespace.other-property",
      "type": "java.lang.String",
      "sourceType": "com.example.spring.configproblem.NestedConfigProperties"
    },
    {
      "name": "example.broken.a-string-value",
      "type": "java.lang.String",
      "sourceType": "com.example.spring.configproblem.BrokenConfigProperties"
    }
  ],
  "hints": []
}

In this case the property a-string-value is missing from the M3 version.

A work around (not fully tested), is to add a default constructor the the config properties object:

    private BrokenConfigProperties() {
        this(null);
    }

    @Autowired
    public BrokenConfigProperties(@Autowired(required = false) NestedConfigProperties nested) {
        this.nested = nested;
    }
bug

Most helpful comment

We've added a dedicated @ConstructorBinding directive in #18469 so we're going to reuse this issue to fix the metadata to check for a dedicated signal that constructor binding should be used.

All 4 comments

@bdemers that setup is slightly odd in the first place. If you use @EnableConfigurationProperties you're asking Spring Boot to instantiate those classes for you and bind them to the environment. In such case, injecting other collaborators (i.e. other beans) is not foreseen. There is actually some note in the documentation about this:

We recommend that @ConfigurationProperties only deal with the environment and, in particular, does not inject other beans from the context.

In the 2.2 line we've started to support constructor binding so it's not only about metadata-generation. That constructor is now flagged as the preferred way to create the bean and bind its content from the environment, see this part of the 2.2 doc and it won't inject another bean but rather try to resolve example.broken.nested as a NestedConfigProperties type. There is no example.broken.nested value in the environment so it's injecting null but the @Autowired annotations there are ignored.

If you really need to get this, I'll use a setter injection for NestedConfigProperties but this object should not expose NestedConfigProperties again (getNested()). As you're exposing it with a public getter, you're basically telling the binder to bind something that was already bound. Not injecting other beans in such class is also a good way to avoid that kind of mistake.

Having said all that, getNested wasn't exposed in the metadata in 2.1.x and it should have been so I'll keep this issue open to investigate why that is. Can you please apply the suggestion above on your current version and let us know how it goes? (Also, @Autowired on constructor parameter is not a thing).

@snicoll I wouldn't read that tip, as don't do it, but more be careful if you are doing this.
I also wouldn't expect this functionality to change in a minor revision (as this a breaking issue).

Good catch on the public Nested getNested() would expect that to show up in 2.1.5 as well. I actual project this is a private method (so i've updated my example project to reflect this)

The metadata generated was not effected by this change (both in 2.1.5 and 2.2.0.M3)

Thanks for the heads up on the @Autowire + constructor. For now, I'll try to work around this to support 2.1 and 2.2 with your suggestions.

Thanks again!

Configuration properties are scanned now and their lifecycle is such that if a constructor is present we use it to bind from the environment. That鈥檚 why the nested one shows up btw as it is now considered an explicit property.

You are correct about the note and doing something we don鈥檛 recommend and for which there is a note against is the reason we think it is acceptable to change in a feature release. If you really need to do this, create the bean yourself (using @Bean) and don鈥檛 use classpath scanning/registration per class.

We've added a dedicated @ConstructorBinding directive in #18469 so we're going to reuse this issue to fix the metadata to check for a dedicated signal that constructor binding should be used.

Was this page helpful?
0 / 5 - 0 ratings