From 470ea1342b256438bfbd8ea7738b07e1bfbae0cc Mon Sep 17 00:00:00 2001 From: Steven Kirk Date: Fri, 20 Oct 2017 17:52:29 +0200 Subject: [PATCH 01/16] Removed `IConnection.Repositories`. Removed `IConnection.Repositories` and `IConnectionManager.RefreshRepositories` and replaced them with `LocalRepositories` which acts as a central place to find known repositories. As part of this, also added `IReadOnlyObservableCollection` and `ObservableCollectionEx` because the BCL's `ReadOnlyObservableCollection` sucks. Made `TrackingCollection` implement `IReadOnlyObservableCollection`. --- src/GitHub.App/GitHub.App.csproj | 1 + src/GitHub.App/Services/LocalRepositories.cs | 47 ++++++ .../Collections/TrackingCollection.cs | 2 +- .../GitHub.Exports.Reactive.csproj | 1 + .../Services/LocalRepositoriesExtensions.cs | 29 ++++ src/GitHub.Exports/GitHub.Exports.csproj | 1 + src/GitHub.Exports/Models/IConnection.cs | 3 +- src/GitHub.Exports/Services/Connection.cs | 25 ---- .../Services/IConnectionManager.cs | 1 - .../Services/ILocalRepositories.cs | 23 +++ .../GitHub.Extensions.csproj | 2 + .../IReadOnlyObservableCollection.cs | 17 +++ .../ObservableCollectionEx.cs | 42 ++++++ .../Connect/GitHubConnectSection.cs | 10 +- .../Connect/GitHubConnectSection0.cs | 5 +- .../Connect/GitHubConnectSection1.cs | 5 +- .../Services/ConnectionManager.cs | 17 --- .../Services/LocalRepositoriesTests.cs | 136 ++++++++++++++++++ .../Services/ConnectionManagerTests.cs | 2 +- 19 files changed, 315 insertions(+), 54 deletions(-) create mode 100644 src/GitHub.App/Services/LocalRepositories.cs create mode 100644 src/GitHub.Exports.Reactive/Services/LocalRepositoriesExtensions.cs create mode 100644 src/GitHub.Exports/Services/ILocalRepositories.cs create mode 100644 src/GitHub.Extensions/IReadOnlyObservableCollection.cs create mode 100644 src/GitHub.Extensions/ObservableCollectionEx.cs create mode 100644 src/UnitTests/GitHub.App/Services/LocalRepositoriesTests.cs diff --git a/src/GitHub.App/GitHub.App.csproj b/src/GitHub.App/GitHub.App.csproj index 206ece6bf9..a8372b644b 100644 --- a/src/GitHub.App/GitHub.App.csproj +++ b/src/GitHub.App/GitHub.App.csproj @@ -130,6 +130,7 @@ + diff --git a/src/GitHub.App/Services/LocalRepositories.cs b/src/GitHub.App/Services/LocalRepositories.cs new file mode 100644 index 0000000000..bd092a4b11 --- /dev/null +++ b/src/GitHub.App/Services/LocalRepositories.cs @@ -0,0 +1,47 @@ +using System; +using System.ComponentModel.Composition; +using System.Linq; +using System.Threading.Tasks; +using GitHub.Extensions; +using GitHub.Helpers; +using GitHub.Models; +using GitHub.Services; + +namespace GitHub.App.Services +{ + /// + /// Stores a collection of known local repositories. + /// + /// + /// The list of repositories exposed here is the list of local repositories known to Team + /// Explorer. + /// + [Export(typeof(ILocalRepositories))] + public class LocalRepositories : ILocalRepositories + { + readonly IVSGitServices vsGitServices; + + [ImportingConstructor] + public LocalRepositories(IVSGitServices vsGitServices) + { + this.vsGitServices = vsGitServices; + } + + /// + public async Task Refresh() + { + await ThreadingHelper.SwitchToPoolThreadAsync(); + var list = vsGitServices.GetKnownRepositories(); + await ThreadingHelper.SwitchToMainThreadAsync(); + + repositories.Except(list).ToList().ForEach(x => repositories.Remove(x)); + list.Except(repositories).ToList().ForEach(x => repositories.Add(x)); + } + + readonly ObservableCollectionEx repositories + = new ObservableCollectionEx(); + + /// + public IReadOnlyObservableCollection Repositories => repositories; + } +} diff --git a/src/GitHub.Exports.Reactive/Collections/TrackingCollection.cs b/src/GitHub.Exports.Reactive/Collections/TrackingCollection.cs index 6bb77050ee..ca82f1d802 100644 --- a/src/GitHub.Exports.Reactive/Collections/TrackingCollection.cs +++ b/src/GitHub.Exports.Reactive/Collections/TrackingCollection.cs @@ -164,7 +164,7 @@ static void UpdateStickieItems( /// for T /// /// - public class TrackingCollection : ObservableCollection, ITrackingCollection, IDisposable + public class TrackingCollection : ObservableCollection, ITrackingCollection, IReadOnlyObservableCollection, IDisposable where T : class, ICopyable { enum TheAction diff --git a/src/GitHub.Exports.Reactive/GitHub.Exports.Reactive.csproj b/src/GitHub.Exports.Reactive/GitHub.Exports.Reactive.csproj index a647c32267..3091dca509 100644 --- a/src/GitHub.Exports.Reactive/GitHub.Exports.Reactive.csproj +++ b/src/GitHub.Exports.Reactive/GitHub.Exports.Reactive.csproj @@ -112,6 +112,7 @@ + diff --git a/src/GitHub.Exports.Reactive/Services/LocalRepositoriesExtensions.cs b/src/GitHub.Exports.Reactive/Services/LocalRepositoriesExtensions.cs new file mode 100644 index 0000000000..76c808724a --- /dev/null +++ b/src/GitHub.Exports.Reactive/Services/LocalRepositoriesExtensions.cs @@ -0,0 +1,29 @@ +using System; +using GitHub.Models; +using GitHub.Primitives; +using ReactiveUI; + +namespace GitHub.Services +{ + /// + /// Implements extension methods for . + /// + public static class LocalRepositoriesExtensions + { + /// + /// Gets a derived collection that contains all known repositories with the specified + /// clone URL, ordered by name. + /// + /// The local repositories object. + /// The address. + public static IReactiveDerivedList GetRepositoriesForAddress( + this ILocalRepositories repos, + HostAddress address) + { + return repos.Repositories.CreateDerivedCollection( + x => x, + x => x.CloneUrl != null && address.Equals(HostAddress.Create(x.CloneUrl)), + OrderedComparer.OrderBy(x => x.Name).Compare); + } + } +} diff --git a/src/GitHub.Exports/GitHub.Exports.csproj b/src/GitHub.Exports/GitHub.Exports.csproj index 525fd7a02d..955132356b 100644 --- a/src/GitHub.Exports/GitHub.Exports.csproj +++ b/src/GitHub.Exports/GitHub.Exports.csproj @@ -137,6 +137,7 @@ + diff --git a/src/GitHub.Exports/Models/IConnection.cs b/src/GitHub.Exports/Models/IConnection.cs index 26e3a172e1..93d29522fe 100644 --- a/src/GitHub.Exports/Models/IConnection.cs +++ b/src/GitHub.Exports/Models/IConnection.cs @@ -6,12 +6,11 @@ namespace GitHub.Models { - public interface IConnection : IDisposable + public interface IConnection { HostAddress HostAddress { get; } string Username { get; } IObservable Login(); void Logout(); - ObservableCollection Repositories { get; } } } diff --git a/src/GitHub.Exports/Services/Connection.cs b/src/GitHub.Exports/Services/Connection.cs index 1fc3ad53a9..655d475b23 100644 --- a/src/GitHub.Exports/Services/Connection.cs +++ b/src/GitHub.Exports/Services/Connection.cs @@ -1,9 +1,6 @@ using System; using GitHub.Models; using GitHub.Primitives; -using System.Threading.Tasks; -using System.Collections.ObjectModel; -using System.Collections.Specialized; namespace GitHub.Services { @@ -16,12 +13,10 @@ public Connection(IConnectionManager cm, HostAddress hostAddress, string userNam manager = cm; HostAddress = hostAddress; Username = userName; - Repositories = new ObservableCollection(); } public HostAddress HostAddress { get; private set; } public string Username { get; private set; } - public ObservableCollection Repositories { get; } public IObservable Login() { @@ -32,25 +27,5 @@ public void Logout() { manager.RequestLogout(this); } - - #region IDisposable Support - private bool disposed = false; // To detect redundant calls - - protected virtual void Dispose(bool disposing) - { - if (!disposed) - { - if (disposing) - Repositories.Clear(); - disposed = true; - } - } - - public void Dispose() - { - Dispose(true); - GC.SuppressFinalize(this); - } - #endregion } } diff --git a/src/GitHub.Exports/Services/IConnectionManager.cs b/src/GitHub.Exports/Services/IConnectionManager.cs index 90599eb3be..7631ce49e7 100644 --- a/src/GitHub.Exports/Services/IConnectionManager.cs +++ b/src/GitHub.Exports/Services/IConnectionManager.cs @@ -20,6 +20,5 @@ public interface IConnectionManager // for telling IRepositoryHosts that we need to login from cache [SuppressMessage("Microsoft.Design", "CA1009:DeclareEventHandlersCorrectly")] event Func> DoLogin; - Task RefreshRepositories(); } } diff --git a/src/GitHub.Exports/Services/ILocalRepositories.cs b/src/GitHub.Exports/Services/ILocalRepositories.cs new file mode 100644 index 0000000000..1d77fbd13c --- /dev/null +++ b/src/GitHub.Exports/Services/ILocalRepositories.cs @@ -0,0 +1,23 @@ +using System; +using System.Threading.Tasks; +using GitHub.Extensions; +using GitHub.Models; + +namespace GitHub.Services +{ + /// + /// Stores a collection of known local repositories. + /// + public interface ILocalRepositories + { + /// + /// Gets the currently known local repositories. + /// + IReadOnlyObservableCollection Repositories { get; } + + /// + /// Updates . + /// + Task Refresh(); + } +} diff --git a/src/GitHub.Extensions/GitHub.Extensions.csproj b/src/GitHub.Extensions/GitHub.Extensions.csproj index 1696f0c55b..6fa1a678d6 100644 --- a/src/GitHub.Extensions/GitHub.Extensions.csproj +++ b/src/GitHub.Extensions/GitHub.Extensions.csproj @@ -59,7 +59,9 @@ + + Properties\SolutionInfo.cs diff --git a/src/GitHub.Extensions/IReadOnlyObservableCollection.cs b/src/GitHub.Extensions/IReadOnlyObservableCollection.cs new file mode 100644 index 0000000000..847512ce92 --- /dev/null +++ b/src/GitHub.Extensions/IReadOnlyObservableCollection.cs @@ -0,0 +1,17 @@ +using System.Collections.Generic; +using System.Collections.ObjectModel; +using System.Collections.Specialized; +using System.ComponentModel; + +namespace GitHub.Extensions +{ + /// + /// Represents a read-only interface to an + /// + /// The type of elements in the collection. + public interface IReadOnlyObservableCollection : IReadOnlyList, + INotifyCollectionChanged, + INotifyPropertyChanged + { + } +} \ No newline at end of file diff --git a/src/GitHub.Extensions/ObservableCollectionEx.cs b/src/GitHub.Extensions/ObservableCollectionEx.cs new file mode 100644 index 0000000000..6e3c50c39a --- /dev/null +++ b/src/GitHub.Extensions/ObservableCollectionEx.cs @@ -0,0 +1,42 @@ +using System.Collections.Generic; +using System.Collections.ObjectModel; + +namespace GitHub.Extensions +{ + /// + /// A non-braindead way to expose a read-only view on an . + /// + /// The type of elements in the collection. + /// + /// fails in its only purpose by Not. Freaking. + /// Exposing. INotifyCollectionChanged. We define our own + /// type and use this class to expose it. Seriously. + /// + public class ObservableCollectionEx : ObservableCollection, IReadOnlyObservableCollection + { + /// + /// Initializes a new instance of the class. + /// + public ObservableCollectionEx() + { + } + + /// + /// Initializes a new instance of the class that + /// contains elements copied from the specified list. + /// + public ObservableCollectionEx(List list) + : base(list) + { + } + + /// + /// Initializes a new instance of the class that + /// contains elements copied from the specified collection. + /// + public ObservableCollectionEx(IEnumerable collection) + : base(collection) + { + } + } +} \ No newline at end of file diff --git a/src/GitHub.TeamFoundation.14/Connect/GitHubConnectSection.cs b/src/GitHub.TeamFoundation.14/Connect/GitHubConnectSection.cs index 078fdb9bf2..0862970d61 100644 --- a/src/GitHub.TeamFoundation.14/Connect/GitHubConnectSection.cs +++ b/src/GitHub.TeamFoundation.14/Connect/GitHubConnectSection.cs @@ -32,6 +32,7 @@ public class GitHubConnectSection : TeamExplorerSectionBase, IGitHubConnectSecti readonly int sectionIndex; readonly IDialogService dialogService; readonly IRepositoryCloneService cloneService; + readonly ILocalRepositories localRepositories; bool isCloning; bool isCreating; @@ -98,6 +99,7 @@ public GitHubConnectSection(IGitHubServiceProvider serviceProvider, IVSServices vsServices, IRepositoryCloneService cloneService, IDialogService dialogService, + ILocalRepositories localRepositories, int index) : base(serviceProvider, apiFactory, holder, manager) { @@ -108,6 +110,7 @@ public GitHubConnectSection(IGitHubServiceProvider serviceProvider, Guard.ArgumentNotNull(vsServices, nameof(vsServices)); Guard.ArgumentNotNull(cloneService, nameof(cloneService)); Guard.ArgumentNotNull(dialogService, nameof(dialogService)); + Guard.ArgumentNotNull(localRepositories, nameof(localRepositories)); Title = "GitHub"; IsEnabled = true; @@ -119,6 +122,7 @@ public GitHubConnectSection(IGitHubServiceProvider serviceProvider, this.vsServices = vsServices; this.cloneService = cloneService; this.dialogService = dialogService; + this.localRepositories = localRepositories; Clone = CreateAsyncCommandHack(DoClone); @@ -194,8 +198,8 @@ protected void Refresh(IConnection connection) if (connection != SectionConnection) { SectionConnection = connection; - Repositories = SectionConnection.Repositories.CreateDerivedCollection(x => x, - orderer: OrderedComparer.OrderBy(x => x.Name).Compare); + Repositories?.Dispose(); + Repositories = localRepositories.GetRepositoriesForAddress(connection.HostAddress); Repositories.CollectionChanged += UpdateRepositoryList; Title = connection.HostAddress.Title; IsVisible = true; @@ -368,7 +372,7 @@ async Task RefreshRepositories() { // TODO: This is wasteful as we can be calling it multiple times for a single changed // signal, once from each section. Needs refactoring. - await connectionManager.RefreshRepositories(); + await localRepositories.Refresh(); RaisePropertyChanged("Repositories"); // trigger a re-check of the visibility of the listview based on item count } diff --git a/src/GitHub.TeamFoundation.14/Connect/GitHubConnectSection0.cs b/src/GitHub.TeamFoundation.14/Connect/GitHubConnectSection0.cs index 05ec3a53e6..652456eaa7 100644 --- a/src/GitHub.TeamFoundation.14/Connect/GitHubConnectSection0.cs +++ b/src/GitHub.TeamFoundation.14/Connect/GitHubConnectSection0.cs @@ -21,8 +21,9 @@ public GitHubConnectSection0(IGitHubServiceProvider serviceProvider, IPackageSettings settings, IVSServices vsServices, IRepositoryCloneService cloneService, - IDialogService dialogService) - : base(serviceProvider, apiFactory, holder, manager, settings, vsServices, cloneService, dialogService, 0) + IDialogService dialogService, + ILocalRepositories localRepositories) + : base(serviceProvider, apiFactory, holder, manager, settings, vsServices, cloneService, dialogService, localRepositories, 0) { } } diff --git a/src/GitHub.TeamFoundation.14/Connect/GitHubConnectSection1.cs b/src/GitHub.TeamFoundation.14/Connect/GitHubConnectSection1.cs index 5866f5958c..e2c7070521 100644 --- a/src/GitHub.TeamFoundation.14/Connect/GitHubConnectSection1.cs +++ b/src/GitHub.TeamFoundation.14/Connect/GitHubConnectSection1.cs @@ -21,8 +21,9 @@ public GitHubConnectSection1(IGitHubServiceProvider serviceProvider, IPackageSettings settings, IVSServices vsServices, IRepositoryCloneService cloneService, - IDialogService dialogService) - : base(serviceProvider, apiFactory, holder, manager, settings, vsServices, cloneService, dialogService, 1) + IDialogService dialogService, + ILocalRepositories localRepositories) + : base(serviceProvider, apiFactory, holder, manager, settings, vsServices, cloneService, dialogService, localRepositories, 1) { } } diff --git a/src/GitHub.VisualStudio/Services/ConnectionManager.cs b/src/GitHub.VisualStudio/Services/ConnectionManager.cs index 08498332bf..fc3ae56adc 100644 --- a/src/GitHub.VisualStudio/Services/ConnectionManager.cs +++ b/src/GitHub.VisualStudio/Services/ConnectionManager.cs @@ -17,7 +17,6 @@ namespace GitHub.VisualStudio [PartCreationPolicy(CreationPolicy.Shared)] public class ConnectionManager : IConnectionManager { - readonly IVSGitServices vsGitServices; readonly IConnectionCache cache; readonly IKeychain keychain; readonly ILoginManager loginManager; @@ -27,13 +26,11 @@ public class ConnectionManager : IConnectionManager [ImportingConstructor] public ConnectionManager( - IVSGitServices vsGitServices, IConnectionCache cache, IKeychain keychain, ILoginManager loginManager, IApiClientFactory apiClientFactory) { - this.vsGitServices = vsGitServices; this.cache = cache; this.keychain = keychain; this.loginManager = loginManager; @@ -87,19 +84,6 @@ public void RequestLogout(IConnection connection) Connections.Remove(connection); } - public async Task RefreshRepositories() - { - var list = await Task.Run(() => vsGitServices.GetKnownRepositories()); - list.GroupBy(r => Connections.FirstOrDefault(c => r.CloneUrl != null && c.HostAddress.Equals(HostAddress.Create(r.CloneUrl)))) - .Where(g => g.Key != null) - .ForEach(g => - { - var repos = g.Key.Repositories; - repos.Except(g).ToList().ForEach(c => repos.Remove(c)); - g.Except(repos).ToList().ForEach(c => repos.Add(c)); - }); - } - IConnection SetupConnection(HostAddress address, string username) { var conn = new Connection(this, address, username); @@ -115,7 +99,6 @@ void RefreshConnections(object sender, System.Collections.Specialized.NotifyColl // RepositoryHosts hasn't been loaded so it can't handle logging out, we have to do it ourselves if (DoLogin == null) keychain.Delete(c.HostAddress).Forget(); - c.Dispose(); } } diff --git a/src/UnitTests/GitHub.App/Services/LocalRepositoriesTests.cs b/src/UnitTests/GitHub.App/Services/LocalRepositoriesTests.cs new file mode 100644 index 0000000000..8ec590721d --- /dev/null +++ b/src/UnitTests/GitHub.App/Services/LocalRepositoriesTests.cs @@ -0,0 +1,136 @@ +using System; +using System.Collections.Generic; +using System.Linq; +using System.Threading.Tasks; +using GitHub.App.Services; +using GitHub.Models; +using GitHub.Primitives; +using GitHub.Services; +using NSubstitute; +using Xunit; + +public class LocalRepositoriesTests : TestBaseClass +{ + const string GitHubAddress = "https://github.com"; + + [Fact] + public void RepositoriesShouldInitiallyBeEmpty() + { + var service = CreateVSGitServices("repo1", "repo2"); + var target = new LocalRepositories(service); + + Assert.Empty(target.Repositories); + } + + [Fact] + public async Task RefreshShouldLoadRepositories() + { + var service = CreateVSGitServices("repo1", "repo2"); + var target = new LocalRepositories(service); + + await target.Refresh(); + + Assert.Equal( + new[] { "repo1", "repo2" }, + target.Repositories.Select(x => x.Name).ToList()); + } + + [Fact] + public async Task RefreshShouldAddNewRepository() + { + var service = CreateVSGitServices("repo1", "repo2"); + var target = new LocalRepositories(service); + + await target.Refresh(); + + Assert.Equal(2, target.Repositories.Count); + + var existing = service.GetKnownRepositories(); + var newRepo = CreateRepository("new"); + service.GetKnownRepositories().Returns(existing.Concat(new[] { newRepo })); + + await target.Refresh(); + + Assert.Equal( + new[] { "repo1", "repo2", "new" }, + target.Repositories.Select(x => x.Name).ToList()); + } + + [Fact] + public async Task RefreshShouldRemoveRepository() + { + var service = CreateVSGitServices("repo1", "repo2"); + var target = new LocalRepositories(service); + + await target.Refresh(); + + Assert.Equal(2, target.Repositories.Count); + + var existing = service.GetKnownRepositories(); + service.GetKnownRepositories().Returns(existing.Skip(1).Take(1)); + + await target.Refresh(); + + Assert.Equal( + new[] { "repo2" }, + target.Repositories.Select(x => x.Name).ToList()); + } + + [Fact] + public async Task GetRepositoriesForAddressShouldFilterRepositories() + { + var service = CreateVSGitServices( + Tuple.Create("repo1", GitHubAddress), + Tuple.Create("repo2", GitHubAddress), + Tuple.Create("repo2", "https://another.com")); + var target = new LocalRepositories(service); + + await target.Refresh(); + + Assert.Equal(3, target.Repositories.Count); + + var result = target.GetRepositoriesForAddress(HostAddress.Create(GitHubAddress)); + + Assert.Equal(2, result.Count); + } + + [Fact] + public async Task GetRepositoriesForAddressShouldSortRepositories() + { + var service = CreateVSGitServices("c", "a", "b"); + var target = new LocalRepositories(service); + + await target.Refresh(); + var result = target.GetRepositoriesForAddress(HostAddress.Create(GitHubAddress)); + + Assert.Equal( + new[] { "a", "b", "c" }, + result.Select(x => x.Name).ToList()); + } + + static IVSGitServices CreateVSGitServices(params string[] names) + { + return CreateVSGitServices(names.Select(x => Tuple.Create(x, GitHubAddress)).ToArray()); + } + + static IVSGitServices CreateVSGitServices(params Tuple[] namesAndAddresses) + { + var result = Substitute.For(); + var repositories = new List(namesAndAddresses.Select(CreateRepository)); + result.GetKnownRepositories().Returns(repositories); + return result; + } + + static ILocalRepositoryModel CreateRepository(string name) + { + return CreateRepository(Tuple.Create(name, "https://github.com")); + } + + static ILocalRepositoryModel CreateRepository(Tuple nameAndAddress) + { + var result = Substitute.For(); + result.Name.Returns(nameAndAddress.Item1); + result.CloneUrl.Returns(new UriString(nameAndAddress.Item2)); + return result; + } +} diff --git a/src/UnitTests/GitHub.VisualStudio/Services/ConnectionManagerTests.cs b/src/UnitTests/GitHub.VisualStudio/Services/ConnectionManagerTests.cs index 269d61ff60..0671760d28 100644 --- a/src/UnitTests/GitHub.VisualStudio/Services/ConnectionManagerTests.cs +++ b/src/UnitTests/GitHub.VisualStudio/Services/ConnectionManagerTests.cs @@ -31,7 +31,7 @@ public void IsSavedToDiskWhenConnectionAdded() var keychain = Substitute.For(); var loginManager = Substitute.For(); var apiClientFactory = Substitute.For(); - var manager = new ConnectionManager(Substitutes.IVSGitServices, cache, keychain, loginManager, apiClientFactory); + var manager = new ConnectionManager(cache, keychain, loginManager, apiClientFactory); manager.Connections.Add(new Connection(manager, HostAddress.GitHubDotComHostAddress, "coolio")); From d98ec51f77e00f0bfc75423e683b3dc72bc1f453 Mon Sep 17 00:00:00 2001 From: Steven Kirk Date: Mon, 23 Oct 2017 11:03:58 +0200 Subject: [PATCH 02/16] Refactored ConnectionManager/Connection. --- src/GitHub.App/Controllers/UIController.cs | 8 +- src/GitHub.App/Models/RepositoryHosts.cs | 28 +- src/GitHub.App/SampleData/SampleViewModels.cs | 18 +- .../ViewModels/RepositoryPublishViewModel.cs | 2 +- .../Extensions/ConnectionManagerExtensions.cs | 32 ++- .../ViewModels/IRepositoryPublishViewModel.cs | 3 +- src/GitHub.Exports/Models/IConnection.cs | 39 ++- src/GitHub.Exports/Services/Connection.cs | 36 ++- .../Services/IConnectionManager.cs | 65 ++++- .../Connect/GitHubConnectSection.cs | 2 +- .../Services/ConnectionManager.cs | 170 ++++++------ .../UI/Views/GitHubPaneViewModel.cs | 3 +- .../Controllers/UIControllerTests.cs | 52 +++- .../RepositoryPublishViewModelTests.cs | 13 +- .../Services/ConnectionManagerTests.cs | 251 ++++++++++++++++-- .../UI/Views/GitHubPaneViewModelTests.cs | 4 +- src/UnitTests/Substitutes.cs | 15 ++ 17 files changed, 520 insertions(+), 221 deletions(-) diff --git a/src/GitHub.App/Controllers/UIController.cs b/src/GitHub.App/Controllers/UIController.cs index d4eaaf01d8..728873bda7 100644 --- a/src/GitHub.App/Controllers/UIController.cs +++ b/src/GitHub.App/Controllers/UIController.cs @@ -219,13 +219,7 @@ public void Start() else // sanity check: it makes zero sense to pass a connection in when calling the auth flow Debug.Assert(false, "Calling the auth flow with a connection makes no sense!"); - connection.Login() - .ObserveOn(RxApp.MainThreadScheduler) - .Subscribe(_ => { }, () => - { - Debug.WriteLine("Start ({0})", GetHashCode()); - Fire(Trigger.Next); - }); + Fire(Trigger.Next); } else { diff --git a/src/GitHub.App/Models/RepositoryHosts.cs b/src/GitHub.App/Models/RepositoryHosts.cs index 8b547496fe..696779cb57 100644 --- a/src/GitHub.App/Models/RepositoryHosts.cs +++ b/src/GitHub.App/Models/RepositoryHosts.cs @@ -75,11 +75,6 @@ public RepositoryHosts( (githubLoggedIn, enterpriseLoggedIn) => githubLoggedIn.Value || enterpriseLoggedIn.Value) .ToProperty(this, x => x.IsLoggedInToAnyHost); - // This part is strictly to support having the IConnectionManager request that a connection - // be logged in. It doesn't know about hosts or load anything reactive, so it gets - // informed of logins by an observable returned by the event - connectionManager.DoLogin += RunLoginHandler; - // monitor the list of connections so we can log out hosts when connections are removed disposables.Add( connectionManager.Connections.CreateDerivedCollection(x => x) @@ -100,24 +95,6 @@ public RepositoryHosts( disposables.Add(persistEntepriseHostObs.Subscribe()); } - IObservable RunLoginHandler(IConnection connection) - { - Guard.ArgumentNotNull(connection, nameof(connection)); - - var handler = new ReplaySubject(); - var address = connection.HostAddress; - var host = LookupHost(address); - if (host == DisconnectedRepositoryHost) - LogInFromCache(address) - .Subscribe(c => handler.OnNext(connection), () => handler.OnCompleted()); - else - { - handler.OnNext(connection); - handler.OnCompleted(); - } - return handler; - } - public IRepositoryHost LookupHost(HostAddress address) { if (address == GitHubHost.Address) @@ -164,7 +141,7 @@ public IObservable LogIn( else enterpriseHost = host; - connectionManager.AddConnection(address, usernameOrEmail); + connectionManager.LogIn(address, usernameOrEmail, password); if (isDotCom) this.RaisePropertyChanged(nameof(GitHubHost)); @@ -223,7 +200,7 @@ public IObservable LogOut(IRepositoryHost host) GitHubHost = null; else EnterpriseHost = null; - connectionManager.RemoveConnection(address); + connectionManager.LogOut(address); RepositoryHostFactory.Remove(host); }); } @@ -237,7 +214,6 @@ protected void Dispose(bool disposing) try { - connectionManager.DoLogin -= RunLoginHandler; disposables.Dispose(); } catch (Exception e) diff --git a/src/GitHub.App/SampleData/SampleViewModels.cs b/src/GitHub.App/SampleData/SampleViewModels.cs index beb1153fb6..da44e1738b 100644 --- a/src/GitHub.App/SampleData/SampleViewModels.cs +++ b/src/GitHub.App/SampleData/SampleViewModels.cs @@ -200,23 +200,15 @@ class Conn : IConnection public string Username { get; set; } public ObservableCollection Repositories { get; set; } - public IObservable Login() - { - return null; - } + public Octokit.User User => null; + public bool IsLoggedIn => true; - public void Logout() - { - } - - public void Dispose() - { - } + public Exception ConnectionError => null; } public RepositoryPublishViewModelDesigner() { - Connections = new ObservableCollection + Connections = new ObservableCollectionEx { new Conn() { HostAddress = new HostAddress() }, new Conn() { HostAddress = HostAddress.Create("ghe.io") } @@ -238,7 +230,7 @@ public IReactiveCommand PublishRepository private set; } - public ObservableCollection Connections + public IReadOnlyObservableCollection Connections { get; private set; diff --git a/src/GitHub.App/ViewModels/RepositoryPublishViewModel.cs b/src/GitHub.App/ViewModels/RepositoryPublishViewModel.cs index ce55cf07bd..2c22ac7e30 100644 --- a/src/GitHub.App/ViewModels/RepositoryPublishViewModel.cs +++ b/src/GitHub.App/ViewModels/RepositoryPublishViewModel.cs @@ -117,7 +117,7 @@ public RepositoryPublishViewModel( public bool CanKeepPrivate { get { return canKeepPrivate.Value; } } public IReactiveCommand PublishRepository { get; private set; } - public ObservableCollection Connections { get; private set; } + public IReadOnlyObservableCollection Connections { get; private set; } IConnection selectedConnection; public IConnection SelectedConnection diff --git a/src/GitHub.Exports.Reactive/Extensions/ConnectionManagerExtensions.cs b/src/GitHub.Exports.Reactive/Extensions/ConnectionManagerExtensions.cs index 762f9a614d..64e11e3e4e 100644 --- a/src/GitHub.Exports.Reactive/Extensions/ConnectionManagerExtensions.cs +++ b/src/GitHub.Exports.Reactive/Extensions/ConnectionManagerExtensions.cs @@ -1,6 +1,9 @@ using System; using System.Linq; using System.Reactive.Linq; +using System.Reactive.Threading.Tasks; +using System.Threading.Tasks; +using GitHub.Api; using GitHub.Models; using GitHub.Primitives; using GitHub.Services; @@ -12,33 +15,40 @@ public static class ConnectionManagerExtensions public static IObservable IsLoggedIn(this IConnectionManager cm, IRepositoryHosts hosts) { Guard.ArgumentNotNull(hosts, nameof(hosts)); - return cm.Connections.ToObservable() - .SelectMany(c => c.Login()) - .Any(c => hosts.LookupHost(c.HostAddress).IsLoggedIn); + + return Observable.FromAsync(async () => + { + var connections = await cm.GetLoadedConnections(); + return connections.Any(x => x.ConnectionError == null); + }); } public static IObservable IsLoggedIn(this IConnectionManager cm, IRepositoryHosts hosts, HostAddress address) { Guard.ArgumentNotNull(hosts, nameof(hosts)); Guard.ArgumentNotNull(address, nameof(address)); - return cm.Connections.ToObservable() - .Where(c => c.HostAddress.Equals(address)) - .SelectMany(c => c.Login()) - .Any(c => hosts.LookupHost(c.HostAddress).IsLoggedIn); + + return Observable.FromAsync(async () => + { + var connections = await cm.GetLoadedConnections(); + return connections.Any(x => x.HostAddress == address && x.ConnectionError == null); + }); } public static IObservable IsLoggedIn(this IConnection connection, IRepositoryHosts hosts) { Guard.ArgumentNotNull(hosts, nameof(hosts)); - return connection?.Login().Any(c => hosts.LookupHost(c.HostAddress).IsLoggedIn) ?? Observable.Return(false); + + return Observable.Return(connection?.IsLoggedIn ?? false); } public static IObservable GetLoggedInConnections(this IConnectionManager cm, IRepositoryHosts hosts) { Guard.ArgumentNotNull(hosts, nameof(hosts)); - return cm.Connections.ToObservable() - .SelectMany(c => c.Login()) - .Where(c => hosts.LookupHost(c.HostAddress).IsLoggedIn); + + return cm.GetLoadedConnections() + .ToObservable() + .Select(x => x.FirstOrDefault(y => y.IsLoggedIn)); } public static IObservable LookupConnection(this IConnectionManager cm, ILocalRepositoryModel repository) diff --git a/src/GitHub.Exports.Reactive/ViewModels/IRepositoryPublishViewModel.cs b/src/GitHub.Exports.Reactive/ViewModels/IRepositoryPublishViewModel.cs index ca54396dd8..cb188876b2 100644 --- a/src/GitHub.Exports.Reactive/ViewModels/IRepositoryPublishViewModel.cs +++ b/src/GitHub.Exports.Reactive/ViewModels/IRepositoryPublishViewModel.cs @@ -2,12 +2,13 @@ using GitHub.Models; using ReactiveUI; using System.Collections.ObjectModel; +using GitHub.Extensions; namespace GitHub.ViewModels { public interface IRepositoryPublishViewModel : IRepositoryForm { - ObservableCollection Connections { get; } + IReadOnlyObservableCollection Connections { get; } /// /// Command that creates the repository. diff --git a/src/GitHub.Exports/Models/IConnection.cs b/src/GitHub.Exports/Models/IConnection.cs index 93d29522fe..3c979fa453 100644 --- a/src/GitHub.Exports/Models/IConnection.cs +++ b/src/GitHub.Exports/Models/IConnection.cs @@ -1,16 +1,41 @@ -using GitHub.Primitives; -using System; -using System.Collections.ObjectModel; -using System.Collections.Specialized; -using System.Threading.Tasks; +using System; +using GitHub.Primitives; +using Octokit; namespace GitHub.Models { + /// + /// Represents a configured connection to a GitHub account. + /// public interface IConnection { + /// + /// Gets the host address of the GitHub instance. + /// HostAddress HostAddress { get; } + + /// + /// Gets the username of the GitHub account. + /// string Username { get; } - IObservable Login(); - void Logout(); + + /// + /// Gets the logged in user. + /// + /// + /// This may be null if is false. + /// + User User { get; } + + /// + /// Gets a value indicating whether the login of the account succeeded. + /// + bool IsLoggedIn { get; } + + /// + /// Gets the exception that occurred when trying to log in, if is + /// false. + /// + Exception ConnectionError { get; } } } diff --git a/src/GitHub.Exports/Services/Connection.cs b/src/GitHub.Exports/Services/Connection.cs index 655d475b23..ba87d3dfae 100644 --- a/src/GitHub.Exports/Services/Connection.cs +++ b/src/GitHub.Exports/Services/Connection.cs @@ -4,28 +4,36 @@ namespace GitHub.Services { + /// + /// Represents a configured connection to a GitHub account. + /// public class Connection : IConnection { - readonly IConnectionManager manager; - - public Connection(IConnectionManager cm, HostAddress hostAddress, string userName) + public Connection( + HostAddress hostAddress, + string userName, + Octokit.User user, + Exception connectionError) { - manager = cm; HostAddress = hostAddress; Username = userName; + User = user; + ConnectionError = connectionError; } - public HostAddress HostAddress { get; private set; } - public string Username { get; private set; } + /// + public HostAddress HostAddress { get; } - public IObservable Login() - { - return manager.RequestLogin(this); - } + /// + public string Username { get; } - public void Logout() - { - manager.RequestLogout(this); - } + /// + public Octokit.User User { get; } + + /// + public bool IsLoggedIn => ConnectionError == null; + + /// + public Exception ConnectionError { get; } } } diff --git a/src/GitHub.Exports/Services/IConnectionManager.cs b/src/GitHub.Exports/Services/IConnectionManager.cs index 7631ce49e7..66045ec693 100644 --- a/src/GitHub.Exports/Services/IConnectionManager.cs +++ b/src/GitHub.Exports/Services/IConnectionManager.cs @@ -1,24 +1,65 @@ using System; -using System.Collections.ObjectModel; -using System.Diagnostics.CodeAnalysis; using System.Threading.Tasks; +using GitHub.Extensions; using GitHub.Models; using GitHub.Primitives; namespace GitHub.Services { + /// + /// Manages the configured s to GitHub instances. + /// public interface IConnectionManager { - IConnection CreateConnection(HostAddress address, string username); - bool AddConnection(HostAddress address, string username); - bool RemoveConnection(HostAddress address); - ObservableCollection Connections { get; } + /// + /// Gets a collection containing the current connections. + /// + /// + /// This collection is lazily initialized: the first time the + /// property is accessed, an async load of the configured connections is started. If you + /// want to ensure that all connections have been loaded, call + /// . + /// + IReadOnlyObservableCollection Connections { get; } - IObservable RequestLogin(IConnection connection); - void RequestLogout(IConnection connection); + /// + /// Gets a connection with the specified host address. + /// + /// The address. + /// + /// A task returning the requested connection, or null if the connection was not found. + /// + Task GetConnection(HostAddress address); - // for telling IRepositoryHosts that we need to login from cache - [SuppressMessage("Microsoft.Design", "CA1009:DeclareEventHandlersCorrectly")] - event Func> DoLogin; + /// + /// Gets the collection, after ensuring that it is fully loaded. + /// + /// + /// A task returning the fully loaded connections collection. + /// + Task> GetLoadedConnections(); + + /// + /// Attempts to login to a GitHub instance. + /// + /// The instance address. + /// The username. + /// The password. + /// + /// A connection if the login succeded. If the login fails, throws an exception. An + /// exception is also thrown if an existing connection with the same host address already + /// exists. + /// + /// + /// A connection to the host already exists. + /// + Task LogIn(HostAddress address, string username, string password); + + /// + /// Logs out of a GitHub instance. + /// + /// + /// A task tracking the operation. + Task LogOut(HostAddress address); } -} +} \ No newline at end of file diff --git a/src/GitHub.TeamFoundation.14/Connect/GitHubConnectSection.cs b/src/GitHub.TeamFoundation.14/Connect/GitHubConnectSection.cs index 0862970d61..019ec059a8 100644 --- a/src/GitHub.TeamFoundation.14/Connect/GitHubConnectSection.cs +++ b/src/GitHub.TeamFoundation.14/Connect/GitHubConnectSection.cs @@ -383,7 +383,7 @@ public void DoCreate() public void SignOut() { - SectionConnection.Logout(); + connectionManager.LogOut(SectionConnection.HostAddress); } public void Login() diff --git a/src/GitHub.VisualStudio/Services/ConnectionManager.cs b/src/GitHub.VisualStudio/Services/ConnectionManager.cs index fc3ae56adc..e624c62bfd 100644 --- a/src/GitHub.VisualStudio/Services/ConnectionManager.cs +++ b/src/GitHub.VisualStudio/Services/ConnectionManager.cs @@ -1,4 +1,5 @@ using System; +using System.Collections.Generic; using System.Collections.ObjectModel; using System.ComponentModel.Composition; using System.Linq; @@ -6,140 +7,149 @@ using System.Threading.Tasks; using GitHub.Api; using GitHub.Extensions; -using GitHub.Factories; using GitHub.Models; using GitHub.Primitives; using GitHub.Services; +using IGitHubClient = Octokit.IGitHubClient; +using GitHubClient = Octokit.GitHubClient; +using User = Octokit.User; namespace GitHub.VisualStudio { + /// + /// Manages the configured s to GitHub instances. + /// [Export(typeof(IConnectionManager))] - [PartCreationPolicy(CreationPolicy.Shared)] public class ConnectionManager : IConnectionManager { + readonly IProgram program; readonly IConnectionCache cache; readonly IKeychain keychain; readonly ILoginManager loginManager; - readonly IApiClientFactory apiClientFactory; - - public event Func> DoLogin; + readonly TaskCompletionSource loaded; + readonly Lazy> connections; [ImportingConstructor] public ConnectionManager( + IProgram program, IConnectionCache cache, IKeychain keychain, - ILoginManager loginManager, - IApiClientFactory apiClientFactory) + ILoginManager loginManager) { + this.program = program; this.cache = cache; this.keychain = keychain; this.loginManager = loginManager; - this.apiClientFactory = apiClientFactory; - - Connections = new ObservableCollection(); - LoadConnectionsFromCache().Forget(); + loaded = new TaskCompletionSource(); + connections = new Lazy>( + this.CreateConnections, + LazyThreadSafetyMode.ExecutionAndPublication); } - public IConnection CreateConnection(HostAddress address, string username) + /// + public IReadOnlyObservableCollection Connections => connections.Value; + + /// + public async Task GetConnection(HostAddress address) { - return SetupConnection(address, username); + return (await GetLoadedConnections()).FirstOrDefault(x => x.HostAddress == address); } - public bool AddConnection(HostAddress address, string username) + /// + public async Task> GetLoadedConnections() { - if (Connections.FirstOrDefault(x => x.HostAddress.Equals(address)) != null) - return false; - Connections.Add(SetupConnection(address, username)); - return true; + return await GetLoadedConnectionsInternal(); } - void AddConnection(Uri hostUrl, string username) + /// + public async Task LogIn(HostAddress address, string userName, string password) { - var address = HostAddress.Create(hostUrl); - if (Connections.FirstOrDefault(x => x.HostAddress.Equals(address)) != null) - return; - var conn = SetupConnection(address, username); - Connections.Add(conn); + var conns = await GetLoadedConnectionsInternal(); + + if (conns.Any(x => x.HostAddress == address)) + { + throw new InvalidOperationException($"A connection to {address} already exists."); + } + + var client = CreateClient(address); + var user = await loginManager.Login(address, client, userName, password); + var connection = new Connection(address, userName, user, null); + conns.Add(connection); + await SaveConnections(); + return connection; } - public bool RemoveConnection(HostAddress address) + /// + public async Task LogOut(HostAddress address) { - var c = Connections.FirstOrDefault(x => x.HostAddress.Equals(address)); - if (c == null) - return false; - RequestLogout(c); - return true; + var connection = await GetConnection(address); + + if (connection == null) + { + throw new KeyNotFoundException($"Could not find a connection to {address}."); + } + + var client = CreateClient(address); + await loginManager.Logout(address, client); + connections.Value.Remove(connection); + await SaveConnections(); } - public IObservable RequestLogin(IConnection connection) + ObservableCollectionEx CreateConnections() { - var handler = DoLogin; - if (handler == null) - return null; - return handler(connection); + var result = new ObservableCollectionEx(); + LoadConnections(result).Forget(); + return result; } - public void RequestLogout(IConnection connection) + IGitHubClient CreateClient(HostAddress address) { - Connections.Remove(connection); + return new GitHubClient( + program.ProductHeader, + new KeychainCredentialStore(keychain, address), + address.ApiUri); } - IConnection SetupConnection(HostAddress address, string username) + async Task> GetLoadedConnectionsInternal() { - var conn = new Connection(this, address, username); - return conn; + var result = Connections; + await loaded.Task; + return connections.Value; } - void RefreshConnections(object sender, System.Collections.Specialized.NotifyCollectionChangedEventArgs e) + async Task LoadConnections(ObservableCollection result) { - if (e.Action == System.Collections.Specialized.NotifyCollectionChangedAction.Remove) + try { - foreach (IConnection c in e.OldItems) + foreach (var c in await cache.Load()) { - // RepositoryHosts hasn't been loaded so it can't handle logging out, we have to do it ourselves - if (DoLogin == null) - keychain.Delete(c.HostAddress).Forget(); + var client = CreateClient(c.HostAddress); + User user = null; + Exception error = null; + + try + { + user = await loginManager.LoginFromCache(c.HostAddress, client); + } + catch (Exception e) + { + error = e; + } + + result.Add(new Connection(c.HostAddress, c.UserName, user, error)); } } - - SaveConnectionsToCache().Forget(); - } - - async Task LoadConnectionsFromCache() - { - foreach (var c in await cache.Load()) + finally { - var client = await apiClientFactory.CreateGitHubClient(c.HostAddress); - var addConnection = true; - - try - { - await loginManager.LoginFromCache(c.HostAddress, client); - } - catch (Octokit.ApiException e) - { - addConnection = false; - VsOutputLogger.WriteLine("Cached credentials for connection {0} were invalid: {1}", c.HostAddress, e); - } - catch (Exception) - { - // Add the connection in this case - could be that there's no internet connection. - } - - if (addConnection) - { - AddConnection(c.HostAddress, c.UserName); - } + loaded.SetResult(null); } - - Connections.CollectionChanged += RefreshConnections; } - async Task SaveConnectionsToCache() + async Task SaveConnections() { - await cache.Save(Connections.Select(x => new ConnectionDetails(x.HostAddress, x.Username))); + var conns = await GetLoadedConnectionsInternal(); + var details = conns.Select(x => new ConnectionDetails(x.HostAddress, x.Username)); + await cache.Save(details); } - - public ObservableCollection Connections { get; private set; } } } diff --git a/src/GitHub.VisualStudio/UI/Views/GitHubPaneViewModel.cs b/src/GitHub.VisualStudio/UI/Views/GitHubPaneViewModel.cs index 09c163c345..4a208800aa 100644 --- a/src/GitHub.VisualStudio/UI/Views/GitHubPaneViewModel.cs +++ b/src/GitHub.VisualStudio/UI/Views/GitHubPaneViewModel.cs @@ -238,11 +238,10 @@ async Task Load(ViewWithData data) IsLoggedIn = false; else { - var isLoggedIn = await connection.IsLoggedIn(hosts); if (reloadCallId != latestReloadCallId) return; - IsLoggedIn = isLoggedIn; + IsLoggedIn = connection.IsLoggedIn; } Load(connection, data); diff --git a/src/UnitTests/GitHub.App/Controllers/UIControllerTests.cs b/src/UnitTests/GitHub.App/Controllers/UIControllerTests.cs index 3817ad5a11..29dd78cae4 100644 --- a/src/UnitTests/GitHub.App/Controllers/UIControllerTests.cs +++ b/src/UnitTests/GitHub.App/Controllers/UIControllerTests.cs @@ -18,6 +18,8 @@ using GitHub.App.Factories; using System.Reactive; using GitHub.Extensions.Reactive; +using System.Threading.Tasks; +using GitHub.Extensions; public class UIControllerTests { @@ -101,8 +103,8 @@ protected IConnection SetupConnection(IServiceProvider provider, IRepositoryHost IRepositoryHost host, bool loggedIn = true) { var connection = provider.GetConnection(); - connection.Login().Returns(Observable.Return(connection)); hosts.LookupHost(connection.HostAddress).Returns(host); + connection.IsLoggedIn.Returns(loggedIn); host.IsLoggedIn.Returns(loggedIn); return connection; } @@ -128,8 +130,10 @@ public void RunningNonAuthFlowWithoutBeingLoggedInRunsAuthFlow() var hosts = provider.GetRepositoryHosts(); var factory = SetupFactory(provider); var cm = provider.GetConnectionManager(); - var cons = new ObservableCollection(); + var cons = new ObservableCollectionEx(); + cm.Connections.Returns(cons); + cm.GetLoadedConnections().Returns(cons); using (var uiController = new UIController((IGitHubServiceProvider)provider, hosts, factory, cm)) { @@ -155,13 +159,16 @@ public void RunningNonAuthFlowWithoutBeingLoggedInRunsAuthFlow() [Fact] public void RunningNonAuthFlowWhenLoggedInRunsNonAuthFlow() + { var provider = Substitutes.GetFullyMockedServiceProvider(); var hosts = provider.GetRepositoryHosts(); var factory = SetupFactory(provider); var cm = provider.GetConnectionManager(); - var cons = new ObservableCollection(); + var cons = new ObservableCollectionEx(); + cm.Connections.Returns(cons); + cm.GetLoadedConnections().Returns(cons); // simulate being logged in cons.Add(SetupConnection(provider, hosts, hosts.GitHubHost)); @@ -195,8 +202,10 @@ public void RunningAuthFlowWithoutBeingLoggedInRunsAuthFlow() var hosts = provider.GetRepositoryHosts(); var factory = SetupFactory(provider); var cm = provider.GetConnectionManager(); - var cons = new ObservableCollection(); + var cons = new ObservableCollectionEx(); + cm.Connections.Returns(cons); + cm.GetLoadedConnections().Returns(cons); using (var uiController = new UIController((IGitHubServiceProvider)provider, hosts, factory, cm)) { @@ -231,8 +240,10 @@ public void RunningAuthFlowWhenLoggedInRunsAuthFlow() // simulate being logged in var host = hosts.GitHubHost; var connection = SetupConnection(provider, hosts, host); - var cons = new ObservableCollection { connection }; + var cons = new ObservableCollectionEx { connection }; + cm.Connections.Returns(cons); + cm.GetLoadedConnections().Returns(cons); using (var uiController = new UIController((IGitHubServiceProvider)provider, hosts, factory, cm)) { @@ -263,8 +274,10 @@ public void AuthFlowWithout2FA() var hosts = provider.GetRepositoryHosts(); var factory = SetupFactory(provider); var cm = provider.GetConnectionManager(); - var cons = new ObservableCollection(); + var cons = new ObservableCollectionEx(); + cm.Connections.Returns(cons); + cm.GetLoadedConnections().Returns(cons); using (var uiController = new UIController((IGitHubServiceProvider)provider, hosts, factory, cm)) { @@ -301,8 +314,10 @@ public void AuthFlowWith2FA() var hosts = provider.GetRepositoryHosts(); var factory = SetupFactory(provider); var cm = provider.GetConnectionManager(); - var cons = new ObservableCollection(); + var cons = new ObservableCollectionEx(); + cm.Connections.Returns(cons); + cm.GetLoadedConnections().Returns(cons); using (var uiController = new UIController((IGitHubServiceProvider)provider, hosts, factory, cm)) { @@ -347,8 +362,10 @@ public void BackAndForth() var hosts = provider.GetRepositoryHosts(); var factory = SetupFactory(provider); var cm = provider.GetConnectionManager(); - var cons = new ObservableCollection(); + var cons = new ObservableCollectionEx(); + cm.Connections.Returns(cons); + cm.GetLoadedConnections().Returns(cons); using (var uiController = new UIController((IGitHubServiceProvider)provider, hosts, factory, cm)) { @@ -413,8 +430,10 @@ public void Flow() var hosts = provider.GetRepositoryHosts(); var factory = SetupFactory(provider); var cm = provider.GetConnectionManager(); - var cons = new ObservableCollection(); + var cons = new ObservableCollectionEx(); + cm.Connections.Returns(cons); + cm.GetLoadedConnections().Returns(cons); // simulate being logged in cons.Add(SetupConnection(provider, hosts, hosts.GitHubHost)); @@ -451,8 +470,10 @@ public void Flow() var hosts = provider.GetRepositoryHosts(); var factory = SetupFactory(provider); var cm = provider.GetConnectionManager(); - var cons = new ObservableCollection(); + var cons = new ObservableCollectionEx(); + cm.Connections.Returns(cons); + cm.GetLoadedConnections().Returns(cons); // simulate being logged in cons.Add(SetupConnection(provider, hosts, hosts.GitHubHost)); @@ -489,7 +510,7 @@ public void FlowWithConnection() var hosts = provider.GetRepositoryHosts(); var factory = SetupFactory(provider); var cm = provider.GetConnectionManager(); - var cons = new ObservableCollection(); + var cons = new ObservableCollectionEx(); cm.Connections.Returns(cons); var connection = SetupConnection(provider, hosts, hosts.GitHubHost); @@ -526,8 +547,11 @@ public void FlowWithoutConnection() var hosts = provider.GetRepositoryHosts(); var factory = SetupFactory(provider); var cm = provider.GetConnectionManager(); - var cons = new ObservableCollection(); + var cons = new ObservableCollectionEx(); + cm.Connections.Returns(cons); + cm.GetLoadedConnections().Returns(cons); + var connection = SetupConnection(provider, hosts, hosts.GitHubHost); // simulate being logged in @@ -566,8 +590,10 @@ public void ShowsGistDialog() var hosts = provider.GetRepositoryHosts(); var factory = SetupFactory(provider); var cm = provider.GetConnectionManager(); - var cons = new ObservableCollection(); + var cons = new ObservableCollectionEx(); + cm.Connections.Returns(cons); + cm.GetLoadedConnections().Returns(cons); // simulate being logged in cons.Add(SetupConnection(provider, hosts, hosts.GitHubHost, true)); diff --git a/src/UnitTests/GitHub.App/ViewModels/RepositoryPublishViewModelTests.cs b/src/UnitTests/GitHub.App/ViewModels/RepositoryPublishViewModelTests.cs index 8a0d33479b..343f4bdc63 100644 --- a/src/UnitTests/GitHub.App/ViewModels/RepositoryPublishViewModelTests.cs +++ b/src/UnitTests/GitHub.App/ViewModels/RepositoryPublishViewModelTests.cs @@ -12,6 +12,7 @@ using System.Collections.Generic; using System.Collections.ObjectModel; using System.Linq; +using GitHub.Extensions; public class RepositoryPublishViewModelTests { @@ -70,7 +71,7 @@ public static IRepositoryPublishViewModel SetupConnectionsAndViewModel( var conns = new List(); SetupConnections(hosts, cm, adds, conns, hsts, uri); hsts[0].ModelService.GetAccounts().Returns(Observable.Return(new List())); - cm.Connections.Returns(new ObservableCollection(conns)); + cm.Connections.Returns(new ObservableCollectionEx(conns)); return GetViewModel(hosts, service, notificationService, cm); } @@ -106,7 +107,7 @@ public void ConnectionsMatchHosts(string arg1, string arg2) foreach(var host in hsts) host.ModelService.GetAccounts().Returns(Observable.Return(new List())); - cm.Connections.Returns(new ObservableCollection(conns)); + cm.Connections.Returns(new ObservableCollectionEx(conns)); var vm = Helpers.GetViewModel(hosts: hosts, connectionManager: cm); @@ -140,7 +141,7 @@ public void DefaultsToGitHub(string arg1, string arg2) foreach (var host in hsts) host.ModelService.GetAccounts().Returns(Observable.Return(new List())); - cm.Connections.Returns(new ObservableCollection(conns)); + cm.Connections.Returns(new ObservableCollectionEx(conns)); var vm = Helpers.GetViewModel(hosts, connectionManager: cm); Assert.Same(adds.First(x => x.IsGitHubDotCom()), vm.SelectedConnection.HostAddress); @@ -168,7 +169,7 @@ public void IsPopulatedByTheAccountsForTheSelectedHost() hsts.First(x => x.Address.IsGitHubDotCom()).ModelService.GetAccounts().Returns(Observable.Return(gitHubAccounts)); hsts.First(x => !x.Address.IsGitHubDotCom()).ModelService.GetAccounts().Returns(Observable.Return(enterpriseAccounts)); - cm.Connections.Returns(new ObservableCollection(conns)); + cm.Connections.Returns(new ObservableCollectionEx(conns)); var vm = Helpers.GetViewModel(hosts, connectionManager: cm); Assert.Equal(2, vm.Accounts.Count); @@ -355,7 +356,7 @@ public async Task ResetsWhenSwitchingHosts() foreach (var host in hsts) host.ModelService.GetAccounts().Returns(Observable.Return(new List())); - cm.Connections.Returns(new ObservableCollection(conns)); + cm.Connections.Returns(new ObservableCollectionEx(conns)); var notificationService = Substitute.For(); @@ -392,7 +393,7 @@ public async Task ResetsWhenSwitchingAccounts() var accounts = new List { Substitute.For(), Substitute.For() }; hsts[0].ModelService.GetAccounts().Returns(Observable.Return(accounts)); - cm.Connections.Returns(new ObservableCollection(conns)); + cm.Connections.Returns(new ObservableCollectionEx(conns)); var notificationService = Substitute.For(); diff --git a/src/UnitTests/GitHub.VisualStudio/Services/ConnectionManagerTests.cs b/src/UnitTests/GitHub.VisualStudio/Services/ConnectionManagerTests.cs index 0671760d28..f3faafee4c 100644 --- a/src/UnitTests/GitHub.VisualStudio/Services/ConnectionManagerTests.cs +++ b/src/UnitTests/GitHub.VisualStudio/Services/ConnectionManagerTests.cs @@ -1,43 +1,244 @@ using System; -using System.Text; +using System.Collections.Generic; +using System.Linq; +using System.Threading.Tasks; +using GitHub.Api; using GitHub.Models; using GitHub.Primitives; using GitHub.Services; using GitHub.VisualStudio; using NSubstitute; -using Rothko; +using Octokit; using Xunit; -using UnitTests; -using System.Collections.Generic; -using System.Linq; -using GitHub.Factories; -using GitHub.Api; public class ConnectionManagerTests { - public class TheConnectionsProperty : TestBaseClass + public class TheGetInitializedConnectionsMethod + { + [Fact] + public async Task ReturnsValidConnections() + { + var target = new ConnectionManager( + CreateProgram(), + CreateConnectionCache("github", "valid"), + Substitute.For(), + CreateLoginManager()); + var result = await target.GetLoadedConnections(); + + Assert.Equal(2, result.Count); + Assert.Equal("https://github.com/", result[0].HostAddress.WebUri.ToString()); + Assert.Equal("https://valid.com/", result[1].HostAddress.WebUri.ToString()); + Assert.Null(result[0].ConnectionError); + Assert.Null(result[1].ConnectionError); + } + + [Fact] + public async Task ReturnsInvalidConnections() + { + var target = new ConnectionManager( + CreateProgram(), + CreateConnectionCache("github", "invalid"), + Substitute.For(), + CreateLoginManager()); + var result = await target.GetLoadedConnections(); + + Assert.Equal(2, result.Count); + Assert.Equal("https://github.com/", result[0].HostAddress.WebUri.ToString()); + Assert.Equal("https://invalid.com/", result[1].HostAddress.WebUri.ToString()); + Assert.Null(result[0].ConnectionError); + Assert.NotNull(result[1].ConnectionError); + } + } + + public class TheGetConnectionMethod { [Fact] - public void IsSavedToDiskWhenConnectionAdded() + public async Task ReturnsCorrectConnection() { - var program = Substitute.For(); - program.ApplicationName.Returns("GHfVS"); - var operatingSystem = Substitute.For(); - operatingSystem.Environment.GetFolderPath(System.Environment.SpecialFolder.LocalApplicationData) - .Returns(@"c:\fake"); - operatingSystem.File.Exists(@"c:\fake\GHfVS\ghfvs.connections").Returns(true); - operatingSystem.File.ReadAllText(@"c:\fake\GHfVS\ghfvs.connections", Encoding.UTF8).Returns(""); - var cache = Substitute.For(); - var keychain = Substitute.For(); - var loginManager = Substitute.For(); - var apiClientFactory = Substitute.For(); - var manager = new ConnectionManager(cache, keychain, loginManager, apiClientFactory); + var target = new ConnectionManager( + CreateProgram(), + CreateConnectionCache("github", "valid"), + Substitute.For(), + CreateLoginManager()); + var result = await target.GetConnection(HostAddress.Create("valid.com")); + + Assert.Equal("https://valid.com/", result.HostAddress.WebUri.ToString()); + } - manager.Connections.Add(new Connection(manager, HostAddress.GitHubDotComHostAddress, "coolio")); + [Fact] + public async Task ReturnsCorrectNullForNotFoundConnection() + { + var target = new ConnectionManager( + CreateProgram(), + CreateConnectionCache("github", "valid"), + Substitute.For(), + CreateLoginManager()); + var result = await target.GetConnection(HostAddress.Create("another.com")); - Assert.Equal(1, manager.Connections.Count); - cache.Received(1).Save(Arg.Is>(x => - x.SequenceEqual(new[] { new ConnectionDetails(HostAddress.GitHubDotComHostAddress, "coolio") }))); + Assert.Null(result); } } + + public class TheLoginMethod + { + [Fact] + public async Task ReturnsLoggedInConnection() + { + var target = new ConnectionManager( + CreateProgram(), + CreateConnectionCache(), + Substitute.For(), + CreateLoginManager()); + var result = await target.LogIn(HostAddress.GitHubDotComHostAddress, "user", "pass"); + + Assert.NotNull(result); + } + + [Fact] + public async Task AddsLoggedInConnectionToConnections() + { + var target = new ConnectionManager( + CreateProgram(), + CreateConnectionCache(), + Substitute.For(), + CreateLoginManager()); + + await target.LogIn(HostAddress.GitHubDotComHostAddress, "user", "pass"); + + Assert.Equal(1, target.Connections.Count); + } + + [Fact] + public async Task ThrowsWhenLoginFails() + { + var target = new ConnectionManager( + CreateProgram(), + CreateConnectionCache(), + Substitute.For(), + CreateLoginManager()); + + await Assert.ThrowsAsync(async () => + await target.LogIn(HostAddress.Create("invalid.com"), "user", "pass")); + } + + [Fact] + public async Task ThrowsWhenExistingConnectionExists() + { + var target = new ConnectionManager( + CreateProgram(), + CreateConnectionCache("github"), + Substitute.For(), + CreateLoginManager()); + + await Assert.ThrowsAsync(async () => + await target.LogIn(HostAddress.GitHubDotComHostAddress, "user", "pass")); + } + + [Fact] + public async Task SavesConnectionToCache() + { + var cache = CreateConnectionCache(); + var target = new ConnectionManager( + CreateProgram(), + cache, + Substitute.For(), + CreateLoginManager()); + + await target.LogIn(HostAddress.GitHubDotComHostAddress, "user", "pass"); + + await cache.Received(1).Save(Arg.Is>(x => + x.Count() == 1 && x.ElementAt(0).HostAddress == HostAddress.Create("https://github.com"))); + } + } + + public class TheLogOutMethod + { + [Fact] + public async Task CallsLoginManagerLogOut() + { + var loginManager = CreateLoginManager(); + var target = new ConnectionManager( + CreateProgram(), + CreateConnectionCache("github"), + Substitute.For(), + loginManager); + + await target.LogOut(HostAddress.GitHubDotComHostAddress); + + await loginManager.Received().Logout( + HostAddress.GitHubDotComHostAddress, + Arg.Any()); + } + + [Fact] + public async Task RemovesConnectionFromConnections() + { + var loginManager = CreateLoginManager(); + var target = new ConnectionManager( + CreateProgram(), + CreateConnectionCache("github"), + Substitute.For(), + loginManager); + + await target.LogOut(HostAddress.GitHubDotComHostAddress); + + Assert.Empty(target.Connections); + } + + [Fact] + public async Task ThrowsIfConnectionDoesntExist() + { + var loginManager = CreateLoginManager(); + var target = new ConnectionManager( + CreateProgram(), + CreateConnectionCache("valid"), + Substitute.For(), + loginManager); + + await Assert.ThrowsAsync(async () => + await target.LogOut(HostAddress.GitHubDotComHostAddress)); + } + + [Fact] + public async Task RemovesConnectionFromCache() + { + var cache = CreateConnectionCache("github"); + var target = new ConnectionManager( + CreateProgram(), + cache, + Substitute.For(), + CreateLoginManager()); + + await target.LogOut(HostAddress.GitHubDotComHostAddress); + + await cache.Received(1).Save(Arg.Is>(x => x.Count() == 0)); + } + } + + static IProgram CreateProgram() + { + var result = Substitute.For(); + result.ProductHeader.Returns(new ProductHeaderValue("foo")); + return result; + } + + static IConnectionCache CreateConnectionCache(params string[] hosts) + { + var result = Substitute.For(); + var details = hosts.Select(x => new ConnectionDetails( + HostAddress.Create($"https://{x}.com"), + "user")); + result.Load().Returns(details.ToList()); + return result; + } + + static ILoginManager CreateLoginManager() + { + var result = Substitute.For(); + result.Login(HostAddress.Create("invalid.com"), Arg.Any(), Arg.Any(), Arg.Any()) + .Returns(_ => { throw new AuthorizationException(); }); + result.LoginFromCache(HostAddress.Create("invalid.com"), Arg.Any()) + .Returns(_ => { throw new AuthorizationException(); }); + return result; + } } diff --git a/src/UnitTests/GitHub.VisualStudio/UI/Views/GitHubPaneViewModelTests.cs b/src/UnitTests/GitHub.VisualStudio/UI/Views/GitHubPaneViewModelTests.cs index d7866c74a9..77bae612fe 100644 --- a/src/UnitTests/GitHub.VisualStudio/UI/Views/GitHubPaneViewModelTests.cs +++ b/src/UnitTests/GitHub.VisualStudio/UI/Views/GitHubPaneViewModelTests.cs @@ -16,6 +16,7 @@ using System.Reactive.Linq; using UnitTests; using Xunit; +using GitHub.Extensions; public class GitHubPaneViewModelTests : TestBaseClass { @@ -45,10 +46,9 @@ public GitHubPaneViewModelTests() var connection = Substitutes.Connection; var connectionHostAddress = HostAddress.Create(activeRepo.CloneUrl.ToString()); connection.HostAddress.Returns(connectionHostAddress); - connectionManager.Connections.Returns(new ObservableCollection(new[] { + connectionManager.Connections.Returns(new ObservableCollectionEx(new[] { connection })); - connection.Login().Returns(Observable.Return(connection)); var host = Substitute.For(); host.IsLoggedIn.Returns(true); diff --git a/src/UnitTests/Substitutes.cs b/src/UnitTests/Substitutes.cs index 8372e18a58..0062bb3112 100644 --- a/src/UnitTests/Substitutes.cs +++ b/src/UnitTests/Substitutes.cs @@ -9,6 +9,7 @@ using System; using System.ComponentModel.Composition; using System.ComponentModel.Composition.Hosting; +using GitHub.Api; namespace UnitTests { @@ -63,7 +64,9 @@ public static IOperatingSystem OperatingSystem public static IRepositoryHosts RepositoryHosts { get { return Substitute.For(); } } public static IConnection Connection { get { return Substitute.For(); } } + public static IConnection NewConnection { get { return Substitute.For(); } } public static IConnectionManager ConnectionManager { get { return Substitute.For(); } } + public static IConnectionManager NewConnectionManager { get { return Substitute.For(); } } public static IDelegatingTwoFactorChallengeHandler TwoFactorChallengeHandler { get { return Substitute.For(); } } public static IGistPublishService GistPublishService { get { return Substitute.For(); } } public static IPullRequestService PullRequestService { get { return Substitute.For(); } } @@ -127,7 +130,9 @@ public static IGitHubServiceProvider GetServiceProvider( ret.GetService(typeof(IExportFactoryProvider)).Returns(ExportFactoryProvider); ret.GetService(typeof(IUIFactory)).Returns(UIFactory); ret.GetService(typeof(IConnection)).Returns(Connection); + ret.GetService(typeof(IConnection)).Returns(NewConnection); ret.GetService(typeof(IConnectionManager)).Returns(ConnectionManager); + ret.GetService(typeof(IConnectionManager)).Returns(NewConnectionManager); ret.GetService(typeof(IAvatarProvider)).Returns(avatarProvider); ret.GetService(typeof(IDelegatingTwoFactorChallengeHandler)).Returns(TwoFactorChallengeHandler); ret.GetService(typeof(IGistPublishService)).Returns(GistPublishService); @@ -190,11 +195,21 @@ public static IConnection GetConnection(this IServiceProvider provider) return provider.GetService(typeof(IConnection)) as IConnection; } + public static IConnection GetNewConnection(this IServiceProvider provider) + { + return provider.GetService(typeof(IConnection)) as IConnection; + } + public static IConnectionManager GetConnectionManager(this IServiceProvider provider) { return provider.GetService(typeof(IConnectionManager)) as IConnectionManager; } + public static IConnectionManager GetNewConnectionManager(this IServiceProvider provider) + { + return provider.GetService(typeof(IConnectionManager)) as IConnectionManager; + } + public static IAvatarProvider GetAvatarProvider(this IServiceProvider provider) { return provider.GetService(typeof(IAvatarProvider)) as IAvatarProvider; From aa78f0343fa7840f262c0d7de9f88b14834e4cb0 Mon Sep 17 00:00:00 2001 From: Steven Kirk Date: Mon, 23 Oct 2017 16:48:43 +0200 Subject: [PATCH 03/16] Refactored RepositoryHost(s). --- src/GitHub.App/Controllers/UIController.cs | 14 +- .../Factories/RepositoryHostFactory.cs | 49 +---- .../Models/DisconnectedRepositoryHosts.cs | 5 - src/GitHub.App/Models/RepositoryHost.cs | 158 ++----------- src/GitHub.App/Models/RepositoryHosts.cs | 207 +++++++----------- src/GitHub.App/SampleData/SampleViewModels.cs | 15 -- .../ViewModels/PullRequestListViewModel.cs | 10 +- .../Factories/IRepositoryHostFactory.cs | 6 +- .../Models/IRepositoryHost.cs | 6 +- .../Models/IRepositoryHosts.cs | 5 +- .../Services/IConnectionManager.cs | 13 ++ .../GitHub.Extensions.csproj | 1 + .../ObservableCollectionExtensions.cs | 101 +++++++++ .../Services/PullRequestSessionManager.cs | 10 - .../Services/ConnectionManager.cs | 17 +- .../GitHub.App/Models/RepositoryHostTests.cs | 111 ---------- .../GitHub.App/Models/RepositoryHostsTests.cs | 198 +++++++++++++++++ src/UnitTests/UnitTests.csproj | 1 + 18 files changed, 444 insertions(+), 483 deletions(-) create mode 100644 src/GitHub.Extensions/ObservableCollectionExtensions.cs create mode 100644 src/UnitTests/GitHub.App/Models/RepositoryHostsTests.cs diff --git a/src/GitHub.App/Controllers/UIController.cs b/src/GitHub.App/Controllers/UIController.cs index 728873bda7..6c41367962 100644 --- a/src/GitHub.App/Controllers/UIController.cs +++ b/src/GitHub.App/Controllers/UIController.cs @@ -14,6 +14,7 @@ using System.Reactive.Disposables; using System.Reactive.Linq; using System.Reactive.Subjects; +using System.Reactive.Threading.Tasks; using System.Windows; namespace GitHub.Controllers @@ -219,7 +220,13 @@ public void Start() else // sanity check: it makes zero sense to pass a connection in when calling the auth flow Debug.Assert(false, "Calling the auth flow with a connection makes no sense!"); - Fire(Trigger.Next); + hosts.EnsureInitialized().ToObservable() + .ObserveOn(RxApp.MainThreadScheduler) + .Subscribe(_ => { }, () => + { + Debug.WriteLine("Start ({0})", GetHashCode()); + Fire(Trigger.Next); + }); } else { @@ -438,8 +445,7 @@ void ConfigureSingleViewLogic(UIControllerFlow flow, UIViewType type) UIControllerFlow SelectActiveFlow() { - var host = connection != null ? hosts.LookupHost(connection.HostAddress) : null; - var loggedIn = host?.IsLoggedIn ?? false; + var loggedIn = connection?.IsLoggedIn ?? false; return loggedIn ? selectedFlow : UIControllerFlow.Authentication; } @@ -704,7 +710,7 @@ public void Dispose() public bool IsStopped => uiStateMachine.IsInState(UIViewType.None) || stopping; public UIControllerFlow CurrentFlow => activeFlow; public UIControllerFlow SelectedFlow => selectedFlow; - bool LoggedIn => connection != null && hosts.LookupHost(connection.HostAddress).IsLoggedIn; + bool LoggedIn => connection?.IsLoggedIn ?? false; bool? Success { get; set; } } } diff --git a/src/GitHub.App/Factories/RepositoryHostFactory.cs b/src/GitHub.App/Factories/RepositoryHostFactory.cs index 6cb5c662a1..a80c3f54b1 100644 --- a/src/GitHub.App/Factories/RepositoryHostFactory.cs +++ b/src/GitHub.App/Factories/RepositoryHostFactory.cs @@ -1,12 +1,7 @@ -using System; -using System.ComponentModel.Composition; -using GitHub.Caches; +using System.ComponentModel.Composition; +using System.Threading.Tasks; using GitHub.Models; -using GitHub.Primitives; using GitHub.Services; -using System.Reactive.Disposables; -using System.Threading.Tasks; -using GitHub.Api; namespace GitHub.Factories { @@ -16,59 +11,27 @@ public class RepositoryHostFactory : IRepositoryHostFactory { readonly IApiClientFactory apiClientFactory; readonly IHostCacheFactory hostCacheFactory; - readonly ILoginManager loginManager; - readonly IKeychain keychain; readonly IAvatarProvider avatarProvider; - readonly CompositeDisposable hosts = new CompositeDisposable(); - readonly IUsageTracker usage; [ImportingConstructor] public RepositoryHostFactory( IApiClientFactory apiClientFactory, IHostCacheFactory hostCacheFactory, - ILoginManager loginManager, - IKeychain keychain, - IAvatarProvider avatarProvider, - IUsageTracker usage) + IAvatarProvider avatarProvider) { this.apiClientFactory = apiClientFactory; this.hostCacheFactory = hostCacheFactory; - this.loginManager = loginManager; - this.keychain = keychain; this.avatarProvider = avatarProvider; - this.usage = usage; } - public async Task Create(HostAddress hostAddress) + public async Task Create(IConnection connection) { + var hostAddress = connection.HostAddress; var apiClient = await apiClientFactory.Create(hostAddress); var hostCache = await hostCacheFactory.Create(hostAddress); var modelService = new ModelService(apiClient, hostCache, avatarProvider); - var host = new RepositoryHost(apiClient, modelService, loginManager, keychain, usage); - hosts.Add(host); + var host = new RepositoryHost(connection, apiClient, modelService); return host; } - - public void Remove(IRepositoryHost host) - { - hosts.Remove(host); - } - - bool disposed; - protected virtual void Dispose(bool disposing) - { - if (disposing) - { - if (disposed) return; - disposed = true; - hosts.Dispose(); - } - } - - public void Dispose() - { - Dispose(true); - GC.SuppressFinalize(this); - } } } diff --git a/src/GitHub.App/Models/DisconnectedRepositoryHosts.cs b/src/GitHub.App/Models/DisconnectedRepositoryHosts.cs index cb3e2b721e..c0f2790304 100644 --- a/src/GitHub.App/Models/DisconnectedRepositoryHosts.cs +++ b/src/GitHub.App/Models/DisconnectedRepositoryHosts.cs @@ -48,10 +48,5 @@ public IObservable LogOut() { return Observable.Return(Unit.Default); } - - public void Dispose() - { - GC.SuppressFinalize(this); - } } } diff --git a/src/GitHub.App/Models/RepositoryHost.cs b/src/GitHub.App/Models/RepositoryHost.cs index 819270bbd5..249cc06eae 100644 --- a/src/GitHub.App/Models/RepositoryHost.cs +++ b/src/GitHub.App/Models/RepositoryHost.cs @@ -1,20 +1,9 @@ using System; using System.Diagnostics; -using System.Diagnostics.CodeAnalysis; -using System.Globalization; -using System.Linq; -using System.Reactive; -using System.Reactive.Linq; -using System.Reactive.Threading.Tasks; -using System.Threading.Tasks; using GitHub.Api; -using GitHub.Authentication; -using GitHub.Caches; using GitHub.Extensions; using GitHub.Primitives; using GitHub.Services; -using NLog; -using Octokit; using ReactiveUI; namespace GitHub.Models @@ -22,147 +11,26 @@ namespace GitHub.Models [DebuggerDisplay("{DebuggerDisplay,nq}")] public class RepositoryHost : ReactiveObject, IRepositoryHost { - static readonly Logger log = LogManager.GetCurrentClassLogger(); - - readonly ILoginManager loginManager; - readonly HostAddress hostAddress; - readonly IKeychain keychain; - readonly IUsageTracker usage; - - bool isLoggedIn; + readonly IConnection connection; public RepositoryHost( + IConnection connection, IApiClient apiClient, - IModelService modelService, - ILoginManager loginManager, - IKeychain keychain, - IUsageTracker usage) + IModelService modelService) { + Guard.ArgumentNotNull(connection, nameof(connection)); + Guard.ArgumentNotNull(apiClient, nameof(apiClient)); + Guard.ArgumentNotNull(modelService, nameof(modelService)); + + this.connection = connection; ApiClient = apiClient; ModelService = modelService; - this.loginManager = loginManager; - this.keychain = keychain; - this.usage = usage; - - Debug.Assert(apiClient.HostAddress != null, "HostAddress of an api client shouldn't be null"); - Address = apiClient.HostAddress; - hostAddress = apiClient.HostAddress; - Title = apiClient.HostAddress.Title; - } - - public HostAddress Address { get; private set; } - - public IApiClient ApiClient { get; private set; } - - public bool IsLoggedIn - { - get { return isLoggedIn; } - private set { this.RaiseAndSetIfChanged(ref isLoggedIn, value); } - } - - public string Title { get; private set; } - - [SuppressMessage("Microsoft.Reliability", "CA2000:Dispose objects before losing scope")] - public IObservable LogInFromCache() - { - Func> f = async () => - { - try - { - var user = await loginManager.LoginFromCache(Address, ApiClient.GitHubClient); - var accountCacheItem = new AccountCacheItem(user); - usage.IncrementLoginCount().Forget(); - await ModelService.InsertUser(accountCacheItem); - - if (user != null) - { - IsLoggedIn = true; - return AuthenticationResult.Success; - } - else - { - return AuthenticationResult.VerificationFailure; - } - } - catch (AuthorizationException) - { - return AuthenticationResult.CredentialFailure; - } - }; - - return f().ToObservable(); - } - - public IObservable LogIn(string usernameOrEmail, string password) - { - Guard.ArgumentNotEmptyString(usernameOrEmail, nameof(usernameOrEmail)); - Guard.ArgumentNotEmptyString(password, nameof(password)); - - return Observable.Defer(async () => - { - var user = await loginManager.Login(Address, ApiClient.GitHubClient, usernameOrEmail, password); - var accountCacheItem = new AccountCacheItem(user); - usage.IncrementLoginCount().Forget(); - await ModelService.InsertUser(accountCacheItem); - - if (user != null) - { - IsLoggedIn = true; - return Observable.Return(AuthenticationResult.Success); - } - else - { - return Observable.Return(AuthenticationResult.VerificationFailure); - } - }); } - public IObservable LogOut() - { - if (!IsLoggedIn) return Observable.Return(Unit.Default); - - log.Info(CultureInfo.InvariantCulture, "Logged off of host '{0}'", hostAddress.ApiUri); - - return keychain.Delete(Address).ToObservable() - .Catch(e => - { - log.Warn("ASSERT! Failed to erase login. Going to invalidate cache anyways.", e); - return Observable.Return(Unit.Default); - }) - .SelectMany(_ => ModelService.InvalidateAll()) - .Catch(e => - { - log.Warn("ASSERT! Failed to invaldiate caches", e); - return Observable.Return(Unit.Default); - }) - .ObserveOn(RxApp.MainThreadScheduler) - .Finally(() => - { - IsLoggedIn = false; - }); - } - - protected virtual void Dispose(bool disposing) - {} - - public void Dispose() - { - Dispose(true); - GC.SuppressFinalize(this); - } - - internal string DebuggerDisplay - { - get - { - return string.Format(CultureInfo.InvariantCulture, "RepositoryHost: {0} {1}", Title, hostAddress.ApiUri); - } - } - - public IModelService ModelService - { - get; - private set; - } + public HostAddress Address => ApiClient.HostAddress; + public IApiClient ApiClient { get;} + public bool IsLoggedIn => connection.IsLoggedIn; + public IModelService ModelService { get; } + public string Title => ApiClient.HostAddress.Title; } } diff --git a/src/GitHub.App/Models/RepositoryHosts.cs b/src/GitHub.App/Models/RepositoryHosts.cs index 696779cb57..8832d80638 100644 --- a/src/GitHub.App/Models/RepositoryHosts.cs +++ b/src/GitHub.App/Models/RepositoryHosts.cs @@ -1,14 +1,10 @@ using System; using System.ComponentModel.Composition; -using System.Globalization; -using System.Linq; using System.Reactive; using System.Reactive.Disposables; using System.Reactive.Linq; -using System.Reactive.Subjects; -using Akavache; +using System.Threading.Tasks; using GitHub.Authentication; -using GitHub.Caches; using GitHub.Extensions; using GitHub.Factories; using GitHub.Primitives; @@ -28,15 +24,14 @@ public class RepositoryHosts : ReactiveObject, IRepositoryHosts readonly ObservableAsPropertyHelper isLoggedInToAnyHost; readonly IConnectionManager connectionManager; readonly CompositeDisposable disposables = new CompositeDisposable(); + readonly TaskCompletionSource loaded; [ImportingConstructor] public RepositoryHosts( IRepositoryHostFactory repositoryHostFactory, - ISharedCache sharedCache, IConnectionManager connectionManager) { Guard.ArgumentNotNull(repositoryHostFactory, nameof(repositoryHostFactory)); - Guard.ArgumentNotNull(sharedCache, nameof(sharedCache)); Guard.ArgumentNotNull(connectionManager, nameof(connectionManager)); this.connectionManager = connectionManager; @@ -45,56 +40,21 @@ public RepositoryHosts( GitHubHost = DisconnectedRepositoryHost; EnterpriseHost = DisconnectedRepositoryHost; - var persistEntepriseHostObs = this.WhenAny(x => x.EnterpriseHost, x => x.Value) - .Skip(1) // The first value will be null or something already in the db - .SelectMany(enterpriseHost => - { - if (!enterpriseHost.IsLoggedIn) - { - return sharedCache.UserAccount - .InvalidateObject(EnterpriseHostApiBaseUriCacheKey) - .Catch(ex => - { - log.Warn("Failed to invalidate enterprise host uri", ex); - return Observable.Return(Unit.Default); - }); - } - - return sharedCache.UserAccount - .InsertObject(EnterpriseHostApiBaseUriCacheKey, enterpriseHost.Address.ApiUri) - .Catch(ex => - { - log.Warn("Failed to persist enterprise host uri", ex); - return Observable.Return(Unit.Default); - }); - }); - isLoggedInToAnyHost = this.WhenAny( x => x.GitHubHost.IsLoggedIn, x => x.EnterpriseHost.IsLoggedIn, (githubLoggedIn, enterpriseLoggedIn) => githubLoggedIn.Value || enterpriseLoggedIn.Value) .ToProperty(this, x => x.IsLoggedInToAnyHost); - // monitor the list of connections so we can log out hosts when connections are removed - disposables.Add( - connectionManager.Connections.CreateDerivedCollection(x => x) - .ItemsRemoved - .SelectMany(async x => - { - var host = LookupHost(x.HostAddress); - if (host.Address != x.HostAddress) - host = await RepositoryHostFactory.Create(x.HostAddress); - return host; - }) - .Select(h => LogOut(h)) - .Merge().ToList().Select(_ => Unit.Default).Subscribe()); + connectionManager.ConnectionCreated = ConnectionAdded; - - // Wait until we've loaded (or failed to load) an enterprise uri from the db and then - // start tracking changes to the EnterpriseHost property and persist every change to the db - disposables.Add(persistEntepriseHostObs.Subscribe()); + loaded = new TaskCompletionSource(); + disposables.Add(connectionManager.Connections.ForEachItem(_ => { }, ConnectionRemoved, ConnectionsReset)); + Load(); } + public Task EnsureInitialized() => loaded.Task; + public IRepositoryHost LookupHost(HostAddress address) { if (address == GitHubHost.Address) @@ -104,107 +64,35 @@ public IRepositoryHost LookupHost(HostAddress address) return DisconnectedRepositoryHost; } - public IObservable LogIn( - HostAddress address, - string usernameOrEmail, - string password) + public IObservable LogIn(HostAddress address, string username, string password) { Guard.ArgumentNotNull(address, nameof(address)); - Guard.ArgumentNotEmptyString(usernameOrEmail, nameof(usernameOrEmail)); + Guard.ArgumentNotEmptyString(username, nameof(username)); Guard.ArgumentNotEmptyString(password, nameof(password)); - var isDotCom = HostAddress.GitHubDotComHostAddress == address; - return Observable.Defer(async () => { - var host = await RepositoryHostFactory.Create(address); - - return host.LogIn(usernameOrEmail, password) - .Catch(Observable.Throw) - .ObserveOn(RxApp.MainThreadScheduler) - .Do(result => - { - bool successful = result.IsSuccess(); - log.Info(CultureInfo.InvariantCulture, "Log in to {3} host '{0}' with username '{1}' {2}", - address.ApiUri, - usernameOrEmail, - successful ? "SUCCEEDED" : "FAILED", - isDotCom ? "GitHub.com" : address.WebUri.Host - ); - if (successful) - { - // Make sure that GitHubHost/EnterpriseHost are set when the connections - // changed event is raised and likewise that the connection is added when - // the property changed notification is sent. - if (isDotCom) - githubHost = host; - else - enterpriseHost = host; - - connectionManager.LogIn(address, usernameOrEmail, password); + if (GitHubHost != DisconnectedRepositoryHost) + { + await connectionManager.LogOut(address); + } - if (isDotCom) - this.RaisePropertyChanged(nameof(GitHubHost)); - else - this.RaisePropertyChanged(nameof(EnterpriseHost)); - } - }); + var connection = await connectionManager.LogIn(address, username, password); + return Observable.Return(AuthenticationResult.Success); }); } - - /// - /// This is only called by the connection manager when logging in connections - /// that already exist so we don't have to add the connection. - /// - /// - /// - public IObservable LogInFromCache(HostAddress address) + + public IObservable LogOut(IRepositoryHost host) { - Guard.ArgumentNotNull(address, nameof(address)); - - var isDotCom = HostAddress.GitHubDotComHostAddress == address; + Guard.ArgumentNotNull(host, nameof(host)); return Observable.Defer(async () => { - var host = await RepositoryHostFactory.Create(address); - return host.LogInFromCache() - .Catch(Observable.Throw) - .ObserveOn(RxApp.MainThreadScheduler) - .Do(result => - { - bool successful = result.IsSuccess(); - if (successful) - { - if (isDotCom) - GitHubHost = host; - else - EnterpriseHost = host; - } - }); + await connectionManager.LogOut(host.Address); + return Observable.Return(Unit.Default); }); } - public IObservable LogOut(IRepositoryHost host) - { - Guard.ArgumentNotNull(host, nameof(host)); - - var address = host.Address; - var isDotCom = HostAddress.GitHubDotComHostAddress == address; - return host.LogOut() - .ObserveOn(RxApp.MainThreadScheduler) - .Do(result => - { - // reset the logged out host property to null - // it'll know what to do - if (isDotCom) - GitHubHost = null; - else - EnterpriseHost = null; - connectionManager.LogOut(address); - RepositoryHostFactory.Remove(host); - }); - } - bool disposed; protected void Dispose(bool disposing) { @@ -255,5 +143,58 @@ public IRepositoryHost EnterpriseHost public IRepositoryHostFactory RepositoryHostFactory { get; private set; } public bool IsLoggedInToAnyHost { get { return isLoggedInToAnyHost.Value; } } + + async Task ConnectionAdded(IConnection connection) + { + try + { + var host = await RepositoryHostFactory.Create(connection); + + if (connection.HostAddress == HostAddress.GitHubDotComHostAddress) + GitHubHost = host; + else + EnterpriseHost = host; + } + catch (Exception e) + { + log.Error("Error adding repository host.", e); + } + } + + void ConnectionRemoved(IConnection connection) + { + if (connection.HostAddress == HostAddress.GitHubDotComHostAddress) + { + GitHubHost = DisconnectedRepositoryHost; + } + else if (connection.HostAddress == EnterpriseHost.Address) + { + EnterpriseHost = DisconnectedRepositoryHost; + } + } + + void ConnectionsReset() + { + throw new NotSupportedException("ConnectionManager.Connections should never be reset."); + } + + async void Load() + { + try + { + foreach (var connection in connectionManager.Connections) + { + await ConnectionAdded(connection); + } + } + catch (Exception e) + { + log.Error("Error loading repository hosts.", e); + } + finally + { + loaded.SetResult(null); + } + } } } \ No newline at end of file diff --git a/src/GitHub.App/SampleData/SampleViewModels.cs b/src/GitHub.App/SampleData/SampleViewModels.cs index da44e1738b..a4a09da869 100644 --- a/src/GitHub.App/SampleData/SampleViewModels.cs +++ b/src/GitHub.App/SampleData/SampleViewModels.cs @@ -283,21 +283,6 @@ public string Title public void Dispose() { } - - public IObservable LogIn(string usernameOrEmail, string password) - { - throw new NotImplementedException(); - } - - public IObservable LogInFromCache() - { - throw new NotImplementedException(); - } - - public IObservable LogOut() - { - throw new NotImplementedException(); - } } [ExcludeFromCodeCoverage] diff --git a/src/GitHub.App/ViewModels/PullRequestListViewModel.cs b/src/GitHub.App/ViewModels/PullRequestListViewModel.cs index b7f8fbb505..63e6d73189 100644 --- a/src/GitHub.App/ViewModels/PullRequestListViewModel.cs +++ b/src/GitHub.App/ViewModels/PullRequestListViewModel.cs @@ -148,11 +148,11 @@ async Task Load() pullRequests.OriginalCompleted .ObserveOn(RxApp.MainThreadScheduler) - .Catch(ex => - { - log.Info("Received AuthorizationException reading pull requests", ex); - return repositoryHost.LogOut(); - }) + ////.Catch(ex => + ////{ + //// log.Info("Received AuthorizationException reading pull requests", ex); + //// return repositoryHost.LogOut(); + ////}) .Catch(ex => { // Occurs on network error, when the repository was deleted on GitHub etc. diff --git a/src/GitHub.Exports.Reactive/Factories/IRepositoryHostFactory.cs b/src/GitHub.Exports.Reactive/Factories/IRepositoryHostFactory.cs index 814a36d0cb..10f84644a3 100644 --- a/src/GitHub.Exports.Reactive/Factories/IRepositoryHostFactory.cs +++ b/src/GitHub.Exports.Reactive/Factories/IRepositoryHostFactory.cs @@ -1,13 +1,11 @@ using System; using System.Threading.Tasks; using GitHub.Models; -using GitHub.Primitives; namespace GitHub.Factories { - public interface IRepositoryHostFactory : IDisposable + public interface IRepositoryHostFactory { - Task Create(HostAddress hostAddress); - void Remove(IRepositoryHost host); + Task Create(IConnection connection); } } \ No newline at end of file diff --git a/src/GitHub.Exports.Reactive/Models/IRepositoryHost.cs b/src/GitHub.Exports.Reactive/Models/IRepositoryHost.cs index 2dca26cdd4..e7cfac2c56 100644 --- a/src/GitHub.Exports.Reactive/Models/IRepositoryHost.cs +++ b/src/GitHub.Exports.Reactive/Models/IRepositoryHost.cs @@ -7,16 +7,12 @@ namespace GitHub.Models { - public interface IRepositoryHost : IDisposable + public interface IRepositoryHost { HostAddress Address { get; } IApiClient ApiClient { get; } IModelService ModelService { get; } bool IsLoggedIn { get; } string Title { get; } - - IObservable LogIn(string usernameOrEmail, string password); - IObservable LogInFromCache(); - IObservable LogOut(); } } diff --git a/src/GitHub.Exports.Reactive/Models/IRepositoryHosts.cs b/src/GitHub.Exports.Reactive/Models/IRepositoryHosts.cs index e9b9b45096..9cd6e44168 100644 --- a/src/GitHub.Exports.Reactive/Models/IRepositoryHosts.cs +++ b/src/GitHub.Exports.Reactive/Models/IRepositoryHosts.cs @@ -1,4 +1,5 @@ using System; +using System.Threading.Tasks; using GitHub.Authentication; using GitHub.Factories; using GitHub.Primitives; @@ -10,11 +11,11 @@ public interface IRepositoryHosts : IReactiveObject, IDisposable { IRepositoryHost EnterpriseHost { get; set; } IRepositoryHost GitHubHost { get; } + Task EnsureInitialized(); IObservable LogIn( HostAddress hostAddress, - string usernameOrEmail, + string username, string password); - IObservable LogInFromCache(HostAddress hostAddress); IRepositoryHostFactory RepositoryHostFactory { get; } bool IsLoggedInToAnyHost { get; } IRepositoryHost LookupHost(HostAddress address); diff --git a/src/GitHub.Exports/Services/IConnectionManager.cs b/src/GitHub.Exports/Services/IConnectionManager.cs index 66045ec693..aa79e5cf24 100644 --- a/src/GitHub.Exports/Services/IConnectionManager.cs +++ b/src/GitHub.Exports/Services/IConnectionManager.cs @@ -22,6 +22,19 @@ public interface IConnectionManager /// IReadOnlyObservableCollection Connections { get; } + /// + /// Gets a callback that is called after a new is created but + /// before it is added to . + /// + /// + /// This is a hack and should be removed as soon as possible! It's needed because creating + /// a RepositoryHost is async meaning that for a short time a connection can exist without + /// a repository host, but other older parts of the code get confused by this. This callback + /// allows RepositoryHosts to hook into the creation of the connection to make sure a host + /// is available by the time the connection is made available. + /// + Func ConnectionCreated { get; set; } + /// /// Gets a connection with the specified host address. /// diff --git a/src/GitHub.Extensions/GitHub.Extensions.csproj b/src/GitHub.Extensions/GitHub.Extensions.csproj index 6fa1a678d6..51ac047fb4 100644 --- a/src/GitHub.Extensions/GitHub.Extensions.csproj +++ b/src/GitHub.Extensions/GitHub.Extensions.csproj @@ -62,6 +62,7 @@ + Properties\SolutionInfo.cs diff --git a/src/GitHub.Extensions/ObservableCollectionExtensions.cs b/src/GitHub.Extensions/ObservableCollectionExtensions.cs new file mode 100644 index 0000000000..0a761eb256 --- /dev/null +++ b/src/GitHub.Extensions/ObservableCollectionExtensions.cs @@ -0,0 +1,101 @@ +using System; +using System.Collections; +using System.Collections.Specialized; + +namespace GitHub.Extensions +{ + public static class ObservableCollectionExtensions + { + /// + /// Invokes an action for each item in a collection and subsequently each item added or + /// removed from the collection. + /// + /// The type of the collection items. + /// The collection. + /// + /// An action called initially for each item in the collection and subsequently for each + /// item added to the collection. + /// + /// + /// An action called for each item removed from the collection. + /// + /// + /// An action called when the collection is reset. This will be followed by calls to + /// for each item present in the collection after the reset. + /// + /// A disposable used to terminate the subscription. + public static IDisposable ForEachItem( + this IReadOnlyObservableCollection collection, + Action added, + Action removed, + Action reset) + { + Action add = items => + { + foreach (T item in items) + { + added(item); + } + }; + + Action remove = items => + { + for (var i = items.Count - 1; i >= 0; --i) + { + removed((T)items[i]); + } + }; + + NotifyCollectionChangedEventHandler handler = (_, e) => + { + switch (e.Action) + { + case NotifyCollectionChangedAction.Add: + add(e.NewItems); + break; + + case NotifyCollectionChangedAction.Move: + case NotifyCollectionChangedAction.Replace: + remove(e.OldItems); + add(e.NewItems); + break; + + case NotifyCollectionChangedAction.Remove: + remove(e.OldItems); + break; + + case NotifyCollectionChangedAction.Reset: + if (reset == null) + { + throw new InvalidOperationException( + "Reset called on collection without reset handler."); + } + + reset(); + add((IList)collection); + break; + } + }; + + add((IList)collection); + collection.CollectionChanged += handler; + + return new ActionDisposable(() => collection.CollectionChanged -= handler); + } + + class ActionDisposable : IDisposable + { + Action dispose; + + public ActionDisposable(Action dispose) + { + this.dispose = dispose; + } + + public void Dispose() + { + dispose(); + } + } + } +} diff --git a/src/GitHub.InlineReviews/Services/PullRequestSessionManager.cs b/src/GitHub.InlineReviews/Services/PullRequestSessionManager.cs index 63ce55063b..ea5db6d669 100644 --- a/src/GitHub.InlineReviews/Services/PullRequestSessionManager.cs +++ b/src/GitHub.InlineReviews/Services/PullRequestSessionManager.cs @@ -179,7 +179,6 @@ async Task RepoChanged(ILocalRepositoryModel repository) try { await ThreadingHelper.SwitchToMainThreadAsync(); - await EnsureLoggedIn(repository); if (repository != this.repository) { @@ -269,15 +268,6 @@ await modelService.GetCurrentUser(), return session; } - async Task EnsureLoggedIn(ILocalRepositoryModel repository) - { - if (!hosts.IsLoggedInToAnyHost && !string.IsNullOrWhiteSpace(repository?.CloneUrl)) - { - var hostAddress = HostAddress.Create(repository.CloneUrl); - await hosts.LogInFromCache(hostAddress); - } - } - async Task UpdateLiveFile(PullRequestSessionLiveFile file, bool rebuildThreads) { var session = CurrentSession; diff --git a/src/GitHub.VisualStudio/Services/ConnectionManager.cs b/src/GitHub.VisualStudio/Services/ConnectionManager.cs index e624c62bfd..777113e2f9 100644 --- a/src/GitHub.VisualStudio/Services/ConnectionManager.cs +++ b/src/GitHub.VisualStudio/Services/ConnectionManager.cs @@ -49,6 +49,8 @@ public ConnectionManager( /// public IReadOnlyObservableCollection Connections => connections.Value; + public Func ConnectionCreated { get; set; } + /// public async Task GetConnection(HostAddress address) { @@ -74,6 +76,12 @@ public async Task LogIn(HostAddress address, string userName, strin var client = CreateClient(address); var user = await loginManager.Login(address, client, userName, password); var connection = new Connection(address, userName, user, null); + + if (ConnectionCreated != null) + { + await ConnectionCreated(connection); + } + conns.Add(connection); await SaveConnections(); return connection; @@ -136,7 +144,14 @@ async Task LoadConnections(ObservableCollection result) error = e; } - result.Add(new Connection(c.HostAddress, c.UserName, user, error)); + var connection = new Connection(c.HostAddress, c.UserName, user, error); + + if (ConnectionCreated != null) + { + await ConnectionCreated(connection); + } + + result.Add(connection); } } finally diff --git a/src/UnitTests/GitHub.App/Models/RepositoryHostTests.cs b/src/UnitTests/GitHub.App/Models/RepositoryHostTests.cs index 93db5bf0a7..68ad8bbcc8 100644 --- a/src/UnitTests/GitHub.App/Models/RepositoryHostTests.cs +++ b/src/UnitTests/GitHub.App/Models/RepositoryHostTests.cs @@ -16,115 +16,4 @@ public class RepositoryHostTests { - public class TheLoginMethod : TestBaseClass - { - [Fact] - public async Task LogsTheUserInSuccessfullyAndCachesRelevantInfo() - { - var apiClient = Substitute.For(); - apiClient.HostAddress.Returns(HostAddress.GitHubDotComHostAddress); - apiClient.GetOrCreateApplicationAuthenticationCode( - Args.TwoFactorChallengCallback, Args.String, Args.Boolean) - .Returns(Observable.Return(new ApplicationAuthorization("S3CR3TS"))); - apiClient.GetUser().Returns(Observable.Return(CreateOctokitUser("baymax"))); - var hostCache = new InMemoryBlobCache(); - var modelService = new ModelService(apiClient, hostCache, Substitute.For()); - var loginManager = Substitute.For(); - loginManager.Login(HostAddress.GitHubDotComHostAddress, Arg.Any(), "baymax", "aPassword").Returns(CreateOctokitUser("baymax")); - var keychain = Substitute.For(); - var usage = Substitute.For(); - var host = new RepositoryHost(apiClient, modelService, loginManager, keychain, usage); - - var result = await host.LogIn("baymax", "aPassword"); - - Assert.Equal(AuthenticationResult.Success, result); - var user = await hostCache.GetObject("user"); - Assert.NotNull(user); - Assert.Equal("baymax", user.Login); - } - - [Fact] - public async Task IncrementsLoginCount() - { - var apiClient = Substitute.For(); - apiClient.HostAddress.Returns(HostAddress.GitHubDotComHostAddress); - apiClient.GetOrCreateApplicationAuthenticationCode( - Args.TwoFactorChallengCallback, Args.String, Args.Boolean) - .Returns(Observable.Return(new ApplicationAuthorization("S3CR3TS"))); - apiClient.GetUser().Returns(Observable.Return(CreateOctokitUser("baymax"))); - var hostCache = new InMemoryBlobCache(); - var modelService = Substitute.For(); - var loginManager = Substitute.For(); - loginManager.Login(HostAddress.GitHubDotComHostAddress, Arg.Any(), "baymax", "aPassword").Returns(CreateOctokitUser("baymax")); - var keychain = Substitute.For(); - var usage = Substitute.For(); - var host = new RepositoryHost(apiClient, modelService, loginManager, keychain, usage); - - var result = await host.LogIn("baymax", "aPassword"); - - await usage.Received().IncrementLoginCount(); - } - - [Fact] - public async Task DoesNotLogInWhenRetrievingOauthTokenFails() - { - var apiClient = Substitute.For(); - apiClient.HostAddress.Returns(HostAddress.GitHubDotComHostAddress); - var hostCache = new InMemoryBlobCache(); - var modelService = new ModelService(apiClient, hostCache, Substitute.For()); - var loginManager = Substitute.For(); - loginManager.Login(HostAddress.GitHubDotComHostAddress, Arg.Any(), "jiminy", "cricket") - .Returns(_ => { throw new NotFoundException("", HttpStatusCode.BadGateway); }); - var keychain = Substitute.For(); - var usage = Substitute.For(); - var host = new RepositoryHost(apiClient, modelService, loginManager, keychain, usage); - - await Assert.ThrowsAsync(async () => await host.LogIn("jiminy", "cricket")); - - await Assert.ThrowsAsync( - async () => await hostCache.GetObject("user")); - } - } - - public class TheLoginFromCacheMethod : TestBaseClass - { - [Fact] - public async Task LogsTheUserInSuccessfullyAndCachesRelevantInfo() - { - var apiClient = Substitute.For(); - apiClient.HostAddress.Returns(HostAddress.GitHubDotComHostAddress); - var hostCache = new InMemoryBlobCache(); - var modelService = new ModelService(apiClient, hostCache, Substitute.For()); - var loginManager = Substitute.For(); - loginManager.LoginFromCache(HostAddress.GitHubDotComHostAddress, Arg.Any()).Returns(CreateOctokitUser("baymax")); - var keychain = Substitute.For(); - var usage = Substitute.For(); - var host = new RepositoryHost(apiClient, modelService, loginManager, keychain, usage); - - var result = await host.LogInFromCache(); - - Assert.Equal(AuthenticationResult.Success, result); - var user = await hostCache.GetObject("user"); - Assert.NotNull(user); - Assert.Equal("baymax", user.Login); - } - - [Fact] - public async Task IncrementsLoginCount() - { - var apiClient = Substitute.For(); - apiClient.HostAddress.Returns(HostAddress.GitHubDotComHostAddress); - var hostCache = new InMemoryBlobCache(); - var modelService = Substitute.For(); - var loginManager = Substitute.For(); - loginManager.LoginFromCache(HostAddress.GitHubDotComHostAddress, Arg.Any()).Returns(CreateOctokitUser("baymax")); - var keychain = Substitute.For(); - var usage = Substitute.For(); - var host = new RepositoryHost(apiClient, modelService, loginManager, keychain, usage); - - var result = await host.LogInFromCache(); - - await usage.Received().IncrementLoginCount(); - } - } } diff --git a/src/UnitTests/GitHub.App/Models/RepositoryHostsTests.cs b/src/UnitTests/GitHub.App/Models/RepositoryHostsTests.cs new file mode 100644 index 0000000000..ada90bb711 --- /dev/null +++ b/src/UnitTests/GitHub.App/Models/RepositoryHostsTests.cs @@ -0,0 +1,198 @@ +using System; +using System.Linq; +using System.Reactive.Linq; +using System.Threading.Tasks; +using GitHub.Extensions; +using GitHub.Factories; +using GitHub.Models; +using GitHub.Primitives; +using GitHub.Services; +using NSubstitute; +using Xunit; + +namespace UnitTests.GitHub.App.Models +{ + public class RepositoryHostsTests + { + static readonly HostAddress EnterpriseHostAddress = HostAddress.Create("https://enterprise.host"); + static readonly HostAddress InvalidHostAddress = HostAddress.Create("https://invalid.host"); + + public class TheGitHubHostProperty + { + [Fact] + public void ShouldInitiallyBeDisconnectedRepositoryHost() + { + var target = new RepositoryHosts( + CreateRepositoryHostFactory(), + CreateConnectionManager()); + + Assert.Same(RepositoryHosts.DisconnectedRepositoryHost, target.GitHubHost); + } + + [Fact] + public void ShouldCreateHostForExistingConnection() + { + var target = new RepositoryHosts( + CreateRepositoryHostFactory(), + CreateConnectionManager(HostAddress.GitHubDotComHostAddress)); + + Assert.NotSame(RepositoryHosts.DisconnectedRepositoryHost, target.GitHubHost); + } + + [Fact] + public async Task ShouldCreateHostWhenConnectionLoggedIn() + { + var connectionManager = CreateConnectionManager(); + var target = new RepositoryHosts( + CreateRepositoryHostFactory(), + connectionManager); + + await connectionManager.LogIn(HostAddress.GitHubDotComHostAddress, "user", "pass"); + + Assert.NotSame(RepositoryHosts.DisconnectedRepositoryHost, target.GitHubHost); + } + + [Fact] + public async Task ShouldRemoveHostWhenConnectionLoggedOut() + { + var connectionManager = CreateConnectionManager(HostAddress.GitHubDotComHostAddress); + var target = new RepositoryHosts( + CreateRepositoryHostFactory(), + connectionManager); + + await connectionManager.LogOut(HostAddress.GitHubDotComHostAddress); + + Assert.Same(RepositoryHosts.DisconnectedRepositoryHost, target.GitHubHost); + } + } + + public class TheEnterpriseHostProperty + { + [Fact] + public void ShouldInitiallyBeDisconnectedRepositoryHost() + { + var target = new RepositoryHosts( + CreateRepositoryHostFactory(), + CreateConnectionManager()); + + Assert.Same(RepositoryHosts.DisconnectedRepositoryHost, target.EnterpriseHost); + } + + [Fact] + public void ShouldCreateHostForExistingConnection() + { + var target = new RepositoryHosts( + CreateRepositoryHostFactory(), + CreateConnectionManager(EnterpriseHostAddress)); + + Assert.NotSame(RepositoryHosts.DisconnectedRepositoryHost, target.EnterpriseHost); + } + + [Fact] + public async Task ShouldCreateHostWhenConnectionLoggedIn() + { + var connectionManager = CreateConnectionManager(); + var target = new RepositoryHosts( + CreateRepositoryHostFactory(), + connectionManager); + + await connectionManager.LogIn(EnterpriseHostAddress, "user", "pass"); + + Assert.NotSame(RepositoryHosts.DisconnectedRepositoryHost, target.EnterpriseHost); + } + + [Fact] + public async Task ShouldRemoveHostWhenConnectionLoggedOut() + { + var connectionManager = CreateConnectionManager(EnterpriseHostAddress); + var target = new RepositoryHosts( + CreateRepositoryHostFactory(), + connectionManager); + + await connectionManager.LogOut(EnterpriseHostAddress); + + Assert.Same(RepositoryHosts.DisconnectedRepositoryHost, target.EnterpriseHost); + } + } + + public class TheLoginMethod + { + [Fact] + public async Task ShouldCreateGitHubHost() + { + var target = new RepositoryHosts( + CreateRepositoryHostFactory(), + CreateConnectionManager()); + + await target.LogIn(HostAddress.GitHubDotComHostAddress, "user", "pass"); + + Assert.NotSame(RepositoryHosts.DisconnectedRepositoryHost, target.GitHubHost); + } + + [Fact] + public async Task ShouldFailForInvalidHost() + { + var target = new RepositoryHosts( + CreateRepositoryHostFactory(), + CreateConnectionManager()); + + await Assert.ThrowsAsync(async () => + { + await target.LogIn(InvalidHostAddress, "user", "pass"); + }); + + Assert.Same(RepositoryHosts.DisconnectedRepositoryHost, target.GitHubHost); + } + } + + static IConnectionManager CreateConnectionManager(params HostAddress[] hostAddresses) + { + var result = Substitute.For(); + var connectionSource = hostAddresses.Select(x => new Connection(x, "user", null, null)); + var connections = new ObservableCollectionEx(connectionSource); + + result.Connections.Returns(connections); + + result.LogIn(null, null, null).ReturnsForAnyArgs(x => + { + var hostAddress = x.Arg(); + + if (hostAddress == InvalidHostAddress) + { + throw new Exception("Invalid host address."); + } + + var connection = new Connection( + hostAddress, + x.ArgAt(1), + null, + null); + connections.Add(connection); + return connection; + }); + + result.LogOut(null).ReturnsForAnyArgs(x => + { + var connection = connections.Single(y => y.HostAddress == x.Arg()); + connections.Remove(connection); + return Task.CompletedTask; + }); + + return result; + } + + static IRepositoryHostFactory CreateRepositoryHostFactory() + { + var result = Substitute.For(); + + result.Create(null).ReturnsForAnyArgs(x => + { + var host = Substitute.For(); + host.Address.Returns(x.Arg().HostAddress); + return host; + }); + + return result; + } + } +} diff --git a/src/UnitTests/UnitTests.csproj b/src/UnitTests/UnitTests.csproj index fd5693fcb0..93b8ac533f 100644 --- a/src/UnitTests/UnitTests.csproj +++ b/src/UnitTests/UnitTests.csproj @@ -227,6 +227,7 @@ + From 7cf89c9ebf9bbec30c1d6046f34ec5f682a97dd3 Mon Sep 17 00:00:00 2001 From: Steven Kirk Date: Mon, 23 Oct 2017 16:57:18 +0200 Subject: [PATCH 04/16] Added DebuggerDisplay back in. --- src/GitHub.App/Models/RepositoryHost.cs | 3 +++ 1 file changed, 3 insertions(+) diff --git a/src/GitHub.App/Models/RepositoryHost.cs b/src/GitHub.App/Models/RepositoryHost.cs index 249cc06eae..c1bcd734fa 100644 --- a/src/GitHub.App/Models/RepositoryHost.cs +++ b/src/GitHub.App/Models/RepositoryHost.cs @@ -5,6 +5,7 @@ using GitHub.Primitives; using GitHub.Services; using ReactiveUI; +using static System.FormattableString; namespace GitHub.Models { @@ -32,5 +33,7 @@ public RepositoryHost( public bool IsLoggedIn => connection.IsLoggedIn; public IModelService ModelService { get; } public string Title => ApiClient.HostAddress.Title; + + internal string DebuggerDisplay => Invariant($"RepositoryHost: {Title} {Address.ApiUri}"); } } From 213a7fc7269e1244908815863d8c9553e2ba5837 Mon Sep 17 00:00:00 2001 From: Steven Kirk Date: Tue, 24 Oct 2017 12:20:08 +0200 Subject: [PATCH 05/16] Moved LocalRepositories to GitHub.VisualStudio. Following advice by @shana. --- src/GitHub.App/GitHub.App.csproj | 1 - src/GitHub.VisualStudio/GitHub.VisualStudio.csproj | 1 + .../Services/LocalRepositories.cs | 3 +-- 3 files changed, 2 insertions(+), 3 deletions(-) rename src/{GitHub.App => GitHub.VisualStudio}/Services/LocalRepositories.cs (96%) diff --git a/src/GitHub.App/GitHub.App.csproj b/src/GitHub.App/GitHub.App.csproj index a8372b644b..206ece6bf9 100644 --- a/src/GitHub.App/GitHub.App.csproj +++ b/src/GitHub.App/GitHub.App.csproj @@ -130,7 +130,6 @@ - diff --git a/src/GitHub.VisualStudio/GitHub.VisualStudio.csproj b/src/GitHub.VisualStudio/GitHub.VisualStudio.csproj index 6ff30377c2..65c3ccb241 100644 --- a/src/GitHub.VisualStudio/GitHub.VisualStudio.csproj +++ b/src/GitHub.VisualStudio/GitHub.VisualStudio.csproj @@ -303,6 +303,7 @@ + diff --git a/src/GitHub.App/Services/LocalRepositories.cs b/src/GitHub.VisualStudio/Services/LocalRepositories.cs similarity index 96% rename from src/GitHub.App/Services/LocalRepositories.cs rename to src/GitHub.VisualStudio/Services/LocalRepositories.cs index bd092a4b11..d413b39422 100644 --- a/src/GitHub.App/Services/LocalRepositories.cs +++ b/src/GitHub.VisualStudio/Services/LocalRepositories.cs @@ -5,9 +5,8 @@ using GitHub.Extensions; using GitHub.Helpers; using GitHub.Models; -using GitHub.Services; -namespace GitHub.App.Services +namespace GitHub.Services { /// /// Stores a collection of known local repositories. From 90c35c573f881e6ed2d6a448733c270415f98a66 Mon Sep 17 00:00:00 2001 From: Steven Kirk Date: Thu, 26 Oct 2017 11:06:44 +0200 Subject: [PATCH 06/16] Track login metrics. --- .../Services/ConnectionManager.cs | 7 +++- .../Services/ConnectionManagerTests.cs | 39 ++++++++++++------- 2 files changed, 32 insertions(+), 14 deletions(-) diff --git a/src/GitHub.VisualStudio/Services/ConnectionManager.cs b/src/GitHub.VisualStudio/Services/ConnectionManager.cs index 777113e2f9..e69ed64610 100644 --- a/src/GitHub.VisualStudio/Services/ConnectionManager.cs +++ b/src/GitHub.VisualStudio/Services/ConnectionManager.cs @@ -28,18 +28,21 @@ public class ConnectionManager : IConnectionManager readonly ILoginManager loginManager; readonly TaskCompletionSource loaded; readonly Lazy> connections; + readonly IUsageTracker usageTracker; [ImportingConstructor] public ConnectionManager( IProgram program, IConnectionCache cache, IKeychain keychain, - ILoginManager loginManager) + ILoginManager loginManager, + IUsageTracker usageTracker) { this.program = program; this.cache = cache; this.keychain = keychain; this.loginManager = loginManager; + this.usageTracker = usageTracker; loaded = new TaskCompletionSource(); connections = new Lazy>( this.CreateConnections, @@ -84,6 +87,7 @@ public async Task LogIn(HostAddress address, string userName, strin conns.Add(connection); await SaveConnections(); + await usageTracker.IncrementLoginCount(); return connection; } @@ -152,6 +156,7 @@ async Task LoadConnections(ObservableCollection result) } result.Add(connection); + await usageTracker.IncrementLoginCount(); } } finally diff --git a/src/UnitTests/GitHub.VisualStudio/Services/ConnectionManagerTests.cs b/src/UnitTests/GitHub.VisualStudio/Services/ConnectionManagerTests.cs index f3faafee4c..dc1af0573c 100644 --- a/src/UnitTests/GitHub.VisualStudio/Services/ConnectionManagerTests.cs +++ b/src/UnitTests/GitHub.VisualStudio/Services/ConnectionManagerTests.cs @@ -22,7 +22,8 @@ public async Task ReturnsValidConnections() CreateProgram(), CreateConnectionCache("github", "valid"), Substitute.For(), - CreateLoginManager()); + CreateLoginManager(), + Substitute.For()); var result = await target.GetLoadedConnections(); Assert.Equal(2, result.Count); @@ -39,7 +40,8 @@ public async Task ReturnsInvalidConnections() CreateProgram(), CreateConnectionCache("github", "invalid"), Substitute.For(), - CreateLoginManager()); + CreateLoginManager(), + Substitute.For()); var result = await target.GetLoadedConnections(); Assert.Equal(2, result.Count); @@ -59,7 +61,8 @@ public async Task ReturnsCorrectConnection() CreateProgram(), CreateConnectionCache("github", "valid"), Substitute.For(), - CreateLoginManager()); + CreateLoginManager(), + Substitute.For()); var result = await target.GetConnection(HostAddress.Create("valid.com")); Assert.Equal("https://valid.com/", result.HostAddress.WebUri.ToString()); @@ -72,7 +75,8 @@ public async Task ReturnsCorrectNullForNotFoundConnection() CreateProgram(), CreateConnectionCache("github", "valid"), Substitute.For(), - CreateLoginManager()); + CreateLoginManager(), + Substitute.For()); var result = await target.GetConnection(HostAddress.Create("another.com")); Assert.Null(result); @@ -88,7 +92,8 @@ public async Task ReturnsLoggedInConnection() CreateProgram(), CreateConnectionCache(), Substitute.For(), - CreateLoginManager()); + CreateLoginManager(), + Substitute.For()); var result = await target.LogIn(HostAddress.GitHubDotComHostAddress, "user", "pass"); Assert.NotNull(result); @@ -101,7 +106,8 @@ public async Task AddsLoggedInConnectionToConnections() CreateProgram(), CreateConnectionCache(), Substitute.For(), - CreateLoginManager()); + CreateLoginManager(), + Substitute.For()); await target.LogIn(HostAddress.GitHubDotComHostAddress, "user", "pass"); @@ -115,7 +121,8 @@ public async Task ThrowsWhenLoginFails() CreateProgram(), CreateConnectionCache(), Substitute.For(), - CreateLoginManager()); + CreateLoginManager(), + Substitute.For()); await Assert.ThrowsAsync(async () => await target.LogIn(HostAddress.Create("invalid.com"), "user", "pass")); @@ -128,7 +135,8 @@ public async Task ThrowsWhenExistingConnectionExists() CreateProgram(), CreateConnectionCache("github"), Substitute.For(), - CreateLoginManager()); + CreateLoginManager(), + Substitute.For()); await Assert.ThrowsAsync(async () => await target.LogIn(HostAddress.GitHubDotComHostAddress, "user", "pass")); @@ -142,7 +150,8 @@ public async Task SavesConnectionToCache() CreateProgram(), cache, Substitute.For(), - CreateLoginManager()); + CreateLoginManager(), + Substitute.For()); await target.LogIn(HostAddress.GitHubDotComHostAddress, "user", "pass"); @@ -161,7 +170,8 @@ public async Task CallsLoginManagerLogOut() CreateProgram(), CreateConnectionCache("github"), Substitute.For(), - loginManager); + loginManager, + Substitute.For()); await target.LogOut(HostAddress.GitHubDotComHostAddress); @@ -178,7 +188,8 @@ public async Task RemovesConnectionFromConnections() CreateProgram(), CreateConnectionCache("github"), Substitute.For(), - loginManager); + loginManager, + Substitute.For()); await target.LogOut(HostAddress.GitHubDotComHostAddress); @@ -193,7 +204,8 @@ public async Task ThrowsIfConnectionDoesntExist() CreateProgram(), CreateConnectionCache("valid"), Substitute.For(), - loginManager); + loginManager, + Substitute.For()); await Assert.ThrowsAsync(async () => await target.LogOut(HostAddress.GitHubDotComHostAddress)); @@ -207,7 +219,8 @@ public async Task RemovesConnectionFromCache() CreateProgram(), cache, Substitute.For(), - CreateLoginManager()); + CreateLoginManager(), + Substitute.For()); await target.LogOut(HostAddress.GitHubDotComHostAddress); From 9db05a73139d354bb8246739c03c92cf3ce04cba Mon Sep 17 00:00:00 2001 From: Steven Kirk Date: Thu, 26 Oct 2017 11:11:53 +0200 Subject: [PATCH 07/16] Fix failing tests. --- .../GitHub.App/Models/RepositoryHostsTests.cs | 12 ++++++++++-- 1 file changed, 10 insertions(+), 2 deletions(-) diff --git a/src/UnitTests/GitHub.App/Models/RepositoryHostsTests.cs b/src/UnitTests/GitHub.App/Models/RepositoryHostsTests.cs index ada90bb711..13f11c6019 100644 --- a/src/UnitTests/GitHub.App/Models/RepositoryHostsTests.cs +++ b/src/UnitTests/GitHub.App/Models/RepositoryHostsTests.cs @@ -153,7 +153,7 @@ static IConnectionManager CreateConnectionManager(params HostAddress[] hostAddre result.Connections.Returns(connections); - result.LogIn(null, null, null).ReturnsForAnyArgs(x => + Func> login = async x => { var hostAddress = x.Arg(); @@ -167,9 +167,17 @@ static IConnectionManager CreateConnectionManager(params HostAddress[] hostAddre x.ArgAt(1), null, null); + + if (result.ConnectionCreated != null) + { + await result.ConnectionCreated(connection); + } + connections.Add(connection); return connection; - }); + }; + + result.LogIn(null, null, null).ReturnsForAnyArgs(login); result.LogOut(null).ReturnsForAnyArgs(x => { From f854b79087a99cc631f8f245acb0f08330d79b7a Mon Sep 17 00:00:00 2001 From: Steven Kirk Date: Thu, 26 Oct 2017 18:39:56 +0200 Subject: [PATCH 08/16] WIP: Removing RepositoryHost(s) --- src/GitHub.App/Controllers/UIController.cs | 10 +- .../Factories/RepositoryHostFactory.cs | 37 ---- src/GitHub.App/GitHub.App.csproj | 4 - .../Models/ConnectionRepositoryHostMap.cs | 34 --- .../Models/DisconnectedRepositoryHosts.cs | 52 ----- src/GitHub.App/Models/RepositoryHost.cs | 39 ---- src/GitHub.App/Models/RepositoryHosts.cs | 200 ------------------ src/GitHub.App/SampleData/SampleViewModels.cs | 43 ---- src/GitHub.App/Services/ModelService.cs | 5 +- src/GitHub.App/Services/PullRequestService.cs | 10 +- .../ViewModels/GistCreationViewModel.cs | 11 +- .../ViewModels/LoginControlViewModel.cs | 13 +- .../ViewModels/LoginTabViewModel.cs | 13 +- .../LoginToGitHubForEnterpriseViewModel.cs | 5 +- .../ViewModels/LoginToGitHubViewModel.cs | 4 +- .../PullRequestCreationViewModel.cs | 26 +-- .../ViewModels/PullRequestDetailViewModel.cs | 2 +- .../ViewModels/PullRequestListViewModel.cs | 14 +- .../ViewModels/RepositoryCloneViewModel.cs | 18 +- .../ViewModels/RepositoryCreationViewModel.cs | 19 +- .../ViewModels/RepositoryPublishViewModel.cs | 21 +- .../ViewModels/StartPageCloneViewModel.cs | 13 +- .../Extensions/ConnectionManagerExtensions.cs | 18 +- .../Factories/IApiClientFactory.cs | 1 + .../Factories/IRepositoryHostFactory.cs | 11 - .../GitHub.Exports.Reactive.csproj | 4 - .../Models/IConnectionRepositoryHostMap.cs | 13 -- .../Models/IRepositoryHost.cs | 18 -- .../Models/IRepositoryHosts.cs | 23 -- .../Services/IModelService.cs | 1 + .../Services/IPullRequestService.cs | 3 +- 31 files changed, 93 insertions(+), 592 deletions(-) delete mode 100644 src/GitHub.App/Factories/RepositoryHostFactory.cs delete mode 100644 src/GitHub.App/Models/ConnectionRepositoryHostMap.cs delete mode 100644 src/GitHub.App/Models/DisconnectedRepositoryHosts.cs delete mode 100644 src/GitHub.App/Models/RepositoryHost.cs delete mode 100644 src/GitHub.App/Models/RepositoryHosts.cs delete mode 100644 src/GitHub.Exports.Reactive/Factories/IRepositoryHostFactory.cs delete mode 100644 src/GitHub.Exports.Reactive/Models/IConnectionRepositoryHostMap.cs delete mode 100644 src/GitHub.Exports.Reactive/Models/IRepositoryHost.cs delete mode 100644 src/GitHub.Exports.Reactive/Models/IRepositoryHosts.cs diff --git a/src/GitHub.App/Controllers/UIController.cs b/src/GitHub.App/Controllers/UIController.cs index 6c41367962..f0cb4e36cb 100644 --- a/src/GitHub.App/Controllers/UIController.cs +++ b/src/GitHub.App/Controllers/UIController.cs @@ -98,7 +98,6 @@ internal enum Trigger readonly IUIFactory factory; readonly IGitHubServiceProvider gitHubServiceProvider; - readonly IRepositoryHosts hosts; readonly IConnectionManager connectionManager; readonly CompositeDisposable disposables = new CompositeDisposable(); @@ -140,7 +139,6 @@ internal enum Trigger public UIController(IGitHubServiceProvider serviceProvider) : this(serviceProvider, - serviceProvider.TryGetService(), serviceProvider.TryGetService(), serviceProvider.TryGetService()) { @@ -148,17 +146,15 @@ public UIController(IGitHubServiceProvider serviceProvider) } public UIController(IGitHubServiceProvider gitHubServiceProvider, - IRepositoryHosts hosts, IUIFactory factory, + IUIFactory factory, IConnectionManager connectionManager) { Guard.ArgumentNotNull(gitHubServiceProvider, nameof(gitHubServiceProvider)); - Guard.ArgumentNotNull(hosts, nameof(hosts)); Guard.ArgumentNotNull(factory, nameof(factory)); Guard.ArgumentNotNull(connectionManager, nameof(connectionManager)); this.factory = factory; this.gitHubServiceProvider = gitHubServiceProvider; - this.hosts = hosts; this.connectionManager = connectionManager; #if DEBUG @@ -220,7 +216,7 @@ public void Start() else // sanity check: it makes zero sense to pass a connection in when calling the auth flow Debug.Assert(false, "Calling the auth flow with a connection makes no sense!"); - hosts.EnsureInitialized().ToObservable() + connectionManager.GetLoadedConnections().ToObservable() .ObserveOn(RxApp.MainThreadScheduler) .Subscribe(_ => { }, () => { @@ -231,7 +227,7 @@ public void Start() else { connectionManager - .GetLoggedInConnections(hosts) + .GetLoggedInConnections() .FirstOrDefaultAsync() .Select(c => { diff --git a/src/GitHub.App/Factories/RepositoryHostFactory.cs b/src/GitHub.App/Factories/RepositoryHostFactory.cs deleted file mode 100644 index a80c3f54b1..0000000000 --- a/src/GitHub.App/Factories/RepositoryHostFactory.cs +++ /dev/null @@ -1,37 +0,0 @@ -using System.ComponentModel.Composition; -using System.Threading.Tasks; -using GitHub.Models; -using GitHub.Services; - -namespace GitHub.Factories -{ - [Export(typeof(IRepositoryHostFactory))] - [PartCreationPolicy(CreationPolicy.Shared)] - public class RepositoryHostFactory : IRepositoryHostFactory - { - readonly IApiClientFactory apiClientFactory; - readonly IHostCacheFactory hostCacheFactory; - readonly IAvatarProvider avatarProvider; - - [ImportingConstructor] - public RepositoryHostFactory( - IApiClientFactory apiClientFactory, - IHostCacheFactory hostCacheFactory, - IAvatarProvider avatarProvider) - { - this.apiClientFactory = apiClientFactory; - this.hostCacheFactory = hostCacheFactory; - this.avatarProvider = avatarProvider; - } - - public async Task Create(IConnection connection) - { - var hostAddress = connection.HostAddress; - var apiClient = await apiClientFactory.Create(hostAddress); - var hostCache = await hostCacheFactory.Create(hostAddress); - var modelService = new ModelService(apiClient, hostCache, avatarProvider); - var host = new RepositoryHost(connection, apiClient, modelService); - return host; - } - } -} diff --git a/src/GitHub.App/GitHub.App.csproj b/src/GitHub.App/GitHub.App.csproj index a8372b644b..b9e0011224 100644 --- a/src/GitHub.App/GitHub.App.csproj +++ b/src/GitHub.App/GitHub.App.csproj @@ -183,9 +183,6 @@ - - - @@ -209,7 +206,6 @@ - diff --git a/src/GitHub.App/Models/ConnectionRepositoryHostMap.cs b/src/GitHub.App/Models/ConnectionRepositoryHostMap.cs deleted file mode 100644 index 856f657eca..0000000000 --- a/src/GitHub.App/Models/ConnectionRepositoryHostMap.cs +++ /dev/null @@ -1,34 +0,0 @@ -using System.ComponentModel.Composition; -using GitHub.Models; -using GitHub.Services; -using GitHub.Extensions; - -namespace GitHub.ViewModels -{ - [Export(typeof(IConnectionRepositoryHostMap))] - [PartCreationPolicy(CreationPolicy.NonShared)] - public class ConnectionRepositoryHostMap : IConnectionRepositoryHostMap - { - [ImportingConstructor] - public ConnectionRepositoryHostMap(IGitHubServiceProvider provider, IRepositoryHosts hosts) - : this(provider.TryGetService(), hosts) - { - } - - public ConnectionRepositoryHostMap(IRepositoryHost repositoryHost) - { - CurrentRepositoryHost = repositoryHost; - } - - protected ConnectionRepositoryHostMap(IConnection connection, IRepositoryHosts hosts) - : this(hosts.LookupHost(connection.HostAddress)) - { - } - - /// - /// The current repository host. This is set in the MEF sub-container when the user clicks on an action - /// related to a host such as clone or create. - /// - public IRepositoryHost CurrentRepositoryHost { get; private set; } - } -} diff --git a/src/GitHub.App/Models/DisconnectedRepositoryHosts.cs b/src/GitHub.App/Models/DisconnectedRepositoryHosts.cs deleted file mode 100644 index c0f2790304..0000000000 --- a/src/GitHub.App/Models/DisconnectedRepositoryHosts.cs +++ /dev/null @@ -1,52 +0,0 @@ - -using System; -using System.Reactive; -using System.Reactive.Linq; -using GitHub.Api; -using GitHub.Authentication; -using GitHub.Extensions; -using GitHub.Primitives; -using GitHub.Services; -using ReactiveUI; - -namespace GitHub.Models -{ - public class DisconnectedRepositoryHost : ReactiveObject, IRepositoryHost - { - public DisconnectedRepositoryHost() - { - Address = HostAddress.Create(new Uri("https://null/")); - Organizations = new ReactiveList(); - Accounts = new ReactiveList(); - } - - public HostAddress Address { get; private set; } - public IApiClient ApiClient { get; private set; } - - public bool IsLoggedIn { get; private set; } - public bool IsLoggingIn { get; private set; } - public ReactiveList Organizations { get; private set; } - public ReactiveList Accounts { get; private set; } - public string Title { get; private set; } - public IAccount User { get; private set; } - public IModelService ModelService { get; private set; } - - public IObservable LogIn(string usernameOrEmail, string password) - { - Guard.ArgumentNotEmptyString(usernameOrEmail, nameof(usernameOrEmail)); - Guard.ArgumentNotNull(password, nameof(password)); - - return Observable.Return(AuthenticationResult.CredentialFailure); - } - - public IObservable LogInFromCache() - { - return Observable.Return(AuthenticationResult.CredentialFailure); - } - - public IObservable LogOut() - { - return Observable.Return(Unit.Default); - } - } -} diff --git a/src/GitHub.App/Models/RepositoryHost.cs b/src/GitHub.App/Models/RepositoryHost.cs deleted file mode 100644 index c1bcd734fa..0000000000 --- a/src/GitHub.App/Models/RepositoryHost.cs +++ /dev/null @@ -1,39 +0,0 @@ -using System; -using System.Diagnostics; -using GitHub.Api; -using GitHub.Extensions; -using GitHub.Primitives; -using GitHub.Services; -using ReactiveUI; -using static System.FormattableString; - -namespace GitHub.Models -{ - [DebuggerDisplay("{DebuggerDisplay,nq}")] - public class RepositoryHost : ReactiveObject, IRepositoryHost - { - readonly IConnection connection; - - public RepositoryHost( - IConnection connection, - IApiClient apiClient, - IModelService modelService) - { - Guard.ArgumentNotNull(connection, nameof(connection)); - Guard.ArgumentNotNull(apiClient, nameof(apiClient)); - Guard.ArgumentNotNull(modelService, nameof(modelService)); - - this.connection = connection; - ApiClient = apiClient; - ModelService = modelService; - } - - public HostAddress Address => ApiClient.HostAddress; - public IApiClient ApiClient { get;} - public bool IsLoggedIn => connection.IsLoggedIn; - public IModelService ModelService { get; } - public string Title => ApiClient.HostAddress.Title; - - internal string DebuggerDisplay => Invariant($"RepositoryHost: {Title} {Address.ApiUri}"); - } -} diff --git a/src/GitHub.App/Models/RepositoryHosts.cs b/src/GitHub.App/Models/RepositoryHosts.cs deleted file mode 100644 index 8832d80638..0000000000 --- a/src/GitHub.App/Models/RepositoryHosts.cs +++ /dev/null @@ -1,200 +0,0 @@ -using System; -using System.ComponentModel.Composition; -using System.Reactive; -using System.Reactive.Disposables; -using System.Reactive.Linq; -using System.Threading.Tasks; -using GitHub.Authentication; -using GitHub.Extensions; -using GitHub.Factories; -using GitHub.Primitives; -using GitHub.Services; -using ReactiveUI; - -namespace GitHub.Models -{ - [Export(typeof(IRepositoryHosts))] - [PartCreationPolicy(CreationPolicy.Shared)] - public class RepositoryHosts : ReactiveObject, IRepositoryHosts - { - static readonly NLog.Logger log = NLog.LogManager.GetCurrentClassLogger(); - - public static DisconnectedRepositoryHost DisconnectedRepositoryHost = new DisconnectedRepositoryHost(); - public const string EnterpriseHostApiBaseUriCacheKey = "enterprise-host-api-base-uri"; - readonly ObservableAsPropertyHelper isLoggedInToAnyHost; - readonly IConnectionManager connectionManager; - readonly CompositeDisposable disposables = new CompositeDisposable(); - readonly TaskCompletionSource loaded; - - [ImportingConstructor] - public RepositoryHosts( - IRepositoryHostFactory repositoryHostFactory, - IConnectionManager connectionManager) - { - Guard.ArgumentNotNull(repositoryHostFactory, nameof(repositoryHostFactory)); - Guard.ArgumentNotNull(connectionManager, nameof(connectionManager)); - - this.connectionManager = connectionManager; - - RepositoryHostFactory = repositoryHostFactory; - GitHubHost = DisconnectedRepositoryHost; - EnterpriseHost = DisconnectedRepositoryHost; - - isLoggedInToAnyHost = this.WhenAny( - x => x.GitHubHost.IsLoggedIn, - x => x.EnterpriseHost.IsLoggedIn, - (githubLoggedIn, enterpriseLoggedIn) => githubLoggedIn.Value || enterpriseLoggedIn.Value) - .ToProperty(this, x => x.IsLoggedInToAnyHost); - - connectionManager.ConnectionCreated = ConnectionAdded; - - loaded = new TaskCompletionSource(); - disposables.Add(connectionManager.Connections.ForEachItem(_ => { }, ConnectionRemoved, ConnectionsReset)); - Load(); - } - - public Task EnsureInitialized() => loaded.Task; - - public IRepositoryHost LookupHost(HostAddress address) - { - if (address == GitHubHost.Address) - return GitHubHost; - if (address == EnterpriseHost.Address) - return EnterpriseHost; - return DisconnectedRepositoryHost; - } - - public IObservable LogIn(HostAddress address, string username, string password) - { - Guard.ArgumentNotNull(address, nameof(address)); - Guard.ArgumentNotEmptyString(username, nameof(username)); - Guard.ArgumentNotEmptyString(password, nameof(password)); - - return Observable.Defer(async () => - { - if (GitHubHost != DisconnectedRepositoryHost) - { - await connectionManager.LogOut(address); - } - - var connection = await connectionManager.LogIn(address, username, password); - return Observable.Return(AuthenticationResult.Success); - }); - } - - public IObservable LogOut(IRepositoryHost host) - { - Guard.ArgumentNotNull(host, nameof(host)); - - return Observable.Defer(async () => - { - await connectionManager.LogOut(host.Address); - return Observable.Return(Unit.Default); - }); - } - - bool disposed; - protected void Dispose(bool disposing) - { - if (disposing) - { - if (disposed) return; - - try - { - disposables.Dispose(); - } - catch (Exception e) - { - log.Warn("Exception occured while disposing RepositoryHosts", e); - } - disposed = true; - } - } - - public void Dispose() - { - Dispose(true); - GC.SuppressFinalize(this); - } - - IRepositoryHost githubHost; - public IRepositoryHost GitHubHost - { - get { return githubHost; } - private set - { - var newHost = value ?? DisconnectedRepositoryHost; - this.RaiseAndSetIfChanged(ref githubHost, newHost); - } - } - - IRepositoryHost enterpriseHost; - public IRepositoryHost EnterpriseHost - { - get { return enterpriseHost; } - set - { - var newHost = value ?? DisconnectedRepositoryHost; - this.RaiseAndSetIfChanged(ref enterpriseHost, newHost); - } - } - - public IRepositoryHostFactory RepositoryHostFactory { get; private set; } - - public bool IsLoggedInToAnyHost { get { return isLoggedInToAnyHost.Value; } } - - async Task ConnectionAdded(IConnection connection) - { - try - { - var host = await RepositoryHostFactory.Create(connection); - - if (connection.HostAddress == HostAddress.GitHubDotComHostAddress) - GitHubHost = host; - else - EnterpriseHost = host; - } - catch (Exception e) - { - log.Error("Error adding repository host.", e); - } - } - - void ConnectionRemoved(IConnection connection) - { - if (connection.HostAddress == HostAddress.GitHubDotComHostAddress) - { - GitHubHost = DisconnectedRepositoryHost; - } - else if (connection.HostAddress == EnterpriseHost.Address) - { - EnterpriseHost = DisconnectedRepositoryHost; - } - } - - void ConnectionsReset() - { - throw new NotSupportedException("ConnectionManager.Connections should never be reset."); - } - - async void Load() - { - try - { - foreach (var connection in connectionManager.Connections) - { - await ConnectionAdded(connection); - } - } - catch (Exception e) - { - log.Error("Error loading repository hosts.", e); - } - finally - { - loaded.SetResult(null); - } - } - } -} \ No newline at end of file diff --git a/src/GitHub.App/SampleData/SampleViewModels.cs b/src/GitHub.App/SampleData/SampleViewModels.cs index a4a09da869..afd25ca6ef 100644 --- a/src/GitHub.App/SampleData/SampleViewModels.cs +++ b/src/GitHub.App/SampleData/SampleViewModels.cs @@ -242,49 +242,6 @@ public IConnection SelectedConnection } } - [ExcludeFromCodeCoverage] - public sealed class RepositoryHostDesigner : ReactiveObject, IRepositoryHost - { - public RepositoryHostDesigner(string title) - { - this.Title = title; - } - - public HostAddress Address - { - get; - private set; - } - - public IApiClient ApiClient - { - get; - private set; - } - - public bool IsLoggedIn - { - get; - private set; - } - - public IModelService ModelService - { - get; - private set; - } - - public string Title - { - get; - private set; - } - - public void Dispose() - { - } - } - [ExcludeFromCodeCoverage] public static class RepositoryModelDesigner { diff --git a/src/GitHub.App/Services/ModelService.cs b/src/GitHub.App/Services/ModelService.cs index cf8e1a410b..7218dba685 100644 --- a/src/GitHub.App/Services/ModelService.cs +++ b/src/GitHub.App/Services/ModelService.cs @@ -33,7 +33,10 @@ public class ModelService : IModelService readonly IBlobCache hostCache; readonly IAvatarProvider avatarProvider; - public ModelService(IApiClient apiClient, IBlobCache hostCache, IAvatarProvider avatarProvider) + public ModelService( + IApiClient apiClient, + IBlobCache hostCache, + IAvatarProvider avatarProvider) { this.apiClient = apiClient; this.hostCache = hostCache; diff --git a/src/GitHub.App/Services/PullRequestService.cs b/src/GitHub.App/Services/PullRequestService.cs index 890a602a76..2116bc2196 100644 --- a/src/GitHub.App/Services/PullRequestService.cs +++ b/src/GitHub.App/Services/PullRequestService.cs @@ -50,13 +50,13 @@ public PullRequestService(IGitClient gitClient, IGitService gitService, IOperati this.usageTracker = usageTracker; } - public IObservable CreatePullRequest(IRepositoryHost host, + public IObservable CreatePullRequest(IConnection connection, ILocalRepositoryModel sourceRepository, IRepositoryModel targetRepository, IBranch sourceBranch, IBranch targetBranch, string title, string body ) { - Extensions.Guard.ArgumentNotNull(host, nameof(host)); + Extensions.Guard.ArgumentNotNull(connection, nameof(connection)); Extensions.Guard.ArgumentNotNull(sourceRepository, nameof(sourceRepository)); Extensions.Guard.ArgumentNotNull(targetRepository, nameof(targetRepository)); Extensions.Guard.ArgumentNotNull(sourceBranch, nameof(sourceBranch)); @@ -64,7 +64,7 @@ public IObservable CreatePullRequest(IRepositoryHost host, Extensions.Guard.ArgumentNotNull(title, nameof(title)); Extensions.Guard.ArgumentNotNull(body, nameof(body)); - return PushAndCreatePR(host, sourceRepository, targetRepository, sourceBranch, targetBranch, title, body).ToObservable(); + return PushAndCreatePR(connection, sourceRepository, targetRepository, sourceBranch, targetBranch, title, body).ToObservable(); } public IObservable GetPullRequestTemplate(ILocalRepositoryModel repository) @@ -487,7 +487,7 @@ async Task MarkBranchAsPullRequest(IRepository repo, string branchName, IPullReq await gitClient.SetConfig(repo, prConfigKey, BuildGHfVSConfigKeyValue(pullRequest)); } - async Task PushAndCreatePR(IRepositoryHost host, + async Task PushAndCreatePR(IConnection connection, ILocalRepositoryModel sourceRepository, IRepositoryModel targetRepository, IBranch sourceBranch, IBranch targetBranch, string title, string body) @@ -503,7 +503,7 @@ async Task PushAndCreatePR(IRepositoryHost host, if (!Splat.ModeDetector.Current.InUnitTestRunner().GetValueOrDefault()) await Task.Delay(TimeSpan.FromSeconds(5)); - var ret = await host.ModelService.CreatePullRequest(sourceRepository, targetRepository, sourceBranch, targetBranch, title, body); + var ret = await connection.ModelService.CreatePullRequest(sourceRepository, targetRepository, sourceBranch, targetBranch, title, body); await usageTracker.IncrementUpstreamPullRequestCount(); return ret; } diff --git a/src/GitHub.App/ViewModels/GistCreationViewModel.cs b/src/GitHub.App/ViewModels/GistCreationViewModel.cs index 900317a1ad..b4bcb6584e 100644 --- a/src/GitHub.App/ViewModels/GistCreationViewModel.cs +++ b/src/GitHub.App/ViewModels/GistCreationViewModel.cs @@ -13,6 +13,7 @@ using NLog; using Octokit; using ReactiveUI; +using IConnection = GitHub.Models.IConnection; namespace GitHub.ViewModels { @@ -30,14 +31,14 @@ public class GistCreationViewModel : DialogViewModelBase, IGistCreationViewModel [ImportingConstructor] GistCreationViewModel( - IConnectionRepositoryHostMap connectionRepositoryHostMap, + IConnection connection, ISelectedTextProvider selectedTextProvider, IGistPublishService gistPublishService, INotificationService notificationService, IUsageTracker usageTracker) - : this(connectionRepositoryHostMap.CurrentRepositoryHost, selectedTextProvider, gistPublishService, usageTracker) + : this(connection, selectedTextProvider, gistPublishService, usageTracker) { - Guard.ArgumentNotNull(connectionRepositoryHostMap, nameof(connectionRepositoryHostMap)); + Guard.ArgumentNotNull(connection, nameof(connection)); Guard.ArgumentNotNull(selectedTextProvider, nameof(selectedTextProvider)); Guard.ArgumentNotNull(gistPublishService, nameof(gistPublishService)); Guard.ArgumentNotNull(notificationService, nameof(notificationService)); @@ -47,12 +48,12 @@ public class GistCreationViewModel : DialogViewModelBase, IGistCreationViewModel } public GistCreationViewModel( - IRepositoryHost repositoryHost, + IConnection connection, ISelectedTextProvider selectedTextProvider, IGistPublishService gistPublishService, IUsageTracker usageTracker) { - Guard.ArgumentNotNull(repositoryHost, nameof(repositoryHost)); + Guard.ArgumentNotNull(connection, nameof(connection)); Guard.ArgumentNotNull(selectedTextProvider, nameof(selectedTextProvider)); Guard.ArgumentNotNull(gistPublishService, nameof(gistPublishService)); Guard.ArgumentNotNull(usageTracker, nameof(usageTracker)); diff --git a/src/GitHub.App/ViewModels/LoginControlViewModel.cs b/src/GitHub.App/ViewModels/LoginControlViewModel.cs index f421426655..416e4e418d 100644 --- a/src/GitHub.App/ViewModels/LoginControlViewModel.cs +++ b/src/GitHub.App/ViewModels/LoginControlViewModel.cs @@ -7,6 +7,7 @@ using GitHub.Exports; using GitHub.Extensions.Reactive; using GitHub.Models; +using GitHub.Services; using ReactiveUI; namespace GitHub.ViewModels @@ -17,12 +18,12 @@ public class LoginControlViewModel : DialogViewModelBase, ILoginControlViewModel { [ImportingConstructor] public LoginControlViewModel( - IRepositoryHosts hosts, + IConnectionManager connectionManager, ILoginToGitHubViewModel loginToGitHubViewModel, ILoginToGitHubForEnterpriseViewModel loginToGitHubEnterpriseViewModel) { Title = Resources.LoginTitle; - RepositoryHosts = hosts; + ConnectionManager = connectionManager; GitHubLogin = loginToGitHubViewModel; EnterpriseLogin = loginToGitHubEnterpriseViewModel; @@ -66,11 +67,11 @@ public ILoginToGitHubForEnterpriseViewModel EnterpriseLogin set { this.RaiseAndSetIfChanged(ref githubEnterprise, value); } } - IRepositoryHosts repositoryHosts; - public IRepositoryHosts RepositoryHosts + IConnectionManager connectionManager; + public IConnectionManager ConnectionManager { - get { return repositoryHosts; } - set { this.RaiseAndSetIfChanged(ref repositoryHosts, value); } + get { return connectionManager; } + set { this.RaiseAndSetIfChanged(ref connectionManager, value); } } readonly ObservableAsPropertyHelper loginMode; diff --git a/src/GitHub.App/ViewModels/LoginTabViewModel.cs b/src/GitHub.App/ViewModels/LoginTabViewModel.cs index ee95dab62e..5e31e1a987 100644 --- a/src/GitHub.App/ViewModels/LoginTabViewModel.cs +++ b/src/GitHub.App/ViewModels/LoginTabViewModel.cs @@ -3,6 +3,7 @@ using System.Globalization; using System.Reactive; using System.Reactive.Linq; +using System.Reactive.Threading.Tasks; using System.Threading.Tasks; using GitHub.App; using GitHub.Authentication; @@ -23,12 +24,12 @@ public abstract class LoginTabViewModel : ReactiveObject { static readonly Logger log = LogManager.GetCurrentClassLogger(); - protected LoginTabViewModel(IRepositoryHosts repositoryHosts, IVisualStudioBrowser browser) + protected LoginTabViewModel(IConnectionManager connectionManager, IVisualStudioBrowser browser) { - Guard.ArgumentNotNull(repositoryHosts, nameof(repositoryHosts)); + Guard.ArgumentNotNull(connectionManager, nameof(connectionManager)); Guard.ArgumentNotNull(browser, nameof(browser)); - RepositoryHosts = repositoryHosts; + ConnectionManager = connectionManager; UsernameOrEmailValidator = ReactivePropertyValidator.For(this, x => x.UsernameOrEmail) .IfNullOrEmpty(Resources.UsernameOrEmailValidatorEmpty) @@ -76,7 +77,7 @@ protected LoginTabViewModel(IRepositoryHosts repositoryHosts, IVisualStudioBrows return Observable.Return(Unit.Default); }); } - protected IRepositoryHosts RepositoryHosts { get; } + protected IConnectionManager ConnectionManager { get; } protected abstract Uri BaseUri { get; } public IReactiveCommand SignUp { get; } @@ -142,7 +143,9 @@ protected IObservable LogInToHost(HostAddress hostAddress) return Observable.Defer(() => { return hostAddress != null ? - RepositoryHosts.LogIn(hostAddress, UsernameOrEmail, Password) + ConnectionManager.LogIn(hostAddress, UsernameOrEmail, Password) + .ToObservable() + .Select(_ => AuthenticationResult.Success) : Observable.Return(AuthenticationResult.CredentialFailure); }) .ObserveOn(RxApp.MainThreadScheduler) diff --git a/src/GitHub.App/ViewModels/LoginToGitHubForEnterpriseViewModel.cs b/src/GitHub.App/ViewModels/LoginToGitHubForEnterpriseViewModel.cs index a5cf08d10b..28c8e8e53a 100644 --- a/src/GitHub.App/ViewModels/LoginToGitHubForEnterpriseViewModel.cs +++ b/src/GitHub.App/ViewModels/LoginToGitHubForEnterpriseViewModel.cs @@ -20,9 +20,10 @@ namespace GitHub.ViewModels public class LoginToGitHubForEnterpriseViewModel : LoginTabViewModel, ILoginToGitHubForEnterpriseViewModel { [ImportingConstructor] - public LoginToGitHubForEnterpriseViewModel(IRepositoryHosts hosts, IVisualStudioBrowser browser) : base(hosts, browser) + public LoginToGitHubForEnterpriseViewModel(IConnectionManager connectionManager, IVisualStudioBrowser browser) + : base(connectionManager, browser) { - Guard.ArgumentNotNull(hosts, nameof(hosts)); + Guard.ArgumentNotNull(connectionManager, nameof(connectionManager)); Guard.ArgumentNotNull(browser, nameof(browser)); EnterpriseUrlValidator = ReactivePropertyValidator.For(this, x => x.EnterpriseUrl) diff --git a/src/GitHub.App/ViewModels/LoginToGitHubViewModel.cs b/src/GitHub.App/ViewModels/LoginToGitHubViewModel.cs index b5557df5e2..104d941990 100644 --- a/src/GitHub.App/ViewModels/LoginToGitHubViewModel.cs +++ b/src/GitHub.App/ViewModels/LoginToGitHubViewModel.cs @@ -16,8 +16,8 @@ namespace GitHub.ViewModels public class LoginToGitHubViewModel : LoginTabViewModel, ILoginToGitHubViewModel { [ImportingConstructor] - public LoginToGitHubViewModel(IRepositoryHosts repositoryHosts, IVisualStudioBrowser browser) - : base(repositoryHosts, browser) + public LoginToGitHubViewModel(IConnectionManager connectionManager, IVisualStudioBrowser browser) + : base(connectionManager, browser) { BaseUri = HostAddress.GitHubDotComHostAddress.WebUri; diff --git a/src/GitHub.App/ViewModels/PullRequestCreationViewModel.cs b/src/GitHub.App/ViewModels/PullRequestCreationViewModel.cs index 401117af2d..104fc4d638 100644 --- a/src/GitHub.App/ViewModels/PullRequestCreationViewModel.cs +++ b/src/GitHub.App/ViewModels/PullRequestCreationViewModel.cs @@ -18,6 +18,7 @@ using GitHub.Extensions; using System.Reactive.Disposables; using System.Reactive; +using IConnection = GitHub.Models.IConnection; namespace GitHub.ViewModels { @@ -30,31 +31,32 @@ public class PullRequestCreationViewModel : DialogViewModelBase, IPullRequestCre readonly ObservableAsPropertyHelper githubRepository; readonly ObservableAsPropertyHelper isExecuting; - readonly IRepositoryHost repositoryHost; + readonly IConnection connection; + readonly IModelService modelService; readonly IObservable githubObs; readonly CompositeDisposable disposables = new CompositeDisposable(); readonly ILocalRepositoryModel activeLocalRepo; [ImportingConstructor] PullRequestCreationViewModel( - IConnectionRepositoryHostMap connectionRepositoryHostMap, ITeamExplorerServiceHolder teservice, - IPullRequestService service, INotificationService notifications) - : this(connectionRepositoryHostMap?.CurrentRepositoryHost, teservice?.ActiveRepo, service, - notifications) + IConnection connection, + ITeamExplorerServiceHolder teservice, + IPullRequestService service, INotificationService notifications) + : this(connection, teservice?.ActiveRepo, service, notifications) {} - public PullRequestCreationViewModel(IRepositoryHost repositoryHost, ILocalRepositoryModel activeRepo, + public PullRequestCreationViewModel(IConnection connection, ILocalRepositoryModel activeRepo, IPullRequestService service, INotificationService notifications) { - Extensions.Guard.ArgumentNotNull(repositoryHost, nameof(repositoryHost)); + Extensions.Guard.ArgumentNotNull(connection, nameof(connection)); Extensions.Guard.ArgumentNotNull(activeRepo, nameof(activeRepo)); Extensions.Guard.ArgumentNotNull(service, nameof(service)); Extensions.Guard.ArgumentNotNull(notifications, nameof(notifications)); - this.repositoryHost = repositoryHost; + this.connection = connection; activeLocalRepo = activeRepo; - var obs = repositoryHost.ApiClient.GetRepository(activeRepo.Owner, activeRepo.Name) + var obs = modelService.ApiClient.GetRepository(activeRepo.Owner, activeRepo.Name) .Select(r => new RemoteRepositoryModel(r)) .PublishLast(); disposables.Add(obs.Connect()); @@ -97,7 +99,7 @@ public PullRequestCreationViewModel(IRepositoryHost repositoryHost, ILocalReposi CreatePullRequest = ReactiveCommand.CreateAsyncObservable(whenAnyValidationResultChanges, _ => service - .CreatePullRequest(repositoryHost, activeRepo, TargetBranch.Repository, SourceBranch, TargetBranch, PRTitle, Description ?? String.Empty) + .CreatePullRequest(connection, activeRepo, TargetBranch.Repository, SourceBranch, TargetBranch, PRTitle, Description ?? String.Empty) .Catch(ex => { log.Error(ex); @@ -132,12 +134,12 @@ public override void Initialize(ViewWithData data = null) var b = Observable.Empty(); if (r.IsFork) { - b = repositoryHost.ModelService.GetBranches(r.Parent).Select(x => + b = modelService.GetBranches(r.Parent).Select(x => { return x; }); } - return b.Concat(repositoryHost.ModelService.GetBranches(r)); + return b.Concat(modelService.GetBranches(r)); }) .ToList() .ObserveOn(RxApp.MainThreadScheduler) diff --git a/src/GitHub.App/ViewModels/PullRequestDetailViewModel.cs b/src/GitHub.App/ViewModels/PullRequestDetailViewModel.cs index 95378c8597..0276952696 100644 --- a/src/GitHub.App/ViewModels/PullRequestDetailViewModel.cs +++ b/src/GitHub.App/ViewModels/PullRequestDetailViewModel.cs @@ -59,7 +59,7 @@ public class PullRequestDetailViewModel : PanePageViewModelBase, IPullRequestDet /// The usage tracker. [ImportingConstructor] PullRequestDetailViewModel( - IConnectionRepositoryHostMap connectionRepositoryHostMap, + IConnection connection, ITeamExplorerServiceHolder teservice, IPullRequestService pullRequestsService, IPullRequestSessionManager sessionManager, diff --git a/src/GitHub.App/ViewModels/PullRequestListViewModel.cs b/src/GitHub.App/ViewModels/PullRequestListViewModel.cs index 63e6d73189..6f15681196 100644 --- a/src/GitHub.App/ViewModels/PullRequestListViewModel.cs +++ b/src/GitHub.App/ViewModels/PullRequestListViewModel.cs @@ -26,7 +26,7 @@ public class PullRequestListViewModel : PanePageViewModelBase, IPullRequestListV { static readonly Logger log = LogManager.GetCurrentClassLogger(); - readonly IRepositoryHost repositoryHost; + readonly IConnection connection; readonly ILocalRepositoryModel localRepository; readonly TrackingCollection trackingAuthors; readonly TrackingCollection trackingAssignees; @@ -38,30 +38,30 @@ public class PullRequestListViewModel : PanePageViewModelBase, IPullRequestListV [ImportingConstructor] PullRequestListViewModel( - IConnectionRepositoryHostMap connectionRepositoryHostMap, + IConnection connection, ITeamExplorerServiceHolder teservice, IPackageSettings settings, IVisualStudioBrowser visualStudioBrowser) - : this(connectionRepositoryHostMap.CurrentRepositoryHost, teservice.ActiveRepo, settings, visualStudioBrowser) + : this(connection, teservice.ActiveRepo, settings, visualStudioBrowser) { - Guard.ArgumentNotNull(connectionRepositoryHostMap, nameof(connectionRepositoryHostMap)); + Guard.ArgumentNotNull(connection, nameof(connection)); Guard.ArgumentNotNull(teservice, nameof(teservice)); Guard.ArgumentNotNull(settings, nameof(settings)); } public PullRequestListViewModel( - IRepositoryHost repositoryHost, + IConnection connection, ILocalRepositoryModel repository, IPackageSettings settings, IVisualStudioBrowser visualStudioBrowser) { - Guard.ArgumentNotNull(repositoryHost, nameof(repositoryHost)); + Guard.ArgumentNotNull(connection, nameof(connection)); Guard.ArgumentNotNull(repository, nameof(repository)); Guard.ArgumentNotNull(settings, nameof(settings)); Guard.ArgumentNotNull(visualStudioBrowser, nameof(visualStudioBrowser)); constructing = true; - this.repositoryHost = repositoryHost; + this.connection = connection; this.localRepository = repository; this.settings = settings; this.visualStudioBrowser = visualStudioBrowser; diff --git a/src/GitHub.App/ViewModels/RepositoryCloneViewModel.cs b/src/GitHub.App/ViewModels/RepositoryCloneViewModel.cs index 4cf8544b1f..a300210b77 100644 --- a/src/GitHub.App/ViewModels/RepositoryCloneViewModel.cs +++ b/src/GitHub.App/ViewModels/RepositoryCloneViewModel.cs @@ -31,7 +31,7 @@ public class RepositoryCloneViewModel : DialogViewModelBase, IRepositoryCloneVie { static readonly Logger log = LogManager.GetCurrentClassLogger(); - readonly IRepositoryHost repositoryHost; + readonly IConnection connection; readonly IOperatingSystem operatingSystem; readonly ReactiveCommand browseForDirectoryCommand = ReactiveCommand.Create(); bool noRepositoriesFound; @@ -41,26 +41,18 @@ public class RepositoryCloneViewModel : DialogViewModelBase, IRepositoryCloneVie [ImportingConstructor] RepositoryCloneViewModel( - IConnectionRepositoryHostMap connectionRepositoryHostMap, - IRepositoryCloneService repositoryCloneService, - IOperatingSystem operatingSystem) - : this(connectionRepositoryHostMap.CurrentRepositoryHost, repositoryCloneService, operatingSystem) - { } - - - public RepositoryCloneViewModel( - IRepositoryHost repositoryHost, + IConnection connection, IRepositoryCloneService cloneService, IOperatingSystem operatingSystem) { - Guard.ArgumentNotNull(repositoryHost, nameof(repositoryHost)); + Guard.ArgumentNotNull(connection, nameof(connection)); Guard.ArgumentNotNull(cloneService, nameof(cloneService)); Guard.ArgumentNotNull(operatingSystem, nameof(operatingSystem)); - this.repositoryHost = repositoryHost; + this.connection = connection; this.operatingSystem = operatingSystem; - Title = string.Format(CultureInfo.CurrentCulture, Resources.CloneTitle, repositoryHost.Title); + Title = string.Format(CultureInfo.CurrentCulture, Resources.CloneTitle, connection.HostAddress.Title); Repositories = new TrackingCollection(); repositories.ProcessingDelay = TimeSpan.Zero; diff --git a/src/GitHub.App/ViewModels/RepositoryCreationViewModel.cs b/src/GitHub.App/ViewModels/RepositoryCreationViewModel.cs index 35ad4d7f34..535081885f 100644 --- a/src/GitHub.App/ViewModels/RepositoryCreationViewModel.cs +++ b/src/GitHub.App/ViewModels/RepositoryCreationViewModel.cs @@ -21,6 +21,7 @@ using ReactiveUI; using Rothko; using GitHub.Collections; +using IConnection = GitHub.Models.IConnection; namespace GitHub.ViewModels { @@ -32,7 +33,7 @@ public class RepositoryCreationViewModel : RepositoryFormViewModel, IRepositoryC readonly ReactiveCommand browseForDirectoryCommand = ReactiveCommand.Create(); readonly ObservableAsPropertyHelper> accounts; - readonly IRepositoryHost repositoryHost; + readonly IConnection connection; readonly IRepositoryCreationService repositoryCreationService; readonly ObservableAsPropertyHelper isCreating; readonly ObservableAsPropertyHelper canKeepPrivate; @@ -41,30 +42,22 @@ public class RepositoryCreationViewModel : RepositoryFormViewModel, IRepositoryC [ImportingConstructor] RepositoryCreationViewModel( - IConnectionRepositoryHostMap connectionRepositoryHostMap, - IOperatingSystem operatingSystem, - IRepositoryCreationService repositoryCreationService, - IUsageTracker usageTracker) - : this(connectionRepositoryHostMap.CurrentRepositoryHost, operatingSystem, repositoryCreationService, usageTracker) - {} - - public RepositoryCreationViewModel( - IRepositoryHost repositoryHost, + IConnection connection, IOperatingSystem operatingSystem, IRepositoryCreationService repositoryCreationService, IUsageTracker usageTracker) { - Guard.ArgumentNotNull(repositoryHost, nameof(repositoryHost)); + Guard.ArgumentNotNull(connection, nameof(connection)); Guard.ArgumentNotNull(operatingSystem, nameof(operatingSystem)); Guard.ArgumentNotNull(repositoryCreationService, nameof(repositoryCreationService)); Guard.ArgumentNotNull(usageTracker, nameof(usageTracker)); - this.repositoryHost = repositoryHost; + this.connection = connection; this.operatingSystem = operatingSystem; this.repositoryCreationService = repositoryCreationService; this.usageTracker = usageTracker; - Title = string.Format(CultureInfo.CurrentCulture, Resources.CreateTitle, repositoryHost.Title); + Title = string.Format(CultureInfo.CurrentCulture, Resources.CreateTitle, connection.HostAddress.Title); SelectedGitIgnoreTemplate = GitIgnoreItem.None; SelectedLicense = LicenseItem.None; diff --git a/src/GitHub.App/ViewModels/RepositoryPublishViewModel.cs b/src/GitHub.App/ViewModels/RepositoryPublishViewModel.cs index 2c22ac7e30..ae23b3ab97 100644 --- a/src/GitHub.App/ViewModels/RepositoryPublishViewModel.cs +++ b/src/GitHub.App/ViewModels/RepositoryPublishViewModel.cs @@ -25,7 +25,7 @@ public class RepositoryPublishViewModel : RepositoryFormViewModel, IRepositoryPu { static readonly Logger log = LogManager.GetCurrentClassLogger(); - readonly IRepositoryHosts hosts; + readonly IConnectionManager connectionManager; readonly IRepositoryPublishService repositoryPublishService; readonly INotificationService notificationService; readonly ObservableAsPropertyHelper> accounts; @@ -36,26 +36,24 @@ public class RepositoryPublishViewModel : RepositoryFormViewModel, IRepositoryPu [ImportingConstructor] public RepositoryPublishViewModel( - IRepositoryHosts hosts, IRepositoryPublishService repositoryPublishService, INotificationService notificationService, IConnectionManager connectionManager, IUsageTracker usageTracker) { - Guard.ArgumentNotNull(hosts, nameof(hosts)); Guard.ArgumentNotNull(repositoryPublishService, nameof(repositoryPublishService)); Guard.ArgumentNotNull(notificationService, nameof(notificationService)); Guard.ArgumentNotNull(connectionManager, nameof(connectionManager)); Guard.ArgumentNotNull(usageTracker, nameof(usageTracker)); this.notificationService = notificationService; - this.hosts = hosts; + this.connectionManager = connectionManager; this.usageTracker = usageTracker; title = this.WhenAny( - x => x.SelectedHost, + x => x.SelectedConnection, x => x.Value != null ? - string.Format(CultureInfo.CurrentCulture, Resources.PublishToTitle, x.Value.Title) : + string.Format(CultureInfo.CurrentCulture, Resources.PublishToTitle, x.Value.HostAddress.Title) : Resources.PublishTitle ) .ToProperty(this, x => x.Title); @@ -66,8 +64,8 @@ public RepositoryPublishViewModel( if (Connections.Any()) SelectedConnection = Connections.FirstOrDefault(x => x.HostAddress.IsGitHubDotCom()) ?? Connections[0]; - accounts = this.WhenAny(x => x.SelectedConnection, x => x.Value != null ? hosts.LookupHost(x.Value.HostAddress) : RepositoryHosts.DisconnectedRepositoryHost) - .Where(x => !(x is DisconnectedRepositoryHost)) + accounts = this.WhenAnyValue(x => x.SelectedConnection) + .Where(x => x != null) .SelectMany(host => host.ModelService.GetAccounts()) .ObserveOn(RxApp.MainThreadScheduler) .ToProperty(this, x => x.Accounts, initialValue: new ReadOnlyCollection(new IAccount[] {})); @@ -126,11 +124,6 @@ public IConnection SelectedConnection set { this.RaiseAndSetIfChanged(ref selectedConnection, value); } } - IRepositoryHost SelectedHost - { - get { return selectedConnection != null ? hosts.LookupHost(selectedConnection.HostAddress) : null; } - } - public IReadOnlyList Accounts { get { return accounts.Value; } @@ -157,7 +150,7 @@ IObservable OnPublishRepository(object arg) var newRepository = GatherRepositoryInfo(); var account = SelectedAccount; - return repositoryPublishService.PublishRepository(newRepository, account, SelectedHost.ApiClient) + return repositoryPublishService.PublishRepository(newRepository, account, SelectedConnection.ApiClient) .Do(_ => usageTracker.IncrementPublishCount().Forget()) .Select(_ => ProgressState.Success) .Catch(ex => diff --git a/src/GitHub.App/ViewModels/StartPageCloneViewModel.cs b/src/GitHub.App/ViewModels/StartPageCloneViewModel.cs index f4a20e755b..895b882372 100644 --- a/src/GitHub.App/ViewModels/StartPageCloneViewModel.cs +++ b/src/GitHub.App/ViewModels/StartPageCloneViewModel.cs @@ -32,24 +32,17 @@ public class StartPageCloneViewModel : DialogViewModelBase, IBaseCloneViewModel [ImportingConstructor] StartPageCloneViewModel( - IConnectionRepositoryHostMap connectionRepositoryHostMap, - IRepositoryCloneService repositoryCloneService, - IOperatingSystem operatingSystem) - : this(connectionRepositoryHostMap.CurrentRepositoryHost, repositoryCloneService, operatingSystem) - { } - - public StartPageCloneViewModel( - IRepositoryHost repositoryHost, + IConnection connection, IRepositoryCloneService cloneService, IOperatingSystem operatingSystem) { - Guard.ArgumentNotNull(repositoryHost, nameof(repositoryHost)); + Guard.ArgumentNotNull(connection, nameof(connection)); Guard.ArgumentNotNull(cloneService, nameof(cloneService)); Guard.ArgumentNotNull(operatingSystem, nameof(operatingSystem)); this.operatingSystem = operatingSystem; - Title = string.Format(CultureInfo.CurrentCulture, Resources.CloneTitle, repositoryHost.Title); + Title = string.Format(CultureInfo.CurrentCulture, Resources.CloneTitle, connection.HostAddress.Title); var baseRepositoryPath = this.WhenAny( x => x.BaseRepositoryPath, diff --git a/src/GitHub.Exports.Reactive/Extensions/ConnectionManagerExtensions.cs b/src/GitHub.Exports.Reactive/Extensions/ConnectionManagerExtensions.cs index 64e11e3e4e..d74af9e74b 100644 --- a/src/GitHub.Exports.Reactive/Extensions/ConnectionManagerExtensions.cs +++ b/src/GitHub.Exports.Reactive/Extensions/ConnectionManagerExtensions.cs @@ -2,8 +2,6 @@ using System.Linq; using System.Reactive.Linq; using System.Reactive.Threading.Tasks; -using System.Threading.Tasks; -using GitHub.Api; using GitHub.Models; using GitHub.Primitives; using GitHub.Services; @@ -12,9 +10,9 @@ namespace GitHub.Extensions { public static class ConnectionManagerExtensions { - public static IObservable IsLoggedIn(this IConnectionManager cm, IRepositoryHosts hosts) + public static IObservable IsLoggedIn(this IConnectionManager cm) { - Guard.ArgumentNotNull(hosts, nameof(hosts)); + Guard.ArgumentNotNull(cm, nameof(cm)); return Observable.FromAsync(async () => { @@ -23,9 +21,9 @@ public static IObservable IsLoggedIn(this IConnectionManager cm, IReposito }); } - public static IObservable IsLoggedIn(this IConnectionManager cm, IRepositoryHosts hosts, HostAddress address) + public static IObservable IsLoggedIn(this IConnectionManager cm, HostAddress address) { - Guard.ArgumentNotNull(hosts, nameof(hosts)); + Guard.ArgumentNotNull(cm, nameof(cm)); Guard.ArgumentNotNull(address, nameof(address)); return Observable.FromAsync(async () => @@ -35,16 +33,16 @@ public static IObservable IsLoggedIn(this IConnectionManager cm, IReposito }); } - public static IObservable IsLoggedIn(this IConnection connection, IRepositoryHosts hosts) + public static IObservable IsLoggedIn(this IConnection connection) { - Guard.ArgumentNotNull(hosts, nameof(hosts)); + Guard.ArgumentNotNull(connection, nameof(connection)); return Observable.Return(connection?.IsLoggedIn ?? false); } - public static IObservable GetLoggedInConnections(this IConnectionManager cm, IRepositoryHosts hosts) + public static IObservable GetLoggedInConnections(this IConnectionManager cm) { - Guard.ArgumentNotNull(hosts, nameof(hosts)); + Guard.ArgumentNotNull(cm, nameof(cm)); return cm.GetLoadedConnections() .ToObservable() diff --git a/src/GitHub.Exports.Reactive/Factories/IApiClientFactory.cs b/src/GitHub.Exports.Reactive/Factories/IApiClientFactory.cs index 3d5d65df81..5253d3f470 100644 --- a/src/GitHub.Exports.Reactive/Factories/IApiClientFactory.cs +++ b/src/GitHub.Exports.Reactive/Factories/IApiClientFactory.cs @@ -3,6 +3,7 @@ using System; using System.Threading.Tasks; using Octokit; +using GitHub.Services; namespace GitHub.Factories { diff --git a/src/GitHub.Exports.Reactive/Factories/IRepositoryHostFactory.cs b/src/GitHub.Exports.Reactive/Factories/IRepositoryHostFactory.cs deleted file mode 100644 index 10f84644a3..0000000000 --- a/src/GitHub.Exports.Reactive/Factories/IRepositoryHostFactory.cs +++ /dev/null @@ -1,11 +0,0 @@ -using System; -using System.Threading.Tasks; -using GitHub.Models; - -namespace GitHub.Factories -{ - public interface IRepositoryHostFactory - { - Task Create(IConnection connection); - } -} \ No newline at end of file diff --git a/src/GitHub.Exports.Reactive/GitHub.Exports.Reactive.csproj b/src/GitHub.Exports.Reactive/GitHub.Exports.Reactive.csproj index 3091dca509..ffbe81c94e 100644 --- a/src/GitHub.Exports.Reactive/GitHub.Exports.Reactive.csproj +++ b/src/GitHub.Exports.Reactive/GitHub.Exports.Reactive.csproj @@ -102,10 +102,8 @@ - - @@ -154,8 +152,6 @@ - - diff --git a/src/GitHub.Exports.Reactive/Models/IConnectionRepositoryHostMap.cs b/src/GitHub.Exports.Reactive/Models/IConnectionRepositoryHostMap.cs deleted file mode 100644 index e1a7050355..0000000000 --- a/src/GitHub.Exports.Reactive/Models/IConnectionRepositoryHostMap.cs +++ /dev/null @@ -1,13 +0,0 @@ -namespace GitHub.Models -{ - /// - /// Used to associate the current connection to its related repository host. - /// - public interface IConnectionRepositoryHostMap - { - /// - /// The current repository host associated with the current action. - /// - IRepositoryHost CurrentRepositoryHost { get; } - } -} diff --git a/src/GitHub.Exports.Reactive/Models/IRepositoryHost.cs b/src/GitHub.Exports.Reactive/Models/IRepositoryHost.cs deleted file mode 100644 index e7cfac2c56..0000000000 --- a/src/GitHub.Exports.Reactive/Models/IRepositoryHost.cs +++ /dev/null @@ -1,18 +0,0 @@ -using System; -using System.Reactive; -using GitHub.Api; -using GitHub.Authentication; -using GitHub.Primitives; -using GitHub.Services; - -namespace GitHub.Models -{ - public interface IRepositoryHost - { - HostAddress Address { get; } - IApiClient ApiClient { get; } - IModelService ModelService { get; } - bool IsLoggedIn { get; } - string Title { get; } - } -} diff --git a/src/GitHub.Exports.Reactive/Models/IRepositoryHosts.cs b/src/GitHub.Exports.Reactive/Models/IRepositoryHosts.cs deleted file mode 100644 index 9cd6e44168..0000000000 --- a/src/GitHub.Exports.Reactive/Models/IRepositoryHosts.cs +++ /dev/null @@ -1,23 +0,0 @@ -using System; -using System.Threading.Tasks; -using GitHub.Authentication; -using GitHub.Factories; -using GitHub.Primitives; -using ReactiveUI; - -namespace GitHub.Models -{ - public interface IRepositoryHosts : IReactiveObject, IDisposable - { - IRepositoryHost EnterpriseHost { get; set; } - IRepositoryHost GitHubHost { get; } - Task EnsureInitialized(); - IObservable LogIn( - HostAddress hostAddress, - string username, - string password); - IRepositoryHostFactory RepositoryHostFactory { get; } - bool IsLoggedInToAnyHost { get; } - IRepositoryHost LookupHost(HostAddress address); - } -} diff --git a/src/GitHub.Exports.Reactive/Services/IModelService.cs b/src/GitHub.Exports.Reactive/Services/IModelService.cs index aae1608481..791d9f5997 100644 --- a/src/GitHub.Exports.Reactive/Services/IModelService.cs +++ b/src/GitHub.Exports.Reactive/Services/IModelService.cs @@ -4,6 +4,7 @@ using GitHub.Models; using GitHub.Caches; using GitHub.Collections; +using GitHub.Api; namespace GitHub.Services { diff --git a/src/GitHub.Exports.Reactive/Services/IPullRequestService.cs b/src/GitHub.Exports.Reactive/Services/IPullRequestService.cs index 3becf7223f..d6cf781dfc 100644 --- a/src/GitHub.Exports.Reactive/Services/IPullRequestService.cs +++ b/src/GitHub.Exports.Reactive/Services/IPullRequestService.cs @@ -4,12 +4,13 @@ using GitHub.Models; using LibGit2Sharp; using Octokit; +using IConnection = GitHub.Models.IConnection; namespace GitHub.Services { public interface IPullRequestService { - IObservable CreatePullRequest(IRepositoryHost host, + IObservable CreatePullRequest(IConnection connection, ILocalRepositoryModel sourceRepository, IRepositoryModel targetRepository, IBranch sourceBranch, IBranch targetBranch, string title, string body); From 324037273c9c3fb6722dfb1afe9e0c854ce9e3bf Mon Sep 17 00:00:00 2001 From: Steven Kirk Date: Thu, 26 Oct 2017 23:18:16 +0200 Subject: [PATCH 09/16] WIP: Still removing RepositoryHost(s) --- src/GitHub.App/GitHub.App.csproj | 1 - src/GitHub.App/Services/ModelService.cs | 37 ++-- src/GitHub.App/Services/PullRequestService.cs | 10 +- .../ViewModels/GistCreationViewModel.cs | 11 +- .../ViewModels/LoginControlViewModel.cs | 26 +-- .../PullRequestCreationViewModel.cs | 48 ++-- .../ViewModels/PullRequestDetailViewModel.cs | 4 +- .../ViewModels/PullRequestListViewModel.cs | 17 +- .../ViewModels/RepositoryCloneViewModel.cs | 13 +- .../ViewModels/RepositoryCreationViewModel.cs | 14 +- .../ViewModels/RepositoryPublishViewModel.cs | 11 +- .../Extensions/ConnectionManagerExtensions.cs | 11 + .../Factories/IModelServiceFactory.cs | 13 ++ .../GitHub.Exports.Reactive.csproj | 1 + .../Services/IModelService.cs | 2 + .../Services/IPullRequestService.cs | 2 +- .../Services/PullRequestSessionManager.cs | 27 ++- .../Base/EnsureLoggedInSection.cs | 13 +- .../Sync/EnsureLoggedInSectionSync.cs | 6 +- .../Sync/GitHubPublishSection.cs | 11 +- .../UI/Views/GitHubPaneViewModel.cs | 7 +- .../GitHub.App/Models/RepositoryHostTests.cs | 19 -- .../GitHub.App/Models/RepositoryHostsTests.cs | 206 ------------------ .../ConnectionManagerExtensionsTests.cs | 13 ++ src/UnitTests/UnitTests.csproj | 1 + .../PullRequestSessionManagerTests.cs | 119 ++++++---- 26 files changed, 270 insertions(+), 373 deletions(-) create mode 100644 src/GitHub.Exports.Reactive/Factories/IModelServiceFactory.cs delete mode 100644 src/UnitTests/GitHub.App/Models/RepositoryHostTests.cs delete mode 100644 src/UnitTests/GitHub.App/Models/RepositoryHostsTests.cs create mode 100644 src/UnitTests/GitHub.Exports.Reactive/Extensions/ConnectionManagerExtensionsTests.cs diff --git a/src/GitHub.App/GitHub.App.csproj b/src/GitHub.App/GitHub.App.csproj index b9e0011224..df19e1577a 100644 --- a/src/GitHub.App/GitHub.App.csproj +++ b/src/GitHub.App/GitHub.App.csproj @@ -193,7 +193,6 @@ - diff --git a/src/GitHub.App/Services/ModelService.cs b/src/GitHub.App/Services/ModelService.cs index 7218dba685..990fad442e 100644 --- a/src/GitHub.App/Services/ModelService.cs +++ b/src/GitHub.App/Services/ModelService.cs @@ -29,7 +29,6 @@ public class ModelService : IModelService const string CachedFilesDirectory = "CachedFiles"; static readonly Logger log = LogManager.GetCurrentClassLogger(); - readonly IApiClient apiClient; readonly IBlobCache hostCache; readonly IAvatarProvider avatarProvider; @@ -38,11 +37,13 @@ public ModelService( IBlobCache hostCache, IAvatarProvider avatarProvider) { - this.apiClient = apiClient; + this.ApiClient = apiClient; this.hostCache = hostCache; this.avatarProvider = avatarProvider; } + public IApiClient ApiClient { get; } + public IObservable GetCurrentUser() { return GetUserFromCache().Select(Create); @@ -93,14 +94,14 @@ public IObservable> GetAccounts() IObservable GetLicensesFromApi() { - return apiClient.GetLicenses() + return ApiClient.GetLicenses() .WhereNotNull() .Select(LicenseCacheItem.Create); } IObservable GetGitIgnoreTemplatesFromApi() { - return apiClient.GetGitIgnoreTemplates() + return ApiClient.GetGitIgnoreTemplates() .WhereNotNull() .Select(GitIgnoreCacheItem.Create); } @@ -108,7 +109,7 @@ IObservable GetGitIgnoreTemplatesFromApi() IObservable> GetUser() { return hostCache.GetAndRefreshObject("user", - () => apiClient.GetUser().Select(AccountCacheItem.Create), TimeSpan.FromMinutes(5), TimeSpan.FromDays(7)) + () => ApiClient.GetUser().Select(AccountCacheItem.Create), TimeSpan.FromMinutes(5), TimeSpan.FromDays(7)) .TakeLast(1) .ToList(); } @@ -117,7 +118,7 @@ IObservable> GetUserOrganizations() { return GetUserFromCache().SelectMany(user => hostCache.GetAndRefreshObject(user.Login + "|orgs", - () => apiClient.GetOrganizations().Select(AccountCacheItem.Create).ToList(), + () => ApiClient.GetOrganizations().Select(AccountCacheItem.Create).ToList(), TimeSpan.FromMinutes(2), TimeSpan.FromDays(7))) // TODO: Akavache returns the cached version followed by the fresh version if > 2 // minutes have expired from the last request. Here we make sure the latest value is @@ -171,7 +172,7 @@ public ITrackingCollection GetPullRequests(IRepositoryModel r var source = Observable.Defer(() => keyobs .SelectMany(key => hostCache.GetAndFetchLatestFromIndex(key, () => - apiClient.GetPullRequestsForRepository(repo.CloneUrl.Owner, repo.CloneUrl.RepositoryName) + ApiClient.GetPullRequestsForRepository(repo.CloneUrl.Owner, repo.CloneUrl.RepositoryName) .Select(PullRequestCacheItem.Create), item => { @@ -197,10 +198,10 @@ public IObservable GetPullRequest(string owner, string name, { return hostCache.GetAndRefreshObject(PRPrefix + '|' + number, () => Observable.CombineLatest( - apiClient.GetPullRequest(owner, name, number), - apiClient.GetPullRequestFiles(owner, name, number).ToList(), - apiClient.GetIssueComments(owner, name, number).ToList(), - apiClient.GetPullRequestReviewComments(owner, name, number).ToList(), + ApiClient.GetPullRequest(owner, name, number), + ApiClient.GetPullRequestFiles(owner, name, number).ToList(), + ApiClient.GetIssueComments(owner, name, number).ToList(), + ApiClient.GetPullRequestReviewComments(owner, name, number).ToList(), (pr, files, comments, reviewComments) => new { PullRequest = pr, @@ -228,7 +229,7 @@ public IObservable GetRepository(string owner, string re .SelectMany(key => hostCache.GetAndFetchLatest( key, - () => apiClient.GetRepository(owner, repo).Select(RepositoryCacheItem.Create)) + () => ApiClient.GetRepository(owner, repo).Select(RepositoryCacheItem.Create)) .Select(Create))); } @@ -240,7 +241,7 @@ public ITrackingCollection GetRepositories(ITrackingColl var source = Observable.Defer(() => keyobs .SelectMany(key => hostCache.GetAndFetchLatestFromIndex(key, () => - apiClient.GetRepositories() + ApiClient.GetRepositories() .Select(RepositoryCacheItem.Create), item => { @@ -270,7 +271,7 @@ public IObservable CreatePullRequest(ILocalRepositoryModel so return Observable.Defer(() => keyobs .SelectMany(key => hostCache.PutAndUpdateIndex(key, () => - apiClient.CreatePullRequest( + ApiClient.CreatePullRequest( new NewPullRequest(title, string.Format(CultureInfo.InvariantCulture, "{0}:{1}", sourceRepository.Owner, sourceBranch.Name), targetBranch.Name) @@ -300,7 +301,7 @@ public IObservable GetFileContents(IRepositoryModel repo, string commitS if (!File.Exists(tempFile)) { - var contents = await apiClient.GetFileContents(repo.Owner, repo.Name, commitSha, path); + var contents = await ApiClient.GetFileContents(repo.Owner, repo.Name, commitSha, path); Directory.CreateDirectory(tempDir); File.WriteAllBytes(tempFile, Convert.FromBase64String(contents.EncodedContent)); } @@ -331,7 +332,7 @@ IObservable> GetUserRepositories(Repositor IObservable> GetUserRepositoriesFromApi(RepositoryType repositoryType) { - return apiClient.GetUserRepositories(repositoryType) + return ApiClient.GetUserRepositories(repositoryType) .WhereNotNull() .Select(RepositoryCacheItem.Create) .ToList() @@ -349,7 +350,7 @@ IObservable> GetOrganizationRepositories(s { return Observable.Defer(() => GetUserFromCache().SelectMany(user => hostCache.GetAndRefreshObject(string.Format(CultureInfo.InvariantCulture, "{0}|{1}|repos", user.Login, organization), - () => apiClient.GetRepositoriesForOrganization(organization).Select( + () => ApiClient.GetRepositoriesForOrganization(organization).Select( RepositoryCacheItem.Create).ToList(), TimeSpan.FromMinutes(2), TimeSpan.FromDays(7))) @@ -373,7 +374,7 @@ public IObservable GetBranches(IRepositoryModel repo) .Select(user => string.Format(CultureInfo.InvariantCulture, "{0}|{1}|branch", user.Login, repo.Name)); return Observable.Defer(() => keyobs - .SelectMany(key => apiClient.GetBranches(repo.CloneUrl.Owner, repo.CloneUrl.RepositoryName))) + .SelectMany(key => ApiClient.GetBranches(repo.CloneUrl.Owner, repo.CloneUrl.RepositoryName))) .Select(x => new BranchModel(x, repo)); } diff --git a/src/GitHub.App/Services/PullRequestService.cs b/src/GitHub.App/Services/PullRequestService.cs index 2116bc2196..deee073200 100644 --- a/src/GitHub.App/Services/PullRequestService.cs +++ b/src/GitHub.App/Services/PullRequestService.cs @@ -50,13 +50,13 @@ public PullRequestService(IGitClient gitClient, IGitService gitService, IOperati this.usageTracker = usageTracker; } - public IObservable CreatePullRequest(IConnection connection, + public IObservable CreatePullRequest(IModelService modelService, ILocalRepositoryModel sourceRepository, IRepositoryModel targetRepository, IBranch sourceBranch, IBranch targetBranch, string title, string body ) { - Extensions.Guard.ArgumentNotNull(connection, nameof(connection)); + Extensions.Guard.ArgumentNotNull(modelService, nameof(modelService)); Extensions.Guard.ArgumentNotNull(sourceRepository, nameof(sourceRepository)); Extensions.Guard.ArgumentNotNull(targetRepository, nameof(targetRepository)); Extensions.Guard.ArgumentNotNull(sourceBranch, nameof(sourceBranch)); @@ -64,7 +64,7 @@ public IObservable CreatePullRequest(IConnection connection, Extensions.Guard.ArgumentNotNull(title, nameof(title)); Extensions.Guard.ArgumentNotNull(body, nameof(body)); - return PushAndCreatePR(connection, sourceRepository, targetRepository, sourceBranch, targetBranch, title, body).ToObservable(); + return PushAndCreatePR(modelService, sourceRepository, targetRepository, sourceBranch, targetBranch, title, body).ToObservable(); } public IObservable GetPullRequestTemplate(ILocalRepositoryModel repository) @@ -487,7 +487,7 @@ async Task MarkBranchAsPullRequest(IRepository repo, string branchName, IPullReq await gitClient.SetConfig(repo, prConfigKey, BuildGHfVSConfigKeyValue(pullRequest)); } - async Task PushAndCreatePR(IConnection connection, + async Task PushAndCreatePR(IModelService modelService, ILocalRepositoryModel sourceRepository, IRepositoryModel targetRepository, IBranch sourceBranch, IBranch targetBranch, string title, string body) @@ -503,7 +503,7 @@ async Task PushAndCreatePR(IConnection connection, if (!Splat.ModeDetector.Current.InUnitTestRunner().GetValueOrDefault()) await Task.Delay(TimeSpan.FromSeconds(5)); - var ret = await connection.ModelService.CreatePullRequest(sourceRepository, targetRepository, sourceBranch, targetBranch, title, body); + var ret = await modelService.CreatePullRequest(sourceRepository, targetRepository, sourceBranch, targetBranch, title, body); await usageTracker.IncrementUpstreamPullRequestCount(); return ret; } diff --git a/src/GitHub.App/ViewModels/GistCreationViewModel.cs b/src/GitHub.App/ViewModels/GistCreationViewModel.cs index b4bcb6584e..e83494de07 100644 --- a/src/GitHub.App/ViewModels/GistCreationViewModel.cs +++ b/src/GitHub.App/ViewModels/GistCreationViewModel.cs @@ -8,6 +8,7 @@ using GitHub.Exports; using GitHub.Extensions; using GitHub.Extensions.Reactive; +using GitHub.Factories; using GitHub.Models; using GitHub.Services; using NLog; @@ -32,11 +33,12 @@ public class GistCreationViewModel : DialogViewModelBase, IGistCreationViewModel [ImportingConstructor] GistCreationViewModel( IConnection connection, + IModelServiceFactory modelServiceFactory, ISelectedTextProvider selectedTextProvider, IGistPublishService gistPublishService, INotificationService notificationService, IUsageTracker usageTracker) - : this(connection, selectedTextProvider, gistPublishService, usageTracker) + : this(connection, modelServiceFactory, selectedTextProvider, gistPublishService, usageTracker) { Guard.ArgumentNotNull(connection, nameof(connection)); Guard.ArgumentNotNull(selectedTextProvider, nameof(selectedTextProvider)); @@ -49,6 +51,7 @@ public class GistCreationViewModel : DialogViewModelBase, IGistCreationViewModel public GistCreationViewModel( IConnection connection, + IModelServiceFactory modelServiceFactory, ISelectedTextProvider selectedTextProvider, IGistPublishService gistPublishService, IUsageTracker usageTracker) @@ -59,15 +62,17 @@ public GistCreationViewModel( Guard.ArgumentNotNull(usageTracker, nameof(usageTracker)); Title = Resources.CreateGistTitle; - apiClient = repositoryHost.ApiClient; this.gistPublishService = gistPublishService; this.usageTracker = usageTracker; FileName = VisualStudio.Services.GetFileNameFromActiveDocument() ?? Resources.DefaultGistFileName; SelectedText = selectedTextProvider.GetSelectedText(); + var modelService = modelServiceFactory.CreateBlocking(connection); + apiClient = modelService.ApiClient; + // This class is only instantiated after we are logged into to a github account, so we should be safe to grab the first one here as the defaut. - account = repositoryHost.ModelService.GetAccounts() + account = modelService.GetAccounts() .FirstAsync() .Select(a => a.First()) .ObserveOn(RxApp.MainThreadScheduler) diff --git a/src/GitHub.App/ViewModels/LoginControlViewModel.cs b/src/GitHub.App/ViewModels/LoginControlViewModel.cs index 416e4e418d..03092c9674 100644 --- a/src/GitHub.App/ViewModels/LoginControlViewModel.cs +++ b/src/GitHub.App/ViewModels/LoginControlViewModel.cs @@ -33,20 +33,20 @@ public LoginControlViewModel( (x, y) => x.Value || y.Value ).ToProperty(this, vm => vm.IsLoginInProgress); - loginMode = this.WhenAny( - x => x.RepositoryHosts.GitHubHost.IsLoggedIn, - x => x.RepositoryHosts.EnterpriseHost.IsLoggedIn, - (x, y) => - { - var canLogInToGitHub = x.Value == false; - var canLogInToEnterprise = y.Value == false; + ////loginMode = this.WhenAny( + //// x => x.RepositoryHosts.GitHubHost.IsLoggedIn, + //// x => x.RepositoryHosts.EnterpriseHost.IsLoggedIn, + //// (x, y) => + //// { + //// var canLogInToGitHub = x.Value == false; + //// var canLogInToEnterprise = y.Value == false; - return canLogInToGitHub && canLogInToEnterprise ? LoginMode.DotComOrEnterprise - : canLogInToGitHub ? LoginMode.DotComOnly - : canLogInToEnterprise ? LoginMode.EnterpriseOnly - : LoginMode.None; + //// return canLogInToGitHub && canLogInToEnterprise ? LoginMode.DotComOrEnterprise + //// : canLogInToGitHub ? LoginMode.DotComOnly + //// : canLogInToEnterprise ? LoginMode.EnterpriseOnly + //// : LoginMode.None; - }).ToProperty(this, x => x.LoginMode); + //// }).ToProperty(this, x => x.LoginMode); AuthenticationResults = Observable.Merge( loginToGitHubViewModel.Login, @@ -74,7 +74,7 @@ public IConnectionManager ConnectionManager set { this.RaiseAndSetIfChanged(ref connectionManager, value); } } - readonly ObservableAsPropertyHelper loginMode; + readonly ObservableAsPropertyHelper loginMode = null; public LoginMode LoginMode { get { return loginMode.Value; } } readonly ObservableAsPropertyHelper isLoginInProgress; diff --git a/src/GitHub.App/ViewModels/PullRequestCreationViewModel.cs b/src/GitHub.App/ViewModels/PullRequestCreationViewModel.cs index 104fc4d638..4cd4422ade 100644 --- a/src/GitHub.App/ViewModels/PullRequestCreationViewModel.cs +++ b/src/GitHub.App/ViewModels/PullRequestCreationViewModel.cs @@ -1,23 +1,24 @@ using System; +using System.Collections.Generic; using System.ComponentModel.Composition; +using System.Diagnostics.CodeAnalysis; +using System.Globalization; +using System.Linq; +using System.Reactive; +using System.Reactive.Disposables; +using System.Reactive.Linq; +using GitHub.App; using GitHub.Exports; +using GitHub.Extensions; +using GitHub.Extensions.Reactive; +using GitHub.Factories; using GitHub.Models; -using System.Collections.Generic; -using ReactiveUI; using GitHub.Services; -using System.Reactive.Linq; -using GitHub.Extensions.Reactive; using GitHub.UI; -using System.Linq; using GitHub.Validation; -using GitHub.App; -using System.Diagnostics.CodeAnalysis; -using Octokit; using NLog; -using System.Globalization; -using GitHub.Extensions; -using System.Reactive.Disposables; -using System.Reactive; +using Octokit; +using ReactiveUI; using IConnection = GitHub.Models.IConnection; namespace GitHub.ViewModels @@ -40,21 +41,28 @@ public class PullRequestCreationViewModel : DialogViewModelBase, IPullRequestCre [ImportingConstructor] PullRequestCreationViewModel( IConnection connection, + IModelServiceFactory modelServiceFactory, ITeamExplorerServiceHolder teservice, IPullRequestService service, INotificationService notifications) - : this(connection, teservice?.ActiveRepo, service, notifications) + : this(connection, modelServiceFactory, teservice?.ActiveRepo, service, notifications) {} - public PullRequestCreationViewModel(IConnection connection, ILocalRepositoryModel activeRepo, - IPullRequestService service, INotificationService notifications) + public PullRequestCreationViewModel( + IConnection connection, + IModelServiceFactory modelServiceFactory, + ILocalRepositoryModel activeRepo, + IPullRequestService service, + INotificationService notifications) { - Extensions.Guard.ArgumentNotNull(connection, nameof(connection)); - Extensions.Guard.ArgumentNotNull(activeRepo, nameof(activeRepo)); - Extensions.Guard.ArgumentNotNull(service, nameof(service)); - Extensions.Guard.ArgumentNotNull(notifications, nameof(notifications)); + Guard.ArgumentNotNull(connection, nameof(connection)); + Guard.ArgumentNotNull(modelServiceFactory, nameof(modelServiceFactory)); + Guard.ArgumentNotNull(activeRepo, nameof(activeRepo)); + Guard.ArgumentNotNull(service, nameof(service)); + Guard.ArgumentNotNull(notifications, nameof(notifications)); this.connection = connection; activeLocalRepo = activeRepo; + modelService = modelServiceFactory.CreateBlocking(connection); var obs = modelService.ApiClient.GetRepository(activeRepo.Owner, activeRepo.Name) .Select(r => new RemoteRepositoryModel(r)) @@ -99,7 +107,7 @@ public PullRequestCreationViewModel(IConnection connection, ILocalRepositoryMode CreatePullRequest = ReactiveCommand.CreateAsyncObservable(whenAnyValidationResultChanges, _ => service - .CreatePullRequest(connection, activeRepo, TargetBranch.Repository, SourceBranch, TargetBranch, PRTitle, Description ?? String.Empty) + .CreatePullRequest(modelService, activeRepo, TargetBranch.Repository, SourceBranch, TargetBranch, PRTitle, Description ?? String.Empty) .Catch(ex => { log.Error(ex); diff --git a/src/GitHub.App/ViewModels/PullRequestDetailViewModel.cs b/src/GitHub.App/ViewModels/PullRequestDetailViewModel.cs index 0276952696..9bdbda8315 100644 --- a/src/GitHub.App/ViewModels/PullRequestDetailViewModel.cs +++ b/src/GitHub.App/ViewModels/PullRequestDetailViewModel.cs @@ -17,6 +17,7 @@ using LibGit2Sharp; using ReactiveUI; using NLog; +using GitHub.Factories; namespace GitHub.ViewModels { @@ -60,12 +61,13 @@ public class PullRequestDetailViewModel : PanePageViewModelBase, IPullRequestDet [ImportingConstructor] PullRequestDetailViewModel( IConnection connection, + IModelServiceFactory modelServiceFactory, ITeamExplorerServiceHolder teservice, IPullRequestService pullRequestsService, IPullRequestSessionManager sessionManager, IUsageTracker usageTracker) : this(teservice.ActiveRepo, - connectionRepositoryHostMap.CurrentRepositoryHost.ModelService, + modelServiceFactory.CreateBlocking(connection), pullRequestsService, sessionManager, usageTracker) diff --git a/src/GitHub.App/ViewModels/PullRequestListViewModel.cs b/src/GitHub.App/ViewModels/PullRequestListViewModel.cs index 6f15681196..74f1a5147e 100644 --- a/src/GitHub.App/ViewModels/PullRequestListViewModel.cs +++ b/src/GitHub.App/ViewModels/PullRequestListViewModel.cs @@ -11,6 +11,7 @@ using GitHub.Collections; using GitHub.Exports; using GitHub.Extensions; +using GitHub.Factories; using GitHub.Models; using GitHub.Services; using GitHub.Settings; @@ -27,6 +28,7 @@ public class PullRequestListViewModel : PanePageViewModelBase, IPullRequestListV static readonly Logger log = LogManager.GetCurrentClassLogger(); readonly IConnection connection; + readonly IModelServiceFactory modelServiceFactory; readonly ILocalRepositoryModel localRepository; readonly TrackingCollection trackingAuthors; readonly TrackingCollection trackingAssignees; @@ -35,14 +37,16 @@ public class PullRequestListViewModel : PanePageViewModelBase, IPullRequestListV readonly PullRequestListUIState listSettings; readonly bool constructing; IRemoteRepositoryModel remoteRepository; + IModelService modelService; [ImportingConstructor] PullRequestListViewModel( IConnection connection, + IModelServiceFactory modelServiceFactory, ITeamExplorerServiceHolder teservice, IPackageSettings settings, IVisualStudioBrowser visualStudioBrowser) - : this(connection, teservice.ActiveRepo, settings, visualStudioBrowser) + : this(connection, modelServiceFactory, teservice.ActiveRepo, settings, visualStudioBrowser) { Guard.ArgumentNotNull(connection, nameof(connection)); Guard.ArgumentNotNull(teservice, nameof(teservice)); @@ -51,6 +55,7 @@ public class PullRequestListViewModel : PanePageViewModelBase, IPullRequestListV public PullRequestListViewModel( IConnection connection, + IModelServiceFactory modelServiceFactory, ILocalRepositoryModel repository, IPackageSettings settings, IVisualStudioBrowser visualStudioBrowser) @@ -62,6 +67,7 @@ public PullRequestListViewModel( constructing = true; this.connection = connection; + this.modelServiceFactory = modelServiceFactory; this.localRepository = repository; this.settings = settings; this.visualStudioBrowser = visualStudioBrowser; @@ -128,9 +134,14 @@ async Task Load() { IsBusy = true; + if (modelService == null) + { + modelService = await modelServiceFactory.CreateAsync(connection); + } + if (remoteRepository == null) { - remoteRepository = await repositoryHost.ModelService.GetRepository( + remoteRepository = await modelService.GetRepository( localRepository.Owner, localRepository.Name); Repositories = remoteRepository.IsFork ? @@ -139,7 +150,7 @@ async Task Load() SelectedRepository = Repositories[0]; } - PullRequests = repositoryHost.ModelService.GetPullRequests(SelectedRepository, pullRequests); + PullRequests = modelService.GetPullRequests(SelectedRepository, pullRequests); pullRequests.Subscribe(pr => { trackingAssignees.AddItem(pr.Assignee); diff --git a/src/GitHub.App/ViewModels/RepositoryCloneViewModel.cs b/src/GitHub.App/ViewModels/RepositoryCloneViewModel.cs index a300210b77..0f68d8d431 100644 --- a/src/GitHub.App/ViewModels/RepositoryCloneViewModel.cs +++ b/src/GitHub.App/ViewModels/RepositoryCloneViewModel.cs @@ -22,6 +22,7 @@ using GitHub.Collections; using GitHub.UI; using GitHub.Extensions.Reactive; +using GitHub.Factories; namespace GitHub.ViewModels { @@ -32,8 +33,10 @@ public class RepositoryCloneViewModel : DialogViewModelBase, IRepositoryCloneVie static readonly Logger log = LogManager.GetCurrentClassLogger(); readonly IConnection connection; + readonly IModelServiceFactory modelServiceFactory; readonly IOperatingSystem operatingSystem; readonly ReactiveCommand browseForDirectoryCommand = ReactiveCommand.Create(); + IModelService modelService; bool noRepositoriesFound; readonly ObservableAsPropertyHelper canClone; string baseRepositoryPath; @@ -42,14 +45,17 @@ public class RepositoryCloneViewModel : DialogViewModelBase, IRepositoryCloneVie [ImportingConstructor] RepositoryCloneViewModel( IConnection connection, + IModelServiceFactory modelServiceFactory, IRepositoryCloneService cloneService, IOperatingSystem operatingSystem) { Guard.ArgumentNotNull(connection, nameof(connection)); + Guard.ArgumentNotNull(modelServiceFactory, nameof(modelServiceFactory)); Guard.ArgumentNotNull(cloneService, nameof(cloneService)); Guard.ArgumentNotNull(operatingSystem, nameof(operatingSystem)); this.connection = connection; + this.modelServiceFactory = modelServiceFactory; this.operatingSystem = operatingSystem; Title = string.Format(CultureInfo.CurrentCulture, Resources.CloneTitle, connection.HostAddress.Title); @@ -118,8 +124,13 @@ public override void Initialize(ViewWithData data) { base.Initialize(data); + if (modelService == null) + { + modelService = modelServiceFactory.CreateBlocking(connection); + } + IsBusy = true; - repositoryHost.ModelService.GetRepositories(repositories); + modelService.GetRepositories(repositories); repositories.OriginalCompleted .ObserveOn(RxApp.MainThreadScheduler) .Subscribe( diff --git a/src/GitHub.App/ViewModels/RepositoryCreationViewModel.cs b/src/GitHub.App/ViewModels/RepositoryCreationViewModel.cs index 535081885f..5dd731443b 100644 --- a/src/GitHub.App/ViewModels/RepositoryCreationViewModel.cs +++ b/src/GitHub.App/ViewModels/RepositoryCreationViewModel.cs @@ -22,6 +22,7 @@ using Rothko; using GitHub.Collections; using IConnection = GitHub.Models.IConnection; +using GitHub.Factories; namespace GitHub.ViewModels { @@ -34,6 +35,7 @@ public class RepositoryCreationViewModel : RepositoryFormViewModel, IRepositoryC readonly ReactiveCommand browseForDirectoryCommand = ReactiveCommand.Create(); readonly ObservableAsPropertyHelper> accounts; readonly IConnection connection; + readonly IModelService modelService; readonly IRepositoryCreationService repositoryCreationService; readonly ObservableAsPropertyHelper isCreating; readonly ObservableAsPropertyHelper canKeepPrivate; @@ -43,11 +45,13 @@ public class RepositoryCreationViewModel : RepositoryFormViewModel, IRepositoryC [ImportingConstructor] RepositoryCreationViewModel( IConnection connection, + IModelServiceFactory modelServiceFactory, IOperatingSystem operatingSystem, IRepositoryCreationService repositoryCreationService, IUsageTracker usageTracker) { Guard.ArgumentNotNull(connection, nameof(connection)); + Guard.ArgumentNotNull(modelServiceFactory, nameof(modelServiceFactory)); Guard.ArgumentNotNull(operatingSystem, nameof(operatingSystem)); Guard.ArgumentNotNull(repositoryCreationService, nameof(repositoryCreationService)); Guard.ArgumentNotNull(usageTracker, nameof(usageTracker)); @@ -61,7 +65,9 @@ public class RepositoryCreationViewModel : RepositoryFormViewModel, IRepositoryC SelectedGitIgnoreTemplate = GitIgnoreItem.None; SelectedLicense = LicenseItem.None; - accounts = repositoryHost.ModelService.GetAccounts() + modelService = modelServiceFactory.CreateBlocking(connection); + + accounts = modelService.GetAccounts() .ObserveOn(RxApp.MainThreadScheduler) .ToProperty(this, vm => vm.Accounts, initialValue: new ReadOnlyCollection(new IAccount[] {})); @@ -109,7 +115,7 @@ public class RepositoryCreationViewModel : RepositoryFormViewModel, IRepositoryC .ToProperty(this, x => x.IsCreating); GitIgnoreTemplates = TrackingCollection.CreateListenerCollectionAndRun( - repositoryHost.ModelService.GetGitIgnoreTemplates(), + modelService.GetGitIgnoreTemplates(), new[] { GitIgnoreItem.None }, OrderedComparer.OrderByDescending(item => GitIgnoreItem.IsRecommended(item.Name)).Compare, x => @@ -119,7 +125,7 @@ public class RepositoryCreationViewModel : RepositoryFormViewModel, IRepositoryC }); Licenses = TrackingCollection.CreateListenerCollectionAndRun( - repositoryHost.ModelService.GetLicenses(), + modelService.GetLicenses(), new[] { LicenseItem.None }, OrderedComparer.OrderByDescending(item => LicenseItem.IsRecommended(item.Name)).Compare); @@ -265,7 +271,7 @@ IObservable OnCreateRepository(object state) newRepository, SelectedAccount, BaseRepositoryPath, - repositoryHost.ApiClient) + modelService.ApiClient) .Do(_ => usageTracker.IncrementCreateCount().Forget()); } diff --git a/src/GitHub.App/ViewModels/RepositoryPublishViewModel.cs b/src/GitHub.App/ViewModels/RepositoryPublishViewModel.cs index ae23b3ab97..f48df1ecaa 100644 --- a/src/GitHub.App/ViewModels/RepositoryPublishViewModel.cs +++ b/src/GitHub.App/ViewModels/RepositoryPublishViewModel.cs @@ -10,6 +10,7 @@ using GitHub.Exports; using GitHub.Extensions; using GitHub.Extensions.Reactive; +using GitHub.Factories; using GitHub.Models; using GitHub.Services; using GitHub.UserErrors; @@ -28,6 +29,7 @@ public class RepositoryPublishViewModel : RepositoryFormViewModel, IRepositoryPu readonly IConnectionManager connectionManager; readonly IRepositoryPublishService repositoryPublishService; readonly INotificationService notificationService; + readonly IModelServiceFactory modelServiceFactory; readonly ObservableAsPropertyHelper> accounts; readonly ObservableAsPropertyHelper isHostComboBoxVisible; readonly ObservableAsPropertyHelper canKeepPrivate; @@ -39,16 +41,19 @@ public RepositoryPublishViewModel( IRepositoryPublishService repositoryPublishService, INotificationService notificationService, IConnectionManager connectionManager, + IModelServiceFactory modelServiceFactory, IUsageTracker usageTracker) { Guard.ArgumentNotNull(repositoryPublishService, nameof(repositoryPublishService)); Guard.ArgumentNotNull(notificationService, nameof(notificationService)); Guard.ArgumentNotNull(connectionManager, nameof(connectionManager)); Guard.ArgumentNotNull(usageTracker, nameof(usageTracker)); + Guard.ArgumentNotNull(modelServiceFactory, nameof(modelServiceFactory)); this.notificationService = notificationService; this.connectionManager = connectionManager; this.usageTracker = usageTracker; + this.modelServiceFactory = modelServiceFactory; title = this.WhenAny( x => x.SelectedConnection, @@ -66,7 +71,8 @@ public RepositoryPublishViewModel( accounts = this.WhenAnyValue(x => x.SelectedConnection) .Where(x => x != null) - .SelectMany(host => host.ModelService.GetAccounts()) + .SelectMany(async c => (await modelServiceFactory.CreateAsync(c)).GetAccounts()) + .Switch() .ObserveOn(RxApp.MainThreadScheduler) .ToProperty(this, x => x.Accounts, initialValue: new ReadOnlyCollection(new IAccount[] {})); @@ -149,8 +155,9 @@ IObservable OnPublishRepository(object arg) { var newRepository = GatherRepositoryInfo(); var account = SelectedAccount; + var modelService = modelServiceFactory.CreateBlocking(SelectedConnection); - return repositoryPublishService.PublishRepository(newRepository, account, SelectedConnection.ApiClient) + return repositoryPublishService.PublishRepository(newRepository, account, modelService.ApiClient) .Do(_ => usageTracker.IncrementPublishCount().Forget()) .Select(_ => ProgressState.Success) .Catch(ex => diff --git a/src/GitHub.Exports.Reactive/Extensions/ConnectionManagerExtensions.cs b/src/GitHub.Exports.Reactive/Extensions/ConnectionManagerExtensions.cs index d74af9e74b..ef13bedfff 100644 --- a/src/GitHub.Exports.Reactive/Extensions/ConnectionManagerExtensions.cs +++ b/src/GitHub.Exports.Reactive/Extensions/ConnectionManagerExtensions.cs @@ -2,6 +2,8 @@ using System.Linq; using System.Reactive.Linq; using System.Reactive.Threading.Tasks; +using System.Threading.Tasks; +using GitHub.Factories; using GitHub.Models; using GitHub.Primitives; using GitHub.Services; @@ -49,6 +51,15 @@ public static IObservable GetLoggedInConnections(this IConnectionMa .Select(x => x.FirstOrDefault(y => y.IsLoggedIn)); } + public static async Task GetModelService( + this IConnectionManager cm, + ILocalRepositoryModel repository, + IModelServiceFactory factory) + { + var connection = await cm.LookupConnection(repository); + return connection != null ? await factory.CreateAsync(connection) : null; + } + public static IObservable LookupConnection(this IConnectionManager cm, ILocalRepositoryModel repository) { return Observable.Return(repository?.CloneUrl != null diff --git a/src/GitHub.Exports.Reactive/Factories/IModelServiceFactory.cs b/src/GitHub.Exports.Reactive/Factories/IModelServiceFactory.cs new file mode 100644 index 0000000000..8e476cd607 --- /dev/null +++ b/src/GitHub.Exports.Reactive/Factories/IModelServiceFactory.cs @@ -0,0 +1,13 @@ +using System; +using System.Threading.Tasks; +using GitHub.Models; +using GitHub.Services; + +namespace GitHub.Factories +{ + public interface IModelServiceFactory + { + Task CreateAsync(IConnection connection); + IModelService CreateBlocking(IConnection connection); + } +} diff --git a/src/GitHub.Exports.Reactive/GitHub.Exports.Reactive.csproj b/src/GitHub.Exports.Reactive/GitHub.Exports.Reactive.csproj index ffbe81c94e..49b121cece 100644 --- a/src/GitHub.Exports.Reactive/GitHub.Exports.Reactive.csproj +++ b/src/GitHub.Exports.Reactive/GitHub.Exports.Reactive.csproj @@ -100,6 +100,7 @@ + diff --git a/src/GitHub.Exports.Reactive/Services/IModelService.cs b/src/GitHub.Exports.Reactive/Services/IModelService.cs index 791d9f5997..f5e11a9168 100644 --- a/src/GitHub.Exports.Reactive/Services/IModelService.cs +++ b/src/GitHub.Exports.Reactive/Services/IModelService.cs @@ -14,6 +14,8 @@ namespace GitHub.Services /// public interface IModelService : IDisposable { + IApiClient ApiClient { get; } + IObservable GetCurrentUser(); IObservable InsertUser(AccountCacheItem user); IObservable> GetAccounts(); diff --git a/src/GitHub.Exports.Reactive/Services/IPullRequestService.cs b/src/GitHub.Exports.Reactive/Services/IPullRequestService.cs index d6cf781dfc..0e199c2cd6 100644 --- a/src/GitHub.Exports.Reactive/Services/IPullRequestService.cs +++ b/src/GitHub.Exports.Reactive/Services/IPullRequestService.cs @@ -10,7 +10,7 @@ namespace GitHub.Services { public interface IPullRequestService { - IObservable CreatePullRequest(IConnection connection, + IObservable CreatePullRequest(IModelService modelService, ILocalRepositoryModel sourceRepository, IRepositoryModel targetRepository, IBranch sourceBranch, IBranch targetBranch, string title, string body); diff --git a/src/GitHub.InlineReviews/Services/PullRequestSessionManager.cs b/src/GitHub.InlineReviews/Services/PullRequestSessionManager.cs index ea5db6d669..0521c237c7 100644 --- a/src/GitHub.InlineReviews/Services/PullRequestSessionManager.cs +++ b/src/GitHub.InlineReviews/Services/PullRequestSessionManager.cs @@ -8,10 +8,10 @@ using System.Reactive.Threading.Tasks; using System.Threading.Tasks; using GitHub.Extensions; +using GitHub.Factories; using GitHub.Helpers; using GitHub.InlineReviews.Models; using GitHub.Models; -using GitHub.Primitives; using GitHub.Services; using Microsoft.VisualStudio.Text; using Microsoft.VisualStudio.Text.Editor; @@ -29,7 +29,8 @@ public class PullRequestSessionManager : ReactiveObject, IPullRequestSessionMana { readonly IPullRequestService service; readonly IPullRequestSessionService sessionService; - readonly IRepositoryHosts hosts; + readonly IConnectionManager connectionManager; + readonly IModelServiceFactory modelServiceFactory; readonly ITeamExplorerServiceHolder teamExplorerService; readonly Dictionary, WeakReference> sessions = new Dictionary, WeakReference>(); @@ -39,27 +40,29 @@ public class PullRequestSessionManager : ReactiveObject, IPullRequestSessionMana /// /// Initializes a new instance of the class. /// - /// The git service to use. - /// The git client to use. - /// The diff service to use. - /// The pull request service to use. - /// The repository hosts. + /// The PR service to use. + /// The PR session service to use. + /// The connectionManager to use. + /// The ModelService factory. /// The team explorer service to use. [ImportingConstructor] public PullRequestSessionManager( IPullRequestService service, IPullRequestSessionService sessionService, - IRepositoryHosts hosts, + IConnectionManager connectionManager, + IModelServiceFactory modelServiceFactory, ITeamExplorerServiceHolder teamExplorerService) { Guard.ArgumentNotNull(service, nameof(service)); Guard.ArgumentNotNull(sessionService, nameof(sessionService)); - Guard.ArgumentNotNull(hosts, nameof(hosts)); + Guard.ArgumentNotNull(connectionManager, nameof(connectionManager)); + Guard.ArgumentNotNull(modelServiceFactory, nameof(modelServiceFactory)); Guard.ArgumentNotNull(teamExplorerService, nameof(teamExplorerService)); this.service = service; this.sessionService = sessionService; - this.hosts = hosts; + this.connectionManager = connectionManager; + this.modelServiceFactory = modelServiceFactory; this.teamExplorerService = teamExplorerService; teamExplorerService.Subscribe(this, x => RepoChanged(x).Forget()); } @@ -189,7 +192,7 @@ async Task RepoChanged(ILocalRepositoryModel repository) if (string.IsNullOrWhiteSpace(repository?.CloneUrl)) return; - var modelService = hosts.LookupHost(HostAddress.Create(repository.CloneUrl))?.ModelService; + var modelService = await connectionManager.GetModelService(repository, modelServiceFactory); var session = CurrentSession; if (modelService != null) @@ -246,7 +249,7 @@ async Task GetSessionInternal(IPullRequestModel pullRequest) if (session == null) { - var modelService = hosts.LookupHost(HostAddress.Create(repository.CloneUrl))?.ModelService; + var modelService = await connectionManager.GetModelService(repository, modelServiceFactory); if (modelService != null) { diff --git a/src/GitHub.TeamFoundation.14/Base/EnsureLoggedInSection.cs b/src/GitHub.TeamFoundation.14/Base/EnsureLoggedInSection.cs index de1a9d8e9e..1722840b27 100644 --- a/src/GitHub.TeamFoundation.14/Base/EnsureLoggedInSection.cs +++ b/src/GitHub.TeamFoundation.14/Base/EnsureLoggedInSection.cs @@ -1,30 +1,27 @@ using System; +using System.Globalization; using System.Reactive.Linq; +using System.Threading.Tasks; using GitHub.Api; using GitHub.Extensions; -using GitHub.Models; +using GitHub.Primitives; using GitHub.Services; using GitHub.UI; using GitHub.VisualStudio.Base; -using System.Globalization; -using GitHub.Primitives; -using System.Threading.Tasks; using GitHub.VisualStudio.UI; namespace GitHub.VisualStudio.TeamExplorer.Sync { public class EnsureLoggedInSection : TeamExplorerSectionBase { - readonly IRepositoryHosts hosts; readonly ITeamExplorerServices teServices; public EnsureLoggedInSection(IGitHubServiceProvider serviceProvider, ISimpleApiClientFactory apiFactory, ITeamExplorerServiceHolder holder, - IConnectionManager cm, IRepositoryHosts hosts, ITeamExplorerServices teServices) + IConnectionManager cm, ITeamExplorerServices teServices) : base(serviceProvider, apiFactory, holder, cm) { IsVisible = false; - this.hosts = hosts; this.teServices = teServices; } @@ -52,7 +49,7 @@ async Task CheckLogin() teServices.ClearNotifications(); var add = HostAddress.Create(ActiveRepoUri); - bool loggedIn = await connectionManager.IsLoggedIn(hosts, add); + bool loggedIn = await connectionManager.IsLoggedIn(add); if (!loggedIn) { var msg = string.Format(CultureInfo.CurrentUICulture, Resources.NotLoggedInMessage, add.Title, add.Title); diff --git a/src/GitHub.TeamFoundation.14/Sync/EnsureLoggedInSectionSync.cs b/src/GitHub.TeamFoundation.14/Sync/EnsureLoggedInSectionSync.cs index 7989393770..76be64acf1 100644 --- a/src/GitHub.TeamFoundation.14/Sync/EnsureLoggedInSectionSync.cs +++ b/src/GitHub.TeamFoundation.14/Sync/EnsureLoggedInSectionSync.cs @@ -1,8 +1,6 @@ using System.ComponentModel.Composition; using GitHub.Api; -using GitHub.Models; using GitHub.Services; -using Microsoft.TeamFoundation.Controls; namespace GitHub.VisualStudio.TeamExplorer.Sync { @@ -17,8 +15,8 @@ public class EnsureLoggedInSectionSync : EnsureLoggedInSection [ImportingConstructor] public EnsureLoggedInSectionSync(IGitHubServiceProvider serviceProvider, ISimpleApiClientFactory apiFactory, ITeamExplorerServiceHolder holder, - IConnectionManager cm, IRepositoryHosts hosts, ITeamExplorerServices teServices) - : base(serviceProvider, apiFactory, holder, cm, hosts, teServices) + IConnectionManager cm, ITeamExplorerServices teServices) + : base(serviceProvider, apiFactory, holder, cm, teServices) {} } } \ No newline at end of file diff --git a/src/GitHub.TeamFoundation.14/Sync/GitHubPublishSection.cs b/src/GitHub.TeamFoundation.14/Sync/GitHubPublishSection.cs index c234b2c11e..d147d45f4d 100644 --- a/src/GitHub.TeamFoundation.14/Sync/GitHubPublishSection.cs +++ b/src/GitHub.TeamFoundation.14/Sync/GitHubPublishSection.cs @@ -30,19 +30,16 @@ public class GitHubPublishSection : TeamExplorerSectionBase, IGitHubInvitationSe public const string GitHubPublishSectionId = "92655B52-360D-4BF5-95C5-D9E9E596AC76"; readonly Lazy lazyBrowser; - readonly IRepositoryHosts hosts; bool loggedIn; [ImportingConstructor] public GitHubPublishSection(IGitHubServiceProvider serviceProvider, ISimpleApiClientFactory apiFactory, ITeamExplorerServiceHolder holder, - IConnectionManager cm, Lazy browser, - IRepositoryHosts hosts) + IConnectionManager cm, Lazy browser) : base(serviceProvider, apiFactory, holder, cm) { lazyBrowser = browser; - this.hosts = hosts; Title = Resources.GitHubPublishSectionTitle; Name = "GitHub"; Provider = "GitHub, Inc"; @@ -68,7 +65,7 @@ async void Setup() { IsVisible = true; ShowGetStarted = true; - loggedIn = await connectionManager.IsLoggedIn(hosts); + loggedIn = await connectionManager.IsLoggedIn(); ShowLogin = !loggedIn; ShowSignup = !loggedIn; } @@ -91,7 +88,7 @@ protected override void RepoChanged(bool changed) public async Task Connect() { - loggedIn = await connectionManager.IsLoggedIn(hosts); + loggedIn = await connectionManager.IsLoggedIn(); if (loggedIn) ShowPublish(); else @@ -176,7 +173,7 @@ async Task Login() var uiProvider = ServiceProvider.GetService(); uiProvider.RunInDialog(UIControllerFlow.Authentication); - loggedIn = await connectionManager.IsLoggedIn(hosts); + loggedIn = await connectionManager.IsLoggedIn(); if (loggedIn) ShowPublish(); } diff --git a/src/GitHub.VisualStudio/UI/Views/GitHubPaneViewModel.cs b/src/GitHub.VisualStudio/UI/Views/GitHubPaneViewModel.cs index 4a208800aa..44a006f60c 100644 --- a/src/GitHub.VisualStudio/UI/Views/GitHubPaneViewModel.cs +++ b/src/GitHub.VisualStudio/UI/Views/GitHubPaneViewModel.cs @@ -28,7 +28,6 @@ public class GitHubPaneViewModel : TeamExplorerItemBase, IGitHubPaneViewModel bool initialized; readonly CompositeDisposable disposables = new CompositeDisposable(); - readonly IRepositoryHosts hosts; readonly IConnectionManager connectionManager; readonly IUIProvider uiProvider; readonly IVisualStudioBrowser browser; @@ -42,7 +41,7 @@ public class GitHubPaneViewModel : TeamExplorerItemBase, IGitHubPaneViewModel [ImportingConstructor] public GitHubPaneViewModel(IGitHubServiceProvider serviceProvider, ISimpleApiClientFactory apiFactory, ITeamExplorerServiceHolder holder, - IConnectionManager cm, IRepositoryHosts hosts, IUIProvider uiProvider, IVisualStudioBrowser vsBrowser, + IConnectionManager cm, IUIProvider uiProvider, IVisualStudioBrowser vsBrowser, IUsageTracker usageTracker) : base(serviceProvider, apiFactory, holder) { @@ -50,13 +49,11 @@ public GitHubPaneViewModel(IGitHubServiceProvider serviceProvider, Guard.ArgumentNotNull(apiFactory, nameof(apiFactory)); Guard.ArgumentNotNull(holder, nameof(holder)); Guard.ArgumentNotNull(cm, nameof(cm)); - Guard.ArgumentNotNull(hosts, nameof(hosts)); Guard.ArgumentNotNull(uiProvider, nameof(uiProvider)); Guard.ArgumentNotNull(vsBrowser, nameof(vsBrowser)); Guard.ArgumentNotNull(usageTracker, nameof(usageTracker)); this.connectionManager = cm; - this.hosts = hosts; this.uiProvider = uiProvider; this.usageTracker = usageTracker; @@ -150,7 +147,7 @@ public override void Initialize(IServiceProvider serviceProvider) base.Initialize(serviceProvider); - hosts.WhenAnyValue(x => x.IsLoggedInToAnyHost).Subscribe(_ => LoadDefault()); + ////hosts.WhenAnyValue(x => x.IsLoggedInToAnyHost).Subscribe(_ => LoadDefault()); } public void Initialize(ViewWithData data = null) diff --git a/src/UnitTests/GitHub.App/Models/RepositoryHostTests.cs b/src/UnitTests/GitHub.App/Models/RepositoryHostTests.cs deleted file mode 100644 index 68ad8bbcc8..0000000000 --- a/src/UnitTests/GitHub.App/Models/RepositoryHostTests.cs +++ /dev/null @@ -1,19 +0,0 @@ -using System; -using System.Collections.Generic; -using System.Net; -using System.Reactive.Linq; -using System.Threading.Tasks; -using Akavache; -using GitHub.Api; -using GitHub.Authentication; -using GitHub.Caches; -using GitHub.Models; -using GitHub.Primitives; -using GitHub.Services; -using NSubstitute; -using Octokit; -using Xunit; - -public class RepositoryHostTests -{ -} diff --git a/src/UnitTests/GitHub.App/Models/RepositoryHostsTests.cs b/src/UnitTests/GitHub.App/Models/RepositoryHostsTests.cs deleted file mode 100644 index 13f11c6019..0000000000 --- a/src/UnitTests/GitHub.App/Models/RepositoryHostsTests.cs +++ /dev/null @@ -1,206 +0,0 @@ -using System; -using System.Linq; -using System.Reactive.Linq; -using System.Threading.Tasks; -using GitHub.Extensions; -using GitHub.Factories; -using GitHub.Models; -using GitHub.Primitives; -using GitHub.Services; -using NSubstitute; -using Xunit; - -namespace UnitTests.GitHub.App.Models -{ - public class RepositoryHostsTests - { - static readonly HostAddress EnterpriseHostAddress = HostAddress.Create("https://enterprise.host"); - static readonly HostAddress InvalidHostAddress = HostAddress.Create("https://invalid.host"); - - public class TheGitHubHostProperty - { - [Fact] - public void ShouldInitiallyBeDisconnectedRepositoryHost() - { - var target = new RepositoryHosts( - CreateRepositoryHostFactory(), - CreateConnectionManager()); - - Assert.Same(RepositoryHosts.DisconnectedRepositoryHost, target.GitHubHost); - } - - [Fact] - public void ShouldCreateHostForExistingConnection() - { - var target = new RepositoryHosts( - CreateRepositoryHostFactory(), - CreateConnectionManager(HostAddress.GitHubDotComHostAddress)); - - Assert.NotSame(RepositoryHosts.DisconnectedRepositoryHost, target.GitHubHost); - } - - [Fact] - public async Task ShouldCreateHostWhenConnectionLoggedIn() - { - var connectionManager = CreateConnectionManager(); - var target = new RepositoryHosts( - CreateRepositoryHostFactory(), - connectionManager); - - await connectionManager.LogIn(HostAddress.GitHubDotComHostAddress, "user", "pass"); - - Assert.NotSame(RepositoryHosts.DisconnectedRepositoryHost, target.GitHubHost); - } - - [Fact] - public async Task ShouldRemoveHostWhenConnectionLoggedOut() - { - var connectionManager = CreateConnectionManager(HostAddress.GitHubDotComHostAddress); - var target = new RepositoryHosts( - CreateRepositoryHostFactory(), - connectionManager); - - await connectionManager.LogOut(HostAddress.GitHubDotComHostAddress); - - Assert.Same(RepositoryHosts.DisconnectedRepositoryHost, target.GitHubHost); - } - } - - public class TheEnterpriseHostProperty - { - [Fact] - public void ShouldInitiallyBeDisconnectedRepositoryHost() - { - var target = new RepositoryHosts( - CreateRepositoryHostFactory(), - CreateConnectionManager()); - - Assert.Same(RepositoryHosts.DisconnectedRepositoryHost, target.EnterpriseHost); - } - - [Fact] - public void ShouldCreateHostForExistingConnection() - { - var target = new RepositoryHosts( - CreateRepositoryHostFactory(), - CreateConnectionManager(EnterpriseHostAddress)); - - Assert.NotSame(RepositoryHosts.DisconnectedRepositoryHost, target.EnterpriseHost); - } - - [Fact] - public async Task ShouldCreateHostWhenConnectionLoggedIn() - { - var connectionManager = CreateConnectionManager(); - var target = new RepositoryHosts( - CreateRepositoryHostFactory(), - connectionManager); - - await connectionManager.LogIn(EnterpriseHostAddress, "user", "pass"); - - Assert.NotSame(RepositoryHosts.DisconnectedRepositoryHost, target.EnterpriseHost); - } - - [Fact] - public async Task ShouldRemoveHostWhenConnectionLoggedOut() - { - var connectionManager = CreateConnectionManager(EnterpriseHostAddress); - var target = new RepositoryHosts( - CreateRepositoryHostFactory(), - connectionManager); - - await connectionManager.LogOut(EnterpriseHostAddress); - - Assert.Same(RepositoryHosts.DisconnectedRepositoryHost, target.EnterpriseHost); - } - } - - public class TheLoginMethod - { - [Fact] - public async Task ShouldCreateGitHubHost() - { - var target = new RepositoryHosts( - CreateRepositoryHostFactory(), - CreateConnectionManager()); - - await target.LogIn(HostAddress.GitHubDotComHostAddress, "user", "pass"); - - Assert.NotSame(RepositoryHosts.DisconnectedRepositoryHost, target.GitHubHost); - } - - [Fact] - public async Task ShouldFailForInvalidHost() - { - var target = new RepositoryHosts( - CreateRepositoryHostFactory(), - CreateConnectionManager()); - - await Assert.ThrowsAsync(async () => - { - await target.LogIn(InvalidHostAddress, "user", "pass"); - }); - - Assert.Same(RepositoryHosts.DisconnectedRepositoryHost, target.GitHubHost); - } - } - - static IConnectionManager CreateConnectionManager(params HostAddress[] hostAddresses) - { - var result = Substitute.For(); - var connectionSource = hostAddresses.Select(x => new Connection(x, "user", null, null)); - var connections = new ObservableCollectionEx(connectionSource); - - result.Connections.Returns(connections); - - Func> login = async x => - { - var hostAddress = x.Arg(); - - if (hostAddress == InvalidHostAddress) - { - throw new Exception("Invalid host address."); - } - - var connection = new Connection( - hostAddress, - x.ArgAt(1), - null, - null); - - if (result.ConnectionCreated != null) - { - await result.ConnectionCreated(connection); - } - - connections.Add(connection); - return connection; - }; - - result.LogIn(null, null, null).ReturnsForAnyArgs(login); - - result.LogOut(null).ReturnsForAnyArgs(x => - { - var connection = connections.Single(y => y.HostAddress == x.Arg()); - connections.Remove(connection); - return Task.CompletedTask; - }); - - return result; - } - - static IRepositoryHostFactory CreateRepositoryHostFactory() - { - var result = Substitute.For(); - - result.Create(null).ReturnsForAnyArgs(x => - { - var host = Substitute.For(); - host.Address.Returns(x.Arg().HostAddress); - return host; - }); - - return result; - } - } -} diff --git a/src/UnitTests/GitHub.Exports.Reactive/Extensions/ConnectionManagerExtensionsTests.cs b/src/UnitTests/GitHub.Exports.Reactive/Extensions/ConnectionManagerExtensionsTests.cs new file mode 100644 index 0000000000..40445561ee --- /dev/null +++ b/src/UnitTests/GitHub.Exports.Reactive/Extensions/ConnectionManagerExtensionsTests.cs @@ -0,0 +1,13 @@ +using System; +using System.Collections.Generic; +using System.Linq; +using System.Text; +using System.Threading.Tasks; +using Xunit; + +namespace UnitTests.GitHub.Exports.Reactive.Extensions +{ + public class ConnectionManagerExtensionsTests + { + } +} diff --git a/src/UnitTests/UnitTests.csproj b/src/UnitTests/UnitTests.csproj index 93b8ac533f..52fd07e1dc 100644 --- a/src/UnitTests/UnitTests.csproj +++ b/src/UnitTests/UnitTests.csproj @@ -246,6 +246,7 @@ + diff --git a/test/GitHub.InlineReviews.UnitTests/Services/PullRequestSessionManagerTests.cs b/test/GitHub.InlineReviews.UnitTests/Services/PullRequestSessionManagerTests.cs index 9f5387b6be..ced70fa162 100644 --- a/test/GitHub.InlineReviews.UnitTests/Services/PullRequestSessionManagerTests.cs +++ b/test/GitHub.InlineReviews.UnitTests/Services/PullRequestSessionManagerTests.cs @@ -5,6 +5,7 @@ using System.Reactive.Subjects; using System.Text; using System.Threading.Tasks; +using GitHub.Extensions; using GitHub.Factories; using GitHub.InlineReviews.Models; using GitHub.InlineReviews.Services; @@ -12,7 +13,6 @@ using GitHub.Models; using GitHub.Primitives; using GitHub.Services; -using LibGit2Sharp; using Microsoft.VisualStudio.Text; using Microsoft.VisualStudio.Text.Editor; using Microsoft.VisualStudio.Utilities; @@ -41,15 +41,17 @@ public void ReadsPullRequestFromCorrectFork() service.GetPullRequestForCurrentBranch(null).ReturnsForAnyArgs( Observable.Return(Tuple.Create("fork", CurrentBranchPullRequestNumber))); - var repositoryHosts = CreateRepositoryHosts(); + var connectionManager = CreateConnectionManager(); + var modelFactory = CreateModelServiceFactory(); var repositoryModel = CreateRepositoryModel(); var target = new PullRequestSessionManager( service, Substitute.For(), - repositoryHosts, + connectionManager, + modelFactory, new FakeTeamExplorerServiceHolder(repositoryModel)); - var modelService = repositoryHosts.LookupHost(HostAddress.Create(repositoryModel.CloneUrl)).ModelService; + var modelService = modelFactory.CreateBlocking(connectionManager.Connections[0]); modelService.Received(1).GetPullRequest("fork", "repo", 15); } } @@ -62,7 +64,8 @@ public void CreatesSessionForCurrentBranch() var target = new PullRequestSessionManager( CreatePullRequestService(), Substitute.For(), - CreateRepositoryHosts(), + CreateConnectionManager(), + CreateModelServiceFactory(), new FakeTeamExplorerServiceHolder(CreateRepositoryModel())); Assert.NotNull(target.CurrentSession); @@ -78,7 +81,8 @@ public void CurrentSessionIsNullIfNoPullRequestForCurrentBranch() var target = new PullRequestSessionManager( service, Substitute.For(), - CreateRepositoryHosts(), + CreateConnectionManager(), + CreateModelServiceFactory(), new FakeTeamExplorerServiceHolder(CreateRepositoryModel())); Assert.Null(target.CurrentSession); @@ -92,7 +96,8 @@ public void CurrentSessionChangesWhenBranchChanges() var target = new PullRequestSessionManager( service, Substitute.For(), - CreateRepositoryHosts(), + CreateConnectionManager(), + CreateModelServiceFactory(), teService); var session = target.CurrentSession; @@ -110,7 +115,8 @@ public void CurrentSessionChangesWhenRepoChanged() var target = new PullRequestSessionManager( CreatePullRequestService(), Substitute.For(), - CreateRepositoryHosts(), + CreateConnectionManager(), + CreateModelServiceFactory(), teService); var session = target.CurrentSession; @@ -127,7 +133,8 @@ public void RepoChangedDoesntCreateNewSessionIfNotNecessary() var target = new PullRequestSessionManager( CreatePullRequestService(), Substitute.For(), - CreateRepositoryHosts(), + CreateConnectionManager(), + CreateModelServiceFactory(), teService); var session = target.CurrentSession; @@ -144,7 +151,8 @@ public void RepoChangedHandlesNullRepository() var target = new PullRequestSessionManager( CreatePullRequestService(), Substitute.For(), - CreateRepositoryHosts(), + CreateConnectionManager(), + CreateModelServiceFactory(), teService); teService.ActiveRepo = null; @@ -158,7 +166,8 @@ public void CreatesSessionWithCorrectRepositoryOwner() var target = new PullRequestSessionManager( CreatePullRequestService("this-owner"), Substitute.For(), - CreateRepositoryHosts(), + CreateConnectionManager(), + CreateModelServiceFactory(), new FakeTeamExplorerServiceHolder(CreateRepositoryModel())); Assert.Equal("this-owner", target.CurrentSession.RepositoryOwner); @@ -177,7 +186,8 @@ public async Task BaseShaIsSet() var target = new PullRequestSessionManager( CreatePullRequestService(), CreateSessionService(), - CreateRepositoryHosts(), + CreateConnectionManager(), + CreateModelServiceFactory(), new FakeTeamExplorerServiceHolder(CreateRepositoryModel())); var file = await target.GetLiveFile(FilePath, textView, textView.TextBuffer); @@ -192,7 +202,8 @@ public async Task CommitShaIsSet() var target = new PullRequestSessionManager( CreatePullRequestService(), CreateSessionService(), - CreateRepositoryHosts(), + CreateConnectionManager(), + CreateModelServiceFactory(), new FakeTeamExplorerServiceHolder(CreateRepositoryModel())); var file = await target.GetLiveFile(FilePath, textView, textView.TextBuffer); @@ -207,7 +218,8 @@ public async Task CommitShaIsNullIfModified() var target = new PullRequestSessionManager( CreatePullRequestService(), CreateSessionService(true), - CreateRepositoryHosts(), + CreateConnectionManager(), + CreateModelServiceFactory(), new FakeTeamExplorerServiceHolder(CreateRepositoryModel())); var file = await target.GetLiveFile(FilePath, textView, textView.TextBuffer); @@ -234,7 +246,8 @@ public async Task DiffIsSet() var target = new PullRequestSessionManager( CreatePullRequestService(), sessionService, - CreateRepositoryHosts(), + CreateConnectionManager(), + CreateModelServiceFactory(), new FakeTeamExplorerServiceHolder(CreateRepositoryModel())); var file = await target.GetLiveFile(FilePath, textView, textView.TextBuffer); @@ -251,7 +264,8 @@ public async Task InlineCommentThreadsIsSet() var target = new PullRequestSessionManager( CreatePullRequestService(), sessionService, - CreateRepositoryHosts(), + CreateConnectionManager(), + CreateModelServiceFactory(), new FakeTeamExplorerServiceHolder(CreateRepositoryModel())); sessionService.BuildCommentThreads( @@ -279,7 +293,8 @@ public async Task CreatesTrackingPointsForThreads() var target = new PullRequestSessionManager( CreatePullRequestService(), sessionService, - CreateRepositoryHosts(), + CreateConnectionManager(), + CreateModelServiceFactory(), new FakeTeamExplorerServiceHolder(CreateRepositoryModel())); sessionService.BuildCommentThreads( @@ -304,7 +319,8 @@ public async Task MovingToNoRepositoryShouldNullOutProperties() var target = new PullRequestSessionManager( CreatePullRequestService(), CreateSessionService(), - CreateRepositoryHosts(), + CreateConnectionManager(), + CreateModelServiceFactory(), teHolder); sessionService.BuildCommentThreads( @@ -347,7 +363,8 @@ public async Task ModifyingBufferMarksThreadsAsStaleAndSignalsRebuild() var target = new PullRequestSessionManager( CreatePullRequestService(), sessionService, - CreateRepositoryHosts(), + CreateConnectionManager(), + CreateModelServiceFactory(), new FakeTeamExplorerServiceHolder(CreateRepositoryModel())); sessionService.BuildCommentThreads( @@ -389,7 +406,8 @@ public async Task RebuildSignalUpdatesCommitSha() var target = new PullRequestSessionManager( CreatePullRequestService(), sessionService, - CreateRepositoryHosts(), + CreateConnectionManager(), + CreateModelServiceFactory(), new FakeTeamExplorerServiceHolder(CreateRepositoryModel())); var file = (PullRequestSessionLiveFile)await target.GetLiveFile(FilePath, textView, textView.TextBuffer); @@ -409,7 +427,8 @@ public async Task ClosingTextViewDisposesFile() var target = new PullRequestSessionManager( CreatePullRequestService(), CreateSessionService(), - CreateRepositoryHosts(), + CreateConnectionManager(), + CreateModelServiceFactory(), new FakeTeamExplorerServiceHolder(CreateRepositoryModel())); var file = (PullRequestSessionLiveFile)await target.GetLiveFile(FilePath, textView, textView.TextBuffer); @@ -450,7 +469,8 @@ Line 2 var target = new PullRequestSessionManager( CreatePullRequestService(), CreateRealSessionService(diff: diffService), - CreateRepositoryHosts(pullRequest), + CreateConnectionManager(), + CreateModelServiceFactory(pullRequest), new FakeTeamExplorerServiceHolder(CreateRepositoryModel())); var file = (PullRequestSessionLiveFile)await target.GetLiveFile(FilePath, textView, textView.TextBuffer); @@ -495,7 +515,8 @@ Line 2 var target = new PullRequestSessionManager( CreatePullRequestService(), CreateRealSessionService(diff: diffService), - CreateRepositoryHosts(pullRequest), + CreateConnectionManager(), + CreateModelServiceFactory(pullRequest), new FakeTeamExplorerServiceHolder(CreateRepositoryModel())); var file = (PullRequestSessionLiveFile)await target.GetLiveFile(FilePath, textView, textView.TextBuffer); @@ -554,7 +575,8 @@ Line 2 var target = new PullRequestSessionManager( CreatePullRequestService(), CreateRealSessionService(diff: diffService), - CreateRepositoryHosts(pullRequest), + CreateConnectionManager(), + CreateModelServiceFactory(pullRequest), new FakeTeamExplorerServiceHolder(CreateRepositoryModel())); var file = (PullRequestSessionLiveFile)await target.GetLiveFile(FilePath, textView, textView.TextBuffer); @@ -608,7 +630,8 @@ Line 2 var target = new PullRequestSessionManager( CreatePullRequestService(), CreateRealSessionService(diff: diffService), - CreateRepositoryHosts(pullRequest), + CreateConnectionManager(), + CreateModelServiceFactory(pullRequest), new FakeTeamExplorerServiceHolder(CreateRepositoryModel())); var file = (PullRequestSessionLiveFile)await target.GetLiveFile(FilePath, textView, textView.TextBuffer); @@ -638,7 +661,8 @@ public async Task CommitShaIsUpdatedOnTextChange() var target = new PullRequestSessionManager( CreatePullRequestService(), sessionService, - CreateRepositoryHosts(), + CreateConnectionManager(), + CreateModelServiceFactory(), new FakeTeamExplorerServiceHolder(CreateRepositoryModel())); var file = await target.GetLiveFile(FilePath, textView, textView.TextBuffer); @@ -666,7 +690,8 @@ public async Task UpdatingCurrentSessionPullRequestTriggersLinesChanged() var target = new PullRequestSessionManager( CreatePullRequestService(), sessionService, - CreateRepositoryHosts(), + CreateConnectionManager(), + CreateModelServiceFactory(), new FakeTeamExplorerServiceHolder(CreateRepositoryModel())); var file = await target.GetLiveFile(FilePath, textView, textView.TextBuffer); var raised = false; @@ -777,7 +802,8 @@ public async Task GetSessionReturnsAndUpdatesCurrentSessionIfNumbersMatch() var target = new PullRequestSessionManager( CreatePullRequestService(), Substitute.For(), - CreateRepositoryHosts(), + CreateConnectionManager(), + CreateModelServiceFactory(), new FakeTeamExplorerServiceHolder(CreateRepositoryModel())); var newModel = CreatePullRequestModel(CurrentBranchPullRequestNumber); @@ -793,7 +819,8 @@ public async Task GetSessionReturnsNewSessionForPullRequestWithDifferentNumber() var target = new PullRequestSessionManager( CreatePullRequestService(), Substitute.For(), - CreateRepositoryHosts(), + CreateConnectionManager(), + CreateModelServiceFactory(), new FakeTeamExplorerServiceHolder(CreateRepositoryModel())); var newModel = CreatePullRequestModel(NotCurrentBranchPullRequestNumber); @@ -810,7 +837,8 @@ public async Task GetSessionReturnsNewSessionForPullRequestWithDifferentBaseOwne var target = new PullRequestSessionManager( CreatePullRequestService(), Substitute.For(), - CreateRepositoryHosts(), + CreateConnectionManager(), + CreateModelServiceFactory(), new FakeTeamExplorerServiceHolder(CreateRepositoryModel())); var newModel = CreatePullRequestModel(CurrentBranchPullRequestNumber, "https://github.com/fork/repo"); @@ -827,7 +855,8 @@ public async Task GetSessionReturnsSameSessionEachTime() var target = new PullRequestSessionManager( CreatePullRequestService(), Substitute.For(), - CreateRepositoryHosts(), + CreateConnectionManager(), + CreateModelServiceFactory(), new FakeTeamExplorerServiceHolder(CreateRepositoryModel())); var newModel = CreatePullRequestModel(NotCurrentBranchPullRequestNumber); @@ -845,7 +874,8 @@ public async Task SessionCanBeCollected() var target = new PullRequestSessionManager( CreatePullRequestService(), Substitute.For(), - CreateRepositoryHosts(), + CreateConnectionManager(), + CreateModelServiceFactory(), new FakeTeamExplorerServiceHolder(CreateRepositoryModel())); Func run = async () => @@ -877,7 +907,8 @@ public async Task GetSessionUpdatesCurrentSessionIfCurrentBranchIsPullRequestBut var target = new PullRequestSessionManager( service, Substitute.For(), - CreateRepositoryHosts(), + CreateConnectionManager(), + CreateModelServiceFactory(), new FakeTeamExplorerServiceHolder(CreateRepositoryModel())); Assert.Null(target.CurrentSession); @@ -913,7 +944,17 @@ IPullRequestService CreatePullRequestService(string owner = "owner") return result; } - IRepositoryHosts CreateRepositoryHosts(IPullRequestModel pullRequest = null) + IConnectionManager CreateConnectionManager() + { + var connection = Substitute.For(); + connection.HostAddress.Returns(HostAddress.Create("https://github.com")); + + var result = Substitute.For(); + result.Connections.Returns(new ObservableCollectionEx(new[] { connection })); + return result; + } + + IModelServiceFactory CreateModelServiceFactory(IPullRequestModel pullRequest = null) { var modelService = Substitute.For(); modelService.GetPullRequest(null, null, 0).ReturnsForAnyArgs(x => @@ -923,12 +964,10 @@ IRepositoryHosts CreateRepositoryHosts(IPullRequestModel pullRequest = null) return Observable.Return(pr); }); - var repositoryHost = Substitute.For(); - repositoryHost.ModelService.Returns(modelService); - - var result = Substitute.For(); - result.LookupHost(null).ReturnsForAnyArgs(repositoryHost); - return result; + var factory = Substitute.For(); + factory.CreateAsync(null).ReturnsForAnyArgs(modelService); + factory.CreateBlocking(null).ReturnsForAnyArgs(modelService); + return factory; } ILocalRepositoryModel CreateRepositoryModel(string cloneUrl = OwnerCloneUrl) From 038f232749618892a9a4a4257f925c661b53dc6f Mon Sep 17 00:00:00 2001 From: Steven Kirk Date: Fri, 27 Oct 2017 00:45:21 +0200 Subject: [PATCH 10/16] WIP: Removing RepositoryHost(s) from tests. --- .../ViewModels/RepositoryCloneViewModel.cs | 2 +- .../ViewModels/RepositoryCreationViewModel.cs | 2 +- .../Controllers/UIControllerTests.cs | 87 ++++------ .../Services/PullRequestServiceTests.cs | 20 +-- .../ViewModels/GistCreationViewModelTests.cs | 30 ++-- .../ViewModels/LoginControlViewModelTests.cs | 13 +- .../ViewModels/LoginToGitHubViewModelTests.cs | 94 +++++------ .../PullRequestCreationViewModelTests.cs | 45 +++-- .../PullRequestListViewModelTests.cs | 33 ++-- .../RepositoryCloneViewModelTests.cs | 159 +++++++++--------- .../RepositoryCreationViewModelTests.cs | 94 +++++------ .../RepositoryPublishViewModelTests.cs | 131 +++++---------- .../UI/Views/GitHubPaneViewModelTests.cs | 23 +-- src/UnitTests/Substitutes.cs | 7 - 14 files changed, 336 insertions(+), 404 deletions(-) diff --git a/src/GitHub.App/ViewModels/RepositoryCloneViewModel.cs b/src/GitHub.App/ViewModels/RepositoryCloneViewModel.cs index 0f68d8d431..3a3877f8cb 100644 --- a/src/GitHub.App/ViewModels/RepositoryCloneViewModel.cs +++ b/src/GitHub.App/ViewModels/RepositoryCloneViewModel.cs @@ -43,7 +43,7 @@ public class RepositoryCloneViewModel : DialogViewModelBase, IRepositoryCloneVie bool loadingFailed; [ImportingConstructor] - RepositoryCloneViewModel( + public RepositoryCloneViewModel( IConnection connection, IModelServiceFactory modelServiceFactory, IRepositoryCloneService cloneService, diff --git a/src/GitHub.App/ViewModels/RepositoryCreationViewModel.cs b/src/GitHub.App/ViewModels/RepositoryCreationViewModel.cs index 5dd731443b..319b59d180 100644 --- a/src/GitHub.App/ViewModels/RepositoryCreationViewModel.cs +++ b/src/GitHub.App/ViewModels/RepositoryCreationViewModel.cs @@ -43,7 +43,7 @@ public class RepositoryCreationViewModel : RepositoryFormViewModel, IRepositoryC readonly IUsageTracker usageTracker; [ImportingConstructor] - RepositoryCreationViewModel( + public RepositoryCreationViewModel( IConnection connection, IModelServiceFactory modelServiceFactory, IOperatingSystem operatingSystem, diff --git a/src/UnitTests/GitHub.App/Controllers/UIControllerTests.cs b/src/UnitTests/GitHub.App/Controllers/UIControllerTests.cs index 29dd78cae4..632aac53a0 100644 --- a/src/UnitTests/GitHub.App/Controllers/UIControllerTests.cs +++ b/src/UnitTests/GitHub.App/Controllers/UIControllerTests.cs @@ -1,25 +1,21 @@ using System; +using System.ComponentModel; using System.ComponentModel.Composition; +using System.Reactive; using System.Reactive.Linq; +using System.Reactive.Subjects; +using GitHub.App.Factories; using GitHub.Controllers; +using GitHub.Extensions; +using GitHub.Extensions.Reactive; using GitHub.Models; using GitHub.Services; using GitHub.UI; -using NSubstitute; -using Xunit; -using UnitTests; using GitHub.ViewModels; +using NSubstitute; using ReactiveUI; -using System.Collections.Generic; -using System.Reactive.Subjects; -using GitHub.Primitives; -using System.ComponentModel; -using System.Collections.ObjectModel; -using GitHub.App.Factories; -using System.Reactive; -using GitHub.Extensions.Reactive; -using System.Threading.Tasks; -using GitHub.Extensions; +using UnitTests; +using Xunit; public class UIControllerTests { @@ -29,10 +25,9 @@ public class TheDisposeMethod : TestBaseClass public void WithMultipleCallsDoesNotThrowException() { var uiProvider = Substitute.For(); - var hosts = Substitute.For(); var factory = Substitute.For(); var cm = Substitutes.ConnectionManager; - var uiController = new UIController(uiProvider, hosts, factory, cm); + var uiController = new UIController(uiProvider, factory, cm); uiController.Dispose(); uiController.Dispose(); @@ -99,13 +94,10 @@ protected IUIFactory SetupFactory(IServiceProvider provider) return new UIFactory(factory); } - protected IConnection SetupConnection(IServiceProvider provider, IRepositoryHosts hosts, - IRepositoryHost host, bool loggedIn = true) + protected IConnection SetupConnection(IServiceProvider provider, bool loggedIn = true) { var connection = provider.GetConnection(); - hosts.LookupHost(connection.HostAddress).Returns(host); connection.IsLoggedIn.Returns(loggedIn); - host.IsLoggedIn.Returns(loggedIn); return connection; } @@ -127,7 +119,6 @@ public class AuthFlow : UIControllerTestBase public void RunningNonAuthFlowWithoutBeingLoggedInRunsAuthFlow() { var provider = Substitutes.GetFullyMockedServiceProvider(); - var hosts = provider.GetRepositoryHosts(); var factory = SetupFactory(provider); var cm = provider.GetConnectionManager(); var cons = new ObservableCollectionEx(); @@ -135,7 +126,7 @@ public void RunningNonAuthFlowWithoutBeingLoggedInRunsAuthFlow() cm.Connections.Returns(cons); cm.GetLoadedConnections().Returns(cons); - using (var uiController = new UIController((IGitHubServiceProvider)provider, hosts, factory, cm)) + using (var uiController = new UIController((IGitHubServiceProvider)provider, factory, cm)) { var count = 0; var flow = uiController.Configure(UIControllerFlow.Clone); @@ -162,7 +153,6 @@ public void RunningNonAuthFlowWhenLoggedInRunsNonAuthFlow() { var provider = Substitutes.GetFullyMockedServiceProvider(); - var hosts = provider.GetRepositoryHosts(); var factory = SetupFactory(provider); var cm = provider.GetConnectionManager(); var cons = new ObservableCollectionEx(); @@ -171,9 +161,9 @@ public void RunningNonAuthFlowWhenLoggedInRunsNonAuthFlow() cm.GetLoadedConnections().Returns(cons); // simulate being logged in - cons.Add(SetupConnection(provider, hosts, hosts.GitHubHost)); + cons.Add(SetupConnection(provider)); - using (var uiController = new UIController((IGitHubServiceProvider)provider, hosts, factory, cm)) + using (var uiController = new UIController((IGitHubServiceProvider)provider, factory, cm)) { var count = 0; var flow = uiController.Configure(UIControllerFlow.Clone); @@ -199,7 +189,6 @@ public void RunningNonAuthFlowWhenLoggedInRunsNonAuthFlow() public void RunningAuthFlowWithoutBeingLoggedInRunsAuthFlow() { var provider = Substitutes.GetFullyMockedServiceProvider(); - var hosts = provider.GetRepositoryHosts(); var factory = SetupFactory(provider); var cm = provider.GetConnectionManager(); var cons = new ObservableCollectionEx(); @@ -207,7 +196,7 @@ public void RunningAuthFlowWithoutBeingLoggedInRunsAuthFlow() cm.Connections.Returns(cons); cm.GetLoadedConnections().Returns(cons); - using (var uiController = new UIController((IGitHubServiceProvider)provider, hosts, factory, cm)) + using (var uiController = new UIController((IGitHubServiceProvider)provider, factory, cm)) { var count = 0; var flow = uiController.Configure(UIControllerFlow.Authentication); @@ -233,19 +222,17 @@ public void RunningAuthFlowWithoutBeingLoggedInRunsAuthFlow() public void RunningAuthFlowWhenLoggedInRunsAuthFlow() { var provider = Substitutes.GetFullyMockedServiceProvider(); - var hosts = provider.GetRepositoryHosts(); var factory = SetupFactory(provider); var cm = provider.GetConnectionManager(); // simulate being logged in - var host = hosts.GitHubHost; - var connection = SetupConnection(provider, hosts, host); + var connection = SetupConnection(provider); var cons = new ObservableCollectionEx { connection }; cm.Connections.Returns(cons); cm.GetLoadedConnections().Returns(cons); - using (var uiController = new UIController((IGitHubServiceProvider)provider, hosts, factory, cm)) + using (var uiController = new UIController((IGitHubServiceProvider)provider, factory, cm)) { var count = 0; var flow = uiController.Configure(UIControllerFlow.Authentication); @@ -271,7 +258,6 @@ public void RunningAuthFlowWhenLoggedInRunsAuthFlow() public void AuthFlowWithout2FA() { var provider = Substitutes.GetFullyMockedServiceProvider(); - var hosts = provider.GetRepositoryHosts(); var factory = SetupFactory(provider); var cm = provider.GetConnectionManager(); var cons = new ObservableCollectionEx(); @@ -279,7 +265,7 @@ public void AuthFlowWithout2FA() cm.Connections.Returns(cons); cm.GetLoadedConnections().Returns(cons); - using (var uiController = new UIController((IGitHubServiceProvider)provider, hosts, factory, cm)) + using (var uiController = new UIController((IGitHubServiceProvider)provider, factory, cm)) { var count = 0; var flow = uiController.Configure(UIControllerFlow.Clone); @@ -291,7 +277,7 @@ public void AuthFlowWithout2FA() case 1: Assert.IsAssignableFrom>(uc); // login - cons.Add(SetupConnection(provider, hosts, hosts.GitHubHost)); + cons.Add(SetupConnection(provider)); TriggerDone(data.View.ViewModel); break; case 2: @@ -311,7 +297,6 @@ public void AuthFlowWithout2FA() public void AuthFlowWith2FA() { var provider = Substitutes.GetFullyMockedServiceProvider(); - var hosts = provider.GetRepositoryHosts(); var factory = SetupFactory(provider); var cm = provider.GetConnectionManager(); var cons = new ObservableCollectionEx(); @@ -319,7 +304,7 @@ public void AuthFlowWith2FA() cm.Connections.Returns(cons); cm.GetLoadedConnections().Returns(cons); - using (var uiController = new UIController((IGitHubServiceProvider)provider, hosts, factory, cm)) + using (var uiController = new UIController((IGitHubServiceProvider)provider, factory, cm)) { var count = 0; var flow = uiController.Configure(UIControllerFlow.Clone); @@ -337,7 +322,7 @@ public void AuthFlowWith2FA() case 2: Assert.IsAssignableFrom>(uc); // login - cons.Add(SetupConnection(provider, hosts, hosts.GitHubHost)); + cons.Add(SetupConnection(provider)); // continue by triggering done on login view var vm2 = factory.CreateViewAndViewModel(GitHub.Exports.UIViewType.Login).ViewModel; TriggerDone(vm2); @@ -359,7 +344,6 @@ public void AuthFlowWith2FA() public void BackAndForth() { var provider = Substitutes.GetFullyMockedServiceProvider(); - var hosts = provider.GetRepositoryHosts(); var factory = SetupFactory(provider); var cm = provider.GetConnectionManager(); var cons = new ObservableCollectionEx(); @@ -367,7 +351,7 @@ public void BackAndForth() cm.Connections.Returns(cons); cm.GetLoadedConnections().Returns(cons); - using (var uiController = new UIController((IGitHubServiceProvider)provider, hosts, factory, cm)) + using (var uiController = new UIController((IGitHubServiceProvider)provider, factory, cm)) { var count = 0; var flow = uiController.Configure(UIControllerFlow.Clone); @@ -401,7 +385,7 @@ public void BackAndForth() case 4: { Assert.IsAssignableFrom>(uc); // login - cons.Add(SetupConnection(provider, hosts, hosts.GitHubHost)); + cons.Add(SetupConnection(provider)); var vm2 = factory.CreateViewAndViewModel(GitHub.Exports.UIViewType.Login).ViewModel; TriggerDone(vm2); break; @@ -427,7 +411,6 @@ public class CloneFlow : UIControllerTestBase public void Flow() { var provider = Substitutes.GetFullyMockedServiceProvider(); - var hosts = provider.GetRepositoryHosts(); var factory = SetupFactory(provider); var cm = provider.GetConnectionManager(); var cons = new ObservableCollectionEx(); @@ -436,9 +419,9 @@ public void Flow() cm.GetLoadedConnections().Returns(cons); // simulate being logged in - cons.Add(SetupConnection(provider, hosts, hosts.GitHubHost)); + cons.Add(SetupConnection(provider)); - using (var uiController = new UIController((IGitHubServiceProvider)provider, hosts, factory, cm)) + using (var uiController = new UIController((IGitHubServiceProvider)provider, factory, cm)) { var count = 0; var flow = uiController.Configure(UIControllerFlow.Clone); @@ -467,7 +450,6 @@ public class CreateFlow : UIControllerTestBase public void Flow() { var provider = Substitutes.GetFullyMockedServiceProvider(); - var hosts = provider.GetRepositoryHosts(); var factory = SetupFactory(provider); var cm = provider.GetConnectionManager(); var cons = new ObservableCollectionEx(); @@ -476,9 +458,9 @@ public void Flow() cm.GetLoadedConnections().Returns(cons); // simulate being logged in - cons.Add(SetupConnection(provider, hosts, hosts.GitHubHost)); + cons.Add(SetupConnection(provider)); - using (var uiController = new UIController((IGitHubServiceProvider)provider, hosts, factory, cm)) + using (var uiController = new UIController((IGitHubServiceProvider)provider, factory, cm)) { var count = 0; var flow = uiController.Configure(UIControllerFlow.Create); @@ -507,17 +489,16 @@ public class PublishFlow : UIControllerTestBase public void FlowWithConnection() { var provider = Substitutes.GetFullyMockedServiceProvider(); - var hosts = provider.GetRepositoryHosts(); var factory = SetupFactory(provider); var cm = provider.GetConnectionManager(); var cons = new ObservableCollectionEx(); cm.Connections.Returns(cons); - var connection = SetupConnection(provider, hosts, hosts.GitHubHost); + var connection = SetupConnection(provider); // simulate being logged in cons.Add(connection); - using (var uiController = new UIController((IGitHubServiceProvider)provider, hosts, factory, cm)) + using (var uiController = new UIController((IGitHubServiceProvider)provider, factory, cm)) { var count = 0; var flow = uiController.Configure(UIControllerFlow.Publish, connection); @@ -544,7 +525,6 @@ public void FlowWithConnection() public void FlowWithoutConnection() { var provider = Substitutes.GetFullyMockedServiceProvider(); - var hosts = provider.GetRepositoryHosts(); var factory = SetupFactory(provider); var cm = provider.GetConnectionManager(); var cons = new ObservableCollectionEx(); @@ -552,12 +532,12 @@ public void FlowWithoutConnection() cm.Connections.Returns(cons); cm.GetLoadedConnections().Returns(cons); - var connection = SetupConnection(provider, hosts, hosts.GitHubHost); + var connection = SetupConnection(provider); // simulate being logged in cons.Add(connection); - using (var uiController = new UIController((IGitHubServiceProvider)provider, hosts, factory, cm)) + using (var uiController = new UIController((IGitHubServiceProvider)provider, factory, cm)) { var count = 0; var flow = uiController.Configure(UIControllerFlow.Publish); @@ -587,7 +567,6 @@ public class GistFlow : UIControllerTestBase public void ShowsGistDialog() { var provider = Substitutes.GetFullyMockedServiceProvider(); - var hosts = provider.GetRepositoryHosts(); var factory = SetupFactory(provider); var cm = provider.GetConnectionManager(); var cons = new ObservableCollectionEx(); @@ -596,9 +575,9 @@ public void ShowsGistDialog() cm.GetLoadedConnections().Returns(cons); // simulate being logged in - cons.Add(SetupConnection(provider, hosts, hosts.GitHubHost, true)); + cons.Add(SetupConnection(provider, true)); - using (var uiController = new UIController((IGitHubServiceProvider)provider, hosts, factory, cm)) + using (var uiController = new UIController((IGitHubServiceProvider)provider, factory, cm)) { var count = 0; bool? success = null; diff --git a/src/UnitTests/GitHub.App/Services/PullRequestServiceTests.cs b/src/UnitTests/GitHub.App/Services/PullRequestServiceTests.cs index c0674226f3..2eba98b71a 100644 --- a/src/UnitTests/GitHub.App/Services/PullRequestServiceTests.cs +++ b/src/UnitTests/GitHub.App/Services/PullRequestServiceTests.cs @@ -224,7 +224,7 @@ public void CreatePullRequestAllArgsMandatory() var serviceProvider = Substitutes.ServiceProvider; var service = new PullRequestService(Substitute.For(), serviceProvider.GetGitService(), serviceProvider.GetOperatingSystem(), Substitute.For()); - IRepositoryHost host = null; + IModelService ms = null; ILocalRepositoryModel sourceRepo = null; ILocalRepositoryModel targetRepo = null; string title = null; @@ -232,28 +232,28 @@ public void CreatePullRequestAllArgsMandatory() IBranch source = null; IBranch target = null; - Assert.Throws(() => service.CreatePullRequest(host, sourceRepo, targetRepo, source, target, title, body)); + Assert.Throws(() => service.CreatePullRequest(ms, sourceRepo, targetRepo, source, target, title, body)); - host = serviceProvider.GetRepositoryHosts().GitHubHost; - Assert.Throws(() => service.CreatePullRequest(host, sourceRepo, targetRepo, source, target, title, body)); + ms = Substitute.For(); + Assert.Throws(() => service.CreatePullRequest(ms, sourceRepo, targetRepo, source, target, title, body)); sourceRepo = new LocalRepositoryModel("name", new GitHub.Primitives.UriString("http://github.com/github/stuff"), "c:\\path"); - Assert.Throws(() => service.CreatePullRequest(host, sourceRepo, targetRepo, source, target, title, body)); + Assert.Throws(() => service.CreatePullRequest(ms, sourceRepo, targetRepo, source, target, title, body)); targetRepo = new LocalRepositoryModel("name", new GitHub.Primitives.UriString("http://github.com/github/stuff"), "c:\\path"); - Assert.Throws(() => service.CreatePullRequest(host, sourceRepo, targetRepo, source, target, title, body)); + Assert.Throws(() => service.CreatePullRequest(ms, sourceRepo, targetRepo, source, target, title, body)); title = "a title"; - Assert.Throws(() => service.CreatePullRequest(host, sourceRepo, targetRepo, source, target, title, body)); + Assert.Throws(() => service.CreatePullRequest(ms, sourceRepo, targetRepo, source, target, title, body)); body = "a body"; - Assert.Throws(() => service.CreatePullRequest(host, sourceRepo, targetRepo, source, target, title, body)); + Assert.Throws(() => service.CreatePullRequest(ms, sourceRepo, targetRepo, source, target, title, body)); source = new BranchModel("source", sourceRepo); - Assert.Throws(() => service.CreatePullRequest(host, sourceRepo, targetRepo, source, target, title, body)); + Assert.Throws(() => service.CreatePullRequest(ms, sourceRepo, targetRepo, source, target, title, body)); target = new BranchModel("target", targetRepo); - var pr = service.CreatePullRequest(host, sourceRepo, targetRepo, source, target, title, body); + var pr = service.CreatePullRequest(ms, sourceRepo, targetRepo, source, target, title, body); Assert.NotNull(pr); } diff --git a/src/UnitTests/GitHub.App/ViewModels/GistCreationViewModelTests.cs b/src/UnitTests/GitHub.App/ViewModels/GistCreationViewModelTests.cs index 43057711d8..0e27fd3eea 100644 --- a/src/UnitTests/GitHub.App/ViewModels/GistCreationViewModelTests.cs +++ b/src/UnitTests/GitHub.App/ViewModels/GistCreationViewModelTests.cs @@ -1,19 +1,18 @@ using System; -using System.Collections.Generic; using System.Linq; -using System.Text; -using System.Threading.Tasks; +using System.Reactive.Linq; +using GitHub.Api; using GitHub.Factories; +using GitHub.Models; +using GitHub.SampleData; using GitHub.Services; using GitHub.ViewModels; using NSubstitute; using Octokit; +using ReactiveUI; using UnitTests; using Xunit; -using System.Reactive.Linq; -using GitHub.Models; -using ReactiveUI; -using GitHub.SampleData; +using IConnection = GitHub.Models.IConnection; public class GistCreationViewModelTests { @@ -21,11 +20,19 @@ static IGistCreationViewModel CreateViewModel(IServiceProvider provider, string { var selectedTextProvider = Substitute.For(); selectedTextProvider.GetSelectedText().Returns(selectedText); - var repositoryHost = provider.GetRepositoryHosts().GitHubHost; + var accounts = new ReactiveList() { Substitute.For(), Substitute.For() }; - repositoryHost.ModelService.GetAccounts().Returns(Observable.Return(accounts)); + var modelService = Substitute.For(); + modelService.GetAccounts().Returns(Observable.Return(accounts)); + + var modelServiceFactory = Substitute.For(); + modelServiceFactory.CreateAsync(null).ReturnsForAnyArgs(modelService); + modelServiceFactory.CreateBlocking(null).ReturnsForAnyArgs(modelService); + var gistPublishService = provider.GetGistPublishService(); - return new GistCreationViewModel(repositoryHost, selectedTextProvider, gistPublishService, Substitute.For()) + var connection = Substitute.For(); + + return new GistCreationViewModel(connection, modelServiceFactory, selectedTextProvider, gistPublishService, Substitute.For()) { FileName = fileName, IsPrivate = isPrivate @@ -42,13 +49,12 @@ public void CreatesAGistUsingTheApiClient(string selectedText, string fileName, var provider = Substitutes.ServiceProvider; var vm = CreateViewModel(provider, selectedText, fileName, isPrivate); var gistPublishService = provider.GetGistPublishService(); - var repositoryHost = provider.GetRepositoryHosts().GitHubHost; vm.CreateGist.Execute(null); gistPublishService .Received() .PublishGist( - repositoryHost.ApiClient, + Arg.Any(), Arg.Is(g => g.Public == !isPrivate && g.Files.First().Key == fileName && g.Files.First().Value == selectedText)); diff --git a/src/UnitTests/GitHub.App/ViewModels/LoginControlViewModelTests.cs b/src/UnitTests/GitHub.App/ViewModels/LoginControlViewModelTests.cs index 8201740eef..b8fd39d24b 100644 --- a/src/UnitTests/GitHub.App/ViewModels/LoginControlViewModelTests.cs +++ b/src/UnitTests/GitHub.App/ViewModels/LoginControlViewModelTests.cs @@ -3,6 +3,7 @@ using System.Threading.Tasks; using GitHub.Authentication; using GitHub.Models; +using GitHub.Services; using GitHub.ViewModels; using NSubstitute; using ReactiveUI; @@ -15,7 +16,7 @@ public class TheAuthenticationResultsCommand : TestBaseClass [Fact] public async Task SucessfulGitHubLoginSignalsDone() { - var repositoryHosts = Substitute.For(); + var connectionManager = Substitute.For(); var gitHubLogin = Substitute.For(); var gitHubLoginCommand = ReactiveCommand.CreateAsyncObservable(_ => @@ -23,7 +24,7 @@ public async Task SucessfulGitHubLoginSignalsDone() gitHubLogin.Login.Returns(gitHubLoginCommand); var enterpriseLogin = Substitute.For(); - var loginViewModel = new LoginControlViewModel(repositoryHosts, gitHubLogin, enterpriseLogin); + var loginViewModel = new LoginControlViewModel(connectionManager, gitHubLogin, enterpriseLogin); var signalled = false; loginViewModel.Done.Subscribe(_ => signalled = true); @@ -35,7 +36,7 @@ public async Task SucessfulGitHubLoginSignalsDone() [Fact] public async Task FailedGitHubLoginDoesNotSignalDone() { - var repositoryHosts = Substitute.For(); + var connectionManager = Substitute.For(); var gitHubLogin = Substitute.For(); var gitHubLoginCommand = ReactiveCommand.CreateAsyncObservable(_ => @@ -43,7 +44,7 @@ public async Task FailedGitHubLoginDoesNotSignalDone() gitHubLogin.Login.Returns(gitHubLoginCommand); var enterpriseLogin = Substitute.For(); - var loginViewModel = new LoginControlViewModel(repositoryHosts, gitHubLogin, enterpriseLogin); + var loginViewModel = new LoginControlViewModel(connectionManager, gitHubLogin, enterpriseLogin); var signalled = false; loginViewModel.Done.Subscribe(_ => signalled = true); @@ -55,7 +56,7 @@ public async Task FailedGitHubLoginDoesNotSignalDone() [Fact] public async Task AllowsLoginFromEnterpriseAfterGitHubLoginHasFailed() { - var repositoryHosts = Substitute.For(); + var connectionManager = Substitute.For(); var gitHubLogin = Substitute.For(); var gitHubLoginCommand = ReactiveCommand.CreateAsyncObservable(_ => @@ -67,7 +68,7 @@ public async Task AllowsLoginFromEnterpriseAfterGitHubLoginHasFailed() Observable.Return(AuthenticationResult.Success)); enterpriseLogin.Login.Returns(enterpriseLoginCommand); - var loginViewModel = new LoginControlViewModel(repositoryHosts, gitHubLogin, enterpriseLogin); + var loginViewModel = new LoginControlViewModel(connectionManager, gitHubLogin, enterpriseLogin); var success = false; loginViewModel.AuthenticationResults diff --git a/src/UnitTests/GitHub.App/ViewModels/LoginToGitHubViewModelTests.cs b/src/UnitTests/GitHub.App/ViewModels/LoginToGitHubViewModelTests.cs index 8fa35e1c45..5b2d7c6cd9 100644 --- a/src/UnitTests/GitHub.App/ViewModels/LoginToGitHubViewModelTests.cs +++ b/src/UnitTests/GitHub.App/ViewModels/LoginToGitHubViewModelTests.cs @@ -14,60 +14,60 @@ public class LoginToGitHubViewModelTests { - public class TheLoginCommand : TestBaseClass - { - [Fact] - public void ShowsHelpfulTooltipWhenForbiddenResponseReceived() - { - var response = Substitute.For(); - response.StatusCode.Returns(HttpStatusCode.Forbidden); - var repositoryHosts = Substitute.For(); - repositoryHosts.LogIn(HostAddress.GitHubDotComHostAddress, Args.String, Args.String) - .Returns(_ => Observable.Throw(new ForbiddenException(response))); - var browser = Substitute.For(); - var loginViewModel = new LoginToGitHubViewModel(repositoryHosts, browser); + //public class TheLoginCommand : TestBaseClass + //{ + // [Fact] + // public void ShowsHelpfulTooltipWhenForbiddenResponseReceived() + // { + // var response = Substitute.For(); + // response.StatusCode.Returns(HttpStatusCode.Forbidden); + // var repositoryHosts = Substitute.For(); + // repositoryHosts.LogIn(HostAddress.GitHubDotComHostAddress, Args.String, Args.String) + // .Returns(_ => Observable.Throw(new ForbiddenException(response))); + // var browser = Substitute.For(); + // var loginViewModel = new LoginToGitHubViewModel(repositoryHosts, browser); - loginViewModel.Login.Execute(null); + // loginViewModel.Login.Execute(null); - Assert.Equal("Make sure to use your password and not a Personal Access token to sign in.", - loginViewModel.Error.ErrorMessage); - } - } + // Assert.Equal("Make sure to use your password and not a Personal Access token to sign in.", + // loginViewModel.Error.ErrorMessage); + // } + //} - public class TheSignupCommand : TestBaseClass - { - [Fact] - public void LaunchesBrowserToSignUpPage() - { - var repositoryHosts = Substitute.For(); - var gitHubHost = Substitute.For(); - gitHubHost.Address.Returns(HostAddress.GitHubDotComHostAddress); - repositoryHosts.GitHubHost.Returns(gitHubHost); - var browser = Substitute.For(); - var loginViewModel = new LoginToGitHubViewModel(repositoryHosts, browser); + //public class TheSignupCommand : TestBaseClass + //{ + // [Fact] + // public void LaunchesBrowserToSignUpPage() + // { + // var repositoryHosts = Substitute.For(); + // var gitHubHost = Substitute.For(); + // gitHubHost.Address.Returns(HostAddress.GitHubDotComHostAddress); + // repositoryHosts.GitHubHost.Returns(gitHubHost); + // var browser = Substitute.For(); + // var loginViewModel = new LoginToGitHubViewModel(repositoryHosts, browser); - loginViewModel.SignUp.Execute(null); + // loginViewModel.SignUp.Execute(null); - browser.Received().OpenUrl(GitHubUrls.Plans); - } - } + // browser.Received().OpenUrl(GitHubUrls.Plans); + // } + //} - public class TheForgotPasswordCommand : TestBaseClass - { - [Fact] - public void LaunchesBrowserToForgotPasswordPage() - { - var repositoryHosts = Substitute.For(); - var gitHubHost = Substitute.For(); - gitHubHost.Address.Returns(HostAddress.GitHubDotComHostAddress); - repositoryHosts.GitHubHost.Returns(gitHubHost); - var browser = Substitute.For(); - var loginViewModel = new LoginToGitHubViewModel(repositoryHosts, browser); + //public class TheForgotPasswordCommand : TestBaseClass + //{ + // [Fact] + // public void LaunchesBrowserToForgotPasswordPage() + // { + // var repositoryHosts = Substitute.For(); + // var gitHubHost = Substitute.For(); + // gitHubHost.Address.Returns(HostAddress.GitHubDotComHostAddress); + // repositoryHosts.GitHubHost.Returns(gitHubHost); + // var browser = Substitute.For(); + // var loginViewModel = new LoginToGitHubViewModel(repositoryHosts, browser); - loginViewModel.NavigateForgotPassword.Execute(null); + // loginViewModel.NavigateForgotPassword.Execute(null); - browser.Received().OpenUrl(new Uri(HostAddress.GitHubDotComHostAddress.WebUri, GitHubUrls.ForgotPasswordPath)); - } - } + // browser.Received().OpenUrl(new Uri(HostAddress.GitHubDotComHostAddress.WebUri, GitHubUrls.ForgotPasswordPath)); + // } + //} } \ No newline at end of file diff --git a/src/UnitTests/GitHub.App/ViewModels/PullRequestCreationViewModelTests.cs b/src/UnitTests/GitHub.App/ViewModels/PullRequestCreationViewModelTests.cs index 7786be5914..eaa20b48f7 100644 --- a/src/UnitTests/GitHub.App/ViewModels/PullRequestCreationViewModelTests.cs +++ b/src/UnitTests/GitHub.App/ViewModels/PullRequestCreationViewModelTests.cs @@ -1,16 +1,17 @@ -using System.Reactive.Linq; +using System; +using System.Reactive.Linq; using System.Threading.Tasks; -using NSubstitute; -using Xunit; -using UnitTests; +using GitHub.Api; +using GitHub.Factories; using GitHub.Models; -using System; +using GitHub.Primitives; using GitHub.Services; using GitHub.ViewModels; +using NSubstitute; using Octokit; -using GitHub.Api; -using System.ComponentModel; -using Rothko; +using UnitTests; +using Xunit; +using IConnection = GitHub.Models.IConnection; /// /// All the tests in this class are split in subclasses so that when they run @@ -49,9 +50,17 @@ struct TestData public IGitClient GitClient; public IGitService GitService; public INotificationService NotificationService; - public IRepositoryHost RepositoryHost; + public IConnection Connection; public IApiClient ApiClient; public IModelService ModelService; + + public IModelServiceFactory GetModelServiceFactory() + { + var result = Substitute.For(); + result.CreateAsync(Connection).Returns(ModelService); + result.CreateBlocking(Connection).Returns(ModelService); + return result; + } } static TestData PrepareTestData( @@ -64,15 +73,17 @@ static TestData PrepareTestData( var gitService = serviceProvider.GetGitService(); var gitClient = Substitute.For(); var notifications = Substitute.For(); - var host = Substitute.For(); + var connection = Substitute.For(); var api = Substitute.For(); var ms = Substitute.For(); + connection.HostAddress.Returns(HostAddress.Create("https://github.com")); + // this is the local repo instance that is available via TeamExplorerServiceHolder and friends var activeRepo = Substitute.For(); activeRepo.LocalPath.Returns(""); activeRepo.Name.Returns(repoName); - activeRepo.CloneUrl.Returns(new GitHub.Primitives.UriString("http://github.com/" + sourceRepoOwner + "/" + repoName)); + activeRepo.CloneUrl.Returns(new UriString("http://github.com/" + sourceRepoOwner + "/" + repoName)); activeRepo.Owner.Returns(sourceRepoOwner); Repository githubRepoParent = null; @@ -85,10 +96,8 @@ static TestData PrepareTestData( var targetBranch = targetBranchName != targetRepo.DefaultBranch.Name ? new BranchModel(targetBranchName, targetRepo) : targetRepo.DefaultBranch; activeRepo.CurrentBranch.Returns(sourceBranch); - serviceProvider.GetRepositoryHosts().GitHubHost.Returns(host); - host.ApiClient.Returns(api); - host.ModelService.Returns(ms); api.GetRepository(Args.String, Args.String).Returns(Observable.Return(githubRepo)); + ms.ApiClient.Returns(api); // sets up the libgit2sharp repo and branch objects var l2repo = SetupLocalRepoMock(gitClient, gitService, remote, sourceBranchName, sourceBranchIsTracking); @@ -105,7 +114,7 @@ static TestData PrepareTestData( GitClient = gitClient, GitService = gitService, NotificationService = notifications, - RepositoryHost = host, + Connection = connection, ApiClient = api, ModelService = ms }; @@ -117,7 +126,7 @@ public void TargetBranchDisplayNameIncludesRepoOwnerWhenFork() var data = PrepareTestData("octokit.net", "shana", "master", "octokit", "master", "origin", true, true); var prservice = new PullRequestService(data.GitClient, data.GitService, data.ServiceProvider.GetOperatingSystem(), Substitute.For()); prservice.GetPullRequestTemplate(data.ActiveRepo).Returns(Observable.Empty()); - var vm = new PullRequestCreationViewModel(data.RepositoryHost, data.ActiveRepo, prservice, data.NotificationService); + var vm = new PullRequestCreationViewModel(data.Connection, data.GetModelServiceFactory(), data.ActiveRepo, prservice, data.NotificationService); Assert.Equal("octokit/master", vm.TargetBranch.DisplayName); } @@ -151,7 +160,7 @@ public async Task CreatingPRs(int testId, var ms = data.ModelService; var prservice = new PullRequestService(data.GitClient, data.GitService, data.ServiceProvider.GetOperatingSystem(), Substitute.For()); - var vm = new PullRequestCreationViewModel(data.RepositoryHost, data.ActiveRepo, prservice, data.NotificationService); + var vm = new PullRequestCreationViewModel(data.Connection, data.GetModelServiceFactory(), data.ActiveRepo, prservice, data.NotificationService); vm.Initialize(); @@ -186,7 +195,7 @@ public void TemplateIsUsedIfPresent() var prservice = Substitute.For(); prservice.GetPullRequestTemplate(data.ActiveRepo).Returns(Observable.Return("Test PR template")); - var vm = new PullRequestCreationViewModel(data.RepositoryHost, data.ActiveRepo, prservice, data.NotificationService); + var vm = new PullRequestCreationViewModel(data.Connection, data.GetModelServiceFactory(), data.ActiveRepo, prservice, data.NotificationService); vm.Initialize(); Assert.Equal("Test PR template", vm.Description); diff --git a/src/UnitTests/GitHub.App/ViewModels/PullRequestListViewModelTests.cs b/src/UnitTests/GitHub.App/ViewModels/PullRequestListViewModelTests.cs index 25a820eb3f..2d18762409 100644 --- a/src/UnitTests/GitHub.App/ViewModels/PullRequestListViewModelTests.cs +++ b/src/UnitTests/GitHub.App/ViewModels/PullRequestListViewModelTests.cs @@ -10,6 +10,7 @@ using GitHub.Primitives; using NSubstitute; using Xunit; +using GitHub.Factories; namespace UnitTests.GitHub.App.ViewModels { @@ -18,11 +19,12 @@ public class PullRequestListViewModelTests : TestBaseClass [Fact] public void SelectingAssigneeShouldTriggerFilter() { - var repositoryHost = CreateRepositoryHost(); + var connection = Substitute.For(); + var factory = CreateModelServiceFactory(); var repository = Substitute.For(); var settings = CreateSettings(); var browser = Substitute.For(); - var prViewModel = new PullRequestListViewModel(repositoryHost, repository, settings, browser); + var prViewModel = new PullRequestListViewModel(connection, factory, repository, settings, browser); prViewModel.Initialize(null); prViewModel.PullRequests.Received(1).Filter = AnyFilter; @@ -34,11 +36,12 @@ public void SelectingAssigneeShouldTriggerFilter() [Fact] public void ResettingAssigneeToNoneShouldNotTriggerFilter() { - var repositoryHost = CreateRepositoryHost(); + var connection = Substitute.For(); + var factory = CreateModelServiceFactory(); var repository = Substitute.For(); var settings = CreateSettings(); var browser = Substitute.For(); - var prViewModel = new PullRequestListViewModel(repositoryHost, repository, settings, browser); + var prViewModel = new PullRequestListViewModel(connection, factory, repository, settings, browser); prViewModel.Initialize(null); prViewModel.PullRequests.Received(1).Filter = AnyFilter; @@ -56,11 +59,12 @@ public void ResettingAssigneeToNoneShouldNotTriggerFilter() [Fact] public void SelectingAuthorShouldTriggerFilter() { - var repositoryHost = CreateRepositoryHost(); + var connection = Substitute.For(); + var factory = CreateModelServiceFactory(); var repository = Substitute.For(); var settings = CreateSettings(); var browser = Substitute.For(); - var prViewModel = new PullRequestListViewModel(repositoryHost, repository, settings, browser); + var prViewModel = new PullRequestListViewModel(connection, factory, repository, settings, browser); prViewModel.Initialize(null); prViewModel.PullRequests.Received(1).Filter = AnyFilter; @@ -72,11 +76,12 @@ public void SelectingAuthorShouldTriggerFilter() [Fact] public void ResettingAuthorToNoneShouldNotTriggerFilter() { - var repositoryHost = CreateRepositoryHost(); + var connection = Substitute.For(); + var factory = CreateModelServiceFactory(); var repository = Substitute.For(); var settings = CreateSettings(); var browser = Substitute.For(); - var prViewModel = new PullRequestListViewModel(repositoryHost, repository, settings, browser); + var prViewModel = new PullRequestListViewModel(connection, factory, repository, settings, browser); prViewModel.Initialize(null); prViewModel.PullRequests.Received(1).Filter = AnyFilter; @@ -95,11 +100,12 @@ public void ResettingAuthorToNoneShouldNotTriggerFilter() [InlineData("https://github.com/owner/repo", 666, "https://github.com/owner/repo/pull/666")] public void OpenPullRequestOnGitHubShouldOpenBrowser(string cloneUrl, int pullNumber, string expectUrl) { - var repositoryHost = CreateRepositoryHost(); + var connection = Substitute.For(); + var factory = CreateModelServiceFactory(); var repository = Substitute.For(); var settings = CreateSettings(); var browser = Substitute.For(); - var prViewModel = new PullRequestListViewModel(repositoryHost, repository, settings, browser); + var prViewModel = new PullRequestListViewModel(connection, factory, repository, settings, browser); prViewModel.SelectedRepository = Substitute.For(); prViewModel.SelectedRepository.CloneUrl.Returns(new UriString(cloneUrl)); @@ -111,9 +117,8 @@ public void OpenPullRequestOnGitHubShouldOpenBrowser(string cloneUrl, int pullNu Func, bool> AnyFilter => Arg.Any, bool>>(); - IRepositoryHost CreateRepositoryHost() + IModelServiceFactory CreateModelServiceFactory() { - var result = Substitute.For(); var modelService = Substitute.For(); var bitmapSource = Observable.Empty(); @@ -131,8 +136,10 @@ IRepositoryHost CreateRepositoryHost() Arg.Any(), Arg.Any>()) .Returns(pullRequestCollection); - result.ModelService.Returns(modelService); + var result = Substitute.For(); + result.CreateAsync(null).ReturnsForAnyArgs(modelService); + result.CreateBlocking(null).ReturnsForAnyArgs(modelService); return result; } diff --git a/src/UnitTests/GitHub.App/ViewModels/RepositoryCloneViewModelTests.cs b/src/UnitTests/GitHub.App/ViewModels/RepositoryCloneViewModelTests.cs index c7ce31e8f5..4d05602fc0 100644 --- a/src/UnitTests/GitHub.App/ViewModels/RepositoryCloneViewModelTests.cs +++ b/src/UnitTests/GitHub.App/ViewModels/RepositoryCloneViewModelTests.cs @@ -12,14 +12,22 @@ using Xunit; using GitHub.Collections; using NSubstitute.Core; +using GitHub.Factories; +using GitHub.Primitives; public class RepositoryCloneViewModelTests { - static RepositoryCloneViewModel GetVM(IRepositoryHost repositoryHost, IRepositoryCloneService cloneService, - IOperatingSystem os, INotificationService notificationService, IUsageTracker usageTracker) + static RepositoryCloneViewModel GetVM(IModelService modelService, IRepositoryCloneService cloneService, IOperatingSystem os) { + var connection = Substitute.For(); + connection.HostAddress.Returns(HostAddress.GitHubDotComHostAddress); + var modelServiceFactory = Substitute.For(); + modelServiceFactory.CreateAsync(connection).Returns(modelService); + modelServiceFactory.CreateBlocking(connection).Returns(modelService); + var vm = new RepositoryCloneViewModel( - repositoryHost, + connection, + modelServiceFactory, cloneService, os); vm.Initialize(null); @@ -53,17 +61,15 @@ public async Task LoadsRepositories() CreateMockRepositoryModel(), CreateMockRepositoryModel(), }; - var repositoryHost = Substitute.For(); - repositoryHost.ModelService.GetRepositories(Arg.Any>()) + var modelService = Substitute.For(); + modelService.GetRepositories(Arg.Any>()) .Returns(x => SetupRepositories(x, repos.ToObservable())); var cloneService = Substitute.For(); var vm = GetVM( - repositoryHost, + modelService, cloneService, - Substitute.For(), - Substitute.For(), - Substitute.For()); + Substitute.For()); var col = (ITrackingCollection)vm.Repositories; await col.OriginalCompleted; @@ -77,17 +83,15 @@ public class TheIsBusyProperty : TestBaseClass public async Task StartsTrueBecomesFalseWhenCompleted() { var repoSubject = new Subject(); - var repositoryHost = Substitute.For(); - repositoryHost.ModelService.GetRepositories(Arg.Any>()) + var modelService = Substitute.For(); + modelService.GetRepositories(Arg.Any>()) .Returns(x => SetupRepositories(x, repoSubject)); var cloneService = Substitute.For(); var vm = GetVM( - repositoryHost, + modelService, cloneService, - Substitute.For(), - Substitute.For(), - Substitute.For()); + Substitute.For()); var col = (ITrackingCollection)vm.Repositories; Assert.True(vm.IsBusy); @@ -116,17 +120,15 @@ public async Task StartsTrueBecomesFalseWhenCompleted() public void IsFalseWhenLoadingReposFailsImmediately() { var repoSubject = Observable.Throw(new InvalidOperationException("Doh!")); - var repositoryHost = Substitute.For(); - repositoryHost.ModelService.GetRepositories(Arg.Any>()) + var modelService = Substitute.For(); + modelService.GetRepositories(Arg.Any>()) .Returns(x => SetupRepositories(x, repoSubject)); var cloneService = Substitute.For(); var vm = GetVM( - repositoryHost, + modelService, cloneService, - Substitute.For(), - Substitute.For(), - Substitute.For()); + Substitute.For()); Assert.True(vm.LoadingFailed); Assert.False(vm.IsBusy); @@ -139,13 +141,22 @@ public class TheNoRepositoriesFoundProperty : TestBaseClass public void IsTrueInitially() { var repoSubject = new Subject(); - var repositoryHost = Substitute.For(); - repositoryHost.ModelService.GetRepositories(Arg.Any>()) + + var connection = Substitute.For(); + connection.HostAddress.Returns(HostAddress.GitHubDotComHostAddress); + + var modelService = Substitute.For(); + modelService.GetRepositories(Arg.Any>()) .Returns(x => SetupRepositories(x, repoSubject)); + + var modelServiceFactory = Substitute.For(); + modelServiceFactory.CreateBlocking(connection).Returns(modelService); + var cloneService = Substitute.For(); var vm = new RepositoryCloneViewModel( - repositoryHost, + connection, + modelServiceFactory, cloneService, Substitute.For()); @@ -157,16 +168,14 @@ public void IsTrueInitially() public async Task IsFalseWhenLoadingAndCompletedWithRepository() { var repoSubject = new Subject(); - var repositoryHost = Substitute.For(); - repositoryHost.ModelService.GetRepositories(Arg.Any>()) + var modelService = Substitute.For(); + modelService.GetRepositories(Arg.Any>()) .Returns(x => SetupRepositories(x, repoSubject)); var cloneService = Substitute.For(); var vm = GetVM( - repositoryHost, + modelService, cloneService, - Substitute.For(), - Substitute.For(), - Substitute.For()); + Substitute.For()); repoSubject.OnNext(Substitute.For()); @@ -185,16 +194,14 @@ public async Task IsFalseWhenLoadingAndCompletedWithRepository() public void IsFalseWhenFailed() { var repoSubject = new Subject(); - var repositoryHost = Substitute.For(); - repositoryHost.ModelService.GetRepositories(Arg.Any>()) + var modelService = Substitute.For(); + modelService.GetRepositories(Arg.Any>()) .Returns(x => SetupRepositories(x, repoSubject)); var cloneService = Substitute.For(); var vm = GetVM( - repositoryHost, + modelService, cloneService, - Substitute.For(), - Substitute.For(), - Substitute.For()); + Substitute.For()); repoSubject.OnError(new InvalidOperationException()); @@ -205,17 +212,15 @@ public void IsFalseWhenFailed() public void IsTrueWhenLoadingCompleteNotFailedAndNoRepositories() { var repoSubject = new Subject(); - var repositoryHost = Substitute.For(); - repositoryHost.ModelService.GetRepositories(Arg.Any>()) + var modelService = Substitute.For(); + modelService.GetRepositories(Arg.Any>()) .Returns(x => SetupRepositories(x, repoSubject)); var cloneService = Substitute.For(); var vm = GetVM( - repositoryHost, + modelService, cloneService, - Substitute.For(), - Substitute.For(), - Substitute.For()); + Substitute.For()); repoSubject.OnCompleted(); @@ -229,17 +234,15 @@ public class TheFilterTextEnabledProperty : TestBaseClass public void IsTrueInitially() { var repoSubject = new Subject(); - var repositoryHost = Substitute.For(); - repositoryHost.ModelService.GetRepositories(Arg.Any>()) + var modelService = Substitute.For(); + modelService.GetRepositories(Arg.Any>()) .Returns(x => SetupRepositories(x, repoSubject)); var cloneService = Substitute.For(); var vm = GetVM( - repositoryHost, + modelService, cloneService, - Substitute.For(), - Substitute.For(), - Substitute.For()); + Substitute.For()); Assert.False(vm.LoadingFailed); Assert.True(vm.FilterTextIsEnabled); @@ -249,16 +252,14 @@ public void IsTrueInitially() public void IsFalseIfLoadingReposFails() { var repoSubject = new Subject(); - var repositoryHost = Substitute.For(); - repositoryHost.ModelService.GetRepositories(Arg.Any>()) + var modelService = Substitute.For(); + modelService.GetRepositories(Arg.Any>()) .Returns(x => SetupRepositories(x, repoSubject)); var cloneService = Substitute.For(); var vm = GetVM( - repositoryHost, + modelService, cloneService, - Substitute.For(), - Substitute.For(), - Substitute.For()); + Substitute.For()); Assert.False(vm.LoadingFailed); @@ -273,17 +274,15 @@ public void IsFalseIfLoadingReposFails() public void IsFalseWhenLoadingCompleteNotFailedAndNoRepositories() { var repoSubject = new Subject(); - var repositoryHost = Substitute.For(); - repositoryHost.ModelService.GetRepositories(Arg.Any>()) + var modelService = Substitute.For(); + modelService.GetRepositories(Arg.Any>()) .Returns(x => SetupRepositories(x, repoSubject)); var cloneService = Substitute.For(); var vm = GetVM( - repositoryHost, + modelService, cloneService, - Substitute.For(), - Substitute.For(), - Substitute.For()); + Substitute.For()); repoSubject.OnCompleted(); @@ -297,16 +296,14 @@ public class TheLoadingFailedProperty : TestBaseClass public void IsTrueIfLoadingReposFails() { var repoSubject = new Subject(); - var repositoryHost = Substitute.For(); - repositoryHost.ModelService.GetRepositories(Arg.Any>()) + var modelService = Substitute.For(); + modelService.GetRepositories(Arg.Any>()) .Returns(x => SetupRepositories(x, repoSubject)); var cloneService = Substitute.For(); var vm = GetVM( - repositoryHost, + modelService, cloneService, - Substitute.For(), - Substitute.For(), - Substitute.For()); + Substitute.For()); Assert.False(vm.LoadingFailed); @@ -328,8 +325,8 @@ public void IsInvalidWhenDestinationRepositoryExists() repo.Name.Returns("bar"); var data = new[] { repo }.ToObservable(); - var repositoryHost = Substitute.For(); - repositoryHost.ModelService.GetRepositories(Arg.Any>()) + var modelService = Substitute.For(); + modelService.GetRepositories(Arg.Any>()) .Returns(x => SetupRepositories(x, data)); var cloneService = Substitute.For(); @@ -338,11 +335,9 @@ public void IsInvalidWhenDestinationRepositoryExists() os.Directory.Returns(directories); directories.Exists(@"c:\foo\bar").Returns(true); var vm = GetVM( - repositoryHost, + modelService, cloneService, - os, - Substitute.For(), - Substitute.For()); + os); vm.BaseRepositoryPath = @"c:\foo"; vm.SelectedRepository = repo; @@ -356,17 +351,15 @@ public class TheCloneCommand : TestBaseClass [Fact] public void IsEnabledWhenRepositorySelectedAndPathValid() { - var repositoryHost = Substitute.For(); - repositoryHost.ModelService.GetRepositories(Arg.Any>()) + var modelService = Substitute.For(); + modelService.GetRepositories(Arg.Any>()) .Returns(x => SetupRepositories(x, Observable.Empty())); var cloneService = Substitute.For(); var vm = GetVM( - repositoryHost, + modelService, cloneService, - Substitute.For(), - Substitute.For(), - Substitute.For()); + Substitute.For()); Assert.False(vm.CloneCommand.CanExecute(null)); vm.BaseRepositoryPath = @"c:\fake\path"; @@ -378,17 +371,15 @@ public void IsEnabledWhenRepositorySelectedAndPathValid() [Fact] public void IsNotEnabledWhenPathIsNotValid() { - var repositoryHost = Substitute.For(); - repositoryHost.ModelService.GetRepositories(Arg.Any>()) + var modelService = Substitute.For(); + modelService.GetRepositories(Arg.Any>()) .Returns(x => SetupRepositories(x, Observable.Empty())); var cloneService = Substitute.For(); var vm = GetVM( - repositoryHost, + modelService, cloneService, - Substitute.For(), - Substitute.For(), - Substitute.For()); + Substitute.For()); vm.BaseRepositoryPath = @"c:|fake\path"; Assert.False(vm.CloneCommand.CanExecute(null)); diff --git a/src/UnitTests/GitHub.App/ViewModels/RepositoryCreationViewModelTests.cs b/src/UnitTests/GitHub.App/ViewModels/RepositoryCreationViewModelTests.cs index 4cf6246cdf..faa4f31597 100644 --- a/src/UnitTests/GitHub.App/ViewModels/RepositoryCreationViewModelTests.cs +++ b/src/UnitTests/GitHub.App/ViewModels/RepositoryCreationViewModelTests.cs @@ -1,10 +1,14 @@ using System; +using System.Collections.Generic; using System.Linq; using System.Reactive; using System.Reactive.Linq; using System.Threading.Tasks; -using GitHub.Extensions.Reactive; +using GitHub.Extensions; +using GitHub.Factories; using GitHub.Models; +using GitHub.Primitives; +using GitHub.SampleData; using GitHub.Services; using GitHub.ViewModels; using NSubstitute; @@ -12,11 +16,7 @@ using Rothko; using UnitTests; using Xunit; -using GitHub.Extensions; -using GitHub.SampleData; -using System.Collections.Generic; -using System.Collections.ObjectModel; -using System.Reactive.Subjects; +using IConnection = GitHub.Models.IConnection; public class RepositoryCreationViewModelTests { @@ -24,18 +24,29 @@ public class RepositoryCreationViewModelTests static IRepositoryCreationViewModel GetMeAViewModel( IServiceProvider provider = null, - IRepositoryCreationService creationService = null) + IRepositoryCreationService creationService = null, + IModelService modelService = null) { if (provider == null) provider = Substitutes.ServiceProvider; - var repositoryHost = provider.GetRepositoryHosts().GitHubHost; var os = provider.GetOperatingSystem(); creationService = creationService ?? provider.GetRepositoryCreationService(); var avatarProvider = provider.GetAvatarProvider(); var connection = provider.GetConnection(); + connection.HostAddress.Returns(HostAddress.GitHubDotComHostAddress); var usageTracker = Substitute.For(); + modelService = modelService ?? Substitute.For(); + var factory = GetMeAFactory(modelService); + + return new RepositoryCreationViewModel(connection, factory, os, creationService, usageTracker); + } - return new RepositoryCreationViewModel(repositoryHost, os, creationService, usageTracker); + static IModelServiceFactory GetMeAFactory(IModelService ms) + { + var result = Substitute.For(); + result.CreateAsync(null).ReturnsForAnyArgs(ms); + result.CreateBlocking(null).ReturnsForAnyArgs(ms); + return result; } public class TheSafeRepositoryNameProperty : TestBaseClass @@ -330,10 +341,13 @@ public class TheAccountsProperty : TestBaseClass public void IsPopulatedByTheRepositoryHost() { var accounts = new List { new AccountDesigner(), new AccountDesigner() }; - var repositoryHost = Substitute.For(); - repositoryHost.ModelService.GetAccounts().Returns(Observable.Return(accounts)); + var connection = Substitute.For(); + var modelService = Substitute.For(); + connection.HostAddress.Returns(HostAddress.GitHubDotComHostAddress); + modelService.GetAccounts().Returns(Observable.Return(accounts)); var vm = new RepositoryCreationViewModel( - repositoryHost, + connection, + GetMeAFactory(modelService), Substitute.For(), Substitute.For(), Substitute.For()); @@ -356,13 +370,11 @@ public async void IsPopulatedByTheApiAndSortedWithRecommendedFirst() "WordPress" }.Select(GitIgnoreItem.Create); var provider = Substitutes.ServiceProvider; - var hosts = provider.GetRepositoryHosts(); - var host = hosts.GitHubHost; - hosts.LookupHost(Args.HostAddress).Returns(host); - host.ModelService + var modelService = Substitute.For(); + modelService .GetGitIgnoreTemplates() .Returns(gitIgnoreTemplates.ToObservable()); - var vm = GetMeAViewModel(provider); + var vm = GetMeAViewModel(provider, modelService: modelService); // this is how long the default collection waits to process about 5 things with the default UI settings await Task.Delay(100); @@ -396,13 +408,11 @@ public async void IsPopulatedByTheModelService() new LicenseItem("artistic-2.0", "Artistic License 2.0") }; var provider = Substitutes.ServiceProvider; - var hosts = provider.GetRepositoryHosts(); - var host = hosts.GitHubHost; - hosts.LookupHost(Args.HostAddress).Returns(host); - host.ModelService + var modelService = Substitute.For(); + modelService .GetLicenses() .Returns(licenses.ToObservable()); - var vm = GetMeAViewModel(provider); + var vm = GetMeAViewModel(provider, modelService: modelService); // this is how long the default collection waits to process about 5 things with the default UI settings await Task.Delay(100); @@ -437,13 +447,11 @@ public async void DefaultsToVisualStudio() GitIgnoreItem.Create("VisualStudio"), }; var provider = Substitutes.ServiceProvider; - var hosts = provider.GetRepositoryHosts(); - var host = hosts.GitHubHost; - hosts.LookupHost(Args.HostAddress).Returns(host); - host.ModelService + var modelService = Substitute.For(); + modelService .GetGitIgnoreTemplates() .Returns(gitignores.ToObservable()); - var vm = GetMeAViewModel(provider); + var vm = GetMeAViewModel(provider, modelService: modelService); // this is how long the default collection waits to process about 5 things with the default UI settings await Task.Delay(100); @@ -461,13 +469,11 @@ public void DefaultsToNoneIfVisualStudioIsMissingSomehow() GitIgnoreItem.Create("Node"), }; var provider = Substitutes.ServiceProvider; - var hosts = provider.GetRepositoryHosts(); - var host = hosts.GitHubHost; - hosts.LookupHost(Args.HostAddress).Returns(host); - host.ModelService + var modelService = Substitute.For(); + modelService .GetGitIgnoreTemplates() .Returns(gitignores.ToObservable()); - var vm = GetMeAViewModel(provider); + var vm = GetMeAViewModel(provider, modelService: modelService); Assert.Equal("None", vm.SelectedGitIgnoreTemplate.Name); } @@ -502,11 +508,9 @@ public void CreatesARepositoryUsingTheCreationService() var provider = Substitutes.GetServiceProvider(creationService: creationService); var account = Substitute.For(); - var hosts = provider.GetRepositoryHosts(); - var host = hosts.GitHubHost; - hosts.LookupHost(Args.HostAddress).Returns(host); - host.ModelService.GetAccounts().Returns(Observable.Return(new List { account })); - var vm = GetMeAViewModel(provider); + var modelService = Substitute.For(); + modelService.GetAccounts().Returns(Observable.Return(new List { account })); + var vm = GetMeAViewModel(provider, modelService: modelService); vm.RepositoryName = "Krieger"; vm.BaseRepositoryPath = @"c:\dev"; vm.SelectedAccount = account; @@ -533,11 +537,9 @@ public void SetsAutoInitToTrueWhenLicenseSelected() var creationService = Substitutes.RepositoryCreationService; var provider = Substitutes.GetServiceProvider(creationService: creationService); var account = Substitute.For(); - var hosts = provider.GetRepositoryHosts(); - var host = hosts.GitHubHost; - hosts.LookupHost(Args.HostAddress).Returns(host); - host.ModelService.GetAccounts().Returns(Observable.Return(new List { account })); - var vm = GetMeAViewModel(provider); + var modelService = Substitute.For(); + modelService.GetAccounts().Returns(Observable.Return(new List { account })); + var vm = GetMeAViewModel(provider, modelService: modelService); vm.RepositoryName = "Krieger"; vm.BaseRepositoryPath = @"c:\dev"; vm.SelectedAccount = account; @@ -565,11 +567,9 @@ public void SetsAutoInitToTrueWhenGitIgnore() var creationService = Substitutes.RepositoryCreationService; var provider = Substitutes.GetServiceProvider(creationService: creationService); var account = Substitute.For(); - var hosts = provider.GetRepositoryHosts(); - var host = hosts.GitHubHost; - hosts.LookupHost(Args.HostAddress).Returns(host); - host.ModelService.GetAccounts().Returns(Observable.Return(new List { account })); - var vm = GetMeAViewModel(provider); + var modelService = Substitute.For(); + modelService.GetAccounts().Returns(Observable.Return(new List { account })); + var vm = GetMeAViewModel(provider, modelService: modelService); vm.RepositoryName = "Krieger"; vm.BaseRepositoryPath = @"c:\dev"; vm.SelectedAccount = account; diff --git a/src/UnitTests/GitHub.App/ViewModels/RepositoryPublishViewModelTests.cs b/src/UnitTests/GitHub.App/ViewModels/RepositoryPublishViewModelTests.cs index 343f4bdc63..f4593d4eed 100644 --- a/src/UnitTests/GitHub.App/ViewModels/RepositoryPublishViewModelTests.cs +++ b/src/UnitTests/GitHub.App/ViewModels/RepositoryPublishViewModelTests.cs @@ -13,6 +13,7 @@ using System.Collections.ObjectModel; using System.Linq; using GitHub.Extensions; +using GitHub.Factories; public class RepositoryPublishViewModelTests { @@ -20,59 +21,46 @@ public static class Helpers { public static IRepositoryPublishViewModel GetViewModel(IRepositoryPublishService service = null) { - return GetViewModel(null, service, null); + return GetViewModel(service); } public static IRepositoryPublishViewModel GetViewModel( - IRepositoryHosts hosts = null, IRepositoryPublishService service = null, INotificationService notificationService = null, - IConnectionManager connectionManager = null) + IConnectionManager connectionManager = null, + IModelServiceFactory factory = null) { - hosts = hosts ?? Substitutes.RepositoryHosts; service = service ?? Substitute.For(); notificationService = notificationService ?? Substitute.For(); connectionManager = connectionManager ?? Substitutes.ConnectionManager; - return new RepositoryPublishViewModel(hosts, service, notificationService, connectionManager, - Substitute.For()); + factory = factory ?? Substitute.For(); + + return new RepositoryPublishViewModel(service, notificationService, connectionManager, + factory, Substitute.For()); } - public static void SetupConnections(IRepositoryHosts hosts, IConnectionManager cm, - List adds, List conns, List hsts, - string uri) + public static void SetupConnections(List adds, List conns, string uri) { var add = HostAddress.Create(new Uri(uri)); - var host = Substitute.For(); var conn = Substitute.For(); - host.Address.Returns(add); conn.HostAddress.Returns(add); adds.Add(add); - hsts.Add(host); conns.Add(conn); - - if (add.IsGitHubDotCom()) - hosts.GitHubHost.Returns(host); - else - hosts.EnterpriseHost.Returns(host); - hosts.LookupHost(Arg.Is(add)).Returns(host); } public static IRepositoryPublishViewModel SetupConnectionsAndViewModel( - IRepositoryHosts hosts = null, IRepositoryPublishService service = null, INotificationService notificationService = null, IConnectionManager cm = null, string uri = GitHubUrls.GitHub) { cm = cm ?? Substitutes.ConnectionManager; - hosts = hosts ?? Substitute.For(); var adds = new List(); - var hsts = new List(); var conns = new List(); - SetupConnections(hosts, cm, adds, conns, hsts, uri); - hsts[0].ModelService.GetAccounts().Returns(Observable.Return(new List())); + SetupConnections(adds, conns, uri); + //hsts[0].ModelService.GetAccounts().Returns(Observable.Return(new List())); cm.Connections.Returns(new ObservableCollectionEx(conns)); - return GetViewModel(hosts, service, notificationService, cm); + return GetViewModel(service, notificationService, cm); } public static string[] GetArgs(params string[] args) @@ -89,35 +77,24 @@ public static string[] GetArgs(params string[] args) public class TheConnectionsProperty : TestBaseClass { [Theory] - [InlineData(GitHubUrls.GitHub, "https://github.enterprise" )] + [InlineData(GitHubUrls.GitHub, "https://github.enterprise")] [InlineData("https://github.enterprise", null)] [InlineData(GitHubUrls.GitHub, null)] - public void ConnectionsMatchHosts(string arg1, string arg2) + public void ConnectionsMatchConnectionManager(string arg1, string arg2) { var args = Helpers.GetArgs(arg1, arg2); var cm = Substitutes.ConnectionManager; - var hosts = Substitute.For(); var adds = new List(); - var hsts = new List(); var conns = new List(); foreach (var uri in args) - Helpers.SetupConnections(hosts, cm, adds, conns, hsts, uri); - - foreach(var host in hsts) - host.ModelService.GetAccounts().Returns(Observable.Return(new List())); + Helpers.SetupConnections(adds, conns, uri); cm.Connections.Returns(new ObservableCollectionEx(conns)); - var vm = Helpers.GetViewModel(hosts: hosts, connectionManager: cm); + var vm = Helpers.GetViewModel(connectionManager: cm); - var connections = vm.Connections; - - Assert.Equal(args.Length, connections.Count); - for (int i = 0; i < conns.Count; i++) - { - Assert.Same(hsts[i], hosts.LookupHost(conns[i].HostAddress)); - } + Assert.Equal(conns, vm.Connections); } } @@ -131,22 +108,16 @@ public void DefaultsToGitHub(string arg1, string arg2) var args = Helpers.GetArgs(arg1, arg2); var cm = Substitutes.ConnectionManager; - var hosts = Substitute.For(); var adds = new List(); - var hsts = new List(); var conns = new List(); foreach (var uri in args) - Helpers.SetupConnections(hosts, cm, adds, conns, hsts, uri); - - foreach (var host in hsts) - host.ModelService.GetAccounts().Returns(Observable.Return(new List())); + Helpers.SetupConnections(adds, conns, uri); cm.Connections.Returns(new ObservableCollectionEx(conns)); - var vm = Helpers.GetViewModel(hosts, connectionManager: cm); + var vm = Helpers.GetViewModel(connectionManager: cm); Assert.Same(adds.First(x => x.IsGitHubDotCom()), vm.SelectedConnection.HostAddress); Assert.Same(conns.First(x => x.HostAddress.IsGitHubDotCom()), vm.SelectedConnection); - Assert.Same(hsts.First(x => x.Address.IsGitHubDotCom()), hosts.LookupHost(vm.SelectedConnection.HostAddress)); } } @@ -156,21 +127,25 @@ public class TheAccountsProperty : TestBaseClass public void IsPopulatedByTheAccountsForTheSelectedHost() { var cm = Substitutes.ConnectionManager; - var hosts = Substitute.For(); var adds = new List(); - var hsts = new List(); var conns = new List(); - Helpers.SetupConnections(hosts, cm, adds, conns, hsts, GitHubUrls.GitHub); - Helpers.SetupConnections(hosts, cm, adds, conns, hsts, "https://github.enterprise"); + Helpers.SetupConnections(adds, conns, GitHubUrls.GitHub); + Helpers.SetupConnections(adds, conns, "https://github.enterprise"); var gitHubAccounts = new List { Substitute.For(), Substitute.For() }; var enterpriseAccounts = new List { Substitute.For() }; - hsts.First(x => x.Address.IsGitHubDotCom()).ModelService.GetAccounts().Returns(Observable.Return(gitHubAccounts)); - hsts.First(x => !x.Address.IsGitHubDotCom()).ModelService.GetAccounts().Returns(Observable.Return(enterpriseAccounts)); + var gitHubModelService = Substitute.For(); + var enterpriseModelService = Substitute.For(); + gitHubModelService.GetAccounts().Returns(Observable.Return(gitHubAccounts)); + enterpriseModelService.GetAccounts().Returns(Observable.Return(enterpriseAccounts)); + + var factory = Substitute.For(); + factory.CreateAsync(conns[0]).Returns(gitHubModelService); + factory.CreateAsync(conns[1]).Returns(enterpriseModelService); cm.Connections.Returns(new ObservableCollectionEx(conns)); - var vm = Helpers.GetViewModel(hosts, connectionManager: cm); + var vm = Helpers.GetViewModel(connectionManager: cm, factory: factory); Assert.Equal(2, vm.Accounts.Count); Assert.Same(gitHubAccounts[0], vm.SelectedAccount); @@ -188,8 +163,7 @@ public class TheSafeRepositoryNameProperty : TestBaseClass public void IsTheSameAsTheRepositoryNameWhenTheInputIsSafe() { var cm = Substitutes.ConnectionManager; - var hosts = Substitute.For(); - var vm = Helpers.SetupConnectionsAndViewModel(hosts, cm: cm); + var vm = Helpers.SetupConnectionsAndViewModel(cm: cm); vm.RepositoryName = "this-is-bad"; @@ -200,8 +174,7 @@ public void IsTheSameAsTheRepositoryNameWhenTheInputIsSafe() public void IsConvertedWhenTheRepositoryNameIsNotSafe() { var cm = Substitutes.ConnectionManager; - var hosts = Substitute.For(); - var vm = Helpers.SetupConnectionsAndViewModel(hosts, cm: cm); + var vm = Helpers.SetupConnectionsAndViewModel(cm: cm); vm.RepositoryName = "this is bad"; @@ -212,8 +185,7 @@ public void IsConvertedWhenTheRepositoryNameIsNotSafe() public void IsNullWhenRepositoryNameIsNull() { var cm = Substitutes.ConnectionManager; - var hosts = Substitute.For(); - var vm = Helpers.SetupConnectionsAndViewModel(hosts, cm: cm); + var vm = Helpers.SetupConnectionsAndViewModel(cm: cm); Assert.Null(vm.SafeRepositoryName); vm.RepositoryName = "not-null"; @@ -229,8 +201,7 @@ public class TheRepositoryNameValidatorProperty : TestBaseClass public void IsFalseWhenRepoNameEmpty() { var cm = Substitutes.ConnectionManager; - var hosts = Substitute.For(); - var vm = Helpers.SetupConnectionsAndViewModel(hosts, cm: cm); + var vm = Helpers.SetupConnectionsAndViewModel(cm: cm); vm.RepositoryName = ""; @@ -242,8 +213,7 @@ public void IsFalseWhenRepoNameEmpty() public void IsFalseWhenAfterBeingTrue() { var cm = Substitutes.ConnectionManager; - var hosts = Substitute.For(); - var vm = Helpers.SetupConnectionsAndViewModel(hosts, cm: cm); + var vm = Helpers.SetupConnectionsAndViewModel(cm: cm); vm.RepositoryName = "repo"; @@ -261,8 +231,7 @@ public void IsFalseWhenAfterBeingTrue() public void IsTrueWhenRepositoryNameIsValid() { var cm = Substitutes.ConnectionManager; - var hosts = Substitute.For(); - var vm = Helpers.SetupConnectionsAndViewModel(hosts, cm: cm); + var vm = Helpers.SetupConnectionsAndViewModel(cm: cm); vm.RepositoryName = "thisisfine"; @@ -277,8 +246,7 @@ public class TheSafeRepositoryNameWarningValidatorProperty : TestBaseClass public void IsTrueWhenRepoNameIsSafe() { var cm = Substitutes.ConnectionManager; - var hosts = Substitute.For(); - var vm = Helpers.SetupConnectionsAndViewModel(hosts, cm: cm); + var vm = Helpers.SetupConnectionsAndViewModel(cm: cm); vm.RepositoryName = "this-is-bad"; @@ -289,8 +257,7 @@ public void IsTrueWhenRepoNameIsSafe() public void IsFalseWhenRepoNameIsNotSafe() { var cm = Substitutes.ConnectionManager; - var hosts = Substitute.For(); - var vm = Helpers.SetupConnectionsAndViewModel(hosts, cm: cm); + var vm = Helpers.SetupConnectionsAndViewModel(cm: cm); vm.RepositoryName = "this is bad"; @@ -302,8 +269,7 @@ public void IsFalseWhenRepoNameIsNotSafe() public void ResetsSafeNameValidator() { var cm = Substitutes.ConnectionManager; - var hosts = Substitute.For(); - var vm = Helpers.SetupConnectionsAndViewModel(hosts, cm: cm); + var vm = Helpers.SetupConnectionsAndViewModel(cm: cm); vm.RepositoryName = "this"; Assert.True(vm.SafeRepositoryNameWarningValidator.ValidationResult.IsValid); @@ -324,13 +290,12 @@ public class ThePublishRepositoryCommand : TestBaseClass public async Task RepositoryExistsCallsNotificationServiceWithError() { var cm = Substitutes.ConnectionManager; - var hosts = Substitute.For(); var notificationService = Substitute.For(); var repositoryPublishService = Substitute.For(); repositoryPublishService.PublishRepository(Args.NewRepository, Args.Account, Args.ApiClient) .Returns(Observable.Throw(new Octokit.RepositoryExistsException("repo-name", new Octokit.ApiValidationException()))); - var vm = Helpers.SetupConnectionsAndViewModel(hosts, repositoryPublishService, notificationService, cm); + var vm = Helpers.SetupConnectionsAndViewModel(repositoryPublishService, notificationService, cm); vm.RepositoryName = "repo-name"; await vm.PublishRepository.ExecuteAsync().Catch(Observable.Return(ProgressState.Fail)); @@ -346,15 +311,10 @@ public async Task ResetsWhenSwitchingHosts() var args = Helpers.GetArgs(GitHubUrls.GitHub, "https://github.enterprise"); var cm = Substitutes.ConnectionManager; - var hosts = Substitute.For(); var adds = new List(); - var hsts = new List(); var conns = new List(); foreach (var uri in args) - Helpers.SetupConnections(hosts, cm, adds, conns, hsts, uri); - - foreach (var host in hsts) - host.ModelService.GetAccounts().Returns(Observable.Return(new List())); + Helpers.SetupConnections(adds, conns, uri); cm.Connections.Returns(new ObservableCollectionEx(conns)); @@ -363,7 +323,7 @@ public async Task ResetsWhenSwitchingHosts() var repositoryPublishService = Substitute.For(); repositoryPublishService.PublishRepository(Args.NewRepository, Args.Account, Args.ApiClient) .Returns(Observable.Throw(new Octokit.RepositoryExistsException("repo-name", new Octokit.ApiValidationException()))); - var vm = Helpers.GetViewModel(hosts, repositoryPublishService, notificationService, cm); + var vm = Helpers.GetViewModel(repositoryPublishService, notificationService, cm); vm.RepositoryName = "repo-name"; @@ -384,14 +344,11 @@ public async Task ResetsWhenSwitchingHosts() public async Task ResetsWhenSwitchingAccounts() { var cm = Substitutes.ConnectionManager; - var hosts = Substitute.For(); var adds = new List(); - var hsts = new List(); var conns = new List(); - Helpers.SetupConnections(hosts, cm, adds, conns, hsts, GitHubUrls.GitHub); + Helpers.SetupConnections(adds, conns, GitHubUrls.GitHub); var accounts = new List { Substitute.For(), Substitute.For() }; - hsts[0].ModelService.GetAccounts().Returns(Observable.Return(accounts)); cm.Connections.Returns(new ObservableCollectionEx(conns)); @@ -400,7 +357,7 @@ public async Task ResetsWhenSwitchingAccounts() var repositoryPublishService = Substitute.For(); repositoryPublishService.PublishRepository(Args.NewRepository, Args.Account, Args.ApiClient) .Returns(Observable.Throw(new Octokit.RepositoryExistsException("repo-name", new Octokit.ApiValidationException()))); - var vm = Helpers.GetViewModel(hosts, repositoryPublishService, notificationService, cm); + var vm = Helpers.GetViewModel(repositoryPublishService, notificationService, cm); vm.RepositoryName = "repo-name"; diff --git a/src/UnitTests/GitHub.VisualStudio/UI/Views/GitHubPaneViewModelTests.cs b/src/UnitTests/GitHub.VisualStudio/UI/Views/GitHubPaneViewModelTests.cs index 77bae612fe..73318ff368 100644 --- a/src/UnitTests/GitHub.VisualStudio/UI/Views/GitHubPaneViewModelTests.cs +++ b/src/UnitTests/GitHub.VisualStudio/UI/Views/GitHubPaneViewModelTests.cs @@ -1,22 +1,17 @@ -using GitHub.Api; +using System; +using System.Collections.Generic; +using System.ComponentModel.Design; +using System.Reactive.Linq; using GitHub.Exports; +using GitHub.Extensions; using GitHub.Models; using GitHub.Primitives; using GitHub.Services; using GitHub.UI; -using GitHub.VisualStudio; using GitHub.VisualStudio.UI.Views; using NSubstitute; -using System; -using System.Collections.Generic; -using System.Collections.ObjectModel; -using System.ComponentModel.Composition; -using System.ComponentModel.Design; -using System.Linq; -using System.Reactive.Linq; using UnitTests; using Xunit; -using GitHub.Extensions; public class GitHubPaneViewModelTests : TestBaseClass { @@ -28,9 +23,6 @@ public class GitHubPaneViewModelTests : TestBaseClass public GitHubPaneViewModelTests() { - var repositoryHosts = Substitutes.RepositoryHosts; - repositoryHosts.IsLoggedInToAnyHost.Returns(true); - var teamExplorerServiceHolder = Substitute.For(); var activeRepo = Substitute.For(); activeRepo.CloneUrl.Returns(new UriString("https://github.com/foo/foo")); @@ -49,10 +41,7 @@ public GitHubPaneViewModelTests() connectionManager.Connections.Returns(new ObservableCollectionEx(new[] { connection })); - - var host = Substitute.For(); - host.IsLoggedIn.Returns(true); - repositoryHosts.LookupHost(connectionHostAddress).Returns(host); + connection.IsLoggedIn.Returns(true); serviceProvider = Substitutes.GetFullyMockedServiceProvider(); menuCommandService = new FakeMenuCommandService(); diff --git a/src/UnitTests/Substitutes.cs b/src/UnitTests/Substitutes.cs index 0062bb3112..5004d3f1cb 100644 --- a/src/UnitTests/Substitutes.cs +++ b/src/UnitTests/Substitutes.cs @@ -62,7 +62,6 @@ public static IOperatingSystem OperatingSystem public static IRepositoryCreationService RepositoryCreationService { get { return Substitute.For(); } } public static IRepositoryCloneService RepositoryCloneService { get { return Substitute.For(); } } - public static IRepositoryHosts RepositoryHosts { get { return Substitute.For(); } } public static IConnection Connection { get { return Substitute.For(); } } public static IConnection NewConnection { get { return Substitute.For(); } } public static IConnectionManager ConnectionManager { get { return Substitute.For(); } } @@ -126,7 +125,6 @@ public static IGitHubServiceProvider GetServiceProvider( ret.GetService(typeof(IOperatingSystem)).Returns(os); ret.GetService(typeof(IRepositoryCloneService)).Returns(clone); ret.GetService(typeof(IRepositoryCreationService)).Returns(create); - ret.GetService(typeof(IRepositoryHosts)).Returns(RepositoryHosts); ret.GetService(typeof(IExportFactoryProvider)).Returns(ExportFactoryProvider); ret.GetService(typeof(IUIFactory)).Returns(UIFactory); ret.GetService(typeof(IConnection)).Returns(Connection); @@ -176,11 +174,6 @@ public static IRepositoryCreationService GetRepositoryCreationService(this IServ return provider.GetService(typeof(IRepositoryCreationService)) as IRepositoryCreationService; } - public static IRepositoryHosts GetRepositoryHosts(this IServiceProvider provider) - { - return provider.GetService(typeof(IRepositoryHosts)) as IRepositoryHosts; - } - public static IExportFactoryProvider GetExportFactoryProvider(this IServiceProvider provider) { return provider.GetService(typeof(IExportFactoryProvider)) as IExportFactoryProvider; From a57a75967201eb8e755e332907be0eeb2c646842 Mon Sep 17 00:00:00 2001 From: Steven Kirk Date: Fri, 27 Oct 2017 10:02:23 +0200 Subject: [PATCH 11/16] Finished migrating tests (I think). --- .../ViewModels/LoginControlViewModel.cs | 50 ++++++++----------- .../ViewModels/ILoginControlViewModel.cs | 12 ++--- .../UI/Views/GitHubPaneViewModel.cs | 3 +- .../ViewModels/LoginControlViewModelTests.cs | 30 +++++++++++ 4 files changed, 58 insertions(+), 37 deletions(-) diff --git a/src/GitHub.App/ViewModels/LoginControlViewModel.cs b/src/GitHub.App/ViewModels/LoginControlViewModel.cs index 03092c9674..7ebce6cbef 100644 --- a/src/GitHub.App/ViewModels/LoginControlViewModel.cs +++ b/src/GitHub.App/ViewModels/LoginControlViewModel.cs @@ -7,6 +7,7 @@ using GitHub.Exports; using GitHub.Extensions.Reactive; using GitHub.Models; +using GitHub.Primitives; using GitHub.Services; using ReactiveUI; @@ -33,20 +34,8 @@ public LoginControlViewModel( (x, y) => x.Value || y.Value ).ToProperty(this, vm => vm.IsLoginInProgress); - ////loginMode = this.WhenAny( - //// x => x.RepositoryHosts.GitHubHost.IsLoggedIn, - //// x => x.RepositoryHosts.EnterpriseHost.IsLoggedIn, - //// (x, y) => - //// { - //// var canLogInToGitHub = x.Value == false; - //// var canLogInToEnterprise = y.Value == false; - - //// return canLogInToGitHub && canLogInToEnterprise ? LoginMode.DotComOrEnterprise - //// : canLogInToGitHub ? LoginMode.DotComOnly - //// : canLogInToEnterprise ? LoginMode.EnterpriseOnly - //// : LoginMode.None; - - //// }).ToProperty(this, x => x.LoginMode); + UpdateLoginMode(); + connectionManager.Connections.CollectionChanged += (_, __) => UpdateLoginMode(); AuthenticationResults = Observable.Merge( loginToGitHubViewModel.Login, @@ -74,8 +63,12 @@ public IConnectionManager ConnectionManager set { this.RaiseAndSetIfChanged(ref connectionManager, value); } } - readonly ObservableAsPropertyHelper loginMode = null; - public LoginMode LoginMode { get { return loginMode.Value; } } + LoginMode loginMode; + public LoginMode LoginMode + { + get { return loginMode; } + private set { this.RaiseAndSetIfChanged(ref loginMode, value); } + } readonly ObservableAsPropertyHelper isLoginInProgress; public bool IsLoginInProgress { get { return isLoginInProgress.Value; } } @@ -86,21 +79,18 @@ public override IObservable Done { get { return AuthenticationResults.Where(x => x == AuthenticationResult.Success).SelectUnit(); } } - } - public enum LoginTarget - { - None = 0, - DotCom = 1, - Enterprise = 2, - } + void UpdateLoginMode() + { + var result = LoginMode.DotComOrEnterprise; - public enum VisualState - { - None = 0, - DotCom = 1, - Enterprise = 2, - DotComOnly = 3, - EnterpriseOnly = 4 + foreach (var connection in connectionManager.Connections) + { + result &= ~((connection.HostAddress == HostAddress.GitHubDotComHostAddress) ? + LoginMode.DotComOnly : LoginMode.EnterpriseOnly); + } + + LoginMode = result; + } } } diff --git a/src/GitHub.Exports.Reactive/ViewModels/ILoginControlViewModel.cs b/src/GitHub.Exports.Reactive/ViewModels/ILoginControlViewModel.cs index f23c583826..25b7ba36d7 100644 --- a/src/GitHub.Exports.Reactive/ViewModels/ILoginControlViewModel.cs +++ b/src/GitHub.Exports.Reactive/ViewModels/ILoginControlViewModel.cs @@ -1,15 +1,15 @@ -using System.ComponentModel; -using System.Reactive; +using System; using ReactiveUI; namespace GitHub.ViewModels { + [Flags] public enum LoginMode { - None = 0, - DotComOrEnterprise, - DotComOnly = 3, - EnterpriseOnly = 4, + None = 0x00, + DotComOnly = 0x01, + EnterpriseOnly = 0x02, + DotComOrEnterprise = 0x03, } /// diff --git a/src/GitHub.VisualStudio/UI/Views/GitHubPaneViewModel.cs b/src/GitHub.VisualStudio/UI/Views/GitHubPaneViewModel.cs index 44a006f60c..8c451f6df8 100644 --- a/src/GitHub.VisualStudio/UI/Views/GitHubPaneViewModel.cs +++ b/src/GitHub.VisualStudio/UI/Views/GitHubPaneViewModel.cs @@ -147,7 +147,8 @@ public override void Initialize(IServiceProvider serviceProvider) base.Initialize(serviceProvider); - ////hosts.WhenAnyValue(x => x.IsLoggedInToAnyHost).Subscribe(_ => LoadDefault()); + connectionManager.Connections.CollectionChanged += (_, __) => LoadDefault(); + LoadDefault(); } public void Initialize(ViewWithData data = null) diff --git a/src/UnitTests/GitHub.App/ViewModels/LoginControlViewModelTests.cs b/src/UnitTests/GitHub.App/ViewModels/LoginControlViewModelTests.cs index b8fd39d24b..09b52e3284 100644 --- a/src/UnitTests/GitHub.App/ViewModels/LoginControlViewModelTests.cs +++ b/src/UnitTests/GitHub.App/ViewModels/LoginControlViewModelTests.cs @@ -2,7 +2,9 @@ using System.Reactive.Linq; using System.Threading.Tasks; using GitHub.Authentication; +using GitHub.Extensions; using GitHub.Models; +using GitHub.Primitives; using GitHub.Services; using GitHub.ViewModels; using NSubstitute; @@ -80,6 +82,34 @@ public async Task AllowsLoginFromEnterpriseAfterGitHubLoginHasFailed() Assert.True(success); } + + [Fact] + public void LoginModeTracksAvailableConnections() + { + var connectionManager = Substitute.For(); + var connections = new ObservableCollectionEx(); + var gitHubLogin = Substitute.For(); + var enterpriseLogin = Substitute.For(); + var gitHubConnection = Substitute.For(); + var enterpriseConnection = Substitute.For(); + + connectionManager.Connections.Returns(connections); + gitHubConnection.HostAddress.Returns(HostAddress.GitHubDotComHostAddress); + enterpriseConnection.HostAddress.Returns(HostAddress.Create("https://enterprise.url")); + + var loginViewModel = new LoginControlViewModel(connectionManager, gitHubLogin, enterpriseLogin); + + Assert.Equal(LoginMode.DotComOrEnterprise, loginViewModel.LoginMode); + + connections.Add(enterpriseConnection); + Assert.Equal(LoginMode.DotComOnly, loginViewModel.LoginMode); + + connections.Add(gitHubConnection); + Assert.Equal(LoginMode.None, loginViewModel.LoginMode); + + connections.RemoveAt(0); + Assert.Equal(LoginMode.EnterpriseOnly, loginViewModel.LoginMode); + } } } \ No newline at end of file From 95cd0b103628a123d403fe1c0e0ab195494bdb18 Mon Sep 17 00:00:00 2001 From: Steven Kirk Date: Fri, 27 Oct 2017 10:59:55 +0200 Subject: [PATCH 12/16] Added IModelServiceFactory implemenation. --- .../Factories/ModelServiceFactory.cs | 64 +++++++++++++++++++ src/GitHub.App/GitHub.App.csproj | 1 + src/UnitTests/UnitTests.csproj | 2 - 3 files changed, 65 insertions(+), 2 deletions(-) create mode 100644 src/GitHub.App/Factories/ModelServiceFactory.cs diff --git a/src/GitHub.App/Factories/ModelServiceFactory.cs b/src/GitHub.App/Factories/ModelServiceFactory.cs new file mode 100644 index 0000000000..84b4491c8c --- /dev/null +++ b/src/GitHub.App/Factories/ModelServiceFactory.cs @@ -0,0 +1,64 @@ +using System; +using System.Collections.Generic; +using System.ComponentModel.Composition; +using System.Threading; +using System.Threading.Tasks; +using GitHub.Models; +using GitHub.Services; +using Microsoft.VisualStudio.Shell; + +namespace GitHub.Factories +{ + [Export(typeof(IModelServiceFactory))] + [PartCreationPolicy(CreationPolicy.Shared)] + public sealed class ModelServiceFactory : IModelServiceFactory, IDisposable + { + readonly IApiClientFactory apiClientFactory; + readonly IHostCacheFactory hostCacheFactory; + readonly IAvatarProvider avatarProvider; + readonly Dictionary cache = new Dictionary(); + readonly SemaphoreSlim cacheLock = new SemaphoreSlim(1); + + [ImportingConstructor] + public ModelServiceFactory( + IApiClientFactory apiClientFactory, + IHostCacheFactory hostCacheFactory, + IAvatarProvider avatarProvider) + { + this.apiClientFactory = apiClientFactory; + this.hostCacheFactory = hostCacheFactory; + this.avatarProvider = avatarProvider; + } + + public async Task CreateAsync(IConnection connection) + { + ModelService result; + + await cacheLock.WaitAsync(); + + try + { + if (!cache.TryGetValue(connection, out result)) + { + result = new ModelService( + await apiClientFactory.Create(connection.HostAddress), + await hostCacheFactory.Create(connection.HostAddress), + avatarProvider); + } + } + finally + { + cacheLock.Release(); + } + + return result; + } + + public IModelService CreateBlocking(IConnection connection) + { + return ThreadHelper.JoinableTaskFactory.Run(() => CreateAsync(connection)); + } + + public void Dispose() => cacheLock.Dispose(); + } +} diff --git a/src/GitHub.App/GitHub.App.csproj b/src/GitHub.App/GitHub.App.csproj index df19e1577a..3478447133 100644 --- a/src/GitHub.App/GitHub.App.csproj +++ b/src/GitHub.App/GitHub.App.csproj @@ -127,6 +127,7 @@ + diff --git a/src/UnitTests/UnitTests.csproj b/src/UnitTests/UnitTests.csproj index 52fd07e1dc..f2f901ba1b 100644 --- a/src/UnitTests/UnitTests.csproj +++ b/src/UnitTests/UnitTests.csproj @@ -227,8 +227,6 @@ - - From 6034643675c3a982dec54275749338c71c58fcaa Mon Sep 17 00:00:00 2001 From: Steven Kirk Date: Fri, 27 Oct 2017 11:08:13 +0200 Subject: [PATCH 13/16] Added IGlobalConnection. This services the same purpose as `IConnectionRepositoryHostMap` served when `RepositoryHost` was still around: to provide access to the globally registered connection. --- src/GitHub.App/GitHub.App.csproj | 1 + src/GitHub.App/Services/GlobalConnection.cs | 22 +++++++++++++++++++ .../ViewModels/GistCreationViewModel.cs | 4 ++-- .../ViewModels/LoginControlViewModel.cs | 1 - .../ViewModels/LoginTabViewModel.cs | 1 - .../LoginToGitHubForEnterpriseViewModel.cs | 1 - .../ViewModels/LoginToGitHubViewModel.cs | 1 - .../NotAGitHubRepositoryViewModel.cs | 2 -- .../ViewModels/PanePageViewModelBase.cs | 1 - .../PullRequestCreationViewModel.cs | 6 ++--- .../ViewModels/PullRequestDetailViewModel.cs | 4 ++-- .../ViewModels/PullRequestListViewModel.cs | 4 ++-- .../ViewModels/RepositoryCloneViewModel.cs | 9 ++++++++ .../ViewModels/RepositoryCreationViewModel.cs | 12 ++++++++-- .../ViewModels/RepositoryPublishViewModel.cs | 2 -- .../ViewModels/StartPageCloneViewModel.cs | 10 ++++++++- src/GitHub.Exports/GitHub.Exports.csproj | 1 + .../Services/IGlobalConnection.cs | 22 +++++++++++++++++++ 18 files changed, 82 insertions(+), 22 deletions(-) create mode 100644 src/GitHub.App/Services/GlobalConnection.cs create mode 100644 src/GitHub.Exports/Services/IGlobalConnection.cs diff --git a/src/GitHub.App/GitHub.App.csproj b/src/GitHub.App/GitHub.App.csproj index 3478447133..fa5c825457 100644 --- a/src/GitHub.App/GitHub.App.csproj +++ b/src/GitHub.App/GitHub.App.csproj @@ -131,6 +131,7 @@ + diff --git a/src/GitHub.App/Services/GlobalConnection.cs b/src/GitHub.App/Services/GlobalConnection.cs new file mode 100644 index 0000000000..01fb68fd75 --- /dev/null +++ b/src/GitHub.App/Services/GlobalConnection.cs @@ -0,0 +1,22 @@ +using System; +using System.ComponentModel.Composition; +using GitHub.Models; +using GitHub.Services; + +namespace GitHub.App.Services +{ + [Export(typeof(IGlobalConnection))] + [PartCreationPolicy(CreationPolicy.Shared)] + public class GlobalConnection : IGlobalConnection + { + readonly IGitHubServiceProvider serviceProvider; + + [ImportingConstructor] + public GlobalConnection(IGitHubServiceProvider serviceProvider) + { + this.serviceProvider = serviceProvider; + } + + public IConnection Get() => serviceProvider.TryGetService(); + } +} diff --git a/src/GitHub.App/ViewModels/GistCreationViewModel.cs b/src/GitHub.App/ViewModels/GistCreationViewModel.cs index e83494de07..28f725979d 100644 --- a/src/GitHub.App/ViewModels/GistCreationViewModel.cs +++ b/src/GitHub.App/ViewModels/GistCreationViewModel.cs @@ -32,13 +32,13 @@ public class GistCreationViewModel : DialogViewModelBase, IGistCreationViewModel [ImportingConstructor] GistCreationViewModel( - IConnection connection, + IGlobalConnection connection, IModelServiceFactory modelServiceFactory, ISelectedTextProvider selectedTextProvider, IGistPublishService gistPublishService, INotificationService notificationService, IUsageTracker usageTracker) - : this(connection, modelServiceFactory, selectedTextProvider, gistPublishService, usageTracker) + : this(connection.Get(), modelServiceFactory, selectedTextProvider, gistPublishService, usageTracker) { Guard.ArgumentNotNull(connection, nameof(connection)); Guard.ArgumentNotNull(selectedTextProvider, nameof(selectedTextProvider)); diff --git a/src/GitHub.App/ViewModels/LoginControlViewModel.cs b/src/GitHub.App/ViewModels/LoginControlViewModel.cs index 7ebce6cbef..b62b825021 100644 --- a/src/GitHub.App/ViewModels/LoginControlViewModel.cs +++ b/src/GitHub.App/ViewModels/LoginControlViewModel.cs @@ -6,7 +6,6 @@ using GitHub.Authentication; using GitHub.Exports; using GitHub.Extensions.Reactive; -using GitHub.Models; using GitHub.Primitives; using GitHub.Services; using ReactiveUI; diff --git a/src/GitHub.App/ViewModels/LoginTabViewModel.cs b/src/GitHub.App/ViewModels/LoginTabViewModel.cs index 5e31e1a987..91b8aa17ed 100644 --- a/src/GitHub.App/ViewModels/LoginTabViewModel.cs +++ b/src/GitHub.App/ViewModels/LoginTabViewModel.cs @@ -10,7 +10,6 @@ using GitHub.Extensions; using GitHub.Extensions.Reactive; using GitHub.Info; -using GitHub.Models; using GitHub.Primitives; using GitHub.Services; using GitHub.Validation; diff --git a/src/GitHub.App/ViewModels/LoginToGitHubForEnterpriseViewModel.cs b/src/GitHub.App/ViewModels/LoginToGitHubForEnterpriseViewModel.cs index 28c8e8e53a..8ded402ec9 100644 --- a/src/GitHub.App/ViewModels/LoginToGitHubForEnterpriseViewModel.cs +++ b/src/GitHub.App/ViewModels/LoginToGitHubForEnterpriseViewModel.cs @@ -7,7 +7,6 @@ using GitHub.Authentication; using GitHub.Extensions; using GitHub.Info; -using GitHub.Models; using GitHub.Primitives; using GitHub.Services; using GitHub.Validation; diff --git a/src/GitHub.App/ViewModels/LoginToGitHubViewModel.cs b/src/GitHub.App/ViewModels/LoginToGitHubViewModel.cs index 104d941990..24fe832b6a 100644 --- a/src/GitHub.App/ViewModels/LoginToGitHubViewModel.cs +++ b/src/GitHub.App/ViewModels/LoginToGitHubViewModel.cs @@ -4,7 +4,6 @@ using System.Reactive.Linq; using GitHub.Authentication; using GitHub.Info; -using GitHub.Models; using GitHub.Primitives; using GitHub.Services; using ReactiveUI; diff --git a/src/GitHub.App/ViewModels/NotAGitHubRepositoryViewModel.cs b/src/GitHub.App/ViewModels/NotAGitHubRepositoryViewModel.cs index 0956486f4f..847f24ff8d 100644 --- a/src/GitHub.App/ViewModels/NotAGitHubRepositoryViewModel.cs +++ b/src/GitHub.App/ViewModels/NotAGitHubRepositoryViewModel.cs @@ -1,9 +1,7 @@ using System; using System.ComponentModel.Composition; using GitHub.Exports; -using GitHub.Models; using GitHub.Services; -using GitHub.UI; using ReactiveUI; namespace GitHub.ViewModels diff --git a/src/GitHub.App/ViewModels/PanePageViewModelBase.cs b/src/GitHub.App/ViewModels/PanePageViewModelBase.cs index a8bdce3b3f..6efb85265e 100644 --- a/src/GitHub.App/ViewModels/PanePageViewModelBase.cs +++ b/src/GitHub.App/ViewModels/PanePageViewModelBase.cs @@ -1,5 +1,4 @@ using ReactiveUI; -using GitHub.UI; namespace GitHub.ViewModels { diff --git a/src/GitHub.App/ViewModels/PullRequestCreationViewModel.cs b/src/GitHub.App/ViewModels/PullRequestCreationViewModel.cs index 4cd4422ade..ba832cc898 100644 --- a/src/GitHub.App/ViewModels/PullRequestCreationViewModel.cs +++ b/src/GitHub.App/ViewModels/PullRequestCreationViewModel.cs @@ -32,7 +32,6 @@ public class PullRequestCreationViewModel : DialogViewModelBase, IPullRequestCre readonly ObservableAsPropertyHelper githubRepository; readonly ObservableAsPropertyHelper isExecuting; - readonly IConnection connection; readonly IModelService modelService; readonly IObservable githubObs; readonly CompositeDisposable disposables = new CompositeDisposable(); @@ -40,11 +39,11 @@ public class PullRequestCreationViewModel : DialogViewModelBase, IPullRequestCre [ImportingConstructor] PullRequestCreationViewModel( - IConnection connection, + IGlobalConnection connection, IModelServiceFactory modelServiceFactory, ITeamExplorerServiceHolder teservice, IPullRequestService service, INotificationService notifications) - : this(connection, modelServiceFactory, teservice?.ActiveRepo, service, notifications) + : this(connection.Get(), modelServiceFactory, teservice?.ActiveRepo, service, notifications) {} public PullRequestCreationViewModel( @@ -60,7 +59,6 @@ public PullRequestCreationViewModel( Guard.ArgumentNotNull(service, nameof(service)); Guard.ArgumentNotNull(notifications, nameof(notifications)); - this.connection = connection; activeLocalRepo = activeRepo; modelService = modelServiceFactory.CreateBlocking(connection); diff --git a/src/GitHub.App/ViewModels/PullRequestDetailViewModel.cs b/src/GitHub.App/ViewModels/PullRequestDetailViewModel.cs index 9bdbda8315..e29c3b2589 100644 --- a/src/GitHub.App/ViewModels/PullRequestDetailViewModel.cs +++ b/src/GitHub.App/ViewModels/PullRequestDetailViewModel.cs @@ -60,14 +60,14 @@ public class PullRequestDetailViewModel : PanePageViewModelBase, IPullRequestDet /// The usage tracker. [ImportingConstructor] PullRequestDetailViewModel( - IConnection connection, + IGlobalConnection connection, IModelServiceFactory modelServiceFactory, ITeamExplorerServiceHolder teservice, IPullRequestService pullRequestsService, IPullRequestSessionManager sessionManager, IUsageTracker usageTracker) : this(teservice.ActiveRepo, - modelServiceFactory.CreateBlocking(connection), + modelServiceFactory.CreateBlocking(connection.Get()), pullRequestsService, sessionManager, usageTracker) diff --git a/src/GitHub.App/ViewModels/PullRequestListViewModel.cs b/src/GitHub.App/ViewModels/PullRequestListViewModel.cs index 74f1a5147e..593ae2851c 100644 --- a/src/GitHub.App/ViewModels/PullRequestListViewModel.cs +++ b/src/GitHub.App/ViewModels/PullRequestListViewModel.cs @@ -41,12 +41,12 @@ public class PullRequestListViewModel : PanePageViewModelBase, IPullRequestListV [ImportingConstructor] PullRequestListViewModel( - IConnection connection, + IGlobalConnection connection, IModelServiceFactory modelServiceFactory, ITeamExplorerServiceHolder teservice, IPackageSettings settings, IVisualStudioBrowser visualStudioBrowser) - : this(connection, modelServiceFactory, teservice.ActiveRepo, settings, visualStudioBrowser) + : this(connection.Get(), modelServiceFactory, teservice.ActiveRepo, settings, visualStudioBrowser) { Guard.ArgumentNotNull(connection, nameof(connection)); Guard.ArgumentNotNull(teservice, nameof(teservice)); diff --git a/src/GitHub.App/ViewModels/RepositoryCloneViewModel.cs b/src/GitHub.App/ViewModels/RepositoryCloneViewModel.cs index 3a3877f8cb..7a13f25b2f 100644 --- a/src/GitHub.App/ViewModels/RepositoryCloneViewModel.cs +++ b/src/GitHub.App/ViewModels/RepositoryCloneViewModel.cs @@ -43,6 +43,15 @@ public class RepositoryCloneViewModel : DialogViewModelBase, IRepositoryCloneVie bool loadingFailed; [ImportingConstructor] + public RepositoryCloneViewModel( + IGlobalConnection connection, + IModelServiceFactory modelServiceFactory, + IRepositoryCloneService cloneService, + IOperatingSystem operatingSystem) + : this(connection.Get(), modelServiceFactory, cloneService, operatingSystem) + { + } + public RepositoryCloneViewModel( IConnection connection, IModelServiceFactory modelServiceFactory, diff --git a/src/GitHub.App/ViewModels/RepositoryCreationViewModel.cs b/src/GitHub.App/ViewModels/RepositoryCreationViewModel.cs index 319b59d180..1ee81281ea 100644 --- a/src/GitHub.App/ViewModels/RepositoryCreationViewModel.cs +++ b/src/GitHub.App/ViewModels/RepositoryCreationViewModel.cs @@ -34,7 +34,6 @@ public class RepositoryCreationViewModel : RepositoryFormViewModel, IRepositoryC readonly ReactiveCommand browseForDirectoryCommand = ReactiveCommand.Create(); readonly ObservableAsPropertyHelper> accounts; - readonly IConnection connection; readonly IModelService modelService; readonly IRepositoryCreationService repositoryCreationService; readonly ObservableAsPropertyHelper isCreating; @@ -43,6 +42,16 @@ public class RepositoryCreationViewModel : RepositoryFormViewModel, IRepositoryC readonly IUsageTracker usageTracker; [ImportingConstructor] + public RepositoryCreationViewModel( + IGlobalConnection connection, + IModelServiceFactory modelServiceFactory, + IOperatingSystem operatingSystem, + IRepositoryCreationService repositoryCreationService, + IUsageTracker usageTracker) + : this(connection.Get(), modelServiceFactory, operatingSystem, repositoryCreationService, usageTracker) + { + } + public RepositoryCreationViewModel( IConnection connection, IModelServiceFactory modelServiceFactory, @@ -56,7 +65,6 @@ public RepositoryCreationViewModel( Guard.ArgumentNotNull(repositoryCreationService, nameof(repositoryCreationService)); Guard.ArgumentNotNull(usageTracker, nameof(usageTracker)); - this.connection = connection; this.operatingSystem = operatingSystem; this.repositoryCreationService = repositoryCreationService; this.usageTracker = usageTracker; diff --git a/src/GitHub.App/ViewModels/RepositoryPublishViewModel.cs b/src/GitHub.App/ViewModels/RepositoryPublishViewModel.cs index f48df1ecaa..708b85e9e2 100644 --- a/src/GitHub.App/ViewModels/RepositoryPublishViewModel.cs +++ b/src/GitHub.App/ViewModels/RepositoryPublishViewModel.cs @@ -26,7 +26,6 @@ public class RepositoryPublishViewModel : RepositoryFormViewModel, IRepositoryPu { static readonly Logger log = LogManager.GetCurrentClassLogger(); - readonly IConnectionManager connectionManager; readonly IRepositoryPublishService repositoryPublishService; readonly INotificationService notificationService; readonly IModelServiceFactory modelServiceFactory; @@ -51,7 +50,6 @@ public RepositoryPublishViewModel( Guard.ArgumentNotNull(modelServiceFactory, nameof(modelServiceFactory)); this.notificationService = notificationService; - this.connectionManager = connectionManager; this.usageTracker = usageTracker; this.modelServiceFactory = modelServiceFactory; diff --git a/src/GitHub.App/ViewModels/StartPageCloneViewModel.cs b/src/GitHub.App/ViewModels/StartPageCloneViewModel.cs index 895b882372..0a9e1faef7 100644 --- a/src/GitHub.App/ViewModels/StartPageCloneViewModel.cs +++ b/src/GitHub.App/ViewModels/StartPageCloneViewModel.cs @@ -31,7 +31,15 @@ public class StartPageCloneViewModel : DialogViewModelBase, IBaseCloneViewModel string baseRepositoryPath; [ImportingConstructor] - StartPageCloneViewModel( + public StartPageCloneViewModel( + IGlobalConnection connection, + IRepositoryCloneService cloneService, + IOperatingSystem operatingSystem) + : this(connection.Get(), cloneService, operatingSystem) + { + } + + public StartPageCloneViewModel( IConnection connection, IRepositoryCloneService cloneService, IOperatingSystem operatingSystem) diff --git a/src/GitHub.Exports/GitHub.Exports.csproj b/src/GitHub.Exports/GitHub.Exports.csproj index 955132356b..bd01ae2bef 100644 --- a/src/GitHub.Exports/GitHub.Exports.csproj +++ b/src/GitHub.Exports/GitHub.Exports.csproj @@ -137,6 +137,7 @@ + diff --git a/src/GitHub.Exports/Services/IGlobalConnection.cs b/src/GitHub.Exports/Services/IGlobalConnection.cs new file mode 100644 index 0000000000..22bf92689b --- /dev/null +++ b/src/GitHub.Exports/Services/IGlobalConnection.cs @@ -0,0 +1,22 @@ +using System; +using GitHub.Models; + +namespace GitHub.Services +{ + /// + /// Returns the current globally registered . + /// + /// + /// Many view models require a connection with which to work. The UIController registers the + /// connection for the current flow with in its Start() + /// method for this purpose. View models wishing to retreive this value should import this + /// interface and call . + /// + public interface IGlobalConnection + { + /// + /// Gets the globally registered . + /// + IConnection Get(); + } +} From 17aebf8c8b892b17ec3cda881d0321940946456d Mon Sep 17 00:00:00 2001 From: Steven Kirk Date: Fri, 27 Oct 2017 12:17:26 +0200 Subject: [PATCH 14/16] Removed ConnectionManager.ConnectionCreated. --- src/GitHub.Exports/Services/IConnectionManager.cs | 13 ------------- .../Services/ConnectionManager.cs | 12 ------------ 2 files changed, 25 deletions(-) diff --git a/src/GitHub.Exports/Services/IConnectionManager.cs b/src/GitHub.Exports/Services/IConnectionManager.cs index aa79e5cf24..66045ec693 100644 --- a/src/GitHub.Exports/Services/IConnectionManager.cs +++ b/src/GitHub.Exports/Services/IConnectionManager.cs @@ -22,19 +22,6 @@ public interface IConnectionManager /// IReadOnlyObservableCollection Connections { get; } - /// - /// Gets a callback that is called after a new is created but - /// before it is added to . - /// - /// - /// This is a hack and should be removed as soon as possible! It's needed because creating - /// a RepositoryHost is async meaning that for a short time a connection can exist without - /// a repository host, but other older parts of the code get confused by this. This callback - /// allows RepositoryHosts to hook into the creation of the connection to make sure a host - /// is available by the time the connection is made available. - /// - Func ConnectionCreated { get; set; } - /// /// Gets a connection with the specified host address. /// diff --git a/src/GitHub.VisualStudio/Services/ConnectionManager.cs b/src/GitHub.VisualStudio/Services/ConnectionManager.cs index e69ed64610..3a73e2ebac 100644 --- a/src/GitHub.VisualStudio/Services/ConnectionManager.cs +++ b/src/GitHub.VisualStudio/Services/ConnectionManager.cs @@ -52,8 +52,6 @@ public ConnectionManager( /// public IReadOnlyObservableCollection Connections => connections.Value; - public Func ConnectionCreated { get; set; } - /// public async Task GetConnection(HostAddress address) { @@ -80,11 +78,6 @@ public async Task LogIn(HostAddress address, string userName, strin var user = await loginManager.Login(address, client, userName, password); var connection = new Connection(address, userName, user, null); - if (ConnectionCreated != null) - { - await ConnectionCreated(connection); - } - conns.Add(connection); await SaveConnections(); await usageTracker.IncrementLoginCount(); @@ -150,11 +143,6 @@ async Task LoadConnections(ObservableCollection result) var connection = new Connection(c.HostAddress, c.UserName, user, error); - if (ConnectionCreated != null) - { - await ConnectionCreated(connection); - } - result.Add(connection); await usageTracker.IncrementLoginCount(); } From 7e57621d56ec6489a38b4e11d3843ef4cf1d1e7c Mon Sep 17 00:00:00 2001 From: Steven Kirk Date: Tue, 7 Nov 2017 13:50:44 +0100 Subject: [PATCH 15/16] Insert AccountCacheItem for connection. When creating a `ModelService`, ensure that the correct `AccountCacheItem` is inserted. --- .../Factories/ModelServiceFactory.cs | 3 + .../Factories/ModelServiceFactoryTests.cs | 85 +++++++++++++++++++ src/UnitTests/UnitTests.csproj | 1 + 3 files changed, 89 insertions(+) create mode 100644 src/UnitTests/GitHub.App/Factories/ModelServiceFactoryTests.cs diff --git a/src/GitHub.App/Factories/ModelServiceFactory.cs b/src/GitHub.App/Factories/ModelServiceFactory.cs index 84b4491c8c..8a6188b6bd 100644 --- a/src/GitHub.App/Factories/ModelServiceFactory.cs +++ b/src/GitHub.App/Factories/ModelServiceFactory.cs @@ -3,6 +3,7 @@ using System.ComponentModel.Composition; using System.Threading; using System.Threading.Tasks; +using GitHub.Caches; using GitHub.Models; using GitHub.Services; using Microsoft.VisualStudio.Shell; @@ -44,6 +45,8 @@ public async Task CreateAsync(IConnection connection) await apiClientFactory.Create(connection.HostAddress), await hostCacheFactory.Create(connection.HostAddress), avatarProvider); + result.InsertUser(AccountCacheItem.Create(connection.User)); + cache.Add(connection, result); } } finally diff --git a/src/UnitTests/GitHub.App/Factories/ModelServiceFactoryTests.cs b/src/UnitTests/GitHub.App/Factories/ModelServiceFactoryTests.cs new file mode 100644 index 0000000000..54bd2e88f9 --- /dev/null +++ b/src/UnitTests/GitHub.App/Factories/ModelServiceFactoryTests.cs @@ -0,0 +1,85 @@ +using System; +using System.Threading.Tasks; +using GitHub.Caches; +using GitHub.Extensions; +using GitHub.Factories; +using GitHub.Models; +using GitHub.Primitives; +using GitHub.Services; +using NSubstitute; +using Xunit; + +namespace UnitTests.GitHub.App.Factories +{ + public class ModelServiceFactoryTests : TestBaseClass + { + public class TheCreateAsyncMethod + { + [Fact] + public async Task ShouldCreateDifferentModelServiceForDifferentHost() + { + var target = CreateTarget(); + var instance1 = await target.CreateAsync(CreateConnection("https://github.com")); + var instance2 = await target.CreateAsync(CreateConnection("https://another.com")); + + Assert.NotSame(instance1, instance2); + } + + [Fact] + public async Task ShouldCreateDifferentModelServiceForDifferentConnectionsWithSameAddress() + { + var target = CreateTarget(); + var instance1 = await target.CreateAsync(CreateConnection("https://github.com")); + var instance2 = await target.CreateAsync(CreateConnection("https://github.com")); + + Assert.NotSame(instance1, instance2); + } + + [Fact] + public async Task ShouldCacheModelServiceForHost() + { + var target = CreateTarget(); + var connection = CreateConnection("https://github.com"); + var instance1 = await target.CreateAsync(connection); + var instance2 = await target.CreateAsync(connection); + + Assert.Same(instance1, instance2); + } + + [Fact] + public async Task ShouldInsertUser() + { + var hostCacheFactory = Substitute.For(); + var target = CreateTarget(hostCacheFactory: hostCacheFactory); + var connection = CreateConnection("https://github.com"); + var hostCache = await hostCacheFactory.Create(connection.HostAddress); + var modelService = await target.CreateAsync(connection); + + hostCache.Received().Insert("GitHub.Caches.AccountCacheItem___user", Arg.Any()); + } + } + + static ModelServiceFactory CreateTarget( + IHostCacheFactory hostCacheFactory = null) + { + var apiClientFactory = Substitute.For(); + var avatarProvider = Substitute.For(); + + hostCacheFactory = hostCacheFactory ?? Substitute.For(); + + return new ModelServiceFactory( + apiClientFactory, + hostCacheFactory, + avatarProvider); + } + + static IConnection CreateConnection(string address, string login = "user") + { + var result = Substitute.For(); + var user = CreateOctokitUser(login, address); + result.HostAddress.Returns(HostAddress.Create(address)); + result.User.Returns(user); + return result; + } + } +} diff --git a/src/UnitTests/UnitTests.csproj b/src/UnitTests/UnitTests.csproj index f2f901ba1b..c459f69abf 100644 --- a/src/UnitTests/UnitTests.csproj +++ b/src/UnitTests/UnitTests.csproj @@ -223,6 +223,7 @@ + From cd546bbeb0dbedb862199e75e274e08069b3df45 Mon Sep 17 00:00:00 2001 From: Steven Kirk Date: Tue, 7 Nov 2017 18:32:29 +0100 Subject: [PATCH 16/16] Fix CA errors. --- .../Services/PullRequestSessionManager.cs | 2 -- src/GitHub.VisualStudio/Services/ConnectionManager.cs | 8 ++------ 2 files changed, 2 insertions(+), 8 deletions(-) diff --git a/src/GitHub.InlineReviews/Services/PullRequestSessionManager.cs b/src/GitHub.InlineReviews/Services/PullRequestSessionManager.cs index bf35129dac..106c67d6cf 100644 --- a/src/GitHub.InlineReviews/Services/PullRequestSessionManager.cs +++ b/src/GitHub.InlineReviews/Services/PullRequestSessionManager.cs @@ -34,7 +34,6 @@ public class PullRequestSessionManager : ReactiveObject, IPullRequestSessionMana readonly IPullRequestSessionService sessionService; readonly IConnectionManager connectionManager; readonly IModelServiceFactory modelServiceFactory; - readonly ITeamExplorerServiceHolder teamExplorerService; readonly Dictionary, WeakReference> sessions = new Dictionary, WeakReference>(); IPullRequestSession currentSession; @@ -66,7 +65,6 @@ public PullRequestSessionManager( this.sessionService = sessionService; this.connectionManager = connectionManager; this.modelServiceFactory = modelServiceFactory; - this.teamExplorerService = teamExplorerService; teamExplorerService.Subscribe(this, x => RepoChanged(x).Forget()); } diff --git a/src/GitHub.VisualStudio/Services/ConnectionManager.cs b/src/GitHub.VisualStudio/Services/ConnectionManager.cs index b2153cf68c..73749e3af4 100644 --- a/src/GitHub.VisualStudio/Services/ConnectionManager.cs +++ b/src/GitHub.VisualStudio/Services/ConnectionManager.cs @@ -3,18 +3,16 @@ using System.Collections.ObjectModel; using System.ComponentModel.Composition; using System.Linq; +using System.Threading; using System.Threading.Tasks; using GitHub.Api; using GitHub.Extensions; using GitHub.Models; using GitHub.Primitives; using GitHub.Services; -using IGitHubClient = Octokit.IGitHubClient; using GitHubClient = Octokit.GitHubClient; +using IGitHubClient = Octokit.IGitHubClient; using User = Octokit.User; -using Serilog; -using GitHub.Logging; -using System.Threading; namespace GitHub.VisualStudio { @@ -24,8 +22,6 @@ namespace GitHub.VisualStudio [Export(typeof(IConnectionManager))] public class ConnectionManager : IConnectionManager { - static readonly ILogger log = LogManager.ForContext(); - readonly IProgram program; readonly IConnectionCache cache; readonly IKeychain keychain;