WW-5716 Bound the per-locale definition caches in the Tiles plugin

plugins/tiles/src/main/java/org/apache/tiles/core/definition/dao/CachingLocaleUrlDefinitionDAO.java

@@ -26,7 +26,6 @@
 import org.apache.tiles.request.ApplicationResource;
 import org.apache.tiles.request.locale.LocaleUtil;
 
-import java.util.HashMap;
 import java.util.LinkedHashMap;
 import java.util.Locale;
 import java.util.Map;
@@ -53,13 +52,26 @@ public class CachingLocaleUrlDefinitionDAO extends BaseLocaleUrlDefinitionDAO im
      */
     public static final String CHECK_REFRESH_INIT_PARAMETER = "org.apache.tiles.definition.dao.LocaleUrlDefinitionDAO.CHECK_REFRESH";
 
+    /**
+     * Default upper bound on the number of customization keys (locales) whose definitions are cached at once. Since the
+     * customization key is derived from the request locale, this bounds the cache so it cannot grow without limit as
+     * distinct locales are encountered.
+     */
+    public static final int DEFAULT_MAX_CACHED_LOCALES = 1000;
+
     /**
      * The locale-specific set of definitions objects.
      *
      * @since 2.1.0
      */
     protected Map<Locale, Map<String, Definition>> locale2definitionMap;
 
+    /**
+     * Maximum number of customization keys (locales) retained in {@link #locale2definitionMap}. When exceeded, the
+     * eldest entry is evicted (and reloaded on demand if requested again).
+     */
+    protected int maxCachedLocales = DEFAULT_MAX_CACHED_LOCALES;
+
     /**
      * Flag that, when <code>true</code>, enables automatic checking of URLs
      * changing.
@@ -82,7 +94,29 @@ public class CachingLocaleUrlDefinitionDAO extends BaseLocaleUrlDefinitionDAO im
      */
     public CachingLocaleUrlDefinitionDAO(ApplicationContext applicationContext) {
         super(applicationContext);
-        locale2definitionMap = new HashMap<>();
+        locale2definitionMap = new LinkedHashMap<Locale, Map<String, Definition>>(16, 0.75f, false) {
+            @Override
+            protected boolean removeEldestEntry(Map.Entry<Locale, Map<String, Definition>> eldest) {
+                if (size() <= maxCachedLocales) {
+                    return false;
+                }
+                if (definitionResolver != null) {
+                    definitionResolver.removePatternPaths(eldest.getKey());
+                }
+                return true;
+            }
+        };
+    }
+
+    /**
+     * Sets the maximum number of customization keys (locales) whose definitions are cached. When more distinct keys are
+     * requested, the eldest cached entry is evicted so the cache cannot grow without bound. Evicted entries are reloaded
+     * on demand if requested again, so eviction never changes rendering, only re-incurs a load.
+     *
+     * @param maxCachedLocales the maximum number of cached customization keys; values below 1 are treated as 1
+     */
+    public void setMaxCachedLocales(int maxCachedLocales) {
+        this.maxCachedLocales = Math.max(1, maxCachedLocales);
     }
 
     /**

plugins/tiles/src/main/java/org/apache/tiles/core/definition/pattern/AbstractPatternDefinitionResolver.java

@@ -21,9 +21,9 @@
 import org.apache.tiles.api.Definition;
 
 import java.util.ArrayList;
-import java.util.HashMap;
 import java.util.List;
 import java.util.Map;
+import java.util.concurrent.ConcurrentHashMap;
 
 /**
  * A pattern definition resolver that stores {@link DefinitionPatternMatcher}
@@ -39,7 +39,7 @@ public abstract class AbstractPatternDefinitionResolver<T> implements PatternDef
     /**
      * Stores patterns depending on the locale they refer to.
      */
-    private final Map<T, List<DefinitionPatternMatcher>> localePatternPaths = new HashMap<>();
+    private final Map<T, List<DefinitionPatternMatcher>> localePatternPaths = new ConcurrentHashMap<>();
 
     /** {@inheritDoc} */
     public Definition resolveDefinition(String name, T customizationKey) {
@@ -103,4 +103,10 @@ public void clearPatternPaths(T customizationKey) {
         if (localePatternPaths.get(customizationKey) != null)
             localePatternPaths.get(customizationKey).clear();
     }
+
+    /** {@inheritDoc} */
+    @Override
+    public void removePatternPaths(T customizationKey) {
+        localePatternPaths.remove(customizationKey);
+    }
 }

plugins/tiles/src/main/java/org/apache/tiles/core/definition/pattern/PatternDefinitionResolver.java

@@ -60,4 +60,12 @@ public interface PatternDefinitionResolver<T> {
      * @param customizationKey customization key
      */
     void clearPatternPaths(T customizationKey);
+
+    /**
+     * Removes the stored patterns for a specific customization key entirely, including the key itself. Used when the
+     * owning definitions cache evicts a customization key so that the pattern store cannot grow without bound.
+     *
+     * @param customizationKey customization key
+     */
+    void removePatternPaths(T customizationKey);
 }

plugins/tiles/src/test/java/org/apache/tiles/core/definition/dao/CachingLocaleUrlDefinitionDAOTest.java

@@ -34,6 +34,7 @@
 import org.apache.tiles.request.locale.URLApplicationResource;
 
 import java.util.ArrayList;
+import java.util.Arrays;
 import java.util.HashMap;
 import java.util.HashSet;
 import java.util.List;
@@ -368,4 +369,40 @@ public void testListAttributeLocaleInheritance() {
         assertEquals(1, attributes.size());
         verify(applicationContext);
     }
+
+    /**
+     * The definitions cache is keyed by locale, so it must not grow beyond the configured bound as distinct
+     * locales are requested; the eldest entry is evicted instead, and the eviction is propagated to the pattern
+     * resolver so its per-locale store cannot grow without bound either.
+     */
+    public void testLocaleCacheIsBounded() {
+        List<ApplicationResource> sourceURLs = new ArrayList<>();
+        sourceURLs.add(url1);
+        sourceURLs.add(url2);
+        sourceURLs.add(url3);
+        definitionDao.setSources(sourceURLs);
+        definitionDao.setReader(new DigesterDefinitionsReader());
+
+        List<Locale> evicted = new ArrayList<>();
+        WildcardDefinitionPatternMatcherFactory factory = new WildcardDefinitionPatternMatcherFactory();
+        PatternDefinitionResolver<Locale> recordingResolver = new BasicPatternDefinitionResolver<Locale>(factory, factory) {
+            @Override
+            public void removePatternPaths(Locale customizationKey) {
+                evicted.add(customizationKey);
+                super.removePatternPaths(customizationKey);
+            }
+        };
+        definitionDao.setPatternDefinitionResolver(recordingResolver);
+        definitionDao.setMaxCachedLocales(2);
+
+        for (Locale locale : new Locale[]{Locale.US, Locale.FRENCH, Locale.CANADA_FRENCH, Locale.CHINA}) {
+            assertNotNull("Definitions for " + locale + " were not loaded.",
+                definitionDao.getDefinitions(locale));
+        }
+
+        assertEquals("Definitions cache must not grow beyond the configured bound",
+            2, definitionDao.locale2definitionMap.size());
+        assertEquals("Evicted locales must be removed from the pattern resolver in lockstep",
+            Arrays.asList(Locale.US, Locale.FRENCH), evicted);
+    }
 }

plugins/tiles/src/test/java/org/apache/tiles/core/definition/pattern/AbstractPatternDefinitionResolverTest.java

@@ -79,6 +79,35 @@ public void testClearPatternPaths() {
         testResolveDefinitionImpl();
     }
 
+    /**
+     * Test method for
+     * {@link AbstractPatternDefinitionResolver#removePatternPaths(Object)}: the key and its patterns are dropped
+     * entirely, so nothing resolves for that key afterwards.
+     */
+    @Test
+    public void testRemovePatternPaths() {
+        firstMatcher = createMock(DefinitionPatternMatcher.class);
+        thirdMatcher = createMock(DefinitionPatternMatcher.class);
+
+        Definition firstDefinition = new Definition("first", null, null);
+        Definition firstTransformedDefinition = new Definition("firstTransformed", null, null);
+
+        expect(firstMatcher.createDefinition("firstTransformed")).andReturn(firstTransformedDefinition);
+        replay(firstMatcher, thirdMatcher);
+
+        Map<String, Definition> localeDefsMap = new LinkedHashMap<>();
+        localeDefsMap.put("first", firstDefinition);
+        resolver.storeDefinitionPatterns(localeDefsMap, 1);
+
+        assertEquals(firstTransformedDefinition, resolver.resolveDefinition("firstTransformed", 1));
+
+        resolver.removePatternPaths(1);
+        assertNull("removePatternPaths must drop the entry so nothing resolves for the key",
+            resolver.resolveDefinition("firstTransformed", 1));
+
+        verify(firstMatcher, thirdMatcher);
+    }
+
     private void testResolveDefinitionImpl() {
 
         firstMatcher = createMock(DefinitionPatternMatcher.class);