From e0411ed17f0b29e402b8587a8483689d14c93b84 Mon Sep 17 00:00:00 2001 From: Sebastian Sdorra Date: Mon, 29 Jul 2019 11:22:57 +0200 Subject: [PATCH 1/9] create API for parsed diff result command --- .../api/AbstractDiffCommandBuilder.java | 68 ++++++++++++++++++ .../sonia/scm/repository/api/Command.java | 12 +--- .../repository/api/DiffCommandBuilder.java | 69 +++---------------- .../sonia/scm/repository/api/DiffFile.java | 12 ++++ .../sonia/scm/repository/api/DiffLine.java | 12 ++++ .../sonia/scm/repository/api/DiffResult.java | 8 +++ .../api/DiffResultCommandBuilder.java | 41 +++++++++++ .../java/sonia/scm/repository/api/Hunk.java | 12 ++++ .../scm/repository/api/RepositoryService.java | 15 ++++ .../scm/repository/spi/DiffResultCommand.java | 9 +++ .../spi/RepositoryServiceProvider.java | 5 ++ 11 files changed, 192 insertions(+), 71 deletions(-) create mode 100644 scm-core/src/main/java/sonia/scm/repository/api/AbstractDiffCommandBuilder.java create mode 100644 scm-core/src/main/java/sonia/scm/repository/api/DiffFile.java create mode 100644 scm-core/src/main/java/sonia/scm/repository/api/DiffLine.java create mode 100644 scm-core/src/main/java/sonia/scm/repository/api/DiffResult.java create mode 100644 scm-core/src/main/java/sonia/scm/repository/api/DiffResultCommandBuilder.java create mode 100644 scm-core/src/main/java/sonia/scm/repository/api/Hunk.java create mode 100644 scm-core/src/main/java/sonia/scm/repository/spi/DiffResultCommand.java diff --git a/scm-core/src/main/java/sonia/scm/repository/api/AbstractDiffCommandBuilder.java b/scm-core/src/main/java/sonia/scm/repository/api/AbstractDiffCommandBuilder.java new file mode 100644 index 0000000000..b5b2f2a08b --- /dev/null +++ b/scm-core/src/main/java/sonia/scm/repository/api/AbstractDiffCommandBuilder.java @@ -0,0 +1,68 @@ +package sonia.scm.repository.api; + +import sonia.scm.FeatureNotSupportedException; +import sonia.scm.repository.Feature; +import sonia.scm.repository.spi.DiffCommandRequest; + +import java.util.Set; + +abstract class AbstractDiffCommandBuilder { + + + /** request for the diff command implementation */ + final DiffCommandRequest request = new DiffCommandRequest(); + + private final Set supportedFeatures; + + AbstractDiffCommandBuilder(Set supportedFeatures) { + this.supportedFeatures = supportedFeatures; + } + + /** + * Compute the incoming changes of the branch set with {@link #setRevision(String)} in respect to the changeset given + * here. In other words: What changes would be new to the ancestor changeset given here when the branch would + * be merged into it. Requires feature {@link sonia.scm.repository.Feature#INCOMING_REVISION}! + * + * @return {@code this} + */ + public T setAncestorChangeset(String revision) + { + if (!supportedFeatures.contains(Feature.INCOMING_REVISION)) { + throw new FeatureNotSupportedException(Feature.INCOMING_REVISION.name()); + } + request.setAncestorChangeset(revision); + + return self(); + } + + /** + * Show the difference only for the given path. + * + * + * @param path path for difference + * + * @return {@code this} + */ + public T setPath(String path) + { + request.setPath(path); + return self(); + } + + /** + * Show the difference only for the given revision or (using {@link #setAncestorChangeset(String)}) between this + * and another revision. + * + * + * @param revision revision for difference + * + * @return {@code this} + */ + public T setRevision(String revision) + { + request.setRevision(revision); + return self(); + } + + abstract T self(); +} diff --git a/scm-core/src/main/java/sonia/scm/repository/api/Command.java b/scm-core/src/main/java/sonia/scm/repository/api/Command.java index e380727769..3249e54ec3 100644 --- a/scm-core/src/main/java/sonia/scm/repository/api/Command.java +++ b/scm-core/src/main/java/sonia/scm/repository/api/Command.java @@ -53,11 +53,6 @@ public enum Command */ BRANCHES, - /** - * @since 2.0 - */ - BRANCH, - /** * @since 1.31 */ @@ -71,10 +66,5 @@ public enum Command /** * @since 2.0 */ - MODIFICATIONS, - - /** - * @since 2.0 - */ - MERGE + MODIFICATIONS, MERGE, DIFF_RESULT, BRANCH; } diff --git a/scm-core/src/main/java/sonia/scm/repository/api/DiffCommandBuilder.java b/scm-core/src/main/java/sonia/scm/repository/api/DiffCommandBuilder.java index 9e7094d5bf..18d4e11a7f 100644 --- a/scm-core/src/main/java/sonia/scm/repository/api/DiffCommandBuilder.java +++ b/scm-core/src/main/java/sonia/scm/repository/api/DiffCommandBuilder.java @@ -38,10 +38,8 @@ package sonia.scm.repository.api; import com.google.common.base.Preconditions; import org.slf4j.Logger; import org.slf4j.LoggerFactory; -import sonia.scm.FeatureNotSupportedException; import sonia.scm.repository.Feature; import sonia.scm.repository.spi.DiffCommand; -import sonia.scm.repository.spi.DiffCommandRequest; import sonia.scm.util.IOUtil; import java.io.ByteArrayOutputStream; @@ -72,7 +70,7 @@ import java.util.Set; * @author Sebastian Sdorra * @since 1.17 */ -public final class DiffCommandBuilder +public final class DiffCommandBuilder extends AbstractDiffCommandBuilder { /** @@ -81,6 +79,9 @@ public final class DiffCommandBuilder private static final Logger logger = LoggerFactory.getLogger(DiffCommandBuilder.class); + /** implementation of the diff command */ + private final DiffCommand diffCommand; + //~--- constructors --------------------------------------------------------- /** @@ -92,8 +93,8 @@ public final class DiffCommandBuilder */ DiffCommandBuilder(DiffCommand diffCommand, Set supportedFeatures) { + super(supportedFeatures); this.diffCommand = diffCommand; - this.supportedFeatures = supportedFeatures; } //~--- methods -------------------------------------------------------------- @@ -162,54 +163,6 @@ public final class DiffCommandBuilder return this; } - - /** - * Show the difference only for the given path. - * - * - * @param path path for difference - * - * @return {@code this} - */ - public DiffCommandBuilder setPath(String path) - { - request.setPath(path); - - return this; - } - - /** - * Show the difference only for the given revision or (using {@link #setAncestorChangeset(String)}) between this - * and another revision. - * - * - * @param revision revision for difference - * - * @return {@code this} - */ - public DiffCommandBuilder setRevision(String revision) - { - request.setRevision(revision); - - return this; - } - /** - * Compute the incoming changes of the branch set with {@link #setRevision(String)} in respect to the changeset given - * here. In other words: What changes would be new to the ancestor changeset given here when the branch would - * be merged into it. Requires feature {@link sonia.scm.repository.Feature#INCOMING_REVISION}! - * - * @return {@code this} - */ - public DiffCommandBuilder setAncestorChangeset(String revision) - { - if (!supportedFeatures.contains(Feature.INCOMING_REVISION)) { - throw new FeatureNotSupportedException(Feature.INCOMING_REVISION.name()); - } - request.setAncestorChangeset(revision); - - return this; - } - //~--- get methods ---------------------------------------------------------- /** @@ -233,12 +186,8 @@ public final class DiffCommandBuilder diffCommand.getDiffResult(request, outputStream); } - //~--- fields --------------------------------------------------------------- - - /** implementation of the diff command */ - private final DiffCommand diffCommand; - private Set supportedFeatures; - - /** request for the diff command implementation */ - private final DiffCommandRequest request = new DiffCommandRequest(); + @Override + DiffCommandBuilder self() { + return this; + } } diff --git a/scm-core/src/main/java/sonia/scm/repository/api/DiffFile.java b/scm-core/src/main/java/sonia/scm/repository/api/DiffFile.java new file mode 100644 index 0000000000..d1d223e272 --- /dev/null +++ b/scm-core/src/main/java/sonia/scm/repository/api/DiffFile.java @@ -0,0 +1,12 @@ +package sonia.scm.repository.api; + +public interface DiffFile extends Iterable { + + String getOldRevision(); + + String getNewRevision(); + + String getOldName(); + + String getNewName(); +} diff --git a/scm-core/src/main/java/sonia/scm/repository/api/DiffLine.java b/scm-core/src/main/java/sonia/scm/repository/api/DiffLine.java new file mode 100644 index 0000000000..193e5e75d5 --- /dev/null +++ b/scm-core/src/main/java/sonia/scm/repository/api/DiffLine.java @@ -0,0 +1,12 @@ +package sonia.scm.repository.api; + +import java.util.OptionalInt; + +public interface DiffLine { + + OptionalInt getOldLineNumber(); + + OptionalInt getNewLineNumber(); + + String getContent(); +} diff --git a/scm-core/src/main/java/sonia/scm/repository/api/DiffResult.java b/scm-core/src/main/java/sonia/scm/repository/api/DiffResult.java new file mode 100644 index 0000000000..b662db4e2d --- /dev/null +++ b/scm-core/src/main/java/sonia/scm/repository/api/DiffResult.java @@ -0,0 +1,8 @@ +package sonia.scm.repository.api; + +public interface DiffResult extends Iterable { + + String getOldRevision(); + + String getNewRevision(); +} diff --git a/scm-core/src/main/java/sonia/scm/repository/api/DiffResultCommandBuilder.java b/scm-core/src/main/java/sonia/scm/repository/api/DiffResultCommandBuilder.java new file mode 100644 index 0000000000..7e152f3d0f --- /dev/null +++ b/scm-core/src/main/java/sonia/scm/repository/api/DiffResultCommandBuilder.java @@ -0,0 +1,41 @@ +package sonia.scm.repository.api; + +import com.google.common.base.Preconditions; +import org.slf4j.Logger; +import org.slf4j.LoggerFactory; +import sonia.scm.repository.Feature; +import sonia.scm.repository.spi.DiffResultCommand; + +import java.io.IOException; +import java.util.Set; + +public class DiffResultCommandBuilder extends AbstractDiffCommandBuilder { + + private static final Logger LOG = LoggerFactory.getLogger(DiffResultCommandBuilder.class); + + private final DiffResultCommand diffResultCommand; + + DiffResultCommandBuilder(DiffResultCommand diffResultCommand, Set supportedFeatures) { + super(supportedFeatures); + this.diffResultCommand = diffResultCommand; + } + + /** + * Returns the content of the difference as parsed objects. + * + * @return content of the difference + */ + public DiffResult getDiffResult() throws IOException { + Preconditions.checkArgument(request.isValid(), + "path and/or revision is required"); + + LOG.debug("create diff result for {}", request); + + return diffResultCommand.getDiffResult(request); + } + + @Override + DiffResultCommandBuilder self() { + return this; + } +} diff --git a/scm-core/src/main/java/sonia/scm/repository/api/Hunk.java b/scm-core/src/main/java/sonia/scm/repository/api/Hunk.java new file mode 100644 index 0000000000..6e60f8b2bd --- /dev/null +++ b/scm-core/src/main/java/sonia/scm/repository/api/Hunk.java @@ -0,0 +1,12 @@ +package sonia.scm.repository.api; + +public interface Hunk extends Iterable { + + int getOldStart(); + + int getOldLineCount(); + + int getNewStart(); + + int getNewLineCount(); +} diff --git a/scm-core/src/main/java/sonia/scm/repository/api/RepositoryService.java b/scm-core/src/main/java/sonia/scm/repository/api/RepositoryService.java index 90978d75ea..c06edcd918 100644 --- a/scm-core/src/main/java/sonia/scm/repository/api/RepositoryService.java +++ b/scm-core/src/main/java/sonia/scm/repository/api/RepositoryService.java @@ -239,6 +239,21 @@ public final class RepositoryService implements Closeable { return new DiffCommandBuilder(provider.getDiffCommand(), provider.getSupportedFeatures()); } + /** + * The diff command shows differences between revisions for a specified file + * or the entire revision. + * + * @return instance of {@link DiffResultCommandBuilder} + * @throws CommandNotSupportedException if the command is not supported + * by the implementation of the repository service provider. + */ + public DiffResultCommandBuilder getDiffResultCommand() { + LOG.debug("create diff result command for repository {}", + repository.getNamespaceAndName()); + + return new DiffResultCommandBuilder(provider.getDiffResultCommand(), provider.getSupportedFeatures()); + } + /** * The incoming command shows new {@link Changeset}s found in a different * repository location. diff --git a/scm-core/src/main/java/sonia/scm/repository/spi/DiffResultCommand.java b/scm-core/src/main/java/sonia/scm/repository/spi/DiffResultCommand.java new file mode 100644 index 0000000000..ee50178d76 --- /dev/null +++ b/scm-core/src/main/java/sonia/scm/repository/spi/DiffResultCommand.java @@ -0,0 +1,9 @@ +package sonia.scm.repository.spi; + +import sonia.scm.repository.api.DiffResult; + +import java.io.IOException; + +public interface DiffResultCommand { + DiffResult getDiffResult(DiffCommandRequest request) throws IOException; +} diff --git a/scm-core/src/main/java/sonia/scm/repository/spi/RepositoryServiceProvider.java b/scm-core/src/main/java/sonia/scm/repository/spi/RepositoryServiceProvider.java index a82eb7c30a..bf9cdf6a25 100644 --- a/scm-core/src/main/java/sonia/scm/repository/spi/RepositoryServiceProvider.java +++ b/scm-core/src/main/java/sonia/scm/repository/spi/RepositoryServiceProvider.java @@ -158,6 +158,11 @@ public abstract class RepositoryServiceProvider implements Closeable throw new CommandNotSupportedException(Command.DIFF); } + public DiffResultCommand getDiffResultCommand() + { + throw new CommandNotSupportedException(Command.DIFF_RESULT); + } + /** * Method description * From 01379caa08d17a3ec4c2fb671a989dec7ac2af96 Mon Sep 17 00:00:00 2001 From: Sebastian Sdorra Date: Mon, 29 Jul 2019 12:54:58 +0200 Subject: [PATCH 2/9] implement first diff details --- .../sonia/scm/repository/api/DiffFile.java | 4 +- .../java/sonia/scm/repository/spi/Differ.java | 113 ++++++++++++++++++ .../repository/spi/GitDiffResultCommand.java | 85 +++++++++++++ .../spi/GitDiffResultCommandTest.java | 42 +++++++ 4 files changed, 242 insertions(+), 2 deletions(-) create mode 100644 scm-plugins/scm-git-plugin/src/main/java/sonia/scm/repository/spi/Differ.java create mode 100644 scm-plugins/scm-git-plugin/src/main/java/sonia/scm/repository/spi/GitDiffResultCommand.java create mode 100644 scm-plugins/scm-git-plugin/src/test/java/sonia/scm/repository/spi/GitDiffResultCommandTest.java diff --git a/scm-core/src/main/java/sonia/scm/repository/api/DiffFile.java b/scm-core/src/main/java/sonia/scm/repository/api/DiffFile.java index d1d223e272..a3b1bafe0b 100644 --- a/scm-core/src/main/java/sonia/scm/repository/api/DiffFile.java +++ b/scm-core/src/main/java/sonia/scm/repository/api/DiffFile.java @@ -6,7 +6,7 @@ public interface DiffFile extends Iterable { String getNewRevision(); - String getOldName(); + String getOldPath(); - String getNewName(); + String getNewPath(); } diff --git a/scm-plugins/scm-git-plugin/src/main/java/sonia/scm/repository/spi/Differ.java b/scm-plugins/scm-git-plugin/src/main/java/sonia/scm/repository/spi/Differ.java new file mode 100644 index 0000000000..67d503aeff --- /dev/null +++ b/scm-plugins/scm-git-plugin/src/main/java/sonia/scm/repository/spi/Differ.java @@ -0,0 +1,113 @@ +package sonia.scm.repository.spi; + +import com.google.common.base.Strings; +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; +import org.eclipse.jgit.treewalk.EmptyTreeIterator; +import org.eclipse.jgit.treewalk.TreeWalk; +import org.eclipse.jgit.treewalk.filter.PathFilter; +import sonia.scm.repository.GitUtil; +import sonia.scm.util.Util; + +import java.io.IOException; +import java.util.List; +import java.util.function.Consumer; + +final class Differ implements AutoCloseable { + + private final RevWalk walk; + private final TreeWalk treeWalk; + private final RevCommit commit; + + private Differ(RevCommit commit, RevWalk walk, TreeWalk treeWalk) { + this.commit = commit; + this.walk = walk; + this.treeWalk = treeWalk; + } + + public static Differ create(Repository repository, DiffCommandRequest request) throws IOException { + RevWalk walk = new RevWalk(repository); + + ObjectId revision = repository.resolve(request.getRevision()); + RevCommit commit = walk.parseCommit(revision); + + walk.markStart(commit); + commit = walk.next(); + TreeWalk treeWalk = new TreeWalk(repository); + treeWalk.reset(); + treeWalk.setRecursive(true); + + if (Util.isNotEmpty(request.getPath())) + { + treeWalk.setFilter(PathFilter.create(request.getPath())); + } + + + if (!Strings.isNullOrEmpty(request.getAncestorChangeset())) + { + ObjectId otherRevision = repository.resolve(request.getAncestorChangeset()); + ObjectId ancestorId = computeCommonAncestor(repository, revision, otherRevision); + RevTree tree = walk.parseCommit(ancestorId).getTree(); + treeWalk.addTree(tree); + } + else if (commit.getParentCount() > 0) + { + RevTree tree = commit.getParent(0).getTree(); + + if (tree != null) + { + treeWalk.addTree(tree); + } + else + { + treeWalk.addTree(new EmptyTreeIterator()); + } + } + else + { + treeWalk.addTree(new EmptyTreeIterator()); + } + + treeWalk.addTree(commit.getTree()); + + return new Differ(commit, walk, treeWalk); + } + + private static ObjectId computeCommonAncestor(org.eclipse.jgit.lib.Repository repository, ObjectId revision1, ObjectId revision2) throws IOException { + return GitUtil.computeCommonAncestor(repository, revision1, revision2); + } + + public void process(Consumer diffConsumer) throws IOException { + List entries = DiffEntry.scan(treeWalk); + diffConsumer.accept(new Diff(commit, entries)); + } + + @Override + public void close() { + GitUtil.release(walk); + GitUtil.release(treeWalk); + } + + public static class Diff { + + private final RevCommit commit; + private final List entries; + + private Diff(RevCommit commit, List entries) { + this.commit = commit; + this.entries = entries; + } + + public RevCommit getCommit() { + return commit; + } + + public List getEntries() { + return entries; + } + } +} diff --git a/scm-plugins/scm-git-plugin/src/main/java/sonia/scm/repository/spi/GitDiffResultCommand.java b/scm-plugins/scm-git-plugin/src/main/java/sonia/scm/repository/spi/GitDiffResultCommand.java new file mode 100644 index 0000000000..a7c8cc3f84 --- /dev/null +++ b/scm-plugins/scm-git-plugin/src/main/java/sonia/scm/repository/spi/GitDiffResultCommand.java @@ -0,0 +1,85 @@ +package sonia.scm.repository.spi; + +import org.eclipse.jgit.diff.DiffEntry; +import sonia.scm.repository.GitUtil; +import sonia.scm.repository.Repository; +import sonia.scm.repository.api.DiffFile; +import sonia.scm.repository.api.DiffResult; +import sonia.scm.repository.api.Hunk; + +import java.io.IOException; +import java.util.Iterator; +import java.util.stream.Collectors; + +public class GitDiffResultCommand extends AbstractGitCommand implements DiffResultCommand { + + GitDiffResultCommand(GitContext context, Repository repository) { + super(context, repository); + } + + public DiffResult getDiffResult(DiffCommandRequest diffCommandRequest) throws IOException { + try (Differ differ = Differ.create(open(), diffCommandRequest)) { + GitDiffResult result = new GitDiffResult(); + differ.process(result::process); + return result; + } + } + + private class GitDiffResult implements DiffResult { + + private Differ.Diff diff; + + void process(Differ.Diff diff) { + this.diff = diff; + } + + @Override + public String getOldRevision() { + return GitUtil.getId(diff.getCommit().getParent(0).getId()); + } + + @Override + public String getNewRevision() { + return GitUtil.getId(diff.getCommit().getId()); + } + + @Override + public Iterator iterator() { + return diff.getEntries().stream().map(GitDiffFile::new).collect(Collectors.toList()).iterator(); + } + } + + private static class GitDiffFile implements DiffFile { + + private final DiffEntry diffEntry; + + private GitDiffFile(DiffEntry diffEntry) { + this.diffEntry = diffEntry; + } + + @Override + public String getOldRevision() { + return null; + } + + @Override + public String getNewRevision() { + return null; + } + + @Override + public String getOldPath() { + return diffEntry.getOldPath(); + } + + @Override + public String getNewPath() { + return diffEntry.getNewPath(); + } + + @Override + public Iterator iterator() { + return null; + } + } +} diff --git a/scm-plugins/scm-git-plugin/src/test/java/sonia/scm/repository/spi/GitDiffResultCommandTest.java b/scm-plugins/scm-git-plugin/src/test/java/sonia/scm/repository/spi/GitDiffResultCommandTest.java new file mode 100644 index 0000000000..a8b90d2b5c --- /dev/null +++ b/scm-plugins/scm-git-plugin/src/test/java/sonia/scm/repository/spi/GitDiffResultCommandTest.java @@ -0,0 +1,42 @@ +package sonia.scm.repository.spi; + +import org.junit.Test; +import sonia.scm.repository.api.DiffFile; +import sonia.scm.repository.api.DiffResult; + +import java.io.IOException; +import java.util.Iterator; + +import static org.assertj.core.api.Assertions.assertThat; + +public class GitDiffResultCommandTest extends AbstractGitCommandTestBase { + + @Test + public void shouldReturnOldAndNewRevision() throws IOException { + GitDiffResultCommand gitDiffResultCommand = new GitDiffResultCommand(createContext(), repository); + DiffCommandRequest diffCommandRequest = new DiffCommandRequest(); + diffCommandRequest.setRevision("3f76a12f08a6ba0dc988c68b7f0b2cd190efc3c4"); + + DiffResult diffResult = gitDiffResultCommand.getDiffResult(diffCommandRequest); + + assertThat(diffResult.getNewRevision()).isEqualTo("3f76a12f08a6ba0dc988c68b7f0b2cd190efc3c4"); + assertThat(diffResult.getOldRevision()).isEqualTo("592d797cd36432e591416e8b2b98154f4f163411"); + } + + @Test + public void shouldReturnFilePaths() throws IOException { + GitDiffResultCommand gitDiffResultCommand = new GitDiffResultCommand(createContext(), repository); + DiffCommandRequest diffCommandRequest = new DiffCommandRequest(); + diffCommandRequest.setRevision("3f76a12f08a6ba0dc988c68b7f0b2cd190efc3c4"); + + DiffResult diffResult = gitDiffResultCommand.getDiffResult(diffCommandRequest); + Iterator iterator = diffResult.iterator(); + DiffFile a = iterator.next(); + assertThat(a.getNewPath()).isEqualTo("a.txt"); + assertThat(a.getOldPath()).isEqualTo("a.txt"); + + DiffFile b = iterator.next(); + assertThat(b.getOldPath()).isEqualTo("b.txt"); + assertThat(b.getNewPath()).isEqualTo("/dev/null"); + } +} From 07068880bb702b495d3967245b536d13c150d808 Mon Sep 17 00:00:00 2001 From: Sebastian Sdorra Date: Mon, 29 Jul 2019 16:42:49 +0200 Subject: [PATCH 3/9] implemented parsing of git diff hunks --- .../java/sonia/scm/repository/GitUtil.java | 3 +- .../sonia/scm/repository/spi/FileRange.java | 20 ++ .../repository/spi/GitDiffResultCommand.java | 42 ++++- .../sonia/scm/repository/spi/GitHunk.java | 48 +++++ .../scm/repository/spi/GitHunkParser.java | 175 ++++++++++++++++++ .../spi/GitDiffResultCommandTest.java | 67 ++++++- .../scm/repository/spi/GitHunkParserTest.java | 138 ++++++++++++++ 7 files changed, 474 insertions(+), 19 deletions(-) create mode 100644 scm-plugins/scm-git-plugin/src/main/java/sonia/scm/repository/spi/FileRange.java create mode 100644 scm-plugins/scm-git-plugin/src/main/java/sonia/scm/repository/spi/GitHunk.java create mode 100644 scm-plugins/scm-git-plugin/src/main/java/sonia/scm/repository/spi/GitHunkParser.java create mode 100644 scm-plugins/scm-git-plugin/src/test/java/sonia/scm/repository/spi/GitHunkParserTest.java diff --git a/scm-plugins/scm-git-plugin/src/main/java/sonia/scm/repository/GitUtil.java b/scm-plugins/scm-git-plugin/src/main/java/sonia/scm/repository/GitUtil.java index 7aacdb256a..7175d3b646 100644 --- a/scm-plugins/scm-git-plugin/src/main/java/sonia/scm/repository/GitUtil.java +++ b/scm-plugins/scm-git-plugin/src/main/java/sonia/scm/repository/GitUtil.java @@ -43,6 +43,7 @@ import org.eclipse.jgit.api.FetchCommand; import org.eclipse.jgit.api.Git; import org.eclipse.jgit.api.errors.GitAPIException; import org.eclipse.jgit.diff.DiffFormatter; +import org.eclipse.jgit.lib.AnyObjectId; import org.eclipse.jgit.lib.Constants; import org.eclipse.jgit.lib.ObjectId; import org.eclipse.jgit.lib.Ref; @@ -441,7 +442,7 @@ public final class GitUtil * * @return */ - public static String getId(ObjectId objectId) + public static String getId(AnyObjectId objectId) { String id = Util.EMPTY_STRING; diff --git a/scm-plugins/scm-git-plugin/src/main/java/sonia/scm/repository/spi/FileRange.java b/scm-plugins/scm-git-plugin/src/main/java/sonia/scm/repository/spi/FileRange.java new file mode 100644 index 0000000000..8d445e1c44 --- /dev/null +++ b/scm-plugins/scm-git-plugin/src/main/java/sonia/scm/repository/spi/FileRange.java @@ -0,0 +1,20 @@ +package sonia.scm.repository.spi; + +public class FileRange { + + private final int start; + private final int lineCount; + + public FileRange(int start, int lineCount) { + this.start = start; + this.lineCount = lineCount; + } + + public int getStart() { + return start; + } + + public int getLineCount() { + return lineCount; + } +} diff --git a/scm-plugins/scm-git-plugin/src/main/java/sonia/scm/repository/spi/GitDiffResultCommand.java b/scm-plugins/scm-git-plugin/src/main/java/sonia/scm/repository/spi/GitDiffResultCommand.java index a7c8cc3f84..de7c8483f5 100644 --- a/scm-plugins/scm-git-plugin/src/main/java/sonia/scm/repository/spi/GitDiffResultCommand.java +++ b/scm-plugins/scm-git-plugin/src/main/java/sonia/scm/repository/spi/GitDiffResultCommand.java @@ -1,12 +1,15 @@ package sonia.scm.repository.spi; +import com.google.common.base.Throwables; import org.eclipse.jgit.diff.DiffEntry; +import org.eclipse.jgit.diff.DiffFormatter; import sonia.scm.repository.GitUtil; import sonia.scm.repository.Repository; import sonia.scm.repository.api.DiffFile; import sonia.scm.repository.api.DiffResult; import sonia.scm.repository.api.Hunk; +import java.io.ByteArrayOutputStream; import java.io.IOException; import java.util.Iterator; import java.util.stream.Collectors; @@ -18,8 +21,9 @@ public class GitDiffResultCommand extends AbstractGitCommand implements DiffResu } public DiffResult getDiffResult(DiffCommandRequest diffCommandRequest) throws IOException { - try (Differ differ = Differ.create(open(), diffCommandRequest)) { - GitDiffResult result = new GitDiffResult(); + org.eclipse.jgit.lib.Repository repository = open(); + try (Differ differ = Differ.create(repository, diffCommandRequest)) { + GitDiffResult result = new GitDiffResult(repository); differ.process(result::process); return result; } @@ -27,8 +31,13 @@ public class GitDiffResultCommand extends AbstractGitCommand implements DiffResu private class GitDiffResult implements DiffResult { + private final org.eclipse.jgit.lib.Repository repository; private Differ.Diff diff; + private GitDiffResult(org.eclipse.jgit.lib.Repository repository) { + this.repository = repository; + } + void process(Differ.Diff diff) { this.diff = diff; } @@ -45,26 +54,28 @@ public class GitDiffResultCommand extends AbstractGitCommand implements DiffResu @Override public Iterator iterator() { - return diff.getEntries().stream().map(GitDiffFile::new).collect(Collectors.toList()).iterator(); + return diff.getEntries().stream().map(diffEntry -> new GitDiffFile(repository, diffEntry)).collect(Collectors.toList()).iterator(); } } - private static class GitDiffFile implements DiffFile { + private class GitDiffFile implements DiffFile { + private final org.eclipse.jgit.lib.Repository repository; private final DiffEntry diffEntry; - private GitDiffFile(DiffEntry diffEntry) { + private GitDiffFile(org.eclipse.jgit.lib.Repository repository, DiffEntry diffEntry) { + this.repository = repository; this.diffEntry = diffEntry; } @Override public String getOldRevision() { - return null; + return GitUtil.getId(diffEntry.getOldId().toObjectId()); } @Override public String getNewRevision() { - return null; + return GitUtil.getId(diffEntry.getNewId().toObjectId()); } @Override @@ -79,7 +90,22 @@ public class GitDiffResultCommand extends AbstractGitCommand implements DiffResu @Override public Iterator iterator() { - return null; + String content = format(repository, diffEntry); + GitHunkParser parser = new GitHunkParser(); + return parser.parse(content).iterator(); } + + private String format(org.eclipse.jgit.lib.Repository repository, DiffEntry entry) { + try(ByteArrayOutputStream baos = new ByteArrayOutputStream()) { + DiffFormatter formatter = new DiffFormatter(baos); + formatter.setRepository(repository); + formatter.format(entry); + return baos.toString(); + } catch (IOException ex) { + throw Throwables.propagate(ex); + } + } + } + } diff --git a/scm-plugins/scm-git-plugin/src/main/java/sonia/scm/repository/spi/GitHunk.java b/scm-plugins/scm-git-plugin/src/main/java/sonia/scm/repository/spi/GitHunk.java new file mode 100644 index 0000000000..9a272d7b10 --- /dev/null +++ b/scm-plugins/scm-git-plugin/src/main/java/sonia/scm/repository/spi/GitHunk.java @@ -0,0 +1,48 @@ +package sonia.scm.repository.spi; + +import sonia.scm.repository.api.DiffLine; +import sonia.scm.repository.api.Hunk; + +import java.util.Iterator; +import java.util.List; + +public class GitHunk implements Hunk { + + private final FileRange oldFileRange; + private final FileRange newFileRange; + private List lines; + + public GitHunk(FileRange oldFileRange, FileRange newFileRange) { + this.oldFileRange = oldFileRange; + this.newFileRange = newFileRange; + } + + @Override + public int getOldStart() { + return oldFileRange.getStart(); + } + + @Override + public int getOldLineCount() { + return oldFileRange.getLineCount(); + } + + @Override + public int getNewStart() { + return newFileRange.getStart(); + } + + @Override + public int getNewLineCount() { + return newFileRange.getLineCount(); + } + + @Override + public Iterator iterator() { + return lines.iterator(); + } + + void setLines(List lines) { + this.lines = lines; + } +} diff --git a/scm-plugins/scm-git-plugin/src/main/java/sonia/scm/repository/spi/GitHunkParser.java b/scm-plugins/scm-git-plugin/src/main/java/sonia/scm/repository/spi/GitHunkParser.java new file mode 100644 index 0000000000..caedc6605f --- /dev/null +++ b/scm-plugins/scm-git-plugin/src/main/java/sonia/scm/repository/spi/GitHunkParser.java @@ -0,0 +1,175 @@ +package sonia.scm.repository.spi; + +import sonia.scm.repository.api.DiffLine; +import sonia.scm.repository.api.Hunk; + +import java.util.ArrayList; +import java.util.List; +import java.util.OptionalInt; +import java.util.Scanner; + +import static java.util.OptionalInt.of; + +final class GitHunkParser { + private static final int HEADER_PREFIX_LENGTH = "@@ -".length(); + private static final int HEADER_SUFFIX_LENGTH = " @@".length(); + + private GitHunk currentGitHunk = null; + private List collectedLines = null; + private int oldLineCounter = 0; + private int newLineCounter = 0; + + GitHunkParser() { + } + + public List parse(String content) { + List hunks = new ArrayList<>(); + + try (Scanner scanner = new Scanner(content)) { + while (scanner.hasNextLine()) { + String line = scanner.nextLine(); + if (line.startsWith("@@")) { + parseHeader(hunks, line); + } else if (currentGitHunk != null) { + parseDiffLine(line); + } + } + } + if (currentGitHunk != null) { + currentGitHunk.setLines(collectedLines); + } + + return hunks; + } + + private void parseHeader(List hunks, String line) { + if (currentGitHunk != null) { + currentGitHunk.setLines(collectedLines); + } + String hunkHeader = line.substring(HEADER_PREFIX_LENGTH, line.length() - HEADER_SUFFIX_LENGTH); + String[] split = hunkHeader.split("\\s"); + + FileRange oldFileRange = createFileRange(split[0]); + // TODO merge contains two two block which starts with "-" e.g. -1,3 -2,4 +3,6 + FileRange newFileRange = createFileRange(split[1]); + + currentGitHunk = new GitHunk(oldFileRange, newFileRange); + hunks.add(currentGitHunk); + + collectedLines = new ArrayList<>(); + oldLineCounter = currentGitHunk.getOldStart(); + newLineCounter = currentGitHunk.getNewStart(); + } + + private void parseDiffLine(String line) { + String content = line.substring(1); + switch (line.charAt(0)) { + case ' ': + collectedLines.add(new UnchangedGitDiffLine(newLineCounter, oldLineCounter, content)); + ++newLineCounter; + ++oldLineCounter; + break; + case '+': + collectedLines.add(new AddedGitDiffLine(newLineCounter, content)); + ++newLineCounter; + break; + case '-': + collectedLines.add(new RemovedGitDiffLine(oldLineCounter, content)); + ++oldLineCounter; + break; + default: + throw new IllegalStateException("cannot handle diff line: " + line); + } + } + + private static class AddedGitDiffLine implements DiffLine { + private final int newLineNumber; + private final String content; + + private AddedGitDiffLine(int newLineNumber, String content) { + this.newLineNumber = newLineNumber; + this.content = content; + } + + @Override + public OptionalInt getOldLineNumber() { + return OptionalInt.empty(); + } + + @Override + public OptionalInt getNewLineNumber() { + return of(newLineNumber); + } + + @Override + public String getContent() { + return content; + } + } + + private static class RemovedGitDiffLine implements DiffLine { + private final int oldLineNumber; + private final String content; + + private RemovedGitDiffLine(int oldLineNumber, String content) { + this.oldLineNumber = oldLineNumber; + this.content = content; + } + + @Override + public OptionalInt getOldLineNumber() { + return of(oldLineNumber); + } + + @Override + public OptionalInt getNewLineNumber() { + return OptionalInt.empty(); + } + + @Override + public String getContent() { + return content; + } + } + + private static class UnchangedGitDiffLine implements DiffLine { + private final int newLineNumber; + private final int oldLineNumber; + private final String content; + + public UnchangedGitDiffLine(int newLineNumber, int oldLineNumber, String content) { + this.newLineNumber = newLineNumber; + this.oldLineNumber = oldLineNumber; + this.content = content; + } + + @Override + public OptionalInt getOldLineNumber() { + return of(oldLineNumber); + } + + @Override + public OptionalInt getNewLineNumber() { + return of(newLineNumber); + } + + @Override + public String getContent() { + return content; + } + } + + private static FileRange createFileRange(String fileRangeString) { + int start; + int lineCount = 1; + int commaIndex = fileRangeString.indexOf(','); + if (commaIndex > 0) { + start = Integer.parseInt(fileRangeString.substring(0, commaIndex)); + lineCount = Integer.parseInt(fileRangeString.substring(commaIndex + 1)); + } else { + start = Integer.parseInt(fileRangeString); + } + + return new FileRange(start, lineCount); + } +} diff --git a/scm-plugins/scm-git-plugin/src/test/java/sonia/scm/repository/spi/GitDiffResultCommandTest.java b/scm-plugins/scm-git-plugin/src/test/java/sonia/scm/repository/spi/GitDiffResultCommandTest.java index a8b90d2b5c..f359ae987c 100644 --- a/scm-plugins/scm-git-plugin/src/test/java/sonia/scm/repository/spi/GitDiffResultCommandTest.java +++ b/scm-plugins/scm-git-plugin/src/test/java/sonia/scm/repository/spi/GitDiffResultCommandTest.java @@ -3,6 +3,7 @@ package sonia.scm.repository.spi; import org.junit.Test; import sonia.scm.repository.api.DiffFile; import sonia.scm.repository.api.DiffResult; +import sonia.scm.repository.api.Hunk; import java.io.IOException; import java.util.Iterator; @@ -13,11 +14,7 @@ public class GitDiffResultCommandTest extends AbstractGitCommandTestBase { @Test public void shouldReturnOldAndNewRevision() throws IOException { - GitDiffResultCommand gitDiffResultCommand = new GitDiffResultCommand(createContext(), repository); - DiffCommandRequest diffCommandRequest = new DiffCommandRequest(); - diffCommandRequest.setRevision("3f76a12f08a6ba0dc988c68b7f0b2cd190efc3c4"); - - DiffResult diffResult = gitDiffResultCommand.getDiffResult(diffCommandRequest); + DiffResult diffResult = createDiffResult("3f76a12f08a6ba0dc988c68b7f0b2cd190efc3c4"); assertThat(diffResult.getNewRevision()).isEqualTo("3f76a12f08a6ba0dc988c68b7f0b2cd190efc3c4"); assertThat(diffResult.getOldRevision()).isEqualTo("592d797cd36432e591416e8b2b98154f4f163411"); @@ -25,11 +22,7 @@ public class GitDiffResultCommandTest extends AbstractGitCommandTestBase { @Test public void shouldReturnFilePaths() throws IOException { - GitDiffResultCommand gitDiffResultCommand = new GitDiffResultCommand(createContext(), repository); - DiffCommandRequest diffCommandRequest = new DiffCommandRequest(); - diffCommandRequest.setRevision("3f76a12f08a6ba0dc988c68b7f0b2cd190efc3c4"); - - DiffResult diffResult = gitDiffResultCommand.getDiffResult(diffCommandRequest); + DiffResult diffResult = createDiffResult("3f76a12f08a6ba0dc988c68b7f0b2cd190efc3c4"); Iterator iterator = diffResult.iterator(); DiffFile a = iterator.next(); assertThat(a.getNewPath()).isEqualTo("a.txt"); @@ -39,4 +32,58 @@ public class GitDiffResultCommandTest extends AbstractGitCommandTestBase { assertThat(b.getOldPath()).isEqualTo("b.txt"); assertThat(b.getNewPath()).isEqualTo("/dev/null"); } + + @Test + public void shouldReturnFileRevisions() throws IOException { + DiffResult diffResult = createDiffResult("3f76a12f08a6ba0dc988c68b7f0b2cd190efc3c4"); + Iterator iterator = diffResult.iterator(); + + DiffFile a = iterator.next(); + assertThat(a.getOldRevision()).isEqualTo("78981922613b2afb6025042ff6bd878ac1994e85"); + assertThat(a.getNewRevision()).isEqualTo("1dc60c7504f4326bc83b9b628c384ec8d7e57096"); + + DiffFile b = iterator.next(); + assertThat(b.getOldRevision()).isEqualTo("61780798228d17af2d34fce4cfbdf35556832472"); + assertThat(b.getNewRevision()).isEqualTo("0000000000000000000000000000000000000000"); + } + + @Test + public void shouldReturnFileHunks() throws IOException { + DiffResult diffResult = createDiffResult("3f76a12f08a6ba0dc988c68b7f0b2cd190efc3c4"); + Iterator iterator = diffResult.iterator(); + + DiffFile a = iterator.next(); + Iterator hunks = a.iterator(); + + Hunk hunk = hunks.next(); + assertThat(hunk.getOldStart()).isEqualTo(1); + assertThat(hunk.getOldLineCount()).isEqualTo(1); + + assertThat(hunk.getNewStart()).isEqualTo(1); + assertThat(hunk.getNewLineCount()).isEqualTo(1); + } + + @Test + public void shouldReturnFileHunksWithFullFileRange() throws IOException { + DiffResult diffResult = createDiffResult("fcd0ef1831e4002ac43ea539f4094334c79ea9ec"); + Iterator iterator = diffResult.iterator(); + + DiffFile a = iterator.next(); + Iterator hunks = a.iterator(); + + Hunk hunk = hunks.next(); + assertThat(hunk.getOldStart()).isEqualTo(1); + assertThat(hunk.getOldLineCount()).isEqualTo(1); + + assertThat(hunk.getNewStart()).isEqualTo(1); + assertThat(hunk.getNewLineCount()).isEqualTo(2); + } + + private DiffResult createDiffResult(String s) throws IOException { + GitDiffResultCommand gitDiffResultCommand = new GitDiffResultCommand(createContext(), repository); + DiffCommandRequest diffCommandRequest = new DiffCommandRequest(); + diffCommandRequest.setRevision(s); + + return gitDiffResultCommand.getDiffResult(diffCommandRequest); + } } diff --git a/scm-plugins/scm-git-plugin/src/test/java/sonia/scm/repository/spi/GitHunkParserTest.java b/scm-plugins/scm-git-plugin/src/test/java/sonia/scm/repository/spi/GitHunkParserTest.java new file mode 100644 index 0000000000..a58fae644d --- /dev/null +++ b/scm-plugins/scm-git-plugin/src/test/java/sonia/scm/repository/spi/GitHunkParserTest.java @@ -0,0 +1,138 @@ +package sonia.scm.repository.spi; + +import org.junit.jupiter.api.Test; +import sonia.scm.repository.api.DiffLine; +import sonia.scm.repository.api.Hunk; + +import java.util.Iterator; +import java.util.List; + +import static org.assertj.core.api.Assertions.assertThat; +import static org.junit.jupiter.api.Assertions.assertThrows; + +class GitHunkParserTest { + + private static final String DIFF_001 = "diff --git a/a.txt b/a.txt\n" + + "index 7898192..2f8bc28 100644\n" + + "--- a/a.txt\n" + + "+++ b/a.txt\n" + + "@@ -1 +1,2 @@\n" + + " a\n" + + "+added line\n"; + + private static final String DIFF_002 = "diff --git a/file b/file\n" + + "index 5e89957..e8823e1 100644\n" + + "--- a/file\n" + + "+++ b/file\n" + + "@@ -2,6 +2,9 @@\n" + + " 2\n" + + " 3\n" + + " 4\n" + + "+5\n" + + "+6\n" + + "+7\n" + + " 8\n" + + " 9\n" + + " 10\n" + + "@@ -15,14 +18,13 @@\n" + + " 18\n" + + " 19\n" + + " 20\n" + + "+21\n" + + "+22\n" + + " 23\n" + + " 24\n" + + " 25\n" + + " 26\n" + + " 27\n" + + "-a\n" + + "-b\n" + + "-c\n" + + " 28\n" + + " 29\n" + + " 30"; + + private static final String DIFF_003 = "diff --git a/a.txt b/a.txt\n" + + "index 7898192..2f8bc28 100644\n" + + "--- a/a.txt\n" + + "+++ b/a.txt\n" + + "@@ -1,2 +1 @@\n" + + " a\n" + + "-removed line\n"; + + private static final String ILLEGAL_DIFF = "diff --git a/a.txt b/a.txt\n" + + "index 7898192..2f8bc28 100644\n" + + "--- a/a.txt\n" + + "+++ b/a.txt\n" + + "@@ -1,2 +1 @@\n" + + " a\n" + + "~illegal line\n"; + + @Test + void shouldParseHunks() { + List hunks = new GitHunkParser().parse(DIFF_001); + assertThat(hunks).hasSize(1); + assertHunk(hunks.get(0), 1, 1, 1, 2); + } + + @Test + void shouldParseMultipleHunks() { + List hunks = new GitHunkParser().parse(DIFF_002); + + assertThat(hunks).hasSize(2); + assertHunk(hunks.get(0), 2, 6, 2, 9); + assertHunk(hunks.get(1), 15, 14, 18, 13); + } + + @Test + void shouldParseAddedHunkLines() { + List hunks = new GitHunkParser().parse(DIFF_001); + + Hunk hunk = hunks.get(0); + + Iterator lines = hunk.iterator(); + + DiffLine line1 = lines.next(); + assertThat(line1.getOldLineNumber()).hasValue(1); + assertThat(line1.getNewLineNumber()).hasValue(1); + assertThat(line1.getContent()).isEqualTo("a"); + + DiffLine line2 = lines.next(); + assertThat(line2.getOldLineNumber()).isEmpty(); + assertThat(line2.getNewLineNumber()).hasValue(2); + assertThat(line2.getContent()).isEqualTo("added line"); + } + + @Test + void shouldParseRemovedHunkLines() { + List hunks = new GitHunkParser().parse(DIFF_003); + + Hunk hunk = hunks.get(0); + + Iterator lines = hunk.iterator(); + + DiffLine line1 = lines.next(); + assertThat(line1.getOldLineNumber()).hasValue(1); + assertThat(line1.getNewLineNumber()).hasValue(1); + assertThat(line1.getContent()).isEqualTo("a"); + + DiffLine line2 = lines.next(); + assertThat(line2.getOldLineNumber()).hasValue(2); + assertThat(line2.getNewLineNumber()).isEmpty(); + assertThat(line2.getContent()).isEqualTo("removed line"); + } + + @Test + void shouldFailForIllegalLine() { + assertThrows(IllegalStateException.class, () -> new GitHunkParser().parse(ILLEGAL_DIFF)); + } + + private void assertHunk(Hunk hunk, int oldStart, int oldLineCount, int newStart, int newLineCount) { + assertThat(hunk.getOldStart()).isEqualTo(oldStart); + assertThat(hunk.getOldLineCount()).isEqualTo(oldLineCount); + + assertThat(hunk.getNewStart()).isEqualTo(newStart); + assertThat(hunk.getNewLineCount()).isEqualTo(newLineCount); + } + +} From e3787fd764f92cd3ee9d104870d83d3b241e9cfd Mon Sep 17 00:00:00 2001 From: Sebastian Sdorra Date: Tue, 30 Jul 2019 08:06:10 +0200 Subject: [PATCH 4/9] simplify Differ api and use the new api in GitDiffCommand --- .../java/sonia/scm/repository/spi/Differ.java | 13 ++-- .../scm/repository/spi/GitDiffCommand.java | 72 ++----------------- .../repository/spi/GitDiffResultCommand.java | 15 ++-- .../scm/repository/spi/GitHunkParser.java | 3 +- 4 files changed, 20 insertions(+), 83 deletions(-) diff --git a/scm-plugins/scm-git-plugin/src/main/java/sonia/scm/repository/spi/Differ.java b/scm-plugins/scm-git-plugin/src/main/java/sonia/scm/repository/spi/Differ.java index 67d503aeff..ca417550f4 100644 --- a/scm-plugins/scm-git-plugin/src/main/java/sonia/scm/repository/spi/Differ.java +++ b/scm-plugins/scm-git-plugin/src/main/java/sonia/scm/repository/spi/Differ.java @@ -15,7 +15,6 @@ import sonia.scm.util.Util; import java.io.IOException; import java.util.List; -import java.util.function.Consumer; final class Differ implements AutoCloseable { @@ -29,7 +28,13 @@ final class Differ implements AutoCloseable { this.treeWalk = treeWalk; } - public static Differ create(Repository repository, DiffCommandRequest request) throws IOException { + static Diff diff(Repository repository, DiffCommandRequest request) throws IOException { + try (Differ differ = create(repository, request)) { + return differ.diff(); + } + } + + private static Differ create(Repository repository, DiffCommandRequest request) throws IOException { RevWalk walk = new RevWalk(repository); ObjectId revision = repository.resolve(request.getRevision()); @@ -81,9 +86,9 @@ final class Differ implements AutoCloseable { return GitUtil.computeCommonAncestor(repository, revision1, revision2); } - public void process(Consumer diffConsumer) throws IOException { + private Diff diff() throws IOException { List entries = DiffEntry.scan(treeWalk); - diffConsumer.accept(new Diff(commit, entries)); + return new Diff(commit, entries); } @Override diff --git a/scm-plugins/scm-git-plugin/src/main/java/sonia/scm/repository/spi/GitDiffCommand.java b/scm-plugins/scm-git-plugin/src/main/java/sonia/scm/repository/spi/GitDiffCommand.java index 2d56c8e786..7d5e45a5c2 100644 --- a/scm-plugins/scm-git-plugin/src/main/java/sonia/scm/repository/spi/GitDiffCommand.java +++ b/scm-plugins/scm-git-plugin/src/main/java/sonia/scm/repository/spi/GitDiffCommand.java @@ -34,26 +34,15 @@ package sonia.scm.repository.spi; //~--- non-JDK imports -------------------------------------------------------- -import com.google.common.base.Strings; import org.eclipse.jgit.diff.DiffEntry; import org.eclipse.jgit.diff.DiffFormatter; -import org.eclipse.jgit.lib.ObjectId; -import org.eclipse.jgit.revwalk.RevCommit; -import org.eclipse.jgit.revwalk.RevTree; -import org.eclipse.jgit.revwalk.RevWalk; -import org.eclipse.jgit.treewalk.EmptyTreeIterator; -import org.eclipse.jgit.treewalk.TreeWalk; -import org.eclipse.jgit.treewalk.filter.PathFilter; import org.slf4j.Logger; import org.slf4j.LoggerFactory; import sonia.scm.repository.GitUtil; import sonia.scm.repository.Repository; -import sonia.scm.util.Util; import java.io.BufferedOutputStream; -import java.io.IOException; import java.io.OutputStream; -import java.util.List; /** * @@ -78,7 +67,7 @@ public class GitDiffCommand extends AbstractGitCommand implements DiffCommand * @param context * @param repository */ - public GitDiffCommand(GitContext context, Repository repository) + GitDiffCommand(GitContext context, Repository repository) { super(context, repository); } @@ -95,63 +84,18 @@ public class GitDiffCommand extends AbstractGitCommand implements DiffCommand @Override public void getDiffResult(DiffCommandRequest request, OutputStream output) { - RevWalk walk = null; - TreeWalk treeWalk = null; DiffFormatter formatter = null; try { - org.eclipse.jgit.lib.Repository gr = open(); + org.eclipse.jgit.lib.Repository repository = open(); - walk = new RevWalk(gr); - - ObjectId revision = gr.resolve(request.getRevision()); - RevCommit commit = walk.parseCommit(revision); - - walk.markStart(commit); - commit = walk.next(); - treeWalk = new TreeWalk(gr); - treeWalk.reset(); - treeWalk.setRecursive(true); - - if (Util.isNotEmpty(request.getPath())) - { - treeWalk.setFilter(PathFilter.create(request.getPath())); - } - - - if (!Strings.isNullOrEmpty(request.getAncestorChangeset())) - { - ObjectId otherRevision = gr.resolve(request.getAncestorChangeset()); - ObjectId ancestorId = computeCommonAncestor(gr, revision, otherRevision); - RevTree tree = walk.parseCommit(ancestorId).getTree(); - treeWalk.addTree(tree); - } - else if (commit.getParentCount() > 0) - { - RevTree tree = commit.getParent(0).getTree(); - - if (tree != null) - { - treeWalk.addTree(tree); - } - else - { - treeWalk.addTree(new EmptyTreeIterator()); - } - } - else - { - treeWalk.addTree(new EmptyTreeIterator()); - } - - treeWalk.addTree(commit.getTree()); formatter = new DiffFormatter(new BufferedOutputStream(output)); - formatter.setRepository(gr); + formatter.setRepository(repository); - List entries = DiffEntry.scan(treeWalk); + Differ.Diff diff = Differ.diff(repository, request); - for (DiffEntry e : entries) + for (DiffEntry e : diff.getEntries()) { if (!e.getOldId().equals(e.getNewId())) { @@ -168,14 +112,8 @@ public class GitDiffCommand extends AbstractGitCommand implements DiffCommand } finally { - GitUtil.release(walk); - GitUtil.release(treeWalk); GitUtil.release(formatter); } } - private ObjectId computeCommonAncestor(org.eclipse.jgit.lib.Repository repository, ObjectId revision1, ObjectId revision2) throws IOException { - return GitUtil.computeCommonAncestor(repository, revision1, revision2); - } - } diff --git a/scm-plugins/scm-git-plugin/src/main/java/sonia/scm/repository/spi/GitDiffResultCommand.java b/scm-plugins/scm-git-plugin/src/main/java/sonia/scm/repository/spi/GitDiffResultCommand.java index de7c8483f5..0d5f4f7b9e 100644 --- a/scm-plugins/scm-git-plugin/src/main/java/sonia/scm/repository/spi/GitDiffResultCommand.java +++ b/scm-plugins/scm-git-plugin/src/main/java/sonia/scm/repository/spi/GitDiffResultCommand.java @@ -22,23 +22,16 @@ public class GitDiffResultCommand extends AbstractGitCommand implements DiffResu public DiffResult getDiffResult(DiffCommandRequest diffCommandRequest) throws IOException { org.eclipse.jgit.lib.Repository repository = open(); - try (Differ differ = Differ.create(repository, diffCommandRequest)) { - GitDiffResult result = new GitDiffResult(repository); - differ.process(result::process); - return result; - } + return new GitDiffResult(repository, Differ.diff(repository, diffCommandRequest)); } private class GitDiffResult implements DiffResult { private final org.eclipse.jgit.lib.Repository repository; - private Differ.Diff diff; + private final Differ.Diff diff; - private GitDiffResult(org.eclipse.jgit.lib.Repository repository) { + private GitDiffResult(org.eclipse.jgit.lib.Repository repository, Differ.Diff diff) { this.repository = repository; - } - - void process(Differ.Diff diff) { this.diff = diff; } @@ -96,7 +89,7 @@ public class GitDiffResultCommand extends AbstractGitCommand implements DiffResu } private String format(org.eclipse.jgit.lib.Repository repository, DiffEntry entry) { - try(ByteArrayOutputStream baos = new ByteArrayOutputStream()) { + try (ByteArrayOutputStream baos = new ByteArrayOutputStream()) { DiffFormatter formatter = new DiffFormatter(baos); formatter.setRepository(repository); formatter.format(entry); diff --git a/scm-plugins/scm-git-plugin/src/main/java/sonia/scm/repository/spi/GitHunkParser.java b/scm-plugins/scm-git-plugin/src/main/java/sonia/scm/repository/spi/GitHunkParser.java index caedc6605f..b7594bb5d6 100644 --- a/scm-plugins/scm-git-plugin/src/main/java/sonia/scm/repository/spi/GitHunkParser.java +++ b/scm-plugins/scm-git-plugin/src/main/java/sonia/scm/repository/spi/GitHunkParser.java @@ -51,6 +51,7 @@ final class GitHunkParser { FileRange oldFileRange = createFileRange(split[0]); // TODO merge contains two two block which starts with "-" e.g. -1,3 -2,4 +3,6 + // check if it is relevant for our use case FileRange newFileRange = createFileRange(split[1]); currentGitHunk = new GitHunk(oldFileRange, newFileRange); @@ -137,7 +138,7 @@ final class GitHunkParser { private final int oldLineNumber; private final String content; - public UnchangedGitDiffLine(int newLineNumber, int oldLineNumber, String content) { + private UnchangedGitDiffLine(int newLineNumber, int oldLineNumber, String content) { this.newLineNumber = newLineNumber; this.oldLineNumber = oldLineNumber; this.content = content; From 0b76cb7ea5b5af07544b76d37f2df4d9e84f2547 Mon Sep 17 00:00:00 2001 From: Sebastian Sdorra Date: Tue, 30 Jul 2019 10:00:36 +0200 Subject: [PATCH 5/9] added raw header to hunk --- .../java/sonia/scm/repository/api/Hunk.java | 12 +++++ .../sonia/scm/repository/api/HunkTest.java | 53 +++++++++++++++++++ 2 files changed, 65 insertions(+) create mode 100644 scm-core/src/test/java/sonia/scm/repository/api/HunkTest.java diff --git a/scm-core/src/main/java/sonia/scm/repository/api/Hunk.java b/scm-core/src/main/java/sonia/scm/repository/api/Hunk.java index 6e60f8b2bd..c8a3e1ebca 100644 --- a/scm-core/src/main/java/sonia/scm/repository/api/Hunk.java +++ b/scm-core/src/main/java/sonia/scm/repository/api/Hunk.java @@ -2,6 +2,18 @@ package sonia.scm.repository.api; public interface Hunk extends Iterable { + default String getRawHeader() { + return String.format("@@ -%s +%s @@", getLineMarker(getOldStart(), getOldLineCount()), getLineMarker(getNewStart(), getNewLineCount())); + } + + default String getLineMarker(int start, int lineCount) { + if (lineCount == 1) { + return Integer.toString(start); + } else { + return String.format("%s,%s", start, lineCount); + } + } + int getOldStart(); int getOldLineCount(); diff --git a/scm-core/src/test/java/sonia/scm/repository/api/HunkTest.java b/scm-core/src/test/java/sonia/scm/repository/api/HunkTest.java new file mode 100644 index 0000000000..086df81741 --- /dev/null +++ b/scm-core/src/test/java/sonia/scm/repository/api/HunkTest.java @@ -0,0 +1,53 @@ +package sonia.scm.repository.api; + +import org.junit.jupiter.api.Test; + +import java.util.Iterator; + +import static org.assertj.core.api.Assertions.assertThat; + +class HunkTest { + + @Test + void shouldGetComplexHeader() { + String rawHeader = createHunk(2, 3, 4, 5).getRawHeader(); + + assertThat(rawHeader).isEqualTo("@@ -2,3 +4,5 @@"); + } + + @Test + void shouldReturnSingleNumberForOne() { + String rawHeader = createHunk(42, 1, 5, 1).getRawHeader(); + + assertThat(rawHeader).isEqualTo("@@ -42 +5 @@"); + } + + private Hunk createHunk(int oldStart, int oldLineCount, int newStart, int newLineCount) { + return new Hunk() { + @Override + public int getOldStart() { + return oldStart; + } + + @Override + public int getOldLineCount() { + return oldLineCount; + } + + @Override + public int getNewStart() { + return newStart; + } + + @Override + public int getNewLineCount() { + return newLineCount; + } + + @Override + public Iterator iterator() { + return null; + } + }; + } +} From bbdc5a198908507d577a515103a95a2d8a002535 Mon Sep 17 00:00:00 2001 From: Sebastian Sdorra Date: Tue, 30 Jul 2019 10:01:00 +0200 Subject: [PATCH 6/9] fix Diff ui types --- .../packages/ui-components/src/repos/Diff.js | 4 ++-- .../packages/ui-components/src/repos/DiffTypes.js | 13 +++++++++---- .../packages/ui-components/src/repos/LoadingDiff.js | 7 ++++--- .../packages/ui-components/src/repos/index.js | 1 + 4 files changed, 16 insertions(+), 9 deletions(-) diff --git a/scm-ui-components/packages/ui-components/src/repos/Diff.js b/scm-ui-components/packages/ui-components/src/repos/Diff.js index 369e812344..97692210c2 100644 --- a/scm-ui-components/packages/ui-components/src/repos/Diff.js +++ b/scm-ui-components/packages/ui-components/src/repos/Diff.js @@ -1,10 +1,10 @@ //@flow import React from "react"; import DiffFile from "./DiffFile"; -import type { DiffObjectProps } from "./DiffTypes"; +import type { DiffObjectProps, File } from "./DiffTypes"; type Props = DiffObjectProps & { - diff: any + diff: File[] }; class Diff extends React.Component { diff --git a/scm-ui-components/packages/ui-components/src/repos/DiffTypes.js b/scm-ui-components/packages/ui-components/src/repos/DiffTypes.js index 74803e0a4e..dcd23bc9a8 100644 --- a/scm-ui-components/packages/ui-components/src/repos/DiffTypes.js +++ b/scm-ui-components/packages/ui-components/src/repos/DiffTypes.js @@ -27,12 +27,17 @@ export type Hunk = { content: string }; +export type ChangeType = "insert" | "delete" | "normal"; + export type Change = { content: string, - isNormal: boolean, - newLineNumber: number, - oldLineNumber: number, - type: string + isNormal?: boolean, + isInsert?: boolean, + isDelete?: boolean, + lineNumber?: number, + newLineNumber?: number, + oldLineNumber?: number, + type: ChangeType }; export type BaseContext = { diff --git a/scm-ui-components/packages/ui-components/src/repos/LoadingDiff.js b/scm-ui-components/packages/ui-components/src/repos/LoadingDiff.js index ce1a074b41..c8a5250756 100644 --- a/scm-ui-components/packages/ui-components/src/repos/LoadingDiff.js +++ b/scm-ui-components/packages/ui-components/src/repos/LoadingDiff.js @@ -6,14 +6,14 @@ import parser from "gitdiff-parser"; import Loading from "../Loading"; import Diff from "./Diff"; -import type {DiffObjectProps} from "./DiffTypes"; +import type {DiffObjectProps, File} from "./DiffTypes"; type Props = DiffObjectProps & { url: string }; type State = { - diff?: any, + diff?: File[], loading: boolean, error?: Error }; @@ -47,7 +47,8 @@ class LoadingDiff extends React.Component { .get(url) .then(response => response.text()) .then(parser.parse) - .then(diff => { + // $FlowFixMe + .then((diff: File[]) => { this.setState({ loading: false, diff: diff diff --git a/scm-ui-components/packages/ui-components/src/repos/index.js b/scm-ui-components/packages/ui-components/src/repos/index.js index 473bbb3efc..fd1448fbdd 100644 --- a/scm-ui-components/packages/ui-components/src/repos/index.js +++ b/scm-ui-components/packages/ui-components/src/repos/index.js @@ -12,6 +12,7 @@ export type { FileChangeType, Hunk, Change, + ChangeType, BaseContext, AnnotationFactory, AnnotationFactoryContext, From 00fa943e51f574597149be607a60a7bb55dbd01f Mon Sep 17 00:00:00 2001 From: Sebastian Sdorra Date: Wed, 31 Jul 2019 13:24:31 +0200 Subject: [PATCH 7/9] fix SonarQube issues --- .../scm/repository/spi/GitDiffCommand.java | 82 ++++--------------- .../repository/spi/GitDiffResultCommand.java | 12 ++- 2 files changed, 26 insertions(+), 68 deletions(-) diff --git a/scm-plugins/scm-git-plugin/src/main/java/sonia/scm/repository/spi/GitDiffCommand.java b/scm-plugins/scm-git-plugin/src/main/java/sonia/scm/repository/spi/GitDiffCommand.java index 7d5e45a5c2..5d5f27806b 100644 --- a/scm-plugins/scm-git-plugin/src/main/java/sonia/scm/repository/spi/GitDiffCommand.java +++ b/scm-plugins/scm-git-plugin/src/main/java/sonia/scm/repository/spi/GitDiffCommand.java @@ -1,19 +1,19 @@ /** * Copyright (c) 2010, Sebastian Sdorra * All rights reserved. - * + *

* Redistribution and use in source and binary forms, with or without * modification, are permitted provided that the following conditions are met: - * + *

* 1. Redistributions of source code must retain the above copyright notice, - * this list of conditions and the following disclaimer. + * this list of conditions and the following disclaimer. * 2. Redistributions in binary form must reproduce the above copyright notice, - * this list of conditions and the following disclaimer in the documentation - * and/or other materials provided with the distribution. + * this list of conditions and the following disclaimer in the documentation + * and/or other materials provided with the distribution. * 3. Neither the name of SCM-Manager; nor the names of its - * contributors may be used to endorse or promote products derived from this - * software without specific prior written permission. - * + * contributors may be used to endorse or promote products derived from this + * software without specific prior written permission. + *

* THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS "AS IS" * AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT LIMITED TO, THE * IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR A PARTICULAR PURPOSE ARE @@ -24,9 +24,8 @@ * ANY THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT * (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE OF THIS * SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE. - * + *

* http://bitbucket.org/sdorra/scm-manager - * */ @@ -36,84 +35,39 @@ package sonia.scm.repository.spi; import org.eclipse.jgit.diff.DiffEntry; import org.eclipse.jgit.diff.DiffFormatter; -import org.slf4j.Logger; -import org.slf4j.LoggerFactory; -import sonia.scm.repository.GitUtil; import sonia.scm.repository.Repository; import java.io.BufferedOutputStream; +import java.io.IOException; import java.io.OutputStream; /** * * @author Sebastian Sdorra */ -public class GitDiffCommand extends AbstractGitCommand implements DiffCommand -{ +public class GitDiffCommand extends AbstractGitCommand implements DiffCommand { - /** - * the logger for GitDiffCommand - */ - private static final Logger logger = - LoggerFactory.getLogger(GitDiffCommand.class); - - //~--- constructors --------------------------------------------------------- - - /** - * Constructs ... - * - * - * - * @param context - * @param repository - */ - GitDiffCommand(GitContext context, Repository repository) - { + GitDiffCommand(GitContext context, Repository repository) { super(context, repository); } - //~--- get methods ---------------------------------------------------------- - - /** - * Method description - * - * - * @param request - * @param output - */ @Override - public void getDiffResult(DiffCommandRequest request, OutputStream output) - { - DiffFormatter formatter = null; - - try - { - org.eclipse.jgit.lib.Repository repository = open(); - - formatter = new DiffFormatter(new BufferedOutputStream(output)); + public void getDiffResult(DiffCommandRequest request, OutputStream output) throws IOException { + @SuppressWarnings("squid:S2095") // repository will be closed with the RepositoryService + org.eclipse.jgit.lib.Repository repository = open(); + try (DiffFormatter formatter = new DiffFormatter(new BufferedOutputStream(output))) { formatter.setRepository(repository); Differ.Diff diff = Differ.diff(repository, request); - for (DiffEntry e : diff.getEntries()) - { - if (!e.getOldId().equals(e.getNewId())) - { + for (DiffEntry e : diff.getEntries()) { + if (!e.getOldId().equals(e.getNewId())) { formatter.format(e); } } formatter.flush(); } - catch (Exception ex) - { - // TODO throw exception - logger.error("could not create diff", ex); - } - finally - { - GitUtil.release(formatter); - } } } diff --git a/scm-plugins/scm-git-plugin/src/main/java/sonia/scm/repository/spi/GitDiffResultCommand.java b/scm-plugins/scm-git-plugin/src/main/java/sonia/scm/repository/spi/GitDiffResultCommand.java index 0d5f4f7b9e..a3f63b8f5a 100644 --- a/scm-plugins/scm-git-plugin/src/main/java/sonia/scm/repository/spi/GitDiffResultCommand.java +++ b/scm-plugins/scm-git-plugin/src/main/java/sonia/scm/repository/spi/GitDiffResultCommand.java @@ -4,6 +4,7 @@ import com.google.common.base.Throwables; import org.eclipse.jgit.diff.DiffEntry; import org.eclipse.jgit.diff.DiffFormatter; import sonia.scm.repository.GitUtil; +import sonia.scm.repository.InternalRepositoryException; import sonia.scm.repository.Repository; import sonia.scm.repository.api.DiffFile; import sonia.scm.repository.api.DiffResult; @@ -47,7 +48,11 @@ public class GitDiffResultCommand extends AbstractGitCommand implements DiffResu @Override public Iterator iterator() { - return diff.getEntries().stream().map(diffEntry -> new GitDiffFile(repository, diffEntry)).collect(Collectors.toList()).iterator(); + return diff.getEntries() + .stream() + .map(diffEntry -> new GitDiffFile(repository, diffEntry)) + .collect(Collectors.toList()) + .iterator(); } } @@ -89,13 +94,12 @@ public class GitDiffResultCommand extends AbstractGitCommand implements DiffResu } private String format(org.eclipse.jgit.lib.Repository repository, DiffEntry entry) { - try (ByteArrayOutputStream baos = new ByteArrayOutputStream()) { - DiffFormatter formatter = new DiffFormatter(baos); + try (ByteArrayOutputStream baos = new ByteArrayOutputStream(); DiffFormatter formatter = new DiffFormatter(baos)) { formatter.setRepository(repository); formatter.format(entry); return baos.toString(); } catch (IOException ex) { - throw Throwables.propagate(ex); + throw new InternalRepositoryException(GitDiffResultCommand.this.repository, "failed to format diff entry", ex); } } From f10b653a1d2318ae23979df436c3242e2c30f07b Mon Sep 17 00:00:00 2001 From: Sebastian Sdorra Date: Wed, 31 Jul 2019 13:37:12 +0200 Subject: [PATCH 8/9] Fix thrown exceptions --- .../sonia/scm/repository/spi/GitDiffCommandTest.java | 11 ++++++----- 1 file changed, 6 insertions(+), 5 deletions(-) diff --git a/scm-plugins/scm-git-plugin/src/test/java/sonia/scm/repository/spi/GitDiffCommandTest.java b/scm-plugins/scm-git-plugin/src/test/java/sonia/scm/repository/spi/GitDiffCommandTest.java index f6e462f968..fd9c45be5c 100644 --- a/scm-plugins/scm-git-plugin/src/test/java/sonia/scm/repository/spi/GitDiffCommandTest.java +++ b/scm-plugins/scm-git-plugin/src/test/java/sonia/scm/repository/spi/GitDiffCommandTest.java @@ -3,6 +3,7 @@ package sonia.scm.repository.spi; import org.junit.Test; import java.io.ByteArrayOutputStream; +import java.io.IOException; import static org.junit.Assert.assertEquals; @@ -38,7 +39,7 @@ public class GitDiffCommandTest extends AbstractGitCommandTestBase { "+f\n"; @Test - public void diffForOneRevisionShouldCreateDiff() { + public void diffForOneRevisionShouldCreateDiff() throws IOException { GitDiffCommand gitDiffCommand = new GitDiffCommand(createContext(), repository); DiffCommandRequest diffCommandRequest = new DiffCommandRequest(); diffCommandRequest.setRevision("3f76a12f08a6ba0dc988c68b7f0b2cd190efc3c4"); @@ -48,7 +49,7 @@ public class GitDiffCommandTest extends AbstractGitCommandTestBase { } @Test - public void diffForOneBranchShouldCreateDiff() { + public void diffForOneBranchShouldCreateDiff() throws IOException { GitDiffCommand gitDiffCommand = new GitDiffCommand(createContext(), repository); DiffCommandRequest diffCommandRequest = new DiffCommandRequest(); diffCommandRequest.setRevision("test-branch"); @@ -58,7 +59,7 @@ public class GitDiffCommandTest extends AbstractGitCommandTestBase { } @Test - public void diffForPathShouldCreateLimitedDiff() { + public void diffForPathShouldCreateLimitedDiff() throws IOException { GitDiffCommand gitDiffCommand = new GitDiffCommand(createContext(), repository); DiffCommandRequest diffCommandRequest = new DiffCommandRequest(); diffCommandRequest.setRevision("test-branch"); @@ -69,7 +70,7 @@ public class GitDiffCommandTest extends AbstractGitCommandTestBase { } @Test - public void diffBetweenTwoBranchesShouldCreateDiff() { + public void diffBetweenTwoBranchesShouldCreateDiff() throws IOException { GitDiffCommand gitDiffCommand = new GitDiffCommand(createContext(), repository); DiffCommandRequest diffCommandRequest = new DiffCommandRequest(); diffCommandRequest.setRevision("master"); @@ -80,7 +81,7 @@ public class GitDiffCommandTest extends AbstractGitCommandTestBase { } @Test - public void diffBetweenTwoBranchesForPathShouldCreateLimitedDiff() { + public void diffBetweenTwoBranchesForPathShouldCreateLimitedDiff() throws IOException { GitDiffCommand gitDiffCommand = new GitDiffCommand(createContext(), repository); DiffCommandRequest diffCommandRequest = new DiffCommandRequest(); diffCommandRequest.setRevision("master"); From 6caf01f4f6db232c45eda4ae30854cd4cba77460 Mon Sep 17 00:00:00 2001 From: Sebastian Sdorra Date: Wed, 31 Jul 2019 17:14:34 +0200 Subject: [PATCH 9/9] close branch feature/parsed_diff