From a336d2c6767b8e4c6a98d0c27d17bcc6ee3df2c7 Mon Sep 17 00:00:00 2001 From: Sebastian Sdorra Date: Thu, 29 Oct 2020 14:52:05 +0100 Subject: [PATCH] Fix svn diff with property changes --- CHANGELOG.md | 1 + .../repository/spi/SCMSvnDiffGenerator.java | 81 +++++++- .../repository/spi/SvnDiffCommandTest.java | 196 ++++++++++++++++++ 3 files changed, 271 insertions(+), 7 deletions(-) create mode 100644 scm-plugins/scm-svn-plugin/src/test/java/sonia/scm/repository/spi/SvnDiffCommandTest.java diff --git a/CHANGELOG.md b/CHANGELOG.md index ec0bf37b40..6ea46299c4 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -12,6 +12,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ### Fixed - Internal server error for git sub modules without tree object ([#1397](https://github.com/scm-manager/scm-manager/pull/1397)) - Do not expose subversion commit with id 0 ([#1395](https://github.com/scm-manager/scm-manager/pull/1395)) +- SVN diff with property changes ([#1400](https://github.com/scm-manager/scm-manager/pull/1400)) ## [2.8.0] - 2020-10-27 ### Added diff --git a/scm-plugins/scm-svn-plugin/src/main/java/sonia/scm/repository/spi/SCMSvnDiffGenerator.java b/scm-plugins/scm-svn-plugin/src/main/java/sonia/scm/repository/spi/SCMSvnDiffGenerator.java index a353ea489f..2af1cb57ee 100644 --- a/scm-plugins/scm-svn-plugin/src/main/java/sonia/scm/repository/spi/SCMSvnDiffGenerator.java +++ b/scm-plugins/scm-svn-plugin/src/main/java/sonia/scm/repository/spi/SCMSvnDiffGenerator.java @@ -21,7 +21,7 @@ * OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE * SOFTWARE. */ - + package sonia.scm.repository.spi; import de.regnis.q.sequence.line.diff.QDiffGenerator; @@ -56,6 +56,7 @@ import java.io.IOException; import java.io.OutputStream; import java.io.OutputStreamWriter; import java.io.RandomAccessFile; +import java.io.StringWriter; import java.io.UnsupportedEncodingException; import java.io.Writer; import java.util.ArrayList; @@ -344,16 +345,17 @@ public class SCMSvnDiffGenerator implements ISvnDiffGenerator { String label1 = getLabel(newTargetString1, revision1); String label2 = getLabel(newTargetString2, revision2); - boolean shouldStopDisplaying = displayHeader(outputStream, displayPath, false, fallbackToAbsolutePath, SvnDiffCallback.OperationKind.Modified); visitedPaths.add(displayPath); if (useGitFormat) { displayGitDiffHeader(outputStream, SvnDiffCallback.OperationKind.Modified, getRelativeToRootPath(target, originalTarget1), getRelativeToRootPath(target, originalTarget2), null); - } - if (shouldStopDisplaying) { - return; + } else { + boolean shouldStopDisplaying = displayHeader(outputStream, displayPath, false, fallbackToAbsolutePath, SvnDiffCallback.OperationKind.Modified); + if (shouldStopDisplaying) { + return; + } } // if (useGitFormat) { @@ -374,9 +376,74 @@ public class SCMSvnDiffGenerator implements ISvnDiffGenerator { } } - displayPropertyChangesOn(useGitFormat ? getRelativeToRootPath(target, originalTarget1) : displayPath, outputStream); + if (useGitFormat) { + displayGitPropDiffValues(outputStream, propChanges, originalProps); + } else { + displayPropertyChangesOn(displayPath, outputStream); + displayPropDiffValues(outputStream, propChanges, originalProps); + } + } + + private void displayGitPropDiffValues(OutputStream outputStream, SVNProperties diff, SVNProperties baseProps) throws SVNException { + for (Iterator changedPropNames = diff.nameSet().iterator(); changedPropNames.hasNext(); ) { + String name = (String) changedPropNames.next(); + SVNPropertyValue originalValue = baseProps != null ? baseProps.getSVNPropertyValue(name) : null; + SVNPropertyValue newValue = diff.getSVNPropertyValue(name); + + try { + byte[] originalValueBytes = getPropertyAsBytes(originalValue, getEncoding()); + byte[] newValueBytes = getPropertyAsBytes(newValue, getEncoding()); + + if (originalValueBytes == null) { + originalValueBytes = new byte[0]; + } else { + originalValueBytes = maybeAppendEOL(originalValueBytes); + } + + boolean newValueHadEol = newValueBytes != null && newValueBytes.length > 0 && + (newValueBytes[newValueBytes.length - 1] == SVNProperty.EOL_CR_BYTES[0] || + newValueBytes[newValueBytes.length - 1] == SVNProperty.EOL_LF_BYTES[0]); + + if (newValueBytes == null) { + newValueBytes = new byte[0]; + } else { + newValueBytes = maybeAppendEOL(newValueBytes); + } + + QDiffUniGenerator.setup(); + Map properties = new SVNHashMap(); + + properties.put(QDiffGeneratorFactory.IGNORE_EOL_PROPERTY, Boolean.valueOf(getDiffOptions().isIgnoreEOLStyle())); + properties.put(QDiffGeneratorFactory.EOL_PROPERTY, new String(getEOL())); + properties.put(QDiffGeneratorFactory.HUNK_DELIMITER, "@@"); + if (getDiffOptions().isIgnoreAllWhitespace()) { + properties.put(QDiffGeneratorFactory.IGNORE_SPACE_PROPERTY, QDiffGeneratorFactory.IGNORE_ALL_SPACE); + } else if (getDiffOptions().isIgnoreAmountOfWhitespace()) { + properties.put(QDiffGeneratorFactory.IGNORE_SPACE_PROPERTY, QDiffGeneratorFactory.IGNORE_SPACE_CHANGE); + } + + QDiffGenerator generator = new QDiffUniGenerator(properties, ""); + StringWriter writer = new StringWriter(); + QDiffManager.generateTextDiff(new ByteArrayInputStream(originalValueBytes), new ByteArrayInputStream(newValueBytes), + null, writer, generator); + writer.flush(); + + String lines[] = writer.toString().split("\\r?\\n"); + displayString(outputStream, lines[0] + "\n"); + displayString(outputStream, " # property " + name + " has changed\n"); + for (int i=1; i< lines.length; i++) { + displayString(outputStream, lines[i] + "\n"); + } + + if (!newValueHadEol) { + displayString(outputStream, "\\ No newline at end of property"); + displayEOL(outputStream); + } + } catch (IOException e) { + wrapException(e); + } + } - displayPropDiffValues(outputStream, propChanges, originalProps); } private void throwBadRelativePathException(String displayPath, String relativeToPath) throws SVNException { diff --git a/scm-plugins/scm-svn-plugin/src/test/java/sonia/scm/repository/spi/SvnDiffCommandTest.java b/scm-plugins/scm-svn-plugin/src/test/java/sonia/scm/repository/spi/SvnDiffCommandTest.java new file mode 100644 index 0000000000..50fff9af4a --- /dev/null +++ b/scm-plugins/scm-svn-plugin/src/test/java/sonia/scm/repository/spi/SvnDiffCommandTest.java @@ -0,0 +1,196 @@ +/* + * MIT License + * + * Copyright (c) 2020-present Cloudogu GmbH and Contributors + * + * Permission is hereby granted, free of charge, to any person obtaining a copy + * of this software and associated documentation files (the "Software"), to deal + * in the Software without restriction, including without limitation the rights + * to use, copy, modify, merge, publish, distribute, sublicense, and/or sell + * copies of the Software, and to permit persons to whom the Software is + * furnished to do so, subject to the following conditions: + * + * The above copyright notice and this permission notice shall be included in all + * copies or substantial portions of the Software. + * + * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR + * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY, + * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE + * AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER + * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM, + * OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE + * SOFTWARE. + */ + +package sonia.scm.repository.spi; + +import com.google.common.collect.ImmutableMap; +import org.junit.jupiter.api.BeforeEach; +import org.junit.jupiter.api.Test; +import org.junit.jupiter.api.io.TempDir; +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.io.SVNRepositoryFactory; +import org.tmatesoft.svn.core.wc.SVNClientManager; +import org.tmatesoft.svn.core.wc.SVNRevision; +import sonia.scm.repository.RepositoryTestData; +import sonia.scm.repository.api.DiffFormat; + +import javax.annotation.Nonnull; +import java.io.ByteArrayOutputStream; +import java.io.File; +import java.io.IOException; +import java.nio.file.Path; +import java.util.Map; + +import static org.assertj.core.api.Assertions.assertThat; + +class SvnDiffCommandTest { + + private final SVNClientManager client = SVNClientManager.newInstance(); + + private File repository; + private File workingCopy; + + @BeforeEach + void setUpDirectories(@TempDir Path directory) { + repository = directory.resolve("repository").toFile(); + workingCopy = directory.resolve("working-copy").toFile(); + } + + @Test + void shouldCreateGitCompatibleDiffForSinglePropChanges() throws SVNException, IOException { + createRepository(); + commitProperty("scm:awesome", "shit"); + + String diff = gitDiff("1"); + + assertThat(diff).isEqualToIgnoringNewLines(String.join("\n", + "diff --git a/ b/", + "--- a/", + "+++ b/", + "@@ -0,0 +1 @@", + " # property scm:awesome has changed", + "+shit", + "\\ No newline at end of property" + )); + } + + @Test + void shouldCreateGitCompatibleDiffForPropChanges() throws SVNException, IOException { + createRepository(); + commitProperties(ImmutableMap.of("one", "eins", "two", "zwei", "three", "drei")); + + String diff = gitDiff("1"); + + System.out.println(diff); + + assertThat(diff).isEqualToIgnoringNewLines(String.join("\n", + "diff --git a/ b/", + "--- a/", + "+++ b/", + "@@ -0,0 +1 @@", + " # property one has changed", + "+eins", + "\\ No newline at end of property", + "@@ -0,0 +1 @@", + " # property two has changed", + "+zwei", + "\\ No newline at end of property", + "@@ -0,0 +1 @@", + " # property three has changed", + "+drei", + "\\ No newline at end of property" + )); + } + + @Test + void shouldCreateGitCompatibleDiffForModifiedProp() throws SVNException, IOException { + createRepository(); + commitProperty("scm:spaceship", "Razor Crest"); + commitProperty("scm:spaceship", "Heart Of Gold"); + + String diff = gitDiff("2"); + + System.out.println(diff); + + assertThat(diff).isEqualToIgnoringNewLines(String.join("\n", + "diff --git a/ b/", + "--- a/", + "+++ b/", + "@@ -1 +1 @@", + " # property scm:spaceship has changed", + "-Razor Crest", + "+Heart Of Gold", + "\\ No newline at end of property" + )); + } + + @Nonnull + private String gitDiff(String revision) throws IOException { + SvnDiffCommand command = createCommand(); + DiffCommandRequest request = new DiffCommandRequest(); + request.setFormat(DiffFormat.GIT); + request.setRevision(revision); + + ByteArrayOutputStream baos = new ByteArrayOutputStream(); + command.getDiffResult(request).accept(baos); + return baos.toString(); + } + + private SvnDiffCommand createCommand() { + return new SvnDiffCommand(new SvnContext(RepositoryTestData.createHeartOfGold(), repository)); + } + + private void commitProperty(String name, String value) throws SVNException { + setProperty(name, value); + commit("set property " + name + " = " + value); + } + + private void commit(String message) throws SVNException { + client.getCommitClient().doCommit( + new File[]{workingCopy}, + false, + message, + null, + null, + false, + false, + SVNDepth.UNKNOWN + ); + } + + private void setProperty(String name, String value) throws SVNException { + client.getWCClient().doSetProperty( + workingCopy, + name, + SVNPropertyValue.create(value), + true, + SVNDepth.UNKNOWN, + null, + null + ); + } + + private void commitProperties(Map properties) throws SVNException { + for (Map.Entry e : properties.entrySet()) { + setProperty(e.getKey(), e.getValue()); + } + commit("set " + properties.size() + " properties"); + } + + private void createRepository() throws SVNException { + SVNURL url = SVNRepositoryFactory.createLocalRepository(repository, true, false); + client.getUpdateClient().doCheckout( + url, + workingCopy, + SVNRevision.HEAD, + SVNRevision.HEAD, + SVNDepth.INFINITY, + true + ); + } + +}