diff --git a/CHANGELOG.md b/CHANGELOG.md index 314f94b374..de732d6da1 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -8,6 +8,8 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ## Unreleased ### Fixed - Null Pointer Exception on anonymous migration with deleted repositories ([#1371](https://github.com/scm-manager/scm-manager/pull/1371)) +- Null Pointer Exception on parsing SVN properties ([#1373](https://github.com/scm-manager/scm-manager/pull/1373)) + ### Changed - Reduced logging for invalid JWT or api keys ([#1374](https://github.com/scm-manager/scm-manager/pull/1374)) diff --git a/scm-plugins/scm-svn-plugin/src/main/java/sonia/scm/repository/spi/SvnBrowseCommand.java b/scm-plugins/scm-svn-plugin/src/main/java/sonia/scm/repository/spi/SvnBrowseCommand.java index 00a135dd82..e83a0dc365 100644 --- a/scm-plugins/scm-svn-plugin/src/main/java/sonia/scm/repository/spi/SvnBrowseCommand.java +++ b/scm-plugins/scm-svn-plugin/src/main/java/sonia/scm/repository/spi/SvnBrowseCommand.java @@ -193,23 +193,28 @@ public class SvnBrowseCommand extends AbstractSvnCommand repository.getDir(entry.getRelativePath(), revision, properties, (Collection) null); - String[] externals = properties.getStringValue(SVNProperty.EXTERNALS).split("\\r?\\n"); - for (String external : externals) { - String subRepoUrl = ""; - String subRepoPath = ""; - for (String externalPart : external.split(" ")) { - if (shouldSetExternal(externalPart)) { - subRepoUrl = externalPart; - } else if (!externalPart.contains("-r")) { - subRepoPath = externalPart; + String externals = properties.getStringValue(SVNProperty.EXTERNALS); + + if (!Strings.isNullOrEmpty(externals)) { + String[] splitExternals = externals.split("\\r?\\n"); + for (String external : splitExternals) { + String subRepoUrl = ""; + String subRepoPath = ""; + for (String externalPart : external.split(" ")) { + if (shouldSetExternal(externalPart)) { + subRepoUrl = externalPart; + } else if (!externalPart.contains("-r")) { + subRepoPath = externalPart; + } + } + + if (Util.isNotEmpty(external)) { + SubRepository subRepository = new SubRepository(subRepoUrl); + fileObject.addChild(createSubRepoDirectory(subRepository, subRepoPath)); } } - - if (Util.isNotEmpty(external)) { - SubRepository subRepository = new SubRepository(subRepoUrl); - fileObject.addChild(createSubRepoDirectory(subRepository, subRepoPath)); - } } + } catch (SVNException ex) { logger.error("could not fetch file properties", ex); } diff --git a/scm-plugins/scm-svn-plugin/src/test/java/sonia/scm/repository/spi/AbstractSvnCommandTestBase.java b/scm-plugins/scm-svn-plugin/src/test/java/sonia/scm/repository/spi/AbstractSvnCommandTestBase.java index b6d9e4ba26..8228d42f39 100644 --- a/scm-plugins/scm-svn-plugin/src/test/java/sonia/scm/repository/spi/AbstractSvnCommandTestBase.java +++ b/scm-plugins/scm-svn-plugin/src/test/java/sonia/scm/repository/spi/AbstractSvnCommandTestBase.java @@ -21,15 +21,13 @@ * OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE * SOFTWARE. */ - + package sonia.scm.repository.spi; //~--- non-JDK imports -------------------------------------------------------- import org.junit.After; -//~--- JDK imports ------------------------------------------------------------ - import java.io.IOException; /** diff --git a/scm-plugins/scm-svn-plugin/src/test/java/sonia/scm/repository/spi/SvnBrowseCommandTest.java b/scm-plugins/scm-svn-plugin/src/test/java/sonia/scm/repository/spi/SvnBrowseCommandTest.java index 3d3a3131e5..698fec8f37 100644 --- a/scm-plugins/scm-svn-plugin/src/test/java/sonia/scm/repository/spi/SvnBrowseCommandTest.java +++ b/scm-plugins/scm-svn-plugin/src/test/java/sonia/scm/repository/spi/SvnBrowseCommandTest.java @@ -24,10 +24,20 @@ package sonia.scm.repository.spi; +import org.junit.Rule; import org.junit.Test; +import org.junit.rules.TemporaryFolder; +import org.tmatesoft.svn.core.SVNDepth; +import org.tmatesoft.svn.core.SVNException; +import org.tmatesoft.svn.core.SVNPropertyValue; +import org.tmatesoft.svn.core.SVNURL; +import org.tmatesoft.svn.core.wc.SVNClientManager; +import org.tmatesoft.svn.core.wc.SVNRevision; import sonia.scm.repository.BrowserResult; import sonia.scm.repository.FileObject; +import sonia.scm.repository.SubRepository; +import java.io.File; import java.io.IOException; import java.util.Collection; import java.util.Iterator; @@ -36,14 +46,16 @@ import static org.assertj.core.api.Assertions.assertThat; import static org.junit.Assert.assertEquals; import static org.junit.Assert.assertFalse; import static org.junit.Assert.assertNotNull; +import static org.junit.Assert.assertNull; import static org.junit.Assert.assertTrue; /** - * * @author Sebastian Sdorra */ -public class SvnBrowseCommandTest extends AbstractSvnCommandTestBase -{ +public class SvnBrowseCommandTest extends AbstractSvnCommandTestBase { + + @Rule + public TemporaryFolder temporaryFolder = new TemporaryFolder(); @Test public void testBrowseWithFilePath() { @@ -83,7 +95,6 @@ public class SvnBrowseCommandTest extends AbstractSvnCommandTestBase /** * Method description * - * * @throws IOException */ @Test @@ -260,14 +271,51 @@ public class SvnBrowseCommandTest extends AbstractSvnCommandTestBase .containsExactly("e.txt"); } + @Test + public void shouldNotAddSubRepositoryIfNotSetInProperties() { + BrowserResult browserResult = new SvnBrowseCommand(createContext()).getBrowserResult(new BrowseCommandRequest()); + + boolean containsSubRepository = browserResult.getFile().getChildren() + .stream() + .anyMatch(c -> c.getSubRepository() != null); + + assertFalse(containsSubRepository); + } + + @Test + public void shouldAddSubRepositoryIfSetInProperties() throws IOException, SVNException { + String externalLink = "https://scm-manager.org/svn-repo"; + SvnContext svnContext = setProp("svn:externals", "external -r1 " + externalLink); + + BrowserResult browserResult = new SvnBrowseCommand(svnContext).getBrowserResult(new BrowseCommandRequest()); + + boolean containsSubRepository = browserResult.getFile().getChildren() + .stream() + .anyMatch(c -> c.getSubRepository().getRepositoryUrl().equals(externalLink)); + + assertTrue(containsSubRepository); + } + + private SvnContext setProp(String propName, String propValue) throws SVNException, IOException { + SvnContext context = createContext(); + SVNClientManager client = SVNClientManager.newInstance(); + + File workingCopyDirectory = temporaryFolder.newFolder("working-copy"); + + SVNURL url = SVNURL.fromFile(context.getDirectory()); + client.getUpdateClient().doCheckout(url, workingCopyDirectory, SVNRevision.HEAD, SVNRevision.HEAD, SVNDepth.INFINITY, true); + + client.getWCClient().doSetProperty(workingCopyDirectory, propName, SVNPropertyValue.create(propValue), true, SVNDepth.UNKNOWN, null, null); + client.getCommitClient().doCommit(new File[]{workingCopyDirectory}, false, "set prop", null, null, false, false, SVNDepth.UNKNOWN); + return context; + } + /** * Method description * - * * @return */ - private SvnBrowseCommand createCommand() - { + private SvnBrowseCommand createCommand() { return new SvnBrowseCommand(createContext()); } @@ -276,14 +324,11 @@ public class SvnBrowseCommandTest extends AbstractSvnCommandTestBase /** * Method description * - * * @param foList * @param name - * * @return */ - private FileObject getFileObject(Collection foList, String name) - { + private FileObject getFileObject(Collection foList, String name) { return foList.stream() .filter(f -> name.equals(f.getName())) .findFirst()