#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.

release-tools/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<Branch> {
 		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()

release-tools/src/main/java/org/springframework/data/release/git/Branches.java

@@ -48,7 +48,6 @@ public class Branches implements Iterable<Branch> {
 				sorted().collect(Collectors.toList());
 	}
 
-
 	/**
 	 * Returns all {@link Branch}es as {@link List}.
 	 * 

release-tools/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> 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> 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(""));
 				});
 

release-tools/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<Ref> branches = git.lsRemote().setHeads(true).setTags(false).call();
+			Map<String, Branch> ticketIds = getRemoteBranches(project).//
+					filter(branch -> branch.isIssueBranch(project.getTracker())).//
+					collect(Collectors.toMap(Branch::toString, branch -> branch));
 
-			Set<String> 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<Ticket> tickets = tracker.findTickets(project, ticketIds.keySet());
 
-			Collection<Ticket> tickets = tracker.findTickets(project, possibleTicketIds);
-			Map<String, Ticket> 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<Branch, Ticket> ticketBranches = new HashMap<>();
-			branches.stream().//
-					map(branch -> {
-						if (branch.getName().startsWith(Constants.R_HEADS)) {
-							return branch.getName().substring(Constants.R_HEADS.length());
-						}
+	private Stream<Branch> 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<Ref> 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) {

release-tools/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<List<GitHubMilestone>> MILESTONES_TYPE = new ParameterizedTypeReference<List<GitHubMilestone>>() {
-	};
-	private static final ParameterizedTypeReference<List<GitHubIssue>> ISSUES_TYPE = new ParameterizedTypeReference<List<GitHubIssue>>() {
-	};
-	private static final ParameterizedTypeReference<GitHubIssue> ISSUE_TYPE = new ParameterizedTypeReference<GitHubIssue>() {
-	};
+	private static final ParameterizedTypeReference<List<GitHubMilestone>> MILESTONES_TYPE = new ParameterizedTypeReference<List<GitHubMilestone>>() {};
+	private static final ParameterizedTypeReference<List<GitHubIssue>> ISSUES_TYPE = new ParameterizedTypeReference<List<GitHubIssue>>() {};
+	private static final ParameterizedTypeReference<GitHubIssue> ISSUE_TYPE = new ParameterizedTypeReference<GitHubIssue>() {};
 
 	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<Ticket> findTickets(Project project, Collection<String> ticketIds) {
 
-		String repositoryName = new GitProject(project, new GitServer()).getRepositoryName();
+		String repositoryName = GitProject.of(project).getRepositoryName();
 		List<Ticket> 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) {

release-tools/src/main/java/org/springframework/data/release/jira/GithubConnector.java

@@ -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 {
-
-}

release-tools/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

release-tools/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<Project> {
 	Collection<Ticket> findTickets(Project project, Collection<String> ticketIds);
 
 	Changelog getChangelogFor(ModuleIteration iteration);
-
 }

release-tools/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

release-tools/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;

release-tools/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<Branch> {
+@RequiredArgsConstructor(staticName = "from")
+public class TicketBranches implements Streamable<Branch> {
 
-	private Map<Branch, Ticket> ticketBranches;
+	private final @NonNull Map<Branch, Ticket> 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<Branch, Ticket> 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<Branch, Ticket> 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<Ticket> 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<Branch> iterator() {
 		return ticketBranches.keySet().iterator();
 	}
-
-	public Stream<Branch> stream() {
-		return StreamSupport.stream(spliterator(), false);
-	}
-
-	public Optional<Ticket> getTicket(Branch branch) {
-		Assert.notNull(branch, "Branch must not be null!");
-		return Optional.ofNullable(ticketBranches.get(branch));
-	}
 }

release-tools/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;
 }

release-tools/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));
+	}
 }