From 74d2fdcc6cd7563eb87d475e92fd2a7edb6f3d02 Mon Sep 17 00:00:00 2001 From: Kusal Kithul-Godage Date: Fri, 13 Oct 2023 21:56:25 +1100 Subject: [PATCH 01/18] WW-5355 Use LRU cache by default --- .../xwork2/ognl/OgnlDefaultCache.java | 4 +- .../xwork2/ognl/OgnlLRUCache.java | 6 +- .../opensymphony/xwork2/ognl/OgnlUtil.java | 55 +++++++++++-------- .../org/apache/struts2/default.properties | 8 +-- 4 files changed, 40 insertions(+), 33 deletions(-) diff --git a/core/src/main/java/com/opensymphony/xwork2/ognl/OgnlDefaultCache.java b/core/src/main/java/com/opensymphony/xwork2/ognl/OgnlDefaultCache.java index a32736da67..43b24bb18c 100644 --- a/core/src/main/java/com/opensymphony/xwork2/ognl/OgnlDefaultCache.java +++ b/core/src/main/java/com/opensymphony/xwork2/ognl/OgnlDefaultCache.java @@ -30,10 +30,10 @@ public class OgnlDefaultCache implements OgnlCache { private final ConcurrentHashMap ognlCache; - private final AtomicInteger cacheEvictionLimit = new AtomicInteger(25000); + private final AtomicInteger cacheEvictionLimit = new AtomicInteger(); public OgnlDefaultCache(int evictionLimit, int initialCapacity, float loadFactor) { - this.cacheEvictionLimit.set(evictionLimit); + cacheEvictionLimit.set(evictionLimit); ognlCache = new ConcurrentHashMap<>(initialCapacity, loadFactor); } diff --git a/core/src/main/java/com/opensymphony/xwork2/ognl/OgnlLRUCache.java b/core/src/main/java/com/opensymphony/xwork2/ognl/OgnlLRUCache.java index 93ab56d363..81286af349 100644 --- a/core/src/main/java/com/opensymphony/xwork2/ognl/OgnlLRUCache.java +++ b/core/src/main/java/com/opensymphony/xwork2/ognl/OgnlLRUCache.java @@ -36,15 +36,15 @@ public class OgnlLRUCache implements OgnlCache { private final Map ognlLRUCache; - private final AtomicInteger cacheEvictionLimit = new AtomicInteger(2500); + private final AtomicInteger cacheEvictionLimit = new AtomicInteger(); public OgnlLRUCache(int evictionLimit, int initialCapacity, float loadFactor) { - this.cacheEvictionLimit.set(evictionLimit); + 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()); + return size() > cacheEvictionLimit.get(); } }); } 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 4bddad63ae..16c80d99d1 100644 --- a/core/src/main/java/com/opensymphony/xwork2/ognl/OgnlUtil.java +++ b/core/src/main/java/com/opensymphony/xwork2/ognl/OgnlUtil.java @@ -673,9 +673,15 @@ public void copy(final Object from, final Object to, final Map c * note if exclusions AND inclusions are supplied and not null nothing will get copied. * @param editable the class (or interface) to restrict property setting to */ - public void copy(final Object from, final Object to, final Map context, Collection exclusions, Collection inclusions, Class editable) { + public void copy(final Object from, + final Object to, + final Map context, + Collection exclusions, + Collection inclusions, + Class editable) { if (from == null || to == null) { - LOG.warn("Attempting to copy from or to a null source. This is illegal and is bein skipped. This may be due to an error in an OGNL expression, action chaining, or some other event."); + LOG.warn( + "Skipping attempt to copy from, or to, a null source.", new RuntimeException()); return; } @@ -689,8 +695,7 @@ public void copy(final Object from, final Object to, final Map c fromPds = getPropertyDescriptors(from); if (editable != null) { toPds = getPropertyDescriptors(editable); - } - else { + } else { toPds = getPropertyDescriptors(to); } } catch (IntrospectionException e) { @@ -705,29 +710,31 @@ public void copy(final Object from, final Object to, final Map c } for (PropertyDescriptor fromPd : fromPds) { - if (fromPd.getReadMethod() != null) { - boolean copy = true; - if (exclusions != null && exclusions.contains(fromPd.getName())) { - copy = false; - } else if (inclusions != null && !inclusions.contains(fromPd.getName())) { - copy = false; - } - - if (copy) { - PropertyDescriptor toPd = toPdHash.get(fromPd.getName()); - if ((toPd != null) && (toPd.getWriteMethod() != null)) { - try { - Object value = ognlGet(fromPd.getName(), contextFrom, from, null, context, this::checkEnableEvalExpression); - ognlSet(fromPd.getName(), contextTo, to, value, context); - } catch (OgnlException e) { - LOG.debug("Got OGNL exception", e); - } - } + if (fromPd.getReadMethod() == null) { + continue; + } - } + if (exclusions != null && exclusions.contains(fromPd.getName()) || + inclusions != null && !inclusions.contains(fromPd.getName())) { + continue; + } + PropertyDescriptor toPd = toPdHash.get(fromPd.getName()); + if (toPd == null || toPd.getWriteMethod() == null) { + continue; } + try { + Object value = ognlGet(fromPd.getName(), + contextFrom, + from, + null, + context, + this::checkEnableEvalExpression); + ognlSet(fromPd.getName(), contextTo, to, value, context); + } catch (OgnlException e) { + LOG.debug("Got OGNL exception", e); + } } } @@ -746,7 +753,7 @@ public void copy(Object from, Object to, Map context) { } /** - * Get's the java beans property descriptors for the given source. + * Gets the java beans property descriptors for the given source. * * @param source the source object. * @return property descriptors. diff --git a/core/src/main/resources/org/apache/struts2/default.properties b/core/src/main/resources/org/apache/struts2/default.properties index 56bc714de2..60257465b2 100644 --- a/core/src/main/resources/org/apache/struts2/default.properties +++ b/core/src/main/resources/org/apache/struts2/default.properties @@ -243,12 +243,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=10000 ### 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=true ### 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 @@ -256,12 +256,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=5000 ### 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 +struts.ognl.beanInfoCacheLRUMode=true ### 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 From 5011a797774824a23d87d2da8bf1a3dc3a276865 Mon Sep 17 00:00:00 2001 From: Kusal Kithul-Godage Date: Sun, 15 Oct 2023 13:49:24 +1100 Subject: [PATCH 02/18] WW-5355 Prevent AtomicInteger being initialised to zero --- .../java/com/opensymphony/xwork2/ognl/OgnlDefaultCache.java | 4 ++-- .../main/java/com/opensymphony/xwork2/ognl/OgnlLRUCache.java | 4 ++-- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/core/src/main/java/com/opensymphony/xwork2/ognl/OgnlDefaultCache.java b/core/src/main/java/com/opensymphony/xwork2/ognl/OgnlDefaultCache.java index 43b24bb18c..b76984a6b8 100644 --- a/core/src/main/java/com/opensymphony/xwork2/ognl/OgnlDefaultCache.java +++ b/core/src/main/java/com/opensymphony/xwork2/ognl/OgnlDefaultCache.java @@ -30,10 +30,10 @@ public class OgnlDefaultCache implements OgnlCache { private final ConcurrentHashMap ognlCache; - private final AtomicInteger cacheEvictionLimit = new AtomicInteger(); + private final AtomicInteger cacheEvictionLimit; public OgnlDefaultCache(int evictionLimit, int initialCapacity, float loadFactor) { - cacheEvictionLimit.set(evictionLimit); + cacheEvictionLimit = new AtomicInteger(evictionLimit); ognlCache = new ConcurrentHashMap<>(initialCapacity, loadFactor); } diff --git a/core/src/main/java/com/opensymphony/xwork2/ognl/OgnlLRUCache.java b/core/src/main/java/com/opensymphony/xwork2/ognl/OgnlLRUCache.java index 81286af349..4df1963886 100644 --- a/core/src/main/java/com/opensymphony/xwork2/ognl/OgnlLRUCache.java +++ b/core/src/main/java/com/opensymphony/xwork2/ognl/OgnlLRUCache.java @@ -36,10 +36,10 @@ public class OgnlLRUCache implements OgnlCache { private final Map ognlLRUCache; - private final AtomicInteger cacheEvictionLimit = new AtomicInteger(); + private final AtomicInteger cacheEvictionLimit; public OgnlLRUCache(int evictionLimit, int initialCapacity, float loadFactor) { - cacheEvictionLimit.set(evictionLimit); + cacheEvictionLimit = new AtomicInteger(evictionLimit); // Access-order mode selected (order mode true in LinkedHashMap constructor). ognlLRUCache = Collections.synchronizedMap (new LinkedHashMap(initialCapacity, loadFactor, true) { @Override From 9527da5d3603de3ea7423e44c0eb213b5f02cce1 Mon Sep 17 00:00:00 2001 From: Kusal Kithul-Godage Date: Sun, 15 Oct 2023 14:51:15 +1100 Subject: [PATCH 03/18] WW-5355 Initial Caffeine cache implementation --- core/pom.xml | 5 ++ .../xwork2/ognl/OgnlCaffeineCache.java | 82 +++++++++++++++++++ pom.xml | 6 ++ 3 files changed, 93 insertions(+) create mode 100644 core/src/main/java/com/opensymphony/xwork2/ognl/OgnlCaffeineCache.java diff --git a/core/pom.xml b/core/pom.xml index ea2f9ec5d4..26634e2d6a 100644 --- a/core/pom.xml +++ b/core/pom.xml @@ -184,6 +184,11 @@ freemarker + + com.github.ben-manes.caffeine + caffeine + + javax.servlet javax.servlet-api diff --git a/core/src/main/java/com/opensymphony/xwork2/ognl/OgnlCaffeineCache.java b/core/src/main/java/com/opensymphony/xwork2/ognl/OgnlCaffeineCache.java new file mode 100644 index 0000000000..7bb5f69c7f --- /dev/null +++ b/core/src/main/java/com/opensymphony/xwork2/ognl/OgnlCaffeineCache.java @@ -0,0 +1,82 @@ +/* + * 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.github.benmanes.caffeine.cache.Cache; +import com.github.benmanes.caffeine.cache.Caffeine; + +/** + *

This OGNL Cache implementation is backed by {@link Caffeine} which uses the Window TinyLfu algorithm.

+ * + *

An appropriate eviction limit should be chosen for your specific application based on factors and requirements + * such as:

+ *
    + *
  • Quantity and complexity of actions
  • + *
  • Volume of requests
  • + *
  • Rate limits and attack potential/patterns
  • + *
  • Memory constraints
  • + *
+ * + * @param The type for the cache key entries + * @param The type for the cache value entries + */ +public class OgnlCaffeineCache implements OgnlCache { + + private final Cache cache; + private final int evictionLimit; + + public OgnlCaffeineCache(int evictionLimit, int initialCapacity, float loadFactor) { + this.evictionLimit = evictionLimit; + this.cache = Caffeine.newBuilder().initialCapacity(initialCapacity).maximumSize(evictionLimit).build(); + } + + @Override + public Value get(Key key) { + return cache.getIfPresent(key); + } + + @Override + public void put(Key key, Value value) { + cache.put(key, value); + } + + @Override + public void putIfAbsent(Key key, Value value) { + if (cache.getIfPresent(key) == null) { + cache.put(key, value); + } + } + + @Override + public int size() { + return Math.toIntExact(cache.estimatedSize()); + } + + @Override + public void clear() { + cache.invalidateAll(); + } + + @Override + public int getEvictionLimit() { + return evictionLimit; + } + + @Override + public void setEvictionLimit(int cacheEvictionLimit) { + throw new UnsupportedOperationException("Cannot change eviction limit on a Caffeine cache after initialisation"); + } +} diff --git a/pom.xml b/pom.xml index d0a5aed20e..050a598c4e 100644 --- a/pom.xml +++ b/pom.xml @@ -685,6 +685,12 @@ ${freemarker.version}
+ + com.github.ben-manes.caffeine + caffeine + 3.1.8 + + org.apache.felix org.apache.felix.framework From 1573207ee7bc13927f34c42fb8eb1637bfda6d61 Mon Sep 17 00:00:00 2001 From: Kusal Kithul-Godage Date: Sun, 15 Oct 2023 19:47:35 +1100 Subject: [PATCH 04/18] WW-5355 Fix eviction limit in LRU cache not being enforced --- .../main/java/com/opensymphony/xwork2/ognl/OgnlLRUCache.java | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/core/src/main/java/com/opensymphony/xwork2/ognl/OgnlLRUCache.java b/core/src/main/java/com/opensymphony/xwork2/ognl/OgnlLRUCache.java index 4df1963886..17ff5ab651 100644 --- a/core/src/main/java/com/opensymphony/xwork2/ognl/OgnlLRUCache.java +++ b/core/src/main/java/com/opensymphony/xwork2/ognl/OgnlLRUCache.java @@ -81,7 +81,9 @@ public int getEvictionLimit() { @Override public void setEvictionLimit(int cacheEvictionLimit) { + if (cacheEvictionLimit < size()) { + clear(); + } this.cacheEvictionLimit.set(cacheEvictionLimit); } - } From 6ff7e15bfcea229163e05cb4b9cd15ddef1df7d4 Mon Sep 17 00:00:00 2001 From: Kusal Kithul-Godage Date: Sun, 15 Oct 2023 19:47:58 +1100 Subject: [PATCH 05/18] WW-5355 Update JavaDoc for basic and LRU cache --- .../xwork2/ognl/OgnlDefaultCache.java | 11 +++++++---- .../opensymphony/xwork2/ognl/OgnlLRUCache.java | 18 +++++++++--------- 2 files changed, 16 insertions(+), 13 deletions(-) diff --git a/core/src/main/java/com/opensymphony/xwork2/ognl/OgnlDefaultCache.java b/core/src/main/java/com/opensymphony/xwork2/ognl/OgnlDefaultCache.java index b76984a6b8..ac4335fdbb 100644 --- a/core/src/main/java/com/opensymphony/xwork2/ognl/OgnlDefaultCache.java +++ b/core/src/main/java/com/opensymphony/xwork2/ognl/OgnlDefaultCache.java @@ -19,12 +19,15 @@ import java.util.concurrent.atomic.AtomicInteger; /** - * Default OGNL cache implementation. + *

Basic OGNL cache implementation.

* - * Setting a very high eviction limit simulates an unlimited cache. - * Setting too low an eviction limit will make the cache ineffective. + *

This implementation is backed by a {@link ConcurrentHashMap} that is cleared whenever the eviction limit is + * surpassed.

* - * @param The type for the cache key entries + *

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 { diff --git a/core/src/main/java/com/opensymphony/xwork2/ognl/OgnlLRUCache.java b/core/src/main/java/com/opensymphony/xwork2/ognl/OgnlLRUCache.java index 17ff5ab651..6606c9c680 100644 --- a/core/src/main/java/com/opensymphony/xwork2/ognl/OgnlLRUCache.java +++ b/core/src/main/java/com/opensymphony/xwork2/ognl/OgnlLRUCache.java @@ -21,16 +21,16 @@ import java.util.concurrent.atomic.AtomicInteger; /** - * A basic OGNL LRU cache implementation. + *

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. + *

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. + *

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 key entries * @param The type for the cache value entries */ public class OgnlLRUCache implements OgnlCache { @@ -41,9 +41,9 @@ public class OgnlLRUCache implements OgnlCache { public OgnlLRUCache(int evictionLimit, int initialCapacity, float loadFactor) { cacheEvictionLimit = new AtomicInteger(evictionLimit); // Access-order mode selected (order mode true in LinkedHashMap constructor). - ognlLRUCache = Collections.synchronizedMap (new LinkedHashMap(initialCapacity, loadFactor, true) { + ognlLRUCache = Collections.synchronizedMap(new LinkedHashMap(initialCapacity, loadFactor, true) { @Override - protected boolean removeEldestEntry(Map.Entry eldest) { + protected boolean removeEldestEntry(Map.Entry eldest) { return size() > cacheEvictionLimit.get(); } }); From 9c932f2033ef9d195b42223fd442337e735c240c Mon Sep 17 00:00:00 2001 From: Kusal Kithul-Godage Date: Sun, 15 Oct 2023 23:26:43 +1100 Subject: [PATCH 06/18] WW-5355 Introduce new Struts constants and their defaults --- .../xwork2/ognl/OgnlCacheFactory.java | 5 ++ .../org/apache/struts2/StrutsConstants.java | 54 +++++++++---------- .../org/apache/struts2/default.properties | 34 ++++-------- 3 files changed, 42 insertions(+), 51 deletions(-) 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 639bccb9c2..c3604e5c0b 100644 --- a/core/src/main/java/com/opensymphony/xwork2/ognl/OgnlCacheFactory.java +++ b/core/src/main/java/com/opensymphony/xwork2/ognl/OgnlCacheFactory.java @@ -27,4 +27,9 @@ interface OgnlCacheFactory { OgnlCache buildOgnlCache(int evictionLimit, int initialCapacity, float loadFactor, boolean lruCache); int getCacheMaxSize(); boolean getUseLRUCache(); + enum CacheType { + CONCURRENT_BASIC, + SYNC_LINKED_LRU, + CAFFEINE_WTLFU + } } diff --git a/core/src/main/java/org/apache/struts2/StrutsConstants.java b/core/src/main/java/org/apache/struts2/StrutsConstants.java index b965a0614a..b8a133087b 100644 --- a/core/src/main/java/org/apache/struts2/StrutsConstants.java +++ b/core/src/main/java/org/apache/struts2/StrutsConstants.java @@ -277,27 +277,22 @@ public final class StrutsConstants { 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 - * removed when a new entry needs to be added (cache is fully-utilized). + * Specifies the type of cache to use for BeanInfo objects. + * @see StrutsConstants#STRUTS_OGNL_EXPRESSION_CACHE_TYPE + */ + public static final String STRUTS_OGNL_BEANINFO_CACHE_TYPE = "struts.ognl.beanInfoCacheType"; + + /** + * Specifies the maximum cache size for BeanInfo objects. This should be configured based on the cache type chosen + * and application-specific needs. * * @since 6.0.0 */ 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 6.0.0 + * @deprecated since 6.4.0, use {@link StrutsConstants#STRUTS_OGNL_BEANINFO_CACHE_TYPE} instead. */ public static final String STRUTS_OGNL_BEANINFO_CACHE_LRU_MODE = "struts.ognl.beanInfoCacheLRUMode"; @@ -327,28 +322,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). + * Specifies the type of cache to use for parsed OGNL expressions. Valid values defined in + * {@link com.opensymphony.xwork2.ognl.OgnlCacheFactory.CacheType}. + *
    + *
  • For the W-TinyLfu cache, the eviction policy is detailed + * here.
  • + *
  • For the basic cache, exceeding the maximum cache size will cause the entire cache to flush.
  • + *
  • For the LRU cache, once the maximum cache size is reached, the least-recently-used entry will be removed. + *
  • + *
+ */ + public static final String STRUTS_OGNL_EXPRESSION_CACHE_TYPE = "struts.ognl.expressionCacheType"; + + /** + * Specifies the maximum cache size for parsed OGNL expressions. This should be configured based on the cache type + * chosen and application-specific needs. * * @since 6.0.0 */ 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 6.0.0 + * @deprecated since 6.4.0, use {@link StrutsConstants#STRUTS_OGNL_EXPRESSION_CACHE_TYPE} instead. */ + @Deprecated public static final String STRUTS_OGNL_EXPRESSION_CACHE_LRU_MODE = "struts.ognl.expressionCacheLRUMode"; /** diff --git a/core/src/main/resources/org/apache/struts2/default.properties b/core/src/main/resources/org/apache/struts2/default.properties index 60257465b2..13d99b4eaf 100644 --- a/core/src/main/resources/org/apache/struts2/default.properties +++ b/core/src/main/resources/org/apache/struts2/default.properties @@ -237,31 +237,19 @@ struts.ognl.enableExpressionCache=true # struts.ognl.expressionCacheFactory=customOgnlExpressionCacheFactory # struts.ognl.beanInfoCacheFactory=customOgnlBeanInfoCacheFactory -### 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. +### Specifies the type of cache to use for parsed OGNL expressions. See StrutsConstants class for further information. +struts.ognl.expressionCacheType=caffeine_wtlfu + +### Specifies the maximum cache size for parsed OGNL expressions. This should be configured based on the cache type +### chosen and application-specific needs. struts.ognl.expressionCacheMaxSize=10000 -### 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=true - -### 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=5000 - -### 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=true +### Specifies the type of cache to use for BeanInfo objects. See StrutsConstants class for further information. +struts.ognl.beanInfoCacheType=caffeine_wtlfu + +### Specifies the maximum cache size for BeanInfo objects. This should be configured based on the cache type chosen and +### application-specific needs. +struts.ognl.beanInfoCacheMaxSize=10000 ### 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 From bfb4df13ee20cf8bf916489a15d9ae56e10547fd Mon Sep 17 00:00:00 2001 From: Kusal Kithul-Godage Date: Sun, 15 Oct 2023 23:28:04 +1100 Subject: [PATCH 07/18] WW-5355 Unify bootstrap constant declaration --- .../config/impl/DefaultConfiguration.java | 29 ++++++++++++++----- .../xwork2/config/impl/MockConfiguration.java | 18 ++++++++---- .../StrutsDefaultConfigurationProvider.java | 15 ++++------ .../xwork2/ognl/OgnlCacheFactory.java | 2 +- 4 files changed, 41 insertions(+), 23 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 3deda2c812..16025f355b 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 @@ -81,6 +81,7 @@ import com.opensymphony.xwork2.ognl.DefaultOgnlBeanInfoCacheFactory; import com.opensymphony.xwork2.ognl.DefaultOgnlExpressionCacheFactory; import com.opensymphony.xwork2.ognl.ExpressionCacheFactory; +import com.opensymphony.xwork2.ognl.OgnlCacheFactory; import com.opensymphony.xwork2.ognl.OgnlReflectionProvider; import com.opensymphony.xwork2.ognl.OgnlUtil; import com.opensymphony.xwork2.ognl.OgnlValueStackFactory; @@ -109,6 +110,7 @@ import java.util.ArrayList; import java.util.Collections; +import java.util.HashMap; import java.util.LinkedHashMap; import java.util.List; import java.util.Map; @@ -369,18 +371,29 @@ protected Container createBootstrapContainer(List providers) builder.factory(ValueSubstitutor.class, EnvsValueSubstitutor.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"); - builder.constant(StrutsConstants.STRUTS_OGNL_ENABLE_EXPRESSION_CACHE, "true"); - builder.constant(StrutsConstants.STRUTS_CONFIGURATION_XML_RELOAD, "false"); - builder.constant(StrutsConstants.STRUTS_I18N_RELOAD, "false"); - - builder.constant(StrutsConstants.STRUTS_MATCHER_APPEND_NAMED_PARAMETERS, "true"); + for (Map.Entry entry : bootstrapsConstants().entrySet()) { + builder.constant(entry.getKey(), String.valueOf(entry.getValue())); + } return builder.create(true); } + public static Map bootstrapsConstants() { + Map constants = new HashMap<>(); + constants.put(StrutsConstants.STRUTS_DEVMODE, Boolean.FALSE); + constants.put(StrutsConstants.STRUTS_OGNL_LOG_MISSING_PROPERTIES, Boolean.FALSE); + constants.put(StrutsConstants.STRUTS_OGNL_ENABLE_EVAL_EXPRESSION, Boolean.FALSE); + constants.put(StrutsConstants.STRUTS_OGNL_ENABLE_EXPRESSION_CACHE, Boolean.TRUE); + constants.put(StrutsConstants.STRUTS_CONFIGURATION_XML_RELOAD, Boolean.FALSE); + constants.put(StrutsConstants.STRUTS_I18N_RELOAD, Boolean.FALSE); + constants.put(StrutsConstants.STRUTS_MATCHER_APPEND_NAMED_PARAMETERS, Boolean.TRUE); + constants.put(StrutsConstants.STRUTS_OGNL_EXPRESSION_CACHE_TYPE, OgnlCacheFactory.CacheType.CAFFEINE_WTLFU); + constants.put(StrutsConstants.STRUTS_OGNL_EXPRESSION_CACHE_MAXSIZE, 10000); + constants.put(StrutsConstants.STRUTS_OGNL_BEANINFO_CACHE_TYPE, OgnlCacheFactory.CacheType.CAFFEINE_WTLFU); + constants.put(StrutsConstants.STRUTS_OGNL_BEANINFO_CACHE_MAXSIZE, 10000); + return Collections.unmodifiableMap(constants); + } + /** *

* This builds the internal runtime configuration used by Xwork for finding and configuring Actions from the diff --git a/core/src/main/java/com/opensymphony/xwork2/config/impl/MockConfiguration.java b/core/src/main/java/com/opensymphony/xwork2/config/impl/MockConfiguration.java index 3305bca96e..214f29926c 100644 --- a/core/src/main/java/com/opensymphony/xwork2/config/impl/MockConfiguration.java +++ b/core/src/main/java/com/opensymphony/xwork2/config/impl/MockConfiguration.java @@ -18,7 +18,11 @@ */ package com.opensymphony.xwork2.config.impl; -import com.opensymphony.xwork2.config.*; +import com.opensymphony.xwork2.config.Configuration; +import com.opensymphony.xwork2.config.ConfigurationException; +import com.opensymphony.xwork2.config.ContainerProvider; +import com.opensymphony.xwork2.config.PackageProvider; +import com.opensymphony.xwork2.config.RuntimeConfiguration; import com.opensymphony.xwork2.config.entities.PackageConfig; import com.opensymphony.xwork2.config.entities.UnknownHandlerConfig; import com.opensymphony.xwork2.config.providers.StrutsDefaultConfigurationProvider; @@ -28,7 +32,11 @@ import com.opensymphony.xwork2.util.location.LocatableProperties; import org.apache.struts2.StrutsConstants; -import java.util.*; +import java.util.HashMap; +import java.util.HashSet; +import java.util.List; +import java.util.Map; +import java.util.Set; /** @@ -51,9 +59,9 @@ public void selfRegister() { builder.factory(Configuration.class, MockConfiguration.class, Scope.SINGLETON); LocatableProperties props = new LocatableProperties(); new StrutsDefaultConfigurationProvider().register(builder, props); - builder.constant(StrutsConstants.STRUTS_DEVMODE, "false"); - builder.constant(StrutsConstants.STRUTS_CONFIGURATION_XML_RELOAD, "true"); - builder.constant(StrutsConstants.STRUTS_OGNL_ENABLE_EXPRESSION_CACHE, "true"); + for (Map.Entry entry : DefaultConfiguration.bootstrapsConstants().entrySet()) { + builder.constant(entry.getKey(), String.valueOf(entry.getValue())); + } builder.constant(StrutsConstants.STRUTS_ENABLE_DYNAMIC_METHOD_INVOCATION, "false"); container = builder.create(true); } 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 5769228569..3f9c953e94 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 @@ -35,6 +35,7 @@ import com.opensymphony.xwork2.config.Configuration; import com.opensymphony.xwork2.config.ConfigurationException; import com.opensymphony.xwork2.config.ConfigurationProvider; +import com.opensymphony.xwork2.config.impl.DefaultConfiguration; import com.opensymphony.xwork2.conversion.ConversionAnnotationProcessor; import com.opensymphony.xwork2.conversion.ConversionFileProcessor; import com.opensymphony.xwork2.conversion.ConversionPropertiesProcessor; @@ -251,15 +252,11 @@ public void register(ContainerBuilder builder, LocatableProperties props) .factory(ExecutorProvider.class, StrutsExecutorProvider.class, Scope.SINGLETON) ; - props.setProperty(StrutsConstants.STRUTS_ENABLE_DYNAMIC_METHOD_INVOCATION, Boolean.FALSE.toString()); - props.setProperty(StrutsConstants.STRUTS_I18N_RELOAD, Boolean.FALSE.toString()); - props.setProperty(StrutsConstants.STRUTS_DEVMODE, Boolean.FALSE.toString()); - props.setProperty(StrutsConstants.STRUTS_OGNL_LOG_MISSING_PROPERTIES, Boolean.FALSE.toString()); - props.setProperty(StrutsConstants.STRUTS_OGNL_ENABLE_EXPRESSION_CACHE, Boolean.TRUE.toString()); - props.setProperty(StrutsConstants.STRUTS_OGNL_ENABLE_EVAL_EXPRESSION, Boolean.FALSE.toString()); - props.setProperty(StrutsConstants.STRUTS_CONFIGURATION_XML_RELOAD, Boolean.FALSE.toString()); + for (Map.Entry entry : DefaultConfiguration.bootstrapsConstants().entrySet()) { + props.setProperty(entry.getKey(), String.valueOf(entry.getValue())); + } + props.setProperty(StrutsConstants.STRUTS_ALLOW_STATIC_FIELD_ACCESS, Boolean.TRUE.toString()); - props.setProperty(StrutsConstants.STRUTS_MATCHER_APPEND_NAMED_PARAMETERS, Boolean.TRUE.toString()); + props.setProperty(StrutsConstants.STRUTS_ENABLE_DYNAMIC_METHOD_INVOCATION, Boolean.FALSE.toString()); } - } 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 c3604e5c0b..9f7c7d6258 100644 --- a/core/src/main/java/com/opensymphony/xwork2/ognl/OgnlCacheFactory.java +++ b/core/src/main/java/com/opensymphony/xwork2/ognl/OgnlCacheFactory.java @@ -22,7 +22,7 @@ * @param The type for the cache key entries * @param The type for the cache value entries */ -interface OgnlCacheFactory { +public interface OgnlCacheFactory { OgnlCache buildOgnlCache(); OgnlCache buildOgnlCache(int evictionLimit, int initialCapacity, float loadFactor, boolean lruCache); int getCacheMaxSize(); From d245dc55135617827845132dc0f9d86bd513ef8a Mon Sep 17 00:00:00 2001 From: Kusal Kithul-Godage Date: Sun, 15 Oct 2023 23:28:58 +1100 Subject: [PATCH 08/18] WW-5355 Introduce new cache type selection methods and deprecate problematic setter injection --- .../ognl/DefaultOgnlBeanInfoCacheFactory.java | 19 +++--- .../xwork2/ognl/DefaultOgnlCacheFactory.java | 68 ++++++++++++++----- .../DefaultOgnlExpressionCacheFactory.java | 25 +++---- .../xwork2/ognl/OgnlCacheFactory.java | 45 +++++++++++- .../opensymphony/xwork2/ognl/OgnlUtil.java | 10 ++- .../xwork2/ognl/OgnlUtilTest.java | 12 +++- 6 files changed, 132 insertions(+), 47 deletions(-) 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 239b6f8ea4..8f7414a690 100644 --- a/core/src/main/java/com/opensymphony/xwork2/ognl/DefaultOgnlBeanInfoCacheFactory.java +++ b/core/src/main/java/com/opensymphony/xwork2/ognl/DefaultOgnlBeanInfoCacheFactory.java @@ -16,6 +16,7 @@ package com.opensymphony.xwork2.ognl; import com.opensymphony.xwork2.inject.Inject; +import org.apache.commons.lang3.EnumUtils; import org.apache.struts2.StrutsConstants; /** @@ -29,16 +30,16 @@ public class DefaultOgnlBeanInfoCacheFactory extends DefaultOgnlCacheFactory implements BeanInfoCacheFactory { - @Override - @Inject(value = StrutsConstants.STRUTS_OGNL_BEANINFO_CACHE_MAXSIZE, required = false) - protected void setCacheMaxSize(String maxSize) { - super.setCacheMaxSize(maxSize); + /** + * @deprecated since 6.4.0, use {@link #DefaultOgnlBeanInfoCacheFactory(String, String)} + */ + @Deprecated + public DefaultOgnlBeanInfoCacheFactory() { } - @Override - @Inject(value = StrutsConstants.STRUTS_OGNL_BEANINFO_CACHE_LRU_MODE, required = false) - protected void setUseLRUCache(String useLRUMode) { - super.setUseLRUCache(useLRUMode); + @Inject + public DefaultOgnlBeanInfoCacheFactory(@Inject(value = StrutsConstants.STRUTS_OGNL_BEANINFO_CACHE_MAXSIZE) String cacheMaxSize, + @Inject(value = StrutsConstants.STRUTS_OGNL_BEANINFO_CACHE_TYPE) String defaultCacheType) { + super(Integer.parseInt(cacheMaxSize), EnumUtils.getEnumIgnoreCase(CacheType.class, defaultCacheType)); } - } 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 dc9f6d664f..e8a32d78da 100644 --- a/core/src/main/java/com/opensymphony/xwork2/ognl/DefaultOgnlCacheFactory.java +++ b/core/src/main/java/com/opensymphony/xwork2/ognl/DefaultOgnlCacheFactory.java @@ -15,52 +15,84 @@ */ 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. + *

Default OGNL Cache factory implementation.

* - * Currently used for Expression cache and BeanInfo cache creation. + *

Currently used for Expression cache and BeanInfo cache creation.

* - * @param The type for the cache key entries + * @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); + private CacheType defaultCacheType; + private int cacheMaxSize; + + /** + * @deprecated since 6.4.0, use {@link #DefaultOgnlCacheFactory(int, CacheType)} + */ + @Deprecated + public DefaultOgnlCacheFactory() { + this.cacheMaxSize = 25000; + this.defaultCacheType = CacheType.CAFFEINE_WTLFU; + } + + public DefaultOgnlCacheFactory(int cacheMaxSize, CacheType defaultCacheType) { + this.cacheMaxSize = cacheMaxSize; + this.defaultCacheType = defaultCacheType; + } @Override public OgnlCache buildOgnlCache() { - return buildOgnlCache(getCacheMaxSize(), 16, 0.75f, getUseLRUCache()); + return buildOgnlCache(getCacheMaxSize(), 16, 0.75f, defaultCacheType); } @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); + public OgnlCache buildOgnlCache(int evictionLimit, + int initialCapacity, + float loadFactor, + CacheType cacheType) { + switch (cacheType) { + case CONCURRENT_BASIC: + return new OgnlDefaultCache<>(evictionLimit, initialCapacity, loadFactor); + case SYNC_LINKED_LRU: + return new OgnlLRUCache<>(evictionLimit, initialCapacity, loadFactor); + case CAFFEINE_WTLFU: + return new OgnlCaffeineCache<>(evictionLimit, initialCapacity, loadFactor); + default: + throw new IllegalArgumentException("Unknown cache type: " + cacheType); } } @Override public int getCacheMaxSize() { - return cacheMaxSize.get(); + return cacheMaxSize; } + /** + * @deprecated since 6.4.0 + */ + @Deprecated protected void setCacheMaxSize(String maxSize) { - cacheMaxSize.set(Integer.parseInt(maxSize)); + cacheMaxSize = Integer.parseInt(maxSize); } @Override - public boolean getUseLRUCache() { - return useLRUCache.get(); + public CacheType getDefaultCacheType() { + return defaultCacheType; } + /** + * No effect when {@code useLRUMode} is {@code false} + * + * @deprecated since 6.4.0 + */ + @Deprecated protected void setUseLRUCache(String useLRUMode) { - useLRUCache.set(BooleanUtils.toBoolean(useLRUMode)); + if (BooleanUtils.toBoolean(useLRUMode)) { + defaultCacheType = CacheType.SYNC_LINKED_LRU; + } } } 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 5d68f1e614..fadc8b3a54 100644 --- a/core/src/main/java/com/opensymphony/xwork2/ognl/DefaultOgnlExpressionCacheFactory.java +++ b/core/src/main/java/com/opensymphony/xwork2/ognl/DefaultOgnlExpressionCacheFactory.java @@ -16,29 +16,30 @@ package com.opensymphony.xwork2.ognl; import com.opensymphony.xwork2.inject.Inject; +import org.apache.commons.lang3.EnumUtils; 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 key entries * @param The type for the cache value entries */ public class DefaultOgnlExpressionCacheFactory extends DefaultOgnlCacheFactory - implements ExpressionCacheFactory { + implements ExpressionCacheFactory { - @Override - @Inject(value = StrutsConstants.STRUTS_OGNL_EXPRESSION_CACHE_MAXSIZE, required = false) - protected void setCacheMaxSize(String maxSize) { - super.setCacheMaxSize(maxSize); + /** + * @deprecated since 6.4.0, use {@link #DefaultOgnlExpressionCacheFactory(String, String)} + */ + @Deprecated + public DefaultOgnlExpressionCacheFactory() { } - @Override - @Inject(value = StrutsConstants.STRUTS_OGNL_EXPRESSION_CACHE_LRU_MODE, required = false) - protected void setUseLRUCache(String useLRUMode) { - super.setUseLRUCache(useLRUMode); + @Inject + public DefaultOgnlExpressionCacheFactory(@Inject(value = StrutsConstants.STRUTS_OGNL_EXPRESSION_CACHE_MAXSIZE) String cacheMaxSize, + @Inject(value = StrutsConstants.STRUTS_OGNL_EXPRESSION_CACHE_TYPE) String defaultCacheType) { + super(Integer.parseInt(cacheMaxSize), EnumUtils.getEnumIgnoreCase(CacheType.class, defaultCacheType)); } - } 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 9f7c7d6258..5db8cb973b 100644 --- a/core/src/main/java/com/opensymphony/xwork2/ognl/OgnlCacheFactory.java +++ b/core/src/main/java/com/opensymphony/xwork2/ognl/OgnlCacheFactory.java @@ -19,14 +19,53 @@ * 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 key entries * @param The type for the cache value entries */ public interface OgnlCacheFactory { OgnlCache buildOgnlCache(); - OgnlCache buildOgnlCache(int evictionLimit, int initialCapacity, float loadFactor, boolean lruCache); + + /** + * Note that if {@code lruCache} is {@code false}, the cache type could still be LRU if the default cache type is + * configured as such. + * @deprecated since 6.4.0, use {@link #buildOgnlCache(int, int, float, CacheType)} + */ + @Deprecated + default OgnlCache buildOgnlCache(int evictionLimit, + int initialCapacity, + float loadFactor, + boolean lruCache) { + if (lruCache) { + return buildOgnlCache(evictionLimit, initialCapacity, loadFactor, CacheType.SYNC_LINKED_LRU); + } else { + return buildOgnlCache(evictionLimit, + initialCapacity, + loadFactor, + lruCache ? CacheType.SYNC_LINKED_LRU : getDefaultCacheType()); + } + } + + /** + * @param evictionLimit maximum capacity of the cache where applicable for cache type chosen + * @param initialCapacity initial capacity of the cache where applicable for cache type chosen + * @param loadFactor load factor of the cache where applicable for cache type chosen + * @param cacheType type of cache to build + * @return a new cache instance + */ + OgnlCache buildOgnlCache(int evictionLimit, int initialCapacity, float loadFactor, CacheType cacheType); + int getCacheMaxSize(); - boolean getUseLRUCache(); + + /** + * @deprecated since 6.4.0 + */ + @Deprecated + default boolean getUseLRUCache() { + return CacheType.SYNC_LINKED_LRU.equals(getDefaultCacheType()); + } + + CacheType getDefaultCacheType(); + enum CacheType { CONCURRENT_BASIC, SYNC_LINKED_LRU, 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 16c80d99d1..a01bfac41c 100644 --- a/core/src/main/java/com/opensymphony/xwork2/ognl/OgnlUtil.java +++ b/core/src/main/java/com/opensymphony/xwork2/ognl/OgnlUtil.java @@ -143,12 +143,18 @@ protected void setEnableExpressionCache(String cache) { enableExpressionCache = BooleanUtils.toBoolean(cache); } - @Inject(value = StrutsConstants.STRUTS_OGNL_EXPRESSION_CACHE_MAXSIZE, required = false) + /** + * @deprecated since 6.4.0, changing eviction limit after initialisation is not supported. + */ + @Deprecated protected void setExpressionCacheMaxSize(String maxSize) { expressionCache.setEvictionLimit(Integer.parseInt(maxSize)); } - @Inject(value = StrutsConstants.STRUTS_OGNL_BEANINFO_CACHE_MAXSIZE, required = false) + /** + * @deprecated since 6.4.0, changing eviction limit after initialisation is not supported. + */ + @Deprecated protected void setBeanInfoCacheMaxSize(String maxSize) { beanInfoCache.setEvictionLimit(Integer.parseInt(maxSize)); } 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 eb8aef83cc..989853c2d1 100644 --- a/core/src/test/java/com/opensymphony/xwork2/ognl/OgnlUtilTest.java +++ b/core/src/test/java/com/opensymphony/xwork2/ognl/OgnlUtilTest.java @@ -134,13 +134,19 @@ public void testCacheEnabled() throws OgnlException { } public void testCacheEnabledMaxSize() throws OgnlException { - ognlUtil.setEnableExpressionCache("true"); - ognlUtil.setExpressionCacheMaxSize("1"); + super.loadConfigurationProviders(new StubConfigurationProvider() { + @Override + public void register(ContainerBuilder builder, LocatableProperties props) throws ConfigurationException { + props.setProperty(StrutsConstants.STRUTS_OGNL_EXPRESSION_CACHE_TYPE, "concurrent_basic"); + props.setProperty(StrutsConstants.STRUTS_OGNL_EXPRESSION_CACHE_MAXSIZE, "1"); + } + }); + ognlUtil = container.getInstance(OgnlUtil.class); 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). + // Next expression 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"); From 4700dca18889057488d1a71586c8adbce13c1761 Mon Sep 17 00:00:00 2001 From: Kusal Kithul-Godage Date: Sun, 15 Oct 2023 23:42:43 +1100 Subject: [PATCH 09/18] WW-5355 Downgrade Caffeine version --- pom.xml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/pom.xml b/pom.xml index 050a598c4e..286ab085a2 100644 --- a/pom.xml +++ b/pom.xml @@ -688,7 +688,7 @@ com.github.ben-manes.caffeine caffeine - 3.1.8 + 2.9.3 From 7463e1de13c7b65370232c557c4da615017dfdef Mon Sep 17 00:00:00 2001 From: Kusal Kithul-Godage Date: Sun, 15 Oct 2023 23:57:04 +1100 Subject: [PATCH 10/18] WW-5355 Fix interface and unit test bug --- .../opensymphony/xwork2/ognl/OgnlCacheFactory.java | 12 ++++-------- .../com/opensymphony/xwork2/ognl/OgnlUtilTest.java | 2 +- 2 files changed, 5 insertions(+), 9 deletions(-) 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 5db8cb973b..ead1aea84f 100644 --- a/core/src/main/java/com/opensymphony/xwork2/ognl/OgnlCacheFactory.java +++ b/core/src/main/java/com/opensymphony/xwork2/ognl/OgnlCacheFactory.java @@ -35,14 +35,10 @@ default OgnlCache buildOgnlCache(int evictionLimit, int initialCapacity, float loadFactor, boolean lruCache) { - if (lruCache) { - return buildOgnlCache(evictionLimit, initialCapacity, loadFactor, CacheType.SYNC_LINKED_LRU); - } else { - return buildOgnlCache(evictionLimit, - initialCapacity, - loadFactor, - lruCache ? CacheType.SYNC_LINKED_LRU : getDefaultCacheType()); - } + return buildOgnlCache(evictionLimit, + initialCapacity, + loadFactor, + lruCache ? CacheType.SYNC_LINKED_LRU : getDefaultCacheType()); } /** 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 989853c2d1..1066715011 100644 --- a/core/src/test/java/com/opensymphony/xwork2/ognl/OgnlUtilTest.java +++ b/core/src/test/java/com/opensymphony/xwork2/ognl/OgnlUtilTest.java @@ -1870,7 +1870,7 @@ public void testOgnlDefaultCacheFactoryCoverage() { 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); + ognlCache = defaultOgnlCacheFactory.buildOgnlCache(15, 15, 0.75f, true); assertNotNull("No param build method result null ?", ognlCache); assertEquals("Eviction limit for cache mismatches limit for factory ?", 15, ognlCache.getEvictionLimit()); } From 28cc6459bfd75ab3a2eb3075ad2fe8b2143f2b88 Mon Sep 17 00:00:00 2001 From: Kusal Kithul-Godage Date: Mon, 16 Oct 2023 00:11:51 +1100 Subject: [PATCH 11/18] WW-5355 Address code smells --- .../xwork2/ognl/DefaultOgnlCacheFactory.java | 2 +- .../xwork2/ognl/OgnlCaffeineCache.java | 16 ++++++++-------- .../xwork2/ognl/OgnlDefaultCache.java | 14 +++++++------- .../opensymphony/xwork2/ognl/OgnlLRUCache.java | 18 +++++++++--------- .../org/apache/struts2/StrutsConstants.java | 1 + 5 files changed, 26 insertions(+), 25 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 e8a32d78da..8ed28a1031 100644 --- a/core/src/main/java/com/opensymphony/xwork2/ognl/DefaultOgnlCacheFactory.java +++ b/core/src/main/java/com/opensymphony/xwork2/ognl/DefaultOgnlCacheFactory.java @@ -60,7 +60,7 @@ public OgnlCache buildOgnlCache(int evictionLimit, case SYNC_LINKED_LRU: return new OgnlLRUCache<>(evictionLimit, initialCapacity, loadFactor); case CAFFEINE_WTLFU: - return new OgnlCaffeineCache<>(evictionLimit, initialCapacity, loadFactor); + return new OgnlCaffeineCache<>(evictionLimit, initialCapacity); default: throw new IllegalArgumentException("Unknown cache type: " + cacheType); } diff --git a/core/src/main/java/com/opensymphony/xwork2/ognl/OgnlCaffeineCache.java b/core/src/main/java/com/opensymphony/xwork2/ognl/OgnlCaffeineCache.java index 7bb5f69c7f..4cdc321f25 100644 --- a/core/src/main/java/com/opensymphony/xwork2/ognl/OgnlCaffeineCache.java +++ b/core/src/main/java/com/opensymphony/xwork2/ognl/OgnlCaffeineCache.java @@ -30,31 +30,31 @@ *

  • Memory constraints
  • * * - * @param The type for the cache key entries - * @param The type for the cache value entries + * @param The type for the cache key entries + * @param The type for the cache value entries */ -public class OgnlCaffeineCache implements OgnlCache { +public class OgnlCaffeineCache implements OgnlCache { - private final Cache cache; + private final Cache cache; private final int evictionLimit; - public OgnlCaffeineCache(int evictionLimit, int initialCapacity, float loadFactor) { + public OgnlCaffeineCache(int evictionLimit, int initialCapacity) { this.evictionLimit = evictionLimit; this.cache = Caffeine.newBuilder().initialCapacity(initialCapacity).maximumSize(evictionLimit).build(); } @Override - public Value get(Key key) { + public V get(K key) { return cache.getIfPresent(key); } @Override - public void put(Key key, Value value) { + public void put(K key, V value) { cache.put(key, value); } @Override - public void putIfAbsent(Key key, Value value) { + public void putIfAbsent(K key, V value) { if (cache.getIfPresent(key) == null) { cache.put(key, value); } diff --git a/core/src/main/java/com/opensymphony/xwork2/ognl/OgnlDefaultCache.java b/core/src/main/java/com/opensymphony/xwork2/ognl/OgnlDefaultCache.java index ac4335fdbb..920403a5d2 100644 --- a/core/src/main/java/com/opensymphony/xwork2/ognl/OgnlDefaultCache.java +++ b/core/src/main/java/com/opensymphony/xwork2/ognl/OgnlDefaultCache.java @@ -27,12 +27,12 @@ *

    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 + * @param The type for the cache key entries + * @param The type for the cache value entries */ -public class OgnlDefaultCache implements OgnlCache { +public class OgnlDefaultCache implements OgnlCache { - private final ConcurrentHashMap ognlCache; + private final ConcurrentHashMap ognlCache; private final AtomicInteger cacheEvictionLimit; public OgnlDefaultCache(int evictionLimit, int initialCapacity, float loadFactor) { @@ -41,18 +41,18 @@ public OgnlDefaultCache(int evictionLimit, int initialCapacity, float loadFactor } @Override - public Value get(Key key) { + public V get(K key) { return ognlCache.get(key); } @Override - public void put(Key key, Value value) { + public void put(K key, V value) { ognlCache.put(key, value); this.clearIfEvictionLimitExceeded(); } @Override - public void putIfAbsent(Key key, Value value) { + public void putIfAbsent(K key, V value) { ognlCache.putIfAbsent(key, value); this.clearIfEvictionLimitExceeded(); } diff --git a/core/src/main/java/com/opensymphony/xwork2/ognl/OgnlLRUCache.java b/core/src/main/java/com/opensymphony/xwork2/ognl/OgnlLRUCache.java index 6606c9c680..e324418ceb 100644 --- a/core/src/main/java/com/opensymphony/xwork2/ognl/OgnlLRUCache.java +++ b/core/src/main/java/com/opensymphony/xwork2/ognl/OgnlLRUCache.java @@ -30,37 +30,37 @@ *

    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 + * @param The type for the cache key entries + * @param The type for the cache value entries */ -public class OgnlLRUCache implements OgnlCache { +public class OgnlLRUCache implements OgnlCache { - private final Map ognlLRUCache; + private final Map ognlLRUCache; private final AtomicInteger cacheEvictionLimit; public OgnlLRUCache(int evictionLimit, int initialCapacity, float loadFactor) { cacheEvictionLimit = new AtomicInteger(evictionLimit); // Access-order mode selected (order mode true in LinkedHashMap constructor). - ognlLRUCache = Collections.synchronizedMap(new LinkedHashMap(initialCapacity, loadFactor, true) { + ognlLRUCache = Collections.synchronizedMap(new LinkedHashMap(initialCapacity, loadFactor, true) { @Override - protected boolean removeEldestEntry(Map.Entry eldest) { + protected boolean removeEldestEntry(Map.Entry eldest) { return size() > cacheEvictionLimit.get(); } }); } @Override - public Value get(Key key) { + public V get(K key) { return ognlLRUCache.get(key); } @Override - public void put(Key key, Value value) { + public void put(K key, V value) { ognlLRUCache.put(key, value); } @Override - public void putIfAbsent(Key key, Value value) { + public void putIfAbsent(K key, V value) { ognlLRUCache.putIfAbsent(key, value); } diff --git a/core/src/main/java/org/apache/struts2/StrutsConstants.java b/core/src/main/java/org/apache/struts2/StrutsConstants.java index b8a133087b..d882ae82fb 100644 --- a/core/src/main/java/org/apache/struts2/StrutsConstants.java +++ b/core/src/main/java/org/apache/struts2/StrutsConstants.java @@ -294,6 +294,7 @@ public final class StrutsConstants { * @since 6.0.0 * @deprecated since 6.4.0, use {@link StrutsConstants#STRUTS_OGNL_BEANINFO_CACHE_TYPE} instead. */ + @Deprecated public static final String STRUTS_OGNL_BEANINFO_CACHE_LRU_MODE = "struts.ognl.beanInfoCacheLRUMode"; /** From 793d3837117236c9d242ddd40603feaa92594aca Mon Sep 17 00:00:00 2001 From: Kusal Kithul-Godage Date: Mon, 16 Oct 2023 00:17:43 +1100 Subject: [PATCH 12/18] WW-5355 Delegate deprecated constructor --- .../com/opensymphony/xwork2/ognl/DefaultOgnlCacheFactory.java | 3 +-- 1 file changed, 1 insertion(+), 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 8ed28a1031..4943f9b0b3 100644 --- a/core/src/main/java/com/opensymphony/xwork2/ognl/DefaultOgnlCacheFactory.java +++ b/core/src/main/java/com/opensymphony/xwork2/ognl/DefaultOgnlCacheFactory.java @@ -35,8 +35,7 @@ public class DefaultOgnlCacheFactory implements OgnlCacheFactory Date: Mon, 16 Oct 2023 00:20:17 +1100 Subject: [PATCH 13/18] WW-5355 Extract constants into static final fields --- .../opensymphony/xwork2/ognl/DefaultOgnlCacheFactory.java | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) 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 4943f9b0b3..fc3cd016f9 100644 --- a/core/src/main/java/com/opensymphony/xwork2/ognl/DefaultOgnlCacheFactory.java +++ b/core/src/main/java/com/opensymphony/xwork2/ognl/DefaultOgnlCacheFactory.java @@ -27,6 +27,9 @@ */ public class DefaultOgnlCacheFactory implements OgnlCacheFactory { + private static final int DEFAULT_INIT_CAPACITY = 16; + private static final float DEFAULT_LOAD_FACTOR = 0.75f; + private CacheType defaultCacheType; private int cacheMaxSize; @@ -45,7 +48,7 @@ public DefaultOgnlCacheFactory(int cacheMaxSize, CacheType defaultCacheType) { @Override public OgnlCache buildOgnlCache() { - return buildOgnlCache(getCacheMaxSize(), 16, 0.75f, defaultCacheType); + return buildOgnlCache(getCacheMaxSize(), DEFAULT_INIT_CAPACITY, DEFAULT_LOAD_FACTOR, defaultCacheType); } @Override From 3d5beae367d2f48680f1f174a2fae4aea7d11285 Mon Sep 17 00:00:00 2001 From: Kusal Kithul-Godage Date: Mon, 16 Oct 2023 00:39:12 +1100 Subject: [PATCH 14/18] WW-5355 Declare bootstrap constants as final field instead --- .../config/impl/DefaultConfiguration.java | 36 ++++++++++--------- .../xwork2/config/impl/MockConfiguration.java | 2 +- .../StrutsDefaultConfigurationProvider.java | 2 +- 3 files changed, 21 insertions(+), 19 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 16025f355b..4b864e6f18 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 @@ -127,6 +127,24 @@ */ public class DefaultConfiguration implements Configuration { + public static final Map BOOTSTRAP_CONSTANTS; + + static { + Map constants = new HashMap<>(); + constants.put(StrutsConstants.STRUTS_DEVMODE, Boolean.FALSE); + constants.put(StrutsConstants.STRUTS_OGNL_LOG_MISSING_PROPERTIES, Boolean.FALSE); + constants.put(StrutsConstants.STRUTS_OGNL_ENABLE_EVAL_EXPRESSION, Boolean.FALSE); + constants.put(StrutsConstants.STRUTS_OGNL_ENABLE_EXPRESSION_CACHE, Boolean.TRUE); + constants.put(StrutsConstants.STRUTS_CONFIGURATION_XML_RELOAD, Boolean.FALSE); + constants.put(StrutsConstants.STRUTS_I18N_RELOAD, Boolean.FALSE); + constants.put(StrutsConstants.STRUTS_MATCHER_APPEND_NAMED_PARAMETERS, Boolean.TRUE); + constants.put(StrutsConstants.STRUTS_OGNL_EXPRESSION_CACHE_TYPE, OgnlCacheFactory.CacheType.CAFFEINE_WTLFU); + constants.put(StrutsConstants.STRUTS_OGNL_EXPRESSION_CACHE_MAXSIZE, 10000); + constants.put(StrutsConstants.STRUTS_OGNL_BEANINFO_CACHE_TYPE, OgnlCacheFactory.CacheType.CAFFEINE_WTLFU); + constants.put(StrutsConstants.STRUTS_OGNL_BEANINFO_CACHE_MAXSIZE, 10000); + BOOTSTRAP_CONSTANTS = Collections.unmodifiableMap(constants); + } + protected static final Logger LOG = LogManager.getLogger(DefaultConfiguration.class); // Programmatic Action Configurations @@ -371,29 +389,13 @@ protected Container createBootstrapContainer(List providers) builder.factory(ValueSubstitutor.class, EnvsValueSubstitutor.class, Scope.SINGLETON); - for (Map.Entry entry : bootstrapsConstants().entrySet()) { + for (Map.Entry entry : BOOTSTRAP_CONSTANTS.entrySet()) { builder.constant(entry.getKey(), String.valueOf(entry.getValue())); } return builder.create(true); } - public static Map bootstrapsConstants() { - Map constants = new HashMap<>(); - constants.put(StrutsConstants.STRUTS_DEVMODE, Boolean.FALSE); - constants.put(StrutsConstants.STRUTS_OGNL_LOG_MISSING_PROPERTIES, Boolean.FALSE); - constants.put(StrutsConstants.STRUTS_OGNL_ENABLE_EVAL_EXPRESSION, Boolean.FALSE); - constants.put(StrutsConstants.STRUTS_OGNL_ENABLE_EXPRESSION_CACHE, Boolean.TRUE); - constants.put(StrutsConstants.STRUTS_CONFIGURATION_XML_RELOAD, Boolean.FALSE); - constants.put(StrutsConstants.STRUTS_I18N_RELOAD, Boolean.FALSE); - constants.put(StrutsConstants.STRUTS_MATCHER_APPEND_NAMED_PARAMETERS, Boolean.TRUE); - constants.put(StrutsConstants.STRUTS_OGNL_EXPRESSION_CACHE_TYPE, OgnlCacheFactory.CacheType.CAFFEINE_WTLFU); - constants.put(StrutsConstants.STRUTS_OGNL_EXPRESSION_CACHE_MAXSIZE, 10000); - constants.put(StrutsConstants.STRUTS_OGNL_BEANINFO_CACHE_TYPE, OgnlCacheFactory.CacheType.CAFFEINE_WTLFU); - constants.put(StrutsConstants.STRUTS_OGNL_BEANINFO_CACHE_MAXSIZE, 10000); - return Collections.unmodifiableMap(constants); - } - /** *

    * This builds the internal runtime configuration used by Xwork for finding and configuring Actions from the diff --git a/core/src/main/java/com/opensymphony/xwork2/config/impl/MockConfiguration.java b/core/src/main/java/com/opensymphony/xwork2/config/impl/MockConfiguration.java index 214f29926c..d245ccf4b3 100644 --- a/core/src/main/java/com/opensymphony/xwork2/config/impl/MockConfiguration.java +++ b/core/src/main/java/com/opensymphony/xwork2/config/impl/MockConfiguration.java @@ -59,7 +59,7 @@ public void selfRegister() { builder.factory(Configuration.class, MockConfiguration.class, Scope.SINGLETON); LocatableProperties props = new LocatableProperties(); new StrutsDefaultConfigurationProvider().register(builder, props); - for (Map.Entry entry : DefaultConfiguration.bootstrapsConstants().entrySet()) { + for (Map.Entry entry : DefaultConfiguration.BOOTSTRAP_CONSTANTS.entrySet()) { builder.constant(entry.getKey(), String.valueOf(entry.getValue())); } builder.constant(StrutsConstants.STRUTS_ENABLE_DYNAMIC_METHOD_INVOCATION, "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 3f9c953e94..625a4fb17b 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 @@ -252,7 +252,7 @@ public void register(ContainerBuilder builder, LocatableProperties props) .factory(ExecutorProvider.class, StrutsExecutorProvider.class, Scope.SINGLETON) ; - for (Map.Entry entry : DefaultConfiguration.bootstrapsConstants().entrySet()) { + for (Map.Entry entry : DefaultConfiguration.BOOTSTRAP_CONSTANTS.entrySet()) { props.setProperty(entry.getKey(), String.valueOf(entry.getValue())); } From f314b455f7304d7282915c4756ec143e1ef53d81 Mon Sep 17 00:00:00 2001 From: Kusal Kithul-Godage Date: Mon, 16 Oct 2023 00:45:36 +1100 Subject: [PATCH 15/18] WW-5355 Add since tags to StrutsConstants JavaDoc --- core/src/main/java/org/apache/struts2/StrutsConstants.java | 2 ++ 1 file changed, 2 insertions(+) diff --git a/core/src/main/java/org/apache/struts2/StrutsConstants.java b/core/src/main/java/org/apache/struts2/StrutsConstants.java index d882ae82fb..bfe639d2e7 100644 --- a/core/src/main/java/org/apache/struts2/StrutsConstants.java +++ b/core/src/main/java/org/apache/struts2/StrutsConstants.java @@ -278,6 +278,7 @@ public final class StrutsConstants { /** * Specifies the type of cache to use for BeanInfo objects. + * @since 6.4.0 * @see StrutsConstants#STRUTS_OGNL_EXPRESSION_CACHE_TYPE */ public static final String STRUTS_OGNL_BEANINFO_CACHE_TYPE = "struts.ognl.beanInfoCacheType"; @@ -332,6 +333,7 @@ public final class StrutsConstants { *

  • For the LRU cache, once the maximum cache size is reached, the least-recently-used entry will be removed. *
  • * + * @since 6.4.0 */ public static final String STRUTS_OGNL_EXPRESSION_CACHE_TYPE = "struts.ognl.expressionCacheType"; From 9dbea66f9c48ee321eea5cec8f86f9831a438262 Mon Sep 17 00:00:00 2001 From: Kusal Kithul-Godage Date: Mon, 16 Oct 2023 13:06:09 +1100 Subject: [PATCH 16/18] WW-5355 Amend Caffeine cache implementation --- .../opensymphony/xwork2/ognl/OgnlCaffeineCache.java | 12 ++++-------- .../java/com/opensymphony/xwork2/ognl/OgnlUtil.java | 4 ++-- 2 files changed, 6 insertions(+), 10 deletions(-) diff --git a/core/src/main/java/com/opensymphony/xwork2/ognl/OgnlCaffeineCache.java b/core/src/main/java/com/opensymphony/xwork2/ognl/OgnlCaffeineCache.java index 4cdc321f25..b7a0241e39 100644 --- a/core/src/main/java/com/opensymphony/xwork2/ognl/OgnlCaffeineCache.java +++ b/core/src/main/java/com/opensymphony/xwork2/ognl/OgnlCaffeineCache.java @@ -36,10 +36,8 @@ public class OgnlCaffeineCache implements OgnlCache { private final Cache cache; - private final int evictionLimit; public OgnlCaffeineCache(int evictionLimit, int initialCapacity) { - this.evictionLimit = evictionLimit; this.cache = Caffeine.newBuilder().initialCapacity(initialCapacity).maximumSize(evictionLimit).build(); } @@ -55,14 +53,12 @@ public void put(K key, V value) { @Override public void putIfAbsent(K key, V value) { - if (cache.getIfPresent(key) == null) { - cache.put(key, value); - } + cache.asMap().putIfAbsent(key, value); } @Override public int size() { - return Math.toIntExact(cache.estimatedSize()); + return cache.asMap().size(); } @Override @@ -72,11 +68,11 @@ public void clear() { @Override public int getEvictionLimit() { - return evictionLimit; + return Math.toIntExact(cache.policy().eviction().orElseThrow(IllegalStateException::new).getMaximum()); } @Override public void setEvictionLimit(int cacheEvictionLimit) { - throw new UnsupportedOperationException("Cannot change eviction limit on a Caffeine cache after initialisation"); + cache.policy().eviction().orElseThrow(IllegalStateException::new).setMaximum(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 a01bfac41c..7c9b5fbbbc 100644 --- a/core/src/main/java/com/opensymphony/xwork2/ognl/OgnlUtil.java +++ b/core/src/main/java/com/opensymphony/xwork2/ognl/OgnlUtil.java @@ -144,7 +144,7 @@ protected void setEnableExpressionCache(String cache) { } /** - * @deprecated since 6.4.0, changing eviction limit after initialisation is not supported. + * @deprecated since 6.4.0, changing maximum cache size after initialisation is not necessary. */ @Deprecated protected void setExpressionCacheMaxSize(String maxSize) { @@ -152,7 +152,7 @@ protected void setExpressionCacheMaxSize(String maxSize) { } /** - * @deprecated since 6.4.0, changing eviction limit after initialisation is not supported. + * @deprecated since 6.4.0, changing maximum cache size after initialisation is not necessary. */ @Deprecated protected void setBeanInfoCacheMaxSize(String maxSize) { From 7cded18c0946e39eaeb3629f861a7caefcee6bcc Mon Sep 17 00:00:00 2001 From: Kusal Kithul-Godage Date: Tue, 17 Oct 2023 14:16:14 +1100 Subject: [PATCH 17/18] WW-5355 Rename cache types --- .../xwork2/ognl/DefaultOgnlCacheFactory.java | 8 ++++---- .../com/opensymphony/xwork2/ognl/OgnlCacheFactory.java | 10 +++++----- .../resources/org/apache/struts2/default.properties | 4 ++-- 3 files changed, 11 insertions(+), 11 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 fc3cd016f9..e25334c50b 100644 --- a/core/src/main/java/com/opensymphony/xwork2/ognl/DefaultOgnlCacheFactory.java +++ b/core/src/main/java/com/opensymphony/xwork2/ognl/DefaultOgnlCacheFactory.java @@ -57,11 +57,11 @@ public OgnlCache buildOgnlCache(int evictionLimit, float loadFactor, CacheType cacheType) { switch (cacheType) { - case CONCURRENT_BASIC: + case BASIC: return new OgnlDefaultCache<>(evictionLimit, initialCapacity, loadFactor); - case SYNC_LINKED_LRU: + case LRU: return new OgnlLRUCache<>(evictionLimit, initialCapacity, loadFactor); - case CAFFEINE_WTLFU: + case WTLFU: return new OgnlCaffeineCache<>(evictionLimit, initialCapacity); default: throw new IllegalArgumentException("Unknown cache type: " + cacheType); @@ -94,7 +94,7 @@ public CacheType getDefaultCacheType() { @Deprecated protected void setUseLRUCache(String useLRUMode) { if (BooleanUtils.toBoolean(useLRUMode)) { - defaultCacheType = CacheType.SYNC_LINKED_LRU; + defaultCacheType = CacheType.LRU; } } } 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 ead1aea84f..874bf4a2e1 100644 --- a/core/src/main/java/com/opensymphony/xwork2/ognl/OgnlCacheFactory.java +++ b/core/src/main/java/com/opensymphony/xwork2/ognl/OgnlCacheFactory.java @@ -38,7 +38,7 @@ default OgnlCache buildOgnlCache(int evictionLimit, return buildOgnlCache(evictionLimit, initialCapacity, loadFactor, - lruCache ? CacheType.SYNC_LINKED_LRU : getDefaultCacheType()); + lruCache ? CacheType.LRU : getDefaultCacheType()); } /** @@ -57,14 +57,14 @@ default OgnlCache buildOgnlCache(int evictionLimit, */ @Deprecated default boolean getUseLRUCache() { - return CacheType.SYNC_LINKED_LRU.equals(getDefaultCacheType()); + return CacheType.LRU.equals(getDefaultCacheType()); } CacheType getDefaultCacheType(); enum CacheType { - CONCURRENT_BASIC, - SYNC_LINKED_LRU, - CAFFEINE_WTLFU + BASIC, + LRU, + WTLFU } } diff --git a/core/src/main/resources/org/apache/struts2/default.properties b/core/src/main/resources/org/apache/struts2/default.properties index 13d99b4eaf..6b3cb4dfea 100644 --- a/core/src/main/resources/org/apache/struts2/default.properties +++ b/core/src/main/resources/org/apache/struts2/default.properties @@ -238,14 +238,14 @@ struts.ognl.enableExpressionCache=true # struts.ognl.beanInfoCacheFactory=customOgnlBeanInfoCacheFactory ### Specifies the type of cache to use for parsed OGNL expressions. See StrutsConstants class for further information. -struts.ognl.expressionCacheType=caffeine_wtlfu +struts.ognl.expressionCacheType=wtlfu ### Specifies the maximum cache size for parsed OGNL expressions. This should be configured based on the cache type ### chosen and application-specific needs. struts.ognl.expressionCacheMaxSize=10000 ### Specifies the type of cache to use for BeanInfo objects. See StrutsConstants class for further information. -struts.ognl.beanInfoCacheType=caffeine_wtlfu +struts.ognl.beanInfoCacheType=wtlfu ### Specifies the maximum cache size for BeanInfo objects. This should be configured based on the cache type chosen and ### application-specific needs. From 7afc77266b60ce665eb034f177d86770708f4121 Mon Sep 17 00:00:00 2001 From: Kusal Kithul-Godage Date: Tue, 17 Oct 2023 14:16:27 +1100 Subject: [PATCH 18/18] WW-5355 Bootstrap using basic cache --- .../opensymphony/xwork2/config/impl/DefaultConfiguration.java | 4 ++-- .../com/opensymphony/xwork2/ognl/DefaultOgnlCacheFactory.java | 2 +- .../test/java/com/opensymphony/xwork2/ognl/OgnlUtilTest.java | 1 - 3 files changed, 3 insertions(+), 4 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 4b864e6f18..b254842228 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 @@ -138,9 +138,9 @@ public class DefaultConfiguration implements Configuration { constants.put(StrutsConstants.STRUTS_CONFIGURATION_XML_RELOAD, Boolean.FALSE); constants.put(StrutsConstants.STRUTS_I18N_RELOAD, Boolean.FALSE); constants.put(StrutsConstants.STRUTS_MATCHER_APPEND_NAMED_PARAMETERS, Boolean.TRUE); - constants.put(StrutsConstants.STRUTS_OGNL_EXPRESSION_CACHE_TYPE, OgnlCacheFactory.CacheType.CAFFEINE_WTLFU); + constants.put(StrutsConstants.STRUTS_OGNL_EXPRESSION_CACHE_TYPE, OgnlCacheFactory.CacheType.BASIC); constants.put(StrutsConstants.STRUTS_OGNL_EXPRESSION_CACHE_MAXSIZE, 10000); - constants.put(StrutsConstants.STRUTS_OGNL_BEANINFO_CACHE_TYPE, OgnlCacheFactory.CacheType.CAFFEINE_WTLFU); + constants.put(StrutsConstants.STRUTS_OGNL_BEANINFO_CACHE_TYPE, OgnlCacheFactory.CacheType.BASIC); constants.put(StrutsConstants.STRUTS_OGNL_BEANINFO_CACHE_MAXSIZE, 10000); BOOTSTRAP_CONSTANTS = Collections.unmodifiableMap(constants); } 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 e25334c50b..29dc7fc7f6 100644 --- a/core/src/main/java/com/opensymphony/xwork2/ognl/DefaultOgnlCacheFactory.java +++ b/core/src/main/java/com/opensymphony/xwork2/ognl/DefaultOgnlCacheFactory.java @@ -38,7 +38,7 @@ public class DefaultOgnlCacheFactory implements OgnlCacheFactory