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 < [email protected]> wrote: > Am 24.07.2017 um 22:15 schrieb [email protected]: > >> 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.
