From 323f33042dc24f824fb57175a05f443c263a7b22 Mon Sep 17 00:00:00 2001 From: Jamie Cansdale Date: Fri, 23 Feb 2018 10:52:35 +0000 Subject: [PATCH 01/18] Ensure consistent task ordering using ContinueWith Lock prevent tasks being executed at the same time but doesn't stop them being started out of order. --- .../Services/VSGitExt.cs | 33 ++++++++++--------- .../GitHub.TeamFoundation/VSGitExtTests.cs | 4 ++- 2 files changed, 21 insertions(+), 16 deletions(-) diff --git a/src/GitHub.TeamFoundation.14/Services/VSGitExt.cs b/src/GitHub.TeamFoundation.14/Services/VSGitExt.cs index 6dd1d5fd12..81f86e3328 100644 --- a/src/GitHub.TeamFoundation.14/Services/VSGitExt.cs +++ b/src/GitHub.TeamFoundation.14/Services/VSGitExt.cs @@ -1,5 +1,6 @@ using System; using System.Linq; +using System.Threading.Tasks; using System.Collections.Generic; using System.ComponentModel.Composition; using GitHub.Models; @@ -8,7 +9,6 @@ using GitHub.TeamFoundation.Services; using Serilog; using Microsoft.VisualStudio.TeamFoundation.Git.Extensibility; -using Task = System.Threading.Tasks.Task; namespace GitHub.VisualStudio.Base { @@ -24,7 +24,6 @@ public class VSGitExt : IVSGitExt readonly IGitHubServiceProvider serviceProvider; readonly IVSUIContext context; readonly ILocalRepositoryModelFactory repositoryFactory; - readonly object refreshLock = new object(); IGitExt gitService; IReadOnlyList activeRepositories; @@ -41,23 +40,24 @@ public VSGitExt(IGitHubServiceProvider serviceProvider, IVSUIContextFactory fact this.repositoryFactory = repositoryFactory; // The IGitExt service isn't available when a TFS based solution is opened directly. - // It will become available when moving to a Git based solution (cause a UIContext event to fire). + // It will become available when moving to a Git based solution (and cause a UIContext event to fire). context = factory.GetUIContext(new Guid(Guids.GitSccProviderId)); // Start with empty array until we have a change to initialize. ActiveRepositories = Array.Empty(); + InitializeTask = Task.CompletedTask; + if (context.IsActive && TryInitialize()) { // Refresh ActiveRepositories on background thread so we don't delay startup. - InitializeTask = Task.Run(() => RefreshActiveRepositories()); + QueueRefreshActiveRepositories(); } else { // If we're not in the UIContext or TryInitialize fails, have another go when the UIContext changes. context.UIContextChanged += ContextChanged; log.Debug("VSGitExt will be initialized later"); - InitializeTask = Task.CompletedTask; } } @@ -68,7 +68,7 @@ void ContextChanged(object sender, VSUIContextChangedEventArgs e) if (e.Activated && TryInitialize()) { // Refresh ActiveRepositories on background thread so we don't delay UI context change. - InitializeTask = Task.Run(() => RefreshActiveRepositories()); + QueueRefreshActiveRepositories(); context.UIContextChanged -= ContextChanged; log.Debug("Initialized VSGitExt on UIContextChanged"); } @@ -83,7 +83,7 @@ bool TryInitialize() { if (e.PropertyName == nameof(gitService.ActiveRepositories)) { - RefreshActiveRepositories(); + QueueRefreshActiveRepositories(); } }; @@ -95,19 +95,22 @@ bool TryInitialize() return false; } + void QueueRefreshActiveRepositories() + { + // Execute tasks in sequence on thread pool. + InitializeTask = InitializeTask.ContinueWith(_ => RefreshActiveRepositories(), TaskScheduler.Default); + } + void RefreshActiveRepositories() { try { - lock (refreshLock) - { - log.Debug( - "IGitExt.ActiveRepositories (#{Id}) returned {Repositories}", - gitService.GetHashCode(), - gitService?.ActiveRepositories.Select(x => x.RepositoryPath)); + log.Debug( + "IGitExt.ActiveRepositories (#{Id}) returned {Repositories}", + gitService.GetHashCode(), + gitService?.ActiveRepositories.Select(x => x.RepositoryPath)); - ActiveRepositories = gitService?.ActiveRepositories.Select(x => repositoryFactory.Create(x.RepositoryPath)).ToList(); - } + ActiveRepositories = gitService?.ActiveRepositories.Select(x => repositoryFactory.Create(x.RepositoryPath)).ToList(); } catch (Exception e) { diff --git a/test/UnitTests/GitHub.TeamFoundation/VSGitExtTests.cs b/test/UnitTests/GitHub.TeamFoundation/VSGitExtTests.cs index 153d6295fc..bc6ebca7a2 100644 --- a/test/UnitTests/GitHub.TeamFoundation/VSGitExtTests.cs +++ b/test/UnitTests/GitHub.TeamFoundation/VSGitExtTests.cs @@ -63,7 +63,7 @@ public void ActiveRepositories_ReadUsingThreadPoolThread() public class TheActiveRepositoriesChangedEvent { [Test] - public void GitExtPropertyChangedEvent_ActiveRepositoriesChangedIsFired() + public async Task GitExtPropertyChangedEvent_ActiveRepositoriesChangedIsFired() { var context = CreateVSUIContext(true); var gitExt = CreateGitExt(); @@ -75,6 +75,7 @@ public void GitExtPropertyChangedEvent_ActiveRepositoriesChangedIsFired() var eventArgs = new PropertyChangedEventArgs(nameof(gitExt.ActiveRepositories)); gitExt.PropertyChanged += Raise.Event(gitExt, eventArgs); + await target.InitializeTask; Assert.That(wasFired, Is.True); } @@ -188,6 +189,7 @@ public async Task ActiveRepositoriesChangedOrderingShouldBeCorrectAcrossThreads( var task2 = Task.Run(() => gitExt.ActiveRepositories = activeRepositories2); await Task.WhenAll(task1, task2); + await target.InitializeTask; Assert.That(target.ActiveRepositories.Single().LocalPath, Is.EqualTo("repo2")); } From 5200e72947a9fb68fe10d848f031afd3e498c5d6 Mon Sep 17 00:00:00 2001 From: Jamie Cansdale Date: Fri, 23 Feb 2018 11:03:08 +0000 Subject: [PATCH 02/18] Rename InitializeTask to PendingTasks PendingTasks is used to execute tasks in order on the thread pool. --- src/GitHub.TeamFoundation.14/Services/VSGitExt.cs | 13 +++++++------ .../GitHub.TeamFoundation/VSGitExtTests.cs | 14 +++++++------- 2 files changed, 14 insertions(+), 13 deletions(-) diff --git a/src/GitHub.TeamFoundation.14/Services/VSGitExt.cs b/src/GitHub.TeamFoundation.14/Services/VSGitExt.cs index 81f86e3328..d98e3edda5 100644 --- a/src/GitHub.TeamFoundation.14/Services/VSGitExt.cs +++ b/src/GitHub.TeamFoundation.14/Services/VSGitExt.cs @@ -43,11 +43,9 @@ public VSGitExt(IGitHubServiceProvider serviceProvider, IVSUIContextFactory fact // It will become available when moving to a Git based solution (and cause a UIContext event to fire). context = factory.GetUIContext(new Guid(Guids.GitSccProviderId)); - // Start with empty array until we have a change to initialize. + // Start with empty array until we have a chance to initialize. ActiveRepositories = Array.Empty(); - InitializeTask = Task.CompletedTask; - if (context.IsActive && TryInitialize()) { // Refresh ActiveRepositories on background thread so we don't delay startup. @@ -97,8 +95,8 @@ bool TryInitialize() void QueueRefreshActiveRepositories() { - // Execute tasks in sequence on thread pool. - InitializeTask = InitializeTask.ContinueWith(_ => RefreshActiveRepositories(), TaskScheduler.Default); + // Execute tasks in sequence using thread pool (TaskScheduler.Default). + PendingTasks = PendingTasks.ContinueWith(_ => RefreshActiveRepositories(), TaskScheduler.Default); } void RefreshActiveRepositories() @@ -139,6 +137,9 @@ private set public event Action ActiveRepositoriesChanged; - public Task InitializeTask { get; private set; } + /// + /// Tasks that are pending execution on the thread pool. + /// + public Task PendingTasks { get; private set; } = Task.CompletedTask; } } \ No newline at end of file diff --git a/test/UnitTests/GitHub.TeamFoundation/VSGitExtTests.cs b/test/UnitTests/GitHub.TeamFoundation/VSGitExtTests.cs index bc6ebca7a2..fb7c7f82ae 100644 --- a/test/UnitTests/GitHub.TeamFoundation/VSGitExtTests.cs +++ b/test/UnitTests/GitHub.TeamFoundation/VSGitExtTests.cs @@ -75,7 +75,7 @@ public async Task GitExtPropertyChangedEvent_ActiveRepositoriesChangedIsFired() var eventArgs = new PropertyChangedEventArgs(nameof(gitExt.ActiveRepositories)); gitExt.PropertyChanged += Raise.Event(gitExt, eventArgs); - await target.InitializeTask; + await target.PendingTasks; Assert.That(wasFired, Is.True); } @@ -109,7 +109,7 @@ public void WhenUIContextChanged_ActiveRepositoriesChangedIsFired() var eventArgs = new VSUIContextChangedEventArgs(true); context.UIContextChanged += Raise.Event>(context, eventArgs); - target.InitializeTask.Wait(); + target.PendingTasks.Wait(); Assert.That(wasFired, Is.True); } @@ -126,7 +126,7 @@ public void WhenUIContextChanged_FiredUsingThreadPoolThread() var eventArgs = new VSUIContextChangedEventArgs(true); context.UIContextChanged += Raise.Event>(context, eventArgs); - target.InitializeTask.Wait(); + target.PendingTasks.Wait(); Assert.That(threadPool, Is.True); } @@ -151,7 +151,7 @@ public void SccProviderContextIsActive_InitializeWithActiveRepositories() var context = CreateVSUIContext(true); var gitExt = CreateGitExt(new[] { repoPath }); var target = CreateVSGitExt(context, gitExt, repoFactory: repoFactory); - target.InitializeTask.Wait(); + target.PendingTasks.Wait(); var activeRepositories = target.ActiveRepositories; @@ -168,7 +168,7 @@ public void ExceptionRefreshingRepositories_ReturnsEmptyList() var context = CreateVSUIContext(true); var gitExt = CreateGitExt(new[] { repoPath }); var target = CreateVSGitExt(context, gitExt, repoFactory: repoFactory); - target.InitializeTask.Wait(); + target.PendingTasks.Wait(); var activeRepositories = target.ActiveRepositories; @@ -189,7 +189,7 @@ public async Task ActiveRepositoriesChangedOrderingShouldBeCorrectAcrossThreads( var task2 = Task.Run(() => gitExt.ActiveRepositories = activeRepositories2); await Task.WhenAll(task1, task2); - await target.InitializeTask; + await target.PendingTasks; Assert.That(target.ActiveRepositories.Single().LocalPath, Is.EqualTo("repo2")); } @@ -221,7 +221,7 @@ static VSGitExt CreateVSGitExt(IVSUIContext context = null, IGitExt gitExt = nul sp.GetService().Returns(factory); sp.GetService().Returns(gitExt); var vsGitExt = new VSGitExt(sp, factory, repoFactory); - vsGitExt.InitializeTask.Wait(); + vsGitExt.PendingTasks.Wait(); return vsGitExt; } From 68d098a8906eb26d204ecda6745b54f1da979e40 Mon Sep 17 00:00:00 2001 From: Jamie Cansdale Date: Mon, 26 Feb 2018 12:12:53 +0000 Subject: [PATCH 03/18] Do initialization asynchronously on Main thread This will allow this service to be used by a package that is initializing on a background thread. --- .../Services/VSGitExt.cs | 68 ++++++++++++------- .../GitHub.TeamFoundation/VSGitExtTests.cs | 7 +- 2 files changed, 48 insertions(+), 27 deletions(-) diff --git a/src/GitHub.TeamFoundation.14/Services/VSGitExt.cs b/src/GitHub.TeamFoundation.14/Services/VSGitExt.cs index d98e3edda5..835db8214d 100644 --- a/src/GitHub.TeamFoundation.14/Services/VSGitExt.cs +++ b/src/GitHub.TeamFoundation.14/Services/VSGitExt.cs @@ -6,6 +6,7 @@ using GitHub.Models; using GitHub.Services; using GitHub.Logging; +using GitHub.Helpers; using GitHub.TeamFoundation.Services; using Serilog; using Microsoft.VisualStudio.TeamFoundation.Git.Extensibility; @@ -46,34 +47,57 @@ public VSGitExt(IGitHubServiceProvider serviceProvider, IVSUIContextFactory fact // Start with empty array until we have a chance to initialize. ActiveRepositories = Array.Empty(); - if (context.IsActive && TryInitialize()) + PendingTasks = InitializeAsync(); + } + + async Task InitializeAsync() + { + try { - // Refresh ActiveRepositories on background thread so we don't delay startup. - QueueRefreshActiveRepositories(); + if (!context.IsActive || !await TryInitialize()) + { + // If we're not in the UIContext or TryInitialize fails, have another go when the UIContext changes. + context.UIContextChanged += ContextChanged; + log.Debug("VSGitExt will be initialized later"); + } } - else + catch (Exception e) { - // If we're not in the UIContext or TryInitialize fails, have another go when the UIContext changes. - context.UIContextChanged += ContextChanged; - log.Debug("VSGitExt will be initialized later"); + log.Error(e, "Initializing"); } } void ContextChanged(object sender, VSUIContextChangedEventArgs e) { - // If we're in the UIContext and TryInitialize succeeds, we can stop listening for events. - // NOTE: this event can fire with UIContext=true in a TFS solution (not just Git). - if (e.Activated && TryInitialize()) + if (e.Activated) { - // Refresh ActiveRepositories on background thread so we don't delay UI context change. - QueueRefreshActiveRepositories(); - context.UIContextChanged -= ContextChanged; - log.Debug("Initialized VSGitExt on UIContextChanged"); + PendingTasks = ContextChangedAsync(); } } - bool TryInitialize() + async Task ContextChangedAsync() { + try + { + // If we're in the UIContext and TryInitialize succeeds, we can stop listening for events. + // NOTE: this event can fire with UIContext=true in a TFS solution (not just Git). + if (await TryInitialize()) + { + context.UIContextChanged -= ContextChanged; + log.Debug("Initialized VSGitExt on UIContextChanged"); + } + } + catch (Exception e) + { + log.Error(e, "UIContextChanged"); + } + } + + async Task TryInitialize() + { + // GetService must be called on the Main thread. + await ThreadingHelper.SwitchToMainThreadAsync(); + gitService = serviceProvider.GetService(); if (gitService != null) { @@ -81,10 +105,14 @@ bool TryInitialize() { if (e.PropertyName == nameof(gitService.ActiveRepositories)) { - QueueRefreshActiveRepositories(); + // Execute tasks in sequence using thread pool (TaskScheduler.Default). + PendingTasks = PendingTasks.ContinueWith(_ => RefreshActiveRepositories(), TaskScheduler.Default); } }; + // Do this after we start listening so we don't miss an event. + await Task.Run(() => RefreshActiveRepositories()); + log.Debug("Found IGitExt service and initialized VSGitExt"); return true; } @@ -93,12 +121,6 @@ bool TryInitialize() return false; } - void QueueRefreshActiveRepositories() - { - // Execute tasks in sequence using thread pool (TaskScheduler.Default). - PendingTasks = PendingTasks.ContinueWith(_ => RefreshActiveRepositories(), TaskScheduler.Default); - } - void RefreshActiveRepositories() { try @@ -140,6 +162,6 @@ private set /// /// Tasks that are pending execution on the thread pool. /// - public Task PendingTasks { get; private set; } = Task.CompletedTask; + public Task PendingTasks { get; private set; } } } \ No newline at end of file diff --git a/test/UnitTests/GitHub.TeamFoundation/VSGitExtTests.cs b/test/UnitTests/GitHub.TeamFoundation/VSGitExtTests.cs index fb7c7f82ae..7dfe6543c2 100644 --- a/test/UnitTests/GitHub.TeamFoundation/VSGitExtTests.cs +++ b/test/UnitTests/GitHub.TeamFoundation/VSGitExtTests.cs @@ -11,11 +11,10 @@ using Microsoft.VisualStudio.TeamFoundation.Git.Extensibility; using System.Threading.Tasks; using System.Linq; -using GitHub.TeamFoundation.Services; public class VSGitExtTests { - public class TheConstructor + public class TheConstructor : TestBaseClass { [TestCase(true, 1)] [TestCase(false, 0)] @@ -60,7 +59,7 @@ public void ActiveRepositories_ReadUsingThreadPoolThread() } } - public class TheActiveRepositoriesChangedEvent + public class TheActiveRepositoriesChangedEvent : TestBaseClass { [Test] public async Task GitExtPropertyChangedEvent_ActiveRepositoriesChangedIsFired() @@ -132,7 +131,7 @@ public void WhenUIContextChanged_FiredUsingThreadPoolThread() } } - public class TheActiveRepositoriesProperty + public class TheActiveRepositoriesProperty : TestBaseClass { [Test] public void SccProviderContextNotActive_IsEmpty() From dcd806fb80f7939dd173424c0f742d5964eb217b Mon Sep 17 00:00:00 2001 From: Jamie Cansdale Date: Mon, 26 Feb 2018 14:45:01 +0000 Subject: [PATCH 04/18] Add comment about using from background thread --- src/GitHub.TeamFoundation.14/Services/VSGitExt.cs | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/src/GitHub.TeamFoundation.14/Services/VSGitExt.cs b/src/GitHub.TeamFoundation.14/Services/VSGitExt.cs index 835db8214d..1d68f100f0 100644 --- a/src/GitHub.TeamFoundation.14/Services/VSGitExt.cs +++ b/src/GitHub.TeamFoundation.14/Services/VSGitExt.cs @@ -16,6 +16,10 @@ namespace GitHub.VisualStudio.Base /// /// This service acts as an always available version of . /// + /// + /// Initialization for this service will be done asynchronously and the service will be + /// retrieved on the Main thread. This means the service to be constructed and subscribed to on a background thread. + /// [Export(typeof(IVSGitExt))] [PartCreationPolicy(CreationPolicy.Shared)] public class VSGitExt : IVSGitExt From 2a3cef5db0a427940758a4075b3153fb8a25ca25 Mon Sep 17 00:00:00 2001 From: Jamie Cansdale Date: Mon, 26 Feb 2018 15:12:00 +0000 Subject: [PATCH 05/18] Factor out GetServiceAsync --- src/GitHub.TeamFoundation.14/Services/VSGitExt.cs | 12 ++++++++---- 1 file changed, 8 insertions(+), 4 deletions(-) diff --git a/src/GitHub.TeamFoundation.14/Services/VSGitExt.cs b/src/GitHub.TeamFoundation.14/Services/VSGitExt.cs index 1d68f100f0..4956c270f6 100644 --- a/src/GitHub.TeamFoundation.14/Services/VSGitExt.cs +++ b/src/GitHub.TeamFoundation.14/Services/VSGitExt.cs @@ -99,10 +99,7 @@ async Task ContextChangedAsync() async Task TryInitialize() { - // GetService must be called on the Main thread. - await ThreadingHelper.SwitchToMainThreadAsync(); - - gitService = serviceProvider.GetService(); + gitService = await GetServiceAsync(); if (gitService != null) { gitService.PropertyChanged += (s, e) => @@ -125,6 +122,13 @@ async Task TryInitialize() return false; } + async Task GetServiceAsync() where T : class + { + // GetService must be called from the Main thread. + await ThreadingHelper.SwitchToMainThreadAsync(); + return serviceProvider.GetService(); + } + void RefreshActiveRepositories() { try From 16aabcd600ba8b999a084c1a5ac9f2d73d71f246 Mon Sep 17 00:00:00 2001 From: Jamie Cansdale Date: Tue, 27 Feb 2018 09:40:48 +0000 Subject: [PATCH 06/18] Make grammarful English --- src/GitHub.TeamFoundation.14/Services/VSGitExt.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/GitHub.TeamFoundation.14/Services/VSGitExt.cs b/src/GitHub.TeamFoundation.14/Services/VSGitExt.cs index 4956c270f6..19205a5e5f 100644 --- a/src/GitHub.TeamFoundation.14/Services/VSGitExt.cs +++ b/src/GitHub.TeamFoundation.14/Services/VSGitExt.cs @@ -18,7 +18,7 @@ namespace GitHub.VisualStudio.Base /// /// /// Initialization for this service will be done asynchronously and the service will be - /// retrieved on the Main thread. This means the service to be constructed and subscribed to on a background thread. + /// retrieved on the Main thread. This means the service can be constructed and subscribed to on a background thread. /// [Export(typeof(IVSGitExt))] [PartCreationPolicy(CreationPolicy.Shared)] From f23fd248b18874d64f30917d14a7d2e0055797f5 Mon Sep 17 00:00:00 2001 From: Jamie Cansdale Date: Tue, 27 Feb 2018 21:08:15 +0000 Subject: [PATCH 07/18] Expose IVSGitExt as async VS service --- .../Services/IGitHubServiceProvider.cs | 3 ++ .../PullRequestStatusBarPackage.cs | 9 +--- .../Services/VSGitExt.cs | 12 +----- .../GitHub.VisualStudio.csproj | 2 + src/GitHub.VisualStudio/GitHubPackage.cs | 42 ++++++++++++++++++- .../Services/GitHubServiceProvider.cs | 4 ++ 6 files changed, 53 insertions(+), 19 deletions(-) diff --git a/src/GitHub.Exports/Services/IGitHubServiceProvider.cs b/src/GitHub.Exports/Services/IGitHubServiceProvider.cs index c1d142ed7f..000301d0ce 100644 --- a/src/GitHub.Exports/Services/IGitHubServiceProvider.cs +++ b/src/GitHub.Exports/Services/IGitHubServiceProvider.cs @@ -1,4 +1,5 @@ using System; +using System.Threading.Tasks; using System.ComponentModel.Composition.Hosting; using System.Runtime.InteropServices; using GitHub.VisualStudio; @@ -28,5 +29,7 @@ Ret GetService() where T : class /// The owner, which either has to match what was passed to AddService, /// or if it's null, the service will be removed without checking for ownership void RemoveService(Type t, object owner); + + Task GetServiceAsync(Type serviceType); } } diff --git a/src/GitHub.InlineReviews/PullRequestStatusBarPackage.cs b/src/GitHub.InlineReviews/PullRequestStatusBarPackage.cs index 1573a10b7f..8b04edb866 100644 --- a/src/GitHub.InlineReviews/PullRequestStatusBarPackage.cs +++ b/src/GitHub.InlineReviews/PullRequestStatusBarPackage.cs @@ -1,5 +1,4 @@ using System; -using System.Windows; using System.Threading; using System.Runtime.InteropServices; using GitHub.Services; @@ -22,13 +21,9 @@ protected override async Task InitializeAsync(CancellationToken cancellationToke { var usageTracker = (IUsageTracker)await GetServiceAsync(typeof(IUsageTracker)); var serviceProvider = (IGitHubServiceProvider)await GetServiceAsync(typeof(IGitHubServiceProvider)); + var gitExt = (IVSGitExt)await GetServiceAsync(typeof(IVSGitExt)); - // NOTE: ThreadingHelper.SwitchToMainThreadAsync() doesn't return until a solution is loaded. Using Dispatcher.Invoke instead. - Application.Current.Dispatcher.Invoke(() => - { - var gitExt = serviceProvider.GetService(); - new PullRequestStatusBarManager(gitExt, usageTracker, serviceProvider); - }); + new PullRequestStatusBarManager(gitExt, usageTracker, serviceProvider); } } } diff --git a/src/GitHub.TeamFoundation.14/Services/VSGitExt.cs b/src/GitHub.TeamFoundation.14/Services/VSGitExt.cs index 19205a5e5f..478aa5fcbe 100644 --- a/src/GitHub.TeamFoundation.14/Services/VSGitExt.cs +++ b/src/GitHub.TeamFoundation.14/Services/VSGitExt.cs @@ -6,7 +6,6 @@ using GitHub.Models; using GitHub.Services; using GitHub.Logging; -using GitHub.Helpers; using GitHub.TeamFoundation.Services; using Serilog; using Microsoft.VisualStudio.TeamFoundation.Git.Extensibility; @@ -20,8 +19,6 @@ namespace GitHub.VisualStudio.Base /// Initialization for this service will be done asynchronously and the service will be /// retrieved on the Main thread. This means the service can be constructed and subscribed to on a background thread. /// - [Export(typeof(IVSGitExt))] - [PartCreationPolicy(CreationPolicy.Shared)] public class VSGitExt : IVSGitExt { static readonly ILogger log = LogManager.ForContext(); @@ -99,7 +96,7 @@ async Task ContextChangedAsync() async Task TryInitialize() { - gitService = await GetServiceAsync(); + gitService = (IGitExt)await serviceProvider.GetServiceAsync(typeof(IGitExt)); if (gitService != null) { gitService.PropertyChanged += (s, e) => @@ -122,13 +119,6 @@ async Task TryInitialize() return false; } - async Task GetServiceAsync() where T : class - { - // GetService must be called from the Main thread. - await ThreadingHelper.SwitchToMainThreadAsync(); - return serviceProvider.GetService(); - } - void RefreshActiveRepositories() { try diff --git a/src/GitHub.VisualStudio/GitHub.VisualStudio.csproj b/src/GitHub.VisualStudio/GitHub.VisualStudio.csproj index 18f6d03a44..c7d3879d04 100644 --- a/src/GitHub.VisualStudio/GitHub.VisualStudio.csproj +++ b/src/GitHub.VisualStudio/GitHub.VisualStudio.csproj @@ -679,6 +679,7 @@ True BuiltProjectOutputGroup;GetCopyToOutputDirectoryItems;DebugSymbolsProjectOutputGroup; DebugSymbolsProjectOutputGroup; + TF14 {161dbf01-1dbf-4b00-8551-c5c00f26720e} @@ -686,6 +687,7 @@ True BuiltProjectOutputGroup;GetCopyToOutputDirectoryItems;DebugSymbolsProjectOutputGroup; DebugSymbolsProjectOutputGroup; + TF15 {158b05e8-fdbc-4d71-b871-c96e28d5adf5} diff --git a/src/GitHub.VisualStudio/GitHubPackage.cs b/src/GitHub.VisualStudio/GitHubPackage.cs index bcd3c2271a..1503b34fea 100644 --- a/src/GitHub.VisualStudio/GitHubPackage.cs +++ b/src/GitHub.VisualStudio/GitHubPackage.cs @@ -1,4 +1,7 @@ -using System; +extern alias TF14; +extern alias TF15; + +using System; using System.ComponentModel.Composition; using System.Diagnostics; using System.Reflection; @@ -22,6 +25,8 @@ using Octokit; using Serilog; using Task = System.Threading.Tasks.Task; +using VSGitExt14 = TF14.GitHub.VisualStudio.Base.VSGitExt; +using VSGitExt15 = TF15.GitHub.VisualStudio.Base.VSGitExt; namespace GitHub.VisualStudio { @@ -107,12 +112,28 @@ public GHClient(IProgram program) } } + [PartCreationPolicy(CreationPolicy.Shared)] + public class VSGitExtPart + { + readonly IGitHubServiceProvider serviceProvider; + + [ImportingConstructor] + public VSGitExtPart(IGitHubServiceProvider serviceProvider) + { + this.serviceProvider = serviceProvider; + } + + [Export(typeof(IVSGitExt))] + public IVSGitExt VSGitExt => serviceProvider.GetService(); + } + [PackageRegistration(UseManagedResourcesOnly = true, AllowsBackgroundLoading = true)] [ProvideService(typeof(ILoginManager), IsAsyncQueryable = true)] [ProvideService(typeof(IMenuProvider), IsAsyncQueryable = true)] [ProvideService(typeof(IGitHubServiceProvider), IsAsyncQueryable = true)] [ProvideService(typeof(IUsageTracker), IsAsyncQueryable = true)] [ProvideService(typeof(IUsageService), IsAsyncQueryable = true)] + [ProvideService(typeof(IVSGitExt), IsAsyncQueryable = true)] [ProvideService(typeof(IGitHubToolWindowManager))] [Guid(ServiceProviderPackageId)] public sealed class ServiceProviderPackage : AsyncPackage, IServiceProviderPackage, IGitHubToolWindowManager @@ -150,6 +171,7 @@ Version VSVersion protected override Task InitializeAsync(CancellationToken cancellationToken, IProgress progress) { AddService(typeof(IGitHubServiceProvider), CreateService, true); + AddService(typeof(IVSGitExt), CreateService, true); AddService(typeof(IUsageTracker), CreateService, true); AddService(typeof(IUsageService), CreateService, true); AddService(typeof(ILoginManager), CreateService, true); @@ -258,6 +280,11 @@ async Task CreateService(IAsyncServiceContainer container, CancellationT var serviceProvider = await GetServiceAsync(typeof(IGitHubServiceProvider)) as IGitHubServiceProvider; return new UsageTracker(serviceProvider, usageService); } + else if (serviceType == typeof(IVSGitExt)) + { + var sp = await GetServiceAsync(typeof(IGitHubServiceProvider)) as IGitHubServiceProvider; + return CreateVSGitExt(sp); + } else if (serviceType == typeof(IGitHubToolWindowManager)) { return this; @@ -269,5 +296,18 @@ async Task CreateService(IAsyncServiceContainer container, CancellationT return sp.TryGetService(serviceType); } } + + IVSGitExt CreateVSGitExt(IGitHubServiceProvider sp) + { + switch (VSVersion.Major) + { + case 14: + return new Lazy(() => new VSGitExt14(sp)).Value; + case 15: + return new Lazy(() => new VSGitExt15(sp)).Value; + default: + return null; + } + } } } diff --git a/src/GitHub.VisualStudio/Services/GitHubServiceProvider.cs b/src/GitHub.VisualStudio/Services/GitHubServiceProvider.cs index 898df8857a..dbd7ba1640 100644 --- a/src/GitHub.VisualStudio/Services/GitHubServiceProvider.cs +++ b/src/GitHub.VisualStudio/Services/GitHubServiceProvider.cs @@ -71,6 +71,8 @@ public IServiceProvider GitServiceProvider public object TryGetService(Type t) => theRealProvider.TryGetService(t); public T TryGetService() where T : class => theRealProvider.TryGetService(); + + public Task GetServiceAsync(Type serviceType) => theRealProvider.GetServiceAsync(serviceType); } /// @@ -309,5 +311,7 @@ public void Dispose() Dispose(true); GC.SuppressFinalize(this); } + + public Task GetServiceAsync(Type serviceType) => asyncServiceProvider.GetServiceAsync(serviceType); } } From 921f8c57aaf88f58189ac0c9620b414631353b7a Mon Sep 17 00:00:00 2001 From: Jamie Cansdale Date: Wed, 28 Feb 2018 10:59:48 +0000 Subject: [PATCH 08/18] Add generic GetServiceAsync to IGitHubServiceProvider Keep it consistent with GetService and GetService. Fix failing VSGitExt unit tests. --- src/GitHub.Exports/Services/IGitHubServiceProvider.cs | 6 +++++- src/GitHub.TeamFoundation.14/Services/VSGitExt.cs | 2 +- src/GitHub.VisualStudio/Services/GitHubServiceProvider.cs | 7 +++++-- test/UnitTests/GitHub.TeamFoundation/VSGitExtTests.cs | 6 +++--- 4 files changed, 14 insertions(+), 7 deletions(-) diff --git a/src/GitHub.Exports/Services/IGitHubServiceProvider.cs b/src/GitHub.Exports/Services/IGitHubServiceProvider.cs index 000301d0ce..3c0809b4cc 100644 --- a/src/GitHub.Exports/Services/IGitHubServiceProvider.cs +++ b/src/GitHub.Exports/Services/IGitHubServiceProvider.cs @@ -1,5 +1,6 @@ using System; using System.Threading.Tasks; +using System.Diagnostics.CodeAnalysis; using System.ComponentModel.Composition.Hosting; using System.Runtime.InteropServices; using GitHub.VisualStudio; @@ -30,6 +31,9 @@ Ret GetService() where T : class /// or if it's null, the service will be removed without checking for ownership void RemoveService(Type t, object owner); - Task GetServiceAsync(Type serviceType); + Task GetServiceAsync() where T : class; + + [SuppressMessage("Microsoft.Design", "CA1004:GenericMethodsShouldProvideTypeParameter")] + Task GetServiceAsync() where T : class where Ret : class; } } diff --git a/src/GitHub.TeamFoundation.14/Services/VSGitExt.cs b/src/GitHub.TeamFoundation.14/Services/VSGitExt.cs index 478aa5fcbe..1b852dfa89 100644 --- a/src/GitHub.TeamFoundation.14/Services/VSGitExt.cs +++ b/src/GitHub.TeamFoundation.14/Services/VSGitExt.cs @@ -96,7 +96,7 @@ async Task ContextChangedAsync() async Task TryInitialize() { - gitService = (IGitExt)await serviceProvider.GetServiceAsync(typeof(IGitExt)); + gitService = await serviceProvider.GetServiceAsync(); if (gitService != null) { gitService.PropertyChanged += (s, e) => diff --git a/src/GitHub.VisualStudio/Services/GitHubServiceProvider.cs b/src/GitHub.VisualStudio/Services/GitHubServiceProvider.cs index dbd7ba1640..f9e3d2edef 100644 --- a/src/GitHub.VisualStudio/Services/GitHubServiceProvider.cs +++ b/src/GitHub.VisualStudio/Services/GitHubServiceProvider.cs @@ -72,7 +72,9 @@ public IServiceProvider GitServiceProvider public T TryGetService() where T : class => theRealProvider.TryGetService(); - public Task GetServiceAsync(Type serviceType) => theRealProvider.GetServiceAsync(serviceType); + public Task GetServiceAsync() where T : class => theRealProvider.GetServiceAsync(); + + public Task GetServiceAsync() where T : class where Ret : class => theRealProvider.GetServiceAsync(); } /// @@ -312,6 +314,7 @@ public void Dispose() GC.SuppressFinalize(this); } - public Task GetServiceAsync(Type serviceType) => asyncServiceProvider.GetServiceAsync(serviceType); + public async Task GetServiceAsync() where T : class => (T)await asyncServiceProvider.GetServiceAsync(typeof(T)); + public async Task GetServiceAsync() where T : class where Ret : class => (Ret)await asyncServiceProvider.GetServiceAsync(typeof(T)); } } diff --git a/test/UnitTests/GitHub.TeamFoundation/VSGitExtTests.cs b/test/UnitTests/GitHub.TeamFoundation/VSGitExtTests.cs index 7dfe6543c2..579f6b6a52 100644 --- a/test/UnitTests/GitHub.TeamFoundation/VSGitExtTests.cs +++ b/test/UnitTests/GitHub.TeamFoundation/VSGitExtTests.cs @@ -25,7 +25,7 @@ public void GetServiceIGitExt_WhenSccProviderContextIsActive(bool isActive, int var target = CreateVSGitExt(context, sp: sp); - sp.Received(expectCalls).GetService(); + sp.Received(expectCalls).GetServiceAsync(); } [TestCase(true, 1)] @@ -39,7 +39,7 @@ public void GetServiceIGitExt_WhenUIContextChanged(bool activated, int expectCal var eventArgs = new VSUIContextChangedEventArgs(activated); context.UIContextChanged += Raise.Event>(context, eventArgs); - sp.Received(expectCalls).GetService(); + sp.Received(expectCalls).GetServiceAsync(); } [Test] @@ -218,7 +218,7 @@ static VSGitExt CreateVSGitExt(IVSUIContext context = null, IGitExt gitExt = nul var contextGuid = new Guid(Guids.GitSccProviderId); factory.GetUIContext(contextGuid).Returns(context); sp.GetService().Returns(factory); - sp.GetService().Returns(gitExt); + sp.GetServiceAsync().Returns(gitExt); var vsGitExt = new VSGitExt(sp, factory, repoFactory); vsGitExt.PendingTasks.Wait(); return vsGitExt; From ace5498f66a2455797853dfb945e520a4c4468f1 Mon Sep 17 00:00:00 2001 From: Jamie Cansdale Date: Wed, 28 Feb 2018 12:01:21 +0000 Subject: [PATCH 09/18] Use DTE.Version to choose IVSGitExt implementaiton --- src/GitHub.VisualStudio/GitHubPackage.cs | 14 ++++++++------ 1 file changed, 8 insertions(+), 6 deletions(-) diff --git a/src/GitHub.VisualStudio/GitHubPackage.cs b/src/GitHub.VisualStudio/GitHubPackage.cs index 1503b34fea..e01b1aab15 100644 --- a/src/GitHub.VisualStudio/GitHubPackage.cs +++ b/src/GitHub.VisualStudio/GitHubPackage.cs @@ -4,7 +4,6 @@ using System; using System.ComponentModel.Composition; using System.Diagnostics; -using System.Reflection; using System.Runtime.InteropServices; using System.Threading; using System.Threading.Tasks; @@ -22,6 +21,7 @@ using Microsoft.VisualStudio; using Microsoft.VisualStudio.Shell; using Microsoft.VisualStudio.Shell.Interop; +using EnvDTE; using Octokit; using Serilog; using Task = System.Threading.Tasks.Task; @@ -282,8 +282,9 @@ async Task CreateService(IAsyncServiceContainer container, CancellationT } else if (serviceType == typeof(IVSGitExt)) { + var dte = await GetServiceAsync(typeof(DTE)) as DTE; var sp = await GetServiceAsync(typeof(IGitHubServiceProvider)) as IGitHubServiceProvider; - return CreateVSGitExt(sp); + return CreateVSGitExt(dte.Version, sp); } else if (serviceType == typeof(IGitHubToolWindowManager)) { @@ -297,15 +298,16 @@ async Task CreateService(IAsyncServiceContainer container, CancellationT } } - IVSGitExt CreateVSGitExt(IGitHubServiceProvider sp) + IVSGitExt CreateVSGitExt(string dteVersion, IGitHubServiceProvider sp) { - switch (VSVersion.Major) + switch (dteVersion) { - case 14: + case "14.0": return new Lazy(() => new VSGitExt14(sp)).Value; - case 15: + case "15.0": return new Lazy(() => new VSGitExt15(sp)).Value; default: + log.Error("There is no IVSGitExt implementation for DTE version {Version}", dteVersion); return null; } } From 738b04679f196c48ab0a1bcb32c97d16b9297720 Mon Sep 17 00:00:00 2001 From: Jamie Cansdale Date: Wed, 28 Feb 2018 12:26:52 +0000 Subject: [PATCH 10/18] Add some comments --- src/GitHub.Exports/Services/IGitHubServiceProvider.cs | 10 ++++++++++ src/GitHub.TeamFoundation.14/Services/VSGitExt.cs | 2 +- src/GitHub.VisualStudio/GitHubPackage.cs | 1 + 3 files changed, 12 insertions(+), 1 deletion(-) diff --git a/src/GitHub.Exports/Services/IGitHubServiceProvider.cs b/src/GitHub.Exports/Services/IGitHubServiceProvider.cs index 3c0809b4cc..91145c2321 100644 --- a/src/GitHub.Exports/Services/IGitHubServiceProvider.cs +++ b/src/GitHub.Exports/Services/IGitHubServiceProvider.cs @@ -31,8 +31,18 @@ Ret GetService() where T : class /// or if it's null, the service will be removed without checking for ownership void RemoveService(Type t, object owner); + /// + /// Gets a Visual Studio service asynchronously. + /// + /// The type of the service. Task GetServiceAsync() where T : class; + /// + /// Gets a Visual Studio service asynchronously. + /// + /// The type of the service. + /// The type of the service instance to return. + /// [SuppressMessage("Microsoft.Design", "CA1004:GenericMethodsShouldProvideTypeParameter")] Task GetServiceAsync() where T : class where Ret : class; } diff --git a/src/GitHub.TeamFoundation.14/Services/VSGitExt.cs b/src/GitHub.TeamFoundation.14/Services/VSGitExt.cs index 1b852dfa89..34931fdcad 100644 --- a/src/GitHub.TeamFoundation.14/Services/VSGitExt.cs +++ b/src/GitHub.TeamFoundation.14/Services/VSGitExt.cs @@ -17,7 +17,7 @@ namespace GitHub.VisualStudio.Base /// /// /// Initialization for this service will be done asynchronously and the service will be - /// retrieved on the Main thread. This means the service can be constructed and subscribed to on a background thread. + /// retrieved using . This means the service can be constructed and subscribed to from a background thread. /// public class VSGitExt : IVSGitExt { diff --git a/src/GitHub.VisualStudio/GitHubPackage.cs b/src/GitHub.VisualStudio/GitHubPackage.cs index e01b1aab15..83b7445695 100644 --- a/src/GitHub.VisualStudio/GitHubPackage.cs +++ b/src/GitHub.VisualStudio/GitHubPackage.cs @@ -300,6 +300,7 @@ async Task CreateService(IAsyncServiceContainer container, CancellationT IVSGitExt CreateVSGitExt(string dteVersion, IGitHubServiceProvider sp) { + // DTE.Version always ends with ".0" even for later minor versions. switch (dteVersion) { case "14.0": From f3865b1f173c9a5e2603dde4252c9be996d91916 Mon Sep 17 00:00:00 2001 From: Jamie Cansdale Date: Wed, 28 Feb 2018 14:19:34 +0000 Subject: [PATCH 11/18] Use a delegate for GetServiceAsync Rather than extending IGitHubServiceProvider, pass a delegate for GetServiceAsync. --- .../Services/IGitHubServiceProvider.cs | 17 -------------- .../Services/VSGitExt.cs | 12 +++++----- src/GitHub.VisualStudio/GitHubPackage.cs | 9 ++++---- .../Services/GitHubServiceProvider.cs | 7 ------ .../GitHub.TeamFoundation/VSGitExtTests.cs | 22 +++++++++++-------- 5 files changed, 23 insertions(+), 44 deletions(-) diff --git a/src/GitHub.Exports/Services/IGitHubServiceProvider.cs b/src/GitHub.Exports/Services/IGitHubServiceProvider.cs index 91145c2321..c1d142ed7f 100644 --- a/src/GitHub.Exports/Services/IGitHubServiceProvider.cs +++ b/src/GitHub.Exports/Services/IGitHubServiceProvider.cs @@ -1,6 +1,4 @@ using System; -using System.Threading.Tasks; -using System.Diagnostics.CodeAnalysis; using System.ComponentModel.Composition.Hosting; using System.Runtime.InteropServices; using GitHub.VisualStudio; @@ -30,20 +28,5 @@ Ret GetService() where T : class /// The owner, which either has to match what was passed to AddService, /// or if it's null, the service will be removed without checking for ownership void RemoveService(Type t, object owner); - - /// - /// Gets a Visual Studio service asynchronously. - /// - /// The type of the service. - Task GetServiceAsync() where T : class; - - /// - /// Gets a Visual Studio service asynchronously. - /// - /// The type of the service. - /// The type of the service instance to return. - /// - [SuppressMessage("Microsoft.Design", "CA1004:GenericMethodsShouldProvideTypeParameter")] - Task GetServiceAsync() where T : class where Ret : class; } } diff --git a/src/GitHub.TeamFoundation.14/Services/VSGitExt.cs b/src/GitHub.TeamFoundation.14/Services/VSGitExt.cs index 34931fdcad..0c0b6f3376 100644 --- a/src/GitHub.TeamFoundation.14/Services/VSGitExt.cs +++ b/src/GitHub.TeamFoundation.14/Services/VSGitExt.cs @@ -23,7 +23,7 @@ public class VSGitExt : IVSGitExt { static readonly ILogger log = LogManager.ForContext(); - readonly IGitHubServiceProvider serviceProvider; + readonly Func> getServiceAsync; readonly IVSUIContext context; readonly ILocalRepositoryModelFactory repositoryFactory; @@ -31,14 +31,14 @@ public class VSGitExt : IVSGitExt IReadOnlyList activeRepositories; [ImportingConstructor] - public VSGitExt(IGitHubServiceProvider serviceProvider) - : this(serviceProvider, new VSUIContextFactory(), new LocalRepositoryModelFactory()) + public VSGitExt(Func> getServiceAsync) + : this(getServiceAsync, new VSUIContextFactory(), new LocalRepositoryModelFactory()) { } - public VSGitExt(IGitHubServiceProvider serviceProvider, IVSUIContextFactory factory, ILocalRepositoryModelFactory repositoryFactory) + public VSGitExt(Func> getServiceAsync, IVSUIContextFactory factory, ILocalRepositoryModelFactory repositoryFactory) { - this.serviceProvider = serviceProvider; + this.getServiceAsync = getServiceAsync; this.repositoryFactory = repositoryFactory; // The IGitExt service isn't available when a TFS based solution is opened directly. @@ -96,7 +96,7 @@ async Task ContextChangedAsync() async Task TryInitialize() { - gitService = await serviceProvider.GetServiceAsync(); + gitService = (IGitExt)await getServiceAsync(typeof(IGitExt)); if (gitService != null) { gitService.PropertyChanged += (s, e) => diff --git a/src/GitHub.VisualStudio/GitHubPackage.cs b/src/GitHub.VisualStudio/GitHubPackage.cs index 83b7445695..d4eaa28f45 100644 --- a/src/GitHub.VisualStudio/GitHubPackage.cs +++ b/src/GitHub.VisualStudio/GitHubPackage.cs @@ -283,8 +283,7 @@ async Task CreateService(IAsyncServiceContainer container, CancellationT else if (serviceType == typeof(IVSGitExt)) { var dte = await GetServiceAsync(typeof(DTE)) as DTE; - var sp = await GetServiceAsync(typeof(IGitHubServiceProvider)) as IGitHubServiceProvider; - return CreateVSGitExt(dte.Version, sp); + return CreateVSGitExt(dte.Version); } else if (serviceType == typeof(IGitHubToolWindowManager)) { @@ -298,15 +297,15 @@ async Task CreateService(IAsyncServiceContainer container, CancellationT } } - IVSGitExt CreateVSGitExt(string dteVersion, IGitHubServiceProvider sp) + IVSGitExt CreateVSGitExt(string dteVersion) { // DTE.Version always ends with ".0" even for later minor versions. switch (dteVersion) { case "14.0": - return new Lazy(() => new VSGitExt14(sp)).Value; + return new Lazy(() => new VSGitExt14(GetServiceAsync)).Value; case "15.0": - return new Lazy(() => new VSGitExt15(sp)).Value; + return new Lazy(() => new VSGitExt15(GetServiceAsync)).Value; default: log.Error("There is no IVSGitExt implementation for DTE version {Version}", dteVersion); return null; diff --git a/src/GitHub.VisualStudio/Services/GitHubServiceProvider.cs b/src/GitHub.VisualStudio/Services/GitHubServiceProvider.cs index f9e3d2edef..898df8857a 100644 --- a/src/GitHub.VisualStudio/Services/GitHubServiceProvider.cs +++ b/src/GitHub.VisualStudio/Services/GitHubServiceProvider.cs @@ -71,10 +71,6 @@ public IServiceProvider GitServiceProvider public object TryGetService(Type t) => theRealProvider.TryGetService(t); public T TryGetService() where T : class => theRealProvider.TryGetService(); - - public Task GetServiceAsync() where T : class => theRealProvider.GetServiceAsync(); - - public Task GetServiceAsync() where T : class where Ret : class => theRealProvider.GetServiceAsync(); } /// @@ -313,8 +309,5 @@ public void Dispose() Dispose(true); GC.SuppressFinalize(this); } - - public async Task GetServiceAsync() where T : class => (T)await asyncServiceProvider.GetServiceAsync(typeof(T)); - public async Task GetServiceAsync() where T : class where Ret : class => (Ret)await asyncServiceProvider.GetServiceAsync(typeof(T)); } } diff --git a/test/UnitTests/GitHub.TeamFoundation/VSGitExtTests.cs b/test/UnitTests/GitHub.TeamFoundation/VSGitExtTests.cs index 579f6b6a52..e1007ece40 100644 --- a/test/UnitTests/GitHub.TeamFoundation/VSGitExtTests.cs +++ b/test/UnitTests/GitHub.TeamFoundation/VSGitExtTests.cs @@ -21,11 +21,11 @@ public class TheConstructor : TestBaseClass public void GetServiceIGitExt_WhenSccProviderContextIsActive(bool isActive, int expectCalls) { var context = CreateVSUIContext(isActive); - var sp = Substitute.For(); + var sp = Substitute.For(); var target = CreateVSGitExt(context, sp: sp); - sp.Received(expectCalls).GetServiceAsync(); + sp.Received(expectCalls).GetServiceAsync(typeof(IGitExt)); } [TestCase(true, 1)] @@ -33,13 +33,13 @@ public void GetServiceIGitExt_WhenSccProviderContextIsActive(bool isActive, int public void GetServiceIGitExt_WhenUIContextChanged(bool activated, int expectCalls) { var context = CreateVSUIContext(false); - var sp = Substitute.For(); + var sp = Substitute.For(); var target = CreateVSGitExt(context, sp: sp); var eventArgs = new VSUIContextChangedEventArgs(activated); context.UIContextChanged += Raise.Event>(context, eventArgs); - sp.Received(expectCalls).GetServiceAsync(); + sp.Received(expectCalls).GetServiceAsync(typeof(IGitExt)); } [Test] @@ -207,23 +207,27 @@ static IReadOnlyList CreateActiveRepositories(params string[ return repositories.AsReadOnly(); } - static VSGitExt CreateVSGitExt(IVSUIContext context = null, IGitExt gitExt = null, IGitHubServiceProvider sp = null, + static VSGitExt CreateVSGitExt(IVSUIContext context = null, IGitExt gitExt = null, IAsyncServiceProvider sp = null, ILocalRepositoryModelFactory repoFactory = null) { context = context ?? CreateVSUIContext(true); gitExt = gitExt ?? CreateGitExt(); - sp = sp ?? Substitute.For(); + sp = sp ?? Substitute.For(); repoFactory = repoFactory ?? Substitute.For(); var factory = Substitute.For(); var contextGuid = new Guid(Guids.GitSccProviderId); factory.GetUIContext(contextGuid).Returns(context); - sp.GetService().Returns(factory); - sp.GetServiceAsync().Returns(gitExt); - var vsGitExt = new VSGitExt(sp, factory, repoFactory); + sp.GetServiceAsync(typeof(IGitExt)).Returns(gitExt); + var vsGitExt = new VSGitExt(sp.GetServiceAsync, factory, repoFactory); vsGitExt.PendingTasks.Wait(); return vsGitExt; } + public interface IAsyncServiceProvider + { + Task GetServiceAsync(Type serviceType); + } + static IGitExt CreateGitExt(params string[] repositoryPaths) { var gitExt = Substitute.For(); From cc6a112784322ef486c243a3e15ef41ffd1c599c Mon Sep 17 00:00:00 2001 From: Jamie Cansdale Date: Wed, 28 Feb 2018 15:01:57 +0000 Subject: [PATCH 12/18] Move VSGitExtPart into VSGitExtDispatcher --- .../GitHub.VisualStudio.csproj | 1 + src/GitHub.VisualStudio/GitHubPackage.cs | 15 --------------- .../Services/VSGitExtDispatcher.cs | 19 +++++++++++++++++++ 3 files changed, 20 insertions(+), 15 deletions(-) create mode 100644 src/GitHub.VisualStudio/Services/VSGitExtDispatcher.cs diff --git a/src/GitHub.VisualStudio/GitHub.VisualStudio.csproj b/src/GitHub.VisualStudio/GitHub.VisualStudio.csproj index c7d3879d04..9181219a36 100644 --- a/src/GitHub.VisualStudio/GitHub.VisualStudio.csproj +++ b/src/GitHub.VisualStudio/GitHub.VisualStudio.csproj @@ -328,6 +328,7 @@ + diff --git a/src/GitHub.VisualStudio/GitHubPackage.cs b/src/GitHub.VisualStudio/GitHubPackage.cs index d4eaa28f45..d9397d8ad0 100644 --- a/src/GitHub.VisualStudio/GitHubPackage.cs +++ b/src/GitHub.VisualStudio/GitHubPackage.cs @@ -112,21 +112,6 @@ public GHClient(IProgram program) } } - [PartCreationPolicy(CreationPolicy.Shared)] - public class VSGitExtPart - { - readonly IGitHubServiceProvider serviceProvider; - - [ImportingConstructor] - public VSGitExtPart(IGitHubServiceProvider serviceProvider) - { - this.serviceProvider = serviceProvider; - } - - [Export(typeof(IVSGitExt))] - public IVSGitExt VSGitExt => serviceProvider.GetService(); - } - [PackageRegistration(UseManagedResourcesOnly = true, AllowsBackgroundLoading = true)] [ProvideService(typeof(ILoginManager), IsAsyncQueryable = true)] [ProvideService(typeof(IMenuProvider), IsAsyncQueryable = true)] diff --git a/src/GitHub.VisualStudio/Services/VSGitExtDispatcher.cs b/src/GitHub.VisualStudio/Services/VSGitExtDispatcher.cs new file mode 100644 index 0000000000..b6cda6e6d1 --- /dev/null +++ b/src/GitHub.VisualStudio/Services/VSGitExtDispatcher.cs @@ -0,0 +1,19 @@ +using System.ComponentModel.Composition; + +namespace GitHub.Services +{ + [PartCreationPolicy(CreationPolicy.Shared)] + public class VSGitExtDispatcher + { + readonly IGitHubServiceProvider serviceProvider; + + [ImportingConstructor] + public VSGitExtDispatcher(IGitHubServiceProvider serviceProvider) + { + this.serviceProvider = serviceProvider; + } + + [Export(typeof(IVSGitExt))] + public IVSGitExt VSGitExt => serviceProvider.GetService(); + } +} From e51768746c89c8fe7fa7317d26cde474d9b14836 Mon Sep 17 00:00:00 2001 From: Jamie Cansdale Date: Wed, 28 Feb 2018 15:20:02 +0000 Subject: [PATCH 13/18] Move CreateVSGitExt into VSGitExtFactory --- .../GitHub.VisualStudio.csproj | 1 + src/GitHub.VisualStudio/GitHubPackage.cs | 24 ++------------ .../Services/VSGitExtFactory.cs | 32 +++++++++++++++++++ 3 files changed, 35 insertions(+), 22 deletions(-) create mode 100644 src/GitHub.VisualStudio/Services/VSGitExtFactory.cs diff --git a/src/GitHub.VisualStudio/GitHub.VisualStudio.csproj b/src/GitHub.VisualStudio/GitHub.VisualStudio.csproj index 9181219a36..4096db69b9 100644 --- a/src/GitHub.VisualStudio/GitHub.VisualStudio.csproj +++ b/src/GitHub.VisualStudio/GitHub.VisualStudio.csproj @@ -328,6 +328,7 @@ + diff --git a/src/GitHub.VisualStudio/GitHubPackage.cs b/src/GitHub.VisualStudio/GitHubPackage.cs index d9397d8ad0..c3d87d535d 100644 --- a/src/GitHub.VisualStudio/GitHubPackage.cs +++ b/src/GitHub.VisualStudio/GitHubPackage.cs @@ -1,7 +1,4 @@ -extern alias TF14; -extern alias TF15; - -using System; +using System; using System.ComponentModel.Composition; using System.Diagnostics; using System.Runtime.InteropServices; @@ -25,8 +22,6 @@ using Octokit; using Serilog; using Task = System.Threading.Tasks.Task; -using VSGitExt14 = TF14.GitHub.VisualStudio.Base.VSGitExt; -using VSGitExt15 = TF15.GitHub.VisualStudio.Base.VSGitExt; namespace GitHub.VisualStudio { @@ -268,7 +263,7 @@ async Task CreateService(IAsyncServiceContainer container, CancellationT else if (serviceType == typeof(IVSGitExt)) { var dte = await GetServiceAsync(typeof(DTE)) as DTE; - return CreateVSGitExt(dte.Version); + return VSGitExtFactory.Create(dte.Version, this); } else if (serviceType == typeof(IGitHubToolWindowManager)) { @@ -281,20 +276,5 @@ async Task CreateService(IAsyncServiceContainer container, CancellationT return sp.TryGetService(serviceType); } } - - IVSGitExt CreateVSGitExt(string dteVersion) - { - // DTE.Version always ends with ".0" even for later minor versions. - switch (dteVersion) - { - case "14.0": - return new Lazy(() => new VSGitExt14(GetServiceAsync)).Value; - case "15.0": - return new Lazy(() => new VSGitExt15(GetServiceAsync)).Value; - default: - log.Error("There is no IVSGitExt implementation for DTE version {Version}", dteVersion); - return null; - } - } } } diff --git a/src/GitHub.VisualStudio/Services/VSGitExtFactory.cs b/src/GitHub.VisualStudio/Services/VSGitExtFactory.cs new file mode 100644 index 0000000000..08632012c5 --- /dev/null +++ b/src/GitHub.VisualStudio/Services/VSGitExtFactory.cs @@ -0,0 +1,32 @@ +extern alias TF14; +extern alias TF15; + +using System; +using GitHub.Logging; +using Serilog; +using VSGitExt14 = TF14.GitHub.VisualStudio.Base.VSGitExt; +using VSGitExt15 = TF15.GitHub.VisualStudio.Base.VSGitExt; +using Microsoft.VisualStudio.Shell; + +namespace GitHub.Services +{ + public class VSGitExtFactory + { + static readonly ILogger log = LogManager.ForContext(); + + public static IVSGitExt Create(string dteVersion, IAsyncServiceProvider sp) + { + // DTE.Version always ends with ".0" even for later minor versions. + switch (dteVersion) + { + case "14.0": + return new Lazy(() => new VSGitExt14(sp.GetServiceAsync)).Value; + case "15.0": + return new Lazy(() => new VSGitExt15(sp.GetServiceAsync)).Value; + default: + log.Error("There is no IVSGitExt implementation for DTE version {Version}", dteVersion); + return null; + } + } + } +} From b98cc52f86d696151ba5bb7444dcc42587aab1a9 Mon Sep 17 00:00:00 2001 From: Jamie Cansdale Date: Wed, 28 Feb 2018 15:27:04 +0000 Subject: [PATCH 14/18] Merge VSGitExtDispatcher into VSGitExtFactory --- .../GitHub.VisualStudio.csproj | 1 - .../Services/VSGitExtDispatcher.cs | 19 ------------------- .../Services/VSGitExtFactory.cs | 13 +++++++++++++ 3 files changed, 13 insertions(+), 20 deletions(-) delete mode 100644 src/GitHub.VisualStudio/Services/VSGitExtDispatcher.cs diff --git a/src/GitHub.VisualStudio/GitHub.VisualStudio.csproj b/src/GitHub.VisualStudio/GitHub.VisualStudio.csproj index 4096db69b9..056d46301a 100644 --- a/src/GitHub.VisualStudio/GitHub.VisualStudio.csproj +++ b/src/GitHub.VisualStudio/GitHub.VisualStudio.csproj @@ -329,7 +329,6 @@ - diff --git a/src/GitHub.VisualStudio/Services/VSGitExtDispatcher.cs b/src/GitHub.VisualStudio/Services/VSGitExtDispatcher.cs deleted file mode 100644 index b6cda6e6d1..0000000000 --- a/src/GitHub.VisualStudio/Services/VSGitExtDispatcher.cs +++ /dev/null @@ -1,19 +0,0 @@ -using System.ComponentModel.Composition; - -namespace GitHub.Services -{ - [PartCreationPolicy(CreationPolicy.Shared)] - public class VSGitExtDispatcher - { - readonly IGitHubServiceProvider serviceProvider; - - [ImportingConstructor] - public VSGitExtDispatcher(IGitHubServiceProvider serviceProvider) - { - this.serviceProvider = serviceProvider; - } - - [Export(typeof(IVSGitExt))] - public IVSGitExt VSGitExt => serviceProvider.GetService(); - } -} diff --git a/src/GitHub.VisualStudio/Services/VSGitExtFactory.cs b/src/GitHub.VisualStudio/Services/VSGitExtFactory.cs index 08632012c5..db3c39b4ee 100644 --- a/src/GitHub.VisualStudio/Services/VSGitExtFactory.cs +++ b/src/GitHub.VisualStudio/Services/VSGitExtFactory.cs @@ -2,6 +2,7 @@ extern alias TF15; using System; +using System.ComponentModel.Composition; using GitHub.Logging; using Serilog; using VSGitExt14 = TF14.GitHub.VisualStudio.Base.VSGitExt; @@ -10,10 +11,22 @@ namespace GitHub.Services { + [PartCreationPolicy(CreationPolicy.Shared)] public class VSGitExtFactory { static readonly ILogger log = LogManager.ForContext(); + readonly IGitHubServiceProvider serviceProvider; + + [ImportingConstructor] + public VSGitExtFactory(IGitHubServiceProvider serviceProvider) + { + this.serviceProvider = serviceProvider; + } + + [Export(typeof(IVSGitExt))] + public IVSGitExt VSGitExt => serviceProvider.GetService(); + public static IVSGitExt Create(string dteVersion, IAsyncServiceProvider sp) { // DTE.Version always ends with ".0" even for later minor versions. From 719e624ae2f62fb32501f553b138b8e483ebdffe Mon Sep 17 00:00:00 2001 From: Jamie Cansdale Date: Wed, 28 Feb 2018 15:31:23 +0000 Subject: [PATCH 15/18] Move all construction logic into VSGitExtFactory.Create --- src/GitHub.VisualStudio/GitHubPackage.cs | 4 +--- src/GitHub.VisualStudio/Services/VSGitExtFactory.cs | 12 ++++++++---- 2 files changed, 9 insertions(+), 7 deletions(-) diff --git a/src/GitHub.VisualStudio/GitHubPackage.cs b/src/GitHub.VisualStudio/GitHubPackage.cs index c3d87d535d..8235511d09 100644 --- a/src/GitHub.VisualStudio/GitHubPackage.cs +++ b/src/GitHub.VisualStudio/GitHubPackage.cs @@ -18,7 +18,6 @@ using Microsoft.VisualStudio; using Microsoft.VisualStudio.Shell; using Microsoft.VisualStudio.Shell.Interop; -using EnvDTE; using Octokit; using Serilog; using Task = System.Threading.Tasks.Task; @@ -262,8 +261,7 @@ async Task CreateService(IAsyncServiceContainer container, CancellationT } else if (serviceType == typeof(IVSGitExt)) { - var dte = await GetServiceAsync(typeof(DTE)) as DTE; - return VSGitExtFactory.Create(dte.Version, this); + return await VSGitExtFactory.Create(this); } else if (serviceType == typeof(IGitHubToolWindowManager)) { diff --git a/src/GitHub.VisualStudio/Services/VSGitExtFactory.cs b/src/GitHub.VisualStudio/Services/VSGitExtFactory.cs index db3c39b4ee..cc2b61078b 100644 --- a/src/GitHub.VisualStudio/Services/VSGitExtFactory.cs +++ b/src/GitHub.VisualStudio/Services/VSGitExtFactory.cs @@ -2,12 +2,14 @@ extern alias TF15; using System; +using System.Threading.Tasks; using System.ComponentModel.Composition; using GitHub.Logging; using Serilog; +using EnvDTE; +using Microsoft.VisualStudio.Shell; using VSGitExt14 = TF14.GitHub.VisualStudio.Base.VSGitExt; using VSGitExt15 = TF15.GitHub.VisualStudio.Base.VSGitExt; -using Microsoft.VisualStudio.Shell; namespace GitHub.Services { @@ -27,17 +29,19 @@ public VSGitExtFactory(IGitHubServiceProvider serviceProvider) [Export(typeof(IVSGitExt))] public IVSGitExt VSGitExt => serviceProvider.GetService(); - public static IVSGitExt Create(string dteVersion, IAsyncServiceProvider sp) + public async static Task Create(IAsyncServiceProvider sp) { + var dte = await sp.GetServiceAsync(typeof(DTE)) as DTE; + // DTE.Version always ends with ".0" even for later minor versions. - switch (dteVersion) + switch (dte.Version) { case "14.0": return new Lazy(() => new VSGitExt14(sp.GetServiceAsync)).Value; case "15.0": return new Lazy(() => new VSGitExt15(sp.GetServiceAsync)).Value; default: - log.Error("There is no IVSGitExt implementation for DTE version {Version}", dteVersion); + log.Error("There is no IVSGitExt implementation for DTE version {Version}", dte.Version); return null; } } From af9f36405b3795ace7393d42d8da727e70bf23a7 Mon Sep 17 00:00:00 2001 From: Jamie Cansdale Date: Wed, 28 Feb 2018 16:27:19 +0000 Subject: [PATCH 16/18] Add comments to factory method Cache the call to GetService(). Use Func rather than Lazy (which was a hack). --- .../Services/VSGitExtFactory.cs | 18 ++++++++++-------- 1 file changed, 10 insertions(+), 8 deletions(-) diff --git a/src/GitHub.VisualStudio/Services/VSGitExtFactory.cs b/src/GitHub.VisualStudio/Services/VSGitExtFactory.cs index cc2b61078b..85649c4e48 100644 --- a/src/GitHub.VisualStudio/Services/VSGitExtFactory.cs +++ b/src/GitHub.VisualStudio/Services/VSGitExtFactory.cs @@ -18,17 +18,12 @@ public class VSGitExtFactory { static readonly ILogger log = LogManager.ForContext(); - readonly IGitHubServiceProvider serviceProvider; - [ImportingConstructor] public VSGitExtFactory(IGitHubServiceProvider serviceProvider) { - this.serviceProvider = serviceProvider; + VSGitExt = serviceProvider.GetService(); } - [Export(typeof(IVSGitExt))] - public IVSGitExt VSGitExt => serviceProvider.GetService(); - public async static Task Create(IAsyncServiceProvider sp) { var dte = await sp.GetServiceAsync(typeof(DTE)) as DTE; @@ -37,13 +32,20 @@ public async static Task Create(IAsyncServiceProvider sp) switch (dte.Version) { case "14.0": - return new Lazy(() => new VSGitExt14(sp.GetServiceAsync)).Value; + return Create(() => new VSGitExt14(sp.GetServiceAsync)); case "15.0": - return new Lazy(() => new VSGitExt15(sp.GetServiceAsync)).Value; + return Create(() => new VSGitExt15(sp.GetServiceAsync)); default: log.Error("There is no IVSGitExt implementation for DTE version {Version}", dte.Version); return null; } } + + // NOTE: We're being careful to only reference VSGitExt14 and VSGitExt15 from inside a lambda expression. + // This ensures that only the type that's compatible with the running DTE version is loaded. + static IVSGitExt Create(Func factory) => factory.Invoke(); + + [Export] + public IVSGitExt VSGitExt { get; } } } From fc06da7c0eac08bb73d5ea7f18802ab10acca85d Mon Sep 17 00:00:00 2001 From: Jamie Cansdale Date: Wed, 28 Feb 2018 16:47:08 +0000 Subject: [PATCH 17/18] Use ApplicationInfo.GetHostVersionInfo not DTE.Version --- src/GitHub.VisualStudio/GitHubPackage.cs | 3 ++- .../Services/VSGitExtFactory.cs | 16 ++++++---------- 2 files changed, 8 insertions(+), 11 deletions(-) diff --git a/src/GitHub.VisualStudio/GitHubPackage.cs b/src/GitHub.VisualStudio/GitHubPackage.cs index 8235511d09..04d896c6aa 100644 --- a/src/GitHub.VisualStudio/GitHubPackage.cs +++ b/src/GitHub.VisualStudio/GitHubPackage.cs @@ -261,7 +261,8 @@ async Task CreateService(IAsyncServiceContainer container, CancellationT } else if (serviceType == typeof(IVSGitExt)) { - return await VSGitExtFactory.Create(this); + var vsVersion = ApplicationInfo.GetHostVersionInfo().FileMajorPart; + return VSGitExtFactory.Create(vsVersion, this); } else if (serviceType == typeof(IGitHubToolWindowManager)) { diff --git a/src/GitHub.VisualStudio/Services/VSGitExtFactory.cs b/src/GitHub.VisualStudio/Services/VSGitExtFactory.cs index 85649c4e48..88bba6a1bb 100644 --- a/src/GitHub.VisualStudio/Services/VSGitExtFactory.cs +++ b/src/GitHub.VisualStudio/Services/VSGitExtFactory.cs @@ -2,11 +2,10 @@ extern alias TF15; using System; -using System.Threading.Tasks; using System.ComponentModel.Composition; +using GitHub.Info; using GitHub.Logging; using Serilog; -using EnvDTE; using Microsoft.VisualStudio.Shell; using VSGitExt14 = TF14.GitHub.VisualStudio.Base.VSGitExt; using VSGitExt15 = TF15.GitHub.VisualStudio.Base.VSGitExt; @@ -24,19 +23,16 @@ public VSGitExtFactory(IGitHubServiceProvider serviceProvider) VSGitExt = serviceProvider.GetService(); } - public async static Task Create(IAsyncServiceProvider sp) + public static IVSGitExt Create(int vsVersion, IAsyncServiceProvider sp) { - var dte = await sp.GetServiceAsync(typeof(DTE)) as DTE; - - // DTE.Version always ends with ".0" even for later minor versions. - switch (dte.Version) + switch (vsVersion) { - case "14.0": + case 14: return Create(() => new VSGitExt14(sp.GetServiceAsync)); - case "15.0": + case 15: return Create(() => new VSGitExt15(sp.GetServiceAsync)); default: - log.Error("There is no IVSGitExt implementation for DTE version {Version}", dte.Version); + log.Error("There is no IVSGitExt implementation for DTE version {Version}", vsVersion); return null; } } From ee9ab4af7f71ad9c604a4013ba7b87836a5cffb8 Mon Sep 17 00:00:00 2001 From: Jamie Cansdale Date: Fri, 2 Mar 2018 16:59:36 +0000 Subject: [PATCH 18/18] Simplify the VSGitExt service - Use `UIContext.WhenActivated` instead of UIContext.UIContextChanged event - Don't use async InitializeAsync now we're initializing on background thread - No more need to use ContinueWith --- .../Services/IVSUIContextFactory.cs | 2 +- .../Services/VSGitExt.cs | 90 +++++-------------- .../Services/VSUIContextFactory.cs | 28 +----- .../GitHub.TeamFoundation/VSGitExtTests.cs | 47 +++++++--- 4 files changed, 63 insertions(+), 104 deletions(-) diff --git a/src/GitHub.Exports/Services/IVSUIContextFactory.cs b/src/GitHub.Exports/Services/IVSUIContextFactory.cs index e3aca6cbb7..57c6542d9b 100644 --- a/src/GitHub.Exports/Services/IVSUIContextFactory.cs +++ b/src/GitHub.Exports/Services/IVSUIContextFactory.cs @@ -20,6 +20,6 @@ public VSUIContextChangedEventArgs(bool activated) public interface IVSUIContext { bool IsActive { get; } - event EventHandler UIContextChanged; + void WhenActivated(Action action); } } \ No newline at end of file diff --git a/src/GitHub.TeamFoundation.14/Services/VSGitExt.cs b/src/GitHub.TeamFoundation.14/Services/VSGitExt.cs index 0c0b6f3376..5626ad4cd1 100644 --- a/src/GitHub.TeamFoundation.14/Services/VSGitExt.cs +++ b/src/GitHub.TeamFoundation.14/Services/VSGitExt.cs @@ -24,8 +24,8 @@ public class VSGitExt : IVSGitExt static readonly ILogger log = LogManager.ForContext(); readonly Func> getServiceAsync; - readonly IVSUIContext context; readonly ILocalRepositoryModelFactory repositoryFactory; + readonly object refreshLock = new object(); IGitExt gitService; IReadOnlyList activeRepositories; @@ -41,94 +41,50 @@ public VSGitExt(Func> getServiceAsync, IVSUIContextFactory fa this.getServiceAsync = getServiceAsync; this.repositoryFactory = repositoryFactory; - // The IGitExt service isn't available when a TFS based solution is opened directly. - // It will become available when moving to a Git based solution (and cause a UIContext event to fire). - context = factory.GetUIContext(new Guid(Guids.GitSccProviderId)); - // Start with empty array until we have a chance to initialize. ActiveRepositories = Array.Empty(); - PendingTasks = InitializeAsync(); - } - - async Task InitializeAsync() - { - try - { - if (!context.IsActive || !await TryInitialize()) - { - // If we're not in the UIContext or TryInitialize fails, have another go when the UIContext changes. - context.UIContextChanged += ContextChanged; - log.Debug("VSGitExt will be initialized later"); - } - } - catch (Exception e) - { - log.Error(e, "Initializing"); - } - } - - void ContextChanged(object sender, VSUIContextChangedEventArgs e) - { - if (e.Activated) - { - PendingTasks = ContextChangedAsync(); - } + // The IGitExt service isn't available when a TFS based solution is opened directly. + // It will become available when moving to a Git based solution (and cause a UIContext event to fire). + var context = factory.GetUIContext(new Guid(Guids.GitSccProviderId)); + context.WhenActivated(() => Initialize()); } - async Task ContextChangedAsync() + void Initialize() { - try + PendingTasks = getServiceAsync(typeof(IGitExt)).ContinueWith(t => { - // If we're in the UIContext and TryInitialize succeeds, we can stop listening for events. - // NOTE: this event can fire with UIContext=true in a TFS solution (not just Git). - if (await TryInitialize()) + gitService = (IGitExt)t.Result; + if (gitService == null) { - context.UIContextChanged -= ContextChanged; - log.Debug("Initialized VSGitExt on UIContextChanged"); + log.Error("Couldn't find IGitExt service"); + return; } - } - catch (Exception e) - { - log.Error(e, "UIContextChanged"); - } - } - async Task TryInitialize() - { - gitService = (IGitExt)await getServiceAsync(typeof(IGitExt)); - if (gitService != null) - { + RefreshActiveRepositories(); gitService.PropertyChanged += (s, e) => { if (e.PropertyName == nameof(gitService.ActiveRepositories)) { - // Execute tasks in sequence using thread pool (TaskScheduler.Default). - PendingTasks = PendingTasks.ContinueWith(_ => RefreshActiveRepositories(), TaskScheduler.Default); + RefreshActiveRepositories(); } }; - - // Do this after we start listening so we don't miss an event. - await Task.Run(() => RefreshActiveRepositories()); - - log.Debug("Found IGitExt service and initialized VSGitExt"); - return true; - } - - log.Error("Couldn't find IGitExt service"); - return false; + }, TaskScheduler.Default); } void RefreshActiveRepositories() { try { - log.Debug( - "IGitExt.ActiveRepositories (#{Id}) returned {Repositories}", - gitService.GetHashCode(), - gitService?.ActiveRepositories.Select(x => x.RepositoryPath)); + lock (refreshLock) + { + log.Debug( + "IGitExt.ActiveRepositories (#{Id}) returned {Repositories}", + gitService.GetHashCode(), + gitService.ActiveRepositories.Select(x => x.RepositoryPath)); - ActiveRepositories = gitService?.ActiveRepositories.Select(x => repositoryFactory.Create(x.RepositoryPath)).ToList(); + ActiveRepositories = gitService?.ActiveRepositories.Select(x => repositoryFactory.Create(x.RepositoryPath)).ToList(); + } } catch (Exception e) { @@ -160,6 +116,6 @@ private set /// /// Tasks that are pending execution on the thread pool. /// - public Task PendingTasks { get; private set; } + public Task PendingTasks { get; private set; } = Task.CompletedTask; } } \ No newline at end of file diff --git a/src/GitHub.TeamFoundation.14/Services/VSUIContextFactory.cs b/src/GitHub.TeamFoundation.14/Services/VSUIContextFactory.cs index 6cedf85cd8..3797f75c02 100644 --- a/src/GitHub.TeamFoundation.14/Services/VSUIContextFactory.cs +++ b/src/GitHub.TeamFoundation.14/Services/VSUIContextFactory.cs @@ -1,6 +1,4 @@ using System; -using System.Collections.Generic; -using System.ComponentModel.Composition; using Microsoft.VisualStudio.Shell; using GitHub.Services; @@ -17,8 +15,7 @@ public IVSUIContext GetUIContext(Guid contextGuid) class VSUIContext : IVSUIContext { readonly UIContext context; - readonly Dictionary, EventHandler> handlers = - new Dictionary, EventHandler>(); + public VSUIContext(UIContext context) { this.context = context; @@ -26,27 +23,6 @@ public VSUIContext(UIContext context) public bool IsActive { get { return context.IsActive; } } - public event EventHandler UIContextChanged - { - add - { - EventHandler handler = null; - if (!handlers.TryGetValue(value, out handler)) - { - handler = (s, e) => value.Invoke(s, new VSUIContextChangedEventArgs(e.Activated)); - handlers.Add(value, handler); - } - context.UIContextChanged += handler; - } - remove - { - EventHandler handler = null; - if (handlers.TryGetValue(value, out handler)) - { - handlers.Remove(value); - context.UIContextChanged -= handler; - } - } - } + public void WhenActivated(Action action) => context.WhenActivated(action); } } diff --git a/test/UnitTests/GitHub.TeamFoundation/VSGitExtTests.cs b/test/UnitTests/GitHub.TeamFoundation/VSGitExtTests.cs index e1007ece40..daa0dbd037 100644 --- a/test/UnitTests/GitHub.TeamFoundation/VSGitExtTests.cs +++ b/test/UnitTests/GitHub.TeamFoundation/VSGitExtTests.cs @@ -36,9 +36,9 @@ public void GetServiceIGitExt_WhenUIContextChanged(bool activated, int expectCal var sp = Substitute.For(); var target = CreateVSGitExt(context, sp: sp); - var eventArgs = new VSUIContextChangedEventArgs(activated); - context.UIContextChanged += Raise.Event>(context, eventArgs); + context.IsActive = activated; + target.PendingTasks.Wait(); sp.Received(expectCalls).GetServiceAsync(typeof(IGitExt)); } @@ -106,8 +106,7 @@ public void WhenUIContextChanged_ActiveRepositoriesChangedIsFired() bool wasFired = false; target.ActiveRepositoriesChanged += () => wasFired = true; - var eventArgs = new VSUIContextChangedEventArgs(true); - context.UIContextChanged += Raise.Event>(context, eventArgs); + context.IsActive = true; target.PendingTasks.Wait(); Assert.That(wasFired, Is.True); @@ -123,8 +122,7 @@ public void WhenUIContextChanged_FiredUsingThreadPoolThread() bool threadPool = false; target.ActiveRepositoriesChanged += () => threadPool = Thread.CurrentThread.IsThreadPoolThread; - var eventArgs = new VSUIContextChangedEventArgs(true); - context.UIContextChanged += Raise.Event>(context, eventArgs); + context.IsActive = true; target.PendingTasks.Wait(); Assert.That(threadPool, Is.True); @@ -236,11 +234,40 @@ static IGitExt CreateGitExt(params string[] repositoryPaths) return gitExt; } - static IVSUIContext CreateVSUIContext(bool isActive) + static MockVSUIContext CreateVSUIContext(bool isActive) { - var context = Substitute.For(); - context.IsActive.Returns(isActive); - return context; + return new MockVSUIContext { IsActive = isActive }; + } + + class MockVSUIContext : IVSUIContext + { + bool isActive; + Action action; + + public bool IsActive + { + get { return isActive; } + set + { + isActive = value; + if (isActive && action != null) + { + action.Invoke(); + action = null; + } + } + } + + public void WhenActivated(Action action) + { + if (isActive) + { + action.Invoke(); + return; + } + + this.action = action; + } } class MockGitExt : IGitExt