Skip to content

Register beans as RootBeanDefinition instead of GenericBeanDefinition if feasible #42611

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.

Sign up for GitHub

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

Jump to bottom
Closed
wants to merge 1 commit into from
Closed

Register beans as RootBeanDefinition instead of GenericBeanDefinition if feasible #42611

wants to merge 1 commit into from

Conversation

quaff
Copy link
Contributor

@quaff quaff commented Oct 14, 2024

Align with ConfigurationPropertiesBinder and ConfigurationPropertiesBindingPostProcessor

@spring-projects-issues spring-projects-issues added the status: waiting-for-triage An issue we've not yet triaged label Oct 14, 2024
@quaff
Copy link
Contributor Author

quaff commented Oct 14, 2024

There are several places are using genericBeanDefinition(), but rootBeanDefinition() is more common in Spring Boot codebase, I would like to amend this PR if the team think it's worthy to change them also.

@quaff
Copy link
Contributor Author

quaff commented Oct 14, 2024

Spring Framework recommend to use RootBeanDefinition if it's pre-determined as root bean.
see https://github.com/spring-projects/spring-framework/blob/3f889e997cb8648f54e034c29e009960e8a086e3/spring-beans/src/main/java/org/springframework/beans/factory/support/GenericBeanDefinition.java#L32-L34

@snicoll
Copy link
Member

snicoll commented Oct 14, 2024

Yes, RootBeanDefinition is indeed recommended as the framework will built one internally via the merge process. So having it from the get go allows to skip that bit. There are places were we can't use it though. If you still want to amend this PR to use RBD, please let us know.

@quaff
Copy link
Contributor Author

quaff commented Oct 14, 2024

Yes, RootBeanDefinition is indeed recommended as the framework will built one internally via the merge process. So having it from the get go allows to skip that bit. There are places were we can't use it though. If you still want to amend this PR to use RBD, please let us know.

I will amend it later.

@quaff quaff force-pushed the patch-80 branch 2 times, most recently from 234e2a7 to d4035a8 Compare October 15, 2024 00:39
@quaff quaff changed the title Register BoundConfigurationProperties as RootBeanDefinition instead of GenericBeanDefinition Register beans as RootBeanDefinition instead of GenericBeanDefinition if feasible Oct 15, 2024
@quaff
Copy link
Contributor Author

quaff commented Oct 15, 2024 

> Task :spring-boot-project:spring-boot-actuator:compileJava
/home/runner/work/spring-boot/spring-boot/spring-boot-project/spring-boot-actuator/src/main/java/org/springframework/boot/actuate/endpoint/web/reactive/AbstractWebFluxEndpointHandlerMapping.java:527: warning: [deprecation] check(Supplier<Authentication>,T) in AuthorityAuthorizationManager has been deprecated
				.check(this::getAuthentication, null)
				^
  where T is a type-variable:
    T extends Object declared in class AuthorityAuthorizationManager
error: warnings found and -Werror specified
1 error
1 warning

> Task :spring-boot-project:spring-boot-actuator:compileJava FAILED

It's weird, this task works on my machine.

@quaff
Copy link
Contributor Author

quaff commented Oct 15, 2024 

> Task :spring-boot-project:spring-boot-actuator:compileJava
/home/runner/work/spring-boot/spring-boot/spring-boot-project/spring-boot-actuator/src/main/java/org/springframework/boot/actuate/endpoint/web/reactive/AbstractWebFluxEndpointHandlerMapping.java:527: warning: [deprecation] check(Supplier<Authentication>,T) in AuthorityAuthorizationManager has been deprecated
				.check(this::getAuthentication, null)
				^
  where T is a type-variable:
    T extends Object declared in class AuthorityAuthorizationManager
error: warnings found and -Werror specified
1 error
1 warning

> Task :spring-boot-project:spring-boot-actuator:compileJava FAILED

It's weird, this task works on my machine.

Fixed by #42679

@quaff
Register beans as RootBeanDefinition instead of GenericBeanDefinition…
e7c8880 
… if feasible

Spring Framework recommend to use RootBeanDefinition if it's pre-determined as root bean.
@snicoll snicoll added type: task A general task and removed status: waiting-for-triage An issue we've not yet triaged labels Oct 15, 2024
@snicoll snicoll added this to the 3.4.0-RC1 milestone Oct 15, 2024
@snicoll snicoll self-assigned this Oct 15, 2024
snicoll pushed a commit that referenced this pull request Oct 15, 2024
@quaff @snicoll
Use RootBeanDefinition if possible
7427304 
This commit adapts code that was using GenericBeanDefinition to use
RootBeanDefinition instead. Spring Framework recommend to use
RootBeanDefinition if it's pre-determined as root bean.

See gh-42611
snicoll added a commit that referenced this pull request Oct 15, 2024
@snicoll
Polish "Use RootBeanDefinition if possible"
0c0acb4 
This commit adapts code that was using GenericBeanDefinition to use
RootBeanDefinition instead. Spring Framework recommend to use
RootBeanDefinition if it's pre-determined as root bean.

See gh-42611
@snicoll snicoll closed this in 389dead Oct 15, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees

@snicoll snicoll

Labels
type: task A general task
Projects
None yet
Milestone
3.4.0-RC1
Development

Successfully merging this pull request may close these issues.
3 participants
@quaff @snicoll @spring-projects-issues