[
https://issues.apache.org/jira/browse/CXF-7553?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=16251766#comment-16251766
]
ASF GitHub Bot commented on CXF-7553:
-------------------------------------
andymc12 closed pull request #335: [CXF-7553] Consider the req headers' quality
factors in selectVariant
URL: https://github.com/apache/cxf/pull/335
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/impl/RequestImpl.java
b/rt/frontend/jaxrs/src/main/java/org/apache/cxf/jaxrs/impl/RequestImpl.java
index 09148549d9e..8eb6738e78c 100644
--- a/rt/frontend/jaxrs/src/main/java/org/apache/cxf/jaxrs/impl/RequestImpl.java
+++ b/rt/frontend/jaxrs/src/main/java/org/apache/cxf/jaxrs/impl/RequestImpl.java
@@ -68,46 +68,60 @@ public Variant selectVariant(List<Variant> vars) throws
IllegalArgumentException
List<Locale> acceptLangs = headers.getAcceptableLanguages();
List<String> acceptEncs = parseAcceptEnc(
headers.getRequestHeaders().getFirst(HttpHeaders.ACCEPT_ENCODING));
-
+ List<Variant> requestVariants = sortAllCombinations(acceptMediaTypes,
acceptLangs, acceptEncs);
List<Object> varyValues = new LinkedList<Object>();
-
- List<Variant> matchingVars = new LinkedList<Variant>();
- for (Variant var : vars) {
- MediaType mt = var.getMediaType();
- Locale lang = var.getLanguage();
- String enc = var.getEncoding();
-
- boolean mtMatched = mt == null || acceptMediaTypes.isEmpty()
- || JAXRSUtils.intersectMimeTypes(acceptMediaTypes, mt).size()
!= 0;
- if (mtMatched) {
- handleVaryValues(varyValues, HttpHeaders.ACCEPT);
- }
-
- boolean langMatched = lang == null || acceptLangs.isEmpty()
- || isLanguageMatched(acceptLangs, lang);
- if (langMatched) {
- handleVaryValues(varyValues, HttpHeaders.ACCEPT_LANGUAGE);
- }
-
- boolean encMatched = acceptEncs.isEmpty() || enc == null
- || isEncMatached(acceptEncs, enc);
- if (encMatched) {
- handleVaryValues(varyValues, HttpHeaders.ACCEPT_ENCODING);
- }
-
- if (mtMatched && encMatched && langMatched) {
- matchingVars.add(var);
+ for (Variant requestVar : requestVariants) {
+ for (Variant var : vars) {
+ MediaType mt = var.getMediaType();
+ Locale lang = var.getLanguage();
+ String enc = var.getEncoding();
+
+ boolean mtMatched = mt == null ||
requestVar.getMediaType().isCompatible(mt);
+ if (mtMatched) {
+ handleVaryValues(varyValues, HttpHeaders.ACCEPT);
+ }
+
+ boolean langMatched = lang == null ||
isLanguageMatched(requestVar.getLanguage(), lang);
+ if (langMatched) {
+ handleVaryValues(varyValues, HttpHeaders.ACCEPT_LANGUAGE);
+ }
+
+ boolean encMatched = acceptEncs.isEmpty() || enc == null
+ || isEncMatached(requestVar.getEncoding(), enc);
+ if (encMatched) {
+ handleVaryValues(varyValues, HttpHeaders.ACCEPT_ENCODING);
+ }
+
+ if (mtMatched && encMatched && langMatched) {
+ addVaryHeader(varyValues);
+ return var;
+ }
}
}
- if (matchingVars.size() > 0) {
- addVaryHeader(varyValues);
- Collections.sort(matchingVars, new VariantComparator());
- return matchingVars.get(0);
- }
return null;
}
- private static void handleVaryValues(List<Object> varyValues, String
...values) {
+ private static List<Variant> sortAllCombinations(List<MediaType>
mediaTypes,
+ List<Locale> langs,
+ List<String> encs) {
+ List<Variant> requestVars = new LinkedList<>();
+ for (MediaType mt : mediaTypes) {
+ for (Locale lang : langs) {
+ if (encs.size() < 1) {
+ requestVars.add(new Variant(mt, lang, null));
+ } else {
+ for (String enc : encs) {
+ requestVars.add(new Variant(mt, lang, enc));
+ }
+ }
+
+ }
+ }
+ Collections.sort(requestVars, VariantComparator.INSTANCE);
+ return requestVars;
+ }
+
+ private static void handleVaryValues(List<Object> varyValues,
String...values) {
for (String v : values) {
if (v != null && !varyValues.contains(v)) {
varyValues.add(v);
@@ -136,28 +150,15 @@ private static void addVaryHeader(List<Object>
varyValues) {
}
}
- private static boolean isLanguageMatched(List<Locale> locales, Locale l) {
-
- for (Locale locale : locales) {
- String language = locale.getLanguage();
- if ("*".equals(language)
- || language.equalsIgnoreCase(l.getLanguage())) {
- return true;
- }
- }
- return false;
+ private static boolean isLanguageMatched(Locale locale, Locale l) {
+
+ String language = locale.getLanguage();
+ return "*".equals(language)
+ || language.equalsIgnoreCase(l.getLanguage());
}
- private static boolean isEncMatached(List<String> accepts, String enc) {
- if (accepts.contains(enc)) {
- return true;
- }
- for (String accept : accepts) {
- if ("*".equals(accept)) {
- return true;
- }
- }
- return false;
+ private static boolean isEncMatached(String accepts, String enc) {
+ return accepts == null || "*".equals(accepts) || accepts.contains(enc);
}
private static List<String> parseAcceptEnc(String acceptEnc) {
@@ -339,6 +340,8 @@ public ResponseBuilder evaluatePreconditions() {
private static class VariantComparator implements Comparator<Variant> {
+ static final VariantComparator INSTANCE = new VariantComparator();
+
public int compare(Variant v1, Variant v2) {
int result = compareMediaTypes(v1.getMediaType(),
v2.getMediaType());
@@ -351,7 +354,6 @@ public int compare(Variant v1, Variant v2) {
if (result == 0) {
result = compareEncodings(v1.getEncoding(), v2.getEncoding());
}
-
return result;
}
diff --git
a/rt/frontend/jaxrs/src/test/java/org/apache/cxf/jaxrs/impl/RequestImplTest.java
b/rt/frontend/jaxrs/src/test/java/org/apache/cxf/jaxrs/impl/RequestImplTest.java
index 0690e9d7c4f..5545f90eb4f 100644
---
a/rt/frontend/jaxrs/src/test/java/org/apache/cxf/jaxrs/impl/RequestImplTest.java
+++
b/rt/frontend/jaxrs/src/test/java/org/apache/cxf/jaxrs/impl/RequestImplTest.java
@@ -145,6 +145,34 @@ public void testMultipleVariantsBestMatch() {
assertSame(var3, new RequestImpl(m).selectVariant(list));
}
+ @Test
+ public void testMultipleVariantsBestMatchMediaTypeQualityFactors() {
+ metadata.putSingle(HttpHeaders.ACCEPT, "a/b;q=0.6, c/d;q=0.5,
e/f+json");
+ metadata.putSingle(HttpHeaders.ACCEPT_LANGUAGE, "en-us");
+ metadata.putSingle(HttpHeaders.ACCEPT_ENCODING, "gzip;q=1.0,
compress");
+
+ List<Variant> list = new ArrayList<Variant>();
+ Variant var1 = new Variant(MediaType.valueOf("a/b"), new Locale("en"),
"gzip");
+ Variant var2 = new Variant(MediaType.valueOf("x/z"), new Locale("en"),
"gzip");
+ Variant var3 = new Variant(MediaType.valueOf("e/f+json"), new
Locale("en"), "gzip");
+ Variant var4 = new Variant(MediaType.valueOf("c/d"), new Locale("en"),
"gzip");
+ list.add(var1);
+ list.add(var2);
+ list.add(var3);
+ list.add(var4);
+ assertSame(var3, new RequestImpl(m).selectVariant(list));
+
+ list.clear();
+ list.add(var1);
+ list.add(var4);
+ assertSame(var1, new RequestImpl(m).selectVariant(list));
+
+ list.clear();
+ list.add(var2);
+ list.add(var4);
+ assertSame(var4, new RequestImpl(m).selectVariant(list));
+ }
+
private void assertSameVariant(MediaType mt, Locale lang, String enc) {
Variant var = new Variant(mt, lang, enc);
List<Variant> list = new ArrayList<Variant>();
----------------------------------------------------------------
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:
[email protected]
> selectVariant doesn't take into account quality factors in Accept header
> ------------------------------------------------------------------------
>
> Key: CXF-7553
> URL: https://issues.apache.org/jira/browse/CXF-7553
> Project: CXF
> Issue Type: Bug
> Components: JAX-RS
> Affects Versions: 3.1.14, 3.2.1
> Reporter: Andy McCright
> Assignee: Andy McCright
> Priority: Minor
> Fix For: 3.1.15, 3.2.2
>
>
> We've got a customer moving from Apache Wink (JAX-RS 1.1) to CXF 3.1.X
> (JAX-RS 2.0) and they are seeing subtle differences in the behavior of the
> javax.ws.rs.core.Request.selectVariant method. According to the spec, the
> returned Variant should be one of the Variants passed in to the selectVariant
> method that is the closest match to the request (based not the Accept,
> Accept-Language, and Accept-Encoding headers). They see this behavior in
> Wink, but in CXF, they see that the first Variant in the passed-in list that
> matches the request's headers is chosen.
> For example, a request with "Accept: a/b, c/d; q=0.5" will return a response
> with "Content-Type: a/b" - assuming that the resource method produces a/b and
> c/d. However, when the user calls:
> List<Variant> list = new ArrayList<Variant>();
> Variant v1 = new Variant("a/b", (Locale) null, null);
> Variant v2 = new Variant("c/d", (Locale) null, null);
> Variant v3 = new Variant("not/used", (Locale) null, null);
> list.add(v3);
> list.add(v2);
> list.add(v1);
> assertSame(v1, Request.selectVariant(list));
> the assertion fails. Even though the user prefers a/b over c/d by a quality
> factor of 1 vs 0.5, the selectVariant method returns the Variant with c/d.
> This is because the RequestImpl's implementation of the selectVariant method
> only checks the request headers when intersecting the Mime (Media)Types.
> To match Wink's implementation (and my understanding of the spec, though to
> be fair, the spec is somewhat ambiguous...), I think CXF should take the
> request header's quality factors into account when determining which Variant
> to return.
--
This message was sent by Atlassian JIRA
(v6.4.14#64029)
