diff --git a/plugins/tiles/src/main/java/org/apache/tiles/core/definition/dao/CachingLocaleUrlDefinitionDAO.java b/plugins/tiles/src/main/java/org/apache/tiles/core/definition/dao/CachingLocaleUrlDefinitionDAO.java index b280fc97b0..5caaef402f 100644 --- a/plugins/tiles/src/main/java/org/apache/tiles/core/definition/dao/CachingLocaleUrlDefinitionDAO.java +++ b/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,6 +52,13 @@ 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. * @@ -60,6 +66,12 @@ public class CachingLocaleUrlDefinitionDAO extends BaseLocaleUrlDefinitionDAO im */ protected Map> 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 true, 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>(16, 0.75f, false) { + @Override + protected boolean removeEldestEntry(Map.Entry> 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); } /** diff --git a/plugins/tiles/src/main/java/org/apache/tiles/core/definition/pattern/AbstractPatternDefinitionResolver.java b/plugins/tiles/src/main/java/org/apache/tiles/core/definition/pattern/AbstractPatternDefinitionResolver.java index a960ec9bf2..0d51e1c776 100644 --- a/plugins/tiles/src/main/java/org/apache/tiles/core/definition/pattern/AbstractPatternDefinitionResolver.java +++ b/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 implements PatternDef /** * Stores patterns depending on the locale they refer to. */ - private final Map> localePatternPaths = new HashMap<>(); + private final Map> 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); + } } diff --git a/plugins/tiles/src/main/java/org/apache/tiles/core/definition/pattern/PatternDefinitionResolver.java b/plugins/tiles/src/main/java/org/apache/tiles/core/definition/pattern/PatternDefinitionResolver.java index 959f082b9e..82772bdeb0 100644 --- a/plugins/tiles/src/main/java/org/apache/tiles/core/definition/pattern/PatternDefinitionResolver.java +++ b/plugins/tiles/src/main/java/org/apache/tiles/core/definition/pattern/PatternDefinitionResolver.java @@ -60,4 +60,12 @@ public interface PatternDefinitionResolver { * @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); } diff --git a/plugins/tiles/src/test/java/org/apache/tiles/core/definition/dao/CachingLocaleUrlDefinitionDAOTest.java b/plugins/tiles/src/test/java/org/apache/tiles/core/definition/dao/CachingLocaleUrlDefinitionDAOTest.java index 0485773219..51f5501fff 100644 --- a/plugins/tiles/src/test/java/org/apache/tiles/core/definition/dao/CachingLocaleUrlDefinitionDAOTest.java +++ b/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 sourceURLs = new ArrayList<>(); + sourceURLs.add(url1); + sourceURLs.add(url2); + sourceURLs.add(url3); + definitionDao.setSources(sourceURLs); + definitionDao.setReader(new DigesterDefinitionsReader()); + + List evicted = new ArrayList<>(); + WildcardDefinitionPatternMatcherFactory factory = new WildcardDefinitionPatternMatcherFactory(); + PatternDefinitionResolver recordingResolver = new BasicPatternDefinitionResolver(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); + } } diff --git a/plugins/tiles/src/test/java/org/apache/tiles/core/definition/pattern/AbstractPatternDefinitionResolverTest.java b/plugins/tiles/src/test/java/org/apache/tiles/core/definition/pattern/AbstractPatternDefinitionResolverTest.java index db9c00f6eb..dc46e0cf08 100644 --- a/plugins/tiles/src/test/java/org/apache/tiles/core/definition/pattern/AbstractPatternDefinitionResolverTest.java +++ b/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 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);