From bac0f2953c102ba1ff737a7fd35d20d38e527cea Mon Sep 17 00:00:00 2001 From: Yasser Zamani Date: Wed, 31 Oct 2018 19:51:37 +0330 Subject: [PATCH 1/2] monitor not-monitored loaded files on demand See WW-4974 --- .../xwork2/util/fs/DefaultFileManager.java | 26 ++++++++++++++++--- .../struts2/util/fs/JBossFileManager.java | 2 +- .../XmlConfigurationProviderTest.java | 12 ++++----- 3 files changed, 29 insertions(+), 11 deletions(-) diff --git a/core/src/main/java/com/opensymphony/xwork2/util/fs/DefaultFileManager.java b/core/src/main/java/com/opensymphony/xwork2/util/fs/DefaultFileManager.java index 21d6cc0ad..d39e9144e 100644 --- a/core/src/main/java/com/opensymphony/xwork2/util/fs/DefaultFileManager.java +++ b/core/src/main/java/com/opensymphony/xwork2/util/fs/DefaultFileManager.java @@ -40,7 +40,7 @@ public class DefaultFileManager implements FileManager { private static final Pattern JAR_PATTERN = Pattern.compile("^(jar:|wsjar:|zip:|vfsfile:|code-source:)?(file:)?(.*?)(\\!/|\\.jar/)(.*)"); private static final int JAR_FILE_PATH = 3; - protected static Map files = Collections.synchronizedMap(new HashMap()); + protected static final Map files = Collections.synchronizedMap(new HashMap()); protected boolean reloadingConfigs = false; @@ -48,6 +48,22 @@ public class DefaultFileManager implements FileManager { } public void setReloadingConfigs(boolean reloadingConfigs) { + if (reloadingConfigs && !this.reloadingConfigs) { + this.reloadingConfigs = true; + //starting monitoring not-monitored loaded files on demand + synchronized (files) { + for (String fileName : + files.keySet()) { + if (null == files.get(fileName)) { + try { + monitorFile(new URL(fileName)); + } catch (MalformedURLException e) { + LOG.warn("Error creating URL from [{}]!", fileName, e); + } + } + } + } + } this.reloadingConfigs = reloadingConfigs; } @@ -70,9 +86,7 @@ public class DefaultFileManager implements FileManager { return null; } InputStream is = openFile(fileUrl); - if (reloadingConfigs) { - monitorFile(fileUrl); - } + monitorFile(fileUrl); return is; } @@ -90,6 +104,10 @@ public class DefaultFileManager implements FileManager { public void monitorFile(URL fileUrl) { String fileName = fileUrl.toString(); + if (!reloadingConfigs) { + files.put(fileName, null); + return; + } Revision revision; LOG.debug("Creating revision for URL: {}", fileName); if (isJarURL(fileUrl)) { diff --git a/core/src/main/java/org/apache/struts2/util/fs/JBossFileManager.java b/core/src/main/java/org/apache/struts2/util/fs/JBossFileManager.java index b3a19a79f..7b7de30ca 100644 --- a/core/src/main/java/org/apache/struts2/util/fs/JBossFileManager.java +++ b/core/src/main/java/org/apache/struts2/util/fs/JBossFileManager.java @@ -81,7 +81,7 @@ public class JBossFileManager extends DefaultFileManager { @Override public void monitorFile(URL fileUrl) { - if (isJBossUrl(fileUrl)) { + if (reloadingConfigs && isJBossUrl(fileUrl)) { String fileName = fileUrl.toString(); LOG.debug("Creating revision for URL: {}", fileName); URL normalizedUrl = normalizeToFileProtocol(fileUrl); diff --git a/core/src/test/java/com/opensymphony/xwork2/config/providers/XmlConfigurationProviderTest.java b/core/src/test/java/com/opensymphony/xwork2/config/providers/XmlConfigurationProviderTest.java index b66c2ff57..63f9b5392 100644 --- a/core/src/test/java/com/opensymphony/xwork2/config/providers/XmlConfigurationProviderTest.java +++ b/core/src/test/java/com/opensymphony/xwork2/config/providers/XmlConfigurationProviderTest.java @@ -80,10 +80,10 @@ public class XmlConfigurationProviderTest extends ConfigurationTestBase { public void testNeedsReload() throws Exception { final String filename = "com/opensymphony/xwork2/config/providers/xwork-test-actions.xml"; ConfigurationProvider provider = new XmlConfigurationProvider(filename, true); - container.getInstance(FileManagerFactory.class).getFileManager().setReloadingConfigs(true); container.inject(provider); provider.init(configuration); provider.loadPackages(); + container.getInstance(FileManagerFactory.class).getFileManager().setReloadingConfigs(true); assertFalse(provider.needsReload()); // Revision exists and timestamp didn't change @@ -176,10 +176,10 @@ public class XmlConfigurationProviderTest extends ConfigurationTestBase { public void testEmptySpaces() throws Exception { final String filename = "com/opensymphony/xwork2/config/providers/xwork- test.xml"; ConfigurationProvider provider = new XmlConfigurationProvider(filename, true); - container.getInstance(FileManagerFactory.class).getFileManager().setReloadingConfigs(true); container.inject(provider); provider.init(configuration); provider.loadPackages(); + container.getInstance(FileManagerFactory.class).getFileManager().setReloadingConfigs(true); assertFalse(provider.needsReload()); @@ -215,7 +215,6 @@ public class XmlConfigurationProviderTest extends ConfigurationTestBase { } public void testConfigsInJarFiles() throws Exception { - container.getInstance(FileManagerFactory.class).getFileManager().setReloadingConfigs(true); testProvider("xwork-jar.xml"); testProvider("xwork-zip.xml"); testProvider("xwork - jar.xml"); @@ -229,7 +228,8 @@ public class XmlConfigurationProviderTest extends ConfigurationTestBase { private void testProvider(String configFile) throws Exception { ConfigurationProvider provider = buildConfigurationProvider(configFile); - assertTrue(!provider.needsReload()); + container.getInstance(FileManagerFactory.class).getFileManager().setReloadingConfigs(true); + assertFalse(provider.needsReload()); String fullPath = ClassLoaderUtil.getResource(configFile, ConfigurationProvider.class).toString(); @@ -241,9 +241,9 @@ public class XmlConfigurationProviderTest extends ConfigurationTestBase { File file = new File(jar); assertTrue("File [" + file + "] doesn't exist!", file.exists()); - file.setLastModified(System.currentTimeMillis()); + changeFileTime(file); - assertTrue(!provider.needsReload()); + assertFalse(provider.needsReload()); } public void testIncludeWithWildcard() throws Exception { From ecb7beef38ffb011e84d98f7a1699a0152031238 Mon Sep 17 00:00:00 2001 From: Yasser Zamani Date: Thu, 1 Nov 2018 15:17:02 +0330 Subject: [PATCH 2/2] not miss container provided fileUrl and ... lazy monitoring for performance. Fix and test stopping reload configs at runtime. Fixes WW-4974 --- .../xwork2/util/fs/DefaultFileManager.java | 21 +++++------- .../XmlConfigurationProviderTest.java | 33 +++++++++++++++++++ .../config/providers/xwork-test-reload.xml | 31 +++++++++++++++++ 3 files changed, 73 insertions(+), 12 deletions(-) create mode 100644 core/src/test/resources/com/opensymphony/xwork2/config/providers/xwork-test-reload.xml diff --git a/core/src/main/java/com/opensymphony/xwork2/util/fs/DefaultFileManager.java b/core/src/main/java/com/opensymphony/xwork2/util/fs/DefaultFileManager.java index d39e9144e..5708bd5f2 100644 --- a/core/src/main/java/com/opensymphony/xwork2/util/fs/DefaultFileManager.java +++ b/core/src/main/java/com/opensymphony/xwork2/util/fs/DefaultFileManager.java @@ -41,6 +41,7 @@ public class DefaultFileManager implements FileManager { private static final int JAR_FILE_PATH = 3; protected static final Map files = Collections.synchronizedMap(new HashMap()); + private static final List lazyMonitoredFilesCache = Collections.synchronizedList(new ArrayList()); protected boolean reloadingConfigs = false; @@ -49,19 +50,13 @@ public class DefaultFileManager implements FileManager { public void setReloadingConfigs(boolean reloadingConfigs) { if (reloadingConfigs && !this.reloadingConfigs) { + //starting monitoring cached not-monitored files (lazy monitoring on demand because of performance) this.reloadingConfigs = true; - //starting monitoring not-monitored loaded files on demand - synchronized (files) { - for (String fileName : - files.keySet()) { - if (null == files.get(fileName)) { - try { - monitorFile(new URL(fileName)); - } catch (MalformedURLException e) { - LOG.warn("Error creating URL from [{}]!", fileName, e); - } - } + synchronized (lazyMonitoredFilesCache) { + for (URL fileUrl : lazyMonitoredFilesCache) { + monitorFile(fileUrl); } + lazyMonitoredFilesCache.clear(); } } this.reloadingConfigs = reloadingConfigs; @@ -105,7 +100,9 @@ public class DefaultFileManager implements FileManager { public void monitorFile(URL fileUrl) { String fileName = fileUrl.toString(); if (!reloadingConfigs) { - files.put(fileName, null); + //reserve file for monitoring on demand because of performance + files.remove(fileName); + lazyMonitoredFilesCache.add(fileUrl); return; } Revision revision; diff --git a/core/src/test/java/com/opensymphony/xwork2/config/providers/XmlConfigurationProviderTest.java b/core/src/test/java/com/opensymphony/xwork2/config/providers/XmlConfigurationProviderTest.java index 63f9b5392..faa6b937a 100644 --- a/core/src/test/java/com/opensymphony/xwork2/config/providers/XmlConfigurationProviderTest.java +++ b/core/src/test/java/com/opensymphony/xwork2/config/providers/XmlConfigurationProviderTest.java @@ -31,6 +31,9 @@ import java.io.File; import java.io.IOException; import java.net.URI; import java.net.URL; +import java.nio.file.Files; +import java.nio.file.Path; +import java.nio.file.Paths; import java.util.ArrayList; import java.util.Iterator; import java.util.List; @@ -94,6 +97,36 @@ public class XmlConfigurationProviderTest extends ConfigurationTestBase { assertTrue(provider.needsReload()); } + public void testReload() throws Exception { + final String filename = "com/opensymphony/xwork2/config/providers/xwork-test-reload.xml"; + ConfigurationProvider provider = new XmlConfigurationProvider(filename, true); + loadConfigurationProviders(provider); + + assertFalse(provider.needsReload()); // Revision exists and timestamp didn't change + + File file = new File(getClass().getResource("/" + filename).toURI()); + assertTrue("not exists: " + file.toString(), file.exists()); + + Path configPath = Paths.get(file.getAbsolutePath()); + String content = new String(Files.readAllBytes(configPath)); + content = content.replaceAll("", + ""); + Files.write(configPath, content.getBytes()); // user demand: stop reloading configs + + try { + assertTrue(provider.needsReload()); // config file has changed in previous lines + + configurationManager.reload(); + + changeFileTime(file); + assertFalse(provider.needsReload()); // user already has stopped reloading configs + } finally { + content = content.replaceAll("", + ""); + Files.write(configPath, content.getBytes()); + } + } + public void testNeedsReloadNotReloadingConfigs() throws Exception { final String filename = "com/opensymphony/xwork2/config/providers/xwork-test-actions.xml"; buildConfigurationProvider(filename); diff --git a/core/src/test/resources/com/opensymphony/xwork2/config/providers/xwork-test-reload.xml b/core/src/test/resources/com/opensymphony/xwork2/config/providers/xwork-test-reload.xml new file mode 100644 index 000000000..ffd575523 --- /dev/null +++ b/core/src/test/resources/com/opensymphony/xwork2/config/providers/xwork-test-reload.xml @@ -0,0 +1,31 @@ + + + + + + + + +