Skip to content

SearchStrategy argument of MethodValidationExcludeFilter byAnnotation(Class, SearchStrategy) is not used #30631

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
wilkinsona opened this issue Apr 11, 2022 · 5 comments
Assignees
Labels
type: bug A general bug
Milestone

Comments

@wilkinsona
Copy link
Member

From @chanhyeong on Gitter:

I have a question about the static method byAnnotation(Class<? extends Annotation> annotationType, SearchStrategy searchStrategy) in MethodValidationExcludeFilter.
It has a searchStrategy argument that doesn't be used (has a fixed value SearchStrategy.SUPERCLASS)

This looks like a bug to me. The search strategy argument should be passed into the call to MergedAnnotations.from.

@wilkinsona wilkinsona added the type: bug A general bug label Apr 11, 2022
@wilkinsona wilkinsona added this to the 2.5.x milestone Apr 11, 2022
@chanhyeong
Copy link
Contributor

@wilkinsona
I've checked that MethodValidationExcludeFilterTests returns success, regardless of whether it is passed or not.
Would you mind If I modified this code?

@wilkinsona
Copy link
Member Author

wilkinsona commented Apr 12, 2022

Thanks for the offer, @chanhyeong. The tests passing even when the code is changed is actually a bad thing. It tells me that the tests need to be changed too.

If you’d like to work on an issue where we provide some guidance, please watch for one labelled as ideal for contribution or, if you haven’t contributed before, first-timers only.

@wilkinsona wilkinsona self-assigned this Apr 14, 2022
@pruidong
Copy link
Contributor

pruidong commented Apr 15, 2022

@wilkinsona I think I can submit a fix for this.

@wilkinsona
Copy link
Member Author

Thanks for the offer. I have a fix and a test ready to push once I’m back in the office.

@pruidong
Copy link
Contributor

Thanks for the offer. I have a fix and a test ready to push once I’m back in the office.

Ok, hope to have the opportunity to provide PR to Spring Boot in the future.

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
3 participants