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

Commit d7a8b69

Browse files
authored
Merge pull request #1292 from github/fixes/testable-teserviceholder
Making TeamExplorerServiceHolder testable
2 parents 63ceb38 + 0bb882a commit d7a8b69

File tree

7 files changed

+182
-28
lines changed

7 files changed

+182
-28
lines changed

src/GitHub.Exports/GitHub.Exports.csproj

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -265,6 +265,8 @@
265265
<Compile Include="Primitives\HostAddress.cs" />
266266
<Compile Include="UI\IUIController.cs" />
267267
<Compile Include="Models\IPullRequestModel.cs" />
268+
<Compile Include="Services\IVSGitExt.cs" />
269+
<Compile Include="Services\IVSUIContext.cs" />
268270
</ItemGroup>
269271
<ItemGroup>
270272
<ProjectReference Include="..\..\submodules\octokit.net\Octokit\Octokit.csproj">
Lines changed: 15 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,15 @@
1+
using GitHub.Models;
2+
using System;
3+
using System.Collections.Generic;
4+
using System.ComponentModel;
5+
using System.ComponentModel.Composition;
6+
7+
namespace GitHub.Services
8+
{
9+
public interface IVSGitExt
10+
{
11+
void Refresh(IServiceProvider serviceProvider);
12+
IEnumerable<ILocalRepositoryModel> ActiveRepositories { get; }
13+
event Action ActiveRepositoriesChanged;
14+
}
15+
}
Lines changed: 21 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,21 @@
1+
using Microsoft.VisualStudio.Shell;
2+
using System;
3+
4+
namespace GitHub.Services
5+
{
6+
public interface IVSUIContextFactory
7+
{
8+
IVSUIContext GetUIContext(Guid contextGuid);
9+
}
10+
11+
public interface IVSUIContextChangedEventArgs
12+
{
13+
bool Activated { get; }
14+
}
15+
16+
public interface IVSUIContext
17+
{
18+
bool IsActive { get; }
19+
event EventHandler<IVSUIContextChangedEventArgs> UIContextChanged;
20+
}
21+
}

src/GitHub.TeamFoundation.14/Base/TeamExplorerServiceHolder.cs

Lines changed: 103 additions & 28 deletions
Original file line numberDiff line numberDiff line change
@@ -26,14 +26,24 @@ public class TeamExplorerServiceHolder : ITeamExplorerServiceHolder
2626
bool activeRepoNotified = false;
2727

2828
IServiceProvider serviceProvider;
29-
IGitExt gitService;
30-
UIContext gitUIContext;
29+
IVSGitExt gitService;
30+
IVSUIContext gitUIContext;
31+
IVSUIContextFactory uiContextFactory;
3132

3233
// ActiveRepositories PropertyChanged event comes in on a non-main thread
3334
readonly SynchronizationContext syncContext;
3435

35-
public TeamExplorerServiceHolder()
36+
/// <summary>
37+
/// This class relies on IVSUIContextFactory to get the UIContext object that provides information
38+
/// when VS switches repositories. Unfortunately, for some reason MEF fails to create the instance
39+
/// when imported from the constructor, so it's imported manually when first accessed via the
40+
/// ServiceProvider instance (when mocking, make sure that the ServiceProvider includes this factory)
41+
/// </summary>
42+
/// <param name="gitService"></param>
43+
[ImportingConstructor]
44+
public TeamExplorerServiceHolder(IVSGitExt gitService)
3645
{
46+
this.GitService = gitService;
3747
syncContext = SynchronizationContext.Current;
3848
}
3949

@@ -49,7 +59,7 @@ public IServiceProvider ServiceProvider
4959
serviceProvider = value;
5060
if (serviceProvider == null)
5161
return;
52-
GitUIContext = GitUIContext ?? UIContext.FromUIContextGuid(new Guid(Guids.GitSccProviderId));
62+
GitUIContext = GitUIContext ?? UIContextFactory.GetUIContext(new Guid(Guids.GitSccProviderId));
5363
UIContextChanged(GitUIContext?.IsActive ?? false, false);
5464
}
5565
}
@@ -77,7 +87,7 @@ public void Subscribe(object who, Action<ILocalRepositoryModel> handler)
7787

7888
bool notificationsExist;
7989
ILocalRepositoryModel repo;
80-
lock(activeRepoHandlers)
90+
lock (activeRepoHandlers)
8191
{
8292
repo = ActiveRepo;
8393
notificationsExist = activeRepoNotified;
@@ -125,7 +135,7 @@ public void ClearServiceProvider(IServiceProvider provider)
125135

126136
public void Refresh()
127137
{
128-
GitUIContext = GitUIContext ?? UIContext.FromUIContextGuid(new Guid(Guids.GitSccProviderId));
138+
GitUIContext = GitUIContext ?? UIContextFactory.GetUIContext(new Guid(Guids.GitSccProviderId));
129139
UIContextChanged(GitUIContext?.IsActive ?? false, true);
130140
}
131141

@@ -139,14 +149,19 @@ void NotifyActiveRepo()
139149
}
140150
}
141151

142-
void UIContextChanged(object sender, UIContextChangedEventArgs e)
152+
void UIContextChanged(object sender, IVSUIContextChangedEventArgs e)
143153
{
144154
Guard.ArgumentNotNull(e, nameof(e));
145155

146156
ActiveRepo = null;
147157
UIContextChanged(e.Activated, false);
148158
}
149159

160+
/// <summary>
161+
/// This is called on a background thread. Do not do synchronous GetService calls here.
162+
/// </summary>
163+
/// <param name="active"></param>
164+
/// <param name="refresh"></param>
150165
async void UIContextChanged(bool active, bool refresh)
151166
{
152167
Debug.Assert(ServiceProvider != null, "UIContextChanged called before service provider is set");
@@ -155,42 +170,34 @@ async void UIContextChanged(bool active, bool refresh)
155170

156171
if (active)
157172
{
158-
GitService = GitService ?? ServiceProvider.GetServiceSafe<IGitExt>();
159173
if (ActiveRepo == null || refresh)
174+
{
160175
ActiveRepo = await System.Threading.Tasks.Task.Run(() =>
161176
{
162-
var repos = GitService?.ActiveRepositories;
177+
var repos = GitService.ActiveRepositories;
163178
// Looks like this might return null after a while, for some unknown reason
164179
// if it does, let's refresh the GitService instance in case something got wonky
165180
// and try again. See issue #23
166181
if (repos == null)
167182
{
168183
log.Error("Error 2001: ActiveRepositories is null. GitService: '{GitService}'", GitService);
169-
GitService = ServiceProvider?.GetServiceSafe<IGitExt>();
170-
repos = GitService?.ActiveRepositories;
184+
GitService.Refresh(ServiceProvider);
185+
repos = GitService.ActiveRepositories;
171186
if (repos == null)
172187
log.Error("Error 2002: ActiveRepositories is null. GitService: '{GitService}'", GitService);
173188
}
174-
return repos?.FirstOrDefault()?.ToModel();
189+
return repos?.FirstOrDefault();
175190
});
191+
}
176192
}
177193
else
178194
ActiveRepo = null;
179195
}
180196

181-
void CheckAndUpdate(object sender, System.ComponentModel.PropertyChangedEventArgs e)
197+
void UpdateActiveRepo()
182198
{
183-
Guard.ArgumentNotNull(e, nameof(e));
184-
185-
if (e.PropertyName != "ActiveRepositories")
186-
return;
187-
188-
var service = GitService;
189-
if (service == null)
190-
return;
191-
192-
var repo = service.ActiveRepositories.FirstOrDefault()?.ToModel();
193-
if (repo != ActiveRepo)
199+
var repo = gitService.ActiveRepositories.FirstOrDefault();
200+
if (!Equals(repo, ActiveRepo))
194201
// so annoying that this is on the wrong thread
195202
syncContext.Post(r => ActiveRepo = r as ILocalRepositoryModel, repo);
196203
}
@@ -221,7 +228,7 @@ ITeamExplorerPage PageService
221228
get { return ServiceProvider.GetServiceSafe<ITeamExplorerPage>(); }
222229
}
223230

224-
UIContext GitUIContext
231+
IVSUIContext GitUIContext
225232
{
226233
get { return gitUIContext; }
227234
set
@@ -236,18 +243,86 @@ UIContext GitUIContext
236243
}
237244
}
238245

239-
IGitExt GitService
246+
IVSGitExt GitService
240247
{
241248
get { return gitService; }
242249
set
243250
{
244251
if (gitService == value)
245252
return;
246253
if (gitService != null)
247-
gitService.PropertyChanged -= CheckAndUpdate;
254+
gitService.ActiveRepositoriesChanged -= UpdateActiveRepo;
248255
gitService = value;
249256
if (gitService != null)
250-
gitService.PropertyChanged += CheckAndUpdate;
257+
gitService.ActiveRepositoriesChanged += UpdateActiveRepo;
258+
}
259+
}
260+
261+
IVSUIContextFactory UIContextFactory
262+
{
263+
get
264+
{
265+
if (uiContextFactory == null)
266+
{
267+
uiContextFactory = ServiceProvider.GetServiceSafe<IVSUIContextFactory>();
268+
}
269+
return uiContextFactory;
270+
}
271+
}
272+
}
273+
274+
[Export(typeof(IVSUIContextFactory))]
275+
[PartCreationPolicy(CreationPolicy.Shared)]
276+
class VSUIContextFactory : IVSUIContextFactory
277+
{
278+
public IVSUIContext GetUIContext(Guid contextGuid)
279+
{
280+
return new VSUIContext(UIContext.FromUIContextGuid(contextGuid));
281+
}
282+
}
283+
284+
class VSUIContextChangedEventArgs : IVSUIContextChangedEventArgs
285+
{
286+
public bool Activated { get; }
287+
288+
public VSUIContextChangedEventArgs(bool activated)
289+
{
290+
Activated = activated;
291+
}
292+
}
293+
294+
class VSUIContext : IVSUIContext
295+
{
296+
readonly UIContext context;
297+
readonly Dictionary<EventHandler<IVSUIContextChangedEventArgs>, EventHandler<UIContextChangedEventArgs>> handlers =
298+
new Dictionary<EventHandler<IVSUIContextChangedEventArgs>, EventHandler<UIContextChangedEventArgs>>();
299+
public VSUIContext(UIContext context)
300+
{
301+
this.context = context;
302+
}
303+
304+
public bool IsActive { get { return context.IsActive; } }
305+
306+
public event EventHandler<IVSUIContextChangedEventArgs> UIContextChanged
307+
{
308+
add
309+
{
310+
EventHandler<UIContextChangedEventArgs> handler = null;
311+
if (!handlers.TryGetValue(value, out handler))
312+
{
313+
handler = (s, e) => value.Invoke(s, new VSUIContextChangedEventArgs(e.Activated));
314+
handlers.Add(value, handler);
315+
}
316+
context.UIContextChanged += handler;
317+
}
318+
remove
319+
{
320+
EventHandler<UIContextChangedEventArgs> handler = null;
321+
if (handlers.TryGetValue(value, out handler))
322+
{
323+
handlers.Remove(value);
324+
context.UIContextChanged -= handler;
325+
}
251326
}
252327
}
253328
}

src/GitHub.TeamFoundation.14/GitHub.TeamFoundation.14.csproj

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -131,6 +131,7 @@
131131
<Reference Include="WindowsBase" />
132132
</ItemGroup>
133133
<ItemGroup>
134+
<Compile Include="Services\VSGitExt.cs" />
134135
<Compile Include="RegistryHelper.cs" />
135136
<Compile Include="Services\VSGitServices.cs" />
136137
<Compile Include="Settings.cs" />
Lines changed: 39 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,39 @@
1+
using System;
2+
using System.Collections.Generic;
3+
using System.ComponentModel.Composition;
4+
using System.Linq;
5+
using GitHub.Extensions;
6+
using GitHub.Models;
7+
using GitHub.Services;
8+
using Microsoft.VisualStudio.TeamFoundation.Git.Extensibility;
9+
10+
namespace GitHub.VisualStudio.Base
11+
{
12+
[Export(typeof(IVSGitExt))]
13+
[PartCreationPolicy(CreationPolicy.Shared)]
14+
public class VSGitExt : IVSGitExt
15+
{
16+
IGitExt gitService;
17+
18+
public void Refresh(IServiceProvider serviceProvider)
19+
{
20+
if (gitService != null)
21+
gitService.PropertyChanged -= CheckAndUpdate;
22+
gitService = serviceProvider.GetServiceSafe<IGitExt>();
23+
if (gitService != null)
24+
gitService.PropertyChanged += CheckAndUpdate;
25+
}
26+
27+
28+
void CheckAndUpdate(object sender, System.ComponentModel.PropertyChangedEventArgs e)
29+
{
30+
Guard.ArgumentNotNull(e, nameof(e));
31+
if (e.PropertyName != "ActiveRepositories" || gitService == null)
32+
return;
33+
ActiveRepositoriesChanged?.Invoke();
34+
}
35+
36+
public IEnumerable<ILocalRepositoryModel> ActiveRepositories => gitService?.ActiveRepositories.Select(x => x.ToModel());
37+
public event Action ActiveRepositoriesChanged;
38+
}
39+
}

src/common/GitHubVS.ruleset

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,7 @@
66
<Rule Id="CA1002" Action="None" />
77
<Rule Id="CA1004" Action="Warning" />
88
<Rule Id="CA1006" Action="None" />
9+
<Rule Id="CA1009" Action="None" />
910
<Rule Id="CA1014" Action="None" />
1011
<Rule Id="CA1017" Action="None" />
1112
<Rule Id="CA1020" Action="None" />

0 commit comments

Comments
 (0)