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

Configuration metadata annotation processor may use the wrong accessor for boolean properties #24002

Closed
mzeijen opened this issue Nov 3, 2020 · 5 comments
Assignees
Labels
type: bug A general bug
Milestone

Comments

@mzeijen
Copy link
Contributor

mzeijen commented Nov 3, 2020

Spring Boot version: 2.3.5.RELEASE

The server.tomcat.use-relative-redirects property is incorrectly marked as deprecated in the spring-configuration-metadata.json of the spring-boot-autoconfiguration module

The problem is that the server.tomcat.use-relative-redirects is not deprecated, but only the Boolean typed getter and setter methods of the org.springframework.boot.autoconfigure.web.ServerProperties.Tomcat#useRelativeRedirects are deprecated (see the daed512 commit and the #20796 issue for the related changes). The addition of the @Deprecated to the Boolean getUseRelativeRedirects caused the deprecated to be set on true for the server.tomcat.use-relative-redirects entry in the spring-configuration-metadata.json of the spring-boot-autoconfiguration module.

The incorrect deprecation of this property now causes the properties migration listener to warn about the use of this property:

The use of configuration keys that are no longer supported was found in the environment:

Property source 'applicationConfig':
	Key: server.tomcat.use-relative-redirects
		Reason: none


Please refer to the release notes or reference guide for potential alternatives.

The Intellij application properties editor now also warns about setting this property.

@spring-projects-issues spring-projects-issues added the status: waiting-for-triage An issue we've not yet triaged label Nov 3, 2020
@snicoll
Copy link
Member

snicoll commented Nov 3, 2020

Good catch @mzeijen, we wanted to re-introduce the Boolean version in a deprecated fashion but unfortunately that triggered the annotation processor the way you indicated.

See #20796

@snicoll snicoll added type: bug A general bug and removed status: waiting-for-triage An issue we've not yet triaged labels Nov 3, 2020
@snicoll snicoll added this to the 2.2.x milestone Nov 3, 2020
@snicoll
Copy link
Member

snicoll commented Nov 3, 2020

Unfortunately, we won't be able to "undo" the deprecation with manual metadata as things stand. Manual metadata is used to override the reason, replacement or level but not the fact the property is deprecated.

@snicoll snicoll added the for: team-attention An issue we'd like other members of the team to review label Nov 3, 2020
@mzeijen
Copy link
Contributor Author

mzeijen commented Nov 3, 2020

For now I created a workaround where I set the useRelativeRedirects directly on the Tomcat Context via a WebServerFactoryCustomizer<TomcatServletWebServerFactory>.
Luckily you made it possible to have multiple roads to Rome :).

It would be nice to be able to get rid of this workaround at some point, so I would appreciate some kind of fix. But if that can only happen in a next big release of Spring Boot, then so be it.

@wilkinsona wilkinsona removed the for: team-attention An issue we'd like other members of the team to review label Nov 4, 2020
snicoll added a commit to snicoll/spring-boot that referenced this issue Nov 5, 2020
This commit makes sure to use the most specific getter if more than
one candidate exists.

Closes spring-projectsgh-24002
@snicoll snicoll changed the title server.tomcat.use-relative-redirects property incorrectly marked as deprecated Configuration metadata annotation processor may use the wrong accessor for boolean properties Nov 5, 2020
@snicoll
Copy link
Member

snicoll commented Nov 5, 2020

I've repurposed this issue to fix a bug in the annotation processor where it may use the wrong accessor for a boolean type in case of multiple candidates, as in this case. The former Boolean accessor for the properties are deprecated and we should use the one matching the field type, really (boolean). Doing so will prevent the property to be wrongly identified as deprecated.

@mzeijen
Copy link
Contributor Author

mzeijen commented Nov 5, 2020

That is a great solution. Thank you very much.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
type: bug A general bug
Projects
None yet
Development

No branches or pull requests

4 participants