From 08df11362780c1ca7d0dc734ffd613aae060ae1f Mon Sep 17 00:00:00 2001 From: Marquis Wang Date: Mon, 18 Nov 2019 14:29:07 -0600 Subject: [PATCH] Add "local" connector type (#206) Add local connector type. Runs analysis tools on the HEAD of the local git repository and reports to the log. Fixes #195 --- 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