From 2020e2fa2548cc40be25b08625afb7383756ddd7 Mon Sep 17 00:00:00 2001 From: JCgH4164838Gh792C124B5 <43964333+JCgH4164838Gh792C124B5@users.noreply.github.com> Date: Sun, 27 Dec 2020 16:21:47 -0500 Subject: [PATCH] Updated commit based on feedback, plus a few minor changes: - Changed formatting as per review. - Added null guard for unlikely dereference in clearTomcatCache() logging statement. - Added a unit test for basic correctness check and better code coverage. --- .../util/AbstractLocalizedTextProvider.java | 7 +- .../util/StrutsLocalizedTextProviderTest.java | 78 +++++++++++++++++++ 2 files changed, 81 insertions(+), 4 deletions(-) diff --git a/core/src/main/java/com/opensymphony/xwork2/util/AbstractLocalizedTextProvider.java b/core/src/main/java/com/opensymphony/xwork2/util/AbstractLocalizedTextProvider.java index f214a9289..6ed6202d8 100644 --- a/core/src/main/java/com/opensymphony/xwork2/util/AbstractLocalizedTextProvider.java +++ b/core/src/main/java/com/opensymphony/xwork2/util/AbstractLocalizedTextProvider.java @@ -336,7 +336,6 @@ abstract class AbstractLocalizedTextProvider implements LocalizedTextProvider { * * Note: With Java 9+, calling this method may result in "Illegal reflective access" warnings. Be aware * its logic may fail in a future version of Java that blocks the reflection calls needed for this method. - * { */ private void clearTomcatCache() { ClassLoader loader = getCurrentThreadContextClassLoader(); @@ -345,8 +344,8 @@ abstract class AbstractLocalizedTextProvider implements LocalizedTextProvider { Class superCl = cl.getSuperclass(); try { - if ( (TOMCAT_WEBAPP_CLASSLOADER.equals(cl.getName()) || TOMCAT_PARALLEL_WEBAPP_CLASSLOADER.equals(cl.getName())) && - (superCl != null && TOMCAT_WEBAPP_CLASSLOADER_BASE.equals(superCl.getName())) ) { + if ((TOMCAT_WEBAPP_CLASSLOADER.equals(cl.getName()) || TOMCAT_PARALLEL_WEBAPP_CLASSLOADER.equals(cl.getName())) && + (superCl != null && TOMCAT_WEBAPP_CLASSLOADER_BASE.equals(superCl.getName()))) { // The classloader name and superclass name match the expecations for a Tomcat classloader. // Expect the classloader superclass to have the field, otherwise fallback to the classloader class if the field is not found. clearMap(superCl, loader, TOMCAT_RESOURCE_ENTRIES_FIELD); @@ -363,7 +362,7 @@ abstract class AbstractLocalizedTextProvider implements LocalizedTextProvider { LOG.warn("Couldn't clear tomcat cache using {}", cl.getName(), e); } } catch (Exception e) { - LOG.warn("Couldn't clear tomcat cache using {}", superCl.getName(), e); + LOG.warn("Couldn't clear tomcat cache using {}", (superCl != null ? superCl.getName() : null), e); } } diff --git a/core/src/test/java/com/opensymphony/xwork2/util/StrutsLocalizedTextProviderTest.java b/core/src/test/java/com/opensymphony/xwork2/util/StrutsLocalizedTextProviderTest.java index aef469dd8..2a3038ff0 100644 --- a/core/src/test/java/com/opensymphony/xwork2/util/StrutsLocalizedTextProviderTest.java +++ b/core/src/test/java/com/opensymphony/xwork2/util/StrutsLocalizedTextProviderTest.java @@ -327,6 +327,50 @@ public class StrutsLocalizedTextProviderTest extends XWorkTestCase { testStrutsLocalizedTextProvider.callClearMissingBundlesCache(); } + /** + * Unit test to confirm the basic behaviour of bundle reload methods provided to + * StrutsLocalizedTextProvider (from AbstractLocalizedTextProvider). + * + * @since 2.6 + */ + public void testLocalizedTextProviderReloadMethods() { + TestStrutsLocalizedTextProvider testStrutsLocalizedTextProvider = new TestStrutsLocalizedTextProvider(); + assertTrue("testStrutsLocalizedTextProvider not instance of AbstractLocalizedTextProvider ?", + testStrutsLocalizedTextProvider instanceof AbstractLocalizedTextProvider); + assertEquals("testStrutsLocalizedTextProvider starting default bundle map size not 0 before any retrievals ?", + 0, testStrutsLocalizedTextProvider.currentBundlesMapSize()); + + // Access the two default bundles to populate their cache entries and test bundle map size. + ResourceBundle tempBundle = testStrutsLocalizedTextProvider.findResourceBundle( + TestStrutsLocalizedTextProvider.XWORK_MESSAGES_BUNDLE, Locale.ENGLISH); + assertNotNull("XWORK_MESSAGES_BUNDLE retrieval null ?", tempBundle); + tempBundle = testStrutsLocalizedTextProvider.findResourceBundle( + TestStrutsLocalizedTextProvider.STRUTS_MESSAGES_BUNDLE, Locale.ENGLISH); + assertNotNull("STRUTS_MESSAGES_BUNDLE retrieval null ?", tempBundle); + assertEquals("testStrutsLocalizedTextProvider bundle map size not 2 after retrievals ?", + 2, testStrutsLocalizedTextProvider.currentBundlesMapSize()); + + // Force a bundle reload call for code coverage and to confirm it causes the bundle map to be emptied. + assertNotNull("ActionContext is somehow null ?", ActionContext.getContext()); + boolean bundlesReloadedBeforeCall = testStrutsLocalizedTextProvider.getBundlesReloadedIndicatorValue(); + assertFalse("Bundles reload value true before forced reload ?", bundlesReloadedBeforeCall); + testStrutsLocalizedTextProvider.callReloadBundlesForceReload(); + boolean bundlesReloadedAfterCall = testStrutsLocalizedTextProvider.getBundlesReloadedIndicatorValue(); + assertTrue("Bundles reload value false after forced reload ?", bundlesReloadedAfterCall); + assertEquals("testStrutsLocalizedTextProvider bundle map size not 0 after reload (which should clear it) ?", + 0, testStrutsLocalizedTextProvider.currentBundlesMapSize()); + + // Access the two default bundles again (after reload) to populate their cache entries and test bundle map size. + tempBundle = testStrutsLocalizedTextProvider.findResourceBundle( + TestStrutsLocalizedTextProvider.XWORK_MESSAGES_BUNDLE, Locale.ENGLISH); + assertNotNull("XWORK_MESSAGES_BUNDLE retrieval null ?", tempBundle); + tempBundle = testStrutsLocalizedTextProvider.findResourceBundle( + TestStrutsLocalizedTextProvider.STRUTS_MESSAGES_BUNDLE, Locale.ENGLISH); + assertNotNull("STRUTS_MESSAGES_BUNDLE retrieval null ?", tempBundle); + assertEquals("testStrutsLocalizedTextProvider bundle map size not 2 after retrievals ?", + 2, testStrutsLocalizedTextProvider.currentBundlesMapSize()); + } + @Override protected void setUp() throws Exception { super.setUp(); @@ -352,6 +396,13 @@ public class StrutsLocalizedTextProviderTest extends XWorkTestCase { */ class TestStrutsLocalizedTextProvider extends StrutsLocalizedTextProvider { + /** + * Some test correctness depends on this {@link #RELOADED} value matching that of the private ancestor + * field {@link AbstractLocalizedTextProvider#RELOADED}. If the ancestor field value changes, ensure this + * field's value is updated to match it exactly. + */ + private static final String RELOADED = "com.opensymphony.xwork2.util.LocalizedTextProvider.reloaded"; + public void callClearBundleNoLocale(String bundleName) { super.clearBundle(bundleName); } @@ -367,5 +418,32 @@ public class StrutsLocalizedTextProviderTest extends XWorkTestCase { public int currentBundlesMapSize() { return super.bundlesMap.size(); } + + /** + * Attempt to force the resource bundles to be reloaded, even if configuration would otherwise prevent it. + * It will preserve the current reloadBundles state, attempt to force a reload and then restore the + * original reloadBundles value. + */ + public void callReloadBundlesForceReload() { + final boolean originalReloadState = super.reloadBundles; + try { + super.setReloadBundles(Boolean.TRUE.toString()); + super.reloadBundles(); + } finally { + super.setReloadBundles(Boolean.toString(originalReloadState)); + } + } + + /** + * Returns the value of the resource bundles reloaded state from the context, provided that one was + * previously set. If no value is found, the result will be false (same as if bundles had not been reloaded). + * + * @return true if resource bundles reloaded indicator is true, false otherwise (including if value was never set). + */ + public boolean getBundlesReloadedIndicatorValue() { + final ActionContext actionContext = ActionContext.getContext(); + final Object reloadedObject = actionContext.get(RELOADED); + return ((reloadedObject instanceof Boolean) ? ((Boolean) reloadedObject).booleanValue() : false); + } } }