Description
ReaderInterceptors and WriterInterceptors are not sortered when a provider comparator is provided.
Please see attached patch for a proposed (probably excessively naive) solution.
Attachments
Attachments
- patch.diff
- 4 kB
- Carlos Sierra Andrés
Issue Links
- links to
Activity
csierra commented on issue #385: CXF-7657 Reader and Writer interceptors are not sorted by the provi…
URL: https://github.com/apache/cxf/pull/385#issuecomment-368851321
hi @andymc12,
I can try but the problem is that the failures are random because the fallback case when the providers come with the same `@Priority` is ```Integer.compare(o1.hashCode(), o2.hashCode())```
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
csierra commented on issue #385: CXF-7657 Reader and Writer interceptors are not sorted by the provi…
URL: https://github.com/apache/cxf/pull/385#issuecomment-371901800
hey @andymc12,
sorry this took so long. I updated the test showing the problem with the ReaderInterceptor and WriterInterceptor and the `Comparator`. If you revert the fix you will see the test failing. There is a small chance, though, that the test passes because the current behaviour is random, so it might happen that the writers fall in the expected order even though they are not being sorted.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
andymc12 closed pull request #385: CXF-7657 Reader and Writer interceptors are not sorted by the provi…
URL: https://github.com/apache/cxf/pull/385
This is a PR merged from a forked repository.
As GitHub hides the original diff on merge, it is displayed below for
the sake of provenance:
As this is a foreign pull request (from a fork), the diff is supplied
below (as it won't show otherwise due to GitHub magic):
diff --git a/rt/frontend/jaxrs/src/main/java/org/apache/cxf/jaxrs/provider/ProviderFactory.java b/rt/frontend/jaxrs/src/main/java/org/apache/cxf/jaxrs/provider/ProviderFactory.java
index a5767a0c165..9f2baeba90b 100644
— a/rt/frontend/jaxrs/src/main/java/org/apache/cxf/jaxrs/provider/ProviderFactory.java
+++ b/rt/frontend/jaxrs/src/main/java/org/apache/cxf/jaxrs/provider/ProviderFactory.java
@@ -1212,12 +1212,24 @@ public T getContext(Class<?> cls) {
private String name;
private Integer priority;
private Class<?> providerCls;
+ private ProviderInfo<?> providerInfo;
+
public NameKey(String name,
int priority,
Class<?> providerCls)
+
+ public NameKey(String name,
+ int priority,
+ Class<?> providerCls,
+ ProviderInfo<?> provider)
public String getName() {
@@ -1228,6 +1240,10 @@ public Integer getPriority()
+ public ProviderInfo<?> getProviderInfo()
{ + return providerInfo; + }+
public boolean equals(Object o) {
if (!(o instanceof NameKey)) {
return false;
@@ -1257,7 +1273,7 @@ public String toString() {
int priority = getFilterPriority(p, providerCls);
for (String name : names)
{ - map.put(new NameKey(name, priority, p.getClass()), p); + map.put(new NameKey(name, priority, p.getClass(), p), p); }}
@@ -1288,8 +1304,17 @@ protected static int getFilterPriority(ProviderInfo<?> p, Class<?> providerCls)
protected static class NameKeyComparator extends AbstractPriorityComparator
implements Comparator<NameKey> {
+ private final Comparator<ProviderInfo<?>> comparator;
+
public NameKeyComparator(boolean ascending)
+
+ public NameKeyComparator(
+ Comparator<ProviderInfo<?>> comparator, boolean ascending)
@Override
@@ -1298,14 +1323,29 @@ public int compare(NameKey key1, NameKey key2) {
if (result != 0)
+
+ if (comparator != null) {
+ result = comparator.compare(
+ key1.getProviderInfo(), key2.getProviderInfo());
+
+ if (result != 0)
+ }
+
return compare(key1.hashCode(), key2.hashCode());
}
-
}
protected static class NameKeyMap<T> extends TreeMap<NameKey, T> {
private static final long serialVersionUID = -4352258671270502204L;
+ public NameKeyMap(
+ Comparator<ProviderInfo<?>> comparator, boolean ascending)
+
public NameKeyMap(boolean ascending)
@@ -1371,8 +1411,21 @@ protected static boolean filterContractSupported(ProviderInfo<?> provider,
public void setProviderComparator(Comparator<?> providerComparator)
{ this.providerComparator = providerComparator; + sortReaders(); sortWriters(); + + NameKeyMap<ProviderInfo<ReaderInterceptor>> sortedReaderInterceptors = + new NameKeyMap<>( + (Comparator<ProviderInfo<?>>) providerComparator, true); + sortedReaderInterceptors.putAll(readerInterceptors); + NameKeyMap<ProviderInfo<WriterInterceptor>> sortedWriterInterceptors = + new NameKeyMap<>( + (Comparator<ProviderInfo<?>>) providerComparator, true); + sortedWriterInterceptors.putAll(writerInterceptors); + + readerInterceptors = sortedReaderInterceptors; + writerInterceptors = sortedWriterInterceptors; } }
diff --git a/rt/frontend/jaxrs/src/test/java/org/apache/cxf/jaxrs/provider/ProviderFactoryTest.java b/rt/frontend/jaxrs/src/test/java/org/apache/cxf/jaxrs/provider/ProviderFactoryTest.java
index e34c4597d5c..0878870b8f0 100644
— a/rt/frontend/jaxrs/src/test/java/org/apache/cxf/jaxrs/provider/ProviderFactoryTest.java
+++ b/rt/frontend/jaxrs/src/test/java/org/apache/cxf/jaxrs/provider/ProviderFactoryTest.java
@@ -26,8 +26,11 @@
import java.lang.annotation.Annotation;
import java.lang.reflect.Type;
import java.util.ArrayList;
+import java.util.Arrays;
+import java.util.Collection;
import java.util.Collections;
import java.util.Comparator;
+import java.util.Iterator;
import java.util.List;
import java.util.Map;
@@ -49,6 +52,8 @@
import javax.ws.rs.ext.MessageBodyWriter;
import javax.ws.rs.ext.ParamConverter;
import javax.ws.rs.ext.ParamConverterProvider;
+import javax.ws.rs.ext.WriterInterceptor;
+import javax.ws.rs.ext.WriterInterceptorContext;
import javax.xml.bind.JAXBContext;
import javax.xml.bind.annotation.XmlRootElement;
import javax.xml.validation.Schema;
@@ -299,6 +304,79 @@ public int compare(ProviderInfo<MessageBodyWriter<?>> o1, ProviderInfo<MessageBo
assertFalse(lastReader instanceof StringTextProvider);
}
+ @Test
+ public void testCustomProviderSortingWIOnly() {
+ ProviderFactory pf = ServerProviderFactory.getInstance();
+
+ pf.setUserProviders(
+ Arrays.asList(
+ new DWriterInterceptor(), new CWriterInterceptor(),
+ new AWriterInterceptor(), new BWriterInterceptor()));
+
+ Comparator<ProviderInfo<WriterInterceptor>> comp =
+ new Comparator<ProviderInfo<WriterInterceptor>>() {
+
+ @Override
+ public int compare(
+ ProviderInfo<WriterInterceptor> o1,
+ ProviderInfo<WriterInterceptor> o2)
+
+ };
+
+ pf.setProviderComparator(comp);
+
+ Collection<ProviderInfo<WriterInterceptor>> values =
+ pf.writerInterceptors.values();
+
+ assertEquals(4, values.size());
+
+ Iterator<ProviderInfo<WriterInterceptor>> iterator = values.iterator();
+
+ assertEquals(AWriterInterceptor.class, iterator.next().getProvider().getClass());
+ assertEquals(BWriterInterceptor.class, iterator.next().getProvider().getClass());
+ assertEquals(CWriterInterceptor.class, iterator.next().getProvider().getClass());
+ assertEquals(DWriterInterceptor.class, iterator.next().getProvider().getClass());
+ }
+
+ private static class AWriterInterceptor implements WriterInterceptor {
+ @Override
+ public void aroundWriteTo(WriterInterceptorContext context) throws
+ IOException, WebApplicationException
+ }
+
+ private static class BWriterInterceptor implements WriterInterceptor {
+ @Override
+ public void aroundWriteTo(WriterInterceptorContext context) throws
+ IOException, WebApplicationException {++ }
+ }
+
+ private static class CWriterInterceptor implements WriterInterceptor {
+ @Override
+ public void aroundWriteTo(WriterInterceptorContext context) throws
+ IOException, WebApplicationException
+ }
+
+ private static class DWriterInterceptor implements WriterInterceptor {
+ @Override
+ public void aroundWriteTo(WriterInterceptorContext context) throws
+ IOException, WebApplicationException {++ }
+ }
+
@Test
public void testCustomProviderSortingWithBus() {
WildcardReader wc1 = new WildcardReader();
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
csierra opened a new pull request #385:
CXF-7657Reader and Writer interceptors are not sorted by the provi…URL: https://github.com/apache/cxf/pull/385
…der comparator
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org