diff --git a/src/GitHub.Api/LoginManager.cs b/src/GitHub.Api/LoginManager.cs index 01cfe196dd..399266b816 100644 --- a/src/GitHub.Api/LoginManager.cs +++ b/src/GitHub.Api/LoginManager.cs @@ -14,7 +14,7 @@ public class LoginManager : ILoginManager { readonly string[] scopes = { "user", "repo", "gist", "write:public_key" }; readonly IKeychain keychain; - readonly ITwoFactorChallengeHandler twoFactorChallengeHandler; + readonly Lazy twoFactorChallengeHandler; readonly string clientId; readonly string clientSecret; readonly string authorizationNote; @@ -31,7 +31,7 @@ public class LoginManager : ILoginManager /// The machine fingerprint. public LoginManager( IKeychain keychain, - ITwoFactorChallengeHandler twoFactorChallengeHandler, + Lazy twoFactorChallengeHandler, string clientId, string clientSecret, string authorizationNote = null, @@ -194,7 +194,7 @@ async Task HandleTwoFactorAuthorization( { for (;;) { - var challengeResult = await twoFactorChallengeHandler.HandleTwoFactorException(exception); + var challengeResult = await twoFactorChallengeHandler.Value.HandleTwoFactorException(exception); if (challengeResult == null) { @@ -218,7 +218,7 @@ async Task HandleTwoFactorAuthorization( } catch (Exception e) { - await twoFactorChallengeHandler.ChallengeFailed(e); + await twoFactorChallengeHandler.Value.ChallengeFailed(e); await keychain.Delete(hostAddress).ConfigureAwait(false); throw; } diff --git a/src/GitHub.InlineReviews/InlineCommentMarginProvider.cs b/src/GitHub.InlineReviews/InlineCommentMarginProvider.cs index 0aaa798090..19acd8d60b 100644 --- a/src/GitHub.InlineReviews/InlineCommentMarginProvider.cs +++ b/src/GitHub.InlineReviews/InlineCommentMarginProvider.cs @@ -28,27 +28,43 @@ internal sealed class InlineCommentMarginProvider : IWpfTextViewMarginProvider const string MarginName = "InlineComment"; const string MarginPropertiesName = "Indicator Margin"; // Same background color as Glyph margin + readonly IGitHubServiceProvider serviceProvider; readonly IEditorFormatMapService editorFormatMapService; readonly IViewTagAggregatorFactoryService tagAggregatorFactory; readonly IInlineCommentPeekService peekService; - readonly IPullRequestSessionManager sessionManager; readonly IPackageSettings packageSettings; + IPullRequestSessionManager sessionManager; [ImportingConstructor] public InlineCommentMarginProvider( + IGitHubServiceProvider serviceProvider, IEditorFormatMapService editorFormatMapService, IViewTagAggregatorFactoryService tagAggregatorFactory, IInlineCommentPeekService peekService, - IPullRequestSessionManager sessionManager, IPackageSettings packageSettings) { + this.serviceProvider = serviceProvider; this.editorFormatMapService = editorFormatMapService; this.tagAggregatorFactory = tagAggregatorFactory; this.peekService = peekService; - this.sessionManager = sessionManager; this.packageSettings = packageSettings; } + IPullRequestSessionManager SessionManager + { + get + { + // Lazily load the pull request session manager to prevent all of our assemblies + // being loaded on VS startup. + if (sessionManager == null) + { + sessionManager = serviceProvider.GetService(); + } + + return sessionManager; + } + } + public IWpfTextViewMargin CreateMargin(IWpfTextViewHost wpfTextViewHost, IWpfTextViewMargin parent) { if (IsMarginDisabled(wpfTextViewHost)) @@ -95,7 +111,7 @@ void TrackCommentGlyph(IWpfTextViewHost host, UIElement marginElement) bool IsMarginVisible(ITextBuffer buffer) { - if (sessionManager.GetTextBufferInfo(buffer) != null) + if (SessionManager.GetTextBufferInfo(buffer) != null) { return true; } diff --git a/src/GitHub.TeamFoundation.14/Connect/GitHubConnectSection.cs b/src/GitHub.TeamFoundation.14/Connect/GitHubConnectSection.cs index 0ded1f655c..643e078989 100644 --- a/src/GitHub.TeamFoundation.14/Connect/GitHubConnectSection.cs +++ b/src/GitHub.TeamFoundation.14/Connect/GitHubConnectSection.cs @@ -32,8 +32,6 @@ public class GitHubConnectSection : TeamExplorerSectionBase, IGitHubConnectSecti readonly IPackageSettings packageSettings; readonly IVSServices vsServices; readonly int sectionIndex; - readonly IDialogService dialogService; - readonly IRepositoryCloneService cloneService; readonly ILocalRepositories localRepositories; bool isCloning; @@ -100,8 +98,6 @@ public GitHubConnectSection(IGitHubServiceProvider serviceProvider, IConnectionManager manager, IPackageSettings packageSettings, IVSServices vsServices, - IRepositoryCloneService cloneService, - IDialogService dialogService, ILocalRepositories localRepositories, int index) : base(serviceProvider, apiFactory, holder, manager) @@ -111,8 +107,6 @@ public GitHubConnectSection(IGitHubServiceProvider serviceProvider, Guard.ArgumentNotNull(manager, nameof(manager)); Guard.ArgumentNotNull(packageSettings, nameof(packageSettings)); Guard.ArgumentNotNull(vsServices, nameof(vsServices)); - Guard.ArgumentNotNull(cloneService, nameof(cloneService)); - Guard.ArgumentNotNull(dialogService, nameof(dialogService)); Guard.ArgumentNotNull(localRepositories, nameof(localRepositories)); Title = "GitHub"; @@ -123,8 +117,6 @@ public GitHubConnectSection(IGitHubServiceProvider serviceProvider, this.packageSettings = packageSettings; this.vsServices = vsServices; - this.cloneService = cloneService; - this.dialogService = dialogService; this.localRepositories = localRepositories; Clone = CreateAsyncCommandHack(DoClone); @@ -136,6 +128,7 @@ public GitHubConnectSection(IGitHubServiceProvider serviceProvider, async Task DoClone() { + var dialogService = ServiceProvider.GetService(); var result = await dialogService.ShowCloneDialog(SectionConnection); if (result != null) @@ -143,6 +136,7 @@ async Task DoClone() try { ServiceProvider.GitServiceProvider = TEServiceProvider; + var cloneService = ServiceProvider.GetService(); await cloneService.CloneRepository( result.Repository.CloneUrl, result.Repository.Name, @@ -381,6 +375,7 @@ async Task RefreshRepositories() public void DoCreate() { + var dialogService = ServiceProvider.GetService(); dialogService.ShowCreateRepositoryDialog(SectionConnection); } @@ -391,6 +386,7 @@ public void SignOut() public void Login() { + var dialogService = ServiceProvider.GetService(); dialogService.ShowLoginDialog(); } diff --git a/src/GitHub.TeamFoundation.14/Connect/GitHubConnectSection0.cs b/src/GitHub.TeamFoundation.14/Connect/GitHubConnectSection0.cs index 652456eaa7..46725ca867 100644 --- a/src/GitHub.TeamFoundation.14/Connect/GitHubConnectSection0.cs +++ b/src/GitHub.TeamFoundation.14/Connect/GitHubConnectSection0.cs @@ -20,10 +20,8 @@ public GitHubConnectSection0(IGitHubServiceProvider serviceProvider, IConnectionManager manager, IPackageSettings settings, IVSServices vsServices, - IRepositoryCloneService cloneService, - IDialogService dialogService, ILocalRepositories localRepositories) - : base(serviceProvider, apiFactory, holder, manager, settings, vsServices, cloneService, dialogService, localRepositories, 0) + : base(serviceProvider, apiFactory, holder, manager, settings, vsServices, localRepositories, 0) { } } diff --git a/src/GitHub.TeamFoundation.14/Connect/GitHubConnectSection1.cs b/src/GitHub.TeamFoundation.14/Connect/GitHubConnectSection1.cs index e2c7070521..6059020a4c 100644 --- a/src/GitHub.TeamFoundation.14/Connect/GitHubConnectSection1.cs +++ b/src/GitHub.TeamFoundation.14/Connect/GitHubConnectSection1.cs @@ -20,10 +20,8 @@ public GitHubConnectSection1(IGitHubServiceProvider serviceProvider, IConnectionManager manager, IPackageSettings settings, IVSServices vsServices, - IRepositoryCloneService cloneService, - IDialogService dialogService, ILocalRepositories localRepositories) - : base(serviceProvider, apiFactory, holder, manager, settings, vsServices, cloneService, dialogService, localRepositories, 1) + : base(serviceProvider, apiFactory, holder, manager, settings, vsServices, localRepositories, 1) { } } diff --git a/src/GitHub.TeamFoundation.14/Home/GitHubHomeSection.cs b/src/GitHub.TeamFoundation.14/Home/GitHubHomeSection.cs index 2ab3a1f835..c93e77091d 100644 --- a/src/GitHub.TeamFoundation.14/Home/GitHubHomeSection.cs +++ b/src/GitHub.TeamFoundation.14/Home/GitHubHomeSection.cs @@ -1,22 +1,19 @@ using System; using System.ComponentModel.Composition; +using System.Diagnostics; +using System.Windows.Input; +using GitHub.Api; +using GitHub.Extensions; +using GitHub.Info; +using GitHub.Primitives; +using GitHub.Services; +using GitHub.Settings; using GitHub.UI; using GitHub.VisualStudio.Base; using GitHub.VisualStudio.Helpers; +using GitHub.VisualStudio.UI; using GitHub.VisualStudio.UI.Views; using Microsoft.TeamFoundation.Controls; -using GitHub.Services; -using GitHub.Api; -using GitHub.Primitives; -using System.Threading.Tasks; -using System.Diagnostics; -using GitHub.Extensions; -using System.Windows.Input; -using ReactiveUI; -using GitHub.VisualStudio.UI; -using GitHub.Settings; -using System.Windows.Threading; -using GitHub.Info; namespace GitHub.VisualStudio.TeamExplorer.Home { @@ -32,7 +29,6 @@ public class GitHubHomeSection : TeamExplorerSectionBase, IGitHubHomeSection readonly ITeamExplorerServices teamExplorerServices; readonly IPackageSettings settings; readonly IUsageTracker usageTracker; - readonly IDialogService dialogService; [ImportingConstructor] public GitHubHomeSection(IGitHubServiceProvider serviceProvider, @@ -41,8 +37,7 @@ public GitHubHomeSection(IGitHubServiceProvider serviceProvider, IVisualStudioBrowser visualStudioBrowser, ITeamExplorerServices teamExplorerServices, IPackageSettings settings, - IUsageTracker usageTracker, - IDialogService dialogService) + IUsageTracker usageTracker) : base(serviceProvider, apiFactory, holder) { Title = "GitHub"; @@ -52,10 +47,8 @@ public GitHubHomeSection(IGitHubServiceProvider serviceProvider, this.teamExplorerServices = teamExplorerServices; this.settings = settings; this.usageTracker = usageTracker; - this.dialogService = dialogService; - var openOnGitHub = ReactiveCommand.Create(); - openOnGitHub.Subscribe(_ => DoOpenOnGitHub()); + var openOnGitHub = new RelayCommand(_ => DoOpenOnGitHub()); OpenOnGitHub = openOnGitHub; } @@ -118,6 +111,7 @@ static Octicon GetIcon(bool isPrivate, bool isHosted, bool isFork) public void Login() { + var dialogService = ServiceProvider.GetService(); dialogService.ShowLoginDialog(); } diff --git a/src/GitHub.VisualStudio/GitHubPackage.cs b/src/GitHub.VisualStudio/GitHubPackage.cs index 029d5bcec8..b04b53b1c5 100644 --- a/src/GitHub.VisualStudio/GitHubPackage.cs +++ b/src/GitHub.VisualStudio/GitHubPackage.cs @@ -1,6 +1,7 @@ using System; using System.ComponentModel.Composition; using System.Diagnostics; +using System.Reflection; using System.Runtime.InteropServices; using System.Threading; using System.Threading.Tasks; @@ -211,11 +212,20 @@ async Task CreateService(IAsyncServiceContainer container, CancellationT { var serviceProvider = await GetServiceAsync(typeof(IGitHubServiceProvider)) as IGitHubServiceProvider; var keychain = serviceProvider.GetService(); - var twoFaHandler = serviceProvider.GetService(); + + // HACK: We need to make sure this is run on the main thread. We really + // shouldn't be injecting a view model concern into LoginManager - this + // needs to be refactored. See #1398. + var lazy2Fa = new Lazy(() => + ThreadHelper.JoinableTaskFactory.Run(async () => + { + await ThreadHelper.JoinableTaskFactory.SwitchToMainThreadAsync(); + return serviceProvider.GetService(); + })); return new LoginManager( keychain, - twoFaHandler, + lazy2Fa, ApiClientConfiguration.ClientId, ApiClientConfiguration.ClientSecret, ApiClientConfiguration.AuthorizationNote, diff --git a/src/GitHub.VisualStudio/Services/GitHubServiceProvider.cs b/src/GitHub.VisualStudio/Services/GitHubServiceProvider.cs index 12985499c8..898df8857a 100644 --- a/src/GitHub.VisualStudio/Services/GitHubServiceProvider.cs +++ b/src/GitHub.VisualStudio/Services/GitHubServiceProvider.cs @@ -7,7 +7,6 @@ using System.Diagnostics.CodeAnalysis; using System.Globalization; using System.Linq; -using System.Reactive.Disposables; using GitHub.Logging; using GitHub.Models; using GitHub.Exports; @@ -89,11 +88,11 @@ class OwnedComposablePart } static readonly ILogger log = LogManager.ForContext(); - CompositeDisposable disposables = new CompositeDisposable(); readonly IServiceProviderPackage asyncServiceProvider; readonly IServiceProvider syncServiceProvider; readonly Dictionary tempParts; readonly Version currentVersion; + List disposables = new List(); bool initialized = false; public ExportProvider ExportProvider { get; private set; } @@ -290,7 +289,13 @@ protected void Dispose(bool disposing) if (disposed) return; if (disposables != null) - disposables.Dispose(); + { + foreach (var disposable in disposables) + { + disposable.Dispose(); + } + } + disposables = null; if (tempContainer != null) tempContainer.Dispose(); diff --git a/test/UnitTests/GitHub.Api/LoginManagerTests.cs b/test/UnitTests/GitHub.Api/LoginManagerTests.cs index 00f317d3db..333ad6fc55 100644 --- a/test/UnitTests/GitHub.Api/LoginManagerTests.cs +++ b/test/UnitTests/GitHub.Api/LoginManagerTests.cs @@ -22,7 +22,7 @@ public async Task LoginTokenIsSavedToCache() .Returns(new ApplicationAuthorization("123abc")); var keychain = Substitute.For(); - var tfa = Substitute.For(); + var tfa = new Lazy(() => Substitute.For()); var target = new LoginManager(keychain, tfa, "id", "secret"); await target.Login(host, client, "foo", "bar"); @@ -40,7 +40,7 @@ public async Task LoggedInUserIsReturned() client.User.Current().Returns(user); var keychain = Substitute.For(); - var tfa = Substitute.For(); + var tfa = new Lazy(() => Substitute.For()); var target = new LoginManager(keychain, tfa, "id", "secret"); var result = await target.Login(host, client, "foo", "bar"); @@ -63,7 +63,7 @@ public async Task DeletesExistingAuthenticationIfNullTokenReturned() client.User.Current().Returns(user); var keychain = Substitute.For(); - var tfa = Substitute.For(); + var tfa = new Lazy(() => Substitute.For()); var target = new LoginManager(keychain, tfa, "id", "secret"); var result = await target.Login(host, client, "foo", "bar"); @@ -85,8 +85,8 @@ public async Task TwoFactorExceptionIsPassedToHandler() .Returns(new ApplicationAuthorization("123abc")); var keychain = Substitute.For(); - var tfa = Substitute.For(); - tfa.HandleTwoFactorException(exception).Returns(new TwoFactorChallengeResult("123456")); + var tfa = new Lazy(() => Substitute.For()); + tfa.Value.HandleTwoFactorException(exception).Returns(new TwoFactorChallengeResult("123456")); var target = new LoginManager(keychain, tfa, "id", "secret"); await target.Login(host, client, "foo", "bar"); @@ -112,8 +112,8 @@ public async Task Failed2FACodeResultsInRetry() .Returns(new ApplicationAuthorization("123abc")); var keychain = Substitute.For(); - var tfa = Substitute.For(); - tfa.HandleTwoFactorException(exception).Returns( + var tfa = new Lazy(() => Substitute.For()); + tfa.Value.HandleTwoFactorException(exception).Returns( new TwoFactorChallengeResult("111111"), new TwoFactorChallengeResult("123456")); @@ -147,8 +147,8 @@ public async Task HandlerNotifiedOfExceptionIn2FAChallengeResponse() .Returns(_ => { throw loginAttemptsException; }); var keychain = Substitute.For(); - var tfa = Substitute.For(); - tfa.HandleTwoFactorException(twoFaException).Returns( + var tfa = new Lazy(() => Substitute.For()); + tfa.Value.HandleTwoFactorException(twoFaException).Returns( new TwoFactorChallengeResult("111111"), new TwoFactorChallengeResult("123456")); @@ -160,7 +160,7 @@ await client.Authorization.Received(1).GetOrCreateApplicationAuthentication( "secret", Arg.Any(), "111111"); - tfa.Received(1).ChallengeFailed(loginAttemptsException); + tfa.Value.Received(1).ChallengeFailed(loginAttemptsException); } [Fact] @@ -177,8 +177,8 @@ public async Task RequestResendCodeResultsInRetryingLogin() client.User.Current().Returns(user); var keychain = Substitute.For(); - var tfa = Substitute.For(); - tfa.HandleTwoFactorException(exception).Returns( + var tfa = new Lazy(() => Substitute.For()); + tfa.Value.HandleTwoFactorException(exception).Returns( TwoFactorChallengeResult.RequestResendCode, new TwoFactorChallengeResult("123456")); @@ -202,7 +202,7 @@ public async Task UsesUsernameAndPasswordInsteadOfAuthorizationTokenWhenEnterpri client.User.Current().Returns(user); var keychain = Substitute.For(); - var tfa = Substitute.For(); + var tfa = new Lazy(() => Substitute.For()); var target = new LoginManager(keychain, tfa, "id", "secret"); await target.Login(enterprise, client, "foo", "bar"); @@ -220,7 +220,7 @@ public async Task ErasesLoginWhenUnauthorized() .Returns(_ => { throw new AuthorizationException(); }); var keychain = Substitute.For(); - var tfa = Substitute.For(); + var tfa = new Lazy(() => Substitute.For()); var target = new LoginManager(keychain, tfa, "id", "secret"); await Assert.ThrowsAsync(async () => await target.Login(enterprise, client, "foo", "bar")); @@ -238,7 +238,7 @@ public async Task ErasesLoginWhenNonOctokitExceptionThrown() .Returns(_ => { throw new InvalidOperationException(); }); var keychain = Substitute.For(); - var tfa = Substitute.For(); + var tfa = new Lazy(() => Substitute.For()); var target = new LoginManager(keychain, tfa, "id", "secret"); await Assert.ThrowsAsync(async () => await target.Login(host, client, "foo", "bar")); @@ -260,8 +260,8 @@ public async Task ErasesLoginWhenNonOctokitExceptionThrownIn2FA() client.User.Current().Returns(user); var keychain = Substitute.For(); - var tfa = Substitute.For(); - tfa.HandleTwoFactorException(exception).Returns(new TwoFactorChallengeResult("123456")); + var tfa = new Lazy(() => Substitute.For()); + tfa.Value.HandleTwoFactorException(exception).Returns(new TwoFactorChallengeResult("123456")); var target = new LoginManager(keychain, tfa, "id", "secret"); await Assert.ThrowsAsync(async () => await target.Login(host, client, "foo", "bar"));