From e1344e158737d649eee744cbe13bbfb99f20a573 Mon Sep 17 00:00:00 2001 From: Marquis Wong Date: Tue, 15 Oct 2019 17:08:58 -0500 Subject: [PATCH] Add "local" connector type Runs analysis tools on the HEAD of the local git repository and reports to the log. Allows someone to check for any comments before pushing to Gerrit/Gitlab. --- build.gradle | 1 + .../sputnik/configuration/GeneralOption.java | 2 +- .../connector/ConnectorFacadeFactory.java | 4 + .../touk/sputnik/connector/ConnectorType.java | 3 +- .../pl/touk/sputnik/connector/Connectors.java | 2 +- .../sputnik/connector/local/LocalFacade.java | 102 ++++++++++++++ .../connector/local/LocalFacadeBuilder.java | 21 +++ .../connector/local/LocalFacadeOutput.java | 16 +++ .../pl/touk/sputnik/review/ReviewFile.java | 1 - .../connector/local/LocalFacadeTest.java | 128 ++++++++++++++++++ 10 files changed, 276 insertions(+), 4 deletions(-) create mode 100644 src/main/java/pl/touk/sputnik/connector/local/LocalFacade.java create mode 100644 src/main/java/pl/touk/sputnik/connector/local/LocalFacadeBuilder.java create mode 100644 src/main/java/pl/touk/sputnik/connector/local/LocalFacadeOutput.java create mode 100644 src/test/java/pl/touk/sputnik/connector/local/LocalFacadeTest.java diff --git a/build.gradle b/build.gradle index c6b54092..2b60a70d 100644 --- a/build.gradle +++ b/build.gradle @@ -72,6 +72,7 @@ dependencies { compile 'commons-cli:commons-cli:1.2' compile 'com.jayway.jsonpath:json-path:2.4.0' compile 'commons-io:commons-io:2.4' + compile 'org.eclipse.jgit:org.eclipse.jgit:5.5.1.201910021850-r' compile 'com.urswolfer.gerrit.client.rest:gerrit-rest-java-client:0.8.8' diff --git a/src/main/java/pl/touk/sputnik/configuration/GeneralOption.java b/src/main/java/pl/touk/sputnik/configuration/GeneralOption.java index d5567d0e..6ea7318f 100644 --- a/src/main/java/pl/touk/sputnik/configuration/GeneralOption.java +++ b/src/main/java/pl/touk/sputnik/configuration/GeneralOption.java @@ -19,7 +19,7 @@ public enum GeneralOption implements ConfigurationOption { MESSAGE_PROBLEM_FORMAT("message.problemFormat", "Sputnik problem format. {0}: reporter, {1}: message", "There is a problem with {0}: {1}"), MESSAGE_SCORE_PASSING_COMMENT("message.scorePassingComment", "Comment when no errors are found", "Perfect!"), - CONNECTOR_TYPE("connector.type", "Connector: ", ConnectorType.GERRIT.getName()), + CONNECTOR_TYPE("connector.type", "Connector: ", ConnectorType.GERRIT.getName()), HOST("connector.host", "Connector server host", "localhost"), PORT("connector.port", "Connector server port", "80"), PATH("connector.path", "Connector server path", ""), diff --git a/src/main/java/pl/touk/sputnik/connector/ConnectorFacadeFactory.java b/src/main/java/pl/touk/sputnik/connector/ConnectorFacadeFactory.java index 68317246..e108b5ab 100644 --- a/src/main/java/pl/touk/sputnik/connector/ConnectorFacadeFactory.java +++ b/src/main/java/pl/touk/sputnik/connector/ConnectorFacadeFactory.java @@ -5,6 +5,7 @@ import pl.touk.sputnik.configuration.GeneralOptionNotSupportedException; import pl.touk.sputnik.connector.gerrit.GerritFacadeBuilder; import pl.touk.sputnik.connector.github.GithubFacadeBuilder; +import pl.touk.sputnik.connector.local.LocalFacadeBuilder; import pl.touk.sputnik.connector.saas.SaasFacadeBuilder; import pl.touk.sputnik.connector.stash.StashFacadeBuilder; @@ -15,6 +16,7 @@ public class ConnectorFacadeFactory { StashFacadeBuilder stashFacadeBuilder = new StashFacadeBuilder(); GithubFacadeBuilder githubFacadeBuilder = new GithubFacadeBuilder(); SaasFacadeBuilder saasFacadeBuilder = new SaasFacadeBuilder(); + LocalFacadeBuilder localFacadeBuilder = new LocalFacadeBuilder(); @NotNull public ConnectorFacade build(@NotNull ConnectorType type, Configuration configuration) { @@ -27,6 +29,8 @@ public ConnectorFacade build(@NotNull ConnectorType type, Configuration configur return githubFacadeBuilder.build(configuration); case SAAS: return saasFacadeBuilder.build(configuration); + case LOCAL: + return localFacadeBuilder.build(); default: throw new GeneralOptionNotSupportedException("Connector " + type.getName() + " is not supported"); } diff --git a/src/main/java/pl/touk/sputnik/connector/ConnectorType.java b/src/main/java/pl/touk/sputnik/connector/ConnectorType.java index 933b2cc0..bf7858c0 100644 --- a/src/main/java/pl/touk/sputnik/connector/ConnectorType.java +++ b/src/main/java/pl/touk/sputnik/connector/ConnectorType.java @@ -13,7 +13,8 @@ public enum ConnectorType { GERRIT("gerrit"), STASH("stash"), GITHUB("github"), - SAAS("saas"); + SAAS("saas"), + LOCAL("local"); /** Name used in configuration file. */ @Getter diff --git a/src/main/java/pl/touk/sputnik/connector/Connectors.java b/src/main/java/pl/touk/sputnik/connector/Connectors.java index 14883537..fa48a246 100644 --- a/src/main/java/pl/touk/sputnik/connector/Connectors.java +++ b/src/main/java/pl/touk/sputnik/connector/Connectors.java @@ -1,5 +1,5 @@ package pl.touk.sputnik.connector; public enum Connectors { - STASH, GERRIT, GITHUB, SAAS + STASH, GERRIT, GITHUB, SAAS, LOCAL } diff --git a/src/main/java/pl/touk/sputnik/connector/local/LocalFacade.java b/src/main/java/pl/touk/sputnik/connector/local/LocalFacade.java new file mode 100644 index 00000000..e9bbe3a6 --- /dev/null +++ b/src/main/java/pl/touk/sputnik/connector/local/LocalFacade.java @@ -0,0 +1,102 @@ +package pl.touk.sputnik.connector.local; + +import lombok.AllArgsConstructor; +import lombok.extern.slf4j.Slf4j; +import org.eclipse.jgit.diff.DiffEntry; +import org.eclipse.jgit.diff.DiffEntry.ChangeType; +import org.eclipse.jgit.diff.DiffFormatter; +import org.eclipse.jgit.lib.Constants; +import org.eclipse.jgit.lib.ObjectId; +import org.eclipse.jgit.lib.Repository; +import org.jetbrains.annotations.NotNull; +import pl.touk.sputnik.configuration.Configuration; +import pl.touk.sputnik.configuration.GeneralOption; +import pl.touk.sputnik.configuration.GeneralOptionNotSupportedException; +import pl.touk.sputnik.connector.ConnectorFacade; +import pl.touk.sputnik.connector.ConnectorValidator; +import pl.touk.sputnik.connector.Connectors; +import pl.touk.sputnik.connector.ReviewPublisher; +import pl.touk.sputnik.review.Comment; +import pl.touk.sputnik.review.Review; +import pl.touk.sputnik.review.ReviewFile; + +import java.io.IOException; +import java.util.List; + +import static java.util.stream.Collectors.toList; + +@AllArgsConstructor +@Slf4j +public class LocalFacade implements ConnectorFacade, ConnectorValidator, ReviewPublisher { + private final Repository repository; + private final DiffFormatter diffFormatter; + private final LocalFacadeOutput output; + + @NotNull + @Override + public List listFiles() { + try { + ObjectId head = repository.resolve(Constants.HEAD); + ObjectId headParent = repository.resolve(Constants.HEAD + "^"); + List diffs = diffFormatter.scan(headParent, head); + return diffs.stream() + .filter(this::isNotDeleted) + .map(DiffEntry::getNewPath) + .map(ReviewFile::new) + .collect(toList()); + } catch (IOException e) { + throw new RuntimeException("Error when listing files", e); + } + } + + private boolean isNotDeleted(DiffEntry aDiffEntry) { + return aDiffEntry.getChangeType() != ChangeType.DELETE; + } + + @Override + public void publish(@NotNull Review review) { + long numFilesWithComments = review.getFiles().stream().filter(files -> !files.getComments().isEmpty()).count(); + if (numFilesWithComments == 0) { + output.info("No sputnik comments"); + return; + } + + for (String message : review.getMessages()) { + output.warn(message); + } + + for (ReviewFile file : review.getFiles()) { + if (file.getComments().isEmpty()) { + continue; + } + + output.warn(""); + + output.warn("{} comment(s) on {}", file.getComments().size(), file.getReviewFilename()); + for (Comment comment : file.getComments()) { + output.warn("Line {}: {}", comment.getLine(), comment.getMessage()); + } + } + } + + @Override + public Connectors name() { + return Connectors.LOCAL; + } + + @Override + public void validate(Configuration configuration) throws GeneralOptionNotSupportedException { + boolean commentOnlyChangedLines = Boolean.parseBoolean(configuration + .getProperty(GeneralOption.COMMENT_ONLY_CHANGED_LINES)); + + if (commentOnlyChangedLines) { + throw new GeneralOptionNotSupportedException("This connector does not support " + + GeneralOption.COMMENT_ONLY_CHANGED_LINES.getKey()); + } + } + + @Override + public void setReview(@NotNull Review review) { + publish(review); + } +} diff --git a/src/main/java/pl/touk/sputnik/connector/local/LocalFacadeBuilder.java b/src/main/java/pl/touk/sputnik/connector/local/LocalFacadeBuilder.java new file mode 100644 index 00000000..f71187c1 --- /dev/null +++ b/src/main/java/pl/touk/sputnik/connector/local/LocalFacadeBuilder.java @@ -0,0 +1,21 @@ +package pl.touk.sputnik.connector.local; + +import org.eclipse.jgit.diff.DiffFormatter; +import org.eclipse.jgit.lib.Repository; +import org.eclipse.jgit.storage.file.FileRepositoryBuilder; +import org.eclipse.jgit.util.io.NullOutputStream; + +import java.io.IOException; + +public class LocalFacadeBuilder { + public LocalFacade build() { + try (Repository repository = new FileRepositoryBuilder().readEnvironment().findGitDir().build()) { + try (DiffFormatter diffFormatter = new DiffFormatter(NullOutputStream.INSTANCE)) { + diffFormatter.setRepository(repository); + return new LocalFacade(repository, diffFormatter, new LocalFacadeOutput()); + } + } catch (IOException e) { + throw new RuntimeException("Error getting git repository", e); + } + } +} diff --git a/src/main/java/pl/touk/sputnik/connector/local/LocalFacadeOutput.java b/src/main/java/pl/touk/sputnik/connector/local/LocalFacadeOutput.java new file mode 100644 index 00000000..5d0654e7 --- /dev/null +++ b/src/main/java/pl/touk/sputnik/connector/local/LocalFacadeOutput.java @@ -0,0 +1,16 @@ +package pl.touk.sputnik.connector.local; + +import lombok.AllArgsConstructor; +import lombok.extern.slf4j.Slf4j; + +@AllArgsConstructor +@Slf4j +public class LocalFacadeOutput { + void info(String message, Object ... arguments) { + log.info(message, arguments); + } + + void warn(String message, Object ... arguments) { + log.warn(message, arguments); + } +} diff --git a/src/main/java/pl/touk/sputnik/review/ReviewFile.java b/src/main/java/pl/touk/sputnik/review/ReviewFile.java index 7ea95eb1..ffd5824e 100644 --- a/src/main/java/pl/touk/sputnik/review/ReviewFile.java +++ b/src/main/java/pl/touk/sputnik/review/ReviewFile.java @@ -39,5 +39,4 @@ public String getSourceDir() { private String createJavaClassName() { return StringUtils.substringBeforeLast(ENTRY_PATTERN.matcher(reviewFilename).replaceFirst(""), DOT).replace('/', '.'); } - } diff --git a/src/test/java/pl/touk/sputnik/connector/local/LocalFacadeTest.java b/src/test/java/pl/touk/sputnik/connector/local/LocalFacadeTest.java new file mode 100644 index 00000000..7c0af374 --- /dev/null +++ b/src/test/java/pl/touk/sputnik/connector/local/LocalFacadeTest.java @@ -0,0 +1,128 @@ +package pl.touk.sputnik.connector.local; + +import com.google.common.collect.ImmutableList; +import org.eclipse.jgit.diff.DiffEntry; +import org.eclipse.jgit.diff.DiffEntry.ChangeType; +import org.eclipse.jgit.diff.DiffFormatter; +import org.eclipse.jgit.lib.Constants; +import org.eclipse.jgit.lib.ObjectId; +import org.eclipse.jgit.lib.Repository; +import org.junit.jupiter.api.BeforeEach; +import org.junit.jupiter.api.Test; +import org.junit.jupiter.api.extension.ExtendWith; +import org.mockito.InOrder; +import org.mockito.Mock; +import org.mockito.Mockito; +import org.mockito.junit.jupiter.MockitoExtension; +import pl.touk.sputnik.review.Comment; +import pl.touk.sputnik.review.Review; +import pl.touk.sputnik.review.ReviewFile; + +import java.io.IOException; +import java.util.List; + +import static org.assertj.core.api.Assertions.assertThat; +import static org.mockito.Answers.RETURNS_DEEP_STUBS; +import static org.mockito.ArgumentMatchers.anyString; +import static org.mockito.Mockito.mock; +import static org.mockito.Mockito.never; +import static org.mockito.Mockito.verify; +import static org.mockito.Mockito.when; + +@ExtendWith(MockitoExtension.class) +class LocalFacadeTest { + @Mock(answer = RETURNS_DEEP_STUBS) + private Repository repo; + @Mock + private DiffFormatter diffFormatter; + @Mock + private DiffEntry modifiedFile; + @Mock + private DiffEntry deletedFile; + @Mock + private DiffEntry newFile; + @Mock + private Review review; + @Mock + private LocalFacadeOutput output; + + private LocalFacade localFacade; + + @BeforeEach + void setUp() { + localFacade = new LocalFacade(repo, diffFormatter, output); + } + + private void setUpDiff() throws IOException { + when(modifiedFile.getNewPath()).thenReturn("/path/to/modifiedFile"); + when(modifiedFile.getChangeType()).thenReturn(ChangeType.MODIFY); + + when(newFile.getNewPath()).thenReturn("/path/to/newFile"); + when(newFile.getChangeType()).thenReturn(ChangeType.ADD); + + when(deletedFile.getChangeType()).thenReturn(ChangeType.DELETE); + + ObjectId head = mock(ObjectId.class); + ObjectId headParent = mock(ObjectId.class); + when(repo.resolve(Constants.HEAD)).thenReturn(head); + when(repo.resolve(Constants.HEAD + "^")).thenReturn(headParent); + when(diffFormatter.scan(headParent, head)).thenReturn(ImmutableList.of(modifiedFile, deletedFile, newFile)); + } + + @Test + void shouldParseListFilesResponse() throws IOException { + setUpDiff(); + + List reviewFiles = localFacade.listFiles(); + assertThat(reviewFiles).isNotEmpty(); + } + + @Test + void shouldNotListDeletedFiles() throws IOException { + setUpDiff(); + + List reviewFiles = localFacade.listFiles(); + assertThat(reviewFiles) + .hasSize(2) + .extracting(ReviewFile::getReviewFilename).containsExactly(modifiedFile.getNewPath(), newFile.getNewPath()); + } + + @Test + void shouldPublishNoCommentsIfAllFilesHaveNoComments() { + ReviewFile review1 = mock(ReviewFile.class); + ReviewFile review2 = mock(ReviewFile.class); + when(review.getFiles()).thenReturn(ImmutableList.of(review1, review2)); + + localFacade.publish(review); + + verify(output).info("No sputnik comments"); + verify(output, never()).warn(anyString()); + } + + @Test + void shouldWarnWithCommentsAndLineNumbers() { + ReviewFile review1 = mock(ReviewFile.class); + when(review1.getReviewFilename()).thenReturn("/path/to/file1"); + ReviewFile review2 = mock(ReviewFile.class); + ReviewFile review3 = mock(ReviewFile.class); + when(review3.getReviewFilename()).thenReturn("/path/to/file3"); + when(review1.getComments()).thenReturn(ImmutableList.of(new Comment(11, "Comment 1"), new Comment(14, "Comment 2"))); + when(review3.getComments()).thenReturn(ImmutableList.of(new Comment(15, "Comment 3"))); + when(review.getMessages()).thenReturn(ImmutableList.of("message 1", "message 2")); + when(review.getFiles()).thenReturn(ImmutableList.of(review1, review2, review3)); + + localFacade.publish(review); + + verify(output, never()).info(anyString()); + InOrder order = Mockito.inOrder(output); + order.verify(output).warn("message 1"); + order.verify(output).warn("message 2"); + order.verify(output).warn(""); + order.verify(output).warn("{} comment(s) on {}", 2, "/path/to/file1"); + order.verify(output).warn("Line {}: {}", 11, "Comment 1"); + order.verify(output).warn("Line {}: {}", 14, "Comment 2"); + order.verify(output).warn(""); + order.verify(output).warn("{} comment(s) on {}", 1, "/path/to/file3"); + order.verify(output).warn("Line {}: {}", 15, "Comment 3"); + } +} \ No newline at end of file