Fix svn diff with property changes

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

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);
-      }
+      } else {
-      if (shouldStopDisplaying) {
+        boolean shouldStopDisplaying = displayHeader(outputStream, displayPath, false, fallbackToAbsolutePath, SvnDiffCallback.OperationKind.Modified);
-        return;
+        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 {

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<String, String> properties) throws SVNException {
+    for (Map.Entry<String, String> 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
+    );
+  }
+
+}