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();
