Merge pull request #469 from FundRequest/feature-395-improve-github-s…

…olver-resolver Feature 395 improve GitHub solver resolver
FundRequest · Jun 28, 2018 · 852e3f1 · 852e3f1
2 parents 3a2a172 + 3289cec
commit 852e3f1
Show file tree

Hide file tree

Showing 10 changed files with 456 additions and 83 deletions.
diff --git a/github/pom.xml b/github/pom.xml
@@ -79,6 +79,21 @@
             <artifactId>spring-boot-starter-test</artifactId>
             <scope>test</scope>
         </dependency>
+        <dependency>
+            <groupId>org.junit.platform</groupId>
+            <artifactId>junit-platform-launcher</artifactId>
+            <scope>test</scope>
+        </dependency>
+        <dependency>
+            <groupId>org.junit.jupiter</groupId>
+            <artifactId>junit-jupiter-engine</artifactId>
+            <scope>test</scope>
+        </dependency>
+        <dependency>
+            <groupId>org.junit.jupiter</groupId>
+            <artifactId>junit-jupiter-params</artifactId>
+            <scope>test</scope>
+        </dependency>
     </dependencies>
 
 </project>
diff --git a/github/src/main/java/io/fundrequest/platform/github/parser/GithubResult.java b/github/src/main/java/io/fundrequest/platform/github/parser/GithubResult.java
@@ -15,7 +15,8 @@ public class GithubResult {
     private String number;
     private String title;
     private String state;
-    @JsonProperty("body_html")
     private String body;
+    @JsonProperty("body_html")
+    private String bodyHtml;
     private GithubUser user;
 }
diff --git a/github/src/main/java/io/fundrequest/platform/github/scraper/GithubScraper.java b/github/src/main/java/io/fundrequest/platform/github/scraper/GithubScraper.java
@@ -1,6 +1,7 @@
 package io.fundrequest.platform.github.scraper;
 
 import io.fundrequest.common.infrastructure.JsoupSpringWrapper;
+import io.fundrequest.platform.github.scraper.model.GithubId;
 import io.fundrequest.platform.github.scraper.model.GithubIssue;
 import org.jsoup.nodes.Document;
 import org.springframework.stereotype.Component;
@@ -29,7 +30,7 @@ public GithubIssue fetchGithubIssue(final String owner, final String repo, final
         }
         return GithubIssue.builder()
                           .number(number)
-                          .solver(solverResolver.resolve(document, owner, repo))
+                          .solver(solverResolver.resolve(document, GithubId.builder().owner(owner).repo(repo).number(number).build()).orElse(null))
                           .status(statusResolver.resolve(document))
                           .build();
     }

diff --git a/github/src/main/java/io/fundrequest/platform/github/scraper/GithubSolverResolver.java b/github/src/main/java/io/fundrequest/platform/github/scraper/GithubSolverResolver.java
@@ -2,54 +2,76 @@
 
 import io.fundrequest.platform.github.GithubGateway;
 import io.fundrequest.platform.github.parser.GithubResult;
+import io.fundrequest.platform.github.scraper.model.GithubId;
 import org.apache.commons.lang.StringUtils;
 import org.jsoup.nodes.Document;
 import org.jsoup.nodes.Element;
 import org.jsoup.select.Elements;
 import org.springframework.stereotype.Component;
 
+import java.util.Arrays;
+import java.util.List;
+import java.util.Optional;
+import java.util.regex.Pattern;
+
 @Component
 public class GithubSolverResolver {
 
+    private static final List<String> CLOSING_KEYWORDS = Arrays.asList("close", "closes", "closed", "fix", "fixes", "fixed", "resolve", "resolves", "resolved");
+    private static final String CLOSING_KEYWORD_ISSUE_MATCHER_REGEX = "(?:<span.+class=\"issue-keyword.*\".*>)?\\b%1$s\\b(?:</span>)?:?\\s*<a.+class=\"issue-link js-issue-link\".*>(?:%2$s/%3$s)?#%4$s";
+
     private final GithubGateway githubGateway;
 
     public GithubSolverResolver(final GithubGateway githubGateway) {
         this.githubGateway = githubGateway;
     }
 
-    public String resolve(final Document document, final String owner, final String repo) {
+    public Optional<String> resolve(final Document document, final GithubId issueGithubId) {
         return document.select(".discussion-item")
                        .stream()
                        .filter(this::isPullRequest)
                        .filter(this::isMerged)
-                       .map(this::resolvePullRequestNumber)
-                       .map(pullRequestNumber -> fetchAuthorFromPullRequest(pullRequestNumber, owner, repo))
+                       .map(this::resolvePullRequestGithubId)
+                       .map(this::fetchPullrequest)
+                       .filter(pullRequest -> pullRequest != null && pullRequestFixesIssue(pullRequest, issueGithubId))
+                       .map(pullRequest -> pullRequest.getUser().getLogin())
                        .filter(StringUtils::isNotEmpty)
-                       .findFirst()
-                       .orElse(null);
+                       .findFirst();
     }
 
-    private String resolvePullRequestNumber(final Element discussionItem) {
-        final String pullRequestNumber;
+    private GithubId resolvePullRequestGithubId(final Element discussionItem) {
         if (isPullRequestInSingleDiscussionItem(discussionItem)) {
-            pullRequestNumber = getPullRequestNumberFromSingleDiscussionItem(discussionItem);
+            return getPullRequestGithubIdFromSingleDiscussionItem(discussionItem);
         } else {
-            pullRequestNumber = getPullRequestNumberFromInlineDiscussionItem(discussionItem);
+            return getPullRequestGithubIdFromInlineDiscussionItem(discussionItem);
         }
-        return pullRequestNumber.replace("#", "");
     }
 
-    private String getPullRequestNumberFromSingleDiscussionItem(final Element discussionItem) {
-        return discussionItem.select(".discussion-item [id^=ref-pullrequest-] ~ .discussion-item-ref-title span.issue-num").text();
+    private GithubId getPullRequestGithubIdFromSingleDiscussionItem(final Element discussionItem) {
+        return GithubId.fromString(discussionItem.select(".discussion-item [id^=ref-pullrequest-] ~ .discussion-item-ref-title a").attr("href"))
+                       .orElseThrow(() -> new RuntimeException("No pullrequest identifier is found"));
+    }
+
+    private GithubId getPullRequestGithubIdFromInlineDiscussionItem(final Element discussionItem) {
+        return GithubId.fromString(discussionItem.select(".discussion-item [id^=ref-pullrequest-] a").attr("href"))
+                       .orElseThrow(() -> new RuntimeException("No pullrequest identifier is found"));
     }
 
-    private String getPullRequestNumberFromInlineDiscussionItem(final Element discussionItem) {
-        return discussionItem.select(".discussion-item [id^=ref-pullrequest-] span.issue-num").text();
+    private GithubResult fetchPullrequest(GithubId pullRequestGithubId) {
+        return githubGateway.getPullrequest(pullRequestGithubId.getOwner(), pullRequestGithubId.getRepo(), pullRequestGithubId.getNumber());
     }
 
-    private String fetchAuthorFromPullRequest(final String pullRequestNumber, final String owner, final String repo) {
-        final GithubResult pullRequest = githubGateway.getPullrequest(owner, repo, pullRequestNumber);
-        return pullRequest.getUser().getLogin();
+    private boolean pullRequestFixesIssue(final GithubResult pullRequest, final GithubId issueGithubId) {
+        final String pullRequestBody = pullRequest.getBodyHtml();
+        return pullRequestBody != null && CLOSING_KEYWORDS.stream()
+                                                          .map(keyword -> String.format(CLOSING_KEYWORD_ISSUE_MATCHER_REGEX,
+                                                                                        keyword.toLowerCase(),
+                                                                                        issueGithubId.getOwner(),
+                                                                                        issueGithubId.getRepo(),
+                                                                                        issueGithubId.getNumber()))
+                                                          .anyMatch(regex -> Pattern.compile(regex)
+                                                                                    .matcher(pullRequestBody.toLowerCase())
+                                                                                    .find());
     }
 
     private boolean isPullRequestInSingleDiscussionItem(final Element discussionItem) {

diff --git a/github/src/main/java/io/fundrequest/platform/github/scraper/model/GithubId.java b/github/src/main/java/io/fundrequest/platform/github/scraper/model/GithubId.java
@@ -0,0 +1,32 @@
+package io.fundrequest.platform.github.scraper.model;
+
+import lombok.Builder;
+import lombok.EqualsAndHashCode;
+import lombok.Getter;
+
+import java.util.Optional;
+import java.util.regex.Matcher;
+import java.util.regex.Pattern;
+
+@Getter
+@Builder
+@EqualsAndHashCode
+public class GithubId {
+
+    private final String owner;
+    private final String repo;
+    private final String number;
+
+    public static Optional<GithubId> fromString(final String githubIdAsString) {
+        final Pattern pattern = Pattern.compile("^.*/(?<owner>.+)/(?<repo>.+)/.+/(?<number>\\d+)$");
+        final Matcher matcher = pattern.matcher(githubIdAsString);
+        if (matcher.matches()) {
+            return Optional.of(GithubId.builder()
+                                       .owner(matcher.group("owner"))
+                                       .repo(matcher.group("repo"))
+                                       .number(matcher.group("number"))
+                                       .build());
+        }
+        return Optional.empty();
+    }
+}
diff --git a/github/src/test/java/io/fundrequest/platform/github/scraper/DocumentMockBuilder.java b/github/src/test/java/io/fundrequest/platform/github/scraper/DocumentMockBuilder.java
@@ -1,12 +1,14 @@
 package io.fundrequest.platform.github.scraper;
 
+import io.fundrequest.platform.github.scraper.model.GithubId;
 import org.jsoup.nodes.Document;
 import org.jsoup.nodes.Element;
 import org.jsoup.select.Elements;
 
 import java.util.ArrayList;
 import java.util.List;
 
+import static org.mockito.Mockito.RETURNS_DEEP_STUBS;
 import static org.mockito.Mockito.mock;
 import static org.mockito.Mockito.when;
 
@@ -41,7 +43,7 @@ public static class DiscussionItemBuilder {
         private final Element element;
 
         private DiscussionItemBuilder() {
-            this.element = mock(Element.class);
+            this.element = mock(Element.class, RETURNS_DEEP_STUBS);
         }
 
         public static DiscussionItemBuilder builder() {
@@ -69,19 +71,17 @@ public DiscussionItemBuilder withAuthor(final String author) {
             return this;
         }
 
-        public DiscussionItemBuilder withIssueNum(final int issueNum, final boolean isInlinePullRequest) {
+        public DiscussionItemBuilder withPullrequestReference(final GithubId githubId, final boolean isInlinePullRequest) {
             final Elements pullRequestElements = mock(Elements.class);
-            final Elements pullRequestIssueNumElements = mock(Elements.class);
 
             when(pullRequestElements.isEmpty()).thenReturn(!isInlinePullRequest);
             when(element.select(".discussion-item .discussion-item-rollup-ref [id^=ref-pullrequest-]")).thenReturn(pullRequestElements);
 
+            final String githubIssueId = String.format("/%s/%s/pulls/%s", githubId.getOwner(), githubId.getRepo(), githubId.getNumber());
             if (isInlinePullRequest) {
-                when(pullRequestIssueNumElements.text()).thenReturn("#" + String.valueOf(issueNum));
-                when(element.select(".discussion-item [id^=ref-pullrequest-] span.issue-num")).thenReturn(pullRequestIssueNumElements);
+                when(element.select(".discussion-item [id^=ref-pullrequest-] a").attr("href")).thenReturn(githubIssueId);
             } else {
-                when(pullRequestIssueNumElements.text()).thenReturn("#" + String.valueOf(issueNum));
-                when(element.select(".discussion-item [id^=ref-pullrequest-] ~ .discussion-item-ref-title span.issue-num")).thenReturn(pullRequestIssueNumElements);
+                when(element.select(".discussion-item [id^=ref-pullrequest-] ~ .discussion-item-ref-title a").attr("href")).thenReturn(githubIssueId);
             }
             return this;
         }

diff --git a/...ub/src/test/java/io/fundrequest/platform/github/scraper/GithubScraperIntegrationTest.java b/...ub/src/test/java/io/fundrequest/platform/github/scraper/GithubScraperIntegrationTest.java
@@ -54,8 +54,7 @@ public void fetch_parsesCorrectSolver_whenReferencerInIssueIsNotOwnerPullRequest
 
         assertThat(githubIssue.getNumber()).isEqualTo("48");
         assertThat(githubIssue.getSolver()).isEqualTo("pauliax");
-        //TODO Uncomment when issue https://github.com/FundRequest/contracts/issues/48 is closed.
-        //        assertThat(githubIssue.getStatus()).isEqualTo("Closed");
+        assertThat(githubIssue.getStatus()).isEqualTo("Closed");
     }
 
     @Test
@@ -70,4 +69,56 @@ public void fetch_whenNoSolver() {
         assertThat(githubIssue.getSolver()).isNull();
         assertThat(githubIssue.getStatus()).isEqualTo("Closed");
     }
+
+    @Test
+    public void fetch_issueRandomlyReferencedInPullRequestFromSameRepo() {
+        final String owner = "aragon";
+        final String repo = "aragonOS";
+        final String number = "280";
+
+        final GithubIssue githubIssue = scraper.fetchGithubIssue(owner, repo, number);
+
+        assertThat(githubIssue.getNumber()).isEqualTo("280");
+        assertThat(githubIssue.getSolver()).isNull();
+        assertThat(githubIssue.getStatus()).isEqualTo("Open");
+    }
+
+    @Test
+    public void fetch_issueRandomlyReferencedInPullRequestFromOtherRepo() {
+        final String owner = "trufflesuite";
+        final String repo = "truffle";
+        final String number = "501";
+
+        final GithubIssue githubIssue = scraper.fetchGithubIssue(owner, repo, number);
+
+        assertThat(githubIssue.getNumber()).isEqualTo("501");
+        assertThat(githubIssue.getSolver()).isNull();
+        assertThat(githubIssue.getStatus()).isEqualTo("Open");
+    }
+
+    @Test
+    public void fetch_githubClosingKeywordsAreFound() {
+        final String owner = "poanetwork";
+        final String repo = "poa-explorer";
+        final String number = "237";
+
+        final GithubIssue githubIssue = scraper.fetchGithubIssue(owner, repo, number);
+
+        assertThat(githubIssue.getNumber()).isEqualTo("237");
+        assertThat(githubIssue.getSolver()).isEqualTo("katibest");
+        assertThat(githubIssue.getStatus()).isEqualTo("Closed");
+    }
+
+    @Test
+    public void fetch_issueFixedByPullRequestFromOtherRepo() {
+        final String owner = "brave";
+        final String repo = "brave-browser";
+        final String number = "240";
+
+        final GithubIssue githubIssue = scraper.fetchGithubIssue(owner, repo, number);
+
+        assertThat(githubIssue.getNumber()).isEqualTo("240");
+        assertThat(githubIssue.getSolver()).isEqualTo("cezaraugusto");
+        assertThat(githubIssue.getStatus()).isEqualTo("Closed");
+    }
 }
diff --git a/github/src/test/java/io/fundrequest/platform/github/scraper/GithubScraperTest.java b/github/src/test/java/io/fundrequest/platform/github/scraper/GithubScraperTest.java
@@ -2,12 +2,14 @@
 
 
 import io.fundrequest.common.infrastructure.JsoupSpringWrapper;
+import io.fundrequest.platform.github.scraper.model.GithubId;
 import io.fundrequest.platform.github.scraper.model.GithubIssue;
 import org.jsoup.nodes.Document;
 import org.junit.Before;
 import org.junit.Test;
 
 import java.io.IOException;
+import java.util.Optional;
 
 import static org.assertj.core.api.Assertions.assertThat;
 import static org.mockito.Mockito.RETURNS_DEEP_STUBS;
@@ -39,7 +41,7 @@ public void fetchGithubIssue() throws IOException {
         final Document document = mock(Document.class);
 
         when(jsoup.connect("https://github.com/" + owner + "/" + repo + "/issues/" + number).get()).thenReturn(document);
-        when(solverParser.resolve(document, owner, repo)).thenReturn(expectedSolver);
+        when(solverParser.resolve(document, GithubId.builder().owner(owner).repo(repo).number(number).build())).thenReturn(Optional.of(expectedSolver));
         when(statusParser.resolve(document)).thenReturn(expectedStatus);
 
         final GithubIssue returnedIssue = scraper.fetchGithubIssue(owner, repo, number);