cxf-commits mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From serg...@apache.org
Subject svn commit: r1386577 - in /cxf/trunk: rt/frontend/jaxrs/src/main/java/org/apache/cxf/jaxrs/interceptor/ rt/frontend/jaxrs/src/main/java/org/apache/cxf/jaxrs/provider/ rt/frontend/jaxrs/src/main/java/org/apache/cxf/jaxrs/utils/ systests/jaxrs/src/test/j...
Date Mon, 17 Sep 2012 12:12:00 GMT
Author: sergeyb
Date: Mon Sep 17 12:11:59 2012
New Revision: 1386577

URL: http://svn.apache.org/viewvc?rev=1386577&view=rev
Log:
[CXF-4455] Sorting post match filters according to binding priorities

Modified:
    cxf/trunk/rt/frontend/jaxrs/src/main/java/org/apache/cxf/jaxrs/interceptor/JAXRSOutInterceptor.java
    cxf/trunk/rt/frontend/jaxrs/src/main/java/org/apache/cxf/jaxrs/provider/ProviderFactory.java
    cxf/trunk/rt/frontend/jaxrs/src/main/java/org/apache/cxf/jaxrs/utils/AnnotationUtils.java
    cxf/trunk/rt/frontend/jaxrs/src/main/java/org/apache/cxf/jaxrs/utils/JAXRSUtils.java
    cxf/trunk/systests/jaxrs/src/test/java/org/apache/cxf/systest/jaxrs/JAXRS20ClientServerBookTest.java

Modified: cxf/trunk/rt/frontend/jaxrs/src/main/java/org/apache/cxf/jaxrs/interceptor/JAXRSOutInterceptor.java
URL: http://svn.apache.org/viewvc/cxf/trunk/rt/frontend/jaxrs/src/main/java/org/apache/cxf/jaxrs/interceptor/JAXRSOutInterceptor.java?rev=1386577&r1=1386576&r2=1386577&view=diff
==============================================================================
--- cxf/trunk/rt/frontend/jaxrs/src/main/java/org/apache/cxf/jaxrs/interceptor/JAXRSOutInterceptor.java
(original)
+++ cxf/trunk/rt/frontend/jaxrs/src/main/java/org/apache/cxf/jaxrs/interceptor/JAXRSOutInterceptor.java
Mon Sep 17 12:11:59 2012
@@ -131,14 +131,13 @@ public class JAXRSOutInterceptor extends
             .getName());
         
         // Global post-match and name-bound response filters
-        if (ori != null) {
-            JAXRSUtils.runContainerResponseFilters(providerFactory, response, message, ori,
false);
-            Response updatedResponse = message.get(Response.class);
-            if (updatedResponse != null) {
-                response = updatedResponse;
-            }
+        JAXRSUtils.runContainerResponseFilters(providerFactory, response, message, ori);
+        Response updatedResponse = message.get(Response.class);
+        if (updatedResponse != null) {
+            response = updatedResponse;
         }
         
+        
         List<ProviderInfo<ResponseHandler>> handlers = 
             ProviderFactory.getInstance(message).getResponseHandlers();
         for (ProviderInfo<ResponseHandler> rh : handlers) {

Modified: cxf/trunk/rt/frontend/jaxrs/src/main/java/org/apache/cxf/jaxrs/provider/ProviderFactory.java
URL: http://svn.apache.org/viewvc/cxf/trunk/rt/frontend/jaxrs/src/main/java/org/apache/cxf/jaxrs/provider/ProviderFactory.java?rev=1386577&r1=1386576&r2=1386577&view=diff
==============================================================================
--- cxf/trunk/rt/frontend/jaxrs/src/main/java/org/apache/cxf/jaxrs/provider/ProviderFactory.java
(original)
+++ cxf/trunk/rt/frontend/jaxrs/src/main/java/org/apache/cxf/jaxrs/provider/ProviderFactory.java
Mon Sep 17 12:11:59 2012
@@ -36,6 +36,7 @@ import java.util.Map;
 import java.util.Set;
 import java.util.logging.Logger;
 
+import javax.ws.rs.BindingPriority;
 import javax.ws.rs.Produces;
 import javax.ws.rs.WebApplicationException;
 import javax.ws.rs.client.ClientRequestFilter;
@@ -80,6 +81,7 @@ public final class ProviderFactory {
     private static final Logger LOG = LogUtils.getL7dLogger(ProviderFactory.class);
     private static final ProviderFactory SHARED_FACTORY = getInstance();
     
+    private static final String DEFAULT_FILTER_NAME_BINDING = "org.apache.cxf.filter.binding";
     private static final String JAXB_PROVIDER_NAME = "org.apache.cxf.jaxrs.provider.JAXBElementProvider";
     private static final String JSON_PROVIDER_NAME = "org.apache.cxf.jaxrs.provider.json.JSONProvider";
     
@@ -113,18 +115,12 @@ public final class ProviderFactory {
         new ArrayList<ProviderInfo<ResponseHandler>>(1);
     
     // ContainerRequestFilter & ContainerResponseFilter are introduced in JAX-RS 2.0
-    private List<ProviderInfo<ContainerRequestFilter>> globalContainerRequestFilters
= 
+    private List<ProviderInfo<ContainerRequestFilter>> preMatchContainerRequestFilters
= 
         new ArrayList<ProviderInfo<ContainerRequestFilter>>(1);
-    private List<ProviderInfo<ContainerRequestFilter>> globalPreContainerRequestFilters
= 
-        new ArrayList<ProviderInfo<ContainerRequestFilter>>(1);
-    private Map<String, ProviderInfo<ContainerRequestFilter>> boundContainerRequestFilters
= 
-        new LinkedHashMap<String, ProviderInfo<ContainerRequestFilter>>();
-    private List<ProviderInfo<ContainerResponseFilter>> globalContainerResponseFilters
= 
-        new ArrayList<ProviderInfo<ContainerResponseFilter>>(1);
-    private List<ProviderInfo<ContainerResponseFilter>> globalPreContainerResponseFilters
= 
-        new ArrayList<ProviderInfo<ContainerResponseFilter>>(1);
-    private Map<String, ProviderInfo<ContainerResponseFilter>> boundContainerResponseFilters
= 
-        new LinkedHashMap<String, ProviderInfo<ContainerResponseFilter>>();
+    private Map<NameKey, ProviderInfo<ContainerRequestFilter>> postMatchContainerRequestFilters
= 
+        new LinkedHashMap<NameKey, ProviderInfo<ContainerRequestFilter>>();
+    private Map<NameKey, ProviderInfo<ContainerResponseFilter>> postMatchContainerResponseFilters
= 
+        new LinkedHashMap<NameKey, ProviderInfo<ContainerResponseFilter>>();
     
     // ParamConverter and ParamConverterProvider is introduced in JAX-RS 2.0
     // ParameterHandler will have to be deprecated
@@ -476,22 +472,16 @@ public final class ProviderFactory {
     }
     
     public List<ProviderInfo<ContainerRequestFilter>> getPreMatchContainerRequestFilters()
{
-        return Collections.unmodifiableList(globalPreContainerRequestFilters);
+        return Collections.unmodifiableList(preMatchContainerRequestFilters);
     }
     
     public List<ProviderInfo<ContainerRequestFilter>> getPostMatchContainerRequestFilters(List<String>
names) {
-        return getPostMatchContainerFilters(globalContainerRequestFilters, 
-                                            boundContainerRequestFilters, 
+        return getPostMatchContainerFilters(postMatchContainerRequestFilters, 
                                             names);
     }
     
-    public List<ProviderInfo<ContainerResponseFilter>> getPreMatchContainerResponseFilters()
{
-        return Collections.unmodifiableList(globalPreContainerResponseFilters);
-    }
-    
-    public List<ProviderInfo<ContainerResponseFilter>> getPostMatchContainerResponseFilters(List<String>
names) {
-        return getPostMatchContainerFilters(globalContainerResponseFilters, 
-                                            boundContainerResponseFilters, 
+    public List<ProviderInfo<ContainerResponseFilter>> getContainerResponseFilters(List<String>
names) {
+        return getPostMatchContainerFilters(postMatchContainerResponseFilters, 
                                             names);
     }
     
@@ -502,24 +492,23 @@ public final class ProviderFactory {
     public List<ProviderInfo<ClientResponseFilter>> getClientResponseFilters()
{
         return Collections.unmodifiableList(clientResponseFilters);
     }
-    
-    private static <T> List<ProviderInfo<T>> getPostMatchContainerFilters(List<ProviderInfo<T>>
globalFilters,
-                                                                         Map<String, ProviderInfo<T>>
boundFilters,
-                                                                         List<String>
names) {
+
+    //TODO: Also sort based on BindingPriority
+    private static <T> List<ProviderInfo<T>> getPostMatchContainerFilters(Map<NameKey,
ProviderInfo<T>> boundFilters,
+                                                                          List<String>
names) {
         
-        if (globalFilters.isEmpty() && boundFilters.isEmpty()) {
+        if (boundFilters.isEmpty()) {
             return Collections.emptyList();
         }
+        boolean namesEmpty = names == null || names.isEmpty();
         
         List<ProviderInfo<T>> list = new LinkedList<ProviderInfo<T>>();
-        list.addAll(globalFilters);
-
-        if (names != null) {
-            for (String name : names) {
-                ProviderInfo<T> filter = boundFilters.get(name);
-                if (filter != null) {
-                    list.add(filter);
-                }
+        // TODO: Replace with a plain array
+        for (Map.Entry<NameKey, ProviderInfo<T>> entry : boundFilters.entrySet())
{
+            String entryName = entry.getKey().getName();
+            if (!namesEmpty && names.contains(entryName)
+                || entryName.equals(DEFAULT_FILTER_NAME_BINDING)) {
+                list.add(entry.getValue());                    
             }
         }
         return list;
@@ -589,6 +578,11 @@ public final class ProviderFactory {
 //CHECKSTYLE:OFF       
     private void setProviders(Object... providers) {
         
+        List<ProviderInfo<ContainerRequestFilter>> postMatchRequestFilters =

+            new LinkedList<ProviderInfo<ContainerRequestFilter>>();
+        List<ProviderInfo<ContainerResponseFilter>> postMatchResponseFilters
= 
+            new LinkedList<ProviderInfo<ContainerResponseFilter>>();
+        
         for (Object o : providers) {
             if (o == null) {
                 continue;
@@ -620,13 +614,15 @@ public final class ProviderFactory {
             }
             
             if (ContainerRequestFilter.class.isAssignableFrom(oClass)) {
-                addContainerRequestFilter(
-                   new ProviderInfo<ContainerRequestFilter>((ContainerRequestFilter)o,
bus));
+                addContainerFilter(postMatchRequestFilters,
+                   new ProviderInfo<ContainerRequestFilter>((ContainerRequestFilter)o,
bus),
+                   preMatchContainerRequestFilters);
             }
             
             if (ContainerResponseFilter.class.isAssignableFrom(oClass)) {
-                addContainerResponseFilter(
-                   new ProviderInfo<ContainerResponseFilter>((ContainerResponseFilter)o,
bus)); 
+                addContainerFilter(postMatchResponseFilters,
+                   new ProviderInfo<ContainerResponseFilter>((ContainerResponseFilter)o,
bus),
+                   null); 
             }
             
             if (ClientRequestFilter.class.isAssignableFrom(oClass)) {
@@ -651,48 +647,50 @@ public final class ProviderFactory {
                 paramHandlers.add(new ProviderInfo<ParameterHandler<?>>((ParameterHandler<?>)o,
bus)); 
             }
         }
-        //TODO: BindingPriority has to be checked too
         sortReaders();
         sortWriters();
         sortContextResolvers();
         
+        Collections.sort(preMatchContainerRequestFilters, new BindingPriorityComparator(true));
+        mapContainerFilters(postMatchContainerRequestFilters, postMatchRequestFilters);
+        mapContainerFilters(postMatchContainerResponseFilters, postMatchResponseFilters);
+        
         injectContextProxies(messageReaders, messageWriters, contextResolvers, 
             requestHandlers, responseHandlers, exceptionMappers,
-            boundContainerRequestFilters.values(), globalPreContainerRequestFilters, globalContainerRequestFilters,
-            boundContainerResponseFilters.values(), globalPreContainerResponseFilters, globalContainerResponseFilters,
+            postMatchContainerRequestFilters.values(), preMatchContainerRequestFilters,
+            postMatchContainerResponseFilters.values(),
             responseExceptionMappers, clientRequestFilters, clientResponseFilters);
     }
 //CHECKSTYLE:ON
     
-    private void addContainerRequestFilter(ProviderInfo<ContainerRequestFilter> p)
{
-        addContainerFilter(p, boundContainerRequestFilters, 
-                           globalPreContainerRequestFilters, globalContainerRequestFilters);
-    }
-    
-    private void addContainerResponseFilter(ProviderInfo<ContainerResponseFilter> p)
{
-        addContainerFilter(p, boundContainerResponseFilters, 
-                           globalPreContainerResponseFilters, globalContainerResponseFilters);
+    private static <T> void mapContainerFilters(Map<NameKey, ProviderInfo<T>>
map,
+                                                List<ProviderInfo<T>> postMatchFilters)
{
+        
+        Collections.sort(postMatchFilters, new PostMatchFilterComparator(true));
+        for (ProviderInfo<T> p : postMatchFilters) { 
+            List<String> names = AnnotationUtils.getNameBindings(
+                p.getProvider().getClass().getAnnotations());
+            names = names.isEmpty() ? Collections.singletonList(DEFAULT_FILTER_NAME_BINDING)
: names;
+            //TODO:  Would it be faster to have a single NameKey to keep all the names ?
+            for (String name : names) {
+                map.put(new NameKey(name), p);
+            }
+        }
+        
     }
     
-    private static <T> void addContainerFilter(ProviderInfo<T> p,
-                                               Map<String, ProviderInfo<T>> boundFilters,
-                                               List<ProviderInfo<T>> globalPreFilters,
-                                               List<ProviderInfo<T>> globalPostFilters)
{
+    private static <T> void addContainerFilter(List<ProviderInfo<T>> postMatchFilters,
+                                               ProviderInfo<T> p,
+                                               List<ProviderInfo<T>> preMatchFilters)
{
         T filter = p.getProvider();
-        Annotation[] annotations = filter.getClass().getAnnotations();
-        List<String> names = AnnotationUtils.getNameBindings(annotations);
-        if (!names.isEmpty()) {
-            for (String name : names) {
-                boundFilters.put(name, p);
-            }
+        if (preMatchFilters != null
+            && AnnotationUtils.getAnnotation(filter.getClass().getAnnotations(),

+                                             PreMatching.class) != null) {
+            preMatchFilters.add(p);
         } else {
-            boolean isPreMatch = AnnotationUtils.getAnnotation(annotations, PreMatching.class)
!= null;
-            if (isPreMatch) {
-                globalPreFilters.add(p);
-            } else {
-                globalPostFilters.add(p);
-            }
+            postMatchFilters.add(p);
         }
+        
     }
     
     static void injectContextValues(ProviderInfo<?> pi, Message m) {
@@ -981,12 +979,9 @@ public final class ProviderFactory {
         exceptionMappers.clear();
         requestHandlers.clear();
         responseHandlers.clear();
-        globalContainerRequestFilters.clear();
-        globalContainerResponseFilters.clear();
-        boundContainerRequestFilters.clear();
-        boundContainerResponseFilters.clear();
-        globalPreContainerRequestFilters.clear();
-        globalPreContainerResponseFilters.clear();
+        postMatchContainerRequestFilters.clear();
+        postMatchContainerResponseFilters.clear();
+        preMatchContainerRequestFilters.clear();
         paramHandlers.clear();
         responseExceptionMappers.clear();
         clientRequestFilters.clear();
@@ -1108,6 +1103,58 @@ public final class ProviderFactory {
         return getGenericInterfaces(cls.getSuperclass());
     }
     
+    private static class PostMatchFilterComparator extends BindingPriorityComparator {
+        public PostMatchFilterComparator(boolean ascending) {
+            super(ascending);
+        }
+        
+        @Override
+        public int compare(ProviderInfo<?> p1, ProviderInfo<?> p2) {
+            int result = super.compare(p1, p2);
+            if (result == 0) {
+                Integer namesSize1 = 
+                    AnnotationUtils.getNameBindings(p1.getProvider().getClass().getAnnotations()).size();
+                Integer namesSize2 = 
+                    AnnotationUtils.getNameBindings(p2.getProvider().getClass().getAnnotations()).size();
+                
+                // if we have two filters with the same binding priority, 
+                // then put a filter with more name bindings upfront 
+                // (this effectively puts name bound filters before global ones)
+                result = namesSize1.compareTo(namesSize2) * -1;
+            }
+            return result; 
+        }
+    }
+    
+    private static class BindingPriorityComparator extends AbstactBindingPriorityComparator
{
+        public BindingPriorityComparator(boolean ascending) {
+            super(ascending);
+        }
+    }
+    
+    private abstract static class AbstactBindingPriorityComparator implements 
+        Comparator<ProviderInfo<?>> {
+    
+        private boolean ascending; 
+        
+        protected AbstactBindingPriorityComparator(boolean ascending) {
+            this.ascending = ascending; 
+        }
+        
+        public int compare(ProviderInfo<?> p1, ProviderInfo<?> p2) {
+            Integer b1Value = getBindingPriorityValue(p1);
+            Integer b2Value = getBindingPriorityValue(p2);
+            
+            int result = b1Value.compareTo(b2Value);
+            return ascending ? result : result * -1;      
+        }
+        
+        private int getBindingPriorityValue(ProviderInfo<?> p) {
+            BindingPriority b = p.getProvider().getClass().getAnnotation(BindingPriority.class);
+            return b == null ? BindingPriority.USER : b.value();
+        }
+    }
+    
     static class ContextResolverProxy<T> implements ContextResolver<T> {
         private List<ContextResolver<T>> candidates; 
         public ContextResolverProxy(List<ContextResolver<T>> candidates) {
@@ -1127,4 +1174,15 @@ public final class ProviderFactory {
             return candidates;
         }
     }
+    
+    private static class NameKey { 
+        private String name;
+        public NameKey(String name) {
+            this.name = name;
+        }
+        
+        public String getName() {
+            return name;
+        }
+    }
 }

Modified: cxf/trunk/rt/frontend/jaxrs/src/main/java/org/apache/cxf/jaxrs/utils/AnnotationUtils.java
URL: http://svn.apache.org/viewvc/cxf/trunk/rt/frontend/jaxrs/src/main/java/org/apache/cxf/jaxrs/utils/AnnotationUtils.java?rev=1386577&r1=1386576&r2=1386577&view=diff
==============================================================================
--- cxf/trunk/rt/frontend/jaxrs/src/main/java/org/apache/cxf/jaxrs/utils/AnnotationUtils.java
(original)
+++ cxf/trunk/rt/frontend/jaxrs/src/main/java/org/apache/cxf/jaxrs/utils/AnnotationUtils.java
Mon Sep 17 12:11:59 2012
@@ -130,7 +130,7 @@ public final class AnnotationUtils {
         for (Annotation a : targetAnns) {
             NameBinding nb = a.annotationType().getAnnotation(NameBinding.class);
             if (nb != null) {
-                names.add(a.getClass().getName());
+                names.add(a.annotationType().getName());
             }
         }
         return names;

Modified: cxf/trunk/rt/frontend/jaxrs/src/main/java/org/apache/cxf/jaxrs/utils/JAXRSUtils.java
URL: http://svn.apache.org/viewvc/cxf/trunk/rt/frontend/jaxrs/src/main/java/org/apache/cxf/jaxrs/utils/JAXRSUtils.java?rev=1386577&r1=1386576&r2=1386577&view=diff
==============================================================================
--- cxf/trunk/rt/frontend/jaxrs/src/main/java/org/apache/cxf/jaxrs/utils/JAXRSUtils.java (original)
+++ cxf/trunk/rt/frontend/jaxrs/src/main/java/org/apache/cxf/jaxrs/utils/JAXRSUtils.java Mon
Sep 17 12:11:59 2012
@@ -1362,15 +1362,13 @@ public final class JAXRSUtils {
     public static void runContainerResponseFilters(ProviderFactory pf,
                                                    Response r,
                                                    Message m, 
-                                                   OperationResourceInfo ori,
-                                                   boolean preMatch) {
-        List<ProviderInfo<ContainerResponseFilter>> containerFilters = preMatch

-            ? pf.getPreMatchContainerResponseFilters() 
-            : pf.getPostMatchContainerResponseFilters(ori == null ? null : ori.getNameBindings());
+                                                   OperationResourceInfo ori) {
+        List<ProviderInfo<ContainerResponseFilter>> containerFilters =  
+            pf.getContainerResponseFilters(ori == null ? null : ori.getNameBindings());
         if (!containerFilters.isEmpty()) {
             ContainerRequestContext requestContext = 
                 new ContainerRequestContextImpl(m.getExchange().getInMessage(), 
-                                               preMatch,
+                                               false,
                                                true);
             ContainerResponseContext responseContext = 
                 new ContainerResponseContextImpl(r, m, ori);

Modified: cxf/trunk/systests/jaxrs/src/test/java/org/apache/cxf/systest/jaxrs/JAXRS20ClientServerBookTest.java
URL: http://svn.apache.org/viewvc/cxf/trunk/systests/jaxrs/src/test/java/org/apache/cxf/systest/jaxrs/JAXRS20ClientServerBookTest.java?rev=1386577&r1=1386576&r2=1386577&view=diff
==============================================================================
--- cxf/trunk/systests/jaxrs/src/test/java/org/apache/cxf/systest/jaxrs/JAXRS20ClientServerBookTest.java
(original)
+++ cxf/trunk/systests/jaxrs/src/test/java/org/apache/cxf/systest/jaxrs/JAXRS20ClientServerBookTest.java
Mon Sep 17 12:11:59 2012
@@ -62,6 +62,7 @@ public class JAXRS20ClientServerBookTest
         providers.add(new ClientHeaderRequestFilter());
         providers.add(new ClientHeaderResponseFilter());
         WebClient wc = WebClient.create(address, providers);
+        WebClient.getConfig(wc).getHttpConduit().getClient().setReceiveTimeout(1000000L);
         Book book = wc.get(Book.class);
         assertEquals(124L, book.getId());
         Response response = wc.getResponse();



Mime
View raw message