#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.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 boolean isMasterBranch() {
 		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.TableHeader;
 import org.springframework.util.StringUtils;
 
-import lombok.RequiredArgsConstructor;
-
 /**
  * @author Oliver Gierke
  */
@@ -133,32 +132,27 @@ public void backportChangelogs(@CliOption(key = "", mandatory = true) TrainItera
 	 * @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 VersionTags getTags(Project project) {
 	/**
 	 * 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();
-
-			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, possibleTicketIds);
-			Map<String, Ticket> ticketMap = tickets.stream().collect(Collectors.toMap(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());
-						}
-
-						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) -> {
-
-						Matcher matcher = pattern.matcher(branchName);
-						matcher.find();
-						String ticketId = matcher.group(1);
-						ticketBranches.put(Branch.from(branchName), ticketMap.get(ticketId));
-					});
-
-			return TicketBranches.from(ticketBranches);
-		} catch (Exception o_O) {
-			throw new RuntimeException(o_O);
-		}
+
+			Map<String, Branch> ticketIds = getRemoteBranches(project).//
+					filter(branch -> branch.isIssueBranch(project.getTracker())).//
+					collect(Collectors.toMap(Branch::toString, branch -> branch));
+
+			Collection<Ticket> tickets = tracker.findTickets(project, ticketIds.keySet());
+
+			return TicketBranches.from(tickets.stream()
+					.collect(Collectors.toMap(ticket -> ticketIds.get(ticket.getId()), ticket -> ticket)));
+		});
+	}
+
+	private Stream<Branch> getRemoteBranches(Project project) {
+
+		return doWithGit(project, git -> {
+
+			Collection<Ref> refs = git.lsRemote().//
+					setHeads(true).//
+					setTags(false).//
+					call();
+
+			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 @@
  * @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 @@ public Ticket getReleaseTicketFor(ModuleIteration module) {
 				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 @@ private Ticket toTicket(GitHubIssue issue) {
 	@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 @@ public Collection<Ticket> findTickets(Project project, Collection<String> ticket
 			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 @@ private List<GitHubIssue> getIssuesFor(ModuleIteration module) {
 		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/GithubTicketStatus.java

@@ -20,6 +20,7 @@
 
 /**
  * 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 @@ public Ticket getReleaseTicketFor(ModuleIteration iteration) {
 
 	/**
 	 * (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;