You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@cxf.apache.org by GitBox <gi...@apache.org> on 2017/11/14 17:20:38 UTC

[GitHub] andymc12 closed pull request #335: [CXF-7553] Consider the req headers' quality factors in selectVariant

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:
users@infra.apache.org


With regards,
Apache Git Services