Skip to content

Commit bca1cca

Browse files
asbjornuasbjornu
committed
Refactor RepositoryStore.GetBranchesContainingCommit
Refactor `RepositoryStore.GetBranchesContainingCommit into the `BranchesContainingCommitFinder` class to improve readability, provide better stack traces if an exception occurs and provide better protection against `null`.
1 parent 5532346 commit bca1cca

File tree

2 files changed

+81
-65
lines changed

2 files changed

+81
-65
lines changed

‎src/GitVersion.Core/Core/BranchesContainingCommitFinder.cs

Lines changed: 80 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,80 @@
1+
using GitVersion.Extensions;
2+
using GitVersion.Logging;
3+
4+
namespace GitVersion;
5+
6+
internal class BranchesContainingCommitFinder
7+
{
8+
private readonly ILog log;
9+
private readonly IGitRepository repository;
10+
11+
public BranchesContainingCommitFinder(IGitRepository repository, ILog log)
12+
{
13+
this.repository = repository.NotNull();
14+
this.log = log.NotNull();
15+
}
16+
17+
public IEnumerable<IBranch> GetBranchesContainingCommit(ICommit? commit, IEnumerable<IBranch>? branches = null, bool onlyTrackedBranches = false)
18+
{
19+
commit = commit.NotNull();
20+
branches ??= this.repository.Branches.ToList();
21+
22+
// TODO Should we cache this?
23+
// Yielding part is split from the main part of the method to avoid having the exception check performed lazily.
24+
// Details at https://github.com/GitTools/GitVersion/issues/2755
25+
return InnerGetBranchesContainingCommit(commit, branches, onlyTrackedBranches);
26+
}
27+
28+
private IEnumerable<IBranch> InnerGetBranchesContainingCommit(IGitObject commit, IEnumerable<IBranch> branches, bool onlyTrackedBranches)
29+
{
30+
using (log.IndentLog($"Getting branches containing the commit '{commit.Id}'."))
31+
{
32+
var directBranchHasBeenFound = false;
33+
log.Info("Trying to find direct branches.");
34+
// TODO: It looks wasteful looping through the branches twice. Can't these loops be merged somehow? @asbjornu
35+
List<IBranch> branchList = branches.ToList();
36+
foreach (var branch in branchList.Where(branch => BranchTipIsNullOrCommit(branch, commit) && !IncludeTrackedBranches(branch, onlyTrackedBranches)))
37+
{
38+
directBranchHasBeenFound = true;
39+
log.Info($"Direct branch found: '{branch}'.");
40+
yield return branch;
41+
}
42+
43+
if (directBranchHasBeenFound)
44+
{
45+
yield break;
46+
}
47+
48+
log.Info($"No direct branches found, searching through {(onlyTrackedBranches ? "tracked" : "all")} branches.");
49+
foreach (IBranch branch in branchList.Where(b => IncludeTrackedBranches(b, onlyTrackedBranches)))
50+
{
51+
log.Info($"Searching for commits reachable from '{branch}'.");
52+
53+
var commits = GetCommitsReacheableFrom(commit, branch);
54+
55+
if (!commits.Any())
56+
{
57+
log.Info($"The branch '{branch}' has no matching commits.");
58+
continue;
59+
}
60+
61+
log.Info($"The branch '{branch}' has a matching commit.");
62+
yield return branch;
63+
}
64+
}
65+
}
66+
67+
private IEnumerable<ICommit> GetCommitsReacheableFrom(IGitObject commit, IBranch branch)
68+
{
69+
var filter = new CommitFilter { IncludeReachableFrom = branch };
70+
var commitCollection = this.repository.Commits.QueryBy(filter);
71+
72+
return commitCollection.Where(c => c.Sha == commit.Sha);
73+
}
74+
75+
private static bool IncludeTrackedBranches(IBranch branch, bool includeOnlyTracked)
76+
=> includeOnlyTracked && branch.IsTracking || !includeOnlyTracked;
77+
78+
private static bool BranchTipIsNullOrCommit(IBranch branch, IGitObject commit)
79+
=> branch.Tip == null || branch.Tip.Sha == commit.Sha;
80+
}

‎src/GitVersion.Core/Core/RepositoryStore.cs

Lines changed: 1 addition & 65 deletions
Original file line numberDiff line numberDiff line change
@@ -154,64 +154,8 @@ public IEnumerable<IBranch> GetReleaseBranches(IEnumerable<KeyValuePair<string,
154154

155155
public IEnumerable<IBranch> ExcludingBranches(IEnumerable<IBranch> branchesToExclude) => this.repository.Branches.ExcludeBranches(branchesToExclude);
156156

157-
// TODO Should we cache this?
158157
public IEnumerable<IBranch> GetBranchesContainingCommit(ICommit? commit, IEnumerable<IBranch>? branches = null, bool onlyTrackedBranches = false)
159-
{
160-
commit = commit.NotNull();
161-
162-
return InnerGetBranchesContainingCommit(commit, branches, onlyTrackedBranches, this.repository, this.log);
163-
164-
static bool IncludeTrackedBranches(IBranch branch, bool includeOnlyTracked)
165-
=> includeOnlyTracked && branch.IsTracking || !includeOnlyTracked;
166-
167-
// Yielding part is split from the main part of the method to avoid having the exception check performed lazily.
168-
// Details at https://github.com/GitTools/GitVersion/issues/2755
169-
static IEnumerable<IBranch> InnerGetBranchesContainingCommit(IGitObject commit, IEnumerable<IBranch>? branches, bool onlyTrackedBranches, IGitRepository repository, ILog log)
170-
{
171-
branches ??= repository.Branches.ToList();
172-
173-
using (log.IndentLog($"Getting branches containing the commit '{commit.Id}'."))
174-
{
175-
var directBranchHasBeenFound = false;
176-
log.Info("Trying to find direct branches.");
177-
// TODO: It looks wasteful looping through the branches twice. Can't these loops be merged somehow? @asbjornu
178-
List<IBranch> branchList = branches.ToList();
179-
foreach (IBranch branch in branchList)
180-
{
181-
if (branch.Tip != null && branch.Tip.Sha != commit.Sha || IncludeTrackedBranches(branch, onlyTrackedBranches))
182-
{
183-
continue;
184-
}
185-
186-
directBranchHasBeenFound = true;
187-
log.Info($"Direct branch found: '{branch}'.");
188-
yield return branch;
189-
}
190-
191-
if (directBranchHasBeenFound)
192-
{
193-
yield break;
194-
}
195-
196-
log.Info($"No direct branches found, searching through {(onlyTrackedBranches ? "tracked" : "all")} branches.");
197-
foreach (IBranch branch in branchList.Where(b => IncludeTrackedBranches(b, onlyTrackedBranches)))
198-
{
199-
log.Info($"Searching for commits reachable from '{branch}'.");
200-
201-
var commits = GetCommitsReacheableFrom(repository, commit, branch);
202-
203-
if (!commits.Any())
204-
{
205-
log.Info($"The branch '{branch}' has no matching commits.");
206-
continue;
207-
}
208-
209-
log.Info($"The branch '{branch}' has a matching commit.");
210-
yield return branch;
211-
}
212-
}
213-
}
214-
}
158+
=> new BranchesContainingCommitFinder(this.repository, this.log).GetBranchesContainingCommit(commit, branches, onlyTrackedBranches);
215159

216160
public Dictionary<string, List<IBranch>> GetMainlineBranches(ICommit commit, Config configuration, IEnumerable<KeyValuePair<string, BranchConfig>>? mainlineBranchConfigs)
217161
{
@@ -389,12 +333,4 @@ private bool BranchMatchesMainlineConfig(INamedReference branch, KeyValuePair<st
389333
this.log.Info($"Found no merge base or parent commit for '{branchName}'.");
390334
return null;
391335
}
392-
393-
private static IEnumerable<ICommit> GetCommitsReacheableFrom(IGitRepository repository, IGitObject commit, IBranch branch)
394-
{
395-
var filter = new CommitFilter { IncludeReachableFrom = branch };
396-
var commitCollection = repository.Commits.QueryBy(filter);
397-
398-
return commitCollection.Where(c => c.Sha == commit.Sha);
399-
}
400336
}

0 commit comments

Comments
 (0)