Compare commits

..

1 Commits

Author SHA1 Message Date
Lukasz Lenart cc69a45311 WW-5716 fix(tiles): bound the per-locale definition caches
The Tiles definition caches are keyed by the resolved Locale, which by
default derives from the request. Both CachingLocaleUrlDefinitionDAO's
locale2definitionMap and AbstractPatternDefinitionResolver's
localePatternPaths grew without limit and were never reduced for the
lifetime of the web application.

Bound locale2definitionMap with an insertion-order LinkedHashMap capped
at maxCachedLocales (default 1000, configurable via setMaxCachedLocales).
On eviction the DAO removes the same key from the pattern resolver via
the new PatternDefinitionResolver#removePatternPaths, keeping both maps
in lockstep (the resolver's keys are always a subset of the DAO's).
localePatternPaths becomes a ConcurrentHashMap since the DAO now removes
keys off the request thread. Eviction only re-incurs a load, never
changes rendering.

Fixes https://issues.apache.org/jira/browse/WW-5716

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_015huuB72yvZygWEXKUYDAou
2026-09-04 10:28:32 +02:00
5 changed files with 118 additions and 4 deletions
@@ -26,7 +26,6 @@ import org.apache.tiles.request.ApplicationContext;
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<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);
}
/**
@@ -21,9 +21,9 @@ package org.apache.tiles.core.definition.pattern;
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 abstract class AbstractPatternDefinitionResolver<T> implements PatternDef
if (localePatternPaths.get(customizationKey) != null)
localePatternPaths.get(customizationKey).clear();
}
/** {@inheritDoc} */
@Override
public void removePatternPaths(T customizationKey) {
localePatternPaths.remove(customizationKey);
}
}
@@ -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);
}
@@ -34,6 +34,7 @@ import org.apache.tiles.request.ApplicationResource;
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 class CachingLocaleUrlDefinitionDAOTest extends TestCase {
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);
}
}
@@ -79,6 +79,35 @@ public class AbstractPatternDefinitionResolverTest {
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);