Skip to content
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

Introduce reverse methods in AOP MethodMatchers and ClassFilters #26725

Closed
wants to merge 6 commits into from

Conversation

Jinhui-Z
Copy link
Contributor

Add reverse methed in MethodMatchers and ClassFilters.

Add support for MethodMatchers and ClassFilters to reverse the given MethodMatcher and ClassFilter.
It is convenient in some complex conditions,Or is there another way to do it?Thanks.

@spring-projects-issues spring-projects-issues added the status: waiting-for-triage An issue we've not yet triaged or decided on label Mar 25, 2021
Copy link

@cuspymd cuspymd left a comment

Choose a reason for hiding this comment

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

Isn't it simpler to do "Logical NOT" on the results of the matches() of the original filter? Could you give me an example of how these classes can be useful?

@Jinhui-Z
Copy link
Contributor Author

Jinhui-Z commented Mar 25, 2021

@cuspymd Thanks for the reply.
Some complex combinations are handy, for example in the SpringCloud, if I need to develop a spring-boot-starter project that collects logs for all requests by default and provides @logIgnore to ignore certain requests. For Feign, I could write the Advisor like this:

public class Example {

    /**
     * this FeignClient require logging
     */
    @FeignClient("svr-name")
    public interface Client extends Api {
    }


    /**
     * this FeignClient does not require logging
     */
    @LogIgnore
    @FeignClient("svr-name")
    public interface IgnoreClient extends Api {
    }


    /**
     * in other Jar
     */
    interface Api {
        /**
         * this method does not require logging
         */
        @LogIgnore
        @RequestMapping("/ignoreApi")
        void ignoreApi();

        /**
         * this method require logging
         */
        @RequestMapping("/api")
        void api();
    }


    /**
     * Annotated classes or methods do not need to be logged
     */
    @Target({ElementType.METHOD, ElementType.TYPE})
    @Retention(RetentionPolicy.RUNTIME)
    @Inherited
    @Documented
    @interface LogIgnore {
    }


    /**
     * The Advisor for Feign
     */
    public class FeignAdvisor extends AbstractPointcutAdvisor {

        @Override
        public Pointcut getPointcut() {
            return new Pointcut() {
                @Override
                public ClassFilter getClassFilter() {
                    //This class mast contain @FeignClient
                    ClassFilter cf1 = new AnnotationClassFilter(FeignClient.class, true);
                    //This class cannot contain @LogIgnore
                    ClassFilter cf2 = ClassFilters.reversion(new AnnotationClassFilter(LogIgnore.class, true));

                    //maybe more ClassFilter

                    return ClassFilters.intersection(cf1, cf2);
                }

                @Override
                public MethodMatcher getMethodMatcher() {
                    //This method mast contain @RequestMapping
                    MethodMatcher cf1 = new AnnotationMethodMatcher(RequestMapping.class, true);
                    //This method cannot contain @LogIgnore
                    MethodMatcher cf2 = MethodMatchers.reversion(new AnnotationMethodMatcher(LogIgnore.class, true));

                    //maybe more MethodMatcher

                    return MethodMatchers.intersection(cf1, cf2);
                }
            };
        }

        @Override
        public Advice getAdvice() {
            //do something
            return (MethodInterceptor) (MethodInvocation invocation) -> invocation.proceed();
        }
    }
}

@cuspymd
Copy link

cuspymd commented Mar 26, 2021

Some complex combinations are handy, for example in the SpringCloud, if I need to develop a spring-boot-starter project that collects logs for all requests by default and provides @logIgnore to ignore certain requests. For Feign, I could write the Advisor like this:

Thanks for the detailed explanation. It looks very useful.

@rstoyanchev rstoyanchev added the in: core Issues in core modules (aop, beans, core, context, expression) label Nov 11, 2021
@Jinhui-Z
Copy link
Contributor Author

@rstoyanchev Could you please have a look into this?

@Pulkit-Garg15
Copy link

Hi, I am a beginner with Open source contributions. This would be my first contribution, can anyone please guide me through it?

@snicoll
Copy link
Member

snicoll commented Jul 21, 2022

@Pulkit-Garg15 thanks for the offer but this is already a PR (see the "Files Changed" tab above) so this isn't open for contribution.

@snicoll
Copy link
Member

snicoll commented Aug 26, 2023

I think a negate on ClassFilter can be added as it is a functional interface. I am wondering if we should do the same for MethodFilter though, and how important the equals/hashCode bit is.

@sbrannen
Copy link
Member

sbrannen commented Aug 27, 2023

I think a negate on ClassFilter can be added as it is a functional interface.

I agree. It's analogous to java.util.function.Predicate.negate() in that sense.

Along the same lines, I could see it being useful to have something like the static java.util.function.Predicate.not(Predicate<? super T>) companion method.

I am wondering if we should do the same for MethodFilter though,

I imagine that could be handy in MethodMatcher, too.

and how important the equals/hashCode bit is.

The filters are used to differentiate pointcuts -- for example, in org.springframework.aop.support.ComposablePointcut.equals(Object). So, unless I'm mistaken, proper equals and hashCode implementations will be required (for example, #15899).

@sbrannen sbrannen changed the title Add reverse methed in MethodMatchers and ClassFilters Introduce reverse methed in AOP MethodMatchers and ClassFilters Aug 27, 2023
@sbrannen sbrannen added the type: enhancement A general enhancement label Aug 27, 2023
@sbrannen sbrannen changed the title Introduce reverse methed in AOP MethodMatchers and ClassFilters Introduce reverse metheds in AOP MethodMatchers and ClassFilters Aug 27, 2023
@snicoll snicoll self-assigned this Aug 28, 2023
@snicoll
Copy link
Member

snicoll commented Aug 28, 2023

Such a shame that we can't do the negate idea as a default method. We have several components in the framework that implements both interface so we're kinda stuck there :/

@snicoll
Copy link
Member

snicoll commented Aug 28, 2023

@Jinhui-Z thanks very much for making your first contribution to Spring Framework.

@snicoll snicoll removed the status: waiting-for-triage An issue we've not yet triaged or decided on label Sep 17, 2023
@izeye
Copy link
Contributor

izeye commented Oct 3, 2023

This seems to have been shipped with 6.1.0-M5 but doesn't have a milestone. There's also a typo in "metheds" in the title.

@snicoll snicoll changed the title Introduce reverse metheds in AOP MethodMatchers and ClassFilters Introduce reverse methods in AOP MethodMatchers and ClassFilters Oct 3, 2023
@snicoll snicoll added this to the 6.1.0-M5 milestone Oct 3, 2023
@snicoll
Copy link
Member

snicoll commented Oct 3, 2023

Good catch @izeye. I've fixed the title and added the missing entry manually to the release notes.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
in: core Issues in core modules (aop, beans, core, context, expression) type: enhancement A general enhancement
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

8 participants