Return-Path: X-Original-To: apmail-cxf-commits-archive@www.apache.org Delivered-To: apmail-cxf-commits-archive@www.apache.org Received: from mail.apache.org (hermes.apache.org [140.211.11.3]) by minotaur.apache.org (Postfix) with SMTP id 6434AD7E4 for ; Mon, 17 Sep 2012 12:12:52 +0000 (UTC) Received: (qmail 79101 invoked by uid 500); 17 Sep 2012 12:12:52 -0000 Delivered-To: apmail-cxf-commits-archive@cxf.apache.org Received: (qmail 78884 invoked by uid 500); 17 Sep 2012 12:12:46 -0000 Mailing-List: contact commits-help@cxf.apache.org; run by ezmlm Precedence: bulk List-Help: List-Unsubscribe: List-Post: List-Id: Reply-To: dev@cxf.apache.org Delivered-To: mailing list commits@cxf.apache.org Received: (qmail 78854 invoked by uid 99); 17 Sep 2012 12:12:45 -0000 Received: from athena.apache.org (HELO athena.apache.org) (140.211.11.136) by apache.org (qpsmtpd/0.29) with ESMTP; Mon, 17 Sep 2012 12:12:45 +0000 X-ASF-Spam-Status: No, hits=-2000.0 required=5.0 tests=ALL_TRUSTED X-Spam-Check-By: apache.org Received: from [140.211.11.4] (HELO eris.apache.org) (140.211.11.4) by apache.org (qpsmtpd/0.29) with ESMTP; Mon, 17 Sep 2012 12:12:43 +0000 Received: from eris.apache.org (localhost [127.0.0.1]) by eris.apache.org (Postfix) with ESMTP id 7553323889BF; Mon, 17 Sep 2012 12:12:00 +0000 (UTC) Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit 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 -0000 To: commits@cxf.apache.org From: sergeyb@apache.org X-Mailer: svnmailer-1.0.8-patched Message-Id: <20120917121200.7553323889BF@eris.apache.org> X-Virus-Checked: Checked by ClamAV on apache.org 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> handlers = ProviderFactory.getInstance(message).getResponseHandlers(); for (ProviderInfo 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>(1); // ContainerRequestFilter & ContainerResponseFilter are introduced in JAX-RS 2.0 - private List> globalContainerRequestFilters = + private List> preMatchContainerRequestFilters = new ArrayList>(1); - private List> globalPreContainerRequestFilters = - new ArrayList>(1); - private Map> boundContainerRequestFilters = - new LinkedHashMap>(); - private List> globalContainerResponseFilters = - new ArrayList>(1); - private List> globalPreContainerResponseFilters = - new ArrayList>(1); - private Map> boundContainerResponseFilters = - new LinkedHashMap>(); + private Map> postMatchContainerRequestFilters = + new LinkedHashMap>(); + private Map> postMatchContainerResponseFilters = + new LinkedHashMap>(); // 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> getPreMatchContainerRequestFilters() { - return Collections.unmodifiableList(globalPreContainerRequestFilters); + return Collections.unmodifiableList(preMatchContainerRequestFilters); } public List> getPostMatchContainerRequestFilters(List names) { - return getPostMatchContainerFilters(globalContainerRequestFilters, - boundContainerRequestFilters, + return getPostMatchContainerFilters(postMatchContainerRequestFilters, names); } - public List> getPreMatchContainerResponseFilters() { - return Collections.unmodifiableList(globalPreContainerResponseFilters); - } - - public List> getPostMatchContainerResponseFilters(List names) { - return getPostMatchContainerFilters(globalContainerResponseFilters, - boundContainerResponseFilters, + public List> getContainerResponseFilters(List names) { + return getPostMatchContainerFilters(postMatchContainerResponseFilters, names); } @@ -502,24 +492,23 @@ public final class ProviderFactory { public List> getClientResponseFilters() { return Collections.unmodifiableList(clientResponseFilters); } - - private static List> getPostMatchContainerFilters(List> globalFilters, - Map> boundFilters, - List names) { + + //TODO: Also sort based on BindingPriority + private static List> getPostMatchContainerFilters(Map> boundFilters, + List names) { - if (globalFilters.isEmpty() && boundFilters.isEmpty()) { + if (boundFilters.isEmpty()) { return Collections.emptyList(); } + boolean namesEmpty = names == null || names.isEmpty(); List> list = new LinkedList>(); - list.addAll(globalFilters); - - if (names != null) { - for (String name : names) { - ProviderInfo filter = boundFilters.get(name); - if (filter != null) { - list.add(filter); - } + // TODO: Replace with a plain array + for (Map.Entry> 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> postMatchRequestFilters = + new LinkedList>(); + List> postMatchResponseFilters = + new LinkedList>(); + 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)o, bus)); + addContainerFilter(postMatchRequestFilters, + new ProviderInfo((ContainerRequestFilter)o, bus), + preMatchContainerRequestFilters); } if (ContainerResponseFilter.class.isAssignableFrom(oClass)) { - addContainerResponseFilter( - new ProviderInfo((ContainerResponseFilter)o, bus)); + addContainerFilter(postMatchResponseFilters, + new ProviderInfo((ContainerResponseFilter)o, bus), + null); } if (ClientRequestFilter.class.isAssignableFrom(oClass)) { @@ -651,48 +647,50 @@ public final class ProviderFactory { paramHandlers.add(new ProviderInfo>((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 p) { - addContainerFilter(p, boundContainerRequestFilters, - globalPreContainerRequestFilters, globalContainerRequestFilters); - } - - private void addContainerResponseFilter(ProviderInfo p) { - addContainerFilter(p, boundContainerResponseFilters, - globalPreContainerResponseFilters, globalContainerResponseFilters); + private static void mapContainerFilters(Map> map, + List> postMatchFilters) { + + Collections.sort(postMatchFilters, new PostMatchFilterComparator(true)); + for (ProviderInfo p : postMatchFilters) { + List 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 void addContainerFilter(ProviderInfo p, - Map> boundFilters, - List> globalPreFilters, - List> globalPostFilters) { + private static void addContainerFilter(List> postMatchFilters, + ProviderInfo p, + List> preMatchFilters) { T filter = p.getProvider(); - Annotation[] annotations = filter.getClass().getAnnotations(); - List 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> { + + 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 implements ContextResolver { private List> candidates; public ContextResolverProxy(List> 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> containerFilters = preMatch - ? pf.getPreMatchContainerResponseFilters() - : pf.getPostMatchContainerResponseFilters(ori == null ? null : ori.getNameBindings()); + OperationResourceInfo ori) { + List> 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();