You are viewing a plain text version of this content. The canonical link for it is here.

Posted to commits@tapestry.apache.org by "Howard M. Lewis Ship (JIRA)" <ji...@apache.org> on 2011/04/20 02:16:06 UTC

[jira] [Created] (TAP5-1510) The @Advise annotation limits advise to just a specific interface type

The @Advise annotation limits advise to just a specific interface type
----------------------------------------------------------------------

                 Key: TAP5-1510
                 URL: https://issues.apache.org/jira/browse/TAP5-1510
             Project: Tapestry 5
          Issue Type: Bug
          Components: tapestry-ioc
    Affects Versions: 5.2.5, 5.3.0
            Reporter: Howard M. Lewis Ship


@Advise requires that you specify a service interface (there's no default value).  This is much more limiting than the advise method naming prefix, which will match all services (subject to the use of @Match), without regard to service interface.

Further, inside ModuleImpl:


    private boolean markerMatched(ServiceDef serviceDef, Markable markable)
    {
        if (!serviceDef.getServiceInterface().equals(markable.getServiceInterface()))
            return false;;


here, the Markable is the AdvisorDef2 instance generated from the @Advise annotation.  This is an exact comparison; I believe this should be:

  if (! markable.getServiceInterface().isAssignableFrom(serviceDef.getServiceInterface())) return false;

That, combined with a default of Object.class for @Advisor.serviceInterface would do the trick ... the @Advise.serviceInterface acts as an umbrella over any services' service interface.

..... ok, did more research and more stepping with the debugger.  The above should be fixed, but it's only the second case of matching, the primary match should be based on the @Match annotation ... but that's broken too:


Frrom DefaultModuleDefImpl:

    private <T extends Annotation> String[] extractPatterns(T annotation, String id, Method method)
    {
        if(annotation != null)
            return new String[]{};
       
        Match match = method.getAnnotation(Match.class);

        if (match == null)
            return new String[]
            { id };

        return match.value();
    }

Here, the annotation is the @Advise annotation; I don't get why it returns empty string array; we should still see if there's a @Match annotation. Looking at the code, I can't see any reason why we would return that empty string array, the presense of the @Advise annotation (or for a decorator method, the @Decorate annotation) has no purpose I can figure out.


In my situation, my advise method was not invoked because
a) Primary check (by service id) failed, because the @Match annotation was ignored
b) Secondary check (by service type and marker annotations) failed, because of inexact match on service interface

So, the end result is the @Advise is only useful to advise a specific service interface, which is the opposite of what method advice is about ... it's supposed to match against a swath of services, adapting the advise to whatever methods are present in those services.

--
This message is automatically generated by JIRA.
For more information on JIRA, see: http://www.atlassian.com/software/jira

[jira] [Commented] (TAP5-1510) The @Advise annotation limits advice to just a specific interface type

Posted by "Hudson (JIRA)" <ji...@apache.org>.

    [ https://issues.apache.org/jira/browse/TAP5-1510?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13041280#comment-13041280 ] 

Hudson commented on TAP5-1510:
------------------------------

Integrated in tapestry-trunk-freestyle #353 (See [https://builds.apache.org/hudson/job/tapestry-trunk-freestyle/353/])
    TAP5-1510: Added tests for decorators

drobiazko : http://svn.apache.org/viewcvs.cgi/?root=Apache-SVN&view=rev&rev=1129336
Files : 
* /tapestry/tapestry5/trunk/tapestry-ioc/src/test/java/org/apache/tapestry5/ioc/internal/DecorateByMarkerModule2.java
* /tapestry/tapestry5/trunk/tapestry-ioc/src/test/java/org/apache/tapestry5/ioc/IntegrationTest.java


> The @Advise annotation limits advice to just a specific interface type
> ----------------------------------------------------------------------
>
>                 Key: TAP5-1510
>                 URL: https://issues.apache.org/jira/browse/TAP5-1510
>             Project: Tapestry 5
>          Issue Type: Bug
>          Components: tapestry-ioc
>    Affects Versions: 5.3.0, 5.2.5
>            Reporter: Howard M. Lewis Ship
>            Assignee: Igor Drobiazko
>             Fix For: 5.3.0
>
>
> @Advise requires that you specify a service interface (there's no default value).  This is much more limiting than the advise method naming prefix, which will match all services (subject to the use of @Match), without regard to service interface.
> Further, inside ModuleImpl:
>     private boolean markerMatched(ServiceDef serviceDef, Markable markable)
>     {
>         if (!serviceDef.getServiceInterface().equals(markable.getServiceInterface()))
>             return false;;
> here, the Markable is the AdvisorDef2 instance generated from the @Advise annotation.  This is an exact comparison; I believe this should be:
>   if (! markable.getServiceInterface().isAssignableFrom(serviceDef.getServiceInterface())) return false;
> That, combined with a default of Object.class for @Advisor.serviceInterface would do the trick ... the @Advise.serviceInterface acts as an umbrella over any services' service interface.
> ..... ok, did more research and more stepping with the debugger.  The above should be fixed, but it's only the second case of matching, the primary match should be based on the @Match annotation ... but that's broken too:
> Frrom DefaultModuleDefImpl:
>     private <T extends Annotation> String[] extractPatterns(T annotation, String id, Method method)
>     {
>         if(annotation != null)
>             return new String[]{};
>        
>         Match match = method.getAnnotation(Match.class);
>         if (match == null)
>             return new String[]
>             { id };
>         return match.value();
>     }
> Here, the annotation is the @Advise annotation; I don't get why it returns empty string array; we should still see if there's a @Match annotation. Looking at the code, I can't see any reason why we would return that empty string array, the presense of the @Advise annotation (or for a decorator method, the @Decorate annotation) has no purpose I can figure out.
> In my situation, my advise method was not invoked because
> a) Primary check (by service id) failed, because the @Match annotation was ignored
> b) Secondary check (by service type and marker annotations) failed, because of inexact match on service interface
> So, the end result is the @Advise is only useful to advise a specific service interface, which is the opposite of what method advice is about ... it's supposed to match against a swath of services, adapting the advise to whatever methods are present in those services.

--
This message is automatically generated by JIRA.
For more information on JIRA, see: http://www.atlassian.com/software/jira