You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@cxf.apache.org by Glen Mazza <gl...@verizon.net> on 2007/07/25 17:13:08 UTC

Re: svn commit: r559358 - in /incubator/cxf/trunk: api/src/main/java/org/apache/cxf/interceptor/ rt/core/src/main/java/org/apache/cxf/interceptor/ rt/frontend/jaxws/src/test/java/org/apache/cxf/jaxws/service/ rt/frontend/simple/src/main/java/org/apache/cxf...

Am Mittwoch, den 25.07.2007, 08:48 +0000 schrieb ffang@apache.org:

> Author: ffang
> Date: Wed Jul 25 01:48:45 2007
> New Revision: 559358
> 
> URL: http://svn.apache.org/viewvc?view=rev&rev=559358
> Log:
> [CXF-803] apply patch provided by Jeff Yu
> 
> Added:
> incubator/cxf/trunk/rt/core/src/main/java/org/apache/cxf/interceptor/AnnotationInterceptors.java
> URL: http://svn.apache.org/viewvc/incubator/cxf/trunk/rt/core/src/main/java/org/apache/cxf/interceptor/AnnotationInterceptors.java?view=auto&rev=559358
> ==============================================================================
> --- incubator/cxf/trunk/rt/core/src/main/java/org/apache/cxf/interceptor/AnnotationInterceptors.java (added)
> +++ incubator/cxf/trunk/rt/core/src/main/java/org/apache/cxf/interceptor/AnnotationInterceptors.java Wed Jul 25 01:48:45 2007
> @@ -0,0 +1,127 @@
> +/**
> +
> +package org.apache.cxf.interceptor;
> +
> +import java.lang.annotation.Annotation;
> +import java.util.ArrayList;
> +import java.util.List;
> +import java.util.ResourceBundle;
> +
> +import javax.jws.WebService;
> +
> +import org.apache.cxf.common.classloader.ClassLoaderUtils;
> +import org.apache.cxf.common.i18n.BundleUtils;
> +import org.apache.cxf.common.i18n.Message;
> +import org.apache.cxf.common.util.StringUtils;
> +import org.apache.cxf.endpoint.EndpointException;
> +
> +public class AnnotationInterceptors {
> +    
> +    private static final ResourceBundle BUNDLE = BundleUtils.getBundle(AnnotationInterceptors.class);
> +    
> +    private Class<?> clazz;
> +    
> +    public AnnotationInterceptors(Class<?> clz) {
> +        clazz = clz;
> +    }
> +    
> +    public List<Interceptor> getInFaultInterceptors() throws EndpointException {
> +        return getInterceptors(InFaultInterceptors.class);
> +    }
> +    
> +    @SuppressWarnings (value = "unchecked")
> +    private List<Interceptor> getInterceptors(Class clz) throws EndpointException {
> +        Annotation  annotation = clazz.getAnnotation(clz);
> +        if (annotation != null) {
> +            return initializeInterceptors(getInterceptorNames(annotation));
> +        } else {

Here, annotation = null means there are no annotations of "clz"
interceptor type tied to the "clazz".  Standard, easy to understand.


> +            WebService ws = clazz.getAnnotation(WebService.class);
> +            if (ws != null && !StringUtils.isEmpty(ws.endpointInterface())) {
> +                String seiClassName = ws.endpointInterface().trim();
> +                Class seiClass = null;
> +                try {
> +                    seiClass = ClassLoaderUtils.loadClass(seiClassName, this.getClass());
> +                } catch (ClassNotFoundException e) {
> +                    throw new RuntimeException("couldnt find class :" + seiClass, e);
> +                }
> +                annotation = seiClass.getAnnotation(clz);
> +                if (annotation != null) {
> +                    return initializeInterceptors(getInterceptorNames(annotation));
> +                }
> +            }           
> +        }
> +        return new ArrayList<Interceptor>();
> +    }


Here, we aren't using "null" but an empty list to signify this.
Is this new list going to be used anywhere, if it is empty (i.e., will
items be further added to this list downstream?)  If not, I think it
would be faster and more robust to just return "null" in this case
instead of creating empty items.  (This would also need to be changed in
initializeInterceptors()--just return a null instead of an empty list.)



> Modified: incubator/cxf/trunk/rt/frontend/simple/src/main/java/org/apache/cxf/frontend/ServerFactoryBean.java
> URL: http://svn.apache.org/viewvc/incubator/cxf/trunk/rt/frontend/simple/src/main/java/org/apache/cxf/frontend/ServerFactoryBean.java?view=diff&rev=559358&r1=559357&r2=559358
> ==============================================================================
> --- incubator/cxf/trunk/rt/frontend/simple/src/main/java/org/apache/cxf/frontend/ServerFactoryBean.java (original)
> +++ incubator/cxf/trunk/rt/frontend/simple/src/main/java/org/apache/cxf/frontend/ServerFactoryBean.java Wed Jul 25 01:48:45 2007

> +    
> +    protected boolean initializeAnnotationInterceptors(Endpoint ep, Class<?> clazz) throws EndpointException {
> +        boolean hasAnnotation = false;
> +        AnnotationInterceptors provider = new AnnotationInterceptors(clazz);
> +        if (provider.getInFaultInterceptors().size() > 0) {
> +            ep.getInFaultInterceptors().addAll(provider.getInFaultInterceptors());
> +            hasAnnotation = true;
> +        }
> +        if (provider.getInInterceptors().size() > 0) {
> +
> ep.getInInterceptors().addAll(provider.getInInterceptors());
> +            hasAnnotation = true;
> +        }
> +        if (provider.getOutFaultInterceptors().size() > 0) {
> +
> ep.getOutFaultInterceptors().addAll(provider.getOutFaultInterceptors());
> +            hasAnnotation = true;
> +        }
> +        if (provider.getOutInterceptors().size() > 0) {
> +
> ep.getOutInterceptors().addAll(provider.getOutInterceptors());
> +            hasAnnotation = true;
> +        }
> +        return hasAnnotation;
>      } 

With this change, the ifs above would simplify to:  
if (provider.getXXXXInterceptors()) { }

Regards,
Glen



Re: svn commit: r559358 - in /incubator/cxf/trunk: api/src/main/java/org/apache/cxf/interceptor/ rt/core/src/main/java/org/apache/cxf/interceptor/ rt/frontend/jaxws/src/test/java/org/apache/cxf/jaxws/service/ rt/frontend/simple/src/main/java/org/apache/cxf...

Posted by "Jeff.Yu" <je...@iona.com>.
Hi, Glen

Thanks for your comments, I will update it accordingly in the next 
patch, since I will add another annotation: @Feature.

Thanks
Jeff Yu


Glen Mazza wrote:
> Am Mittwoch, den 25.07.2007, 08:48 +0000 schrieb ffang@apache.org:
>
>   
>> Author: ffang
>> Date: Wed Jul 25 01:48:45 2007
>> New Revision: 559358
>>
>> URL: http://svn.apache.org/viewvc?view=rev&rev=559358
>> Log:
>> [CXF-803] apply patch provided by Jeff Yu
>>
>> Added:
>> incubator/cxf/trunk/rt/core/src/main/java/org/apache/cxf/interceptor/AnnotationInterceptors.java
>> URL: http://svn.apache.org/viewvc/incubator/cxf/trunk/rt/core/src/main/java/org/apache/cxf/interceptor/AnnotationInterceptors.java?view=auto&rev=559358
>> ==============================================================================
>> --- incubator/cxf/trunk/rt/core/src/main/java/org/apache/cxf/interceptor/AnnotationInterceptors.java (added)
>> +++ incubator/cxf/trunk/rt/core/src/main/java/org/apache/cxf/interceptor/AnnotationInterceptors.java Wed Jul 25 01:48:45 2007
>> @@ -0,0 +1,127 @@
>> +/**
>> +
>> +package org.apache.cxf.interceptor;
>> +
>> +import java.lang.annotation.Annotation;
>> +import java.util.ArrayList;
>> +import java.util.List;
>> +import java.util.ResourceBundle;
>> +
>> +import javax.jws.WebService;
>> +
>> +import org.apache.cxf.common.classloader.ClassLoaderUtils;
>> +import org.apache.cxf.common.i18n.BundleUtils;
>> +import org.apache.cxf.common.i18n.Message;
>> +import org.apache.cxf.common.util.StringUtils;
>> +import org.apache.cxf.endpoint.EndpointException;
>> +
>> +public class AnnotationInterceptors {
>> +    
>> +    private static final ResourceBundle BUNDLE = BundleUtils.getBundle(AnnotationInterceptors.class);
>> +    
>> +    private Class<?> clazz;
>> +    
>> +    public AnnotationInterceptors(Class<?> clz) {
>> +        clazz = clz;
>> +    }
>> +    
>> +    public List<Interceptor> getInFaultInterceptors() throws EndpointException {
>> +        return getInterceptors(InFaultInterceptors.class);
>> +    }
>> +    
>> +    @SuppressWarnings (value = "unchecked")
>> +    private List<Interceptor> getInterceptors(Class clz) throws EndpointException {
>> +        Annotation  annotation = clazz.getAnnotation(clz);
>> +        if (annotation != null) {
>> +            return initializeInterceptors(getInterceptorNames(annotation));
>> +        } else {
>>     
>
> Here, annotation = null means there are no annotations of "clz"
> interceptor type tied to the "clazz".  Standard, easy to understand.
>
>
>   
>> +            WebService ws = clazz.getAnnotation(WebService.class);
>> +            if (ws != null && !StringUtils.isEmpty(ws.endpointInterface())) {
>> +                String seiClassName = ws.endpointInterface().trim();
>> +                Class seiClass = null;
>> +                try {
>> +                    seiClass = ClassLoaderUtils.loadClass(seiClassName, this.getClass());
>> +                } catch (ClassNotFoundException e) {
>> +                    throw new RuntimeException("couldnt find class :" + seiClass, e);
>> +                }
>> +                annotation = seiClass.getAnnotation(clz);
>> +                if (annotation != null) {
>> +                    return initializeInterceptors(getInterceptorNames(annotation));
>> +                }
>> +            }           
>> +        }
>> +        return new ArrayList<Interceptor>();
>> +    }
>>     
>
>
> Here, we aren't using "null" but an empty list to signify this.
> Is this new list going to be used anywhere, if it is empty (i.e., will
> items be further added to this list downstream?)  If not, I think it
> would be faster and more robust to just return "null" in this case
> instead of creating empty items.  (This would also need to be changed in
> initializeInterceptors()--just return a null instead of an empty list.)
>
>
>
>   
>> Modified: incubator/cxf/trunk/rt/frontend/simple/src/main/java/org/apache/cxf/frontend/ServerFactoryBean.java
>> URL: http://svn.apache.org/viewvc/incubator/cxf/trunk/rt/frontend/simple/src/main/java/org/apache/cxf/frontend/ServerFactoryBean.java?view=diff&rev=559358&r1=559357&r2=559358
>> ==============================================================================
>> --- incubator/cxf/trunk/rt/frontend/simple/src/main/java/org/apache/cxf/frontend/ServerFactoryBean.java (original)
>> +++ incubator/cxf/trunk/rt/frontend/simple/src/main/java/org/apache/cxf/frontend/ServerFactoryBean.java Wed Jul 25 01:48:45 2007
>>     
>
>   
>> +    
>> +    protected boolean initializeAnnotationInterceptors(Endpoint ep, Class<?> clazz) throws EndpointException {
>> +        boolean hasAnnotation = false;
>> +        AnnotationInterceptors provider = new AnnotationInterceptors(clazz);
>> +        if (provider.getInFaultInterceptors().size() > 0) {
>> +            ep.getInFaultInterceptors().addAll(provider.getInFaultInterceptors());
>> +            hasAnnotation = true;
>> +        }
>> +        if (provider.getInInterceptors().size() > 0) {
>> +
>> ep.getInInterceptors().addAll(provider.getInInterceptors());
>> +            hasAnnotation = true;
>> +        }
>> +        if (provider.getOutFaultInterceptors().size() > 0) {
>> +
>> ep.getOutFaultInterceptors().addAll(provider.getOutFaultInterceptors());
>> +            hasAnnotation = true;
>> +        }
>> +        if (provider.getOutInterceptors().size() > 0) {
>> +
>> ep.getOutInterceptors().addAll(provider.getOutInterceptors());
>> +            hasAnnotation = true;
>> +        }
>> +        return hasAnnotation;
>>      } 
>>     
>
> With this change, the ifs above would simplify to:  
> if (provider.getXXXXInterceptors()) { }
>
> Regards,
> Glen
>
>
>