Propagate issue properties when creating backports

Reviewed-by: ehelin

bots/notify/src/main/java/org/openjdk/skara/bots/notify/IssueUpdater.java

@@ -26,6 +26,7 @@
 import org.openjdk.skara.issuetracker.Issue;
 import org.openjdk.skara.issuetracker.*;
 import org.openjdk.skara.jcheck.JCheckConfiguration;
+import org.openjdk.skara.json.*;
 import org.openjdk.skara.vcs.*;
 import org.openjdk.skara.vcs.openjdk.*;
 
@@ -61,11 +62,11 @@ public class IssueUpdater implements RepositoryUpdateConsumer, PullRequestUpdate
 
     private boolean isPrimaryIssue(Issue issue) {
         var properties = issue.properties();
-        if (!properties.containsKey("type")) {
+        if (!properties.containsKey("issuetype")) {
             throw new RuntimeException("Unknown type for issue " + issue);
         }
-        var type = properties.get("type");
-        return primaryTypes.contains(type);
+        var type = properties.get("issuetype");
+        return primaryTypes.contains(type.asString());
     }
 
     private final static Pattern majorVersionPattern = Pattern.compile("([0-9]+)(u[0-9]+)?");
@@ -87,25 +88,34 @@ private List<Issue> findBackports(Issue primary) {
         return links.stream()
                     .filter(l -> l.issue().isPresent())
                     .map(l -> l.issue().get())
-                    .filter(i -> i.properties().containsKey("type"))
-                    .filter(i -> i.properties().get("type").equals("Backport"))
+                    .filter(i -> i.properties().containsKey("issuetype"))
+                    .filter(i -> i.properties().get("issuetype").asString().equals("Backport"))
                     .collect(Collectors.toList());
     }
 
     private boolean isNonScratchVersion(String version) {
         return !version.startsWith("tbd") && !version.toLowerCase().equals("unknown");
     }
 
+    private Set<String> fixVersions(Issue issue) {
+        if (!issue.properties().containsKey("fixVersions")) {
+            return Set.of();
+        }
+        return issue.properties().get("fixVersions").stream()
+                    .map(JSONValue::asString)
+                    .collect(Collectors.toSet());
+    }
+
     /**
      * Return true if the issue's fixVersionList matches fixVersion.
      *
      * fixVersionsList must contain one entry that is an exact match for fixVersions; any
      * other entries must be scratch values.
      */
     private boolean matchVersion(Issue issue, String fixVersion) {
-        var nonScratch = issue.fixVersions().stream()
-                              .filter(this::isNonScratchVersion)
-                              .collect(Collectors.toList());
+        var nonScratch = fixVersions(issue).stream()
+                                           .filter(this::isNonScratchVersion)
+                                           .collect(Collectors.toList());
         return nonScratch.size() == 1 && nonScratch.get(0).equals(fixVersion);
     }
 
@@ -123,30 +133,37 @@ private boolean matchPoolVersion(Issue issue, String fixVersion) {
         var poolVersion = majorVersion.get() + "-pool";
         var openVersion = majorVersion.get() + "-open";
 
-        var nonScratch = issue.fixVersions().stream()
-                              .filter(this::isNonScratchVersion)
-                              .collect(Collectors.toList());
+        var nonScratch = fixVersions(issue).stream()
+                                           .filter(this::isNonScratchVersion)
+                                           .collect(Collectors.toList());
         return nonScratch.size() == 1 && (nonScratch.get(0).equals(poolVersion) || nonScratch.get(0).equals(openVersion));
     }
 
     /**
      * Return true if fixVersionList is empty or contains only scratch values.
      */
     private boolean matchScratchVersion(Issue issue) {
-        var nonScratch = issue.fixVersions().stream()
-                              .filter(this::isNonScratchVersion)
-                              .collect(Collectors.toList());
+        var nonScratch = fixVersions(issue).stream()
+                                           .filter(this::isNonScratchVersion)
+                                           .collect(Collectors.toList());
         return nonScratch.size() == 0;
     }
 
+    private final static Set<String> propagatedCustomProperties =
+            Set.of("customfield_10008", "customfield_10000", "customfield_10005");
+
     /**
      * Create a backport of issue.
      */
     private Issue createBackportIssue(Issue primary) {
-        var properties = primary.properties();
-        properties.put("type", "Backport");
+        var filteredProperties = primary.properties().entrySet().stream()
+                .filter(entry -> !entry.getKey().startsWith("customfield_") || propagatedCustomProperties.contains(entry.getKey()))
+                .collect(Collectors.toMap(Map.Entry::getKey, Map.Entry::getValue));
+
+        var finalProperties = new HashMap<>(filteredProperties);
+        finalProperties.put("issuetype", JSON.of("Backport"));
 
-        var backport = primary.project().createIssue(primary.title(), primary.body().lines().collect(Collectors.toList()), properties);
+        var backport = primary.project().createIssue(primary.title(), primary.body().lines().collect(Collectors.toList()), finalProperties);
 
         var backportLink = Link.create(backport, "backported by").build();
         primary.addLink(backportLink);;
@@ -170,7 +187,7 @@ private Issue createBackportIssue(Issue primary) {
     private Issue findIssue(Issue primary, String fixVersion) {
         log.info("Searching for properly versioned issue for primary issue " + primary.id());
         var candidates = Stream.concat(Stream.of(primary), findBackports(primary).stream()).collect(Collectors.toList());
-        candidates.forEach(c -> log.fine("Candidate: " + c.id() + " with versions: " + String.join(",", c.fixVersions())));
+        candidates.forEach(c -> log.fine("Candidate: " + c.id() + " with versions: " + String.join(",", fixVersions(c))));
         var matchingVersionIssue = candidates.stream()
                 .filter(i -> matchVersion(i, fixVersion))
                 .findFirst();
@@ -264,11 +281,7 @@ public void handleCommits(HostedRepository repository, Repository localRepositor
 
                 if (setFixVersion) {
                     if (requestedVersion != null) {
-                        // Remove any previously set versions (can only be scratch versions)
-                        for (var oldVersion : issue.fixVersions()) {
-                            issue.removeFixVersion(oldVersion);
-                        }
-                        issue.addFixVersion(requestedVersion);
+                        issue.setProperty("fixVersions", JSON.of(requestedVersion));
                     }
                 }
             }

bots/notify/src/test/java/org/openjdk/skara/bots/notify/UpdaterTests.java

@@ -24,6 +24,7 @@
 
 import org.openjdk.skara.email.*;
 import org.openjdk.skara.forge.HostedRepository;
+import org.openjdk.skara.issuetracker.Issue;
 import org.openjdk.skara.json.*;
 import org.openjdk.skara.mailinglist.MailingListServerFactory;
 import org.openjdk.skara.storage.StorageBuilder;
@@ -66,6 +67,15 @@ private StorageBuilder<PullRequestIssues> createPullRequestIssuesStorage(HostedR
                 .remoteRepository(repository, "history", "Duke", "duke@openjdk.java.net", "Updated prissues");
     }
 
+    private Set<String> fixVersions(Issue issue) {
+        if (!issue.properties().containsKey("fixVersions")) {
+            return Set.of();
+        }
+        return issue.properties().get("fixVersions").stream()
+                    .map(JSONValue::asString)
+                    .collect(Collectors.toSet());
+    }
+
     @Test
     void testJsonUpdaterBranch(TestInfo testInfo) throws IOException {
         try (var credentials = new HostCredentials(testInfo);
@@ -915,7 +925,7 @@ void testIssue(TestInfo testInfo) throws IOException {
             TestBotRunner.runPeriodicItems(notifyBot);
 
             // Create an issue and commit a fix
-            var issue = issueProject.createIssue("This is an issue", List.of("Indeed"), Map.of("type", "Enhancement"));
+            var issue = issueProject.createIssue("This is an issue", List.of("Indeed"), Map.of("issuetype", JSON.of("Enhancement")));
             var editHash = CheckableRepository.appendAndCommit(localRepo, "Another line", issue.id() + ": Fix that issue");
             localRepo.push(editHash, repo.url(), "master");
             TestBotRunner.runPeriodicItems(notifyBot);
@@ -935,10 +945,7 @@ void testIssue(TestInfo testInfo) throws IOException {
             assertEquals(repo.webUrl(editHash), link.uri().orElseThrow());
 
             // As well as a fixVersion
-            var fixVersions = issue.fixVersions();
-            assertEquals(1, fixVersions.size());
-            var fixVersion = fixVersions.get(0);
-            assertEquals("0.1", fixVersion);
+            assertEquals(Set.of("0.1"), fixVersions(issue));
         }
     }
 
@@ -967,7 +974,7 @@ void testIssueNoVersion(TestInfo testInfo) throws IOException {
             TestBotRunner.runPeriodicItems(notifyBot);
 
             // Create an issue and commit a fix
-            var issue = issueProject.createIssue("This is an issue", List.of("Indeed"), Map.of("type", "Enhancement"));
+            var issue = issueProject.createIssue("This is an issue", List.of("Indeed"), Map.of("issuetype", JSON.of("Enhancement")));
             var editHash = CheckableRepository.appendAndCommit(localRepo, "Another line", issue.id() + ": Fix that issue");
             localRepo.push(editHash, repo.url(), "master");
             TestBotRunner.runPeriodicItems(notifyBot);
@@ -979,8 +986,7 @@ void testIssueNoVersion(TestInfo testInfo) throws IOException {
             assertTrue(comment.body().contains(editHash.abbreviate()));
 
             // But not in the fixVersion
-            var fixVersions = issue.fixVersions();
-            assertEquals(0, fixVersions.size());
+            assertEquals(Set.of(), fixVersions(issue));
         }
     }
 
@@ -1009,7 +1015,7 @@ void testIssueConfiguredVersionNoCommit(TestInfo testInfo) throws IOException {
             TestBotRunner.runPeriodicItems(notifyBot);
 
             // Create an issue and commit a fix
-            var issue = issueProject.createIssue("This is an issue", List.of("Indeed"), Map.of("type", "Enhancement"));
+            var issue = issueProject.createIssue("This is an issue", List.of("Indeed"), Map.of("issuetype", JSON.of("Enhancement")));
             var editHash = CheckableRepository.appendAndCommit(localRepo, "Another line", issue.id() + ": Fix that issue");
             localRepo.push(editHash, repo.url(), "master");
             TestBotRunner.runPeriodicItems(notifyBot);
@@ -1021,10 +1027,7 @@ void testIssueConfiguredVersionNoCommit(TestInfo testInfo) throws IOException {
             assertTrue(comment.body().contains(editHash.abbreviate()));
 
             // As well as a fixVersion - but not the one from the repo
-            var fixVersions = issue.fixVersions();
-            assertEquals(1, fixVersions.size());
-            var fixVersion = fixVersions.get(0);
-            assertEquals("2.0", fixVersion);
+            assertEquals(Set.of("2.0"), fixVersions(issue));
 
             // And no commit link
             var links = issue.links();
@@ -1060,7 +1063,7 @@ void testIssueIdempotence(TestInfo testInfo) throws IOException {
             var historyState = localRepo.fetch(repo.url(), "history");
 
             // Create an issue and commit a fix
-            var issue = issueProject.createIssue("This is an issue", List.of("Indeed"), Map.of("type", "Enhancement"));
+            var issue = issueProject.createIssue("This is an issue", List.of("Indeed"), Map.of("issuetype", JSON.of("Enhancement")));
             var editHash = CheckableRepository.appendAndCommit(localRepo, "Another line", issue.id() + ": Fix that issue");
             localRepo.push(editHash, repo.url(), "master");
             TestBotRunner.runPeriodicItems(notifyBot);
@@ -1080,10 +1083,7 @@ void testIssueIdempotence(TestInfo testInfo) throws IOException {
             assertEquals(repo.webUrl(editHash), link.uri().orElseThrow());
 
             // As well as a fixVersion
-            var fixVersions = issue.fixVersions();
-            assertEquals(1, fixVersions.size());
-            var fixVersion = fixVersions.get(0);
-            assertEquals("0.1", fixVersion);
+            assertEquals(Set.of("0.1"), fixVersions(issue));
 
             // Wipe the history
             localRepo.push(historyState, repo.url(), "history", true);
@@ -1095,7 +1095,7 @@ void testIssueIdempotence(TestInfo testInfo) throws IOException {
             var updatedIssue = issueProject.issue(issue.id()).orElseThrow();
             assertEquals(1, updatedIssue.comments().size());
             assertEquals(1, updatedIssue.links().size());
-            assertEquals(1, updatedIssue.fixVersions().size());
+            assertEquals(Set.of("0.1"), fixVersions(updatedIssue));
         }
     }
 
@@ -1123,19 +1123,15 @@ void testIssuePoolVersion(TestInfo testInfo) throws IOException {
             TestBotRunner.runPeriodicItems(notifyBot);
 
             // Create an issue and commit a fix
-            var issue = issueProject.createIssue("This is an issue", List.of("Indeed"), Map.of("type", "Enhancement"));
-            issue.addFixVersion("12-pool");
-            issue.addFixVersion("tbd13");
-            issue.addFixVersion("unknown");
+            var issue = issueProject.createIssue("This is an issue", List.of("Indeed"), Map.of("issuetype", JSON.of("Enhancement")));
+            issue.setProperty("fixVersions", JSON.array().add("12-pool").add("tbd13").add("unknown"));
 
             var editHash = CheckableRepository.appendAndCommit(localRepo, "Another line", issue.id() + ": Fix that issue");
             localRepo.push(editHash, repo.url(), "master");
             TestBotRunner.runPeriodicItems(notifyBot);
 
             // The fixVersion should have been updated
-            var fixVersions = issue.fixVersions();
-            assertEquals(1, fixVersions.size());
-            assertEquals("12u14", fixVersions.get(0));
+            assertEquals(Set.of("12u14"), fixVersions(issue));
         }
     }
 
@@ -1163,19 +1159,15 @@ void testIssuePoolOpenVersion(TestInfo testInfo) throws IOException {
             TestBotRunner.runPeriodicItems(notifyBot);
 
             // Create an issue and commit a fix
-            var issue = issueProject.createIssue("This is an issue", List.of("Indeed"), Map.of("type", "Enhancement"));
-            issue.addFixVersion("12-open");
-            issue.addFixVersion("tbd13");
-            issue.addFixVersion("unknown");
+            var issue = issueProject.createIssue("This is an issue", List.of("Indeed"), Map.of("issuetype", JSON.of("Enhancement")));
+            issue.setProperty("fixVersions", JSON.array().add("12-pool").add("tbd13").add("unknown"));
 
             var editHash = CheckableRepository.appendAndCommit(localRepo, "Another line", issue.id() + ": Fix that issue");
             localRepo.push(editHash, repo.url(), "master");
             TestBotRunner.runPeriodicItems(notifyBot);
 
             // The fixVersion should have been updated
-            var fixVersions = issue.fixVersions();
-            assertEquals(1, fixVersions.size());
-            assertEquals("12u14", fixVersions.get(0));
+            assertEquals(Set.of("12u14"), fixVersions(issue));
         }
     }
 
@@ -1203,18 +1195,17 @@ void testIssueBackport(TestInfo testInfo) throws IOException {
             TestBotRunner.runPeriodicItems(notifyBot);
 
             // Create an issue and commit a fix
-            var issue = issueProject.createIssue("This is an issue", List.of("Indeed"), Map.of("type", "Enhancement"));
-            issue.addFixVersion("13.0.1");
+            var issue = issueProject.createIssue("This is an issue", List.of("Indeed"), Map.of("issuetype", JSON.of("Enhancement")));
+            issue.setProperty("fixVersions", JSON.array().add("13.0.1"));
+            issue.setProperty("priority", JSON.of("1"));
 
             var editHash = CheckableRepository.appendAndCommit(localRepo, "Another line", issue.id() + ": Fix that issue");
             localRepo.push(editHash, repo.url(), "master");
             TestBotRunner.runPeriodicItems(notifyBot);
 
             // The fixVersion should not have been updated
             var updatedIssue = issueProject.issue(issue.id()).orElseThrow();
-            var fixVersions = updatedIssue.fixVersions();
-            assertEquals(1, fixVersions.size());
-            assertEquals("13.0.1", fixVersions.get(0));
+            assertEquals(Set.of("13.0.1"), fixVersions(updatedIssue));
 
             // There should be a link
             var links = updatedIssue.links();
@@ -1223,10 +1214,10 @@ void testIssueBackport(TestInfo testInfo) throws IOException {
             var backport = link.issue().orElseThrow();
 
             // The backport issue should have a correct fixVersion
-            var backportFixVersions = backport.fixVersions();
-            assertEquals(1, backportFixVersions.size());
-            assertEquals("12.0.2", backportFixVersions.get(0));
-            assertEquals("Backport", backport.properties().get("type"));
+            assertEquals(Set.of("12.0.2"), fixVersions(backport));
+
+            // Custom properties should also propagate
+            assertEquals("1", backport.properties().get("priority").asString());
         }
     }
 
@@ -1257,7 +1248,7 @@ void testPullRequest(TestInfo testInfo) throws IOException {
             TestBotRunner.runPeriodicItems(notifyBot);
 
             // Create an issue and a pull request to fix it
-            var issue = issueProject.createIssue("This is an issue", List.of("Indeed"), Map.of("type", "Enhancement"));
+            var issue = issueProject.createIssue("This is an issue", List.of("Indeed"), Map.of("issuetype", JSON.of("Enhancement")));
             var editHash = CheckableRepository.appendAndCommit(localRepo, "Another line", "Fix that issue");
             localRepo.push(editHash, repo.url(), "edit", true);
             var pr = credentials.createPullRequest(repo, "edit", "master", issue.id() + ": Fix that issue");
@@ -1293,7 +1284,7 @@ void testPullRequest(TestInfo testInfo) throws IOException {
             assertEquals(reviewIcon, links.get(0).iconUrl().orElseThrow());
 
             // Add another issue
-            var issue2 = issueProject.createIssue("This is another issue", List.of("Yes indeed"), Map.of("type", "Enhancement"));
+            var issue2 = issueProject.createIssue("This is another issue", List.of("Yes indeed"), Map.of("issuetype", JSON.of("Enhancement")));
             pr.setBody("\n\n## Issues\n[" + issue.id() + "](http://www.test.test/): The issue\n[" + issue2.id() +
                                "](http://www.test2.test/): The second issue");
             TestBotRunner.runPeriodicItems(notifyBot);
@@ -1345,7 +1336,7 @@ void testPullRequestNoReview(TestInfo testInfo) throws IOException {
             TestBotRunner.runPeriodicItems(notifyBot);
 
             // Create an issue and a pull request to fix it
-            var issue = issueProject.createIssue("This is an issue", List.of("Indeed"), Map.of("type", "Enhancement"));
+            var issue = issueProject.createIssue("This is an issue", List.of("Indeed"), Map.of("issuetype", JSON.of("Enhancement")));
             var editHash = CheckableRepository.appendAndCommit(localRepo, "Another line", "Fix that issue");
             localRepo.push(editHash, repo.url(), "edit", true);
             var pr = credentials.createPullRequest(repo, "edit", "master", issue.id() + ": Fix that issue");