Skip to content

Add missing attributes to ServletRegistration annotation#45007

Closed
dmytrodanilenkov wants to merge 9 commits intospring-projects:mainfrom
dmytrodanilenkov:fix-issue-45001
Closed

Add missing attributes to ServletRegistration annotation#45007
dmytrodanilenkov wants to merge 9 commits intospring-projects:mainfrom
dmytrodanilenkov:fix-issue-45001

Conversation

@dmytrodanilenkov
Copy link
Copy Markdown
Contributor

@dmytrodanilenkov dmytrodanilenkov commented Apr 4, 2025

Closes #45001

@spring-projects-issues spring-projects-issues added the status: waiting-for-triage An issue we've not yet triaged label Apr 4, 2025
@dmytrodanilenkov
Fix for issue spring-projects#45001 - ServletRegistrationBean has tho…
12bf433 
…se properties, but @ServletRegistration hasn't: initParameters, servletRegistrationBeans, multipartConfig

Signed-off-by: Dmytro Danilenkov <milgoff@gmail.com>
@mhalbritter
Copy link
Copy Markdown
Contributor

mhalbritter commented Apr 7, 2025

I like the handling of initParameters and servletRegistrationBeans in #45005. Could you rework your PR to match #45005?

@mhalbritter mhalbritter mentioned this pull request Apr 7, 2025
Closed
@mhalbritter mhalbritter changed the title add properties initParameters servletRegistrationBeans multipartConfig to @ServletRegistration Add missing attributes to ServletRegistration annotation Apr 7, 2025
@mhalbritter mhalbritter added the status: waiting-for-feedback We need additional information before we can continue label Apr 7, 2025
@dmytrodanilenkov
Fix for issue spring-projects#45001 - ServletRegistrationBean has tho…
da9173b 
…se properties, but @ServletRegistration hasn't: initParameters, servletRegistrationBeans, multipartConfig Signed-off-by: Dmytro Danilenkov <milgoff@gmail.com>

Signed-off-by: Dmytro Danilenkov <milgoff@gmail.com>
@dmytrodanilenkov
Copy link
Copy Markdown
Contributor Author

dmytrodanilenkov commented Apr 7, 2025

there’s no built-in servletRegistrationBeans property or method in Spring Boot’s standard ServletRegistrationBean. for now I removed it. or maybe I understand issue incorrectly?

@dmytrodanilenkov
Fix for issue spring-projects#45001 - ServletRegistrationBean has tho…
3ca2d29 
…se properties, but @ServletRegistration hasn't: initParameters, servletRegistrationBeans, multipartConfig Signed-off-by: Dmytro Danilenkov <milgoff@gmail.com>

Signed-off-by: Dmytro Danilenkov <milgoff@gmail.com>
@spring-projects-issues spring-projects-issues added status: feedback-provided Feedback has been provided and removed status: waiting-for-feedback We need additional information before we can continue labels Apr 7, 2025
@mhalbritter
Copy link
Copy Markdown
Contributor

mhalbritter commented Apr 8, 2025

Sorry, my mistake. There's no setServletRegistrationBeans on ServletRegistrationBean, it's only on FilterRegistrationBean.

@mhalbritter mhalbritter added type: enhancement A general enhancement and removed status: waiting-for-triage An issue we've not yet triaged status: feedback-provided Feedback has been provided labels Apr 8, 2025
@mhalbritter mhalbritter added this to the 3.5.x milestone Apr 8, 2025
@mhalbritter mhalbritter mentioned this pull request Apr 8, 2025
Closed
dmytrodanilenkov and others added 3 commits April 8, 2025 10:06
@dmytrodanilenkov
Merge branch 'main' into fix-issue-45001
fce9e44 
Signed-off-by: Dmytro Danilenkov <63580205+dmytrodanilenkov@users.noreply.github.com>
@dmytrodanilenkov
Fix for issue spring-projects#45001 - ServletRegistrationBean has tho…
839b087 
…se properties, but @ServletRegistration hasn't: initParameters, servletRegistrationBeans, multipartConfig Signed-off-by: Dmytro Danilenkov <milgoff@gmail.com>

Signed-off-by: Dmytro Danilenkov <milgoff@gmail.com>
@dmytrodanilenkov
Fix for issue spring-projects#45001 - ServletRegistrationBean has tho…
9bb90c9 
…se properties, but @ServletRegistration hasn't: initParameters, servletRegistrationBeans, multipartConfig Signed-off-by: Dmytro Danilenkov <milgoff@gmail.com>

Signed-off-by: Dmytro Danilenkov <milgoff@gmail.com>
@dmytrodanilenkov
Copy link
Copy Markdown
Contributor Author

dmytrodanilenkov commented Apr 8, 2025

so changes done, please review

nosan
nosan reviewed Apr 9, 2025
View reviewed changes
Copy link
Copy Markdown
Contributor

@nosan nosan left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks, @dmytrodanilenkov!
I've left some comments for your consideration.

Could you also take a look at the Checkstyle issues?

You can verify them using:

./gradlew checkstyleMain checkstyleTest

Comment thread ...-boot/src/main/java/org/springframework/boot/web/servlet/ServletContextInitializerBeans.java Outdated
Comment thread .../src/test/java/org/springframework/boot/web/servlet/ServletContextInitializerBeansTests.java Outdated
@dmytrodanilenkov
Fix for issue spring-projects#45001 - ServletRegistrationBean has tho…
c0a8409 
…se properties, but @ServletRegistration hasn't: initParameters, servletRegistrationBeans, multipartConfig Signed-off-by: Dmytro Danilenkov <milgoff@gmail.com>

Signed-off-by: Dmytro Danilenkov <milgoff@gmail.com>
@nosan
Copy link
Copy Markdown
Contributor

nosan commented Apr 9, 2025 •
edited
Loading

Thanks, @dmytrodanilenkov

Execution failed for task ':spring-boot-project:spring-boot:checkFormatMain'.
> Formatting violations found in the following files:
   * src/main/java/org/springframework/boot/web/servlet/ServletContextInitializerBeans.java

* Try:
> Run with --stacktrace option to get the stack trace.
> Run with --info or --debug option to get more log output.
> Get more help at https://help.gradle.org.
Deprecated Gradle features were used in this build, making it incompatible with Gradle 9.0.

Could you please run ./gradlew format?

Additionally, I've noticed some minor changes unrelated to your PR, such as removed blank lines. Could you please revert these?

To verify your changes: ./gradlew :spring-boot-project:spring-boot:build or ./gradlew build

@dmytrodanilenkov
Fix for issue spring-projects#45001 - ServletRegistrationBean has tho…
d4bdaa1 
…se properties, but @ServletRegistration hasn't: initParameters, servletRegistrationBeans, multipartConfig Signed-off-by: Dmytro Danilenkov <milgoff@gmail.com>

Signed-off-by: Dmytro Danilenkov <milgoff@gmail.com>
@dmytrodanilenkov
Fix for issue spring-projects#45001 - ServletRegistrationBean has tho…
4c1af47 
…se properties, but @ServletRegistration hasn't: initParameters, servletRegistrationBeans, multipartConfig Signed-off-by: Dmytro Danilenkov <milgoff@gmail.com>

Signed-off-by: Dmytro Danilenkov <milgoff@gmail.com>
@dmytrodanilenkov
Copy link
Copy Markdown
Contributor Author

dmytrodanilenkov commented Apr 9, 2025

@nosan done

@mhalbritter mhalbritter self-assigned this Apr 10, 2025
mhalbritter pushed a commit that referenced this pull request Apr 10, 2025
@dmytrodanilenkov @mhalbritter
Add missing attributes to ServletRegistration annotation
d012807 
See gh-45007

Signed-off-by: Dmytro Danilenkov <milgoff@gmail.com>
mhalbritter added a commit that referenced this pull request Apr 10, 2025
@mhalbritter
Polish "Add missing attributes to ServletRegistration annotation"
5401327 
See gh-45007
@mhalbritter
Copy link
Copy Markdown
Contributor

mhalbritter commented Apr 10, 2025

Thanks @dmytrodanilenkov ! And thanks @nosan for the review!

@mhalbritter mhalbritter modified the milestones: 3.5.x, 3.5.0-RC1 Apr 10, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@mhalbritter mhalbritter mhalbritter left review comments

+1 more reviewer

@nosan nosan nosan left review comments

Reviewers whose approvals may not affect merge requirements

Assignees

@mhalbritter mhalbritter

Labels

type: enhancement A general enhancement

Projects

None yet

Milestone

3.5.0-RC1

Development

Successfully merging this pull request may close these issues.

Add missing attributes to ServletRegistration annotation

4 participants

@dmytrodanilenkov @mhalbritter @nosan @spring-projects-issues