Skip to content
This repository was archived by the owner on Jun 21, 2023. It is now read-only.

Commit 713d9c4

Browse files
jcansdalejcansdale
committed
Refactor to IGitService.CreateCurrentBranchModel
Remove the ILocalRepositoryModel.CurrentBranch property and explicitly call IGitService.CreateCurrentBranchModel instead. Fix all the broken tests.
1 parent 47430fd commit 713d9c4

File tree

13 files changed

+90
-125
lines changed

13 files changed

+90
-125
lines changed

‎src/GitHub.App/SampleData/GitServiceDesigner.cs‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -9,7 +9,7 @@ namespace GitHub.SampleData
99
class GitServiceDesigner : IGitService
1010
{
1111
public ILocalRepositoryModel CreateLocalRepositoryModel(string localPath) => null;
12-
public ILocalRepositoryModel CreateLocalRepositoryModel(string name, UriString cloneUrl, string localPath) => null;
12+
public IBranch CreateCurrentBranchModel(ILocalRepositoryModel model) => null;
1313
public void Refresh(ILocalRepositoryModel localRepositoryModel) { }
1414
public Task<string> GetLatestPushedSha(string path, string remote = "origin") => Task.FromResult<string>(null);
1515
public UriString GetRemoteUri(IRepository repo, string remote = "origin") => null;

‎src/GitHub.App/Services/TeamExplorerContext.cs‎

Lines changed: 4 additions & 23 deletions
Original file line numberDiff line numberDiff line change
@@ -40,9 +40,6 @@ public class TeamExplorerContext : ITeamExplorerContext
4040
string solutionPath;
4141
string repositoryPath;
4242
UriString cloneUrl;
43-
string branchName;
44-
string headSha;
45-
string trackedSha;
4643
Tuple<string, int> pullRequest;
4744

4845
ILocalRepositoryModel repositoryModel;
@@ -113,9 +110,6 @@ async Task RefreshAsync()
113110
{
114111
var newRepositoryPath = repo?.LocalPath;
115112
var newCloneUrl = repo?.CloneUrl;
116-
var newBranchName = repo?.CurrentBranch?.Name;
117-
var newHeadSha = repo?.CurrentBranch?.Sha;
118-
var newTrackedSha = repo?.CurrentBranch?.TrackedSha;
119113
var newPullRequest = repo != null ? await pullRequestService.GetPullRequestForCurrentBranch(repo) : null;
120114

121115
if (newRepositoryPath != repositoryPath)
@@ -128,33 +122,20 @@ async Task RefreshAsync()
128122
log.Debug("ActiveRepository changed to {CloneUrl} @ {Path}", repo?.CloneUrl, newRepositoryPath);
129123
ActiveRepository = repo;
130124
}
131-
else if (newBranchName != branchName)
132-
{
133-
log.Debug("Fire StatusChanged event when BranchName changes for ActiveRepository");
134-
StatusChanged?.Invoke(this, EventArgs.Empty);
135-
}
136-
else if (newHeadSha != headSha)
137-
{
138-
log.Debug("Fire StatusChanged event when HeadSha changes for ActiveRepository");
139-
StatusChanged?.Invoke(this, EventArgs.Empty);
140-
}
141-
else if (newTrackedSha != trackedSha)
125+
else if (newPullRequest != pullRequest)
142126
{
143-
log.Debug("Fire StatusChanged event when TrackedSha changes for ActiveRepository");
127+
log.Debug("Fire StatusChanged event when PullRequest changes for ActiveRepository");
144128
StatusChanged?.Invoke(this, EventArgs.Empty);
145129
}
146-
else if (newPullRequest != pullRequest)
130+
else
147131
{
148-
log.Debug("Fire StatusChanged event when PullRequest changes for ActiveRepository");
132+
log.Debug("Fire StatusChanged event when ***anything*** changes");
149133
StatusChanged?.Invoke(this, EventArgs.Empty);
150134
}
151135

152136
repositoryPath = newRepositoryPath;
153137
cloneUrl = newCloneUrl;
154-
branchName = newBranchName;
155-
headSha = newHeadSha;
156138
solutionPath = newSolutionPath;
157-
trackedSha = newTrackedSha;
158139
pullRequest = newPullRequest;
159140
}
160141
}

‎src/GitHub.App/ViewModels/GitHubPane/PullRequestCreationViewModel.cs‎

Lines changed: 9 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -38,6 +38,7 @@ public class PullRequestCreationViewModel : PanePageViewModelBase, IPullRequestC
3838
readonly IPullRequestService service;
3939
readonly IModelServiceFactory modelServiceFactory;
4040
readonly IMessageDraftStore draftStore;
41+
readonly IGitService gitService;
4142
readonly IScheduler timerScheduler;
4243
readonly CompositeDisposable disposables = new CompositeDisposable();
4344
ILocalRepositoryModel activeLocalRepo;
@@ -49,8 +50,9 @@ public PullRequestCreationViewModel(
4950
IModelServiceFactory modelServiceFactory,
5051
IPullRequestService service,
5152
INotificationService notifications,
52-
IMessageDraftStore draftStore)
53-
: this(modelServiceFactory, service, notifications, draftStore, DefaultScheduler.Instance)
53+
IMessageDraftStore draftStore,
54+
IGitService gitService)
55+
: this(modelServiceFactory, service, notifications, draftStore, gitService, DefaultScheduler.Instance)
5456
{
5557
}
5658

@@ -59,17 +61,20 @@ public PullRequestCreationViewModel(
5961
IPullRequestService service,
6062
INotificationService notifications,
6163
IMessageDraftStore draftStore,
64+
IGitService gitService,
6265
IScheduler timerScheduler)
6366
{
6467
Guard.ArgumentNotNull(modelServiceFactory, nameof(modelServiceFactory));
6568
Guard.ArgumentNotNull(service, nameof(service));
6669
Guard.ArgumentNotNull(notifications, nameof(notifications));
6770
Guard.ArgumentNotNull(draftStore, nameof(draftStore));
71+
Guard.ArgumentNotNull(gitService, nameof(gitService));
6872
Guard.ArgumentNotNull(timerScheduler, nameof(timerScheduler));
6973

7074
this.service = service;
7175
this.modelServiceFactory = modelServiceFactory;
7276
this.draftStore = draftStore;
77+
this.gitService = gitService;
7378
this.timerScheduler = timerScheduler;
7479

7580
this.WhenAnyValue(x => x.Branches)
@@ -137,7 +142,7 @@ public async Task InitializeAsync(ILocalRepositoryModel repository, IConnection
137142
{
138143
modelService = await modelServiceFactory.CreateAsync(connection);
139144
activeLocalRepo = repository;
140-
SourceBranch = repository.CurrentBranch;
145+
SourceBranch = gitService.CreateCurrentBranchModel(repository);
141146

142147
var obs = modelService.ApiClient.GetRepository(repository.Owner, repository.Name)
143148
.Select(r => new RemoteRepositoryModel(r))
@@ -207,7 +212,7 @@ async Task LoadInitialState(string draftKey)
207212

208213
void LoadDescriptionFromCommits()
209214
{
210-
SourceBranch = activeLocalRepo.CurrentBranch;
215+
SourceBranch = gitService.CreateCurrentBranchModel(activeLocalRepo);
211216

212217
var uniqueCommits = this.WhenAnyValue(
213218
x => x.SourceBranch,

‎src/GitHub.App/ViewModels/GitHubPane/PullRequestDetailViewModel.cs‎

Lines changed: 2 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -405,9 +405,8 @@ public async Task Load(PullRequestDetailModel pullRequest)
405405

406406
var localBranches = await pullRequestsService.GetLocalBranches(LocalRepository, pullRequest).ToList();
407407

408-
// HACK: This is only necessary because the CurrentBranch is being read too early
409-
gitService.Refresh(LocalRepository);
410-
IsCheckedOut = localBranches.Contains(LocalRepository.CurrentBranch);
408+
var currentBranch = gitService.CreateCurrentBranchModel(LocalRepository);
409+
IsCheckedOut = localBranches.Contains(currentBranch);
411410

412411
if (IsCheckedOut)
413412
{

‎src/GitHub.Exports/Models/ILocalRepositoryModel.cs‎

Lines changed: 0 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -11,10 +11,5 @@ public interface ILocalRepositoryModel : IRepositoryModel, INotifyPropertyChange
1111
/// Gets the path to the repository on the filesystem.
1212
/// </summary>
1313
string LocalPath { get; }
14-
15-
/// <summary>
16-
/// Gets the current branch.
17-
/// </summary>
18-
IBranch CurrentBranch { get; set; }
1914
}
2015
}

‎src/GitHub.Exports/Models/LocalRepositoryModel.cs‎

Lines changed: 0 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -18,14 +18,6 @@ public string LocalPath
1818
get; set;
1919
}
2020

21-
/// <summary>
22-
/// Gets the current branch of the repository.
23-
/// </summary>
24-
public IBranch CurrentBranch
25-
{
26-
get; set;
27-
}
28-
2921
/// <summary>
3022
/// Note: We don't consider CloneUrl a part of the hash code because it can change during the lifetime
3123
/// of a repository. Equals takes care of any hash collisions because of this

‎src/GitHub.Exports/Services/GitService.cs‎

Lines changed: 17 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -49,13 +49,27 @@ public ILocalRepositoryModel CreateLocalRepositoryModel(string localPath)
4949
Icon = Octicon.repo
5050
};
5151

52-
RefreshCurrentBranch(model);
53-
5452
return model;
5553
}
5654

55+
public IBranch CreateCurrentBranchModel(ILocalRepositoryModel model)
56+
{
57+
var localPath = model.LocalPath;
58+
using (var repo = GetRepository(localPath))
59+
{
60+
var headBranch = repo?.Head;
61+
if (headBranch == null)
62+
{
63+
return null;
64+
}
65+
66+
// BranchModel doesn't keep a reference to Repository
67+
return new BranchModel(headBranch, model, this);
68+
}
69+
}
70+
5771
/// <summary>
58-
/// Updates the CloneUrl and CurrentBranch from the local repository.
72+
/// Updates the CloneUrl from the "origin" remote.
5973
/// </summary>
6074
public void Refresh(ILocalRepositoryModel localRepositoryModel)
6175
{
@@ -64,8 +78,6 @@ public void Refresh(ILocalRepositoryModel localRepositoryModel)
6478
return;
6579

6680
localRepositoryModel.CloneUrl = GetUri(localPath);
67-
68-
RefreshCurrentBranch(localRepositoryModel);
6981
}
7082

7183
/// <summary>
@@ -200,19 +212,5 @@ public Task<string> GetLatestPushedSha(string path, string remote = "origin")
200212
}
201213
});
202214
}
203-
204-
void RefreshCurrentBranch(ILocalRepositoryModel model)
205-
{
206-
var localPath = model.LocalPath;
207-
using (var repo = GetRepository(localPath))
208-
{
209-
var headBranch = repo?.Head;
210-
if (headBranch != null)
211-
{
212-
// BranchModel doesn't keep a reference to Repository
213-
model.CurrentBranch = new BranchModel(headBranch, model, this);
214-
}
215-
}
216-
}
217215
}
218216
}

‎src/GitHub.Exports/Services/IGitService.cs‎

Lines changed: 8 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -15,7 +15,14 @@ public interface IGitService
1515
ILocalRepositoryModel CreateLocalRepositoryModel(string localPath);
1616

1717
/// <summary>
18-
/// Updates the CloneUrl information based on the local path.
18+
/// Creates a new branch model for the current branch.
19+
/// </summary>
20+
/// <param name="model">The <see cref="ILocalRepositoryModel" /> to create a current branch model for.</param>
21+
/// <returns>A new branch model.</returns>
22+
IBranch CreateCurrentBranchModel(ILocalRepositoryModel model);
23+
24+
/// <summary>
25+
/// Updates the CloneUrl information based on the "origin" remote.
1926
/// </summary>
2027
/// <param name="localRepositoryModel">The repository model to refresh.</param>
2128
void Refresh(ILocalRepositoryModel localRepositoryModel);

‎src/GitHub.VisualStudio/Commands/OpenFromClipboardCommand.cs‎

Lines changed: 9 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -4,7 +4,6 @@
44
using GitHub.Exports;
55
using GitHub.Services;
66
using GitHub.Services.Vssdk.Commands;
7-
using GitHub.VisualStudio.UI;
87
using Microsoft.VisualStudio.Shell;
98
using Task = System.Threading.Tasks.Task;
109

@@ -13,11 +12,12 @@ namespace GitHub.VisualStudio.Commands
1312
[Export(typeof(IOpenFromClipboardCommand))]
1413
public class OpenFromClipboardCommand : VsCommand<string>, IOpenFromClipboardCommand
1514
{
16-
15+
1716

1817
readonly Lazy<IGitHubContextService> gitHubContextService;
1918
readonly Lazy<ITeamExplorerContext> teamExplorerContext;
2019
readonly Lazy<IVSServices> vsServices;
20+
readonly Lazy<IGitService> gitService;
2121
readonly Lazy<UIContext> uiContext;
2222

2323
/// <summary>
@@ -34,12 +34,14 @@ public class OpenFromClipboardCommand : VsCommand<string>, IOpenFromClipboardCom
3434
public OpenFromClipboardCommand(
3535
Lazy<IGitHubContextService> gitHubContextService,
3636
Lazy<ITeamExplorerContext> teamExplorerContext,
37-
Lazy<IVSServices> vsServices)
37+
Lazy<IVSServices> vsServices,
38+
Lazy<IGitService> gitService)
3839
: base(CommandSet, CommandId)
3940
{
4041
this.gitHubContextService = gitHubContextService;
4142
this.teamExplorerContext = teamExplorerContext;
4243
this.vsServices = vsServices;
44+
this.gitService = gitService;
4345

4446
// See https://code.msdn.microsoft.com/windowsdesktop/AllowParams-2005-9442298f
4547
ParametersDescription = "u"; // accept a single url
@@ -96,9 +98,11 @@ public override async Task Execute(string url)
9698
var hasChanges = gitHubContextService.Value.HasChangesInWorkingDirectory(repositoryDir, commitish, path);
9799
if (hasChanges)
98100
{
99-
// AnnotateFile expects a branch name so we use the current branch
100-
var branchName = activeRepository.CurrentBranch.Name;
101+
// TODO: What if this returns null because we're not on a branch?
102+
var currentBranch = gitService.Value.CreateCurrentBranchModel(activeRepository);
103+
var branchName = currentBranch.Name;
101104

105+
// AnnotateFile expects a branch name so we use the current branch
102106
if (await gitHubContextService.Value.TryAnnotateFile(repositoryDir, branchName, context))
103107
{
104108
return;

‎test/GitHub.App.UnitTests/Services/TeamExplorerContextTests.cs‎

Lines changed: 7 additions & 35 deletions
Original file line numberDiff line numberDiff line change
@@ -151,45 +151,22 @@ public void NoActiveRepositoryChange_SolutionChanges()
151151

152152
public class TheStatusChangedEvent
153153
{
154-
[TestCase(false, "name1", "sha1", "name1", "sha1", false)]
155-
[TestCase(false, "name1", "sha1", "name2", "sha1", true)]
156-
[TestCase(false, "name1", "sha1", "name1", "sha2", true)]
157-
[TestCase(false, "name1", "sha1", "name2", "sha2", true)]
158-
[TestCase(true, "name1", "sha1", "name1", "sha1", false)]
159-
[TestCase(true, "name1", "sha1", "name2", "sha2", false)]
160-
public void SameActiveRepository_ExpectWasRaised(bool changePath, string name1, string sha1, string name2, string sha2, bool expectWasRaised)
154+
[TestCase(true, false)]
155+
[TestCase(false, true)]
156+
public void AlwaysFireWhenNoLocalPathChange(bool changePath, bool expectWasRaised)
161157
{
162158
var gitExt = CreateGitExt();
163159
var repositoryPaths = new[] { Directory.GetCurrentDirectory(), Path.GetTempPath() };
164160
var path1 = Directory.GetCurrentDirectory();
165161
var path2 = changePath ? Path.GetTempPath() : path1;
166-
var repoInfo1 = CreateRepositoryModel(path1, name1, sha1);
167-
var repoInfo2 = CreateRepositoryModel(path2, name2, sha2);
162+
var repoInfo1 = CreateRepositoryModel(path1);
163+
var repoInfo2 = CreateRepositoryModel(path2);
168164

169165
var target = CreateTeamExplorerContext(gitExt);
170-
var eventWasRaised = false;
171-
target.StatusChanged += (s, e) => eventWasRaised = true;
172166

173-
SetActiveRepository(gitExt, repoInfo1);
174-
SetActiveRepository(gitExt, repoInfo2);
175-
176-
Assert.That(eventWasRaised, Is.EqualTo(expectWasRaised));
177-
}
178-
179-
[TestCase("trackedSha", "trackedSha", false)]
180-
[TestCase("trackedSha1", "trackedSha2", true)]
181-
public void TrackedShaChanges_CheckWasRaised(string trackedSha1, string trackedSha2, bool expectWasRaised)
182-
{
183-
var gitExt = CreateGitExt();
184-
var repositoryPaths = new[] { Directory.GetCurrentDirectory(), Path.GetTempPath() };
185-
var repoPath = Directory.GetCurrentDirectory();
186-
var repoInfo1 = CreateRepositoryModel(repoPath, "name", "sha", trackedSha1);
187-
var repoInfo2 = CreateRepositoryModel(repoPath, "name", "sha", trackedSha2);
188-
var target = CreateTeamExplorerContext(gitExt);
189167
SetActiveRepository(gitExt, repoInfo1);
190168
var eventWasRaised = false;
191169
target.StatusChanged += (s, e) => eventWasRaised = true;
192-
193170
SetActiveRepository(gitExt, repoInfo2);
194171

195172
Assert.That(eventWasRaised, Is.EqualTo(expectWasRaised));
@@ -200,7 +177,7 @@ public void SolutionUnloadedAndReloaded_DontFireStatusChanged()
200177
{
201178
var gitExt = CreateGitExt();
202179
var path = Directory.GetCurrentDirectory();
203-
var repoInfo1 = CreateRepositoryModel(path, "name", "sha");
180+
var repoInfo1 = CreateRepositoryModel(path);
204181
var repoInfo2 = CreateRepositoryModel(null);
205182
var target = CreateTeamExplorerContext(gitExt);
206183
SetActiveRepository(gitExt, repoInfo1);
@@ -226,15 +203,10 @@ static TeamExplorerContext CreateTeamExplorerContext(
226203
return new TeamExplorerContext(gitExt, new AsyncLazy<DTE>(() => Task.FromResult(dte)), pullRequestService, joinableTaskContext);
227204
}
228205

229-
static ILocalRepositoryModel CreateRepositoryModel(string path, string branchName = null, string headSha = null, string trackedSha = null)
206+
static ILocalRepositoryModel CreateRepositoryModel(string path)
230207
{
231208
var repo = Substitute.For<ILocalRepositoryModel>();
232209
repo.LocalPath.Returns(path);
233-
var currentBranch = Substitute.For<IBranch>();
234-
currentBranch.Name.Returns(branchName);
235-
currentBranch.Sha.Returns(headSha);
236-
currentBranch.TrackedSha.Returns(trackedSha);
237-
repo.CurrentBranch.Returns(currentBranch);
238210
return repo;
239211
}
240212

0 commit comments

Comments
 (0)