spring-boot icon indicating copy to clipboard operation
spring-boot copied to clipboard

Published 20 hours ago • verified publisher icon spring-projects

ImportsContextCustomizer supports AliasFor

Open lmartelli opened this issue 1 year ago • 3 comments

Fixes https://github.com/spring-projects/spring-boot/issues/34854

Use MergedAnnotations in order to support @AliasFor.

I used plain Predicate instead of AnnotationFilter because it is much simpler and generic.

lmartelli avatar Apr 08 '23 21:04 lmartelli

Would you like to rework your proposal so that it only makes the changes that are required to fix the problem and nothing more?

Sure, I'll do that.

lmartelli avatar Apr 11 '23 13:04 lmartelli

Would you like to rework your proposal so that it only makes the changes that are required to fix the problem and nothing more?

Sure, I'll do that.

@wilkinsona But note that we still have to remove ImportsContextCustomizer.AnnotationFilter and use the one from org.springframework.core.annotation.AnnotationFilter instead since that's what org.springframework.core.annotation.MergedAnnotations.Search#withAnnotationFilter() wants.

lmartelli avatar Apr 14 '23 22:04 lmartelli

@wilkinsona is the new version OK ?

lmartelli avatar Apr 23 '23 09:04 lmartelli

This still looks quite a large change to me and I am not sure the use of stream and reduce is something we want. The MergedAnnotations API you've used doesn't exist in 2.7.x and we need to be able to rebase the change there. Can you please limit your change to support aliases and only that? We can refine the code further as a separate step.

snicoll avatar May 10 '23 14:05 snicoll

@lmartelli do you want to review your contribution based on the suggestion above? If no, no problem and we can close this and reopen the related issue.

snicoll avatar Jun 12 '23 15:06 snicoll

@lmartelli do you want to review your contribution based on the suggestion above? If no, no problem and we can close this and reopen the related issue.

Yes, I will do that. I just need to find a little time.

lmartelli avatar Jun 14 '23 14:06 lmartelli

Thanks very much @lmartelli, I've taken the tests from this commit and merged them into 2.7 along with a minimal fix. I've also taken the changes you made to ImportsContextCustomizer and applied them with a little polish to main (see #36211).

Thanks again!

philwebb avatar Jul 04 '23 13:07 philwebb