You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@jmeter.apache.org by Philippe Mouawad <ph...@gmail.com> on 2017/07/25 17:29:05 UTC

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/

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.