Skip to content

Support multiple RequestRejectedHandler beans. #10603

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

Closed

Conversation

Drezir
Copy link
Contributor

@Drezir Drezir commented Dec 11, 2021

Code changes to the previous PR #8985

@spring-projects-issues spring-projects-issues added the status: waiting-for-triage An issue we've not yet triaged label Dec 11, 2021
@jzheaux jzheaux self-assigned this Dec 13, 2021
@jzheaux jzheaux added in: web An issue in web modules (web, webmvc) type: enhancement A general enhancement and removed status: waiting-for-triage An issue we've not yet triaged labels Dec 13, 2021
Copy link
Contributor

@jzheaux jzheaux left a comment

Choose a reason for hiding this comment

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

Thanks, @Drezir! In addition to my inline feedback, will you please ensure that the copyright sections at the top of each class are up to date?

@jzheaux jzheaux added this to the 5.7.x milestone Dec 13, 2021
@Drezir Drezir force-pushed the feature/requestrejectedhandlers branch from d2b442c to f9ff7e9 Compare December 14, 2021 06:35
@Drezir
Copy link
Contributor Author

Drezir commented Dec 14, 2021

Copyright sections updated

Copy link
Contributor

@jzheaux jzheaux left a comment

Choose a reason for hiding this comment

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

Thanks, @Drezir, for the updates!

In addition to my inline feedback, will you please add some unit tests to WebSecurityTests for the new method and to a class called CompositeRequestRejectedHandlerTests for the new class? Remember to place those tests in their respective commits.

@Drezir
Copy link
Contributor Author

Drezir commented Dec 15, 2021

Thanks, @Drezir, for the updates!

In addition to my inline feedback, will you please add some unit tests to WebSecurityTests for the new method and to a class called CompositeRequestRejectedHandlerTests for the new class? Remember to place those tests in their respective commits.

I have added some tests, thanks to that I have discovered one problem so the tests have proven that they are useful 👍
I did not want to squash commits during code review, would you like me to do it and organize it like 1st commit for CompositeHandler and 2nd commit for WebSecurity setter?

@Drezir Drezir force-pushed the feature/requestrejectedhandlers branch from 0b839b9 to d0e8d29 Compare December 15, 2021 17:53
Copy link

@perovovao perovovao left a comment

Choose a reason for hiding this comment

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

  • [ ]

@Drezir
Copy link
Contributor Author

Drezir commented Jan 12, 2022

I have some issues building project.

Settings file '/home/drezir/Projects/spring-security/settings.gradle' line: 10
Plugin [id: 'io.spring.ge.conventions', version: '0.0.7'] was not found in any of the following sources:

May I ask someone to fix checkstyle issues please? Thank you.

jzheaux pushed a commit that referenced this pull request Jan 15, 2022
jzheaux added a commit that referenced this pull request Jan 15, 2022
@jzheaux
Copy link
Contributor

jzheaux commented Jan 15, 2022

Thanks for the contribution, @Drezir! This is merged into 5.7.x via 4ea57f3 as well as a polish of 75f25bf for the checkstyle fixes.

@jzheaux jzheaux closed this Jan 15, 2022
@jzheaux jzheaux modified the milestones: 5.7.x, 5.7.0-M2 Jan 15, 2022
jzheaux pushed a commit that referenced this pull request Jan 15, 2022
jzheaux added a commit that referenced this pull request Jan 15, 2022
@Drezir Drezir deleted the feature/requestrejectedhandlers branch January 15, 2022 19:53
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
in: web An issue in web modules (web, webmvc) type: enhancement A general enhancement
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants