Use Caffeine for caching HTTP headers instead of commons-collections4 LRUMap

src/protocol/http/build.gradle.kts

@@ -58,7 +58,9 @@ dependencies {
     }
     implementation("org.jsoup:jsoup")
     implementation("oro:oro")
-    implementation("org.apache.commons:commons-collections4")
+    runtimeOnly("org.apache.commons:commons-collections4") {
+        because("commons-collections4 was a dependency in previous JMeter versions, so we keep it for compatibility")
+    }
     implementation("commons-net:commons-net")
     implementation("com.helger.commons:ph-commons") {
         // We don't really need to use/distribute jsr305

src/protocol/http/src/main/java/org/apache/jmeter/protocol/http/control/CacheManager.java

@@ -23,15 +23,13 @@
 import java.net.URLConnection;
 import java.util.ArrayList;
 import java.util.Arrays;
-import java.util.Collections;
 import java.util.Date;
 import java.util.HashMap;
 import java.util.HashSet;
 import java.util.List;
 import java.util.Map;
 import java.util.Set;
 
-import org.apache.commons.collections4.map.LRUMap;
 import org.apache.commons.lang3.StringUtils;
 import org.apache.commons.lang3.tuple.ImmutablePair;
 import org.apache.commons.lang3.tuple.Pair;
@@ -54,6 +52,9 @@
 import org.slf4j.Logger;
 import org.slf4j.LoggerFactory;
 
+import com.github.benmanes.caffeine.cache.Cache;
+import com.github.benmanes.caffeine.cache.Caffeine;
+
 /**
  * Handles HTTP Caching.
  */
@@ -81,15 +82,15 @@ public class CacheManager extends ConfigTestElement implements TestStateListener
     public static final String MAX_SIZE = "maxSize";  // $NON-NLS-1$
     //-
 
-    private transient InheritableThreadLocal<Map<String, CacheEntry>> threadCache;
+    private transient InheritableThreadLocal<Cache<String, CacheEntry>> threadCache;
 
     private transient boolean useExpires; // Cached value
 
     /**
      * used to share the cache between 2 cache managers
      * @see CacheManager#createCacheManagerProxy()
      * @since 3.0 */
-    private transient Map<String, CacheEntry> localCache;
+    private transient Cache<String, CacheEntry> localCache;
 
     public CacheManager() {
         setProperty(new BooleanProperty(CLEAR, false));
@@ -98,7 +99,7 @@ public CacheManager() {
         useExpires = false;
     }
 
-    CacheManager(Map<String, CacheEntry> localCache, boolean useExpires) {
+    CacheManager(Cache<String, CacheEntry> localCache, boolean useExpires) {
         this.localCache = localCache;
         this.useExpires = useExpires;
     }
@@ -282,13 +283,16 @@ private void setCache(String lastModified, String cacheControl, String expires,
             getCache().put(url, new CacheEntry(lastModified, expiresDate, etag, varyHeader.getLeft()));
             getCache().put(varyUrl(url, varyHeader.getLeft(), varyHeader.getRight()), new CacheEntry(lastModified, expiresDate, etag, null));
         } else {
-            if (getCache().get(url) != null) {
-                log.debug("Entry for {} already in cache.", url);
-                return;
-            }
-            CacheEntry cacheEntry = new CacheEntry(lastModified, expiresDate, etag, null);
-            log.debug("Set entry {} into cache for url {}", url, cacheEntry);
-            getCache().put(url, cacheEntry);
+            // Makes expiresDate effectively-final
+            Date entryExpiresDate = expiresDate;
+            getCache().get(
+                    url,
+                    key -> {
+                        CacheEntry cacheEntry = new CacheEntry(lastModified, entryExpiresDate, etag, null);
+                        log.debug("Set entry {} into cache for url {}", url, cacheEntry);
+                        return cacheEntry;
+                    }
+            );
         }
     }
 
@@ -536,7 +540,7 @@ private static boolean entryStillValid(URL url, CacheEntry entry) {
     }
 
     private CacheEntry getEntry(String url, Header[] headers) {
-        CacheEntry entry = getCache().get(url);
+        CacheEntry entry = getCache().getIfPresent(url);
         log.debug("getEntry url:{} entry:{} header:{}", url, entry, headers);
         if (entry == null) {
             log.debug("No entry found for url {}", url);
@@ -566,7 +570,7 @@ private static String varyUrl(String url, String headerName, String headerValue)
         return "vary-" + headerName + "-" + headerValue + "-" + url;
     }
 
-    private Map<String, CacheEntry> getCache() {
+    private Cache<String, CacheEntry> getCache() {
         return localCache != null ? localCache : threadCache.get();
     }
 
@@ -609,12 +613,13 @@ public void clear(){
 
     private void clearCache() {
         log.debug("Clear cache");
-        threadCache = new InheritableThreadLocal<Map<String, CacheEntry>>(){
+        // TODO: avoid re-creating the thread local every time, reset its contents instead
+        threadCache = new InheritableThreadLocal<Cache<String, CacheEntry>>(){
             @Override
-            protected Map<String, CacheEntry> initialValue(){
-                // Bug 51942 - this map may be used from multiple threads
-                Map<String, CacheEntry> map = new LRUMap<>(getMaxSize());
-                return Collections.synchronizedMap(map);
+            protected Cache<String, CacheEntry> initialValue() {
+                return Caffeine.newBuilder()
+                        .maximumSize(getMaxSize())
+                        .build();
             }
         };
     }

src/protocol/http/src/test/java/org/apache/jmeter/protocol/http/control/TestCacheManagerBase.java

@@ -29,7 +29,6 @@
 import java.time.ZoneId;
 import java.time.format.DateTimeFormatter;
 import java.util.Locale;
-import java.util.Map;
 
 import org.apache.jmeter.junit.JMeterTestCase;
 import org.apache.jmeter.protocol.http.control.CacheManager.CacheEntry;
@@ -39,6 +38,8 @@
 import org.junit.jupiter.api.parallel.Execution;
 import org.junit.jupiter.api.parallel.ExecutionMode;
 
+import com.github.benmanes.caffeine.cache.Cache;
+
 @Execution(ExecutionMode.CONCURRENT)
 public abstract class TestCacheManagerBase extends JMeterTestCase {
     protected static final String LOCAL_HOST = "http://localhost/";
@@ -386,7 +387,7 @@ private void cacheResultWithGivenCode(String responseCode) throws Exception {
     @Test
     public void testSaveDetailsWithEmptySampleResultGivesNoCacheEntry() throws Exception {
         cacheResultWithGivenCode("");
-        assertTrue(getThreadCache().isEmpty(), "Saving details with empty SampleResult should not make cache entry.");
+        assertTrue(getThreadCache().asMap().isEmpty(), "Saving details with empty SampleResult should not make cache entry.");
     }
 
     @Test
@@ -423,13 +424,13 @@ public void testSetHeadersHttpMethodWithSampleResultWithResponseCode404GivesNoCa
 
     @Test
     public void testClearCache() throws Exception {
-        assertTrue(getThreadCache().isEmpty(), "ThreadCache should be empty initially.");
+        assertTrue(getThreadCache().asMap().isEmpty(), "ThreadCache should be empty initially.");
         cacheResultWithGivenCode("200");
         assertFalse(
-                getThreadCache().isEmpty(),
+                getThreadCache().asMap().isEmpty(),
                 "ThreadCache should be populated after saving details for HttpMethod with SampleResult with response code 200.");
         this.cacheManager.clear();
-        assertTrue(getThreadCache().isEmpty(), "ThreadCache should be emptied by call to clear.");
+        assertTrue(getThreadCache().asMap().isEmpty(), "ThreadCache should be emptied by call to clear.");
     }
 
     protected HTTPSampleResult getSampleResultWithSpecifiedResponseCode(String code) {
@@ -440,17 +441,17 @@ protected HTTPSampleResult getSampleResultWithSpecifiedResponseCode(String code)
         return sampleResult;
     }
 
-    private Map<String, CacheManager.CacheEntry> getThreadCache() throws Exception {
+    private Cache<String, CacheManager.CacheEntry> getThreadCache() throws Exception {
         Field threadLocalfield = CacheManager.class.getDeclaredField("threadCache");
         threadLocalfield.setAccessible(true);
         @SuppressWarnings("unchecked")
-        ThreadLocal<Map<String, CacheManager.CacheEntry>> threadLocal = (ThreadLocal<Map<String, CacheManager.CacheEntry>>) threadLocalfield
+        ThreadLocal<Cache<String, CacheManager.CacheEntry>> threadLocal = (ThreadLocal<Cache<String, CacheManager.CacheEntry>>) threadLocalfield
                 .get(this.cacheManager);
         return threadLocal.get();
     }
 
     protected CacheManager.CacheEntry getThreadCacheEntry(String url) throws Exception {
-        return getThreadCache().get(url);
+        return getThreadCache().getIfPresent(url);
     }
 
 

src/protocol/http/src/test/java/org/apache/jmeter/protocol/http/control/TestCacheManagerThreadIteration.java

@@ -31,7 +31,6 @@
 import java.util.ArrayList;
 import java.util.List;
 import java.util.Locale;
-import java.util.Map;
 
 import org.apache.http.Header;
 import org.apache.http.HttpEntity;
@@ -53,6 +52,8 @@
 import org.junit.jupiter.api.BeforeEach;
 import org.junit.jupiter.api.Test;
 
+import com.github.benmanes.caffeine.cache.Cache;
+
 /**
  * Test {@link CacheManager} that uses HTTPHC4Impl
  */
@@ -292,17 +293,17 @@ protected void setRequestHeaders() {
         this.cacheManager.setHeaders(this.url, this.httpMethod);
     }
 
-    private Map<String, CacheManager.CacheEntry> getThreadCache() throws Exception {
+    private Cache<String, CacheManager.CacheEntry> getThreadCache() throws Exception {
         Field threadLocalfield = CacheManager.class.getDeclaredField("threadCache");
         threadLocalfield.setAccessible(true);
         @SuppressWarnings("unchecked")
-        ThreadLocal<Map<String, CacheManager.CacheEntry>> threadLocal = (ThreadLocal<Map<String, CacheManager.CacheEntry>>) threadLocalfield
+        ThreadLocal<Cache<String, CacheManager.CacheEntry>> threadLocal = (ThreadLocal<Cache<String, CacheManager.CacheEntry>>) threadLocalfield
                 .get(this.cacheManager);
         return threadLocal.get();
     }
 
     protected CacheManager.CacheEntry getThreadCacheEntry(String url) throws Exception {
-        return getThreadCache().get(url);
+        return getThreadCache().getIfPresent(url);
     }
     @Test
     public void testCacheControlCleared() throws Exception {

src/protocol/http/src/test/java/org/apache/jmeter/protocol/http/parser/TestCssParser.java

@@ -25,9 +25,10 @@
 import java.net.MalformedURLException;
 import java.net.URL;
 import java.nio.charset.StandardCharsets;
+import java.util.ArrayList;
+import java.util.Iterator;
 import java.util.List;
 
-import org.apache.commons.collections4.IteratorUtils;
 import org.apache.jmeter.junit.JMeterTestCase;
 import org.hamcrest.CoreMatchers;
 import org.junit.jupiter.api.Test;
@@ -74,10 +75,12 @@ private List<URL> extractUrls(String css) throws LinkExtractorParseException,
 
     private List<URL> extractUrls(CssParser parser, String css)
             throws LinkExtractorParseException, MalformedURLException {
-        List<URL> result = IteratorUtils.toList(parser.getEmbeddedResourceURLs(
+        List<URL> result = new ArrayList<>();
+        Iterator<URL> urlIterator = parser.getEmbeddedResourceURLs(
                 "Mozilla", css.getBytes(StandardCharsets.UTF_8), new URL(
                         "http://example.org/"), StandardCharsets.UTF_8
-                        .displayName()));
+                        .displayName());
+        urlIterator.forEachRemaining(result::add);
         return result;
     }
 }

xdocs/changes.xml

@@ -74,6 +74,7 @@ Summary
 
 <h3>HTTP Samplers and Test Script Recorder</h3>
 <ul>
+  <li><pr>5911</pr> Use Caffeine for caching HTTP headers instead of commons-collections4 LRUMap</li>
 </ul>
 
 <h3>Other samplers</h3>