From 4d8108e7668f789dd4c509ac4981a688e104e709 Mon Sep 17 00:00:00 2001 From: JCgH4164838Gh792C124B5 <43964333+JCgH4164838Gh792C124B5@users.noreply.github.com> Date: Sun, 30 Jan 2022 20:57:59 -0500 Subject: [PATCH 1/5] Update: - Add support for an optional basic LRU cache for OGNL expressions and OGNL BeanInfo. - Add support for cache limits applying to both normal and LRU caches. For a normal cache the entire cache will flush when the limit is reached. - Add flags to allow switching between normal and LRU caches, and setting the maximum sizes. --- .../opensymphony/xwork2/ognl/OgnlUtil.java | 144 +++++++++++- .../org/apache/struts2/StrutsConstants.java | 50 +++++ .../org/apache/struts2/default.properties | 26 +++ .../xwork2/ognl/OgnlUtilTest.java | 208 ++++++++++++++++++ 4 files changed, 417 insertions(+), 11 deletions(-) diff --git a/core/src/main/java/com/opensymphony/xwork2/ognl/OgnlUtil.java b/core/src/main/java/com/opensymphony/xwork2/ognl/OgnlUtil.java index 007206cb8..c3eb840da 100644 --- a/core/src/main/java/com/opensymphony/xwork2/ognl/OgnlUtil.java +++ b/core/src/main/java/com/opensymphony/xwork2/ognl/OgnlUtil.java @@ -41,6 +41,7 @@ import java.util.*; import java.util.concurrent.ConcurrentHashMap; import java.util.concurrent.ConcurrentMap; import java.util.concurrent.atomic.AtomicBoolean; +import java.util.concurrent.atomic.AtomicInteger; import java.util.regex.Pattern; @@ -57,8 +58,14 @@ public class OgnlUtil { // Flag used to reduce flooding logs with WARNs about using DevMode excluded packages private final AtomicBoolean warnReported = new AtomicBoolean(false); - private final ConcurrentMap expressions = new ConcurrentHashMap<>(); + private final AtomicBoolean useLRUExpressionCache = new AtomicBoolean(false); + private final AtomicBoolean useLRUBeanInfoCache = new AtomicBoolean(false); + private final AtomicInteger expressionsCacheMaxSize = new AtomicInteger(25000); + private final AtomicInteger beanInfoCacheMaxSize = new AtomicInteger(25000); + private final ConcurrentMap expressionsCache = new ConcurrentHashMap<>(); + private final LRUCache expressionsCacheLRU = new LRUCache<>(expressionsCacheMaxSize.get(), 16, 0.75f); private final ConcurrentMap, BeanInfo> beanInfoCache = new ConcurrentHashMap<>(); + private final LRUCache, BeanInfo> beanInfoCacheLRU = new LRUCache<>(beanInfoCacheMaxSize.get(), 16, 0.75f); private TypeConverter defaultConverter; private boolean devMode; @@ -103,6 +110,28 @@ public class OgnlUtil { enableExpressionCache = BooleanUtils.toBoolean(cache); } + @Inject(value = StrutsConstants.STRUTS_OGNL_EXPRESSION_CACHE_MAXSIZE, required = false) + protected void setExpressionsCacheMaxSize(String maxSize) { + expressionsCacheMaxSize.set(Integer.parseInt(maxSize)); + expressionsCacheLRU.setEvictionLimit(expressionsCacheMaxSize.get()); + } + + @Inject(value = StrutsConstants.STRUTS_OGNL_BEANINFO_CACHE_MAXSIZE, required = false) + protected void setBeanInfoCacheMaxSize(String maxSize) { + beanInfoCacheMaxSize.set(Integer.parseInt(maxSize)); + beanInfoCacheLRU.setEvictionLimit(beanInfoCacheMaxSize.get()); + } + + @Inject(value = StrutsConstants.STRUTS_OGNL_EXPRESSION_CACHE_LRU_MODE, required = false) + protected void setUseLRUExpressionCache(String useLRUMode) { + useLRUExpressionCache.set(BooleanUtils.toBoolean(useLRUMode)); + } + + @Inject(value = StrutsConstants.STRUTS_OGNL_BEANINFO_CACHE_LRU_MODE, required = false) + protected void setUseLRUBeanInfoCache(String useLRUMode) { + useLRUBeanInfoCache.set(BooleanUtils.toBoolean(useLRUMode)); + } + @Inject(value = StrutsConstants.STRUTS_OGNL_ENABLE_EVAL_EXPRESSION, required = false) protected void setEnableEvalExpression(String evalExpression) { this.enableEvalExpression = BooleanUtils.toBoolean(evalExpression); @@ -273,7 +302,8 @@ public class OgnlUtil { * @since 2.5.21 */ public void clearExpressionCache() { - expressions.clear(); + expressionsCache.clear(); + expressionsCacheLRU.clear(); } /** @@ -284,7 +314,11 @@ public class OgnlUtil { * @since 2.5.21 */ public int expressionCacheSize() { - return expressions.size(); + if (useLRUExpressionCache.get()) { + return expressionsCacheLRU.size(); + } else { + return expressionsCache.size(); + } } /** @@ -302,6 +336,7 @@ public class OgnlUtil { */ public void clearBeanInfoCache() { beanInfoCache.clear(); + beanInfoCacheLRU.clear(); } /** @@ -312,7 +347,11 @@ public class OgnlUtil { * @since 2.5.21 */ public int beanInfoCacheSize() { - return beanInfoCache.size(); + if (useLRUBeanInfoCache.get()) { + return beanInfoCacheLRU.size(); + } else { + return beanInfoCache.size(); + } } /** @@ -532,11 +571,18 @@ public class OgnlUtil { private Object compileAndExecute(String expression, Map context, OgnlTask task) throws OgnlException { Object tree; if (enableExpressionCache) { - tree = expressions.get(expression); + final Map expressionsCacheRef = (useLRUExpressionCache.get() ? expressionsCacheLRU.backingMapReference() : expressionsCache); + tree = expressionsCacheRef.get(expression); if (tree == null) { tree = Ognl.parseExpression(expression); checkEnableEvalExpression(tree, context); - expressions.putIfAbsent(expression, tree); + expressionsCacheRef.putIfAbsent(expression, tree); + if (! useLRUExpressionCache.get()) { + // If using a standard cache, after attempting to add an expression check cache size against the limit. + if (expressionsCacheRef.size() > expressionsCacheMaxSize.get()) { + expressionsCacheRef.clear(); // Non-LRU cache has exceeded maximum configured size, so flush. + } + } } } else { tree = Ognl.parseExpression(expression); @@ -549,11 +595,18 @@ public class OgnlUtil { private Object compileAndExecuteMethod(String expression, Map context, OgnlTask task) throws OgnlException { Object tree; if (enableExpressionCache) { - tree = expressions.get(expression); + final Map expressionsCacheRef = (useLRUExpressionCache.get() ? expressionsCacheLRU.backingMapReference() : expressionsCache); + tree = expressionsCacheRef.get(expression); if (tree == null) { tree = Ognl.parseExpression(expression); checkSimpleMethod(tree, context); - expressions.putIfAbsent(expression, tree); + expressionsCacheRef.putIfAbsent(expression, tree); + if (! useLRUExpressionCache.get()) { + // If using a standard cache, after attempting to add an expression check cache size against the limit. + if (expressionsCacheRef.size() > expressionsCacheMaxSize.get()) { + expressionsCacheRef.clear(); // Non-LRU cache has exceeded maximum configured size, so flush. + } + } } } else { tree = Ognl.parseExpression(expression); @@ -759,11 +812,18 @@ public class OgnlUtil { * @throws IntrospectionException is thrown if an exception occurs during introspection. */ public BeanInfo getBeanInfo(Class clazz) throws IntrospectionException { - synchronized (beanInfoCache) { - BeanInfo beanInfo = beanInfoCache.get(clazz); + final Map, BeanInfo> beanInfoCacheRef = (useLRUBeanInfoCache.get() ? beanInfoCacheLRU.backingMapReference() : beanInfoCache); + synchronized (beanInfoCacheRef) { + BeanInfo beanInfo = beanInfoCacheRef.get(clazz); if (beanInfo == null) { beanInfo = Introspector.getBeanInfo(clazz, Object.class); - beanInfoCache.putIfAbsent(clazz, beanInfo); + beanInfoCacheRef.putIfAbsent(clazz, beanInfo); + if (! useLRUBeanInfoCache.get()) { + // If using a standard cache, after attempting to add beanInfo check cache size against the limit. + if (beanInfoCacheRef.size() > beanInfoCacheMaxSize.get()) { + beanInfoCacheRef.clear(); // Non-LRU cache has exceeded maximum configured size, so flush. + } + } } return beanInfo; } @@ -822,4 +882,66 @@ public class OgnlUtil { T execute(Object tree) throws OgnlException; } + /** + * A basic LRUCache implementation that utilizes a {@link Collections#synchronizedMap(java.util.Map)} + * backed by a {@link LinkedHashMap}. May be replaced by a more efficient implementation in the future. + * + * @param Key type for the LRUCache + * @param Value type for the LRUCache + */ + protected class LRUCache { + private final Map lruCache; + private final AtomicInteger cacheEvictionLimit = new AtomicInteger(25000); + + public LRUCache(int evictionLimit, int initialCapacity, float loadFactor) { + this.cacheEvictionLimit.set(evictionLimit); + // Access-order mode selected (order mode true in LinkedHashMap constructor). + lruCache = Collections.synchronizedMap (new LinkedHashMap(initialCapacity, loadFactor, true) { + @Override + protected boolean removeEldestEntry(Map.Entry eldest) { + return (this.size() > cacheEvictionLimit.get()); + } + }); + } + + public Value get(Key key) { + return lruCache.get(key); + } + + public void put(Key key, Value value) { + lruCache.put(key, value); + } + + public void putIfAbsent(Key key, Value value) { + lruCache.putIfAbsent(key, value); + } + + public int size() { + return lruCache.size(); + } + + public void clear() { + lruCache.clear(); + } + + /** + * Since the {@link LRUCache} is only intended to be used internally, we can cheat a bit + * and allow access to the backing map directly via its reference. Allows it to be used + * like the other {@link Map} caches within the module. + * + * @return The Map that backs the LRU cache. + */ + public Map backingMapReference() { + return lruCache; + } + + public int getEvictionLimit() { + return this.cacheEvictionLimit.get(); + } + + public void setEvictionLimit(int cacheEvictionLimit) { + this.cacheEvictionLimit.set(cacheEvictionLimit); + } + + } } diff --git a/core/src/main/java/org/apache/struts2/StrutsConstants.java b/core/src/main/java/org/apache/struts2/StrutsConstants.java index a46653d94..ba6ff87ff 100644 --- a/core/src/main/java/org/apache/struts2/StrutsConstants.java +++ b/core/src/main/java/org/apache/struts2/StrutsConstants.java @@ -249,6 +249,31 @@ public final class StrutsConstants { /** Throw RuntimeException when a property is not found, or the evaluation of the expression fails */ public static final String STRUTS_EL_THROW_EXCEPTION = "struts.el.throwExceptionOnFailure"; + /** + * Specifies a maximum number of cached BeanInfo used by OgnlUtility. Not specified/set by default. If + * a positive integer is specified, it will set a limit whose behaviour depends on whether the + * normal (default) cache or optional LRU cache is in place. + * + * For the normal (default) cache, exceeding the maximum will cause the entire cache to flush (clear). + * For the optional LRU cache, once the maximum is reached, the least-recently-used (LRU) entry will be + * removed when a new entry needs to be added (cache is fully-utilized). + * + * @since 2.6 + */ + public static final String STRUTS_OGNL_BEANINFO_CACHE_MAXSIZE = "struts.ognl.beanInfoCacheMaxSize"; + + /** + * Set the cache mode of the BeanInfo cache used by OgnlUtility. A value of true means enable + * least-recently-used (LRU) mode, a value of false (or any non-true value) means to use the + * default cache. + * + * Note: When enabling LRU cache mode you must also set a maximum size (via {@link #STRUTS_OGNL_BEANINFO_CACHE_MAXSIZE}) + * for it to be effective. Otherwise, there is no condition to evict a LRU entry (cache has no limit). + * + * @since 2.6 + */ + public static final String STRUTS_OGNL_BEANINFO_CACHE_LRU_MODE = "struts.ognl.beanInfoCacheLRUMode"; + /** * Logs properties that are not found (very verbose) * @since 2.6 @@ -274,6 +299,31 @@ public final class StrutsConstants { */ public static final String STRUTS_ENABLE_OGNL_EXPRESSION_CACHE = STRUTS_OGNL_ENABLE_EXPRESSION_CACHE; + /** + * Specifies a maximum number of cached parsed OGNL expressions. Not specified/set by default. If + * a positive integer is specified, it will set a limit whose behaviour depends on whether the + * normal (default) cache or optional LRU cache is in place. + * + * For the normal (default) cache, exceeding the maximum will cause the entire cache to flush (clear). + * For the optional LRU cache, once the maximum is reached, the least-recently-used (LRU) entry will be + * removed when a new entry needs to be added (cache is fully-utilized). + * + * @since 2.6 + */ + public static final String STRUTS_OGNL_EXPRESSION_CACHE_MAXSIZE = "struts.ognl.expressionCacheMaxSize"; + + /** + * Set the cache mode of the parsed OGNL expression cache. A value of true means enable + * least-recently-used (LRU) mode, a value of false (or any non-true value) means to use the + * default cache. + * + * Note: When enabling LRU cache mode you must also set a maximum size (via {@link #STRUTS_OGNL_EXPRESSION_CACHE_MAXSIZE}) + * for it to be effective. Otherwise, there is no condition to evict a LRU entry (cache has no limit). + * + * @since 2.6 + */ + public static final String STRUTS_OGNL_EXPRESSION_CACHE_LRU_MODE = "struts.ognl.expressionCacheLRUMode"; + /** * Enables evaluation of OGNL expressions * @since 2.6 diff --git a/core/src/main/resources/org/apache/struts2/default.properties b/core/src/main/resources/org/apache/struts2/default.properties index df4069400..b88b6030d 100644 --- a/core/src/main/resources/org/apache/struts2/default.properties +++ b/core/src/main/resources/org/apache/struts2/default.properties @@ -224,6 +224,32 @@ struts.ognl.logMissingProperties=false ### if the application generates a lot of different expressions struts.ognl.enableExpressionCache=true +### Specify a limit to the number of entries in the OGNL expressionCache. +### For the standard expressionCache mode, when the limit is exceeded the entire cache's +### content will be cleared (can help prevent memory leaks). +### For expressionCacheLRUMode true, the limit will ensure the cache does not exceed +### that size, dropping the oldest (least-recently-used) expressions to add new ones. +### NOTE: If not set, the default is 25000, which may be excessive. +#struts.ognl.expressionCacheMaxSize=1000 + +### Indicates if the OGNL expressionCache should use LRU mode. +### NOTE: When true, make sure to set the expressionCacheMaxSize to a reasonable value +### for your application. Otherwise the default limit will never (practically) be reached. +#struts.ognl.expressionCacheLRUMode=false + +### Specify a limit to the number of entries in the OGNL beanInfoCache. +### For the standard beanInfoCache mode, when the limit is exceeded the entire cache's +### content will be cleared (can help prevent memory leaks). +### For beanInfoCacheLRUMode true, the limit will ensure the cache does not exceed +### that size, dropping the oldest (least-recently-used) expressions to add new ones. +### NOTE: If not set, the default is 25000, which may be excessive. +#struts.ognl.beanInfoCacheMaxSize=1000 + +### Indicates if the OGNL beanInfoCache should use LRU mode. +### NOTE: When true, make sure to set the beanInfoCacheMaxSize to a reasonable value +### for your application. Otherwise the default limit will never (practically) be reached. +#struts.ognl.beanInfoCacheLRUMode=false + ### Indicates if Dispatcher should handle unexpected exceptions by calling sendError() ### or simply rethrow it as a ServletException to allow future processing by other frameworks like Spring Security struts.handle.exception=true diff --git a/core/src/test/java/com/opensymphony/xwork2/ognl/OgnlUtilTest.java b/core/src/test/java/com/opensymphony/xwork2/ognl/OgnlUtilTest.java index 4b2f1ee83..8cebb1670 100644 --- a/core/src/test/java/com/opensymphony/xwork2/ognl/OgnlUtilTest.java +++ b/core/src/test/java/com/opensymphony/xwork2/ognl/OgnlUtilTest.java @@ -129,6 +129,62 @@ public class OgnlUtilTest extends XWorkTestCase { assertSame(expr0, expr2); } + public void testCacheEnabledMaxSize() throws OgnlException { + ognlUtil.setEnableExpressionCache("true"); + ognlUtil.setExpressionsCacheMaxSize("1"); + Object expr0 = ognlUtil.compile("test"); + Object expr2 = ognlUtil.compile("test"); + assertSame(expr0, expr2); + assertEquals("Expression cache size should be at its limit", 1, ognlUtil.expressionCacheSize()); + // Next epxression cached should cause the cache to clear (exceeding maximum sized). + Object expr3 = ognlUtil.compile("test1"); + assertEquals("Expression cache should be empty", 0, ognlUtil.expressionCacheSize()); + Object expr4 = ognlUtil.compile("test1"); + Object expr5 = ognlUtil.compile("test1"); + assertEquals("Expression cache size should still be at its limit", 1, ognlUtil.expressionCacheSize()); + assertNotSame("2nd test expression cache attempt will exceed size and force clear, but somehow they match ?", expr3, expr4); + assertSame(expr4, expr5); + // Next epxression cached should cause the cache to clear (exceeding maximum sized). + Object expr6 = ognlUtil.compile("test"); + assertEquals("Expression cache should be empty", 0, ognlUtil.expressionCacheSize()); + Object expr7 = ognlUtil.compile("test"); + Object expr8 = ognlUtil.compile("test"); + assertNotSame("2nd test expression cache attempt will exceed size and force clear, but somehow they match ?", expr6, expr7); + assertSame(expr7, expr8); + assertEquals("Expression LRU cache size should still be at its limit", 1, ognlUtil.expressionCacheSize()); + assertNotSame("1st test expression identical after ejection from LRU cache ?", expr5, expr0); + ognlUtil.setUseLRUExpressionCache("false"); + } + + public void testLRUCacheEnabled() throws OgnlException { + ognlUtil.setEnableExpressionCache("true"); + ognlUtil.setUseLRUExpressionCache("true"); + Object expr0 = ognlUtil.compile("test"); + Object expr2 = ognlUtil.compile("test"); + assertSame(expr0, expr2); + ognlUtil.setUseLRUExpressionCache("false"); + } + + public void testLRUCacheEnabledMaxSize() throws OgnlException { + ognlUtil.setEnableExpressionCache("true"); + ognlUtil.setUseLRUExpressionCache("true"); + ognlUtil.setExpressionsCacheMaxSize("1"); + Object expr0 = ognlUtil.compile("test"); + Object expr2 = ognlUtil.compile("test"); + assertSame(expr0, expr2); + assertEquals("Expression LRU cache size should be at its limit", 1, ognlUtil.expressionCacheSize()); + Object expr3 = ognlUtil.compile("test1"); + Object expr4 = ognlUtil.compile("test1"); + assertSame(expr3, expr4); + assertEquals("Expression LRU cache size should still be at its limit", 1, ognlUtil.expressionCacheSize()); + Object expr5 = ognlUtil.compile("test"); + Object expr6 = ognlUtil.compile("test"); + assertSame(expr5, expr6); + assertEquals("Expression LRU cache size should still be at its limit", 1, ognlUtil.expressionCacheSize()); + assertNotSame("1st test expression identical after ejection from LRU cache ?", expr5, expr0); + ognlUtil.setUseLRUExpressionCache("false"); + } + public void testExpressionIsCachedIrrespectiveOfItsExecutionStatus() throws OgnlException { Foo foo = new Foo(); OgnlContext context = (OgnlContext) ognlUtil.createDefaultContext(foo); @@ -149,6 +205,29 @@ public class OgnlUtilTest extends XWorkTestCase { } } + public void testExpressionIsLRUCachedIrrespectiveOfItsExecutionStatus() throws OgnlException { + ognlUtil.setEnableExpressionCache("true"); + ognlUtil.setUseLRUExpressionCache("true"); + Foo foo = new Foo(); + OgnlContext context = (OgnlContext) ognlUtil.createDefaultContext(foo); + + // Expression which executes with success + try { + ognlUtil.getValue("@com.opensymphony.xwork2.ognl.OgnlUtilTest@STATIC_FINAL_PUBLIC_ATTRIBUTE", context, foo); + assertEquals("Successfully executed expression must have been cached", ognlUtil.expressionCacheSize(), 1); + } catch (Exception ex) { + fail("Expression execution should have succeeded here. Exception: " + ex); + } + // Expression which executes with failure + try { + ognlUtil.getValue("@com.opensymphony.xwork2.ognl.OgnlUtilTest@STATIC_PRIVATE_ATTRIBUTE", context, foo); + fail("Expression execution should have failed here"); + } catch (Exception ex) { + assertEquals("Expression with failed execution must have been cached nevertheless", ognlUtil.expressionCacheSize(), 2); + } + ognlUtil.setUseLRUExpressionCache("false"); + } + public void testMethodExpressionIsCachedIrrespectiveOfItsExecutionStatus() throws Exception { Foo foo = new Foo(); OgnlContext context = (OgnlContext) ognlUtil.createDefaultContext(foo); @@ -193,6 +272,32 @@ public class OgnlUtilTest extends XWorkTestCase { assertTrue("Expression cache empty after usage ?", ognlUtil.expressionCacheSize() > 0); } + public void testClearExpressionLRUCache() throws OgnlException { + ognlUtil.setEnableExpressionCache("true"); + ognlUtil.setUseLRUExpressionCache("true"); + // Test that the expression cache is functioning as expected. + Object expr0 = ognlUtil.compile("test"); + Object expr1 = ognlUtil.compile("test"); + Object expr2 = ognlUtil.compile("test"); + // Cache in effect, so expr0, expr1, expr2 should be the same. + assertSame(expr0, expr1); + assertSame(expr0, expr2); + assertTrue("Expression cache empty before clear ?", ognlUtil.expressionCacheSize() > 0); + // Clear the Epxression cache and confirm subsequent requests are new. + ognlUtil.clearExpressionCache(); + assertEquals("Expression cache not empty after clear ?", 0, ognlUtil.expressionCacheSize()); + Object expr3 = ognlUtil.compile("test"); + Object expr4 = ognlUtil.compile("test"); + Object expr5 = ognlUtil.compile("test"); + // Cache cleared, expr3 should be a new instance. + assertNotSame(expr0, expr3); + // Cache still in effect, so expr3, expr4, expr5 should be the same. + assertSame(expr3, expr4); + assertSame(expr3, expr5); + assertTrue("Expression cache empty after usage ?", ognlUtil.expressionCacheSize() > 0); + ognlUtil.setUseLRUExpressionCache("false"); + } + public void testClearBeanInfoCache() throws IntrospectionException { final TestBean1 testBean1 = new TestBean1(); final TestBean2 testBean2 = new TestBean2(); @@ -242,6 +347,79 @@ public class OgnlUtilTest extends XWorkTestCase { assertTrue("BeanInfo cache empty after usage ?", ognlUtil.beanInfoCacheSize() > 0); } + public void testBeanInfoCache() throws IntrospectionException { + final TestBean1 testBean1 = new TestBean1(); + final TestBean2 testBean2 = new TestBean2(); + // Test that the BeanInfo cache is functioning as expected. + Object beanInfo1_1 = ognlUtil.getBeanInfo(testBean1); + Object beanInfo1_2 = ognlUtil.getBeanInfo(testBean1); + Object beanInfo1_3 = ognlUtil.getBeanInfo(testBean1); + // Cache in effect, so beanInfo1_1, beanInfo1_2, beanInfo1_3 should be the same. + assertSame(beanInfo1_1, beanInfo1_2); + assertSame(beanInfo1_1, beanInfo1_3); + Object beanInfo2_1 = ognlUtil.getBeanInfo(testBean2); + Object beanInfo2_2 = ognlUtil.getBeanInfo(testBean2); + Object beanInfo2_3 = ognlUtil.getBeanInfo(testBean2); + // Cache in effect, so beanInfo2_1, beanInfo2_2, beanInfo2_3 should be the same. + assertSame(beanInfo2_1, beanInfo2_2); + assertSame(beanInfo2_1, beanInfo2_3); + // BeanInfo for TestBean1 and TestBean2 should always be different. + assertNotSame(beanInfo1_1, beanInfo2_1); + assertTrue("BeanInfo cache empty after usage ?", ognlUtil.beanInfoCacheSize() > 0); + } + + public void testBeanInfoLRUCache() throws IntrospectionException { + ognlUtil.setUseLRUBeanInfoCache("true"); + final TestBean1 testBean1 = new TestBean1(); + final TestBean2 testBean2 = new TestBean2(); + // Test that the BeanInfo cache is functioning as expected. + Object beanInfo1_1 = ognlUtil.getBeanInfo(testBean1); + Object beanInfo1_2 = ognlUtil.getBeanInfo(testBean1); + Object beanInfo1_3 = ognlUtil.getBeanInfo(testBean1); + // Cache in effect, so beanInfo1_1, beanInfo1_2, beanInfo1_3 should be the same. + assertSame(beanInfo1_1, beanInfo1_2); + assertSame(beanInfo1_1, beanInfo1_3); + Object beanInfo2_1 = ognlUtil.getBeanInfo(testBean2); + Object beanInfo2_2 = ognlUtil.getBeanInfo(testBean2); + Object beanInfo2_3 = ognlUtil.getBeanInfo(testBean2); + // Cache in effect, so beanInfo2_1, beanInfo2_2, beanInfo2_3 should be the same. + assertSame(beanInfo2_1, beanInfo2_2); + assertSame(beanInfo2_1, beanInfo2_3); + // BeanInfo for TestBean1 and TestBean2 should always be different. + assertNotSame(beanInfo1_1, beanInfo2_1); + assertTrue("BeanInfo cache empty after usage ?", ognlUtil.beanInfoCacheSize() > 0); + ognlUtil.setUseLRUBeanInfoCache("true"); + } + + public void testBeanInfoLRUCacheLimits() throws IntrospectionException { + ognlUtil.setUseLRUBeanInfoCache("true"); + ognlUtil.setBeanInfoCacheMaxSize("1"); + final TestBean1 testBean1 = new TestBean1(); + final TestBean2 testBean2 = new TestBean2(); + // Test that the BeanInfo cache is functioning as expected. + Object beanInfo1_1 = ognlUtil.getBeanInfo(testBean1); + Object beanInfo1_2 = ognlUtil.getBeanInfo(testBean1); + Object beanInfo1_3 = ognlUtil.getBeanInfo(testBean1); + // Cache in effect, so beanInfo1_1, beanInfo1_2, beanInfo1_3 should be the same. + assertSame(beanInfo1_1, beanInfo1_2); + assertSame(beanInfo1_1, beanInfo1_3); + Object beanInfo2_1 = ognlUtil.getBeanInfo(testBean2); + Object beanInfo2_2 = ognlUtil.getBeanInfo(testBean2); + Object beanInfo2_3 = ognlUtil.getBeanInfo(testBean2); + // Cache in effect, so beanInfo2_1, beanInfo2_2, beanInfo2_3 should be the same. + assertSame(beanInfo2_1, beanInfo2_2); + assertSame(beanInfo2_1, beanInfo2_3); + // BeanInfo for TestBean1 and TestBean2 should always be different. + assertNotSame(beanInfo1_1, beanInfo2_1); + assertTrue("BeanInfo cache empty after usage ?", ognlUtil.beanInfoCacheSize() > 0); + assertEquals("BeanInfo LRU cache size should be at its limit", 1, ognlUtil.beanInfoCacheSize()); + // LRU cache should not contain TestBean1 beaninfo anymore. A new entry should exist in the cache. + Object beanInfo1_4 = ognlUtil.getBeanInfo(testBean1); + assertNotSame("BeanInfo dropped from LRU cache is the same as newly added ?", beanInfo1_1, beanInfo1_4); + ognlUtil.setUseLRUBeanInfoCache("false"); + ognlUtil.setBeanInfoCacheMaxSize(String.valueOf(Integer.MAX_VALUE)); + } + public void testClearRuntimeCache() { // Confirm that no exceptions or failures arise when calling the convenience global clear method. OgnlUtil.clearRuntimeCache(); @@ -1525,6 +1703,36 @@ public class OgnlUtilTest extends XWorkTestCase { } } + public void testOgnlUtilLRUCacheClass() throws OgnlException { + OgnlUtil.LRUCache lruCache = ognlUtil.new LRUCache<>(2, 16, 0.75f); + Map backingMap = lruCache.backingMapReference(); + assertNotNull("Backing Map somehow null ?", backingMap); + assertEquals("Initial evictionLimit did not match initial value", 2, lruCache.getEvictionLimit()); + lruCache.setEvictionLimit(3); + assertEquals("Updated evictionLimit did not match updated value", 3, lruCache.getEvictionLimit()); + String lookupResult = lruCache.get(Integer.valueOf(0)); + assertNull("Lookup of empty cache returned non-null value ?", lookupResult); + lruCache.put(Integer.valueOf(0), "Zero"); + lookupResult = lruCache.get(Integer.valueOf(0)); + assertEquals("Retrieved value does not match put value ?", "Zero", lookupResult); + lruCache.put(Integer.valueOf(1), "One"); + lruCache.put(Integer.valueOf(2), "Two"); + assertEquals("LRU cache not size evictionlimit after adding three values ?", lruCache.getEvictionLimit(), lruCache.size()); + lookupResult = lruCache.get(Integer.valueOf(2)); + assertEquals("Retrieved value does not match put value ?", "Two", lookupResult); + lruCache.put(Integer.valueOf(3), "Three"); + assertEquals("LRU cache not size evictionlimit after adding values ?", lruCache.getEvictionLimit(), lruCache.size()); + lookupResult = lruCache.get(Integer.valueOf(0)); + assertNull("Lookup of value 0 (should have dropped off LRU cache) returned non-null value ?", lookupResult); + lruCache.putIfAbsent(Integer.valueOf(2), "Two"); + lookupResult = lruCache.get(Integer.valueOf(2)); + assertEquals("Retrieved value does not match put value ?", "Two", lookupResult); + assertEquals("LRUCache and backing map size different after puts ?", lruCache.size(), backingMap.size()); + lruCache.clear(); + assertEquals("LRU cache not empty after clear ?", 0, lruCache.size()); + assertEquals("LRUCache and backing map size different after clear ?", lruCache.size(), backingMap.size()); + } + private void reloadTestContainerConfiguration(boolean devMode, boolean allowStaticMethod) { loadConfigurationProviders(new StubConfigurationProvider() { @Override From fbb31ee65b972f2c64fba470f9e88d51aa169a70 Mon Sep 17 00:00:00 2001 From: JCgH4164838Gh792C124B5 <43964333+JCgH4164838Gh792C124B5@users.noreply.github.com> Date: Sun, 6 Mar 2022 21:18:21 -0500 Subject: [PATCH 2/5] Update: - Refactored the cache design to utilize a factory pattern. - Updated unit tests to match refactoring. --- .../config/impl/DefaultConfiguration.java | 6 + .../StrutsDefaultConfigurationProvider.java | 11 ++ .../ognl/DefaultOgnlBeanInfoCacheFactory.java | 43 +++++ .../xwork2/ognl/DefaultOgnlCacheFactory.java | 61 ++++++ .../DefaultOgnlExpressionCacheFactory.java | 43 +++++ .../opensymphony/xwork2/ognl/OgnlCache.java | 41 ++++ .../xwork2/ognl/OgnlCacheFactory.java | 29 +++ .../xwork2/ognl/OgnlDefaultCache.java | 85 +++++++++ .../xwork2/ognl/OgnlLRUCache.java | 87 +++++++++ .../opensymphony/xwork2/ognl/OgnlUtil.java | 177 +++++------------ .../org/apache/struts2/StrutsConstants.java | 12 ++ .../org/apache/struts2/default.properties | 10 +- core/src/main/resources/struts-default.xml | 3 + .../xwork2/ognl/OgnlUtilTest.java | 179 ++++++++++++++++-- 14 files changed, 633 insertions(+), 154 deletions(-) create mode 100644 core/src/main/java/com/opensymphony/xwork2/ognl/DefaultOgnlBeanInfoCacheFactory.java create mode 100644 core/src/main/java/com/opensymphony/xwork2/ognl/DefaultOgnlCacheFactory.java create mode 100644 core/src/main/java/com/opensymphony/xwork2/ognl/DefaultOgnlExpressionCacheFactory.java create mode 100644 core/src/main/java/com/opensymphony/xwork2/ognl/OgnlCache.java create mode 100644 core/src/main/java/com/opensymphony/xwork2/ognl/OgnlCacheFactory.java create mode 100644 core/src/main/java/com/opensymphony/xwork2/ognl/OgnlDefaultCache.java create mode 100644 core/src/main/java/com/opensymphony/xwork2/ognl/OgnlLRUCache.java diff --git a/core/src/main/java/com/opensymphony/xwork2/config/impl/DefaultConfiguration.java b/core/src/main/java/com/opensymphony/xwork2/config/impl/DefaultConfiguration.java index 6db3139da..01c9eee20 100644 --- a/core/src/main/java/com/opensymphony/xwork2/config/impl/DefaultConfiguration.java +++ b/core/src/main/java/com/opensymphony/xwork2/config/impl/DefaultConfiguration.java @@ -28,6 +28,9 @@ import com.opensymphony.xwork2.conversion.*; import com.opensymphony.xwork2.conversion.impl.*; import com.opensymphony.xwork2.factory.*; import com.opensymphony.xwork2.inject.*; +import com.opensymphony.xwork2.ognl.DefaultOgnlBeanInfoCacheFactory; +import com.opensymphony.xwork2.ognl.DefaultOgnlExpressionCacheFactory; +import com.opensymphony.xwork2.ognl.OgnlCacheFactory; import com.opensymphony.xwork2.ognl.OgnlReflectionProvider; import com.opensymphony.xwork2.ognl.OgnlUtil; import com.opensymphony.xwork2.ognl.OgnlValueStackFactory; @@ -283,6 +286,9 @@ public class DefaultConfiguration implements Configuration { builder.factory(ValueSubstitutor.class, EnvsValueSubstitutor.class, Scope.SINGLETON); + builder.factory(OgnlCacheFactory.class, StrutsConstants.STRUTS_OGNL_EXPRESSIONCACHE_FACTORY, DefaultOgnlExpressionCacheFactory.class, Scope.SINGLETON); + builder.factory(OgnlCacheFactory.class, StrutsConstants.STRUTS_OGNL_BEANINFOCACHE_FACTORY, DefaultOgnlBeanInfoCacheFactory.class, Scope.SINGLETON); + builder.constant(StrutsConstants.STRUTS_DEVMODE, "false"); builder.constant(StrutsConstants.STRUTS_OGNL_LOG_MISSING_PROPERTIES, "false"); builder.constant(StrutsConstants.STRUTS_OGNL_ENABLE_EVAL_EXPRESSION, "false"); diff --git a/core/src/main/java/com/opensymphony/xwork2/config/providers/StrutsDefaultConfigurationProvider.java b/core/src/main/java/com/opensymphony/xwork2/config/providers/StrutsDefaultConfigurationProvider.java index 400674d2d..2043970c4 100644 --- a/core/src/main/java/com/opensymphony/xwork2/config/providers/StrutsDefaultConfigurationProvider.java +++ b/core/src/main/java/com/opensymphony/xwork2/config/providers/StrutsDefaultConfigurationProvider.java @@ -91,6 +91,9 @@ import com.opensymphony.xwork2.ognl.accessor.XWorkMapPropertyAccessor; import com.opensymphony.xwork2.ognl.accessor.XWorkMethodAccessor; import com.opensymphony.xwork2.util.CompoundRoot; import com.opensymphony.xwork2.LocalizedTextProvider; +import com.opensymphony.xwork2.ognl.DefaultOgnlBeanInfoCacheFactory; +import com.opensymphony.xwork2.ognl.DefaultOgnlExpressionCacheFactory; +import com.opensymphony.xwork2.ognl.OgnlCacheFactory; import com.opensymphony.xwork2.util.StrutsLocalizedTextProvider; import com.opensymphony.xwork2.util.OgnlTextParser; import com.opensymphony.xwork2.util.PatternMatcher; @@ -127,19 +130,24 @@ import java.util.Set; public class StrutsDefaultConfigurationProvider implements ConfigurationProvider { + @Override public void destroy() { } + @Override public void init(Configuration configuration) throws ConfigurationException { } + @Override public void loadPackages() throws ConfigurationException { } + @Override public boolean needsReload() { return false; } + @Override public void register(ContainerBuilder builder, LocatableProperties props) throws ConfigurationException { @@ -218,6 +226,9 @@ public class StrutsDefaultConfigurationProvider implements ConfigurationProvider , Scope.SINGLETON) .factory(ValueSubstitutor.class, EnvsValueSubstitutor.class, Scope.SINGLETON) + + .factory(OgnlCacheFactory.class, StrutsConstants.STRUTS_OGNL_EXPRESSIONCACHE_FACTORY, DefaultOgnlExpressionCacheFactory.class, Scope.SINGLETON) + .factory(OgnlCacheFactory.class, StrutsConstants.STRUTS_OGNL_BEANINFOCACHE_FACTORY, DefaultOgnlBeanInfoCacheFactory.class, Scope.SINGLETON) ; props.setProperty(StrutsConstants.STRUTS_ENABLE_DYNAMIC_METHOD_INVOCATION, Boolean.FALSE.toString()); diff --git a/core/src/main/java/com/opensymphony/xwork2/ognl/DefaultOgnlBeanInfoCacheFactory.java b/core/src/main/java/com/opensymphony/xwork2/ognl/DefaultOgnlBeanInfoCacheFactory.java new file mode 100644 index 000000000..dd3eb1a44 --- /dev/null +++ b/core/src/main/java/com/opensymphony/xwork2/ognl/DefaultOgnlBeanInfoCacheFactory.java @@ -0,0 +1,43 @@ +/* + * Copyright 2022 Apache Software Foundation. + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ +package com.opensymphony.xwork2.ognl; + +import com.opensymphony.xwork2.inject.Inject; +import org.apache.struts2.StrutsConstants; + +/** + * Default OGNL Cache factory implementation. + * + * Currently used for BeanInfo cache creation. + * + * @param The type for the cache key entries + * @param The type for the cache value entries + */ +public class DefaultOgnlBeanInfoCacheFactory extends DefaultOgnlCacheFactory { + + @Override + @Inject(value = StrutsConstants.STRUTS_OGNL_BEANINFO_CACHE_MAXSIZE, required = false) + protected void setCacheMaxSize(String maxSize) { + super.setCacheMaxSize(maxSize); + } + + @Override + @Inject(value = StrutsConstants.STRUTS_OGNL_BEANINFO_CACHE_LRU_MODE, required = false) + protected void setUseLRUCache(String useLRUMode) { + super.setUseLRUCache(useLRUMode); + } + +} diff --git a/core/src/main/java/com/opensymphony/xwork2/ognl/DefaultOgnlCacheFactory.java b/core/src/main/java/com/opensymphony/xwork2/ognl/DefaultOgnlCacheFactory.java new file mode 100644 index 000000000..8bd9a2099 --- /dev/null +++ b/core/src/main/java/com/opensymphony/xwork2/ognl/DefaultOgnlCacheFactory.java @@ -0,0 +1,61 @@ +/* + * Copyright 2022 Apache Software Foundation. + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ +package com.opensymphony.xwork2.ognl; + +import java.util.concurrent.atomic.AtomicBoolean; +import java.util.concurrent.atomic.AtomicInteger; +import org.apache.commons.lang3.BooleanUtils; + +/** + * Default OGNL Cache factory implementation. + * + * Currently used for Expression cache and BeanInfo cache creation. + * + * @param The type for the cache key entries + * @param The type for the cache value entries + */ +public class DefaultOgnlCacheFactory implements OgnlCacheFactory { + + private final AtomicBoolean useLRUCache = new AtomicBoolean(false); + private final AtomicInteger cacheMaxSize = new AtomicInteger(25000); + + @Override + public OgnlCache buildOgnlCache(int evictionLimit, int initialCapacity, float loadFactor, boolean lruCache) { + if (lruCache) { + return new OgnlLRUCache<>(evictionLimit, initialCapacity, loadFactor); + } else { + return new OgnlDefaultCache<>(evictionLimit, initialCapacity, loadFactor); + } + } + + @Override + public int getCacheMaxSize() { + return cacheMaxSize.get(); + } + + protected void setCacheMaxSize(String maxSize) { + cacheMaxSize.set(Integer.parseInt(maxSize)); + } + + @Override + public boolean getUseLRUCache() { + return useLRUCache.get(); + } + + protected void setUseLRUCache(String useLRUMode) { + useLRUCache.set(BooleanUtils.toBoolean(useLRUMode)); + } +} diff --git a/core/src/main/java/com/opensymphony/xwork2/ognl/DefaultOgnlExpressionCacheFactory.java b/core/src/main/java/com/opensymphony/xwork2/ognl/DefaultOgnlExpressionCacheFactory.java new file mode 100644 index 000000000..ff623b333 --- /dev/null +++ b/core/src/main/java/com/opensymphony/xwork2/ognl/DefaultOgnlExpressionCacheFactory.java @@ -0,0 +1,43 @@ +/* + * Copyright 2022 Apache Software Foundation. + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ +package com.opensymphony.xwork2.ognl; + +import com.opensymphony.xwork2.inject.Inject; +import org.apache.struts2.StrutsConstants; + +/** + * Default OGNL Expression Cache factory implementation. + * + * Currently used for Expression cache creation. + * + * @param The type for the cache key entries + * @param The type for the cache value entries + */ +public class DefaultOgnlExpressionCacheFactory extends DefaultOgnlCacheFactory { + + @Override + @Inject(value = StrutsConstants.STRUTS_OGNL_EXPRESSION_CACHE_MAXSIZE, required = false) + protected void setCacheMaxSize(String maxSize) { + super.setCacheMaxSize(maxSize); + } + + @Override + @Inject(value = StrutsConstants.STRUTS_OGNL_EXPRESSION_CACHE_LRU_MODE, required = false) + protected void setUseLRUCache(String useLRUMode) { + super.setUseLRUCache(useLRUMode); + } + +} diff --git a/core/src/main/java/com/opensymphony/xwork2/ognl/OgnlCache.java b/core/src/main/java/com/opensymphony/xwork2/ognl/OgnlCache.java new file mode 100644 index 000000000..83893c153 --- /dev/null +++ b/core/src/main/java/com/opensymphony/xwork2/ognl/OgnlCache.java @@ -0,0 +1,41 @@ +/* + * Copyright 2022 Apache Software Foundation. + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ +package com.opensymphony.xwork2.ognl; + +/** + * A basic cache interface for use with OGNL processing (such as Expression, BeanInfo). + * All OGNL caches will have an eviction limit, but setting an extremely high value can + * simulate an "effectively unlimited" cache. + * + * @param The type for the cache key entries + * @param The type for the cache value entries + */ +public interface OgnlCache { + + public Value get(Key key); + + public void put(Key key, Value value); + + public void putIfAbsent(Key key, Value value); + + public int size(); + + public void clear(); + + public int getEvictionLimit(); + + public void setEvictionLimit(int cacheEvictionLimit); +} diff --git a/core/src/main/java/com/opensymphony/xwork2/ognl/OgnlCacheFactory.java b/core/src/main/java/com/opensymphony/xwork2/ognl/OgnlCacheFactory.java new file mode 100644 index 000000000..eabb1a955 --- /dev/null +++ b/core/src/main/java/com/opensymphony/xwork2/ognl/OgnlCacheFactory.java @@ -0,0 +1,29 @@ +/* + * Copyright 2022 Apache Software Foundation. + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ +package com.opensymphony.xwork2.ognl; + +/** + * Used by {@link com.opensymphony.xwork2.ognl.OgnlUtil} to create appropriate OGNL + * caches based on configuration. + * + * @param The type for the cache key entries + * @param The type for the cache value entries + */ +public interface OgnlCacheFactory { + OgnlCache buildOgnlCache(int evictionLimit, int initialCapacity, float loadFactor, boolean lruCache); + int getCacheMaxSize(); + boolean getUseLRUCache(); +} diff --git a/core/src/main/java/com/opensymphony/xwork2/ognl/OgnlDefaultCache.java b/core/src/main/java/com/opensymphony/xwork2/ognl/OgnlDefaultCache.java new file mode 100644 index 000000000..20431e133 --- /dev/null +++ b/core/src/main/java/com/opensymphony/xwork2/ognl/OgnlDefaultCache.java @@ -0,0 +1,85 @@ +/* + * Copyright 2022 Apache Software Foundation. + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ +package com.opensymphony.xwork2.ognl; + +import java.util.concurrent.ConcurrentHashMap; +import java.util.concurrent.atomic.AtomicInteger; + +/** + * Default OGNL cache implementation. + * + * Setting a very high eviction limit simulates an unlimited cache. + * Setting too low an eviction limit will make the cache ineffective. + * + * @param The type for the cache key entries + * @param The type for the cache value entries + */ +public class OgnlDefaultCache implements OgnlCache { + + private final ConcurrentHashMap ognlCache; + private final AtomicInteger cacheEvictionLimit = new AtomicInteger(25000); + + public OgnlDefaultCache(int evictionLimit, int initialCapacity, float loadFactor) { + this.cacheEvictionLimit.set(evictionLimit); + ognlCache = new ConcurrentHashMap<>(initialCapacity, loadFactor); + } + + @Override + public Value get(Key key) { + return ognlCache.get(key); + } + + @Override + public void put(Key key, Value value) { + ognlCache.put(key, value); + this.clearIfEvictionLimitExceeded(); + } + + @Override + public void putIfAbsent(Key key, Value value) { + ognlCache.putIfAbsent(key, value); + this.clearIfEvictionLimitExceeded(); + } + + @Override + public int size() { + return ognlCache.size(); + } + + @Override + public void clear() { + ognlCache.clear(); + } + + @Override + public int getEvictionLimit() { + return this.cacheEvictionLimit.get(); + } + + @Override + public void setEvictionLimit(int cacheEvictionLimit) { + this.cacheEvictionLimit.set(cacheEvictionLimit); + } + + /** + * Clear the cache if the eviction limit has been exceeded. + */ + private void clearIfEvictionLimitExceeded() { + if (ognlCache.size() > cacheEvictionLimit.get()) { + ognlCache.clear(); + } + } +} diff --git a/core/src/main/java/com/opensymphony/xwork2/ognl/OgnlLRUCache.java b/core/src/main/java/com/opensymphony/xwork2/ognl/OgnlLRUCache.java new file mode 100644 index 000000000..a99adca2a --- /dev/null +++ b/core/src/main/java/com/opensymphony/xwork2/ognl/OgnlLRUCache.java @@ -0,0 +1,87 @@ +/* + * Copyright 2022 Apache Software Foundation. + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ +package com.opensymphony.xwork2.ognl; + +import java.util.Collections; +import java.util.LinkedHashMap; +import java.util.Map; +import java.util.concurrent.atomic.AtomicInteger; + +/** + * A basic OGNL LRU cache implementation. + * + * The implementation utilizes a {@link Collections#synchronizedMap(java.util.Map)} + * backed by a {@link LinkedHashMap}. May be replaced by a more efficient implementation in the future. + * + * Setting too low an eviction limit will produce more overhead than value. + * Setting too high an eviction limit may also produce more overhead than value. + * An appropriate eviction limit will need to be determined on an individual application basis. + * + * @param The type for the cache key entries + * @param The type for the cache value entries + */ +public class OgnlLRUCache implements OgnlCache { + + private final Map ognlLRUCache; + private final AtomicInteger cacheEvictionLimit = new AtomicInteger(2500); + + public OgnlLRUCache(int evictionLimit, int initialCapacity, float loadFactor) { + this.cacheEvictionLimit.set(evictionLimit); + // Access-order mode selected (order mode true in LinkedHashMap constructor). + ognlLRUCache = Collections.synchronizedMap (new LinkedHashMap(initialCapacity, loadFactor, true) { + @Override + protected boolean removeEldestEntry(Map.Entry eldest) { + return (this.size() > cacheEvictionLimit.get()); + } + }); + } + + @Override + public Value get(Key key) { + return ognlLRUCache.get(key); + } + + @Override + public void put(Key key, Value value) { + ognlLRUCache.put(key, value); + } + + @Override + public void putIfAbsent(Key key, Value value) { + ognlLRUCache.putIfAbsent(key, value); + } + + @Override + public int size() { + return ognlLRUCache.size(); + } + + @Override + public void clear() { + ognlLRUCache.clear(); + } + + @Override + public int getEvictionLimit() { + return this.cacheEvictionLimit.get(); + } + + @Override + public void setEvictionLimit(int cacheEvictionLimit) { + this.cacheEvictionLimit.set(cacheEvictionLimit); + } + +} diff --git a/core/src/main/java/com/opensymphony/xwork2/ognl/OgnlUtil.java b/core/src/main/java/com/opensymphony/xwork2/ognl/OgnlUtil.java index c3eb840da..6854926f5 100644 --- a/core/src/main/java/com/opensymphony/xwork2/ognl/OgnlUtil.java +++ b/core/src/main/java/com/opensymphony/xwork2/ognl/OgnlUtil.java @@ -38,10 +38,7 @@ import java.beans.Introspector; import java.beans.PropertyDescriptor; import java.lang.reflect.Method; import java.util.*; -import java.util.concurrent.ConcurrentHashMap; -import java.util.concurrent.ConcurrentMap; import java.util.concurrent.atomic.AtomicBoolean; -import java.util.concurrent.atomic.AtomicInteger; import java.util.regex.Pattern; @@ -58,14 +55,10 @@ public class OgnlUtil { // Flag used to reduce flooding logs with WARNs about using DevMode excluded packages private final AtomicBoolean warnReported = new AtomicBoolean(false); - private final AtomicBoolean useLRUExpressionCache = new AtomicBoolean(false); - private final AtomicBoolean useLRUBeanInfoCache = new AtomicBoolean(false); - private final AtomicInteger expressionsCacheMaxSize = new AtomicInteger(25000); - private final AtomicInteger beanInfoCacheMaxSize = new AtomicInteger(25000); - private final ConcurrentMap expressionsCache = new ConcurrentHashMap<>(); - private final LRUCache expressionsCacheLRU = new LRUCache<>(expressionsCacheMaxSize.get(), 16, 0.75f); - private final ConcurrentMap, BeanInfo> beanInfoCache = new ConcurrentHashMap<>(); - private final LRUCache, BeanInfo> beanInfoCacheLRU = new LRUCache<>(beanInfoCacheMaxSize.get(), 16, 0.75f); + private final OgnlCacheFactory ognlExpressionCacheFactory; + private final OgnlCacheFactory, BeanInfo> ognlBeanInfoCacheFactory; + private final OgnlCache expressionCache; + private final OgnlCache, BeanInfo> beanInfoCache; private TypeConverter defaultConverter; private boolean devMode; @@ -85,7 +78,25 @@ public class OgnlUtil { private boolean allowStaticMethodAccess; private boolean disallowProxyMemberAccess; + /** + * Construct a new OgnlUtil instance for use with the framework + */ public OgnlUtil() { + this(null, null); // Instantiate default Expression and BeanInfo caches (null factories) + } + + /** + * Construct a new OgnlUtil instance for use with the framework, with optional + * cache factories for Ognl Expression and BeanInfo caches. + * + * @param ognlExpressionCacheFactory factory for Expression cache instance. If null, use default + * @param ognlBeanInfoCacheFactory for BeanInfo cache instance. If null, use default + */ + @Inject + public OgnlUtil( + @Inject(value = StrutsConstants.STRUTS_OGNL_EXPRESSIONCACHE_FACTORY, required = false) OgnlCacheFactory ognlExpressionCacheFactory, + @Inject(value = StrutsConstants.STRUTS_OGNL_BEANINFOCACHE_FACTORY, required = false) OgnlCacheFactory, BeanInfo> ognlBeanInfoCacheFactory + ) { excludedClasses = Collections.unmodifiableSet(new HashSet<>()); excludedPackageNamePatterns = Collections.unmodifiableSet(new HashSet<>()); excludedPackageNames = Collections.unmodifiableSet(new HashSet<>()); @@ -93,6 +104,19 @@ public class OgnlUtil { devModeExcludedClasses = Collections.unmodifiableSet(new HashSet<>()); devModeExcludedPackageNamePatterns = Collections.unmodifiableSet(new HashSet<>()); devModeExcludedPackageNames = Collections.unmodifiableSet(new HashSet<>()); + this.ognlExpressionCacheFactory = ognlExpressionCacheFactory; + this.ognlBeanInfoCacheFactory = ognlBeanInfoCacheFactory; + + if (ognlExpressionCacheFactory != null) { + this.expressionCache = ognlExpressionCacheFactory.buildOgnlCache(ognlExpressionCacheFactory.getCacheMaxSize(), 16, 0.75f, ognlExpressionCacheFactory.getUseLRUCache()); + } else { + this.expressionCache = new OgnlDefaultCache<>(25000, 16, 0.75f); + } + if (ognlBeanInfoCacheFactory != null) { + this.beanInfoCache = ognlBeanInfoCacheFactory.buildOgnlCache(ognlBeanInfoCacheFactory.getCacheMaxSize(), 16, 0.75f, ognlBeanInfoCacheFactory.getUseLRUCache()); + } else { + this.beanInfoCache = new OgnlDefaultCache<>(25000, 16, 0.75f); + } } @Inject @@ -111,25 +135,13 @@ public class OgnlUtil { } @Inject(value = StrutsConstants.STRUTS_OGNL_EXPRESSION_CACHE_MAXSIZE, required = false) - protected void setExpressionsCacheMaxSize(String maxSize) { - expressionsCacheMaxSize.set(Integer.parseInt(maxSize)); - expressionsCacheLRU.setEvictionLimit(expressionsCacheMaxSize.get()); + protected void setExpressionCacheMaxSize(String maxSize) { + expressionCache.setEvictionLimit(Integer.parseInt(maxSize)); } @Inject(value = StrutsConstants.STRUTS_OGNL_BEANINFO_CACHE_MAXSIZE, required = false) protected void setBeanInfoCacheMaxSize(String maxSize) { - beanInfoCacheMaxSize.set(Integer.parseInt(maxSize)); - beanInfoCacheLRU.setEvictionLimit(beanInfoCacheMaxSize.get()); - } - - @Inject(value = StrutsConstants.STRUTS_OGNL_EXPRESSION_CACHE_LRU_MODE, required = false) - protected void setUseLRUExpressionCache(String useLRUMode) { - useLRUExpressionCache.set(BooleanUtils.toBoolean(useLRUMode)); - } - - @Inject(value = StrutsConstants.STRUTS_OGNL_BEANINFO_CACHE_LRU_MODE, required = false) - protected void setUseLRUBeanInfoCache(String useLRUMode) { - useLRUBeanInfoCache.set(BooleanUtils.toBoolean(useLRUMode)); + beanInfoCache.setEvictionLimit(Integer.parseInt(maxSize)); } @Inject(value = StrutsConstants.STRUTS_OGNL_ENABLE_EVAL_EXPRESSION, required = false) @@ -302,8 +314,7 @@ public class OgnlUtil { * @since 2.5.21 */ public void clearExpressionCache() { - expressionsCache.clear(); - expressionsCacheLRU.clear(); + expressionCache.clear(); } /** @@ -314,11 +325,7 @@ public class OgnlUtil { * @since 2.5.21 */ public int expressionCacheSize() { - if (useLRUExpressionCache.get()) { - return expressionsCacheLRU.size(); - } else { - return expressionsCache.size(); - } + return expressionCache.size(); } /** @@ -336,7 +343,6 @@ public class OgnlUtil { */ public void clearBeanInfoCache() { beanInfoCache.clear(); - beanInfoCacheLRU.clear(); } /** @@ -347,11 +353,7 @@ public class OgnlUtil { * @since 2.5.21 */ public int beanInfoCacheSize() { - if (useLRUBeanInfoCache.get()) { - return beanInfoCacheLRU.size(); - } else { - return beanInfoCache.size(); - } + return beanInfoCache.size(); } /** @@ -571,18 +573,11 @@ public class OgnlUtil { private Object compileAndExecute(String expression, Map context, OgnlTask task) throws OgnlException { Object tree; if (enableExpressionCache) { - final Map expressionsCacheRef = (useLRUExpressionCache.get() ? expressionsCacheLRU.backingMapReference() : expressionsCache); - tree = expressionsCacheRef.get(expression); + tree = expressionCache.get(expression); if (tree == null) { tree = Ognl.parseExpression(expression); checkEnableEvalExpression(tree, context); - expressionsCacheRef.putIfAbsent(expression, tree); - if (! useLRUExpressionCache.get()) { - // If using a standard cache, after attempting to add an expression check cache size against the limit. - if (expressionsCacheRef.size() > expressionsCacheMaxSize.get()) { - expressionsCacheRef.clear(); // Non-LRU cache has exceeded maximum configured size, so flush. - } - } + expressionCache.putIfAbsent(expression, tree); } } else { tree = Ognl.parseExpression(expression); @@ -595,18 +590,11 @@ public class OgnlUtil { private Object compileAndExecuteMethod(String expression, Map context, OgnlTask task) throws OgnlException { Object tree; if (enableExpressionCache) { - final Map expressionsCacheRef = (useLRUExpressionCache.get() ? expressionsCacheLRU.backingMapReference() : expressionsCache); - tree = expressionsCacheRef.get(expression); + tree = expressionCache.get(expression); if (tree == null) { tree = Ognl.parseExpression(expression); checkSimpleMethod(tree, context); - expressionsCacheRef.putIfAbsent(expression, tree); - if (! useLRUExpressionCache.get()) { - // If using a standard cache, after attempting to add an expression check cache size against the limit. - if (expressionsCacheRef.size() > expressionsCacheMaxSize.get()) { - expressionsCacheRef.clear(); // Non-LRU cache has exceeded maximum configured size, so flush. - } - } + expressionCache.putIfAbsent(expression, tree); } } else { tree = Ognl.parseExpression(expression); @@ -812,18 +800,11 @@ public class OgnlUtil { * @throws IntrospectionException is thrown if an exception occurs during introspection. */ public BeanInfo getBeanInfo(Class clazz) throws IntrospectionException { - final Map, BeanInfo> beanInfoCacheRef = (useLRUBeanInfoCache.get() ? beanInfoCacheLRU.backingMapReference() : beanInfoCache); - synchronized (beanInfoCacheRef) { - BeanInfo beanInfo = beanInfoCacheRef.get(clazz); + synchronized (beanInfoCache) { + BeanInfo beanInfo = beanInfoCache.get(clazz); if (beanInfo == null) { beanInfo = Introspector.getBeanInfo(clazz, Object.class); - beanInfoCacheRef.putIfAbsent(clazz, beanInfo); - if (! useLRUBeanInfoCache.get()) { - // If using a standard cache, after attempting to add beanInfo check cache size against the limit. - if (beanInfoCacheRef.size() > beanInfoCacheMaxSize.get()) { - beanInfoCacheRef.clear(); // Non-LRU cache has exceeded maximum configured size, so flush. - } - } + beanInfoCache.putIfAbsent(clazz, beanInfo); } return beanInfo; } @@ -882,66 +863,4 @@ public class OgnlUtil { T execute(Object tree) throws OgnlException; } - /** - * A basic LRUCache implementation that utilizes a {@link Collections#synchronizedMap(java.util.Map)} - * backed by a {@link LinkedHashMap}. May be replaced by a more efficient implementation in the future. - * - * @param Key type for the LRUCache - * @param Value type for the LRUCache - */ - protected class LRUCache { - private final Map lruCache; - private final AtomicInteger cacheEvictionLimit = new AtomicInteger(25000); - - public LRUCache(int evictionLimit, int initialCapacity, float loadFactor) { - this.cacheEvictionLimit.set(evictionLimit); - // Access-order mode selected (order mode true in LinkedHashMap constructor). - lruCache = Collections.synchronizedMap (new LinkedHashMap(initialCapacity, loadFactor, true) { - @Override - protected boolean removeEldestEntry(Map.Entry eldest) { - return (this.size() > cacheEvictionLimit.get()); - } - }); - } - - public Value get(Key key) { - return lruCache.get(key); - } - - public void put(Key key, Value value) { - lruCache.put(key, value); - } - - public void putIfAbsent(Key key, Value value) { - lruCache.putIfAbsent(key, value); - } - - public int size() { - return lruCache.size(); - } - - public void clear() { - lruCache.clear(); - } - - /** - * Since the {@link LRUCache} is only intended to be used internally, we can cheat a bit - * and allow access to the backing map directly via its reference. Allows it to be used - * like the other {@link Map} caches within the module. - * - * @return The Map that backs the LRU cache. - */ - public Map backingMapReference() { - return lruCache; - } - - public int getEvictionLimit() { - return this.cacheEvictionLimit.get(); - } - - public void setEvictionLimit(int cacheEvictionLimit) { - this.cacheEvictionLimit.set(cacheEvictionLimit); - } - - } } diff --git a/core/src/main/java/org/apache/struts2/StrutsConstants.java b/core/src/main/java/org/apache/struts2/StrutsConstants.java index ba6ff87ff..84b608077 100644 --- a/core/src/main/java/org/apache/struts2/StrutsConstants.java +++ b/core/src/main/java/org/apache/struts2/StrutsConstants.java @@ -249,6 +249,18 @@ public final class StrutsConstants { /** Throw RuntimeException when a property is not found, or the evaluation of the expression fails */ public static final String STRUTS_EL_THROW_EXCEPTION = "struts.el.throwExceptionOnFailure"; + /** + * Specifies an OGNL expression cache factory implementation. A default implementation is provided, but + * could be replaced by a custom one if desired. + */ + public static final String STRUTS_OGNL_EXPRESSIONCACHE_FACTORY = "struts.ognl.expressionCacheFactory"; + + /** + * Specifies an OGNL BeanInfo cache factory implementation. A default implementation is provided, but + * could be replaced by a custom one if desired. + */ + public static final String STRUTS_OGNL_BEANINFOCACHE_FACTORY = "struts.ognl.beanInfoCacheFactory"; + /** * Specifies a maximum number of cached BeanInfo used by OgnlUtility. Not specified/set by default. If * a positive integer is specified, it will set a limit whose behaviour depends on whether the diff --git a/core/src/main/resources/org/apache/struts2/default.properties b/core/src/main/resources/org/apache/struts2/default.properties index b88b6030d..eef441ebc 100644 --- a/core/src/main/resources/org/apache/struts2/default.properties +++ b/core/src/main/resources/org/apache/struts2/default.properties @@ -230,12 +230,12 @@ struts.ognl.enableExpressionCache=true ### For expressionCacheLRUMode true, the limit will ensure the cache does not exceed ### that size, dropping the oldest (least-recently-used) expressions to add new ones. ### NOTE: If not set, the default is 25000, which may be excessive. -#struts.ognl.expressionCacheMaxSize=1000 +# struts.ognl.expressionCacheMaxSize=1000 ### Indicates if the OGNL expressionCache should use LRU mode. ### NOTE: When true, make sure to set the expressionCacheMaxSize to a reasonable value ### for your application. Otherwise the default limit will never (practically) be reached. -#struts.ognl.expressionCacheLRUMode=false +# struts.ognl.expressionCacheLRUMode=false ### Specify a limit to the number of entries in the OGNL beanInfoCache. ### For the standard beanInfoCache mode, when the limit is exceeded the entire cache's @@ -243,12 +243,12 @@ struts.ognl.enableExpressionCache=true ### For beanInfoCacheLRUMode true, the limit will ensure the cache does not exceed ### that size, dropping the oldest (least-recently-used) expressions to add new ones. ### NOTE: If not set, the default is 25000, which may be excessive. -#struts.ognl.beanInfoCacheMaxSize=1000 +# struts.ognl.beanInfoCacheMaxSize=1000 ### Indicates if the OGNL beanInfoCache should use LRU mode. ### NOTE: When true, make sure to set the beanInfoCacheMaxSize to a reasonable value -### for your application. Otherwise the default limit will never (practically) be reached. -#struts.ognl.beanInfoCacheLRUMode=false +### for your application. Otherwise the default limit will never (practically) be reached. +# struts.ognl.beanInfoCacheLRUMode=false ### Indicates if Dispatcher should handle unexpected exceptions by calling sendError() ### or simply rethrow it as a ServletException to allow future processing by other frameworks like Spring Security diff --git a/core/src/main/resources/struts-default.xml b/core/src/main/resources/struts-default.xml index 9dd8fbfa3..65b89f169 100644 --- a/core/src/main/resources/struts-default.xml +++ b/core/src/main/resources/struts-default.xml @@ -228,6 +228,9 @@ + + + diff --git a/core/src/test/java/com/opensymphony/xwork2/ognl/OgnlUtilTest.java b/core/src/test/java/com/opensymphony/xwork2/ognl/OgnlUtilTest.java index 8cebb1670..9c250ba51 100644 --- a/core/src/test/java/com/opensymphony/xwork2/ognl/OgnlUtilTest.java +++ b/core/src/test/java/com/opensymphony/xwork2/ognl/OgnlUtilTest.java @@ -35,6 +35,7 @@ import com.opensymphony.xwork2.util.Owner; import com.opensymphony.xwork2.util.ValueStack; import com.opensymphony.xwork2.util.location.LocatableProperties; import com.opensymphony.xwork2.util.reflection.ReflectionContextState; +import java.beans.BeanInfo; import ognl.InappropriateExpressionException; import ognl.MethodFailedException; import ognl.NoSuchPropertyException; @@ -131,7 +132,7 @@ public class OgnlUtilTest extends XWorkTestCase { public void testCacheEnabledMaxSize() throws OgnlException { ognlUtil.setEnableExpressionCache("true"); - ognlUtil.setExpressionsCacheMaxSize("1"); + ognlUtil.setExpressionCacheMaxSize("1"); Object expr0 = ognlUtil.compile("test"); Object expr2 = ognlUtil.compile("test"); assertSame(expr0, expr2); @@ -153,22 +154,22 @@ public class OgnlUtilTest extends XWorkTestCase { assertSame(expr7, expr8); assertEquals("Expression LRU cache size should still be at its limit", 1, ognlUtil.expressionCacheSize()); assertNotSame("1st test expression identical after ejection from LRU cache ?", expr5, expr0); - ognlUtil.setUseLRUExpressionCache("false"); } public void testLRUCacheEnabled() throws OgnlException { + // Force usage of LRU cache factories for the OgnlUtil instance + this.ognlUtil = generateOgnlUtilInstanceWithDefaultLRUCacheFactories(); ognlUtil.setEnableExpressionCache("true"); - ognlUtil.setUseLRUExpressionCache("true"); Object expr0 = ognlUtil.compile("test"); Object expr2 = ognlUtil.compile("test"); assertSame(expr0, expr2); - ognlUtil.setUseLRUExpressionCache("false"); } public void testLRUCacheEnabledMaxSize() throws OgnlException { + // Force usage of LRU cache factories for the OgnlUtil instance + this.ognlUtil = generateOgnlUtilInstanceWithDefaultLRUCacheFactories(); ognlUtil.setEnableExpressionCache("true"); - ognlUtil.setUseLRUExpressionCache("true"); - ognlUtil.setExpressionsCacheMaxSize("1"); + ognlUtil.setExpressionCacheMaxSize("1"); Object expr0 = ognlUtil.compile("test"); Object expr2 = ognlUtil.compile("test"); assertSame(expr0, expr2); @@ -182,7 +183,6 @@ public class OgnlUtilTest extends XWorkTestCase { assertSame(expr5, expr6); assertEquals("Expression LRU cache size should still be at its limit", 1, ognlUtil.expressionCacheSize()); assertNotSame("1st test expression identical after ejection from LRU cache ?", expr5, expr0); - ognlUtil.setUseLRUExpressionCache("false"); } public void testExpressionIsCachedIrrespectiveOfItsExecutionStatus() throws OgnlException { @@ -206,8 +206,10 @@ public class OgnlUtilTest extends XWorkTestCase { } public void testExpressionIsLRUCachedIrrespectiveOfItsExecutionStatus() throws OgnlException { + // Force usage of LRU cache factories for the OgnlUtil instance + this.ognlUtil = generateOgnlUtilInstanceWithDefaultLRUCacheFactories(); + ognlUtil.setContainer(container); // Must be explicitly set as the generated OgnlUtil instance has no container ognlUtil.setEnableExpressionCache("true"); - ognlUtil.setUseLRUExpressionCache("true"); Foo foo = new Foo(); OgnlContext context = (OgnlContext) ognlUtil.createDefaultContext(foo); @@ -225,7 +227,6 @@ public class OgnlUtilTest extends XWorkTestCase { } catch (Exception ex) { assertEquals("Expression with failed execution must have been cached nevertheless", ognlUtil.expressionCacheSize(), 2); } - ognlUtil.setUseLRUExpressionCache("false"); } public void testMethodExpressionIsCachedIrrespectiveOfItsExecutionStatus() throws Exception { @@ -273,8 +274,9 @@ public class OgnlUtilTest extends XWorkTestCase { } public void testClearExpressionLRUCache() throws OgnlException { + // Force usage of LRU cache factories for the OgnlUtil instance + this.ognlUtil = generateOgnlUtilInstanceWithDefaultLRUCacheFactories(); ognlUtil.setEnableExpressionCache("true"); - ognlUtil.setUseLRUExpressionCache("true"); // Test that the expression cache is functioning as expected. Object expr0 = ognlUtil.compile("test"); Object expr1 = ognlUtil.compile("test"); @@ -295,7 +297,6 @@ public class OgnlUtilTest extends XWorkTestCase { assertSame(expr3, expr4); assertSame(expr3, expr5); assertTrue("Expression cache empty after usage ?", ognlUtil.expressionCacheSize() > 0); - ognlUtil.setUseLRUExpressionCache("false"); } public void testClearBeanInfoCache() throws IntrospectionException { @@ -369,7 +370,8 @@ public class OgnlUtilTest extends XWorkTestCase { } public void testBeanInfoLRUCache() throws IntrospectionException { - ognlUtil.setUseLRUBeanInfoCache("true"); + // Force usage of LRU cache factories for the OgnlUtil instance + this.ognlUtil = generateOgnlUtilInstanceWithDefaultLRUCacheFactories(); final TestBean1 testBean1 = new TestBean1(); final TestBean2 testBean2 = new TestBean2(); // Test that the BeanInfo cache is functioning as expected. @@ -388,11 +390,11 @@ public class OgnlUtilTest extends XWorkTestCase { // BeanInfo for TestBean1 and TestBean2 should always be different. assertNotSame(beanInfo1_1, beanInfo2_1); assertTrue("BeanInfo cache empty after usage ?", ognlUtil.beanInfoCacheSize() > 0); - ognlUtil.setUseLRUBeanInfoCache("true"); } public void testBeanInfoLRUCacheLimits() throws IntrospectionException { - ognlUtil.setUseLRUBeanInfoCache("true"); + // Force usage of LRU cache factories for the OgnlUtil instance + this.ognlUtil = generateOgnlUtilInstanceWithDefaultLRUCacheFactories(); ognlUtil.setBeanInfoCacheMaxSize("1"); final TestBean1 testBean1 = new TestBean1(); final TestBean2 testBean2 = new TestBean2(); @@ -416,7 +418,6 @@ public class OgnlUtilTest extends XWorkTestCase { // LRU cache should not contain TestBean1 beaninfo anymore. A new entry should exist in the cache. Object beanInfo1_4 = ognlUtil.getBeanInfo(testBean1); assertNotSame("BeanInfo dropped from LRU cache is the same as newly added ?", beanInfo1_1, beanInfo1_4); - ognlUtil.setUseLRUBeanInfoCache("false"); ognlUtil.setBeanInfoCacheMaxSize(String.valueOf(Integer.MAX_VALUE)); } @@ -1312,6 +1313,20 @@ public class OgnlUtilTest extends XWorkTestCase { internalTestOgnlUtilExclusionsImmutable(basicOgnlUtil); } + public void testDefaultOgnlUtilExclusionsAlternateConstructor() { + OgnlUtil basicOgnlUtil = new OgnlUtil(null, null); + + internalTestInitialEmptyOgnlUtilExclusions(basicOgnlUtil); + internalTestOgnlUtilExclusionsImmutable(basicOgnlUtil); + } + + public void testDefaultOgnlUtilExclusionsAlternateConstructorPopulated() { + OgnlUtil basicOgnlUtil = new OgnlUtil(new DefaultOgnlExpressionCacheFactory(), new DefaultOgnlBeanInfoCacheFactory, BeanInfo>()); + + internalTestInitialEmptyOgnlUtilExclusions(basicOgnlUtil); + internalTestOgnlUtilExclusionsImmutable(basicOgnlUtil); + } + public void testOgnlUtilExcludedAdditivity() { Set> excludedClasses; Set excludedPackageNamePatterns; @@ -1675,6 +1690,34 @@ public class OgnlUtilTest extends XWorkTestCase { } } + public void testGetExcludedPackageNamesAlternateConstructor() { + // Getter should return an immutable collection + OgnlUtil util = new OgnlUtil(null, null); + util.setExcludedPackageNames("java.lang,java.awt"); + assertEquals(util.getExcludedPackageNames().size(), 2); + try { + util.getExcludedPackageNames().clear(); + } catch (Exception ex) { + assertTrue(ex instanceof UnsupportedOperationException); + } finally { + assertEquals(util.getExcludedPackageNames().size(), 2); + } + } + + public void testGetExcludedPackageNamesAlternateConstructorPopulated() { + // Getter should return an immutable collection + OgnlUtil util = new OgnlUtil(new DefaultOgnlExpressionCacheFactory(), new DefaultOgnlBeanInfoCacheFactory, BeanInfo>()); + util.setExcludedPackageNames("java.lang,java.awt"); + assertEquals(util.getExcludedPackageNames().size(), 2); + try { + util.getExcludedPackageNames().clear(); + } catch (Exception ex) { + assertTrue(ex instanceof UnsupportedOperationException); + } finally { + assertEquals(util.getExcludedPackageNames().size(), 2); + } + } + public void testGetExcludedClasses() { // Getter should return an immutable collection OgnlUtil util = new OgnlUtil(); @@ -1689,6 +1732,34 @@ public class OgnlUtilTest extends XWorkTestCase { } } + public void testGetExcludedClassesAlternateConstructor() { + // Getter should return an immutable collection + OgnlUtil util = new OgnlUtil(null, null); + util.setExcludedClasses("java.lang.Runtime,java.lang.ProcessBuilder,java.net.URL"); + assertEquals(util.getExcludedClasses().size(), 3); + try { + util.getExcludedClasses().clear(); + } catch (Exception ex) { + assertTrue(ex instanceof UnsupportedOperationException); + } finally { + assertEquals(util.getExcludedClasses().size(), 3); + } + } + + public void testGetExcludedClassesAlternateConstructorPopulated() { + // Getter should return an immutable collection + OgnlUtil util = new OgnlUtil(new DefaultOgnlExpressionCacheFactory(), new DefaultOgnlBeanInfoCacheFactory, BeanInfo>()); + util.setExcludedClasses("java.lang.Runtime,java.lang.ProcessBuilder,java.net.URL"); + assertEquals(util.getExcludedClasses().size(), 3); + try { + util.getExcludedClasses().clear(); + } catch (Exception ex) { + assertTrue(ex instanceof UnsupportedOperationException); + } finally { + assertEquals(util.getExcludedClasses().size(), 3); + } + } + public void testGetExcludedPackageNamePatterns() { // Getter should return an immutable collection OgnlUtil util = new OgnlUtil(); @@ -1703,10 +1774,64 @@ public class OgnlUtilTest extends XWorkTestCase { } } + public void testGetExcludedPackageNamePatternsAlternateConstructor() { + // Getter should return an immutable collection + OgnlUtil util = new OgnlUtil(null, null); + util.setExcludedPackageNamePatterns("java.lang."); + assertEquals(util.getExcludedPackageNamePatterns().size(), 1); + try { + util.getExcludedPackageNamePatterns().clear(); + } catch (Exception ex) { + assertTrue(ex instanceof UnsupportedOperationException); + } finally { + assertEquals(util.getExcludedPackageNamePatterns().size(), 1); + } + } + + public void testGetExcludedPackageNamePatternsAlternateConstructorPopulated() { + // Getter should return an immutable collection + OgnlUtil util = new OgnlUtil(new DefaultOgnlExpressionCacheFactory(), new DefaultOgnlBeanInfoCacheFactory, BeanInfo>()); + util.setExcludedPackageNamePatterns("java.lang."); + assertEquals(util.getExcludedPackageNamePatterns().size(), 1); + try { + util.getExcludedPackageNamePatterns().clear(); + } catch (Exception ex) { + assertTrue(ex instanceof UnsupportedOperationException); + } finally { + assertEquals(util.getExcludedPackageNamePatterns().size(), 1); + } + } + + public void testOgnlUtilDefaultCacheClass() throws OgnlException { + OgnlDefaultCache defaultCache = new OgnlDefaultCache<>(2, 16, 0.75f); + assertEquals("Initial evictionLimit did not match initial value", 2, defaultCache.getEvictionLimit()); + defaultCache.setEvictionLimit(3); + assertEquals("Updated evictionLimit did not match updated value", 3, defaultCache.getEvictionLimit()); + String lookupResult = defaultCache.get(Integer.valueOf(0)); + assertNull("Lookup of empty cache returned non-null value ?", lookupResult); + defaultCache.put(Integer.valueOf(0), "Zero"); + lookupResult = defaultCache.get(Integer.valueOf(0)); + assertEquals("Retrieved value does not match put value ?", "Zero", lookupResult); + defaultCache.put(Integer.valueOf(1), "One"); + defaultCache.put(Integer.valueOf(2), "Two"); + assertEquals("Default cache not size evictionlimit after adding three values ?", defaultCache.getEvictionLimit(), defaultCache.size()); + lookupResult = defaultCache.get(Integer.valueOf(2)); + assertEquals("Retrieved value does not match put value ?", "Two", lookupResult); + defaultCache.put(Integer.valueOf(3), "Three"); + assertEquals("Default cache not size zero after an add that exceeded the evection limit ?", 0, defaultCache.size()); + lookupResult = defaultCache.get(Integer.valueOf(0)); + assertNull("Lookup of value 0 (should have been evicted with everything) returned non-null value ?", lookupResult); + lookupResult = defaultCache.get(Integer.valueOf(3)); + assertNull("Lookup of value 3 (should have been evicted with everything) returned non-null value ?", lookupResult); + defaultCache.putIfAbsent(Integer.valueOf(2), "Two"); + lookupResult = defaultCache.get(Integer.valueOf(2)); + assertEquals("Retrieved value does not match put value ?", "Two", lookupResult); + defaultCache.clear(); + assertEquals("Default cache not empty after clear ?", 0, defaultCache.size()); + } + public void testOgnlUtilLRUCacheClass() throws OgnlException { - OgnlUtil.LRUCache lruCache = ognlUtil.new LRUCache<>(2, 16, 0.75f); - Map backingMap = lruCache.backingMapReference(); - assertNotNull("Backing Map somehow null ?", backingMap); + OgnlLRUCache lruCache = new OgnlLRUCache<>(2, 16, 0.75f); assertEquals("Initial evictionLimit did not match initial value", 2, lruCache.getEvictionLimit()); lruCache.setEvictionLimit(3); assertEquals("Updated evictionLimit did not match updated value", 3, lruCache.getEvictionLimit()); @@ -1727,10 +1852,24 @@ public class OgnlUtilTest extends XWorkTestCase { lruCache.putIfAbsent(Integer.valueOf(2), "Two"); lookupResult = lruCache.get(Integer.valueOf(2)); assertEquals("Retrieved value does not match put value ?", "Two", lookupResult); - assertEquals("LRUCache and backing map size different after puts ?", lruCache.size(), backingMap.size()); lruCache.clear(); assertEquals("LRU cache not empty after clear ?", 0, lruCache.size()); - assertEquals("LRUCache and backing map size different after clear ?", lruCache.size(), backingMap.size()); + } + + /** + * Generate a new OgnlUtil instance (not configured by the {@link ContainerBuilder}) that can be used for + * basic tests, with its Expression and BeanInfo factories set to LRU mode. + * + * @return OgnlUtil instance with LRU enabled Expression and BeanInfo factories + */ + private OgnlUtil generateOgnlUtilInstanceWithDefaultLRUCacheFactories() { + final OgnlUtil result; + final DefaultOgnlCacheFactory expressionFactory = new DefaultOgnlExpressionCacheFactory(); + final DefaultOgnlCacheFactory beanInfoFactory = new DefaultOgnlBeanInfoCacheFactory, BeanInfo>(); + expressionFactory.setUseLRUCache("true"); + beanInfoFactory.setUseLRUCache("true"); + result = new OgnlUtil(expressionFactory, beanInfoFactory); + return result; } private void reloadTestContainerConfiguration(boolean devMode, boolean allowStaticMethod) { From 084c66723d4571c5a7cc3ecab9ab97ca0c519a89 Mon Sep 17 00:00:00 2001 From: JCgH4164838Gh792C124B5 <43964333+JCgH4164838Gh792C124B5@users.noreply.github.com> Date: Sun, 20 Mar 2022 20:39:14 -0400 Subject: [PATCH 3/5] Update: - Implement a no-parameter build method in OgnlCacheFactory. - Update OgnlUtil to use no-parameter cache build method. - Add an additional code coverage test. --- .../xwork2/ognl/DefaultOgnlCacheFactory.java | 5 ++++ .../xwork2/ognl/OgnlCacheFactory.java | 1 + .../opensymphony/xwork2/ognl/OgnlUtil.java | 7 +++-- .../xwork2/ognl/OgnlUtilTest.java | 28 +++++++++++++++++++ 4 files changed, 39 insertions(+), 2 deletions(-) diff --git a/core/src/main/java/com/opensymphony/xwork2/ognl/DefaultOgnlCacheFactory.java b/core/src/main/java/com/opensymphony/xwork2/ognl/DefaultOgnlCacheFactory.java index 8bd9a2099..bc14dcd15 100644 --- a/core/src/main/java/com/opensymphony/xwork2/ognl/DefaultOgnlCacheFactory.java +++ b/core/src/main/java/com/opensymphony/xwork2/ognl/DefaultOgnlCacheFactory.java @@ -32,6 +32,11 @@ public class DefaultOgnlCacheFactory implements OgnlCacheFactory { private final AtomicBoolean useLRUCache = new AtomicBoolean(false); private final AtomicInteger cacheMaxSize = new AtomicInteger(25000); + @Override + public OgnlCache buildOgnlCache() { + return buildOgnlCache(getCacheMaxSize(), 16, 0.75f, getUseLRUCache()); + } + @Override public OgnlCache buildOgnlCache(int evictionLimit, int initialCapacity, float loadFactor, boolean lruCache) { if (lruCache) { diff --git a/core/src/main/java/com/opensymphony/xwork2/ognl/OgnlCacheFactory.java b/core/src/main/java/com/opensymphony/xwork2/ognl/OgnlCacheFactory.java index eabb1a955..a3791dac5 100644 --- a/core/src/main/java/com/opensymphony/xwork2/ognl/OgnlCacheFactory.java +++ b/core/src/main/java/com/opensymphony/xwork2/ognl/OgnlCacheFactory.java @@ -23,6 +23,7 @@ package com.opensymphony.xwork2.ognl; * @param The type for the cache value entries */ public interface OgnlCacheFactory { + OgnlCache buildOgnlCache(); OgnlCache buildOgnlCache(int evictionLimit, int initialCapacity, float loadFactor, boolean lruCache); int getCacheMaxSize(); boolean getUseLRUCache(); diff --git a/core/src/main/java/com/opensymphony/xwork2/ognl/OgnlUtil.java b/core/src/main/java/com/opensymphony/xwork2/ognl/OgnlUtil.java index 6854926f5..cbcc777ee 100644 --- a/core/src/main/java/com/opensymphony/xwork2/ognl/OgnlUtil.java +++ b/core/src/main/java/com/opensymphony/xwork2/ognl/OgnlUtil.java @@ -80,7 +80,10 @@ public class OgnlUtil { /** * Construct a new OgnlUtil instance for use with the framework + * + * @deprecated It is recommended to utilize the {@link OgnlUtil#OgnlUtil(com.opensymphony.xwork2.ognl.OgnlCacheFactory, com.opensymphony.xwork2.ognl.OgnlCacheFactory) method instead. */ + @Deprecated public OgnlUtil() { this(null, null); // Instantiate default Expression and BeanInfo caches (null factories) } @@ -108,12 +111,12 @@ public class OgnlUtil { this.ognlBeanInfoCacheFactory = ognlBeanInfoCacheFactory; if (ognlExpressionCacheFactory != null) { - this.expressionCache = ognlExpressionCacheFactory.buildOgnlCache(ognlExpressionCacheFactory.getCacheMaxSize(), 16, 0.75f, ognlExpressionCacheFactory.getUseLRUCache()); + this.expressionCache = ognlExpressionCacheFactory.buildOgnlCache(); } else { this.expressionCache = new OgnlDefaultCache<>(25000, 16, 0.75f); } if (ognlBeanInfoCacheFactory != null) { - this.beanInfoCache = ognlBeanInfoCacheFactory.buildOgnlCache(ognlBeanInfoCacheFactory.getCacheMaxSize(), 16, 0.75f, ognlBeanInfoCacheFactory.getUseLRUCache()); + this.beanInfoCache = ognlBeanInfoCacheFactory.buildOgnlCache(); } else { this.beanInfoCache = new OgnlDefaultCache<>(25000, 16, 0.75f); } diff --git a/core/src/test/java/com/opensymphony/xwork2/ognl/OgnlUtilTest.java b/core/src/test/java/com/opensymphony/xwork2/ognl/OgnlUtilTest.java index 9c250ba51..0fbd83095 100644 --- a/core/src/test/java/com/opensymphony/xwork2/ognl/OgnlUtilTest.java +++ b/core/src/test/java/com/opensymphony/xwork2/ognl/OgnlUtilTest.java @@ -1856,6 +1856,32 @@ public class OgnlUtilTest extends XWorkTestCase { assertEquals("LRU cache not empty after clear ?", 0, lruCache.size()); } + /** + * Unit test primarily for code coverage + */ + public void testOgnlDefaultCacheFactoryCoverage() { + OgnlCache ognlCache; + DefaultOgnlCacheFactory defaultOgnlCacheFactory = new DefaultOgnlCacheFactory(); + // Normal cache + defaultOgnlCacheFactory.setCacheMaxSize("12"); + defaultOgnlCacheFactory.setUseLRUCache("false"); + ognlCache = defaultOgnlCacheFactory.buildOgnlCache(); + assertNotNull("No param build method result null ?", ognlCache); + assertEquals("Eviction limit for cache mismatches limit for factory ?", 12, ognlCache.getEvictionLimit() ); + ognlCache = defaultOgnlCacheFactory.buildOgnlCache(6, 6, 0.75f, false); + assertNotNull("No param build method result null ?", ognlCache); + assertEquals("Eviction limit for cache mismatches limit for factory ?", 6, ognlCache.getEvictionLimit() ); + // LRU cache + defaultOgnlCacheFactory.setCacheMaxSize("30"); + defaultOgnlCacheFactory.setUseLRUCache("true"); + ognlCache = defaultOgnlCacheFactory.buildOgnlCache(); + assertNotNull("No param build method result null ?", ognlCache); + assertEquals("Eviction limit for cache mismatches limit for factory ?", 30, ognlCache.getEvictionLimit() ); + ognlCache = defaultOgnlCacheFactory.buildOgnlCache(15, 15, 0.75f, false); + assertNotNull("No param build method result null ?", ognlCache); + assertEquals("Eviction limit for cache mismatches limit for factory ?", 15, ognlCache.getEvictionLimit() ); + } + /** * Generate a new OgnlUtil instance (not configured by the {@link ContainerBuilder}) that can be used for * basic tests, with its Expression and BeanInfo factories set to LRU mode. @@ -1867,7 +1893,9 @@ public class OgnlUtilTest extends XWorkTestCase { final DefaultOgnlCacheFactory expressionFactory = new DefaultOgnlExpressionCacheFactory(); final DefaultOgnlCacheFactory beanInfoFactory = new DefaultOgnlBeanInfoCacheFactory, BeanInfo>(); expressionFactory.setUseLRUCache("true"); + expressionFactory.setCacheMaxSize("25"); beanInfoFactory.setUseLRUCache("true"); + beanInfoFactory.setCacheMaxSize("25"); result = new OgnlUtil(expressionFactory, beanInfoFactory); return result; } From 199f35666926b7ccacc91ad516e38e4fdf3d916d Mon Sep 17 00:00:00 2001 From: JCgH4164838Gh792C124B5 <43964333+JCgH4164838Gh792C124B5@users.noreply.github.com> Date: Sun, 1 May 2022 16:25:16 -0400 Subject: [PATCH 4/5] Update: - Added some IDE-recommended annotations and cleanup to some of the modified files. - Applied easier-to-read/differentiate names "ognlExpressionCacheFactory" and "ognlBeanInfoCacheFactory" for the cache factory configuration extension points. - Reorded default configuration factory init for the cache factories (did not help extension override). - Cleanup of parameterized OgnlUtil constructor. - Added extension point aliases to StrutsBeanSelectionProvider. - Added beaninfo for cache factories to ConstantConfig (did not help extension override). - Added cache factory references to default.properties, struts-default.xml. --- .../config/impl/DefaultConfiguration.java | 30 +++++++++++++----- .../StrutsDefaultConfigurationProvider.java | 5 ++- .../opensymphony/xwork2/ognl/OgnlUtil.java | 31 +++++++++---------- .../org/apache/struts2/StrutsConstants.java | 4 +++ .../config/StrutsBeanSelectionProvider.java | 5 +++ .../config/entities/ConstantConfig.java | 28 +++++++++++++++++ .../org/apache/struts2/default.properties | 5 +++ core/src/main/resources/struts-default.xml | 4 +-- 8 files changed, 83 insertions(+), 29 deletions(-) diff --git a/core/src/main/java/com/opensymphony/xwork2/config/impl/DefaultConfiguration.java b/core/src/main/java/com/opensymphony/xwork2/config/impl/DefaultConfiguration.java index 01c9eee20..ea401213d 100644 --- a/core/src/main/java/com/opensymphony/xwork2/config/impl/DefaultConfiguration.java +++ b/core/src/main/java/com/opensymphony/xwork2/config/impl/DefaultConfiguration.java @@ -82,30 +82,37 @@ public class DefaultConfiguration implements Configuration { } + @Override public PackageConfig getPackageConfig(String name) { return packageContexts.get(name); } + @Override public List getUnknownHandlerStack() { return unknownHandlerStack; } + @Override public void setUnknownHandlerStack(List unknownHandlerStack) { this.unknownHandlerStack = unknownHandlerStack; } + @Override public Set getPackageConfigNames() { return packageContexts.keySet(); } + @Override public Map getPackageConfigs() { return packageContexts; } + @Override public Set getLoadedFileNames() { return loadedFileNames; } + @Override public RuntimeConfiguration getRuntimeConfiguration() { return runtimeConfiguration; } @@ -113,10 +120,12 @@ public class DefaultConfiguration implements Configuration { /** * @return the container */ + @Override public Container getContainer() { return container; } + @Override public void addPackageConfig(String name, PackageConfig packageContext) { PackageConfig check = packageContexts.get(name); if (check != null) { @@ -134,6 +143,7 @@ public class DefaultConfiguration implements Configuration { packageContexts.put(name, packageContext); } + @Override public PackageConfig removePackageConfig(String packageName) { return packageContexts.remove(packageName); } @@ -141,11 +151,13 @@ public class DefaultConfiguration implements Configuration { /** * Allows the configuration to clean up any resources used */ + @Override public void destroy() { packageContexts.clear(); loadedFileNames.clear(); } + @Override public void rebuildRuntimeConfiguration() { runtimeConfiguration = buildRuntimeConfiguration(); } @@ -154,10 +166,12 @@ public class DefaultConfiguration implements Configuration { * Calls the ConfigurationProviderFactory.getConfig() to tell it to reload the configuration and then calls * buildRuntimeConfiguration(). * + * @param providers list of ContainerProvider * @return list of package providers * * @throws ConfigurationException in case of any configuration errors */ + @Override public synchronized List reloadContainer(List providers) throws ConfigurationException { packageContexts.clear(); loadedFileNames.clear(); @@ -175,6 +189,7 @@ public class DefaultConfiguration implements Configuration { props.setConstants(builder); builder.factory(Configuration.class, new Factory() { + @Override public Configuration create(Context context) throws Exception { return DefaultConfiguration.this; } @@ -282,13 +297,12 @@ public class DefaultConfiguration implements Configuration { builder.factory(ObjectTypeDeterminer.class, DefaultObjectTypeDeterminer.class, Scope.SINGLETON); builder.factory(PropertyAccessor.class, CompoundRoot.class.getName(), CompoundRootAccessor.class, Scope.SINGLETON); + builder.factory(OgnlCacheFactory.class, "ognlExpressionCacheFactory", DefaultOgnlExpressionCacheFactory.class, Scope.SINGLETON); + builder.factory(OgnlCacheFactory.class, "ognlBeanInfoCacheFactory", DefaultOgnlBeanInfoCacheFactory.class, Scope.SINGLETON); builder.factory(OgnlUtil.class, Scope.SINGLETON); builder.factory(ValueSubstitutor.class, EnvsValueSubstitutor.class, Scope.SINGLETON); - builder.factory(OgnlCacheFactory.class, StrutsConstants.STRUTS_OGNL_EXPRESSIONCACHE_FACTORY, DefaultOgnlExpressionCacheFactory.class, Scope.SINGLETON); - builder.factory(OgnlCacheFactory.class, StrutsConstants.STRUTS_OGNL_BEANINFOCACHE_FACTORY, DefaultOgnlBeanInfoCacheFactory.class, Scope.SINGLETON); - builder.constant(StrutsConstants.STRUTS_DEVMODE, "false"); builder.constant(StrutsConstants.STRUTS_OGNL_LOG_MISSING_PROPERTIES, "false"); builder.constant(StrutsConstants.STRUTS_OGNL_ENABLE_EVAL_EXPRESSION, "false"); @@ -423,10 +437,10 @@ public class DefaultConfiguration implements Configuration { private static class RuntimeConfigurationImpl implements RuntimeConfiguration { - private Map> namespaceActionConfigs; - private Map namespaceActionConfigMatchers; - private NamespaceMatcher namespaceMatcher; - private Map namespaceConfigs; + private final Map> namespaceActionConfigs; + private final Map namespaceActionConfigMatchers; + private final NamespaceMatcher namespaceMatcher; + private final Map namespaceConfigs; public RuntimeConfigurationImpl(Map> namespaceActionConfigs, Map namespaceConfigs, @@ -454,6 +468,7 @@ public class DefaultConfiguration implements Configuration { * @param namespace the namespace for the action or null for the empty namespace, "" * @return the configuration information for action requested */ + @Override public ActionConfig getActionConfig(String namespace, String name) { ActionConfig config = findActionConfigInNamespace(namespace, name); @@ -509,6 +524,7 @@ public class DefaultConfiguration implements Configuration { * * @return a Map of namespace - > Map of ActionConfig objects, with the key being the action name */ + @Override public Map> getActionConfigs() { return namespaceActionConfigs; } diff --git a/core/src/main/java/com/opensymphony/xwork2/config/providers/StrutsDefaultConfigurationProvider.java b/core/src/main/java/com/opensymphony/xwork2/config/providers/StrutsDefaultConfigurationProvider.java index 30551d6b3..d5b6eb0ea 100644 --- a/core/src/main/java/com/opensymphony/xwork2/config/providers/StrutsDefaultConfigurationProvider.java +++ b/core/src/main/java/com/opensymphony/xwork2/config/providers/StrutsDefaultConfigurationProvider.java @@ -216,6 +216,8 @@ public class StrutsDefaultConfigurationProvider implements ConfigurationProvider .factory(TextProviderFactory.class, StrutsTextProviderFactory.class, Scope.SINGLETON) .factory(LocaleProviderFactory.class, DefaultLocaleProviderFactory.class, Scope.SINGLETON) + .factory(OgnlCacheFactory.class, "ognlExpressionCacheFactory", DefaultOgnlExpressionCacheFactory.class, Scope.SINGLETON) + .factory(OgnlCacheFactory.class, "ognlBeanInfoCacheFactory", DefaultOgnlBeanInfoCacheFactory.class, Scope.SINGLETON) .factory(OgnlUtil.class, Scope.SINGLETON) .factory(CollectionConverter.class, Scope.SINGLETON) .factory(ArrayConverter.class, Scope.SINGLETON) @@ -232,9 +234,6 @@ public class StrutsDefaultConfigurationProvider implements ConfigurationProvider .factory(DateFormatter.class, "simpleDateFormatter", SimpleDateFormatAdapter.class, Scope.SINGLETON) .factory(DateFormatter.class, "dateTimeFormatter", DateTimeFormatterAdapter.class, Scope.SINGLETON) - - .factory(OgnlCacheFactory.class, StrutsConstants.STRUTS_OGNL_EXPRESSIONCACHE_FACTORY, DefaultOgnlExpressionCacheFactory.class, Scope.SINGLETON) - .factory(OgnlCacheFactory.class, StrutsConstants.STRUTS_OGNL_BEANINFOCACHE_FACTORY, DefaultOgnlBeanInfoCacheFactory.class, Scope.SINGLETON) ; props.setProperty(StrutsConstants.STRUTS_ENABLE_DYNAMIC_METHOD_INVOCATION, Boolean.FALSE.toString()); diff --git a/core/src/main/java/com/opensymphony/xwork2/ognl/OgnlUtil.java b/core/src/main/java/com/opensymphony/xwork2/ognl/OgnlUtil.java index cbcc777ee..26ed7f4f4 100644 --- a/core/src/main/java/com/opensymphony/xwork2/ognl/OgnlUtil.java +++ b/core/src/main/java/com/opensymphony/xwork2/ognl/OgnlUtil.java @@ -90,15 +90,19 @@ public class OgnlUtil { /** * Construct a new OgnlUtil instance for use with the framework, with optional - * cache factories for Ognl Expression and BeanInfo caches. + * cache factories for OGNL Expression and BeanInfo caches. * - * @param ognlExpressionCacheFactory factory for Expression cache instance. If null, use default - * @param ognlBeanInfoCacheFactory for BeanInfo cache instance. If null, use default + * NOTE: Although the extension points are defined for the optional cache factories, developer-defined overrides do + * do not appear to function at this time (it always appears to instantiate the default factories). + * Construction injectors do not allow the optional flag, so the definitions must be defined. + * + * @param ognlExpressionCacheFactory factory for Expression cache instance. If null, it uses a default + * @param ognlBeanInfoCacheFactory factory for BeanInfo cache instance. If null, it uses a default */ @Inject public OgnlUtil( - @Inject(value = StrutsConstants.STRUTS_OGNL_EXPRESSIONCACHE_FACTORY, required = false) OgnlCacheFactory ognlExpressionCacheFactory, - @Inject(value = StrutsConstants.STRUTS_OGNL_BEANINFOCACHE_FACTORY, required = false) OgnlCacheFactory, BeanInfo> ognlBeanInfoCacheFactory + @Inject(value = "ognlExpressionCacheFactory") OgnlCacheFactory ognlExpressionCacheFactory, + @Inject(value = "ognlBeanInfoCacheFactory") OgnlCacheFactory, BeanInfo> ognlBeanInfoCacheFactory ) { excludedClasses = Collections.unmodifiableSet(new HashSet<>()); excludedPackageNamePatterns = Collections.unmodifiableSet(new HashSet<>()); @@ -107,19 +111,12 @@ public class OgnlUtil { devModeExcludedClasses = Collections.unmodifiableSet(new HashSet<>()); devModeExcludedPackageNamePatterns = Collections.unmodifiableSet(new HashSet<>()); devModeExcludedPackageNames = Collections.unmodifiableSet(new HashSet<>()); - this.ognlExpressionCacheFactory = ognlExpressionCacheFactory; - this.ognlBeanInfoCacheFactory = ognlBeanInfoCacheFactory; - if (ognlExpressionCacheFactory != null) { - this.expressionCache = ognlExpressionCacheFactory.buildOgnlCache(); - } else { - this.expressionCache = new OgnlDefaultCache<>(25000, 16, 0.75f); - } - if (ognlBeanInfoCacheFactory != null) { - this.beanInfoCache = ognlBeanInfoCacheFactory.buildOgnlCache(); - } else { - this.beanInfoCache = new OgnlDefaultCache<>(25000, 16, 0.75f); - } + this.ognlExpressionCacheFactory = (ognlExpressionCacheFactory != null ? ognlExpressionCacheFactory : new DefaultOgnlExpressionCacheFactory<>()); + this.ognlBeanInfoCacheFactory = (ognlBeanInfoCacheFactory != null ? ognlBeanInfoCacheFactory : new DefaultOgnlBeanInfoCacheFactory<>()); + + this.expressionCache = this.ognlExpressionCacheFactory.buildOgnlCache(); + this.beanInfoCache = this.ognlBeanInfoCacheFactory.buildOgnlCache(); } @Inject diff --git a/core/src/main/java/org/apache/struts2/StrutsConstants.java b/core/src/main/java/org/apache/struts2/StrutsConstants.java index a5cf7e0f6..7356d6374 100644 --- a/core/src/main/java/org/apache/struts2/StrutsConstants.java +++ b/core/src/main/java/org/apache/struts2/StrutsConstants.java @@ -256,12 +256,16 @@ public final class StrutsConstants { /** * Specifies an OGNL expression cache factory implementation. A default implementation is provided, but * could be replaced by a custom one if desired. + * + * @since 2.6 */ public static final String STRUTS_OGNL_EXPRESSIONCACHE_FACTORY = "struts.ognl.expressionCacheFactory"; /** * Specifies an OGNL BeanInfo cache factory implementation. A default implementation is provided, but * could be replaced by a custom one if desired. + * + * @since 2.6 */ public static final String STRUTS_OGNL_BEANINFOCACHE_FACTORY = "struts.ognl.beanInfoCacheFactory"; diff --git a/core/src/main/java/org/apache/struts2/config/StrutsBeanSelectionProvider.java b/core/src/main/java/org/apache/struts2/config/StrutsBeanSelectionProvider.java index 69aa9258e..ba0c76f03 100644 --- a/core/src/main/java/org/apache/struts2/config/StrutsBeanSelectionProvider.java +++ b/core/src/main/java/org/apache/struts2/config/StrutsBeanSelectionProvider.java @@ -49,6 +49,7 @@ import com.opensymphony.xwork2.factory.ResultFactory; import com.opensymphony.xwork2.factory.ValidatorFactory; import com.opensymphony.xwork2.inject.ContainerBuilder; import com.opensymphony.xwork2.inject.Scope; +import com.opensymphony.xwork2.ognl.OgnlCacheFactory; import com.opensymphony.xwork2.security.NotExcludedAcceptedPatternsChecker; import com.opensymphony.xwork2.util.PatternMatcher; import com.opensymphony.xwork2.util.TextParser; @@ -366,6 +367,7 @@ import org.apache.struts2.views.util.UrlHelper; */ public class StrutsBeanSelectionProvider extends AbstractBeanSelectionProvider { + @Override public void register(ContainerBuilder builder, LocatableProperties props) { alias(ObjectFactory.class, StrutsConstants.STRUTS_OBJECTFACTORY, builder, props); alias(ActionFactory.class, StrutsConstants.STRUTS_OBJECTFACTORY_ACTIONFACTORY, builder, props); @@ -423,6 +425,9 @@ public class StrutsBeanSelectionProvider extends AbstractBeanSelectionProvider { alias(DateFormatter.class, StrutsConstants.STRUTS_DATE_FORMATTER, builder, props, Scope.SINGLETON); + alias(OgnlCacheFactory.class, StrutsConstants.STRUTS_OGNL_EXPRESSIONCACHE_FACTORY, builder, props, Scope.SINGLETON); + alias(OgnlCacheFactory.class, StrutsConstants.STRUTS_OGNL_BEANINFOCACHE_FACTORY, builder, props, Scope.SINGLETON); + switchDevMode(props); } diff --git a/core/src/main/java/org/apache/struts2/config/entities/ConstantConfig.java b/core/src/main/java/org/apache/struts2/config/entities/ConstantConfig.java index 013608216..2edf33fd1 100644 --- a/core/src/main/java/org/apache/struts2/config/entities/ConstantConfig.java +++ b/core/src/main/java/org/apache/struts2/config/entities/ConstantConfig.java @@ -144,6 +144,8 @@ public class ConstantConfig { private Boolean disallowProxyMemberAccess; private Integer ognlAutoGrowthCollectionLimit; private String staticContentPath; + private BeanConfig expressionCacheFactory; + private BeanConfig beaninfoCacheFactory; protected String beanConfToString(BeanConfig beanConf) { return beanConf == null ? null : beanConf.getName(); @@ -274,6 +276,8 @@ public class ConstantConfig { map.put(StrutsConstants.STRUTS_DISALLOW_PROXY_MEMBER_ACCESS, Objects.toString(disallowProxyMemberAccess, null)); map.put(StrutsConstants.STRUTS_OGNL_AUTO_GROWTH_COLLECTION_LIMIT, Objects.toString(ognlAutoGrowthCollectionLimit, null)); map.put(StrutsConstants.STRUTS_UI_STATIC_CONTENT_PATH, Objects.toString(staticContentPath, StaticContentLoader.DEFAULT_STATIC_CONTENT_PATH)); + map.put(StrutsConstants.STRUTS_OGNL_EXPRESSIONCACHE_FACTORY, beanConfToString(expressionCacheFactory)); + map.put(StrutsConstants.STRUTS_OGNL_BEANINFOCACHE_FACTORY, beanConfToString(beaninfoCacheFactory)); return map; } @@ -1341,4 +1345,28 @@ public class ConstantConfig { public void setStaticContentPath(String staticContentPath) { this.staticContentPath = StaticContentLoader.Validator.validateStaticContentPath(staticContentPath); } + + public BeanConfig getExpressionCacheFactory() { + return expressionCacheFactory; + } + + public void setExpressionCacheFactory(BeanConfig expressionCacheFactory) { + this.expressionCacheFactory = expressionCacheFactory; + } + + public void setExpressionCacheFactory(Class clazz) { + this.expressionCacheFactory = new BeanConfig(clazz, clazz.getName()); + } + + public BeanConfig getBeaninfoCacheFactory() { + return beaninfoCacheFactory; + } + + public void setBeaninfoCacheFactory(BeanConfig beaninfoCacheFactory) { + this.beaninfoCacheFactory = beaninfoCacheFactory; + } + + public void setBeaninfoCacheFactory(Class clazz) { + this.beaninfoCacheFactory = new BeanConfig(clazz, clazz.getName()); + } } diff --git a/core/src/main/resources/org/apache/struts2/default.properties b/core/src/main/resources/org/apache/struts2/default.properties index 74a85f137..4949d6434 100644 --- a/core/src/main/resources/org/apache/struts2/default.properties +++ b/core/src/main/resources/org/apache/struts2/default.properties @@ -229,6 +229,11 @@ struts.ognl.logMissingProperties=false ### if the application generates a lot of different expressions struts.ognl.enableExpressionCache=true +### Specify the OGNL expression cache factory and BeanInfo cache factory to use. +### Currently the default implementations are used, but can be replaced with custom ones if desired. +struts.ognl.expressionCacheFactory=ognlExpressionCacheFactory +struts.ognl.beanInfoCacheFactory=ognlBeanInfoCacheFactory + ### Specify a limit to the number of entries in the OGNL expressionCache. ### For the standard expressionCache mode, when the limit is exceeded the entire cache's ### content will be cleared (can help prevent memory leaks). diff --git a/core/src/main/resources/struts-default.xml b/core/src/main/resources/struts-default.xml index 775b3fbbf..f493fceac 100644 --- a/core/src/main/resources/struts-default.xml +++ b/core/src/main/resources/struts-default.xml @@ -231,8 +231,8 @@ - - + + From 05ca1ff087694ca6e1b99f5d2b12ac13a7198082 Mon Sep 17 00:00:00 2001 From: Lukasz Lenart Date: Mon, 2 May 2022 09:23:31 +0200 Subject: [PATCH 5/5] Ties cache extension points with implementation --- .../config/impl/DefaultConfiguration.java | 7 +++-- .../StrutsDefaultConfigurationProvider.java | 7 +++-- .../xwork2/ognl/BeanInfoCacheFactory.java | 23 +++++++++++++++ .../ognl/DefaultOgnlBeanInfoCacheFactory.java | 7 +++-- .../xwork2/ognl/DefaultOgnlCacheFactory.java | 6 ++-- .../DefaultOgnlExpressionCacheFactory.java | 7 +++-- .../xwork2/ognl/ExpressionCacheFactory.java | 23 +++++++++++++++ .../xwork2/ognl/OgnlCacheFactory.java | 4 +-- .../opensymphony/xwork2/ognl/OgnlUtil.java | 23 +++++++-------- .../org/apache/struts2/StrutsConstants.java | 28 +++++++++---------- .../config/StrutsBeanSelectionProvider.java | 7 +++-- .../config/entities/ConstantConfig.java | 4 +-- .../org/apache/struts2/default.properties | 6 ++-- .../xwork2/ognl/OgnlUtilTest.java | 8 +++--- 14 files changed, 104 insertions(+), 56 deletions(-) create mode 100644 core/src/main/java/com/opensymphony/xwork2/ognl/BeanInfoCacheFactory.java create mode 100644 core/src/main/java/com/opensymphony/xwork2/ognl/ExpressionCacheFactory.java diff --git a/core/src/main/java/com/opensymphony/xwork2/config/impl/DefaultConfiguration.java b/core/src/main/java/com/opensymphony/xwork2/config/impl/DefaultConfiguration.java index ea401213d..91b1b9a33 100644 --- a/core/src/main/java/com/opensymphony/xwork2/config/impl/DefaultConfiguration.java +++ b/core/src/main/java/com/opensymphony/xwork2/config/impl/DefaultConfiguration.java @@ -28,9 +28,10 @@ import com.opensymphony.xwork2.conversion.*; import com.opensymphony.xwork2.conversion.impl.*; import com.opensymphony.xwork2.factory.*; import com.opensymphony.xwork2.inject.*; +import com.opensymphony.xwork2.ognl.BeanInfoCacheFactory; import com.opensymphony.xwork2.ognl.DefaultOgnlBeanInfoCacheFactory; import com.opensymphony.xwork2.ognl.DefaultOgnlExpressionCacheFactory; -import com.opensymphony.xwork2.ognl.OgnlCacheFactory; +import com.opensymphony.xwork2.ognl.ExpressionCacheFactory; import com.opensymphony.xwork2.ognl.OgnlReflectionProvider; import com.opensymphony.xwork2.ognl.OgnlUtil; import com.opensymphony.xwork2.ognl.OgnlValueStackFactory; @@ -297,8 +298,8 @@ public class DefaultConfiguration implements Configuration { builder.factory(ObjectTypeDeterminer.class, DefaultObjectTypeDeterminer.class, Scope.SINGLETON); builder.factory(PropertyAccessor.class, CompoundRoot.class.getName(), CompoundRootAccessor.class, Scope.SINGLETON); - builder.factory(OgnlCacheFactory.class, "ognlExpressionCacheFactory", DefaultOgnlExpressionCacheFactory.class, Scope.SINGLETON); - builder.factory(OgnlCacheFactory.class, "ognlBeanInfoCacheFactory", DefaultOgnlBeanInfoCacheFactory.class, Scope.SINGLETON); + builder.factory(ExpressionCacheFactory.class, "defaultOgnlExpressionCacheFactory", DefaultOgnlExpressionCacheFactory.class, Scope.SINGLETON); + builder.factory(BeanInfoCacheFactory.class, "defaultOgnlBeanInfoCacheFactory", DefaultOgnlBeanInfoCacheFactory.class, Scope.SINGLETON); builder.factory(OgnlUtil.class, Scope.SINGLETON); builder.factory(ValueSubstitutor.class, EnvsValueSubstitutor.class, Scope.SINGLETON); diff --git a/core/src/main/java/com/opensymphony/xwork2/config/providers/StrutsDefaultConfigurationProvider.java b/core/src/main/java/com/opensymphony/xwork2/config/providers/StrutsDefaultConfigurationProvider.java index d5b6eb0ea..6c44c513e 100644 --- a/core/src/main/java/com/opensymphony/xwork2/config/providers/StrutsDefaultConfigurationProvider.java +++ b/core/src/main/java/com/opensymphony/xwork2/config/providers/StrutsDefaultConfigurationProvider.java @@ -26,6 +26,8 @@ import com.opensymphony.xwork2.StrutsTextProviderFactory; import com.opensymphony.xwork2.TextProviderFactory; import com.opensymphony.xwork2.factory.DefaultUnknownHandlerFactory; import com.opensymphony.xwork2.factory.UnknownHandlerFactory; +import com.opensymphony.xwork2.ognl.BeanInfoCacheFactory; +import com.opensymphony.xwork2.ognl.ExpressionCacheFactory; import com.opensymphony.xwork2.ognl.accessor.HttpParametersPropertyAccessor; import com.opensymphony.xwork2.ognl.accessor.ParameterPropertyAccessor; import com.opensymphony.xwork2.security.AcceptedPatternsChecker; @@ -96,7 +98,6 @@ import com.opensymphony.xwork2.util.CompoundRoot; import com.opensymphony.xwork2.LocalizedTextProvider; import com.opensymphony.xwork2.ognl.DefaultOgnlBeanInfoCacheFactory; import com.opensymphony.xwork2.ognl.DefaultOgnlExpressionCacheFactory; -import com.opensymphony.xwork2.ognl.OgnlCacheFactory; import com.opensymphony.xwork2.util.StrutsLocalizedTextProvider; import com.opensymphony.xwork2.util.OgnlTextParser; import com.opensymphony.xwork2.util.PatternMatcher; @@ -216,8 +217,8 @@ public class StrutsDefaultConfigurationProvider implements ConfigurationProvider .factory(TextProviderFactory.class, StrutsTextProviderFactory.class, Scope.SINGLETON) .factory(LocaleProviderFactory.class, DefaultLocaleProviderFactory.class, Scope.SINGLETON) - .factory(OgnlCacheFactory.class, "ognlExpressionCacheFactory", DefaultOgnlExpressionCacheFactory.class, Scope.SINGLETON) - .factory(OgnlCacheFactory.class, "ognlBeanInfoCacheFactory", DefaultOgnlBeanInfoCacheFactory.class, Scope.SINGLETON) + .factory(ExpressionCacheFactory.class, "defaultOgnlExpressionCacheFactory", DefaultOgnlExpressionCacheFactory.class, Scope.SINGLETON) + .factory(BeanInfoCacheFactory.class, "defaultOgnlBeanInfoCacheFactory", DefaultOgnlBeanInfoCacheFactory.class, Scope.SINGLETON) .factory(OgnlUtil.class, Scope.SINGLETON) .factory(CollectionConverter.class, Scope.SINGLETON) .factory(ArrayConverter.class, Scope.SINGLETON) diff --git a/core/src/main/java/com/opensymphony/xwork2/ognl/BeanInfoCacheFactory.java b/core/src/main/java/com/opensymphony/xwork2/ognl/BeanInfoCacheFactory.java new file mode 100644 index 000000000..3ea210001 --- /dev/null +++ b/core/src/main/java/com/opensymphony/xwork2/ognl/BeanInfoCacheFactory.java @@ -0,0 +1,23 @@ +/* + * Copyright 2022 Apache Software Foundation. + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ +package com.opensymphony.xwork2.ognl; + +/** + * A proxy interface to be used with Struts DI mechanism + */ +public interface BeanInfoCacheFactory extends OgnlCacheFactory { + +} diff --git a/core/src/main/java/com/opensymphony/xwork2/ognl/DefaultOgnlBeanInfoCacheFactory.java b/core/src/main/java/com/opensymphony/xwork2/ognl/DefaultOgnlBeanInfoCacheFactory.java index dd3eb1a44..239b6f8ea 100644 --- a/core/src/main/java/com/opensymphony/xwork2/ognl/DefaultOgnlBeanInfoCacheFactory.java +++ b/core/src/main/java/com/opensymphony/xwork2/ognl/DefaultOgnlBeanInfoCacheFactory.java @@ -20,13 +20,14 @@ import org.apache.struts2.StrutsConstants; /** * Default OGNL Cache factory implementation. - * + * * Currently used for BeanInfo cache creation. - * + * * @param The type for the cache key entries * @param The type for the cache value entries */ -public class DefaultOgnlBeanInfoCacheFactory extends DefaultOgnlCacheFactory { +public class DefaultOgnlBeanInfoCacheFactory extends DefaultOgnlCacheFactory + implements BeanInfoCacheFactory { @Override @Inject(value = StrutsConstants.STRUTS_OGNL_BEANINFO_CACHE_MAXSIZE, required = false) diff --git a/core/src/main/java/com/opensymphony/xwork2/ognl/DefaultOgnlCacheFactory.java b/core/src/main/java/com/opensymphony/xwork2/ognl/DefaultOgnlCacheFactory.java index bc14dcd15..dc9f6d664 100644 --- a/core/src/main/java/com/opensymphony/xwork2/ognl/DefaultOgnlCacheFactory.java +++ b/core/src/main/java/com/opensymphony/xwork2/ognl/DefaultOgnlCacheFactory.java @@ -21,13 +21,13 @@ import org.apache.commons.lang3.BooleanUtils; /** * Default OGNL Cache factory implementation. - * + * * Currently used for Expression cache and BeanInfo cache creation. - * + * * @param The type for the cache key entries * @param The type for the cache value entries */ -public class DefaultOgnlCacheFactory implements OgnlCacheFactory { +public class DefaultOgnlCacheFactory implements OgnlCacheFactory { private final AtomicBoolean useLRUCache = new AtomicBoolean(false); private final AtomicInteger cacheMaxSize = new AtomicInteger(25000); diff --git a/core/src/main/java/com/opensymphony/xwork2/ognl/DefaultOgnlExpressionCacheFactory.java b/core/src/main/java/com/opensymphony/xwork2/ognl/DefaultOgnlExpressionCacheFactory.java index ff623b333..5d68f1e61 100644 --- a/core/src/main/java/com/opensymphony/xwork2/ognl/DefaultOgnlExpressionCacheFactory.java +++ b/core/src/main/java/com/opensymphony/xwork2/ognl/DefaultOgnlExpressionCacheFactory.java @@ -20,13 +20,14 @@ import org.apache.struts2.StrutsConstants; /** * Default OGNL Expression Cache factory implementation. - * + * * Currently used for Expression cache creation. - * + * * @param The type for the cache key entries * @param The type for the cache value entries */ -public class DefaultOgnlExpressionCacheFactory extends DefaultOgnlCacheFactory { +public class DefaultOgnlExpressionCacheFactory extends DefaultOgnlCacheFactory + implements ExpressionCacheFactory { @Override @Inject(value = StrutsConstants.STRUTS_OGNL_EXPRESSION_CACHE_MAXSIZE, required = false) diff --git a/core/src/main/java/com/opensymphony/xwork2/ognl/ExpressionCacheFactory.java b/core/src/main/java/com/opensymphony/xwork2/ognl/ExpressionCacheFactory.java new file mode 100644 index 000000000..182a31b88 --- /dev/null +++ b/core/src/main/java/com/opensymphony/xwork2/ognl/ExpressionCacheFactory.java @@ -0,0 +1,23 @@ +/* + * Copyright 2022 Apache Software Foundation. + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ +package com.opensymphony.xwork2.ognl; + +/** + * A proxy interface to be used with Struts DI mechanism + */ +public interface ExpressionCacheFactory extends OgnlCacheFactory { + +} diff --git a/core/src/main/java/com/opensymphony/xwork2/ognl/OgnlCacheFactory.java b/core/src/main/java/com/opensymphony/xwork2/ognl/OgnlCacheFactory.java index a3791dac5..639bccb9c 100644 --- a/core/src/main/java/com/opensymphony/xwork2/ognl/OgnlCacheFactory.java +++ b/core/src/main/java/com/opensymphony/xwork2/ognl/OgnlCacheFactory.java @@ -18,11 +18,11 @@ package com.opensymphony.xwork2.ognl; /** * Used by {@link com.opensymphony.xwork2.ognl.OgnlUtil} to create appropriate OGNL * caches based on configuration. - * + * * @param The type for the cache key entries * @param The type for the cache value entries */ -public interface OgnlCacheFactory { +interface OgnlCacheFactory { OgnlCache buildOgnlCache(); OgnlCache buildOgnlCache(int evictionLimit, int initialCapacity, float loadFactor, boolean lruCache); int getCacheMaxSize(); diff --git a/core/src/main/java/com/opensymphony/xwork2/ognl/OgnlUtil.java b/core/src/main/java/com/opensymphony/xwork2/ognl/OgnlUtil.java index 26ed7f4f4..9079dc6c8 100644 --- a/core/src/main/java/com/opensymphony/xwork2/ognl/OgnlUtil.java +++ b/core/src/main/java/com/opensymphony/xwork2/ognl/OgnlUtil.java @@ -55,8 +55,6 @@ public class OgnlUtil { // Flag used to reduce flooding logs with WARNs about using DevMode excluded packages private final AtomicBoolean warnReported = new AtomicBoolean(false); - private final OgnlCacheFactory ognlExpressionCacheFactory; - private final OgnlCacheFactory, BeanInfo> ognlBeanInfoCacheFactory; private final OgnlCache expressionCache; private final OgnlCache, BeanInfo> beanInfoCache; private TypeConverter defaultConverter; @@ -80,8 +78,8 @@ public class OgnlUtil { /** * Construct a new OgnlUtil instance for use with the framework - * - * @deprecated It is recommended to utilize the {@link OgnlUtil#OgnlUtil(com.opensymphony.xwork2.ognl.OgnlCacheFactory, com.opensymphony.xwork2.ognl.OgnlCacheFactory) method instead. + * + * @deprecated It is recommended to utilize the {@link OgnlUtil#OgnlUtil(com.opensymphony.xwork2.ognl.ExpressionCacheFactory, com.opensymphony.xwork2.ognl.BeanInfoCacheFactory) method instead. */ @Deprecated public OgnlUtil() { @@ -91,18 +89,17 @@ public class OgnlUtil { /** * Construct a new OgnlUtil instance for use with the framework, with optional * cache factories for OGNL Expression and BeanInfo caches. - * + * * NOTE: Although the extension points are defined for the optional cache factories, developer-defined overrides do * do not appear to function at this time (it always appears to instantiate the default factories). * Construction injectors do not allow the optional flag, so the definitions must be defined. - * + * * @param ognlExpressionCacheFactory factory for Expression cache instance. If null, it uses a default * @param ognlBeanInfoCacheFactory factory for BeanInfo cache instance. If null, it uses a default */ - @Inject public OgnlUtil( - @Inject(value = "ognlExpressionCacheFactory") OgnlCacheFactory ognlExpressionCacheFactory, - @Inject(value = "ognlBeanInfoCacheFactory") OgnlCacheFactory, BeanInfo> ognlBeanInfoCacheFactory + @Inject(value = StrutsConstants.STRUTS_OGNL_EXPRESSION_CACHE_FACTORY, required = false) ExpressionCacheFactory ognlExpressionCacheFactory, + @Inject(value = StrutsConstants.STRUTS_OGNL_BEANINFO_CACHE_FACTORY, required = false) BeanInfoCacheFactory, BeanInfo> ognlBeanInfoCacheFactory ) { excludedClasses = Collections.unmodifiableSet(new HashSet<>()); excludedPackageNamePatterns = Collections.unmodifiableSet(new HashSet<>()); @@ -112,11 +109,11 @@ public class OgnlUtil { devModeExcludedPackageNamePatterns = Collections.unmodifiableSet(new HashSet<>()); devModeExcludedPackageNames = Collections.unmodifiableSet(new HashSet<>()); - this.ognlExpressionCacheFactory = (ognlExpressionCacheFactory != null ? ognlExpressionCacheFactory : new DefaultOgnlExpressionCacheFactory<>()); - this.ognlBeanInfoCacheFactory = (ognlBeanInfoCacheFactory != null ? ognlBeanInfoCacheFactory : new DefaultOgnlBeanInfoCacheFactory<>()); + OgnlCacheFactory ognlExpressionCacheFactory1 = (ognlExpressionCacheFactory != null ? ognlExpressionCacheFactory : new DefaultOgnlExpressionCacheFactory<>()); + OgnlCacheFactory, BeanInfo> ognlBeanInfoCacheFactory1 = (ognlBeanInfoCacheFactory != null ? ognlBeanInfoCacheFactory : new DefaultOgnlBeanInfoCacheFactory<>()); - this.expressionCache = this.ognlExpressionCacheFactory.buildOgnlCache(); - this.beanInfoCache = this.ognlBeanInfoCacheFactory.buildOgnlCache(); + this.expressionCache = ognlExpressionCacheFactory1.buildOgnlCache(); + this.beanInfoCache = ognlBeanInfoCacheFactory1.buildOgnlCache(); } @Inject diff --git a/core/src/main/java/org/apache/struts2/StrutsConstants.java b/core/src/main/java/org/apache/struts2/StrutsConstants.java index 7356d6374..d48710374 100644 --- a/core/src/main/java/org/apache/struts2/StrutsConstants.java +++ b/core/src/main/java/org/apache/struts2/StrutsConstants.java @@ -256,28 +256,28 @@ public final class StrutsConstants { /** * Specifies an OGNL expression cache factory implementation. A default implementation is provided, but * could be replaced by a custom one if desired. - * + * * @since 2.6 */ - public static final String STRUTS_OGNL_EXPRESSIONCACHE_FACTORY = "struts.ognl.expressionCacheFactory"; + public static final String STRUTS_OGNL_EXPRESSION_CACHE_FACTORY = "struts.ognl.expressionCacheFactory"; /** * Specifies an OGNL BeanInfo cache factory implementation. A default implementation is provided, but * could be replaced by a custom one if desired. - * + * * @since 2.6 */ - public static final String STRUTS_OGNL_BEANINFOCACHE_FACTORY = "struts.ognl.beanInfoCacheFactory"; + public static final String STRUTS_OGNL_BEANINFO_CACHE_FACTORY = "struts.ognl.beanInfoCacheFactory"; /** * Specifies a maximum number of cached BeanInfo used by OgnlUtility. Not specified/set by default. If * a positive integer is specified, it will set a limit whose behaviour depends on whether the * normal (default) cache or optional LRU cache is in place. - * + * * For the normal (default) cache, exceeding the maximum will cause the entire cache to flush (clear). - * For the optional LRU cache, once the maximum is reached, the least-recently-used (LRU) entry will be + * For the optional LRU cache, once the maximum is reached, the least-recently-used (LRU) entry will be * removed when a new entry needs to be added (cache is fully-utilized). - * + * * @since 2.6 */ public static final String STRUTS_OGNL_BEANINFO_CACHE_MAXSIZE = "struts.ognl.beanInfoCacheMaxSize"; @@ -286,10 +286,10 @@ public final class StrutsConstants { * Set the cache mode of the BeanInfo cache used by OgnlUtility. A value of true means enable * least-recently-used (LRU) mode, a value of false (or any non-true value) means to use the * default cache. - * + * * Note: When enabling LRU cache mode you must also set a maximum size (via {@link #STRUTS_OGNL_BEANINFO_CACHE_MAXSIZE}) * for it to be effective. Otherwise, there is no condition to evict a LRU entry (cache has no limit). - * + * * @since 2.6 */ public static final String STRUTS_OGNL_BEANINFO_CACHE_LRU_MODE = "struts.ognl.beanInfoCacheLRUMode"; @@ -323,11 +323,11 @@ public final class StrutsConstants { * Specifies a maximum number of cached parsed OGNL expressions. Not specified/set by default. If * a positive integer is specified, it will set a limit whose behaviour depends on whether the * normal (default) cache or optional LRU cache is in place. - * + * * For the normal (default) cache, exceeding the maximum will cause the entire cache to flush (clear). - * For the optional LRU cache, once the maximum is reached, the least-recently-used (LRU) entry will be + * For the optional LRU cache, once the maximum is reached, the least-recently-used (LRU) entry will be * removed when a new entry needs to be added (cache is fully-utilized). - * + * * @since 2.6 */ public static final String STRUTS_OGNL_EXPRESSION_CACHE_MAXSIZE = "struts.ognl.expressionCacheMaxSize"; @@ -336,10 +336,10 @@ public final class StrutsConstants { * Set the cache mode of the parsed OGNL expression cache. A value of true means enable * least-recently-used (LRU) mode, a value of false (or any non-true value) means to use the * default cache. - * + * * Note: When enabling LRU cache mode you must also set a maximum size (via {@link #STRUTS_OGNL_EXPRESSION_CACHE_MAXSIZE}) * for it to be effective. Otherwise, there is no condition to evict a LRU entry (cache has no limit). - * + * * @since 2.6 */ public static final String STRUTS_OGNL_EXPRESSION_CACHE_LRU_MODE = "struts.ognl.expressionCacheLRUMode"; diff --git a/core/src/main/java/org/apache/struts2/config/StrutsBeanSelectionProvider.java b/core/src/main/java/org/apache/struts2/config/StrutsBeanSelectionProvider.java index ba0c76f03..f47bbc354 100644 --- a/core/src/main/java/org/apache/struts2/config/StrutsBeanSelectionProvider.java +++ b/core/src/main/java/org/apache/struts2/config/StrutsBeanSelectionProvider.java @@ -23,6 +23,8 @@ import com.opensymphony.xwork2.LocaleProviderFactory; import com.opensymphony.xwork2.LocalizedTextProvider; import com.opensymphony.xwork2.TextProviderFactory; import com.opensymphony.xwork2.factory.UnknownHandlerFactory; +import com.opensymphony.xwork2.ognl.BeanInfoCacheFactory; +import com.opensymphony.xwork2.ognl.ExpressionCacheFactory; import com.opensymphony.xwork2.security.AcceptedPatternsChecker; import com.opensymphony.xwork2.security.ExcludedPatternsChecker; import com.opensymphony.xwork2.FileManager; @@ -49,7 +51,6 @@ import com.opensymphony.xwork2.factory.ResultFactory; import com.opensymphony.xwork2.factory.ValidatorFactory; import com.opensymphony.xwork2.inject.ContainerBuilder; import com.opensymphony.xwork2.inject.Scope; -import com.opensymphony.xwork2.ognl.OgnlCacheFactory; import com.opensymphony.xwork2.security.NotExcludedAcceptedPatternsChecker; import com.opensymphony.xwork2.util.PatternMatcher; import com.opensymphony.xwork2.util.TextParser; @@ -425,8 +426,8 @@ public class StrutsBeanSelectionProvider extends AbstractBeanSelectionProvider { alias(DateFormatter.class, StrutsConstants.STRUTS_DATE_FORMATTER, builder, props, Scope.SINGLETON); - alias(OgnlCacheFactory.class, StrutsConstants.STRUTS_OGNL_EXPRESSIONCACHE_FACTORY, builder, props, Scope.SINGLETON); - alias(OgnlCacheFactory.class, StrutsConstants.STRUTS_OGNL_BEANINFOCACHE_FACTORY, builder, props, Scope.SINGLETON); + alias(ExpressionCacheFactory.class, StrutsConstants.STRUTS_OGNL_EXPRESSION_CACHE_FACTORY, builder, props, Scope.SINGLETON); + alias(BeanInfoCacheFactory.class, StrutsConstants.STRUTS_OGNL_BEANINFO_CACHE_FACTORY, builder, props, Scope.SINGLETON); switchDevMode(props); } diff --git a/core/src/main/java/org/apache/struts2/config/entities/ConstantConfig.java b/core/src/main/java/org/apache/struts2/config/entities/ConstantConfig.java index 2edf33fd1..517cfea28 100644 --- a/core/src/main/java/org/apache/struts2/config/entities/ConstantConfig.java +++ b/core/src/main/java/org/apache/struts2/config/entities/ConstantConfig.java @@ -276,8 +276,8 @@ public class ConstantConfig { map.put(StrutsConstants.STRUTS_DISALLOW_PROXY_MEMBER_ACCESS, Objects.toString(disallowProxyMemberAccess, null)); map.put(StrutsConstants.STRUTS_OGNL_AUTO_GROWTH_COLLECTION_LIMIT, Objects.toString(ognlAutoGrowthCollectionLimit, null)); map.put(StrutsConstants.STRUTS_UI_STATIC_CONTENT_PATH, Objects.toString(staticContentPath, StaticContentLoader.DEFAULT_STATIC_CONTENT_PATH)); - map.put(StrutsConstants.STRUTS_OGNL_EXPRESSIONCACHE_FACTORY, beanConfToString(expressionCacheFactory)); - map.put(StrutsConstants.STRUTS_OGNL_BEANINFOCACHE_FACTORY, beanConfToString(beaninfoCacheFactory)); + map.put(StrutsConstants.STRUTS_OGNL_EXPRESSION_CACHE_FACTORY, beanConfToString(expressionCacheFactory)); + map.put(StrutsConstants.STRUTS_OGNL_BEANINFO_CACHE_FACTORY, beanConfToString(beaninfoCacheFactory)); return map; } diff --git a/core/src/main/resources/org/apache/struts2/default.properties b/core/src/main/resources/org/apache/struts2/default.properties index 4949d6434..b42a64aa2 100644 --- a/core/src/main/resources/org/apache/struts2/default.properties +++ b/core/src/main/resources/org/apache/struts2/default.properties @@ -230,9 +230,9 @@ struts.ognl.logMissingProperties=false struts.ognl.enableExpressionCache=true ### Specify the OGNL expression cache factory and BeanInfo cache factory to use. -### Currently the default implementations are used, but can be replaced with custom ones if desired. -struts.ognl.expressionCacheFactory=ognlExpressionCacheFactory -struts.ognl.beanInfoCacheFactory=ognlBeanInfoCacheFactory +### Currently, the default implementations are used, but can be replaced with custom ones if desired. +struts.ognl.expressionCacheFactory=defaultOgnlExpressionCacheFactory +struts.ognl.beanInfoCacheFactory=defaultOgnlBeanInfoCacheFactory ### Specify a limit to the number of entries in the OGNL expressionCache. ### For the standard expressionCache mode, when the limit is exceeded the entire cache's diff --git a/core/src/test/java/com/opensymphony/xwork2/ognl/OgnlUtilTest.java b/core/src/test/java/com/opensymphony/xwork2/ognl/OgnlUtilTest.java index 0fbd83095..fdc52877f 100644 --- a/core/src/test/java/com/opensymphony/xwork2/ognl/OgnlUtilTest.java +++ b/core/src/test/java/com/opensymphony/xwork2/ognl/OgnlUtilTest.java @@ -1857,7 +1857,7 @@ public class OgnlUtilTest extends XWorkTestCase { } /** - * Unit test primarily for code coverage + * Unit test primarily for code coverage */ public void testOgnlDefaultCacheFactoryCoverage() { OgnlCache ognlCache; @@ -1885,13 +1885,13 @@ public class OgnlUtilTest extends XWorkTestCase { /** * Generate a new OgnlUtil instance (not configured by the {@link ContainerBuilder}) that can be used for * basic tests, with its Expression and BeanInfo factories set to LRU mode. - * + * * @return OgnlUtil instance with LRU enabled Expression and BeanInfo factories */ private OgnlUtil generateOgnlUtilInstanceWithDefaultLRUCacheFactories() { final OgnlUtil result; - final DefaultOgnlCacheFactory expressionFactory = new DefaultOgnlExpressionCacheFactory(); - final DefaultOgnlCacheFactory beanInfoFactory = new DefaultOgnlBeanInfoCacheFactory, BeanInfo>(); + final DefaultOgnlExpressionCacheFactory expressionFactory = new DefaultOgnlExpressionCacheFactory<>(); + final DefaultOgnlBeanInfoCacheFactory, BeanInfo> beanInfoFactory = new DefaultOgnlBeanInfoCacheFactory<>(); expressionFactory.setUseLRUCache("true"); expressionFactory.setCacheMaxSize("25"); beanInfoFactory.setUseLRUCache("true");