Modifications command between two revisions (#1761)

Adds the option to compute the modifications between two revisions unsing the modifications command.

gradle/changelog/modificatsions_for_multiple_revisions.yaml

@@ -0,0 +1,2 @@
+- type: Added
+  description: "Base" revision in modificatsions command to compute modifications between revisions ([#1761](https://github.com/scm-manager/scm-manager/pull/1761))

scm-core/src/main/java/sonia/scm/repository/Feature.java

@@ -42,5 +42,11 @@ public enum Feature
    * The repository supports computation of incoming changes (either diff or list of changesets) of one branch
    * in respect to another target branch.
    */
-  INCOMING_REVISION
+  INCOMING_REVISION,
+  /**
+   * The repository supports computation of modifications between two revisions, not only for a singe revision.
+   *
+   * @since 2.23.0
+   */
+  MODIFICATIONS_BETWEEN_REVISIONS
 }

scm-core/src/main/java/sonia/scm/repository/Modifications.java

@@ -32,9 +32,11 @@ import lombok.ToString;
 import java.io.Serializable;
 import java.util.Collection;
 import java.util.List;
+import java.util.Optional;
 import java.util.stream.Stream;
 
 import static java.util.Arrays.asList;
+import static java.util.Optional.ofNullable;
 import static java.util.stream.Collectors.toList;
 
 @EqualsAndHashCode
@@ -45,6 +47,7 @@ public class Modifications implements Serializable {
   private static final long serialVersionUID = -8902033326668658140L;
 
   private final String revision;
+  private final String baseRevision;
   private final Collection<Modification> modifications;
 
   public Modifications(String revision, Modification... modifications) {
@@ -52,10 +55,28 @@ public class Modifications implements Serializable {
   }
 
   public Modifications(String revision, Collection<Modification> modifications) {
+    this(null, revision, modifications);
+  }
+
+  /**
+   * @since 2.23.0
+   */
+  public Modifications(String baseRevision, String revision, Collection<Modification> modifications) {
+    this.baseRevision = baseRevision;
     this.revision = revision;
     this.modifications = ImmutableList.copyOf(modifications);
   }
 
+  /**
+   * If these modifications are not related to a single revision but represent the
+   * modifications between two revisions, this gives the base revision.
+   *
+   * @since 2.23.0
+   */
+  public Optional<String> getBaseRevision() {
+    return ofNullable(baseRevision);
+  }
+
   public List<String> getEffectedPaths() {
     return effectedPathsStream().collect(toList());
   }

scm-core/src/main/java/sonia/scm/repository/api/ModificationsCommandBuilder.java

@@ -72,15 +72,35 @@ public final class ModificationsCommandBuilder {
   @Setter
   private boolean disablePreProcessors = false;
 
+  /**
+   * Set this to compute either the midifications of the given revision, or additionally set
+   * {@link #baseRevision(String)} to compute the modifications between this and the
+   * other revision.
+   * @return This command builder.
+   */
   public ModificationsCommandBuilder revision(String revision){
     request.setRevision(revision);
     return this;
   }
 
+  /**
+   * Set this to compute the modifications between two revisions. If this is not set,
+   * only the modifications of the revision set by {@link #revision(String)} will be computed.
+   * This is only supported by repositories supporting the feature
+   * {@link sonia.scm.repository.Feature#MODIFICATIONS_BETWEEN_REVISIONS}.
+   * @param baseRevision If set, the command will compute the modifications between this revision
+   *                     and the revision set by {@link #revision(String)}.
+   * @return This command builder.
+   * @since 2.23.0
+   */
+  public ModificationsCommandBuilder baseRevision(String baseRevision){
+    request.setBaseRevision(baseRevision);
+    return this;
+  }
+
   /**
    * Reset each parameter to its default value.
    *
-   *
    * @return {@code this}
    */
   public ModificationsCommandBuilder reset() {
@@ -90,6 +110,9 @@ public final class ModificationsCommandBuilder {
     return this;
   }
 
+  /**
+   * Computes the modifications.
+   */
   public Modifications getModifications() throws IOException {
     Modifications modifications;
     if (disableCache) {

scm-core/src/main/java/sonia/scm/repository/spi/ModificationsCommand.java

@@ -24,13 +24,15 @@
 
 package sonia.scm.repository.spi;
 
+import sonia.scm.FeatureNotSupportedException;
+import sonia.scm.repository.Feature;
 import sonia.scm.repository.Modifications;
 
 import java.io.IOException;
 
 /**
  * Command to get the modifications applied to files in a revision.
- *
+ * <p>
  * Modifications are for example: Add, Update, Delete
  *
  * @author Mohamed Karray
@@ -38,8 +40,35 @@ import java.io.IOException;
  */
 public interface ModificationsCommand {
 
+  /**
+   * Read the modifications for a single revision.
+   */
   Modifications getModifications(String revision) throws IOException;
 
-  Modifications getModifications(ModificationsCommandRequest request) throws IOException;
+  /**
-
+   * Read the modifications between two revisions. The result is similar to a diff between
+   * these two revisions, but without details about the content.
+   * <br>
+   * Make sure your repository supports the feature {@link Feature#MODIFICATIONS_BETWEEN_REVISIONS},
+   * because otherwise this will throw a {@link FeatureNotSupportedException}.
+   *
+   * @throws FeatureNotSupportedException if the repository type does not support the feature
+   *                                      {@link FeatureNotSupportedException}.
+   * @since 2.23.0
+   */
+  default Modifications getModifications(String baseRevision, String revision) throws IOException {
+    throw new FeatureNotSupportedException(Feature.MODIFICATIONS_BETWEEN_REVISIONS.name());
+  }
+
+  /**
+   * Execute the given {@link ModificationsCommandRequest}.
+   */
+  @SuppressWarnings("java:S3655") // don't know why this should be an issue here. We check 'isPresent' before 'get' on 'request.getBaseRevision()'
+  default Modifications getModifications(ModificationsCommandRequest request) throws IOException {
+    if (request.getBaseRevision().isPresent()) {
+      return getModifications(request.getBaseRevision().get(), request.getRevision());
+    } else {
+      return getModifications(request.getRevision());
+    }
+  }
 }

scm-core/src/main/java/sonia/scm/repository/spi/ModificationsCommandRequest.java

@@ -32,6 +32,10 @@ import lombok.NoArgsConstructor;
 import lombok.Setter;
 import lombok.ToString;
 
+import java.util.Optional;
+
+import static java.util.Optional.ofNullable;
+
 @ToString
 @EqualsAndHashCode
 @Getter
@@ -40,9 +44,18 @@ import lombok.ToString;
 @NoArgsConstructor
 public class ModificationsCommandRequest implements Resetable {
   private String revision;
+  private String baseRevision;
 
   @Override
   public void reset() {
     revision = null;
+    baseRevision = null;
+  }
+
+  /**
+   * @since 2.23.0
+   */
+  public Optional<String> getBaseRevision() {
+    return ofNullable(baseRevision);
   }
 }

scm-plugins/scm-git-plugin/src/main/java/sonia/scm/repository/spi/GitModificationsCommand.java

@@ -27,6 +27,7 @@ package sonia.scm.repository.spi;
 import lombok.extern.slf4j.Slf4j;
 import org.eclipse.jgit.diff.DiffEntry;
 import org.eclipse.jgit.lib.ObjectId;
+import org.eclipse.jgit.lib.Repository;
 import org.eclipse.jgit.revwalk.RevCommit;
 import org.eclipse.jgit.revwalk.RevTree;
 import org.eclipse.jgit.revwalk.RevWalk;
@@ -60,10 +61,65 @@ public class GitModificationsCommand extends AbstractGitCommand implements Modif
     super(context);
   }
 
-  private Modifications createModifications(TreeWalk treeWalk, RevCommit commit, RevWalk revWalk, String revision)
+  @Override
-    throws IOException {
+  public Modifications getModifications(String baseRevision, String revision) {
+    org.eclipse.jgit.lib.Repository gitRepository = null;
+    RevWalk revWalk = null;
+    try {
+      gitRepository = open();
+      if (!gitRepository.getAllRefs().isEmpty()) {
+        revWalk = new RevWalk(gitRepository);
+        RevCommit commit = getCommit(revision, gitRepository, revWalk);
+        TreeWalk treeWalk = createTreeWalk(gitRepository);
+        if (baseRevision == null) {
+          determineParentAsBase(treeWalk, commit, revWalk);
+        } else {
+          RevCommit baseCommit = getCommit(baseRevision, gitRepository, revWalk);
+          treeWalk.addTree(baseCommit.getTree());
+        }
+        return new Modifications(baseRevision, revision, createModifications(treeWalk, commit));
+      }
+    } catch (IOException ex) {
+      log.error("could not open repository", ex);
+      throw new InternalRepositoryException(entity(repository), "could not open repository", ex);
+    } finally {
+      GitUtil.release(revWalk);
+      GitUtil.close(gitRepository);
+    }
+    return null;
+  }
+
+  private RevCommit getCommit(String revision, Repository gitRepository, RevWalk revWalk) throws IOException {
+    ObjectId id = GitUtil.getRevisionId(gitRepository, revision);
+    return revWalk.parseCommit(id);
+  }
+
+  @Override
+  public Modifications getModifications(String revision) {
+    return getModifications(null, revision);
+  }
+
+  private TreeWalk createTreeWalk(Repository gitRepository) {
+    TreeWalk treeWalk = new TreeWalk(gitRepository);
     treeWalk.reset();
     treeWalk.setRecursive(true);
+    return treeWalk;
+  }
+
+  private Collection<Modification> createModifications(TreeWalk treeWalk, RevCommit commit)
+    throws IOException {
+    treeWalk.addTree(commit.getTree());
+    List<DiffEntry> entries = Differ.scanWithRename(context.open(), null, treeWalk);
+    Collection<Modification> modifications = new ArrayList<>();
+    for (DiffEntry e : entries) {
+      if (!e.getOldId().equals(e.getNewId()) || !e.getOldPath().equals(e.getNewPath())) {
+        modifications.add(asModification(e));
+      }
+    }
+    return modifications;
+  }
+
+  private void determineParentAsBase(TreeWalk treeWalk, RevCommit commit, RevWalk revWalk) throws IOException {
     if (commit.getParentCount() > 0) {
       RevCommit parent = commit.getParent(0);
       RevTree tree = parent.getTree();
@@ -81,43 +137,6 @@ public class GitModificationsCommand extends AbstractGitCommand implements Modif
       log.trace("no parent available for commit {}", commit.getName());
       treeWalk.addTree(new EmptyTreeIterator());
     }
-    treeWalk.addTree(commit.getTree());
-    List<DiffEntry> entries = Differ.scanWithRename(context.open(), null, treeWalk);
-    Collection<Modification> modifications = new ArrayList<>();
-    for (DiffEntry e : entries) {
-      if (!e.getOldId().equals(e.getNewId()) || !e.getOldPath().equals(e.getNewPath())) {
-        modifications.add(asModification(e));
-      }
-    }
-    return new Modifications(revision, modifications);
-  }
-
-  @Override
-  public Modifications getModifications(String revision) {
-    org.eclipse.jgit.lib.Repository gitRepository = null;
-    RevWalk revWalk = null;
-    try {
-      gitRepository = open();
-      if (!gitRepository.getAllRefs().isEmpty()) {
-        revWalk = new RevWalk(gitRepository);
-        ObjectId id = GitUtil.getRevisionId(gitRepository, revision);
-        RevCommit commit = revWalk.parseCommit(id);
-        TreeWalk treeWalk = new TreeWalk(gitRepository);
-        return createModifications(treeWalk, commit, revWalk, revision);
-      }
-    } catch (IOException ex) {
-      log.error("could not open repository", ex);
-      throw new InternalRepositoryException(entity(repository), "could not open repository", ex);
-    } finally {
-      GitUtil.release(revWalk);
-      GitUtil.close(gitRepository);
-    }
-    return null;
-  }
-
-  @Override
-  public Modifications getModifications(ModificationsCommandRequest request) {
-    return getModifications(request.getRevision());
   }
 
   private Modification asModification(DiffEntry entry) throws UnsupportedModificationTypeException {

scm-plugins/scm-git-plugin/src/main/java/sonia/scm/repository/spi/GitRepositoryServiceProvider.java

@@ -60,7 +60,10 @@ public class GitRepositoryServiceProvider extends RepositoryServiceProvider {
     Command.MIRROR
   );
 
-  protected static final Set<Feature> FEATURES = EnumSet.of(Feature.INCOMING_REVISION);
+  protected static final Set<Feature> FEATURES = EnumSet.of(
+    Feature.INCOMING_REVISION,
+    Feature.MODIFICATIONS_BETWEEN_REVISIONS
+  );
 
   private final GitContext context;
   private final Injector commandInjector;

scm-plugins/scm-git-plugin/src/test/java/sonia/scm/repository/spi/GitModificationsCommandTest.java

@@ -106,6 +106,36 @@ public class GitModificationsCommandTest extends AbstractRemoteCommandTestBase {
     assertModifications.accept(incomingModificationsCommand.getModifications(revision));
   }
 
+  @Test
+  public void shouldFindModificationsBetweenRevisions() throws Exception {
+    write(outgoing, outgoingDirectory, "a.txt", "bal bla");
+    write(outgoing, outgoingDirectory, "d.txt", "some file to be renamed");
+    RevCommit baseCommit = commit(outgoing, "add files");
+
+    write(outgoing, outgoingDirectory, "a.txt", "modified content");
+    commit(outgoing, "modify file");
+    write(outgoing, outgoingDirectory, "c.txt", "brand new file");
+    commit(outgoing, "add file");
+    write(outgoing, outgoingDirectory, "o.txt", "some file to be renamed");
+    outgoing.rm().addFilepattern("d.txt").call();
+    RevCommit targetCommit = commit(outgoing, "move/rename file");
+
+    outgoing.checkout().setName("some_branch").setCreateBranch(true).setStartPoint(baseCommit).call();
+    write(outgoing, outgoingDirectory, "x.txt", "bla bla");
+    RevCommit otherBranchCommit = commit(outgoing, "other branch");
+
+    Modifications modifications = outgoingModificationsCommand.getModifications(otherBranchCommit.getName(), targetCommit.getName());
+
+    assertThat(modifications.getModifications())
+      .hasSize(4)
+      .extracting("class.simpleName")
+      .contains("Modified") // File a.txt has been modified
+      .contains("Removed") // File x.txt from the other branch is not present
+      .contains("Added") // File c.txt has been created on the original branch
+      .contains("Renamed") // File d.txt has been renamed on the original branch
+    ;
+  }
+
   void pushOutgoingAndPullIncoming() throws IOException {
     GitPushCommand cmd = new GitPushCommand(handler, new GitContext(outgoingDirectory, outgoingRepository, null, new GitConfig()));
     PushCommandRequest request = new PushCommandRequest();

scm-plugins/scm-hg-plugin/src/main/java/sonia/scm/repository/spi/HgModificationsCommand.java

@@ -27,7 +27,9 @@ package sonia.scm.repository.spi;
 import sonia.scm.repository.Modification;
 import sonia.scm.repository.Modifications;
 import sonia.scm.repository.spi.javahg.HgLogChangesetCommand;
+import sonia.scm.repository.spi.javahg.StateCommand;
 
+import java.io.IOException;
 import java.util.Collection;
 
 public class HgModificationsCommand extends AbstractCommand implements ModificationsCommand {
@@ -36,7 +38,6 @@ public class HgModificationsCommand extends AbstractCommand implements Modificat
     super(context);
   }
 
-
   @Override
   public Modifications getModifications(String revision) {
     com.aragost.javahg.Repository repository = open();
@@ -46,9 +47,9 @@ public class HgModificationsCommand extends AbstractCommand implements Modificat
   }
 
   @Override
-  public Modifications getModifications(ModificationsCommandRequest request) {
+  public Modifications getModifications(String baseRevision, String revision) throws IOException {
-    return getModifications(request.getRevision());
+    com.aragost.javahg.Repository repository = open();
+    StateCommand stateCommand = new StateCommand(repository);
+    return new Modifications(baseRevision, revision, stateCommand.call(baseRevision, revision));
   }
-
-
 }

scm-plugins/scm-hg-plugin/src/main/java/sonia/scm/repository/spi/HgRepositoryServiceProvider.java

@@ -63,7 +63,10 @@ public class HgRepositoryServiceProvider extends RepositoryServiceProvider {
     Command.FULL_HEALTH_CHECK
   );
 
-  public static final Set<Feature> FEATURES = EnumSet.of(Feature.COMBINED_DEFAULT_BRANCH);
+  public static final Set<Feature> FEATURES = EnumSet.of(
+    Feature.COMBINED_DEFAULT_BRANCH,
+    Feature.MODIFICATIONS_BETWEEN_REVISIONS
+  );
 
   private final HgRepositoryHandler handler;
   private final HgCommandContext context;

scm-plugins/scm-hg-plugin/src/main/java/sonia/scm/repository/spi/javahg/HgModificationParser.java

@@ -33,18 +33,27 @@ import sonia.scm.repository.Renamed;
 
 import java.util.Collection;
 import java.util.LinkedHashSet;
+import java.util.Locale;
 
 class HgModificationParser {
   private final Collection<Modification> modifications = new LinkedHashSet<>();
 
   void addLine(String line) {
-    if (line.startsWith("a ")) {
+    if (line.length() < 2) {
+      return;
+    }
+    String linePrefix = line.substring(0, 2).toLowerCase(Locale.ROOT);
+    switch (linePrefix) {
+      case "a ":
         modifications.add(new Added(line.substring(2)));
-    } else if (line.startsWith("m ")) {
+        break;
+      case "m ":
         modifications.add(new Modified(line.substring(2)));
-    } else if (line.startsWith("d ")) {
+        break;
+      case "r ":
         modifications.add(new Removed(line.substring(2)));
-    } else if (line.startsWith("c ")) {
+        break;
+      case "c ":
         String sourceTarget = line.substring(2);
         int divider = sourceTarget.indexOf('\0');
         String source = sourceTarget.substring(0, divider);
@@ -55,6 +64,9 @@ class HgModificationParser {
         } else {
           modifications.add(new Copied(source, target));
         }
+        break;
+      default:
+        // nothing to do
     }
   }
 

scm-plugins/scm-hg-plugin/src/main/java/sonia/scm/repository/spi/javahg/StateCommand.java

@@ -0,0 +1,55 @@
+/*
+ * 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.javahg;
+
+import com.aragost.javahg.Repository;
+import com.aragost.javahg.internals.HgInputStream;
+import sonia.scm.repository.Modification;
+
+import java.io.IOException;
+import java.util.Collection;
+
+public class StateCommand extends com.aragost.javahg.internals.AbstractCommand {
+  public StateCommand(Repository repository) {
+    super(repository);
+  }
+
+  @Override
+  public String getCommandName() {
+    return "status";
+  }
+
+  public Collection<Modification> call(String from, String to) throws IOException {
+    cmdAppend("--rev", from + ":" + to);
+    HgInputStream in = launchStream();
+    HgModificationParser hgModificationParser = new HgModificationParser();
+    String line = in.textUpTo('\n');
+    while (line != null && line.length() > 0) {
+      hgModificationParser.addLine(line);
+      line = in.textUpTo('\n');
+    }
+    return hgModificationParser.getModifications();
+  }
+}

scm-plugins/scm-hg-plugin/src/main/resources/sonia/scm/styles/changesets-eager.style

@@ -3,7 +3,7 @@ changeset = "{rev}:{node}{author}\n{date|hgdate}\n{branch}\n{parents}{extras}\n{
 tag = "t {tag}\n"
 file_add = "a {file_add}\n"
 file_mod = "m {file_mod}\n"
-file_del = "d {file_del}\n"
+file_del = "r {file_del}\n"
 file_copy = "c {source}\0{name}\n"
 extra = "{key}={value|stringescape},"
 footer = "%{pattern}"

scm-plugins/scm-hg-plugin/src/test/java/sonia/scm/repository/spi/HgModificationsCommandTest.java

@@ -25,14 +25,17 @@
 package sonia.scm.repository.spi;
 
 import com.aragost.javahg.Changeset;
+import com.aragost.javahg.commands.BranchCommand;
 import com.aragost.javahg.commands.CopyCommand;
 import com.aragost.javahg.commands.RemoveCommand;
 import com.aragost.javahg.commands.RenameCommand;
+import com.aragost.javahg.commands.UpdateCommand;
 import org.junit.Before;
 import org.junit.Test;
 import sonia.scm.repository.HgConfigResolver;
 import sonia.scm.repository.HgTestUtil;
 import sonia.scm.repository.Modifications;
+import sonia.scm.repository.client.spi.CheckoutCommand;
 
 import java.io.File;
 import java.util.function.Consumer;
@@ -112,6 +115,37 @@ public class HgModificationsCommandTest extends IncomingOutgoingTestBase {
     assertModifications.accept(outgoingModificationsCommand.getModifications(revision));
   }
 
+  @Test
+  public void shouldFindModificationsBetweenRevisions() throws Exception {
+    writeNewFile(outgoing, outgoingDirectory, "a.txt", "bla bla");
+    writeNewFile(outgoing, outgoingDirectory, "42.txt", "the answer to life and everything");
+    writeNewFile(outgoing, outgoingDirectory, "SpaceX.txt", "Going to infinity and beyond");
+    commit(outgoing, "add files");
+    BranchCommand.on(outgoing).set("some_branch");
+    writeNewFile(outgoing, outgoingDirectory, "x.txt", "bla bla");
+    Changeset otherBranchCommit = commit(outgoing, "other branch");
+
+    UpdateCommand.on(outgoing).rev("default").execute();
+    writeNewFile(outgoing, outgoingDirectory, "a.txt", "modified content");
+    commit(outgoing, "modify file");
+    RenameCommand.on(outgoing).execute("42.txt", "7x6.txt");
+    commit(outgoing, "rename file");
+    CopyCommand.on(outgoing).execute("SpaceX.txt", "Virgin.txt");
+    commit(outgoing, "copy file");
+    writeNewFile(outgoing, outgoingDirectory, "c.txt", "brand new file");
+    Changeset targetChangeset = commit(outgoing, "add file");
+
+    Modifications modifications = outgoingModificationsCommand.getModifications(otherBranchCommit.getNode(), targetChangeset.getNode());
+
+    assertThat(modifications.getModifications())
+      .hasSize(6)
+      .extracting("class.simpleName")
+      .contains("Modified") // File a.txt has been modified
+      .contains("Removed") // File x.txt from the other branch is not present and 42.txt has been removed (via rename)
+      .contains("Added") // File c.txt, Virgin.txt, and 7x6.txt have been created (or copied or renamed) on the original branch
+    ;
+  }
+
   Consumer<Modifications> assertRemovedFiles(String fileName) {
     return (modifications) -> {
       assertThat(modifications).isNotNull();

scm-plugins/scm-hg-plugin/src/test/java/sonia/scm/repository/spi/javahg/HgModificationParserTest.java

@@ -55,7 +55,7 @@ class HgModificationParserTest {
 
   @Test
   void shouldDetectRemovedPath() {
-    parser.addLine("d removed/file");
+    parser.addLine("r removed/file");
 
     assertThat(parser.getModifications())
       .containsExactly(new Removed("removed/file"));
@@ -64,7 +64,7 @@ class HgModificationParserTest {
   @Test
   void shouldDetectRenamedPath() {
     parser.addLine("a new/path");
-    parser.addLine("d old/path");
+    parser.addLine("r old/path");
     parser.addLine("c old/path\0new/path");
 
     assertThat(parser.getModifications())

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

@@ -0,0 +1,131 @@
+/*
+ * 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;
+
+import java.util.Collection;
+import java.util.HashMap;
+import java.util.Map;
+import java.util.Set;
+import java.util.function.BiConsumer;
+import java.util.function.BinaryOperator;
+import java.util.function.Function;
+import java.util.function.Supplier;
+import java.util.stream.Collector;
+
+import static java.util.Collections.emptySet;
+import static java.util.Collections.unmodifiableCollection;
+import static java.util.Collections.unmodifiableSet;
+
+/**
+ * This class can be used to "consolidate" modifications for a stream of commit based modifications.
+ * Single modifications will be changed or ignored if subsequent modifications effect them. An "added"
+ * for a file for example will be ignored, if the same file is marked as "removed" later on. Another
+ * example would be a "modification" of a file, that was "added" beforehand, because in summary this
+ * simply is still an "added" file.
+ */
+class ConsolidatingModificationCollector implements Collector<Modification, ConsolidatedModifications, Collection<Modification>> {
+
+  static ConsolidatingModificationCollector consolidate() {
+    return new ConsolidatingModificationCollector();
+  }
+
+  @Override
+  public Supplier<ConsolidatedModifications> supplier() {
+    return ConsolidatedModifications::new;
+  }
+
+  @Override
+  public BiConsumer<ConsolidatedModifications, Modification> accumulator() {
+    return (existingModifications, currentModification) -> {
+      if (currentModification instanceof Copied) {
+        existingModifications.added(new Added(((Copied) currentModification).getTargetPath()));
+      } else if (currentModification instanceof Removed) {
+        existingModifications.removed((Removed) currentModification);
+      } else if (currentModification instanceof Renamed) {
+        Renamed renameModification = (Renamed) currentModification;
+        existingModifications.removed(new Removed(renameModification.getOldPath()));
+        existingModifications.added(new Added(renameModification.getNewPath()));
+      } else if (currentModification instanceof Added) {
+        existingModifications.added((Added) currentModification);
+      } else if (currentModification instanceof Modified) {
+        existingModifications.modified((Modified) currentModification);
+      } else {
+        throw new IllegalStateException("cannot handle modification of unknown type " + currentModification.getClass());
+      }
+    };
+  }
+
+  @Override
+  public BinaryOperator<ConsolidatedModifications> combiner() {
+    return null; // Combiner not needed because we do not support Collector.Characteristics#CONCURRENT
+  }
+
+  @Override
+  public Function<ConsolidatedModifications, Collection<Modification>> finisher() {
+    return ConsolidatedModifications::getModifications;
+  }
+
+  @Override
+  public Set<Characteristics> characteristics() {
+    return emptySet();
+  }
+}
+
+class ConsolidatedModifications {
+  private final Map<String, Modification> modifications = new HashMap<>();
+
+  Set<String> getPaths() {
+    return unmodifiableSet(modifications.keySet());
+  }
+
+  Collection<Modification> getModifications() {
+    return unmodifiableCollection(modifications.values());
+  }
+
+  void added(Added added) {
+    Modification earlierModification = modifications.get(added.getPath());
+    if (earlierModification instanceof Removed) {
+      modifications.put(added.getPath(), new Modified(added.getPath()));
+    } else {
+      modifications.put(added.getPath(), added);
+    }
+  }
+
+  void modified(Modified modified) {
+    Modification earlierModification = modifications.get(modified.getPath());
+    if (!(earlierModification instanceof Added)) { // added should still be added
+      modified.getEffectedPaths().forEach(path -> modifications.put(path, modified));
+    }
+  }
+
+  void removed(Removed removed) {
+    Modification earlierModification = modifications.get(removed.getPath());
+    if (earlierModification instanceof Added) {
+      modifications.remove(removed.getPath());
+    } else {
+      modifications.put(removed.getPath(), removed);
+    }
+  }
+}

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

@@ -47,15 +47,17 @@ import javax.servlet.http.HttpServletRequest;
 import javax.servlet.http.HttpServletResponse;
 import java.io.IOException;
 import java.io.PrintWriter;
+import java.util.Collection;
 import java.util.List;
 import java.util.Map;
 import java.util.Optional;
-import java.util.stream.Collectors;
+import java.util.stream.Stream;
 
-import static java.util.Collections.emptyList;
 import static java.util.Optional.empty;
+import static java.util.stream.Collectors.toList;
 import static sonia.scm.ContextEntry.ContextBuilder.entity;
 import static sonia.scm.NotFoundException.notFound;
+import static sonia.scm.repository.ConsolidatingModificationCollector.consolidate;
 
 //~--- JDK imports ------------------------------------------------------------
 
@@ -118,22 +120,29 @@ public final class SvnUtil
     return result;
   }
 
-
+  public static Modifications createModifications(String startRevision, String endRevision, Collection<SVNLogEntry> entries) {
-  public static Modifications createModifications(SVNLogEntry entry, String revision) {
+    Collection<Modification> consolidatedModifications =
-    Map<String, SVNLogEntryPath> changeMap = entry.getChangedPaths();
+      entries.stream()
-
+        .flatMap(SvnUtil::createModificationStream)
-    List<Modification> modificationList;
+        .collect(consolidate());
-    if (Util.isNotEmpty(changeMap)) {
+    return new Modifications(startRevision, endRevision, consolidatedModifications);
-      modificationList = changeMap.values().stream()
-        .map(e -> asModification(e.getType(), e.getPath()))
-        .filter(Optional::isPresent)
-        .map(Optional::get)
-        .collect(Collectors.toList());
-    } else {
-      modificationList = emptyList();
   }
 
-    return new Modifications(revision, modificationList);
+  public static Modifications createModifications(SVNLogEntry entry, String revision) {
+    return new Modifications(revision, createModificationStream(entry).collect(toList()));
+  }
+
+  private static Stream<Modification> createModificationStream(SVNLogEntry entry) {
+    Map<String, SVNLogEntryPath> changeMap = entry.getChangedPaths();
+
+    if (Util.isNotEmpty(changeMap)) {
+      return changeMap.values().stream()
+        .map(e -> asModification(e.getType(), e.getPath()))
+        .filter(Optional::isPresent)
+        .map(Optional::get);
+    } else {
+      return Stream.empty();
+    }
   }
 
   public static Optional<Modification> asModification(char type, String path) {
@@ -383,4 +392,5 @@ public final class SvnUtil
   {
     return Strings.nullToEmpty(id).startsWith(ID_TRANSACTION_PREFIX);
   }
+
 }

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

@@ -48,14 +48,12 @@ public class SvnModificationsCommand extends AbstractSvnCommand implements Modif
 
   @Override
   public Modifications getModifications(String revisionOrTransactionId) {
-    Modifications modifications;
     try {
       if (SvnUtil.isTransactionEntryId(revisionOrTransactionId)) {
-        modifications = getModificationsFromTransaction(SvnUtil.getTransactionId(revisionOrTransactionId));
+        return getModificationsFromTransaction(SvnUtil.getTransactionId(revisionOrTransactionId));
       } else {
-        modifications = getModificationFromRevision(revisionOrTransactionId);
+        return getModificationFromRevision(revisionOrTransactionId, revisionOrTransactionId);
       }
-      return modifications;
     } catch (SVNException ex) {
       throw new InternalRepositoryException(
         repository,
@@ -65,15 +63,33 @@ public class SvnModificationsCommand extends AbstractSvnCommand implements Modif
     }
   }
 
+  @Override
+  public Modifications getModifications(String baseRevision, String revision) {
+    try {
+      return getModificationFromRevision(baseRevision, revision);
+    } catch (SVNException ex) {
+      throw new InternalRepositoryException(
+        repository,
+        "failed to get svn modifications from " + baseRevision + " to " + revision,
+        ex
+      );
+    }
+  }
+
   @SuppressWarnings("unchecked")
-  private Modifications getModificationFromRevision(String revision) throws SVNException {
+  private Modifications getModificationFromRevision(String startRevision, String endRevision) throws SVNException {
-    log.debug("get svn modifications from revision: {}", revision);
+    log.debug("get svn modifications from revision {} to {}", startRevision, endRevision);
-    long revisionNumber = SvnUtil.getRevisionNumber(revision, repository);
+    long startRevisionNumber = SvnUtil.getRevisionNumber(startRevision, repository);
+    long endRevisionNumber = SvnUtil.getRevisionNumber(endRevision, repository);
     SVNRepository repo = open();
-    Collection<SVNLogEntry> entries = repo.log(null, null, revisionNumber,
+    Collection<SVNLogEntry> entries = repo.log(null, null, startRevisionNumber,
-      revisionNumber, true, true);
+      endRevisionNumber, true, true);
     if (Util.isNotEmpty(entries)) {
-      return SvnUtil.createModifications(entries.iterator().next(), revision);
+      if (startRevision.equals(endRevision)) {
+        return SvnUtil.createModifications(entries.iterator().next(), endRevision);
+      } else {
+        return SvnUtil.createModifications(startRevision, endRevision, entries);
+      }
     }
     return null;
   }
@@ -87,10 +103,4 @@ public class SvnModificationsCommand extends AbstractSvnCommand implements Modif
 
     return new Modifications(null, modificationList);
   }
-
-  @Override
-  public Modifications getModifications(ModificationsCommandRequest request) {
-    return getModifications(request.getRevision());
-  }
-
 }

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

@@ -26,6 +26,7 @@ package sonia.scm.repository.spi;
 
 import com.google.common.collect.ImmutableSet;
 import com.google.common.io.Closeables;
+import sonia.scm.repository.Feature;
 import sonia.scm.repository.Repository;
 import sonia.scm.repository.SvnRepositoryHandler;
 import sonia.scm.repository.SvnWorkingCopyFactory;
@@ -34,6 +35,7 @@ import sonia.scm.repository.api.HookContextFactory;
 
 import javax.net.ssl.TrustManager;
 import java.io.IOException;
+import java.util.EnumSet;
 import java.util.Set;
 
 /**
@@ -41,7 +43,6 @@ import java.util.Set;
  */
 public class SvnRepositoryServiceProvider extends RepositoryServiceProvider {
 
-  //J-
   public static final Set<Command> COMMANDS = ImmutableSet.of(
     Command.BLAME,
     Command.BROWSE,
@@ -55,8 +56,10 @@ public class SvnRepositoryServiceProvider extends RepositoryServiceProvider {
     Command.FULL_HEALTH_CHECK,
     Command.MIRROR
   );
-  //J+
 
+  public static final Set<Feature> FEATURES = EnumSet.of(
+    Feature.MODIFICATIONS_BETWEEN_REVISIONS
+  );
 
   private final SvnContext context;
   private final SvnWorkingCopyFactory workingCopyFactory;
@@ -129,6 +132,11 @@ public class SvnRepositoryServiceProvider extends RepositoryServiceProvider {
     return COMMANDS;
   }
 
+  @Override
+  public Set<Feature> getSupportedFeatures() {
+    return FEATURES;
+  }
+
   @Override
   public UnbundleCommand getUnbundleCommand() {
     return new SvnUnbundleCommand(context, hookContextFactory, new SvnLogCommand(context));

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

@@ -0,0 +1,133 @@
+/*
+ * 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;
+
+import org.junit.jupiter.api.Test;
+import org.junit.jupiter.api.extension.ExtendWith;
+import org.mockito.junit.jupiter.MockitoExtension;
+
+import java.util.Collection;
+import java.util.stream.Stream;
+
+import static org.assertj.core.api.Assertions.assertThat;
+
+@ExtendWith(MockitoExtension.class)
+class ConsolidatingModificationCollectorTest {
+
+  @Test
+  void shouldKeepIndependentChanges() {
+    Collection<Modification> consolidated =
+      Stream.of(
+        new Added("added"),
+        new Removed("removed"),
+        new Modified("modified")
+      ).collect(new ConsolidatingModificationCollector());
+
+    assertThat(consolidated)
+      .extracting("class")
+      .containsExactlyInAnyOrder(Added.class, Removed.class, Modified.class);
+  }
+
+  @Test
+  void shouldNotListAddedFileIfRemovedLaterOn() {
+    Collection<Modification> consolidated =
+      Stream.of(
+        new Added("file"),
+        new Removed("file")
+      ).collect(new ConsolidatingModificationCollector());
+
+    assertThat(consolidated)
+      .isEmpty();
+  }
+
+  @Test
+  void shouldReplaceModificationWithRemove() {
+    Collection<Modification> consolidated =
+      Stream.of(
+        new Modified("file"),
+        new Removed("file")
+      ).collect(new ConsolidatingModificationCollector());
+
+    assertThat(consolidated)
+      .extracting("class")
+      .containsExactly(Removed.class);
+  }
+
+  @Test
+  void shouldReplaceCopyWithAdd() {
+    Collection<Modification> consolidated =
+      Stream.of(
+        new Copied("source", "target")
+      ).collect(new ConsolidatingModificationCollector());
+
+    assertThat(consolidated)
+      .extracting("class")
+      .containsExactly(Added.class);
+    assertThat(consolidated)
+      .extracting("path")
+      .containsExactly("target");
+  }
+
+  @Test
+  void shouldReplaceRenameWithAddAndRemove() {
+    Collection<Modification> consolidated =
+      Stream.of(
+        new Renamed("source", "target")
+      ).collect(new ConsolidatingModificationCollector());
+
+    assertThat(consolidated)
+      .extracting("class")
+      .containsExactlyInAnyOrder(Added.class, Removed.class);
+    assertThat(consolidated)
+      .extracting("path")
+      .containsExactlyInAnyOrder("source", "target");
+  }
+
+  @Test
+  void shouldNotReplaceAddWithModify() {
+    Collection<Modification> consolidated =
+      Stream.of(
+        new Added("file"),
+        new Modified("file")
+      ).collect(new ConsolidatingModificationCollector());
+
+    assertThat(consolidated)
+      .extracting("class")
+      .containsExactlyInAnyOrder(Added.class);
+  }
+
+  @Test
+  void shouldReplaceAddWithModifyIfRemovedBefore() {
+    Collection<Modification> consolidated =
+      Stream.of(
+        new Removed("file"),
+        new Added("file")
+      ).collect(new ConsolidatingModificationCollector());
+
+    assertThat(consolidated)
+      .extracting("class")
+      .containsExactlyInAnyOrder(Modified.class);
+  }
+}

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

@@ -0,0 +1,53 @@
+/*
+ * 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 org.junit.Test;
+import sonia.scm.repository.Modifications;
+
+import static org.assertj.core.api.Assertions.assertThat;
+
+public class SvnModificationsCommandTest extends AbstractSvnCommandTestBase {
+
+  @Test
+  public void shouldReadModificationsForSingleRevision() {
+    SvnContext context = createContext();
+    SvnModificationsCommand svnModificationsCommand = new SvnModificationsCommand(context);
+
+    Modifications modifications = svnModificationsCommand.getModifications("4");
+
+    assertThat(modifications.getAdded()).hasSize(3);
+  }
+
+  @Test
+  public void shouldReadModificationsForMultipleRevisions() {
+    SvnContext context = createContext();
+    SvnModificationsCommand svnModificationsCommand = new SvnModificationsCommand(context);
+
+    Modifications modifications = svnModificationsCommand.getModifications("1", "4");
+
+    assertThat(modifications.getModifications()).hasSize(4);
+  }
+}