You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@cxf.apache.org by se...@apache.org on 2012/09/17 14:12:00 UTC
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...
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();