From 4b9da6b7f494c348ab8eb81d71731b4a123dcc3a 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 | 5 + .../touk/sputnik/connector/ConnectorType.java | 3 +- .../pl/touk/sputnik/connector/Connectors.java | 2 +- .../sputnik/connector/local/LocalFacade.java | 108 ++++++++++++++++ .../connector/local/LocalFacadeBuilder.java | 19 +++ .../pl/touk/sputnik/review/ReviewFile.java | 1 - .../connector/local/LocalFacadeTest.java | 118 ++++++++++++++++++ 9 files changed, 255 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/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 21da6bab..c1250897 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..6133faab 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,9 @@ 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..82965f7a --- /dev/null +++ b/src/main/java/pl/touk/sputnik/connector/local/LocalFacade.java @@ -0,0 +1,108 @@ +package pl.touk.sputnik.connector.local; + +import com.google.common.annotations.VisibleForTesting; +import lombok.extern.slf4j.Slf4j; +import org.eclipse.jgit.api.Git; +import org.eclipse.jgit.api.errors.GitAPIException; +import org.eclipse.jgit.diff.DiffEntry; +import org.eclipse.jgit.diff.DiffEntry.ChangeType; +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.util.List; + +import static java.util.stream.Collectors.toList; + +@Slf4j +public class LocalFacade implements ConnectorFacade, ConnectorValidator, ReviewPublisher { + private final Git git; + + public LocalFacade(Git git) { + this.git = git; + } + + @NotNull + @Override + public List listFiles() { + try { + List diffs = git.diff().call(); + return diffs.stream() + .filter(this::isNotDeleted) + .map(DiffEntry::getNewPath) + .map(ReviewFile::new) + .collect(toList()); + } catch (GitAPIException 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) { + info("No sputnik comments"); + return; + } + + for (String message : review.getMessages()) { + warn(message); + } + + for (ReviewFile file : review.getFiles()) { + if (file.getComments().isEmpty()) { + continue; + } + + warn(""); + + warn(String.format("%d comment(s) on %s", file.getComments().size(), file.getReviewFilename())); + for (Comment comment : file.getComments()) { + warn(String.format("Line %d: %s", comment.getLine(), comment.getMessage())); + } + } + } + + @VisibleForTesting + void info(String message) { + log.info(message); + } + + @VisibleForTesting + void warn(String message) { + log.warn(message); + } + + @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..da41d63b --- /dev/null +++ b/src/main/java/pl/touk/sputnik/connector/local/LocalFacadeBuilder.java @@ -0,0 +1,19 @@ +package pl.touk.sputnik.connector.local; + +import org.eclipse.jgit.api.Git; +import org.eclipse.jgit.lib.Repository; +import org.eclipse.jgit.storage.file.FileRepositoryBuilder; + +import java.io.IOException; + +public class LocalFacadeBuilder { + public LocalFacade build() { + try (Repository repository = new FileRepositoryBuilder().readEnvironment().findGitDir().build()) { + try (Git git = new Git(repository)) { + return new LocalFacade(git); + } + } catch (IOException e) { + throw new RuntimeException("Error getting git repository", e); + } + } +} 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..8a306192 --- /dev/null +++ b/src/test/java/pl/touk/sputnik/connector/local/LocalFacadeTest.java @@ -0,0 +1,118 @@ +package pl.touk.sputnik.connector.local; + +import com.google.common.collect.ImmutableList; +import org.eclipse.jgit.api.Git; +import org.eclipse.jgit.api.errors.GitAPIException; +import org.eclipse.jgit.diff.DiffEntry; +import org.eclipse.jgit.diff.DiffEntry.ChangeType; +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.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.never; +import static org.mockito.Mockito.spy; +import static org.mockito.Mockito.verify; +import static org.mockito.Mockito.when; + +@ExtendWith(MockitoExtension.class) +class LocalFacadeTest { + @Mock(answer = RETURNS_DEEP_STUBS) + private Git git; + @Mock + private DiffEntry modifiedFile; + @Mock + private DiffEntry deletedFile; + @Mock + private DiffEntry newFile; + @Mock + private Review review; + + private LocalFacade localFacade; + + @BeforeEach + void setUp() { + localFacade = spy(new LocalFacade(git)); + } + + private void setUpDiff() throws GitAPIException { + 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); + + when(git.diff().call()).thenReturn(ImmutableList.of(modifiedFile, deletedFile, newFile)); + } + + @Test + void shouldParseListFilesResponse() throws GitAPIException { + setUpDiff(); + + List reviewFiles = localFacade.listFiles(); + assertThat(reviewFiles).isNotEmpty(); + } + + @Test + void shouldNotListDeletedFiles() throws GitAPIException { + setUpDiff(); + + List reviewFiles = localFacade.listFiles(); + assertThat(reviewFiles) + .hasSize(2) + .extracting(ReviewFile::getReviewFilename).containsExactly(modifiedFile.getNewPath(), newFile.getNewPath()); + } + + @Test + void publishLogsNoCommentsIfAllFilesHaveNoComments() { + ReviewFile review1 = new ReviewFile("/path/to/file1"); + ReviewFile review2 = new ReviewFile("path/to/file2"); + + when(review.getFiles()).thenReturn(ImmutableList.of(review1, review2)); + localFacade.publish(review); + + verify(localFacade).info("No sputnik comments"); + verify(localFacade, never()).warn(anyString()); + } + + @Test + void publishWarnsWithCommentsAndLineNumbers() { + ReviewFile review1 = new ReviewFile("/path/to/file1"); + ReviewFile review2 = new ReviewFile("/path/to/file2"); + ReviewFile review3 = new ReviewFile("/path/to/file3"); + + review1.getComments().add(new Comment(11, "Comment 1")); + review1.getComments().add(new Comment(14, "Comment 2")); + review3.getComments().add(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(localFacade, never()).info(anyString()); + InOrder order = Mockito.inOrder(localFacade); + order.verify(localFacade).warn("message 1"); + order.verify(localFacade).warn("message 2"); + order.verify(localFacade).warn(""); + order.verify(localFacade).warn("2 comment(s) on /path/to/file1"); + order.verify(localFacade).warn("Line 11: Comment 1"); + order.verify(localFacade).warn("Line 14: Comment 2"); + order.verify(localFacade).warn(""); + order.verify(localFacade).warn("1 comment(s) on /path/to/file3"); + order.verify(localFacade).warn("Line 15: Comment 3"); + } +} \ No newline at end of file