From f1cb2f633a792d58aff3d864c5e75ca9a619f86e Mon Sep 17 00:00:00 2001 From: Arturo Bernal Date: Sat, 3 Jun 2023 23:24:58 +0200 Subject: [PATCH] [JSPWIKI-1172] Improve PropertyReader and Installer for workDir setup This commit makes a few key changes: 1. PropertyReader now checks for the existence of jspwiki.workDir. If it's missing, the servlet context's temporary directory (or the system's temporary directory if that's unavailable) is set as the working directory. 2. Removed direct usage of java.io.tmpdir in WikiEngine since jspwiki.workDir is now ensured to be set in PropertyReader. 3. Refactored the Installer to use wiki properties (specifically, jspwiki.workDir) instead of directly using java.io.tmpdir. These changes streamline the handling of the working directory and ensure that it's consistently sourced from the same place (the wiki properties). --- .../org/apache/wiki/api/spi/WikiTest.java | 2 +- .../WikiBootstrapServletContextListener.java | 3 +- ...kiBootstrapServletContextListenerTest.java | 5 +-- .../main/java/org/apache/wiki/WikiEngine.java | 3 -- .../java/org/apache/wiki/ui/Installer.java | 3 +- jspwiki-util/pom.xml | 7 ++++ .../org/apache/wiki/util/PropertyReader.java | 32 +++++++++++++++++++ .../apache/wiki/util/PropertyReaderTest.java | 31 ++++++++++++++++++ 8 files changed, 78 insertions(+), 8 deletions(-) diff --git a/jspwiki-api/src/test/java/org/apache/wiki/api/spi/WikiTest.java b/jspwiki-api/src/test/java/org/apache/wiki/api/spi/WikiTest.java index 1edbbba432..06ca0b3cbe 100644 --- a/jspwiki-api/src/test/java/org/apache/wiki/api/spi/WikiTest.java +++ b/jspwiki-api/src/test/java/org/apache/wiki/api/spi/WikiTest.java @@ -43,7 +43,7 @@ public class WikiTest { public void testWikiInit() { Mockito.doReturn( sc ).when( conf ).getServletContext(); final Properties properties = Wiki.init( sc ); - Assertions.assertEquals( 5, properties.size() ); + Assertions.assertEquals( 6, properties.size() ); // verify SPIs are initialized and can be invoked Assertions.assertNull( Wiki.acls().acl() ); diff --git a/jspwiki-bootstrap/src/main/java/org/apache/wiki/bootstrap/WikiBootstrapServletContextListener.java b/jspwiki-bootstrap/src/main/java/org/apache/wiki/bootstrap/WikiBootstrapServletContextListener.java index 6ce8428486..f2332ca7e5 100644 --- a/jspwiki-bootstrap/src/main/java/org/apache/wiki/bootstrap/WikiBootstrapServletContextListener.java +++ b/jspwiki-bootstrap/src/main/java/org/apache/wiki/bootstrap/WikiBootstrapServletContextListener.java @@ -27,10 +27,12 @@ Licensed to the Apache Software Foundation (ASF) under one import org.apache.wiki.api.spi.Wiki; import org.apache.wiki.util.TextUtil; +import javax.servlet.ServletContext; import javax.servlet.ServletContextEvent; import javax.servlet.ServletContextListener; import java.io.ByteArrayInputStream; import java.io.ByteArrayOutputStream; +import java.io.File; import java.io.IOException; import java.io.InputStream; import java.util.Properties; @@ -97,5 +99,4 @@ ConfigurationSource createConfigurationSource( final Properties properties ) { @Override public void contextDestroyed( final ServletContextEvent sce ) { } - } diff --git a/jspwiki-bootstrap/src/test/java/org/apache/wiki/bootstrap/WikiBootstrapServletContextListenerTest.java b/jspwiki-bootstrap/src/test/java/org/apache/wiki/bootstrap/WikiBootstrapServletContextListenerTest.java index bdc6cc0178..866563ecd8 100644 --- a/jspwiki-bootstrap/src/test/java/org/apache/wiki/bootstrap/WikiBootstrapServletContextListenerTest.java +++ b/jspwiki-bootstrap/src/test/java/org/apache/wiki/bootstrap/WikiBootstrapServletContextListenerTest.java @@ -28,6 +28,8 @@ Licensed to the Apache Software Foundation (ASF) under one import javax.servlet.ServletContextEvent; import java.util.Properties; +import static org.junit.jupiter.api.Assertions.assertEquals; + @ExtendWith( MockitoExtension.class ) public class WikiBootstrapServletContextListenerTest { @@ -41,7 +43,7 @@ public void testWikiInit() { final WikiBootstrapServletContextListener listener = new WikiBootstrapServletContextListener(); final Properties properties = listener.initWikiSPIs( sce ); - Assertions.assertEquals( 35, properties.size() ); + Assertions.assertEquals( 36, properties.size() ); } @Test @@ -62,5 +64,4 @@ public void testServletContextListenerLifeCycle() { Assertions.assertDoesNotThrow( () -> listener.contextInitialized( sce ) ); Assertions.assertDoesNotThrow( () -> listener.contextDestroyed( sce ) ); } - } diff --git a/jspwiki-main/src/main/java/org/apache/wiki/WikiEngine.java b/jspwiki-main/src/main/java/org/apache/wiki/WikiEngine.java index 9830ab0648..4ec79d82e0 100644 --- a/jspwiki-main/src/main/java/org/apache/wiki/WikiEngine.java +++ b/jspwiki-main/src/main/java/org/apache/wiki/WikiEngine.java @@ -348,9 +348,6 @@ public void initialize( final Properties props ) throws WikiException { void createAndFindWorkingDirectory( final Properties props ) throws WikiException { m_workDir = TextUtil.getStringProperty( props, PROP_WORKDIR, null ); - if( StringUtils.isBlank( m_workDir ) ) { - m_workDir = System.getProperty( "java.io.tmpdir", "." ) + File.separator + Release.APPNAME + "-" + m_appid; - } final File f = new File( m_workDir ); try { diff --git a/jspwiki-main/src/main/java/org/apache/wiki/ui/Installer.java b/jspwiki-main/src/main/java/org/apache/wiki/ui/Installer.java index c1c4f5c95a..570d9722e0 100644 --- a/jspwiki-main/src/main/java/org/apache/wiki/ui/Installer.java +++ b/jspwiki-main/src/main/java/org/apache/wiki/ui/Installer.java @@ -66,7 +66,7 @@ public class Installer { public static final String WORK_DIR = Engine.PROP_WORKDIR; public static final String ADMIN_GROUP = "Admin"; public static final String PROPFILENAME = "jspwiki-custom.properties" ; - public static final String TMP_DIR = System.getProperty("java.io.tmpdir"); + public static String TMP_DIR; private final Session m_session; private final File m_propertyFile; private final Properties m_props; @@ -86,6 +86,7 @@ public Installer( final HttpServletRequest request, final ServletConfig config ) // Stash the request m_request = request; m_validated = false; + TMP_DIR = m_engine.getWikiProperties().getProperty( "jspwiki.workDir" ); } /** diff --git a/jspwiki-util/pom.xml b/jspwiki-util/pom.xml index af3e1a57ed..df551f83f5 100644 --- a/jspwiki-util/pom.xml +++ b/jspwiki-util/pom.xml @@ -94,5 +94,12 @@ junit-jupiter-engine test + + + org.mockito + mockito-core + test + + diff --git a/jspwiki-util/src/main/java/org/apache/wiki/util/PropertyReader.java b/jspwiki-util/src/main/java/org/apache/wiki/util/PropertyReader.java index 3d1b27d9a5..ac1fa91392 100644 --- a/jspwiki-util/src/main/java/org/apache/wiki/util/PropertyReader.java +++ b/jspwiki-util/src/main/java/org/apache/wiki/util/PropertyReader.java @@ -139,6 +139,9 @@ public static Properties loadWebAppProps( final ServletContext context ) { // now load the cascade (new in 2.5) loadWebAppPropsCascade( context, props ); + // sets the JSPWiki working directory (jspwiki.workDir) + setWorkDir( context, props ); + // add system properties beginning with jspwiki... final Map< String, String > sysprops = collectPropertiesFrom( System.getProperties().entrySet().stream() .collect( Collectors.toMap( Object::toString, Object::toString ) ) ); @@ -393,4 +396,33 @@ static String createResourceLocation( final String path, final String name ) { return result.toString(); } + /** + * This method sets the JSPWiki working directory (jspwiki.workDir). It first checks if this property + * is already set. If it isn't, it attempts to use the servlet container's temporary directory + * (javax.servlet.context.tempdir). If that is also unavailable, it defaults to the system's temporary + * directory (java.io.tmpdir). + *

+ * This method is package-private to allow for unit testing. + * + * @param properties the JSPWiki properties + * @param servletContext the Servlet context from which to fetch the tempdir if needed + * @since JSPWiki 2.11.1 + */ + static void setWorkDir( final ServletContext servletContext, final Properties properties ) { + final String workDir = TextUtil.getStringProperty(properties, "jspwiki.workDir", null); + if (workDir == null) { + final File tempDir = (File) servletContext.getAttribute("javax.servlet.context.tempdir"); + if (tempDir != null) { + properties.setProperty("jspwiki.workDir", tempDir.getAbsolutePath()); + LOG.info("Setting jspwiki.workDir to ServletContext's temporary directory: {}", tempDir.getAbsolutePath()); + } else { + final String defaultTmpDir = System.getProperty("java.io.tmpdir"); + properties.setProperty("jspwiki.workDir", defaultTmpDir); + LOG.info("ServletContext's temporary directory not found. Setting jspwiki.workDir to system's temporary directory: {}", defaultTmpDir); + } + } else { + LOG.info("jspwiki.workDir is already set to: {}", workDir); + } + } + } diff --git a/jspwiki-util/src/test/java/org/apache/wiki/util/PropertyReaderTest.java b/jspwiki-util/src/test/java/org/apache/wiki/util/PropertyReaderTest.java index 09358a0ecb..1d208db179 100644 --- a/jspwiki-util/src/test/java/org/apache/wiki/util/PropertyReaderTest.java +++ b/jspwiki-util/src/test/java/org/apache/wiki/util/PropertyReaderTest.java @@ -22,11 +22,16 @@ Licensed to the Apache Software Foundation (ASF) under one import org.junit.jupiter.api.Assertions; import org.junit.jupiter.api.Test; +import org.mockito.Mockito; +import javax.servlet.ServletContext; +import java.io.File; import java.util.HashMap; import java.util.Map; import java.util.Properties; +import static org.mockito.Mockito.mock; + /** * Unit test for PropertyReader. @@ -122,4 +127,30 @@ public void testCollectPropertiesFrom() { Assertions.assertNull( test.get( "secretEnv" ) ); } + @Test + public void testSetWorkDir() { + final Properties properties = new Properties(); + final ServletContext servletContext = mock(ServletContext.class); + Mockito.when(servletContext.getAttribute("javax.servlet.context.tempdir")).thenReturn(new File("/tmp")); + + PropertyReader.setWorkDir(servletContext, properties); + + // Test when the "jspwiki.workDir" is not set, it should get set to servlet's temporary directory + String workDir = properties.getProperty("jspwiki.workDir"); + Assertions.assertEquals("/tmp", workDir); + + // Test when the "jspwiki.workDir" is set, it should remain as it is + properties.setProperty("jspwiki.workDir", "/custom/dir"); + PropertyReader.setWorkDir(servletContext, properties); + workDir = properties.getProperty("jspwiki.workDir"); + Assertions.assertEquals("/custom/dir", workDir); + + // Test when the servlet's temporary directory is null, it should get set to system's temporary directory + Mockito.when(servletContext.getAttribute("javax.servlet.context.tempdir")).thenReturn(null); + properties.remove("jspwiki.workDir"); + PropertyReader.setWorkDir(servletContext, properties); + workDir = properties.getProperty("jspwiki.workDir"); + Assertions.assertEquals(System.getProperty("java.io.tmpdir"), workDir); + } + }