diff --git a/bots/notify/src/main/java/org/openjdk/skara/bots/notify/IssueUpdater.java b/bots/notify/src/main/java/org/openjdk/skara/bots/notify/IssueUpdater.java index 1fadc3672..0211d9b12 100644 --- a/bots/notify/src/main/java/org/openjdk/skara/bots/notify/IssueUpdater.java +++ b/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,8 +88,8 @@ private List 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()); } @@ -96,6 +97,15 @@ private boolean isNonScratchVersion(String version) { return !version.startsWith("tbd") && !version.toLowerCase().equals("unknown"); } + private Set 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. * @@ -103,9 +113,9 @@ private boolean isNonScratchVersion(String version) { * 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,9 +133,9 @@ 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)); } @@ -133,20 +143,27 @@ private boolean matchPoolVersion(Issue issue, String fixVersion) { * 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 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)); } } } diff --git a/bots/notify/src/test/java/org/openjdk/skara/bots/notify/UpdaterTests.java b/bots/notify/src/test/java/org/openjdk/skara/bots/notify/UpdaterTests.java index 87555e089..367c52ff7 100644 --- a/bots/notify/src/test/java/org/openjdk/skara/bots/notify/UpdaterTests.java +++ b/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 createPullRequestIssuesStorage(HostedR .remoteRepository(repository, "history", "Duke", "duke@openjdk.java.net", "Updated prissues"); } + private Set 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,8 +1195,9 @@ 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"); @@ -1212,9 +1205,7 @@ void testIssueBackport(TestInfo testInfo) throws IOException { // 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"); diff --git a/bots/tester/src/test/java/org/openjdk/skara/bots/tester/InMemoryPullRequest.java b/bots/tester/src/test/java/org/openjdk/skara/bots/tester/InMemoryPullRequest.java index 2c5d39f91..17c2a1be3 100644 --- a/bots/tester/src/test/java/org/openjdk/skara/bots/tester/InMemoryPullRequest.java +++ b/bots/tester/src/test/java/org/openjdk/skara/bots/tester/InMemoryPullRequest.java @@ -25,6 +25,7 @@ import org.openjdk.skara.forge.*; import org.openjdk.skara.host.*; import org.openjdk.skara.issuetracker.*; +import org.openjdk.skara.json.JSONValue; import org.openjdk.skara.vcs.*; import java.util.*; @@ -244,32 +245,17 @@ public void removeLink(Link link) { } @Override - public List fixVersions() { + public Map properties() { return null; } @Override - public void addFixVersion(String fixVersion) { + public void setProperty(String name, JSONValue value) { } @Override - public void removeFixVersion(String fixVersion) { - - } - - @Override - public Map properties() { - return null; - } - - @Override - public void setProperty(String name, String value) { - - } - - @Override - public void removePropery(String name) { + public void removeProperty(String name) { } diff --git a/forge/src/main/java/org/openjdk/skara/forge/github/GitHubPullRequest.java b/forge/src/main/java/org/openjdk/skara/forge/github/GitHubPullRequest.java index 5362f88fa..b0b59c8b2 100644 --- a/forge/src/main/java/org/openjdk/skara/forge/github/GitHubPullRequest.java +++ b/forge/src/main/java/org/openjdk/skara/forge/github/GitHubPullRequest.java @@ -524,32 +524,17 @@ public void removeLink(Link link) { } @Override - public List fixVersions() { + public Map properties() { throw new RuntimeException("not implemented yet"); } @Override - public void addFixVersion(String fixVersion) { + public void setProperty(String name, JSONValue value) { throw new RuntimeException("not implemented yet"); } @Override - public void removeFixVersion(String fixVersion) { - throw new RuntimeException("not implemented yet"); - } - - @Override - public Map properties() { - throw new RuntimeException("not implemented yet"); - } - - @Override - public void setProperty(String name, String value) { - throw new RuntimeException("not implemented yet"); - } - - @Override - public void removePropery(String name) { + public void removeProperty(String name) { throw new RuntimeException("not implemented yet"); } } diff --git a/forge/src/main/java/org/openjdk/skara/forge/gitlab/GitLabMergeRequest.java b/forge/src/main/java/org/openjdk/skara/forge/gitlab/GitLabMergeRequest.java index ee7d34c0f..4cae3dc1d 100644 --- a/forge/src/main/java/org/openjdk/skara/forge/gitlab/GitLabMergeRequest.java +++ b/forge/src/main/java/org/openjdk/skara/forge/gitlab/GitLabMergeRequest.java @@ -653,32 +653,17 @@ public void removeLink(Link link) { } @Override - public List fixVersions() { + public Map properties() { throw new RuntimeException("not implemented yet"); } @Override - public void addFixVersion(String fixVersion) { + public void setProperty(String name,JSONValue value) { throw new RuntimeException("not implemented yet"); } @Override - public void removeFixVersion(String fixVersion) { - throw new RuntimeException("not implemented yet"); - } - - @Override - public Map properties() { - throw new RuntimeException("not implemented yet"); - } - - @Override - public void setProperty(String name, String value) { - throw new RuntimeException("not implemented yet"); - } - - @Override - public void removePropery(String name) { + public void removeProperty(String name) { throw new RuntimeException("not implemented yet"); } } diff --git a/issuetracker/src/main/java/org/openjdk/skara/issuetracker/Issue.java b/issuetracker/src/main/java/org/openjdk/skara/issuetracker/Issue.java index b66f7c208..69832151f 100644 --- a/issuetracker/src/main/java/org/openjdk/skara/issuetracker/Issue.java +++ b/issuetracker/src/main/java/org/openjdk/skara/issuetracker/Issue.java @@ -23,6 +23,7 @@ package org.openjdk.skara.issuetracker; import org.openjdk.skara.host.HostUser; +import org.openjdk.skara.json.JSONValue; import java.net.URI; import java.time.ZonedDateTime; @@ -154,15 +155,9 @@ enum State { void removeLink(Link link); - List fixVersions(); + Map properties(); - void addFixVersion(String fixVersion); + void setProperty(String name, JSONValue value); - void removeFixVersion(String fixVersion); - - Map properties(); - - void setProperty(String name, String value); - - void removePropery(String name); + void removeProperty(String name); } diff --git a/issuetracker/src/main/java/org/openjdk/skara/issuetracker/IssueProject.java b/issuetracker/src/main/java/org/openjdk/skara/issuetracker/IssueProject.java index 4bcd89fdb..821a104f0 100644 --- a/issuetracker/src/main/java/org/openjdk/skara/issuetracker/IssueProject.java +++ b/issuetracker/src/main/java/org/openjdk/skara/issuetracker/IssueProject.java @@ -22,13 +22,15 @@ */ package org.openjdk.skara.issuetracker; +import org.openjdk.skara.json.JSONValue; + import java.net.URI; import java.util.*; public interface IssueProject { IssueTracker issueTracker(); URI webUrl(); - Issue createIssue(String title, List body, Map properties); + Issue createIssue(String title, List body, Map properties); Optional issue(String id); List issues(); } diff --git a/issuetracker/src/main/java/org/openjdk/skara/issuetracker/jira/JiraIssue.java b/issuetracker/src/main/java/org/openjdk/skara/issuetracker/jira/JiraIssue.java index 55e6deb42..1bdc8fe44 100644 --- a/issuetracker/src/main/java/org/openjdk/skara/issuetracker/jira/JiraIssue.java +++ b/issuetracker/src/main/java/org/openjdk/skara/issuetracker/jira/JiraIssue.java @@ -442,47 +442,30 @@ public void removeLink(Link link) { } @Override - public List fixVersions() { - return json.get("fields").get("fixVersions").stream() - .map(obj -> obj.get("id").asString()) - .map(id -> jiraProject.fixVersionNameFromId(id).orElseThrow()) - .collect(Collectors.toList()); - } - - @Override - public void addFixVersion(String fixVersion) { - var query = JSON.object() - .put("update", JSON.object() - .put("fixVersions", JSON.array().add(JSON.object() - .put("add", JSON.object() - .put("id", jiraProject.fixVersionIdFromName(fixVersion).orElseThrow()))))); - request.put("").body(query).execute(); - } - - @Override - public void removeFixVersion(String fixVersion) { - var query = JSON.object() - .put("update", JSON.object() - .put("fixVersions", JSON.array().add(JSON.object() - .put("remove", JSON.object() - .put("id", jiraProject.fixVersionIdFromName(fixVersion).orElseThrow()))))); - request.put("").body(query).execute(); - } + public Map properties() { + var ret = new HashMap(); - @Override - public Map properties() { - var ret = new HashMap(); - ret.put("type", json.get("fields").get("issuetype").get("name").asString()); + for (var field : json.get("fields").asObject().fields()) { + var value = field.value(); + var decoded = jiraProject.decodeProperty(field.name(), value); + decoded.ifPresent(jsonValue -> ret.put(field.name(), jsonValue)); + } return ret; } @Override - public void setProperty(String name, String value) { - + public void setProperty(String name, JSONValue value) { + var encoded = jiraProject.encodeProperty(name, value); + if (encoded.isEmpty()) { + log.warning("Ignoring unknown property: " + name); + return; + } + var query = JSON.object().put("fields", JSON.object().put(name, encoded.get())); + request.put("").body(query).execute(); } @Override - public void removePropery(String name) { + public void removeProperty(String name) { } } diff --git a/issuetracker/src/main/java/org/openjdk/skara/issuetracker/jira/JiraProject.java b/issuetracker/src/main/java/org/openjdk/skara/issuetracker/jira/JiraProject.java index 2eab5d2d8..f479500ff 100644 --- a/issuetracker/src/main/java/org/openjdk/skara/issuetracker/jira/JiraProject.java +++ b/issuetracker/src/main/java/org/openjdk/skara/issuetracker/jira/JiraProject.java @@ -37,8 +37,6 @@ public class JiraProject implements IssueProject { private final RestRequest request; private JSONObject projectMetadataCache = null; - private Map versionNameToId = null; - private Map versionIdToName = null; private List linkTypes = null; private final Logger log = Logger.getLogger("org.openjdk.skara.issuetracker.jira"); @@ -72,25 +70,12 @@ private Map components() { return ret; } - private void populateVersionsIfNeeded() { - if (versionIdToName != null) { - return; + private Map versions() { + var ret = new HashMap(); + for (var type : project().get("versions").asArray()) { + ret.put(type.get("name").asString(), type.get("id").asString()); } - - versionNameToId = project().get("versions").stream() - .collect(Collectors.toMap(v -> v.get("name").asString(), v -> v.get("id").asString())); - versionIdToName = project().get("versions").stream() - .collect(Collectors.toMap(v -> v.get("id").asString(), v -> v.get("name").asString())); - } - - Optional fixVersionNameFromId(String id) { - populateVersionsIfNeeded(); - return Optional.ofNullable(versionIdToName.getOrDefault(id, null)); - } - - Optional fixVersionIdFromName(String name) { - populateVersionsIfNeeded(); - return Optional.ofNullable(versionNameToId.getOrDefault(name, null)); + return ret; } private void populateLinkTypesIfNeeded() { @@ -134,6 +119,66 @@ JiraHost jiraHost() { return jiraHost; } + private static final Set knownProperties = Set.of("issuetype", "fixVersions", "versions", "priority", "components"); + + boolean isAllowedProperty(String name) { + if (knownProperties.contains(name)) { + return true; + } + return name.startsWith("customfield_"); + } + + Optional decodeProperty(String name, JSONValue value) { + if (!isAllowedProperty(name)) { + return Optional.empty(); + } + if (value.isNull()) { + return Optional.empty(); + } + + // Transform known fields to a better representation + switch (name) { + case "fixVersions": // fall-through + case "versions": // fall-through + case "components": + return Optional.of(new JSONArray(value.stream() + .map(obj -> obj.get("name")) + .collect(Collectors.toList()))); + case "issuetype": + return Optional.of(JSON.of(value.get("name").asString())); + case "priority": + return Optional.of(JSON.of(value.get("id").asString())); + default: + return Optional.of(value); + } + } + + Optional encodeProperty(String name, JSONValue value) { + if (!isAllowedProperty(name)) { + return Optional.empty(); + } + + switch (name) { + case "fixVersions": // fall-through + case "versions": + return Optional.of(new JSONArray(value.stream() + .map(JSONValue::asString) + .map(s -> JSON.object().put("id", versions().get(s))) + .collect(Collectors.toList()))); + case "components": + return Optional.of(new JSONArray(value.stream() + .map(JSONValue::asString) + .map(s -> JSON.object().put("id", components().get(s))) + .collect(Collectors.toList()))); + case "issuetype": + return Optional.of(JSON.object().put("id", issueTypes().get(value.asString()))); + case "priority": + return Optional.of(JSON.object().put("id", value.asString())); + default: + return Optional.of(value); + } + } + @Override public IssueTracker issueTracker() { return jiraHost; @@ -144,36 +189,45 @@ public URI webUrl() { return URIBuilder.base(jiraHost.getUri()).setPath("/projects/" + projectName).build(); } + private boolean isInitialField(String name, JSONValue value) { + if (name.equals("project") || name.equals("summary") || name.equals("description") || name.equals("components")) { + return true; + } + return false; + } + @Override - public Issue createIssue(String title, List body, Map properties) { + public Issue createIssue(String title, List body, Map properties) { var query = JSON.object(); - var fields = JSON.object() - .put("project", JSON.object() - .put("id", projectId())) - .put("components", JSON.array() - .add(JSON.object().put("id", defaultComponent()))) - .put("summary", title) - .put("description", String.join("\n", body)); - - var fixupFields = JSON.object(); + + var finalProperties = new HashMap(properties); + + // Always override certain fields + finalProperties.put("project", JSON.object().put("id", projectId())); + finalProperties.put("summary", JSON.of(title)); + finalProperties.put("description", JSON.of(String.join("\n", body))); + + // Encode optional properties as fields for (var property : properties.entrySet()) { - switch (property.getKey()) { - case "type": - if (!property.getValue().equals("Backport")) { - fields.put("issuetype", JSON.object().put("id", issueTypes().get(property.getValue()))); - } else { - fixupFields.put("issuetype", JSON.object().put("id", issueTypes().get(property.getValue()))); - } - break; - default: - log.warning("Unknown issue property: " + property.getKey()); - break; + var encoded = encodeProperty(property.getKey(), property.getValue()); + if (encoded.isEmpty()) { + continue; } + finalProperties.put(property.getKey(), encoded.get()); } - if (!fields.contains("issuetype")) { - fields.put("issuetype", JSON.object().put("id", defaultIssueType())); - } + // Provide default values for required fields if not present + finalProperties.putIfAbsent("components", JSON.array().add(JSON.object().put("id", defaultComponent()))); + + // Filter out the fields that can be set at creation time + var fields = JSON.object(); + finalProperties.entrySet().stream() + .filter(entry -> isInitialField(entry.getKey(), entry.getValue())) + .forEach(entry -> fields.put(entry.getKey(), entry.getValue())); + + // Certain types can only be set after creation, so always start with the default value + fields.put("issuetype", JSON.object().put("id", defaultIssueType())); + query.put("fields", fields); jiraHost.securityLevel().ifPresent(securityLevel -> fields.put("security", JSON.object() .put("id", securityLevel))); @@ -181,16 +235,22 @@ public Issue createIssue(String title, List body, Map pr .body(query) .execute(); - // Workaround - some fields cannot be set immediately - if (properties.containsKey("type") && properties.get("type").equals("Backport")) { + // Apply fields that have to be set later (if any) + var editFields = JSON.object(); + finalProperties.entrySet().stream() + .filter(entry -> !isInitialField(entry.getKey(), entry.getValue())) + .forEach(entry -> editFields.put(entry.getKey(), entry.getValue())); + + if (editFields.fields().size() > 0) { var id = data.get("key").asString(); if (id.indexOf('-') < 0) { id = projectName.toUpperCase() + "-" + id; } - var updateQuery = JSON.object().put("fields", fixupFields); + var updateQuery = JSON.object().put("fields", editFields); request.put("issue/" + id) .body(updateQuery) .execute(); + } return issue(data.get("key").asString()).orElseThrow(); diff --git a/issuetracker/src/test/java/org/openjdk/skara/issuetracker/IssueTrackerTests.java b/issuetracker/src/test/java/org/openjdk/skara/issuetracker/IssueTrackerTests.java index 6f12fa809..92ce04fc7 100644 --- a/issuetracker/src/test/java/org/openjdk/skara/issuetracker/IssueTrackerTests.java +++ b/issuetracker/src/test/java/org/openjdk/skara/issuetracker/IssueTrackerTests.java @@ -22,6 +22,7 @@ */ package org.openjdk.skara.issuetracker; +import org.openjdk.skara.json.JSON; import org.openjdk.skara.test.HostCredentials; import org.junit.jupiter.api.*; @@ -60,12 +61,13 @@ void simple(TestInfo info) throws IOException { issue.addLabel("another"); issue.removeLabel("label"); issue.setAssignees(List.of(project.issueTracker().currentUser())); - issue.addFixVersion("1.0"); - issue.addFixVersion("2.0"); - issue.removeFixVersion("1.0"); + issue.setProperty("fixVersions", JSON.array().add("1.0")); + issue.setProperty("fixVersions", JSON.array().add("1.0").add("2.0")); + issue.setProperty("fixVersions", JSON.array().add("3.0")); var updated = project.issue(issue.id()).orElseThrow(); assertEquals(List.of("another"), updated.labels()); - assertEquals(List.of("2.0"), updated.fixVersions()); + assertEquals(1, updated.properties().get("fixVersions").asArray().size()); + assertEquals("3.0", updated.properties().get("fixVersions").get(0).asString()); assertEquals(List.of(project.issueTracker().currentUser()), updated.assignees()); assertEquals(1, updated.comments().size()); assertEquals("Updated title", updated.title()); diff --git a/test/src/main/java/org/openjdk/skara/test/IssueData.java b/test/src/main/java/org/openjdk/skara/test/IssueData.java index a4e11a46d..ab6aafc7c 100644 --- a/test/src/main/java/org/openjdk/skara/test/IssueData.java +++ b/test/src/main/java/org/openjdk/skara/test/IssueData.java @@ -24,6 +24,7 @@ import org.openjdk.skara.host.HostUser; import org.openjdk.skara.issuetracker.*; +import org.openjdk.skara.json.JSONValue; import java.time.ZonedDateTime; import java.util.*; @@ -36,8 +37,7 @@ class IssueData { final Set labels = new HashSet<>(); final List assignees = new ArrayList<>(); final List links = new ArrayList<>(); - final Set fixVersions = new HashSet<>(); - final Map properties = new HashMap<>(); + final Map properties = new HashMap<>(); ZonedDateTime created = ZonedDateTime.now(); ZonedDateTime lastUpdate = created; } diff --git a/test/src/main/java/org/openjdk/skara/test/TestHost.java b/test/src/main/java/org/openjdk/skara/test/TestHost.java index c467dcec7..59c29d8c0 100644 --- a/test/src/main/java/org/openjdk/skara/test/TestHost.java +++ b/test/src/main/java/org/openjdk/skara/test/TestHost.java @@ -25,6 +25,7 @@ import org.openjdk.skara.forge.*; import org.openjdk.skara.host.*; import org.openjdk.skara.issuetracker.*; +import org.openjdk.skara.json.JSONValue; import org.openjdk.skara.vcs.*; import java.io.*; @@ -162,7 +163,7 @@ List getPullRequests(TestHostedRepository repository) { .collect(Collectors.toList()); } - TestIssue createIssue(TestIssueProject issueProject, String title, List body, Map properties) { + TestIssue createIssue(TestIssueProject issueProject, String title, List body, Map properties) { var id = issueProject.projectName().toUpperCase() + "-" + (data.issues.size() + 1); var issue = TestIssue.createNew(issueProject, id, title, body, properties); data.issues.put(id ,issue); diff --git a/test/src/main/java/org/openjdk/skara/test/TestIssue.java b/test/src/main/java/org/openjdk/skara/test/TestIssue.java index 42cfb7a55..a261843f7 100644 --- a/test/src/main/java/org/openjdk/skara/test/TestIssue.java +++ b/test/src/main/java/org/openjdk/skara/test/TestIssue.java @@ -24,6 +24,7 @@ import org.openjdk.skara.host.HostUser; import org.openjdk.skara.issuetracker.*; +import org.openjdk.skara.json.JSONValue; import org.openjdk.skara.network.URIBuilder; import java.net.URI; @@ -45,7 +46,7 @@ protected TestIssue(TestIssueProject issueProject, String id, HostUser author, H this.data = data; } - static TestIssue createNew(TestIssueProject issueProject, String id, String title, List body, Map properties) { + static TestIssue createNew(TestIssueProject issueProject, String id, String title, List body, Map properties) { var data = new IssueData(); data.title = title; data.body = String.join("\n", body); @@ -227,35 +228,18 @@ public void removeLink(Link link) { } @Override - public List fixVersions() { - return new ArrayList<>(data.fixVersions); - } - - @Override - public void addFixVersion(String fixVersion) { - data.fixVersions.add(fixVersion); - data.lastUpdate = ZonedDateTime.now(); - } - - @Override - public void removeFixVersion(String fixVersion) { - data.fixVersions.remove(fixVersion); - data.lastUpdate = ZonedDateTime.now(); - } - - @Override - public Map properties() { + public Map properties() { return data.properties; } @Override - public void setProperty(String name, String value) { + public void setProperty(String name, JSONValue value) { data.properties.put(name, value); data.lastUpdate = ZonedDateTime.now(); } @Override - public void removePropery(String name) { + public void removeProperty(String name) { data.properties.remove(name); data.lastUpdate = ZonedDateTime.now(); } diff --git a/test/src/main/java/org/openjdk/skara/test/TestIssueProject.java b/test/src/main/java/org/openjdk/skara/test/TestIssueProject.java index ed48f7050..b8ab8a9d3 100644 --- a/test/src/main/java/org/openjdk/skara/test/TestIssueProject.java +++ b/test/src/main/java/org/openjdk/skara/test/TestIssueProject.java @@ -23,6 +23,7 @@ package org.openjdk.skara.test; import org.openjdk.skara.issuetracker.*; +import org.openjdk.skara.json.JSONValue; import org.openjdk.skara.network.URIBuilder; import java.net.URI; @@ -52,7 +53,7 @@ public TestIssueProject(TestHost host, String projectName) { } @Override - public Issue createIssue(String title, List body, Map properties) { + public Issue createIssue(String title, List body, Map properties) { return host.createIssue(this, title, body, properties); }