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); + } + +}