This is an automated email from the ASF dual-hosted git repository. juanpablo pushed a commit to branch master in repository https://gitbox.apache.org/repos/asf/jspwiki.git
commit ab2155b70e37c4eafa26b82dc9e610fcabfdc25e Author: Juan Pablo Santos RodrÃguez <[email protected]> AuthorDate: Tue Mar 22 12:04:19 2022 +0100 refactor working directory checks to separate methods, fixing a couple of sonarqube issues --- .../src/main/java/org/apache/wiki/WikiEngine.java | 64 +++++++++++----------- .../test/java/org/apache/wiki/WikiEngineTest.java | 59 ++++++++++---------- 2 files changed, 63 insertions(+), 60 deletions(-) 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 f2506a5..c1d1212 100644 --- a/jspwiki-main/src/main/java/org/apache/wiki/WikiEngine.java +++ b/jspwiki-main/src/main/java/org/apache/wiki/WikiEngine.java @@ -259,42 +259,13 @@ public class WikiEngine implements Engine { LOG.debug( "Configuring WikiEngine..." ); - // Create and find the default working directory. - m_workDir = TextUtil.getStringProperty( props, PROP_WORKDIR, null ); - - if( m_workDir == null ) { - m_workDir = System.getProperty( "java.io.tmpdir", "." ) + File.separator + Release.APPNAME + "-" + m_appid; - } - - try { - final File f = new File( m_workDir ); - f.mkdirs(); + createAndFindWorkingDirectory( props ); - // A bunch of sanity checks - if( !f.exists() ) { - throw new WikiException( "Work directory does not exist: " + m_workDir ); - } - if( !f.canRead() ) { - throw new WikiException( "No permission to read work directory: " + m_workDir ); - } - if( !f.canWrite() ) { - throw new WikiException( "No permission to write to work directory: " + m_workDir ); - } - if( !f.isDirectory() ) { - throw new WikiException( "jspwiki.workDir does not point to a directory: " + m_workDir ); - } - } catch( final SecurityException e ) { - LOG.fatal( "Unable to find or create the working directory: {}", m_workDir, e ); - throw new WikiException( "Unable to find or create the working dir: " + m_workDir, e ); - } - - LOG.info( "JSPWiki working directory is '{}'", m_workDir ); - - m_saveUserInfo = TextUtil.getBooleanProperty( props, PROP_STOREUSERNAME, m_saveUserInfo ); m_useUTF8 = StandardCharsets.UTF_8.name().equals( TextUtil.getStringProperty( props, PROP_ENCODING, StandardCharsets.ISO_8859_1.name() ) ); + m_saveUserInfo = TextUtil.getBooleanProperty( props, PROP_STOREUSERNAME, m_saveUserInfo ); + m_frontPage = TextUtil.getStringProperty( props, PROP_FRONTPAGE, "Main" ); m_templateDir = TextUtil.getStringProperty( props, PROP_TEMPLATEDIR, "default" ); enforceValidTemplateDirectory(); - m_frontPage = TextUtil.getStringProperty( props, PROP_FRONTPAGE, "Main" ); // // Initialize the important modules. Any exception thrown by the managers means that we will not start up. @@ -375,6 +346,35 @@ public class WikiEngine implements Engine { m_isConfigured = true; } + 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 { + f.mkdirs(); + } catch( final SecurityException e ) { + LOG.fatal( "Unable to find or create the working directory: {}", m_workDir, e ); + throw new WikiException( "Unable to find or create the working dir: " + m_workDir, e ); + } + + // A bunch of sanity checks + checkWorkingDirectory( !f.exists(), "Work directory does not exist: " + m_workDir ); + checkWorkingDirectory( !f.canRead(), "No permission to read work directory: " + m_workDir ); + checkWorkingDirectory( !f.canWrite(), "No permission to write to work directory: " + m_workDir ); + checkWorkingDirectory( !f.isDirectory(), "jspwiki.workDir does not point to a directory: " + m_workDir ); + + LOG.info( "JSPWiki working directory is '{}'", m_workDir ); + } + + void checkWorkingDirectory( final boolean condition, final String errMsg ) throws WikiException { + if( condition ) { + throw new WikiException( errMsg ); + } + } + void initExtraComponents( final Map< String, String > extraComponents ) { for( final Map.Entry< String, String > extraComponent : extraComponents.entrySet() ) { try { diff --git a/jspwiki-main/src/test/java/org/apache/wiki/WikiEngineTest.java b/jspwiki-main/src/test/java/org/apache/wiki/WikiEngineTest.java index 4a6dc03..402e52d 100644 --- a/jspwiki-main/src/test/java/org/apache/wiki/WikiEngineTest.java +++ b/jspwiki-main/src/test/java/org/apache/wiki/WikiEngineTest.java @@ -16,13 +16,13 @@ specific language governing permissions and limitations under the License. */ - package org.apache.wiki; import org.apache.wiki.api.core.Attachment; import org.apache.wiki.api.core.Context; import org.apache.wiki.api.core.Page; import org.apache.wiki.api.engine.RenderApi; +import org.apache.wiki.api.exceptions.WikiException; import org.apache.wiki.api.spi.Wiki; import org.apache.wiki.attachment.AttachmentManager; import org.apache.wiki.cache.CachingManager; @@ -34,7 +34,6 @@ import org.apache.wiki.references.ReferenceManager; import org.apache.wiki.render.RenderingManager; import org.junit.jupiter.api.AfterEach; import org.junit.jupiter.api.Assertions; -import org.junit.jupiter.api.BeforeEach; import org.junit.jupiter.api.Test; import java.io.File; @@ -43,21 +42,18 @@ import java.util.Collection; import java.util.Iterator; import java.util.Properties; -public class WikiEngineTest { +import static org.apache.wiki.TestEngine.with; - public static final String NAME1 = "Test1"; - Properties props = TestEngine.getTestProperties(); - TestEngine m_engine; +class WikiEngineTest { - @BeforeEach - public void setUp() { - props.setProperty( WikiEngine.PROP_MATCHPLURALS, "true" ); - m_engine = TestEngine.build( props ); - } + static final String NAME1 = "Test1"; + + Properties props = TestEngine.getTestProperties(); + TestEngine m_engine = TestEngine.build( with( WikiEngine.PROP_MATCHPLURALS, "true" ) ); @AfterEach - public void tearDown() { + void tearDown() { final String files = m_engine.getWikiProperties().getProperty( FileSystemProvider.PROP_PAGEDIR ); if( files != null ) { @@ -70,7 +66,7 @@ public class WikiEngineTest { } @Test - public void testNonExistentDirectory() throws Exception { + void testNonExistentDirectory() throws Exception { final String newdir = "." + File.separator + "target" + File.separator + "non-existent-directory"; props.setProperty( FileSystemProvider.PROP_PAGEDIR, newdir ); @@ -84,7 +80,7 @@ public class WikiEngineTest { } @Test - public void testFinalPageName() throws Exception { + void testFinalPageName() throws Exception { m_engine.saveText( "Foobar", "1" ); m_engine.saveText( "Foobars", "2" ); @@ -93,7 +89,7 @@ public class WikiEngineTest { } @Test - public void testFinalPageNameSingular() throws Exception { + void testFinalPageNameSingular() throws Exception { m_engine.saveText( "Foobar", "1" ); Assertions.assertEquals( "Foobar", m_engine.getFinalPageName( "Foobars" ), "plural mistake" ); @@ -101,7 +97,7 @@ public class WikiEngineTest { } @Test - public void testFinalPageNamePlural() throws Exception { + void testFinalPageNamePlural() throws Exception { m_engine.saveText( "Foobars", "1" ); Assertions.assertEquals( "Foobars", m_engine.getFinalPageName( "Foobars" ), "plural mistake" ); @@ -109,13 +105,13 @@ public class WikiEngineTest { } @Test - public void testEncodeNameLatin1() { + void testEncodeNameLatin1() { final String name = "abc\u00e5\u00e4\u00f6"; Assertions.assertEquals( "abc%E5%E4%F6", m_engine.encodeName(name) ); } @Test - public void testEncodeNameUTF8() throws Exception { + void testEncodeNameUTF8() throws Exception { final String name = "\u0041\u2262\u0391\u002E"; props.setProperty( WikiEngine.PROP_ENCODING, StandardCharsets.UTF_8.name() ); final WikiEngine engine = new TestEngine( props ); @@ -127,7 +123,7 @@ public class WikiEngineTest { * Checks, if ReferenceManager is informed of new attachments. */ @Test - public void testAttachmentRefs() throws Exception { + void testAttachmentRefs() throws Exception { final ReferenceManager refMgr = m_engine.getManager( ReferenceManager.class ); final AttachmentManager attMgr = m_engine.getManager( AttachmentManager.class ); m_engine.saveText( NAME1, "fooBar"); @@ -170,7 +166,7 @@ public class WikiEngineTest { */ @Test - public void testAttachmentRefs2() throws Exception { + void testAttachmentRefs2() throws Exception { final ReferenceManager refMgr = m_engine.getManager( ReferenceManager.class ); final AttachmentManager attMgr = m_engine.getManager( AttachmentManager.class ); @@ -211,7 +207,7 @@ public class WikiEngineTest { * Checks, if ReferenceManager is informed if a link to an attachment is added. */ @Test - public void testAttachmentRefs3() throws Exception { + void testAttachmentRefs3() throws Exception { final ReferenceManager refMgr = m_engine.getManager( ReferenceManager.class ); final AttachmentManager attMgr = m_engine.getManager( AttachmentManager.class ); @@ -236,7 +232,7 @@ public class WikiEngineTest { * Checks, if ReferenceManager is informed if a third page references an attachment. */ @Test - public void testAttachmentRefs4() throws Exception { + void testAttachmentRefs4() throws Exception { final ReferenceManager refMgr = m_engine.getManager( ReferenceManager.class ); final AttachmentManager attMgr = m_engine.getManager( AttachmentManager.class ); @@ -260,7 +256,7 @@ public class WikiEngineTest { * Tests BugReadingOfVariableNotWorkingForOlderVersions */ @Test - public void testOldVersionVars() throws Exception { + void testOldVersionVars() throws Exception { final Properties props = TestEngine.getTestProperties("/jspwiki-vers-custom.properties"); props.setProperty( CachingManager.PROP_CACHE_ENABLE, "true" ); final TestEngine engine = new TestEngine( props ); @@ -277,13 +273,13 @@ public class WikiEngineTest { } @Test - public void testSpacedNames1() throws Exception { + void testSpacedNames1() throws Exception { m_engine.saveText("This is a test", "puppaa"); Assertions.assertEquals( "puppaa", m_engine.getManager( PageManager.class ).getText("This is a test").trim(), "normal" ); } @Test - public void testParsedVariables() throws Exception { + void testParsedVariables() throws Exception { m_engine.saveText( "TestPage", "[{SET foo=bar}][{SamplePlugin text='{$foo}'}]"); final String res = m_engine.getManager( RenderingManager.class ).getHTML( "TestPage" ); @@ -294,7 +290,7 @@ public class WikiEngineTest { * Tests BugReferenceToRenamedPageNotCleared */ @Test - public void testRename() throws Exception { + void testRename() throws Exception { m_engine.saveText( "RenameBugTestPage", "Mary had a little generic object" ); m_engine.saveText( "OldNameTestPage", "Linked to RenameBugTestPage" ); @@ -313,7 +309,7 @@ public class WikiEngineTest { } @Test - public void testChangeNoteOldVersion2() throws Exception { + void testChangeNoteOldVersion2() throws Exception { final Page p = Wiki.contents().page( m_engine, NAME1 ); final Context context = Wiki.context().create( m_engine,p ); context.getPage().setAttribute( Page.CHANGENOTE, "Test change" ); @@ -331,7 +327,7 @@ public class WikiEngineTest { } @Test - public void testGetManagers() { + void testGetManagers() { Assertions.assertNull( m_engine.getManager( String.class ) ); Assertions.assertNotNull( m_engine.getManager( RenderApi.class ) ); Assertions.assertNotNull( m_engine.getManager( PageManager.class ) ); @@ -343,4 +339,11 @@ public class WikiEngineTest { Assertions.assertEquals( 4, m_engine.getManagers( ModuleManager.class ).size() ); } + @Test + void testCheckWorkingDirectory() { + Assertions.assertDoesNotThrow( () -> m_engine.checkWorkingDirectory( false, "boo" ) ); + final WikiException we = Assertions.assertThrows( WikiException.class, () -> m_engine.checkWorkingDirectory( true, "boo" ) ); + Assertions.assertEquals( "boo", we.getMessage() ); + } + }
