You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@jmeter.apache.org by pm...@apache.org on 2017/07/24 20:15:05 UTC

svn commit: r1802864 - in /jmeter/trunk: src/protocol/http/org/apache/jmeter/protocol/http/control/ src/protocol/http/org/apache/jmeter/protocol/http/sampler/ test/src/org/apache/jmeter/protocol/http/control/

Author: pmouawad
Date: Mon Jul 24 20:15:05 2017
New Revision: 1802864

URL: http://svn.apache.org/viewvc?rev=1802864&view=rev
Log:
Bug 61332 - NIGHTLY BUILD : HTTP sampler with Cache manager doesn't work with JAVA implementation
Bugzilla Id: 61332

Modified:
    jmeter/trunk/src/protocol/http/org/apache/jmeter/protocol/http/control/CacheManager.java
    jmeter/trunk/src/protocol/http/org/apache/jmeter/protocol/http/sampler/HTTPJavaImpl.java
    jmeter/trunk/test/src/org/apache/jmeter/protocol/http/control/TestCacheManagerUrlConnection.java

Modified: jmeter/trunk/src/protocol/http/org/apache/jmeter/protocol/http/control/CacheManager.java
URL: http://svn.apache.org/viewvc/jmeter/trunk/src/protocol/http/org/apache/jmeter/protocol/http/control/CacheManager.java?rev=1802864&r1=1802863&r2=1802864&view=diff
==============================================================================
--- jmeter/trunk/src/protocol/http/org/apache/jmeter/protocol/http/control/CacheManager.java (original)
+++ jmeter/trunk/src/protocol/http/org/apache/jmeter/protocol/http/control/CacheManager.java Mon Jul 24 20:15:05 2017
@@ -388,10 +388,13 @@ public class CacheManager extends Config
      * <li>If-None-Match</li>
      * </ul>
      * @param url {@link URL} to look up in cache
+     * @param headers Array of {@link org.apache.jmeter.protocol.http.control.Header}
      * @param conn where to set the headers
      */
-    public void setHeaders(HttpURLConnection conn, URL url) {
-        CacheEntry entry = getEntry(url.toString(), asHeaders(conn.getHeaderFields()));
+    public void setHeaders(HttpURLConnection conn,
+            org.apache.jmeter.protocol.http.control.Header[] headers, URL url) {
+        CacheEntry entry = getEntry(url.toString(), 
+                headers != null ? asHeaders(headers) : new Header[0]);
         if (log.isDebugEnabled()){
             log.debug("{}(Java) {} {}", conn.getRequestMethod(), url.toString(), entry);
         }
@@ -451,14 +454,6 @@ public class CacheManager extends Config
         }
         return result.toArray(new Header[result.size()]);
     }
-
-    private Header[] asHeaders(Map<String, List<String>> headers) {
-        List<Header> result = new ArrayList<>(headers.size());
-        for (Map.Entry<String, List<String>> header: headers.entrySet()) {
-            new BasicHeader(header.getKey(), String.join(", ", header.getValue()));
-        }
-        return result.toArray(new Header[result.size()]);
-    }
 
     private static class HeaderAdapter implements Header {
 

Modified: jmeter/trunk/src/protocol/http/org/apache/jmeter/protocol/http/sampler/HTTPJavaImpl.java
URL: http://svn.apache.org/viewvc/jmeter/trunk/src/protocol/http/org/apache/jmeter/protocol/http/sampler/HTTPJavaImpl.java?rev=1802864&r1=1802863&r2=1802864&view=diff
==============================================================================
--- jmeter/trunk/src/protocol/http/org/apache/jmeter/protocol/http/sampler/HTTPJavaImpl.java (original)
+++ jmeter/trunk/src/protocol/http/org/apache/jmeter/protocol/http/sampler/HTTPJavaImpl.java Mon Jul 24 20:15:05 2017
@@ -364,21 +364,26 @@ public class HTTPJavaImpl extends HTTPAb
      *            for this <code>UrlConfig</code>
      * @param cacheManager the CacheManager (may be null)
      */
-    private void setConnectionHeaders(HttpURLConnection conn, URL u, HeaderManager headerManager, CacheManager cacheManager) {
+    private void setConnectionHeaders(HttpURLConnection conn, URL u, 
+            HeaderManager headerManager, CacheManager cacheManager) {
         // Add all the headers from the HeaderManager
+        Header[] arrayOfHeaders = null;
         if (headerManager != null) {
             CollectionProperty headers = headerManager.getHeaders();
             if (headers != null) {
+                int i=0;
+                arrayOfHeaders = new Header[headers.size()];
                 for (JMeterProperty jMeterProperty : headers) {
                     Header header = (Header) jMeterProperty.getObjectValue();
                     String n = header.getName();
                     String v = header.getValue();
+                    arrayOfHeaders[i++] = header;
                     conn.addRequestProperty(n, v);
                 }
             }
         }
         if (cacheManager != null){
-            cacheManager.setHeaders(conn, u);
+            cacheManager.setHeaders(conn, arrayOfHeaders, u);
         }
     }
 

Modified: jmeter/trunk/test/src/org/apache/jmeter/protocol/http/control/TestCacheManagerUrlConnection.java
URL: http://svn.apache.org/viewvc/jmeter/trunk/test/src/org/apache/jmeter/protocol/http/control/TestCacheManagerUrlConnection.java?rev=1802864&r1=1802863&r2=1802864&view=diff
==============================================================================
--- jmeter/trunk/test/src/org/apache/jmeter/protocol/http/control/TestCacheManagerUrlConnection.java (original)
+++ jmeter/trunk/test/src/org/apache/jmeter/protocol/http/control/TestCacheManagerUrlConnection.java Mon Jul 24 20:15:05 2017
@@ -22,6 +22,7 @@ import static org.junit.Assert.assertEqu
 import static org.junit.Assert.assertNotNull;
 
 import java.net.HttpURLConnection;
+import java.util.ArrayList;
 import java.util.List;
 import java.util.Map;
 
@@ -59,10 +60,25 @@ public class TestCacheManagerUrlConnecti
     protected void addRequestHeader(String requestHeader, String value) {
         // no-op
     }
+    
+    private org.apache.jmeter.protocol.http.control.Header[] asHeaders(Map<String, List<String>> headers) {
+        List<org.apache.jmeter.protocol.http.control.Header> result = new ArrayList<>(headers.size());
+        for (Map.Entry<String, List<String>> header: headers.entrySet()) {
+            // Java Implementation returns a null header for URL
+            if(header.getKey() != null) {
+                result.add(new org.apache.jmeter.protocol.http.control.Header(
+                        header.getKey(), String.join(", ", header.getValue())));
+            }
+        }
+        return result.toArray(new org.apache.jmeter.protocol.http.control.Header[result.size()]);
+    }
 
     @Override
     protected void setRequestHeaders() {
-        this.cacheManager.setHeaders((HttpURLConnection)this.urlConnection, this.url);
+        this.cacheManager.setHeaders(
+                (HttpURLConnection)this.urlConnection, 
+                asHeaders(urlConnection.getHeaderFields()),
+                this.url);
     }
 
     private static void checkProperty(Map<String, List<String>> properties, String property, String expectedPropertyValue) {



Re: svn commit: r1802864 - in /jmeter/trunk: src/protocol/http/org/apache/jmeter/protocol/http/control/ src/protocol/http/org/apache/jmeter/protocol/http/sampler/ test/src/org/apache/jmeter/protocol/http/control/

Posted by Philippe Mouawad <ph...@gmail.com>.
Hi Felix,
I'm ok with your simpler patch if it works.
So don't hesitate to change.

Regards
Philippe

On Tue, Jul 25, 2017 at 7:24 PM, Felix Schumacher <
felix.schumacher@internetallee.de> wrote:

> Am 24.07.2017 um 22:15 schrieb pmouawad@apache.org:
>
>> Author: pmouawad
>> Date: Mon Jul 24 20:15:05 2017
>> New Revision: 1802864
>>
>> URL: http://svn.apache.org/viewvc?rev=1802864&view=rev
>> Log:
>> Bug 61332 - NIGHTLY BUILD : HTTP sampler with Cache manager doesn't work
>> with JAVA implementation
>> Bugzilla Id: 61332
>>
>> Modified:
>>      jmeter/trunk/src/protocol/http/org/apache/jmeter/protocol/
>> http/control/CacheManager.java
>>      jmeter/trunk/src/protocol/http/org/apache/jmeter/protocol/
>> http/sampler/HTTPJavaImpl.java
>>      jmeter/trunk/test/src/org/apache/jmeter/protocol/http/contr
>> ol/TestCacheManagerUrlConnection.java
>>
>> Modified: jmeter/trunk/src/protocol/http/org/apache/jmeter/protocol/
>> http/control/CacheManager.java
>> URL: http://svn.apache.org/viewvc/jmeter/trunk/src/protocol/http/
>> org/apache/jmeter/protocol/http/control/CacheManager.
>> java?rev=1802864&r1=1802863&r2=1802864&view=diff
>> ============================================================
>> ==================
>> --- jmeter/trunk/src/protocol/http/org/apache/jmeter/protocol/http/control/CacheManager.java
>> (original)
>> +++ jmeter/trunk/src/protocol/http/org/apache/jmeter/protocol/http/control/CacheManager.java
>> Mon Jul 24 20:15:05 2017
>> @@ -388,10 +388,13 @@ public class CacheManager extends Config
>>        * <li>If-None-Match</li>
>>        * </ul>
>>        * @param url {@link URL} to look up in cache
>> +     * @param headers Array of {@link org.apache.jmeter.protocol.htt
>> p.control.Header}
>>        * @param conn where to set the headers
>>        */
>> -    public void setHeaders(HttpURLConnection conn, URL url) {
>> -        CacheEntry entry = getEntry(url.toString(),
>> asHeaders(conn.getHeaderFields()));
>>
> A simple patch would have been to replace this occurrence of
> conn.getHeaderFields() with conn.getRequestProperties :) (But to be honest,
> I stared for two hours at the code yesterday without a successful patch)
>
> I will look at the rest of the patch, but I suspect that using the Header
> Manager(s) for source of headers is not enough, i.e. will give less headers
> than getRequestProperties.
>
> But note, that even getRequestProperties will probably not give all
> headers, that are actually sent by URLConnection and friends.
>
> Felix
>
> +    public void setHeaders(HttpURLConnection conn,
>> +            org.apache.jmeter.protocol.http.control.Header[] headers,
>> URL url) {
>> +        CacheEntry entry = getEntry(url.toString(),
>> +                headers != null ? asHeaders(headers) : new Header[0]);
>>           if (log.isDebugEnabled()){
>>               log.debug("{}(Java) {} {}", conn.getRequestMethod(),
>> url.toString(), entry);
>>           }
>> @@ -451,14 +454,6 @@ public class CacheManager extends Config
>>           }
>>           return result.toArray(new Header[result.size()]);
>>       }
>> -
>> -    private Header[] asHeaders(Map<String, List<String>> headers) {
>> -        List<Header> result = new ArrayList<>(headers.size());
>> -        for (Map.Entry<String, List<String>> header: headers.entrySet())
>> {
>> -            new BasicHeader(header.getKey(), String.join(", ",
>> header.getValue()));
>> -        }
>> -        return result.toArray(new Header[result.size()]);
>> -    }
>>         private static class HeaderAdapter implements Header {
>>
>> Modified: jmeter/trunk/src/protocol/http/org/apache/jmeter/protocol/
>> http/sampler/HTTPJavaImpl.java
>> URL: http://svn.apache.org/viewvc/jmeter/trunk/src/protocol/http/
>> org/apache/jmeter/protocol/http/sampler/HTTPJavaImpl.
>> java?rev=1802864&r1=1802863&r2=1802864&view=diff
>> ============================================================
>> ==================
>> --- jmeter/trunk/src/protocol/http/org/apache/jmeter/protocol/http/sampler/HTTPJavaImpl.java
>> (original)
>> +++ jmeter/trunk/src/protocol/http/org/apache/jmeter/protocol/http/sampler/HTTPJavaImpl.java
>> Mon Jul 24 20:15:05 2017
>> @@ -364,21 +364,26 @@ public class HTTPJavaImpl extends HTTPAb
>>        *            for this <code>UrlConfig</code>
>>        * @param cacheManager the CacheManager (may be null)
>>        */
>> -    private void setConnectionHeaders(HttpURLConnection conn, URL u,
>> HeaderManager headerManager, CacheManager cacheManager) {
>> +    private void setConnectionHeaders(HttpURLConnection conn, URL u,
>> +            HeaderManager headerManager, CacheManager cacheManager) {
>>           // Add all the headers from the HeaderManager
>> +        Header[] arrayOfHeaders = null;
>>           if (headerManager != null) {
>>               CollectionProperty headers = headerManager.getHeaders();
>>               if (headers != null) {
>> +                int i=0;
>> +                arrayOfHeaders = new Header[headers.size()];
>>                   for (JMeterProperty jMeterProperty : headers) {
>>                       Header header = (Header)
>> jMeterProperty.getObjectValue();
>>                       String n = header.getName();
>>                       String v = header.getValue();
>> +                    arrayOfHeaders[i++] = header;
>>                       conn.addRequestProperty(n, v);
>>                   }
>>               }
>>           }
>>           if (cacheManager != null){
>> -            cacheManager.setHeaders(conn, u);
>> +            cacheManager.setHeaders(conn, arrayOfHeaders, u);
>>           }
>>       }
>>
>> Modified: jmeter/trunk/test/src/org/apache/jmeter/protocol/http/contro
>> l/TestCacheManagerUrlConnection.java
>> URL: http://svn.apache.org/viewvc/jmeter/trunk/test/src/org/apach
>> e/jmeter/protocol/http/control/TestCacheManagerUrlConnection
>> .java?rev=1802864&r1=1802863&r2=1802864&view=diff
>> ============================================================
>> ==================
>> --- jmeter/trunk/test/src/org/apache/jmeter/protocol/http/contro
>> l/TestCacheManagerUrlConnection.java (original)
>> +++ jmeter/trunk/test/src/org/apache/jmeter/protocol/http/contro
>> l/TestCacheManagerUrlConnection.java Mon Jul 24 20:15:05 2017
>> @@ -22,6 +22,7 @@ import static org.junit.Assert.assertEqu
>>   import static org.junit.Assert.assertNotNull;
>>     import java.net.HttpURLConnection;
>> +import java.util.ArrayList;
>>   import java.util.List;
>>   import java.util.Map;
>>   @@ -59,10 +60,25 @@ public class TestCacheManagerUrlConnecti
>>       protected void addRequestHeader(String requestHeader, String value)
>> {
>>           // no-op
>>       }
>> +
>> +    private org.apache.jmeter.protocol.http.control.Header[]
>> asHeaders(Map<String, List<String>> headers) {
>> +        List<org.apache.jmeter.protocol.http.control.Header> result =
>> new ArrayList<>(headers.size());
>> +        for (Map.Entry<String, List<String>> header: headers.entrySet())
>> {
>> +            // Java Implementation returns a null header for URL
>> +            if(header.getKey() != null) {
>> +                result.add(new org.apache.jmeter.protocol.htt
>> p.control.Header(
>> +                        header.getKey(), String.join(", ",
>> header.getValue())));
>> +            }
>> +        }
>> +        return result.toArray(new org.apache.jmeter.protocol.htt
>> p.control.Header[result.size()]);
>> +    }
>>         @Override
>>       protected void setRequestHeaders() {
>> -        this.cacheManager.setHeaders((HttpURLConnection)this.urlConnection,
>> this.url);
>> +        this.cacheManager.setHeaders(
>> +                (HttpURLConnection)this.urlConnection,
>> +                asHeaders(urlConnection.getHeaderFields()),
>> +                this.url);
>>       }
>>         private static void checkProperty(Map<String, List<String>>
>> properties, String property, String expectedPropertyValue) {
>>
>>
>>
>


-- 
Cordialement.
Philippe Mouawad.

Re: svn commit: r1802864 - in /jmeter/trunk: src/protocol/http/org/apache/jmeter/protocol/http/control/ src/protocol/http/org/apache/jmeter/protocol/http/sampler/ test/src/org/apache/jmeter/protocol/http/control/

Posted by Felix Schumacher <fe...@internetallee.de>.
Am 24.07.2017 um 22:15 schrieb pmouawad@apache.org:
> Author: pmouawad
> Date: Mon Jul 24 20:15:05 2017
> New Revision: 1802864
>
> URL: http://svn.apache.org/viewvc?rev=1802864&view=rev
> Log:
> Bug 61332 - NIGHTLY BUILD : HTTP sampler with Cache manager doesn't work with JAVA implementation
> Bugzilla Id: 61332
>
> Modified:
>      jmeter/trunk/src/protocol/http/org/apache/jmeter/protocol/http/control/CacheManager.java
>      jmeter/trunk/src/protocol/http/org/apache/jmeter/protocol/http/sampler/HTTPJavaImpl.java
>      jmeter/trunk/test/src/org/apache/jmeter/protocol/http/control/TestCacheManagerUrlConnection.java
>
> Modified: jmeter/trunk/src/protocol/http/org/apache/jmeter/protocol/http/control/CacheManager.java
> URL: http://svn.apache.org/viewvc/jmeter/trunk/src/protocol/http/org/apache/jmeter/protocol/http/control/CacheManager.java?rev=1802864&r1=1802863&r2=1802864&view=diff
> ==============================================================================
> --- jmeter/trunk/src/protocol/http/org/apache/jmeter/protocol/http/control/CacheManager.java (original)
> +++ jmeter/trunk/src/protocol/http/org/apache/jmeter/protocol/http/control/CacheManager.java Mon Jul 24 20:15:05 2017
> @@ -388,10 +388,13 @@ public class CacheManager extends Config
>        * <li>If-None-Match</li>
>        * </ul>
>        * @param url {@link URL} to look up in cache
> +     * @param headers Array of {@link org.apache.jmeter.protocol.http.control.Header}
>        * @param conn where to set the headers
>        */
> -    public void setHeaders(HttpURLConnection conn, URL url) {
> -        CacheEntry entry = getEntry(url.toString(), asHeaders(conn.getHeaderFields()));
A simple patch would have been to replace this occurrence of 
conn.getHeaderFields() with conn.getRequestProperties :) (But to be 
honest, I stared for two hours at the code yesterday without a 
successful patch)

I will look at the rest of the patch, but I suspect that using the 
Header Manager(s) for source of headers is not enough, i.e. will give 
less headers than getRequestProperties.

But note, that even getRequestProperties will probably not give all 
headers, that are actually sent by URLConnection and friends.

Felix
> +    public void setHeaders(HttpURLConnection conn,
> +            org.apache.jmeter.protocol.http.control.Header[] headers, URL url) {
> +        CacheEntry entry = getEntry(url.toString(),
> +                headers != null ? asHeaders(headers) : new Header[0]);
>           if (log.isDebugEnabled()){
>               log.debug("{}(Java) {} {}", conn.getRequestMethod(), url.toString(), entry);
>           }
> @@ -451,14 +454,6 @@ public class CacheManager extends Config
>           }
>           return result.toArray(new Header[result.size()]);
>       }
> -
> -    private Header[] asHeaders(Map<String, List<String>> headers) {
> -        List<Header> result = new ArrayList<>(headers.size());
> -        for (Map.Entry<String, List<String>> header: headers.entrySet()) {
> -            new BasicHeader(header.getKey(), String.join(", ", header.getValue()));
> -        }
> -        return result.toArray(new Header[result.size()]);
> -    }
>   
>       private static class HeaderAdapter implements Header {
>   
>
> Modified: jmeter/trunk/src/protocol/http/org/apache/jmeter/protocol/http/sampler/HTTPJavaImpl.java
> URL: http://svn.apache.org/viewvc/jmeter/trunk/src/protocol/http/org/apache/jmeter/protocol/http/sampler/HTTPJavaImpl.java?rev=1802864&r1=1802863&r2=1802864&view=diff
> ==============================================================================
> --- jmeter/trunk/src/protocol/http/org/apache/jmeter/protocol/http/sampler/HTTPJavaImpl.java (original)
> +++ jmeter/trunk/src/protocol/http/org/apache/jmeter/protocol/http/sampler/HTTPJavaImpl.java Mon Jul 24 20:15:05 2017
> @@ -364,21 +364,26 @@ public class HTTPJavaImpl extends HTTPAb
>        *            for this <code>UrlConfig</code>
>        * @param cacheManager the CacheManager (may be null)
>        */
> -    private void setConnectionHeaders(HttpURLConnection conn, URL u, HeaderManager headerManager, CacheManager cacheManager) {
> +    private void setConnectionHeaders(HttpURLConnection conn, URL u,
> +            HeaderManager headerManager, CacheManager cacheManager) {
>           // Add all the headers from the HeaderManager
> +        Header[] arrayOfHeaders = null;
>           if (headerManager != null) {
>               CollectionProperty headers = headerManager.getHeaders();
>               if (headers != null) {
> +                int i=0;
> +                arrayOfHeaders = new Header[headers.size()];
>                   for (JMeterProperty jMeterProperty : headers) {
>                       Header header = (Header) jMeterProperty.getObjectValue();
>                       String n = header.getName();
>                       String v = header.getValue();
> +                    arrayOfHeaders[i++] = header;
>                       conn.addRequestProperty(n, v);
>                   }
>               }
>           }
>           if (cacheManager != null){
> -            cacheManager.setHeaders(conn, u);
> +            cacheManager.setHeaders(conn, arrayOfHeaders, u);
>           }
>       }
>   
>
> Modified: jmeter/trunk/test/src/org/apache/jmeter/protocol/http/control/TestCacheManagerUrlConnection.java
> URL: http://svn.apache.org/viewvc/jmeter/trunk/test/src/org/apache/jmeter/protocol/http/control/TestCacheManagerUrlConnection.java?rev=1802864&r1=1802863&r2=1802864&view=diff
> ==============================================================================
> --- jmeter/trunk/test/src/org/apache/jmeter/protocol/http/control/TestCacheManagerUrlConnection.java (original)
> +++ jmeter/trunk/test/src/org/apache/jmeter/protocol/http/control/TestCacheManagerUrlConnection.java Mon Jul 24 20:15:05 2017
> @@ -22,6 +22,7 @@ import static org.junit.Assert.assertEqu
>   import static org.junit.Assert.assertNotNull;
>   
>   import java.net.HttpURLConnection;
> +import java.util.ArrayList;
>   import java.util.List;
>   import java.util.Map;
>   
> @@ -59,10 +60,25 @@ public class TestCacheManagerUrlConnecti
>       protected void addRequestHeader(String requestHeader, String value) {
>           // no-op
>       }
> +
> +    private org.apache.jmeter.protocol.http.control.Header[] asHeaders(Map<String, List<String>> headers) {
> +        List<org.apache.jmeter.protocol.http.control.Header> result = new ArrayList<>(headers.size());
> +        for (Map.Entry<String, List<String>> header: headers.entrySet()) {
> +            // Java Implementation returns a null header for URL
> +            if(header.getKey() != null) {
> +                result.add(new org.apache.jmeter.protocol.http.control.Header(
> +                        header.getKey(), String.join(", ", header.getValue())));
> +            }
> +        }
> +        return result.toArray(new org.apache.jmeter.protocol.http.control.Header[result.size()]);
> +    }
>   
>       @Override
>       protected void setRequestHeaders() {
> -        this.cacheManager.setHeaders((HttpURLConnection)this.urlConnection, this.url);
> +        this.cacheManager.setHeaders(
> +                (HttpURLConnection)this.urlConnection,
> +                asHeaders(urlConnection.getHeaderFields()),
> +                this.url);
>       }
>   
>       private static void checkProperty(Map<String, List<String>> properties, String property, String expectedPropertyValue) {
>
>