Merge branch 'develop' into feature/lookup_api

scm-plugins/scm-svn-plugin/src/main/java/sonia/scm/repository/SvnUtil.java

@@ -198,7 +198,7 @@ public final class SvnUtil
                             Person.toPerson(entry.getAuthor()),
                             entry.getMessage());
 
-    if (revision > 0)
+    if (revision > 1)
     {
       changeset.getParents().add(String.valueOf(revision - 1));
     }
@@ -232,6 +232,7 @@ public final class SvnUtil
    *
    * @return
    */
+  @SuppressWarnings("java:S1149") // we can not use StringBuild SVNXMLUtil requires StringBuffer
   public static String createErrorBody(SVNErrorCode errorCode)
   {
     StringBuffer xmlBuffer = new StringBuffer();

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(), true);
+        byte[] newValueBytes = getPropertyAsBytes(newValue, getEncoding(), true);
+
+        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 {
@@ -1123,6 +1190,10 @@ public class SCMSvnDiffGenerator implements ISvnDiffGenerator {
   }
 
   private byte[] getPropertyAsBytes(SVNPropertyValue value, String encoding) {
+    return getPropertyAsBytes(value, encoding, false);
+  }
+
+  private byte[] getPropertyAsBytes(SVNPropertyValue value, String encoding, boolean replaceBinary) {
     if (value == null) {
       return null;
     }
@@ -1133,7 +1204,11 @@ public class SCMSvnDiffGenerator implements ISvnDiffGenerator {
         return value.getString().getBytes();
       }
     }
-    return value.getBytes();
+    if (replaceBinary) {
+      return String.format("Binary value (%s bytes)", value.getBytes().length).getBytes();
+    } else {
+      return value.getBytes();
+    }
   }
 
   private void displayMergeInfoDiff(OutputStream outputStream, String oldValue, String newValue) throws SVNException, IOException {

scm-plugins/scm-svn-plugin/src/main/java/sonia/scm/repository/spi/SvnLogCommand.java

@@ -26,6 +26,7 @@ package sonia.scm.repository.spi;
 
 //~--- non-JDK imports --------------------------------------------------------
 
+import com.google.common.base.Preconditions;
 import com.google.common.base.Strings;
 import com.google.common.collect.Lists;
 import org.slf4j.Logger;
@@ -76,6 +77,7 @@ public class SvnLogCommand extends AbstractSvnCommand implements LogCommand
     try
     {
       long revisioNumber = parseRevision(revision, repository);
+      Preconditions.checkArgument(revisioNumber > 0, "revision must be greater than zero: %d", revisioNumber);
       SVNRepository repo = open();
       Collection<SVNLogEntry> entries = repo.log(null, null, revisioNumber,
                                           revisioNumber, true, true);

scm-plugins/scm-svn-plugin/src/main/java/sonia/scm/web/SvnUserAgentProvider.java

@@ -49,13 +49,13 @@ public final class SvnUserAgentProvider implements UserAgentProvider
   /** TortoiseSVN */
   @VisibleForTesting
   static final UserAgent TORTOISE_SVN = 
-    UserAgent.builder("TortoiseSVN").browser(false)
+    UserAgent.scmClient("TortoiseSVN")
              .basicAuthenticationCharset(Charsets.UTF_8).build();
 
   /** Subversion cli client */
   @VisibleForTesting
   static final UserAgent SVN =
-    UserAgent.builder("Subversion").browser(false)
+    UserAgent.scmClient("Subversion")
              .basicAuthenticationCharset(Charsets.UTF_8).build();
 
   //~--- methods --------------------------------------------------------------

scm-plugins/scm-svn-plugin/src/test/java/sonia/scm/repository/spi/SvnDiffCommandTest.java

@@ -0,0 +1,225 @@
+/*
+ * 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.Base64;
+import java.util.Map;
+
+import static org.assertj.core.api.Assertions.assertThat;
+
+class SvnDiffCommandTest {
+
+  // the smallest gif of the world
+  private static final String GIF = "R0lGODlhAQABAIABAP///wAAACH5BAEKAAEALAAAAAABAAEAAAICTAEAOw==";
+
+  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");
+
+    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");
+
+    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"
+    ));
+  }
+
+  @Test
+  void shouldCreateGitCompatibleDiffForBinaryProps() throws SVNException, IOException {
+    createRepository();
+
+    byte[] gif = Base64.getDecoder().decode(GIF);
+    commitProperty("scm:gif", gif);
+
+    String diff = gitDiff("1");
+
+    assertThat(diff).isEqualToIgnoringNewLines(String.join("\n",
+      "diff --git a/ b/",
+      "--- a/",
+      "+++ b/",
+      "@@ -0,0 +1 @@",
+      " # property scm:gif has changed",
+      "+Binary value (43 bytes)",
+      "\\ 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, SVNPropertyValue.create(value));
+    commit("set property " + name + " = " + value);
+  }
+
+  private void commitProperty(String name, byte[] value) throws SVNException {
+    commitProperty(name, SVNPropertyValue.create(name, value));
+  }
+
+  private void commitProperty(String name, SVNPropertyValue 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, SVNPropertyValue value) throws SVNException {
+    client.getWCClient().doSetProperty(
+      workingCopy,
+      name,
+      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(), SVNPropertyValue.create(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
+    );
+  }
+
+}

scm-plugins/scm-svn-plugin/src/test/java/sonia/scm/repository/spi/SvnLogCommandTest.java

@@ -32,10 +32,9 @@ import sonia.scm.repository.Changeset;
 import sonia.scm.repository.ChangesetPagingResult;
 import sonia.scm.repository.Modifications;
 
-import static org.junit.Assert.assertEquals;
-import static org.junit.Assert.assertNotNull;
-import static org.junit.Assert.assertNull;
-import static org.junit.Assert.assertTrue;
+import java.util.stream.StreamSupport;
+
+import static org.junit.Assert.*;
 
 //~--- JDK imports ------------------------------------------------------------
 
@@ -56,11 +55,36 @@ public class SvnLogCommandTest extends AbstractSvnCommandTestBase
     assertEquals(5, result.getChangesets().size());
   }
 
+  @Test
+  public void shouldNotReturnChangesetWithIdZero() {
+    ChangesetPagingResult result = createCommand().getChangesets(new LogCommandRequest());
+    boolean found = StreamSupport.stream(result.spliterator(), false).anyMatch(c -> "0".equals(c.getId()));
+    assertFalse(found);
+  }
+
+  @Test(expected = IllegalArgumentException.class)
+  public void shouldThrowIllegalArgumentExceptionForChangesetZero() {
+    createCommand().getChangeset("0", new LogCommandRequest());
+  }
+
+  @Test
+  public void shouldNotReturnChangesetZeroAsParent() {
+    Changeset changeset = createCommand().getChangeset("1", new LogCommandRequest());
+    assertTrue(changeset.getParents().isEmpty());
+  }
+
+  @Test
+  public void shouldAppendParentChangeset() {
+    Changeset changeset = createCommand().getChangeset("2", new LogCommandRequest());
+    assertEquals(1, changeset.getParents().size());
+    assertEquals("1", changeset.getParents().get(0));
+  }
+
   @Test
   public void testShouldStartWithRevisionOne() {
     ChangesetPagingResult result = createCommand().getChangesets(new LogCommandRequest());
     Changeset first = Iterables.getLast(result);
-    assertEquals(first.getId(), "1");
+    assertEquals("1", first.getId());
   }
 
   @Test