From 7181e33e3fed2a46c84bd469e99fa53d615666b0 Mon Sep 17 00:00:00 2001 From: Oliver Gierke Date: Sun, 14 Feb 2016 17:12:51 +0100 Subject: [PATCH] #2 - Polishing Moved more logic into value objects to ease testability. Branches now know whether they're an issue branch for a given tracker. TicketBranches now implements Streamable and allows obtaining a new instance with only resolved tickets in it an inspect whether a it contains a ticket for a given Branch. Cleaned up GitOperations and GitCommands to make use of the new functionality. "git issuebranches" command was tweaked to default resolvable to true in case the option is set. Renamed GitHubConnector to correct case. Fixed imports in GitHubIssueTracker. Formatting. --- .../data/release/git/Branch.java | 13 ++++ .../data/release/git/Branches.java | 1 - .../data/release/git/GitCommands.java | 28 +++---- .../data/release/git/GitOperations.java | 71 +++++++----------- .../data/release/jira/GitHubIssueTracker.java | 31 ++++---- .../data/release/jira/GithubConnector.java | 27 ------- .../data/release/jira/GithubTicketStatus.java | 1 + .../data/release/jira/IssueTracker.java | 3 +- .../data/release/jira/Jira.java | 1 + .../data/release/jira/JiraTicketStatus.java | 12 ++- .../data/release/jira/TicketBranches.java | 74 ++++++++++++------- .../data/release/model/Tracker.java | 22 +++--- .../data/release/git/BranchUnitTests.java | 13 ++++ 13 files changed, 148 insertions(+), 149 deletions(-) delete mode 100644 src/main/java/org/springframework/data/release/jira/GithubConnector.java diff --git a/src/main/java/org/springframework/data/release/git/Branch.java b/src/main/java/org/springframework/data/release/git/Branch.java index 630f83b..982018d 100644 --- a/src/main/java/org/springframework/data/release/git/Branch.java +++ b/src/main/java/org/springframework/data/release/git/Branch.java @@ -20,6 +20,7 @@ import lombok.EqualsAndHashCode; import lombok.RequiredArgsConstructor; import org.springframework.data.release.model.IterationVersion; +import org.springframework.data.release.model.Tracker; import org.springframework.data.release.model.Version; import org.springframework.data.release.model.VersionAware; import org.springframework.util.Assert; @@ -73,6 +74,18 @@ public class Branch implements Comparable { return MASTER.equals(this); } + /** + * Returns whether the current branch is an issue branch for the given {@link Tracker}. + * + * @param tracker must not be {@literal null}. + * @return + */ + public boolean isIssueBranch(Tracker tracker) { + + Assert.notNull(tracker, "Tracker must not be null!"); + return name.matches(tracker.getTicketPattern()); + } + /* * (non-Javadoc) * @see java.lang.Object#toString() diff --git a/src/main/java/org/springframework/data/release/git/Branches.java b/src/main/java/org/springframework/data/release/git/Branches.java index 1e72ed4..b00a4e4 100644 --- a/src/main/java/org/springframework/data/release/git/Branches.java +++ b/src/main/java/org/springframework/data/release/git/Branches.java @@ -48,7 +48,6 @@ public class Branches implements Iterable { sorted().collect(Collectors.toList()); } - /** * Returns all {@link Branch}es as {@link List}. * diff --git a/src/main/java/org/springframework/data/release/git/GitCommands.java b/src/main/java/org/springframework/data/release/git/GitCommands.java index 540b0ff..cbfdb81 100644 --- a/src/main/java/org/springframework/data/release/git/GitCommands.java +++ b/src/main/java/org/springframework/data/release/git/GitCommands.java @@ -15,9 +15,10 @@ */ package org.springframework.data.release.git; -import java.util.Optional; +import lombok.RequiredArgsConstructor; import java.util.List; +import java.util.Optional; import java.util.stream.Collectors; import java.util.stream.Stream; @@ -36,8 +37,6 @@ import org.springframework.shell.support.table.Table; import org.springframework.shell.support.table.TableHeader; import org.springframework.util.StringUtils; -import lombok.RequiredArgsConstructor; - /** * @author Oliver Gierke */ @@ -133,32 +132,27 @@ public class GitCommands implements CommandMarker { * @throws Exception */ @CliCommand("git issuebranches") + @SuppressWarnings("deprecation") public Table issuebranches(@CliOption(key = { "" }, mandatory = true) String projectName, - @CliOption(key = "resolved") Boolean resolved) throws Exception { + @CliOption(key = "resolved", unspecifiedDefaultValue = "false", specifiedDefaultValue = "true") Boolean resolved) + throws Exception { Project project = ReleaseTrains.getProjectByName(projectName); - - Table table = new Table(); TicketBranches ticketBranches = git.listTicketBranches(project); + Table table = new Table(); table.addHeader(1, new TableHeader("Branch")); table.addHeader(2, new TableHeader("Status")); table.addHeader(3, new TableHeader("Description")); ticketBranches.stream().sorted().// - filter(branch -> { - Optional ticket = ticketBranches.getTicket(branch); - if (resolved != null && resolved) { - if (!ticket.isPresent() || (ticket.isPresent() && ticket.get().getTicketStatus().isResolved())) { - return false; - } - } - - return true; - }).// + filter(branch -> ticketBranches.hasTicketFor(branch, resolved)).// forEachOrdered(branch -> { + Optional ticket = ticketBranches.getTicket(branch); - table.addRow(branch.toString(), ticket.map(t -> t.getTicketStatus().getLabel()).orElse(""), + + table.addRow(branch.toString(), // + ticket.map(t -> t.getTicketStatus().getLabel()).orElse(""), // ticket.map(t -> t.getSummary()).orElse("")); }); diff --git a/src/main/java/org/springframework/data/release/git/GitOperations.java b/src/main/java/org/springframework/data/release/git/GitOperations.java index 9dc3df3..aba829e 100644 --- a/src/main/java/org/springframework/data/release/git/GitOperations.java +++ b/src/main/java/org/springframework/data/release/git/GitOperations.java @@ -15,26 +15,22 @@ */ package org.springframework.data.release.git; +import lombok.RequiredArgsConstructor; + import java.io.File; import java.io.IOException; import java.util.Collection; import java.util.List; -import java.util.HashMap; import java.util.Map; import java.util.Optional; -import java.util.Set; -import java.util.regex.Matcher; -import java.util.regex.Pattern; import java.util.stream.Collectors; - -import lombok.RequiredArgsConstructor; +import java.util.stream.Stream; import org.eclipse.jgit.api.CheckoutCommand; import org.eclipse.jgit.api.CreateBranchCommand.SetupUpstreamMode; import org.eclipse.jgit.api.Git; import org.eclipse.jgit.api.ResetCommand.ResetType; import org.eclipse.jgit.api.errors.RefNotFoundException; -import org.eclipse.jgit.lib.Constants; import org.eclipse.jgit.lib.ObjectId; import org.eclipse.jgit.lib.Ref; import org.eclipse.jgit.lib.Repository; @@ -280,58 +276,43 @@ public class GitOperations { /** * Retrieve a list of remote branches where their related ticket is resolved. * - * @param project + * @param project must not be {@literal null}. * @return */ public TicketBranches listTicketBranches(Project project) { + Assert.notNull(project, "Project must not be null!"); + IssueTracker tracker = issueTracker.getPluginFor(project); - try (Git git = new Git(getRepository(project))) { + return doWithGit(project, git -> { + update(project); - Pattern pattern = Pattern.compile(project.getTracker().getTicketPattern()); - Collection branches = git.lsRemote().setHeads(true).setTags(false).call(); + Map ticketIds = getRemoteBranches(project).// + filter(branch -> branch.isIssueBranch(project.getTracker())).// + collect(Collectors.toMap(Branch::toString, branch -> branch)); - Set possibleTicketIds = branches.stream().// - filter(branch -> pattern.matcher(branch.getName()).find()).// - map(branch -> { - Matcher matcher = pattern.matcher(branch.getName()); - matcher.find(); - return matcher.group(1); - }).// - collect(Collectors.toSet()); + Collection tickets = tracker.findTickets(project, ticketIds.keySet()); - Collection tickets = tracker.findTickets(project, possibleTicketIds); - Map ticketMap = tickets.stream().collect(Collectors.toMap(Ticket::getId, ticket -> ticket)); + return TicketBranches.from(tickets.stream() + .collect(Collectors.toMap(ticket -> ticketIds.get(ticket.getId()), ticket -> ticket))); + }); + } - Map ticketBranches = new HashMap<>(); - branches.stream().// - map(branch -> { - if (branch.getName().startsWith(Constants.R_HEADS)) { - return branch.getName().substring(Constants.R_HEADS.length()); - } + private Stream getRemoteBranches(Project project) { - if (branch.getName().startsWith(Constants.R_REMOTES)) { - return branch.getName().substring(Constants.R_REMOTES.length()); - } - return branch.getName(); - }).filter(branchName -> { - Matcher matcher = pattern.matcher(branchName); - return matcher.find(); - }).// - forEach((branchName) -> { + return doWithGit(project, git -> { - Matcher matcher = pattern.matcher(branchName); - matcher.find(); - String ticketId = matcher.group(1); - ticketBranches.put(Branch.from(branchName), ticketMap.get(ticketId)); - }); + Collection refs = git.lsRemote().// + setHeads(true).// + setTags(false).// + call(); - return TicketBranches.from(ticketBranches); - } catch (Exception o_O) { - throw new RuntimeException(o_O); - } + return refs.stream().// + map(Ref::getName).// + map(Branch::from);// + }); } public void tagRelease(TrainIteration iteration) { diff --git a/src/main/java/org/springframework/data/release/jira/GitHubIssueTracker.java b/src/main/java/org/springframework/data/release/jira/GitHubIssueTracker.java index b3a3af9..340fc59 100644 --- a/src/main/java/org/springframework/data/release/jira/GitHubIssueTracker.java +++ b/src/main/java/org/springframework/data/release/jira/GitHubIssueTracker.java @@ -15,15 +15,17 @@ */ package org.springframework.data.release.jira; +import lombok.RequiredArgsConstructor; + import java.net.URI; +import java.util.ArrayList; import java.util.Arrays; +import java.util.Collection; import java.util.HashMap; import java.util.List; import java.util.Map; import java.util.stream.Collectors; -import lombok.RequiredArgsConstructor; - import org.springframework.cache.annotation.CacheEvict; import org.springframework.cache.annotation.Cacheable; import org.springframework.context.ConfigurableApplicationContext; @@ -51,18 +53,15 @@ import org.springframework.web.util.UriTemplate; * @author Oliver Gierke */ @RequiredArgsConstructor -class GitHubIssueTracker implements GitHubConnector { +class GitHubIssueTracker implements IssueTracker { private static final String MILESTONE_URI = "https://api.github.com/repos/spring-projects/{repoName}/milestones?state={state}"; private static final String ISSUES_BY_MILESTONE_URI_TEMPLATE = "https://api.github.com/repos/spring-projects/{repoName}/issues?milestone={id}&state=all"; private static final String ISSUE_BY_ID_URI_TEMPLATE = "https://api.github.com/repos/spring-projects/{repoName}/issues/{id}"; - private static final ParameterizedTypeReference> MILESTONES_TYPE = new ParameterizedTypeReference>() { - }; - private static final ParameterizedTypeReference> ISSUES_TYPE = new ParameterizedTypeReference>() { - }; - private static final ParameterizedTypeReference ISSUE_TYPE = new ParameterizedTypeReference() { - }; + private static final ParameterizedTypeReference> MILESTONES_TYPE = new ParameterizedTypeReference>() {}; + private static final ParameterizedTypeReference> ISSUES_TYPE = new ParameterizedTypeReference>() {}; + private static final ParameterizedTypeReference ISSUE_TYPE = new ParameterizedTypeReference() {}; private final RestOperations operations; private final Logger logger; @@ -91,7 +90,7 @@ class GitHubIssueTracker implements GitHubConnector { findFirst().// map(issue -> toTicket(issue)).// orElseThrow( - () -> new IllegalArgumentException(String.format("Could not find a release ticket for %s!", module))); + () -> new IllegalArgumentException(String.format("Could not find a release ticket for %s!", module))); } private Ticket toTicket(GitHubIssue issue) { @@ -107,7 +106,7 @@ class GitHubIssueTracker implements GitHubConnector { @Cacheable("tickets") public Collection findTickets(Project project, Collection ticketIds) { - String repositoryName = new GitProject(project, new GitServer()).getRepositoryName(); + String repositoryName = GitProject.of(project).getRepositoryName(); List tickets = new ArrayList<>(); for (String ticketId : ticketIds) { @@ -116,9 +115,8 @@ class GitHubIssueTracker implements GitHubConnector { parameters.put("id", ticketId); try { - GitHubIssue gitHubIssue = operations - .exchange(ISSUE_BY_ID_URI_TEMPLATE, HttpMethod.GET, new HttpEntity<>(getAuthenticationHeaders()), ISSUE_TYPE, parameters) - .getBody(); + GitHubIssue gitHubIssue = operations.exchange(ISSUE_BY_ID_URI_TEMPLATE, HttpMethod.GET, + new HttpEntity<>(getAuthenticationHeaders()), ISSUE_TYPE, parameters).getBody(); tickets.add(toTicket(gitHubIssue)); } catch (HttpStatusCodeException e) { @@ -168,9 +166,8 @@ class GitHubIssueTracker implements GitHubConnector { parameters.put("repoName", repositoryName); parameters.put("id", milestone.getNumber()); - return operations - .exchange(ISSUES_BY_MILESTONE_URI_TEMPLATE, HttpMethod.GET, new HttpEntity<>(getAuthenticationHeaders()), ISSUES_TYPE, parameters) - .getBody(); + return operations.exchange(ISSUES_BY_MILESTONE_URI_TEMPLATE, HttpMethod.GET, + new HttpEntity<>(getAuthenticationHeaders()), ISSUES_TYPE, parameters).getBody(); } private GitHubMilestone findMilestone(ModuleIteration module, String repositoryName) { diff --git a/src/main/java/org/springframework/data/release/jira/GithubConnector.java b/src/main/java/org/springframework/data/release/jira/GithubConnector.java deleted file mode 100644 index 58e5ac7..0000000 --- a/src/main/java/org/springframework/data/release/jira/GithubConnector.java +++ /dev/null @@ -1,27 +0,0 @@ -/* - * Copyright 2016 the original author or authors. - * - * Licensed under the Apache License, Version 2.0 (the "License"); - * you may not use this file except in compliance with the License. - * You may obtain a copy of the License at - * - * http://www.apache.org/licenses/LICENSE-2.0 - * - * Unless required by applicable law or agreed to in writing, software - * distributed under the License is distributed on an "AS IS" BASIS, - * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. - * See the License for the specific language governing permissions and - * limitations under the License. - */ -package org.springframework.data.release.jira; - -import org.springframework.data.release.model.Iteration; -import org.springframework.data.release.model.Train; -import org.springframework.data.release.model.TrainIteration; - -/** - * @author Mark Paluch - */ -public interface GitHubConnector extends IssueTracker { - -} diff --git a/src/main/java/org/springframework/data/release/jira/GithubTicketStatus.java b/src/main/java/org/springframework/data/release/jira/GithubTicketStatus.java index f0533b0..c39ef51 100644 --- a/src/main/java/org/springframework/data/release/jira/GithubTicketStatus.java +++ b/src/main/java/org/springframework/data/release/jira/GithubTicketStatus.java @@ -20,6 +20,7 @@ import lombok.AllArgsConstructor; /** * Value object for a GitHub ticket status. + * * @author Mark Paluch */ @AllArgsConstructor diff --git a/src/main/java/org/springframework/data/release/jira/IssueTracker.java b/src/main/java/org/springframework/data/release/jira/IssueTracker.java index a846e21..38ffe16 100644 --- a/src/main/java/org/springframework/data/release/jira/IssueTracker.java +++ b/src/main/java/org/springframework/data/release/jira/IssueTracker.java @@ -1,5 +1,5 @@ /* - * Copyright 2014 the original author or authors. + * Copyright 2014-2016 the original author or authors. * * Licensed under the Apache License, Version 2.0 (the "License"); * you may not use this file except in compliance with the License. @@ -50,5 +50,4 @@ public interface IssueTracker extends Plugin { Collection findTickets(Project project, Collection ticketIds); Changelog getChangelogFor(ModuleIteration iteration); - } diff --git a/src/main/java/org/springframework/data/release/jira/Jira.java b/src/main/java/org/springframework/data/release/jira/Jira.java index 2c2b98a..13b93d4 100644 --- a/src/main/java/org/springframework/data/release/jira/Jira.java +++ b/src/main/java/org/springframework/data/release/jira/Jira.java @@ -90,6 +90,7 @@ class Jira implements JiraConnector { /** * (non-Javadoc) + * * @see org.springframework.data.release.jira.IssueTracker#findTickets(Project, Collection) */ @Override diff --git a/src/main/java/org/springframework/data/release/jira/JiraTicketStatus.java b/src/main/java/org/springframework/data/release/jira/JiraTicketStatus.java index c3782ca..3ba953b 100644 --- a/src/main/java/org/springframework/data/release/jira/JiraTicketStatus.java +++ b/src/main/java/org/springframework/data/release/jira/JiraTicketStatus.java @@ -16,12 +16,12 @@ package org.springframework.data.release.jira; -import lombok.AllArgsConstructor; +import lombok.RequiredArgsConstructor; /** * @author Mark Paluch */ -@AllArgsConstructor +@RequiredArgsConstructor public class JiraTicketStatus implements TicketStatus { public static final JiraTicketStatus UNKNOWN = new JiraTicketStatus(false, "unknown", null); @@ -30,11 +30,19 @@ public class JiraTicketStatus implements TicketStatus { private final String status; private final String resolution; + /* + * (non-Javadoc) + * @see org.springframework.data.release.jira.TicketStatus#getLabel() + */ @Override public String getLabel() { return resolution == null ? status : status + "/" + resolution; } + /* + * (non-Javadoc) + * @see org.springframework.data.release.jira.TicketStatus#isResolved() + */ @Override public boolean isResolved() { return resolved; diff --git a/src/main/java/org/springframework/data/release/jira/TicketBranches.java b/src/main/java/org/springframework/data/release/jira/TicketBranches.java index 822189d..eb51fa9 100644 --- a/src/main/java/org/springframework/data/release/jira/TicketBranches.java +++ b/src/main/java/org/springframework/data/release/jira/TicketBranches.java @@ -13,57 +13,81 @@ * See the License for the specific language governing permissions and * limitations under the License. */ - package org.springframework.data.release.jira; +import lombok.EqualsAndHashCode; +import lombok.NonNull; +import lombok.RequiredArgsConstructor; + import java.util.Iterator; import java.util.Map; +import java.util.Map.Entry; import java.util.Optional; -import java.util.stream.Stream; -import java.util.stream.StreamSupport; - -import lombok.EqualsAndHashCode; +import java.util.stream.Collectors; +import org.springframework.data.release.Streamable; import org.springframework.data.release.git.Branch; import org.springframework.util.Assert; /** - * Value object to represent a collection of {@link Branch}es with optionally assigned tickets. + * Value object to represent a collection of {@link Branch}es with assigned tickets. * * @author Mark Paluch + * @author Oliver Gierke */ @EqualsAndHashCode -public class TicketBranches implements Iterable { +@RequiredArgsConstructor(staticName = "from") +public class TicketBranches implements Streamable { - private Map ticketBranches; + private final @NonNull Map ticketBranches; /** - * Creates a new {@link TicketBranches} instance for the given {@link Map} of {@link Branch}es and {@link Ticket}s. - * - * @param ticketBranches must not be {@literal null}. + * Returns whether there's a ticket available for the given {@link Branch}. If {@code requireResolved} is set to + * {@literal true} the answer will only be true if the available ticket is marked resolved. + * + * @param branch must not be {@literal null}. + * @param requireResolved whether the {@link Ticket} we look for is required to be resolved. + * @return */ - TicketBranches(Map ticketBranches) { + public boolean hasTicketFor(Branch branch, boolean requireResolved) { - Assert.notNull(ticketBranches, "TicketBranches must not be null!"); - this.ticketBranches = ticketBranches; + Assert.notNull(branch, "Branch must not be null!"); + + return getTicket(branch).// + map(ticket -> requireResolved ? ticket.getTicketStatus().isResolved() : true).// + orElse(false); } - public static TicketBranches from(Map ticketBranches) { + /** + * Returns a {@link TicketBranches} containing only the branches for which resolved {@link Ticket}s are found. + * + * @return + */ + public TicketBranches getResolvedTickets() { - return new TicketBranches(ticketBranches); + return new TicketBranches(ticketBranches.entrySet().stream().// + filter(entry -> entry.getValue().getTicketStatus().isResolved()).// + collect(Collectors.toMap(Entry::getKey, Entry::getValue))); } + /** + * Returns the ticket for the given {@link Branch}. + * + * @param branch must not be {@literal null}. + * @return + */ + public Optional getTicket(Branch branch) { + + Assert.notNull(branch, "Branch must not be null!"); + return Optional.ofNullable(ticketBranches.get(branch)); + } + + /* + * (non-Javadoc) + * @see java.lang.Iterable#iterator() + */ @Override public Iterator iterator() { return ticketBranches.keySet().iterator(); } - - public Stream stream() { - return StreamSupport.stream(spliterator(), false); - } - - public Optional getTicket(Branch branch) { - Assert.notNull(branch, "Branch must not be null!"); - return Optional.ofNullable(ticketBranches.get(branch)); - } } diff --git a/src/main/java/org/springframework/data/release/model/Tracker.java b/src/main/java/org/springframework/data/release/model/Tracker.java index b45524a..afc779d 100644 --- a/src/main/java/org/springframework/data/release/model/Tracker.java +++ b/src/main/java/org/springframework/data/release/model/Tracker.java @@ -1,5 +1,5 @@ /* - * Copyright 2014 the original author or authors. + * Copyright 2014-2016 the original author or authors. * * Licensed under the Apache License, Version 2.0 (the "License"); * you may not use this file except in compliance with the License. @@ -15,23 +15,19 @@ */ package org.springframework.data.release.model; +import lombok.AccessLevel; +import lombok.Getter; +import lombok.RequiredArgsConstructor; + /** * @author Oliver Gierke */ +@Getter +@RequiredArgsConstructor(access = AccessLevel.PACKAGE) public enum Tracker { - JIRA("(([A-Z]{1,10})+-\\d+)"), + JIRA("(([A-Z]{1,10})+-\\d+)"), // GITHUB("((#)?\\d+)"); - private String ticketPattern; - - Tracker(String ticketPattern) { - this.ticketPattern = ticketPattern; - } - - public String getTicketPattern() - { - return ticketPattern; - } - + private final String ticketPattern; } diff --git a/src/test/java/org/springframework/data/release/git/BranchUnitTests.java b/src/test/java/org/springframework/data/release/git/BranchUnitTests.java index fe92646..5723e21 100644 --- a/src/test/java/org/springframework/data/release/git/BranchUnitTests.java +++ b/src/test/java/org/springframework/data/release/git/BranchUnitTests.java @@ -22,6 +22,7 @@ import org.junit.Test; import org.springframework.data.release.model.Iteration; import org.springframework.data.release.model.IterationVersion; import org.springframework.data.release.model.SimpleIterationVersion; +import org.springframework.data.release.model.Tracker; import org.springframework.data.release.model.Version; /** @@ -42,4 +43,16 @@ public class BranchUnitTests { IterationVersion iterationVersion = new SimpleIterationVersion(Version.of(1, 4), Iteration.SR1); assertThat(Branch.from(iterationVersion).toString(), is("1.4.x")); } + + /** + * @see #2 + */ + @Test + public void detectsIssueBranches() { + + Branch branch = Branch.from("issue/DATACMNS-4711"); + + assertThat(branch.isIssueBranch(Tracker.JIRA), is(true)); + assertThat(branch.isIssueBranch(Tracker.GITHUB), is(false)); + } }