From d99b34ac4e1a20aabbdb002e571e0484729a68ee Mon Sep 17 00:00:00 2001 From: Andreia Gaita Date: Wed, 23 Sep 2015 15:12:21 +0200 Subject: [PATCH 1/2] Fix IGitService Make static methods that mirror the IGitService instance methods and use either a real mef instance or a fake one to get at the instance methods. Things are slightly broken getting a reference to IGitService - in real life, it's not a service, it's an exported value, but in unit tests it's treated as a service (because this stupid distinction between services and exported values is stupid). So, IGitService is nothing but a bunch of helper methods, and we want to use it as an instance and not just static methods, so that we can bypass hitting the filesystem and libgit2# methods in unit tests. Some bits that need IGitService for realz are running in places where there's no MEF. Therefore, the only time where it really makes a difference when/how the IGitService instance is created is in unit tests. When running for real, it makes zero difference if the instance is coming from mef or if it's just created on the spot, so that's what we're doing now. --- src/GitHub.Exports/Extensions/GitHelpers.cs | 46 -------- .../SimpleRepositoryModelExtensions.cs | 3 +- src/GitHub.Exports/GitHub.Exports.csproj | 1 - .../Models/SimpleRepositoryModel.cs | 5 +- src/GitHub.Exports/Services/GitService.cs | 103 ++++++++++++++++-- src/GitHub.Exports/Services/Services.cs | 35 +++--- .../GitHub.App/Models/RepositoryModelTests.cs | 6 +- src/UnitTests/Substitutes.cs | 13 ++- 8 files changed, 124 insertions(+), 88 deletions(-) delete mode 100644 src/GitHub.Exports/Extensions/GitHelpers.cs diff --git a/src/GitHub.Exports/Extensions/GitHelpers.cs b/src/GitHub.Exports/Extensions/GitHelpers.cs deleted file mode 100644 index 8925fbb75a..0000000000 --- a/src/GitHub.Exports/Extensions/GitHelpers.cs +++ /dev/null @@ -1,46 +0,0 @@ -using GitHub.Primitives; -using LibGit2Sharp; -using Microsoft.VisualStudio.TeamFoundation.Git.Extensibility; -using System; -using System.Linq; - -namespace GitHub.Extensions -{ - public static class GitHelpers - { - public static Repository GetRepoFromIGit(this IGitRepositoryInfo repoInfo) - { - var repoPath = Repository.Discover(repoInfo.RepositoryPath); - if (repoPath == null) - return null; - return new Repository(repoPath); - } - - public static UriString GetUriFromRepository(this IGitRepositoryInfo repoInfo) - { - return repoInfo.GetRepoFromIGit()?.GetUri(); - } - - public static UriString GetUri(this Repository repo) - { - return UriString.ToUriString(GetUriFromRepository(repo)?.ToRepositoryUrl()); - } - - static UriString GetUriFromRepository(Repository repo) - { - return repo - ?.Network - .Remotes - .FirstOrDefault(x => x.Name.Equals("origin", StringComparison.Ordinal)) - ?.Url; - } - - public static Repository GetRepoFromPath(string path) - { - var repoPath = Repository.Discover(path); - if (repoPath == null) - return null; - return new Repository(repoPath); - } - } -} diff --git a/src/GitHub.Exports/Extensions/SimpleRepositoryModelExtensions.cs b/src/GitHub.Exports/Extensions/SimpleRepositoryModelExtensions.cs index 939818e4fd..c5089e3d82 100644 --- a/src/GitHub.Exports/Extensions/SimpleRepositoryModelExtensions.cs +++ b/src/GitHub.Exports/Extensions/SimpleRepositoryModelExtensions.cs @@ -3,6 +3,7 @@ using System.IO; using GitHub.Models; using Microsoft.VisualStudio.TeamFoundation.Git.Extensibility; +using GitHub.Services; namespace GitHub.Extensions { @@ -18,7 +19,7 @@ public static ISimpleRepositoryModel ToModel(this IGitRepositoryInfo repo) public static bool HasCommits(this ISimpleRepositoryModel repository) { - var repo = VisualStudio.Services.IGitService.GetRepo(repository.LocalPath); + var repo = GitService.GitServiceHelper.GetRepo(repository.LocalPath); return repo?.Commits.Any() ?? false; } diff --git a/src/GitHub.Exports/GitHub.Exports.csproj b/src/GitHub.Exports/GitHub.Exports.csproj index 2256606870..d38af1ce82 100644 --- a/src/GitHub.Exports/GitHub.Exports.csproj +++ b/src/GitHub.Exports/GitHub.Exports.csproj @@ -99,7 +99,6 @@ Key.snk - diff --git a/src/GitHub.Exports/Models/SimpleRepositoryModel.cs b/src/GitHub.Exports/Models/SimpleRepositoryModel.cs index 419411f96a..9bb2672660 100644 --- a/src/GitHub.Exports/Models/SimpleRepositoryModel.cs +++ b/src/GitHub.Exports/Models/SimpleRepositoryModel.cs @@ -5,6 +5,7 @@ using GitHub.Primitives; using GitHub.UI; using GitHub.VisualStudio.Helpers; +using GitHub.Services; namespace GitHub.Models { @@ -26,7 +27,7 @@ public SimpleRepositoryModel(string path) var dir = new DirectoryInfo(path); if (!dir.Exists) throw new ArgumentException("Path does not exist", nameof(path)); - var uri = VisualStudio.Services.IGitService.GetUri(path); + var uri = GitService.GitServiceHelper.GetUri(path); var name = uri?.NameWithOwner ?? dir.Name; Name = name; LocalPath = path; @@ -47,7 +48,7 @@ public void Refresh() { if (LocalPath == null) return; - var uri = VisualStudio.Services.IGitService.GetUri(LocalPath); + var uri = GitService.GitServiceHelper.GetUri(LocalPath); if (CloneUrl != uri) CloneUrl = uri; } diff --git a/src/GitHub.Exports/Services/GitService.cs b/src/GitHub.Exports/Services/GitService.cs index ecaa487003..4a9d9d63a2 100644 --- a/src/GitHub.Exports/Services/GitService.cs +++ b/src/GitHub.Exports/Services/GitService.cs @@ -4,11 +4,12 @@ using GitHub.Primitives; using LibGit2Sharp; using Microsoft.VisualStudio.TeamFoundation.Git.Extensibility; +using GitHub.Extensions; namespace GitHub.Services { - [Export(typeof(IVSServices))] - [PartCreationPolicy(CreationPolicy.Shared)] + [Export(typeof(IGitService))] + [PartCreationPolicy(CreationPolicy.NonShared)] public class GitService : IGitService { /// @@ -16,30 +17,56 @@ public class GitService : IGitService /// is null or no remote named origin exists, this method returns null /// /// The repository to look at for the remote. - /// A representing the origin or null if none found. + /// Returns a representing the uri of the "origin" remote normalized to a GitHub repository url or null if none found. public UriString GetUri(IRepository repository) { - return UriString.ToUriString(GetUriFromRepository(repository)?.ToRepositoryUrl()); + return UriString.ToUriString(GetOriginUri(repository)?.ToRepositoryUrl()); + } /// - /// Probes for a git repository and if one is found, returns a for the repository's - /// remote named "origin" if one is found + /// Returns a representing the uri of the "origin" remote normalized to a GitHub repository url or null if none found. + /// + /// + /// Returns a representing the uri of the "origin" remote normalized to a GitHub repository url or null if none found. + public static UriString GetGitHubUri(IRepository repository) + { + return GitServiceHelper.GetUri(repository); + } + + /// + /// Probes for a git repository and if one is found, returns a normalized GitHub uri + /// for the repository's remote named "origin" if one is found /// /// /// The lookup checks to see if the specified is a repository. If it's not, it then /// walks up the parent directories until it either finds a repository, or reaches the root disk. /// /// The path to start probing - /// A representing the origin or null if none found. + /// Returns a representing the uri of the "origin" remote normalized to a GitHub repository url or null if none found. public UriString GetUri(string path) { return GetUri(GetRepo(path)); } /// - /// Probes for a git repository and if one is found, returns a for the repository's - /// remote named "origin" if one is found + /// Probes for a git repository and if one is found, returns a normalized GitHub uri + /// for the repository's remote named "origin" if one is found + /// + /// + /// The lookup checks to see if the specified is a repository. If it's not, it then + /// walks up the parent directories until it either finds a repository, or reaches the root disk. + /// + /// The path to start probing + /// Returns a representing the uri of the "origin" remote normalized to a GitHub repository url or null if none found. + public static UriString GetUriFromPath(string path) + { + return GitServiceHelper.GetUri(path); + } + + /// + /// Probes for a git repository and if one is found, returns a normalized GitHub uri + /// for the repository's remote named "origin" if one is found /// /// /// The lookup checks to see if the path specified by the RepositoryPath property of the specified @@ -47,12 +74,28 @@ public UriString GetUri(string path) /// either finds a repository, or reaches the root disk. /// /// The repository information containing the path to start probing - /// A representing the origin or null if none found. + /// Returns a representing the uri of the "origin" remote normalized to a GitHub repository url or null if none found. public UriString GetUri(IGitRepositoryInfo repoInfo) { return GetUri(GetRepo(repoInfo)); } + /// + /// Probes for a git repository and if one is found, returns a normalized GitHub uri + /// for the repository's remote named "origin" if one is found + /// + /// + /// The lookup checks to see if the path specified by the RepositoryPath property of the specified + /// is a repository. If it's not, it then walks up the parent directories until it + /// either finds a repository, or reaches the root disk. + /// + /// The repository information containing the path to start probing + /// Returns a representing the uri of the "origin" remote normalized to a GitHub repository url or null if none found. + public static UriString GetUriFromVSGit(IGitRepositoryInfo repoInfo) + { + return GitServiceHelper.GetUri(repoInfo); + } + /// /// Probes for a git repository and if one is found, returns a instance for the /// repository. @@ -70,6 +113,22 @@ public IRepository GetRepo(IGitRepositoryInfo repoInfo) return GetRepo(repoInfo?.RepositoryPath); } + /// + /// Probes for a git repository and if one is found, returns a instance for the + /// repository. + /// + /// + /// The lookup checks to see if the path specified by the RepositoryPath property of the specified + /// is a repository. If it's not, it then walks up the parent directories until it + /// either finds a repository, or reaches the root disk. + /// + /// The repository information containing the path to start probing + /// An instance of or null + public static IRepository GetRepoFromVSGit(IGitRepositoryInfo repoInfo) + { + return GitServiceHelper.GetRepo(repoInfo); + } + /// /// Probes for a git repository and if one is found, returns a instance for the /// repository. @@ -86,7 +145,27 @@ public IRepository GetRepo(string path) return repoPath == null ? null : new Repository(repoPath); } - internal static UriString GetUriFromRepository(IRepository repo) + /// + /// Probes for a git repository and if one is found, returns a instance for the + /// repository. + /// + /// + /// The lookup checks to see if the specified is a repository. If it's not, it then + /// walks up the parent directories until it either finds a repository, or reaches the root disk. + /// + /// The path to start probing + /// An instance of or null + public static IRepository GetRepoFromPath(string path) + { + return GitServiceHelper.GetRepo(path); + } + + /// + /// Returns a representing the uri of the "origin" remote with no modifications. + /// + /// + /// + public static UriString GetOriginUri(IRepository repo) { return repo ?.Network @@ -94,5 +173,7 @@ internal static UriString GetUriFromRepository(IRepository repo) .FirstOrDefault(x => x.Name.Equals("origin", StringComparison.Ordinal)) ?.Url; } + + public static IGitService GitServiceHelper => VisualStudio.Services.DefaultExportProvider.GetExportedValueOrDefault() ?? new GitService(); } } diff --git a/src/GitHub.Exports/Services/Services.cs b/src/GitHub.Exports/Services/Services.cs index ceeaf640a4..1af09b5e54 100644 --- a/src/GitHub.Exports/Services/Services.cs +++ b/src/GitHub.Exports/Services/Services.cs @@ -1,7 +1,6 @@ using System; using EnvDTE; using EnvDTE80; -using GitHub.Extensions; using GitHub.Info; using GitHub.Primitives; using GitHub.Services; @@ -9,6 +8,8 @@ using Microsoft.VisualStudio; using Microsoft.VisualStudio.ComponentModelHost; using Microsoft.VisualStudio.Shell.Interop; +using Microsoft.VisualStudio.Shell; +using System.ComponentModel.Composition.Hosting; namespace GitHub.VisualStudio { @@ -17,10 +18,11 @@ public static class Services public static IServiceProvider PackageServiceProvider { get; set; } /// - /// Two ways of getting a service. First, trying the passed-in , - /// then - /// If the passed-in provider returns null, try PackageServiceProvider, returning the fetched value - /// regardless of whether it's null or not. + /// Three ways of getting a service. First, trying the passed-in , + /// then , then + /// If the passed-in provider returns null, try PackageServiceProvider or Package, returning the fetched value + /// regardless of whether it's null or not. Package.GetGlobalService is never called if PackageServiceProvider is set. + /// This is on purpose, to support easy unit testing outside VS. /// /// /// @@ -33,10 +35,13 @@ static Ret GetGlobalService(IServiceProvider provider = null) where T : ret = provider.GetService(typeof(T)) as Ret; if (ret != null) return ret; - return PackageServiceProvider.GetService(typeof(T)) as Ret; + if (PackageServiceProvider != null) + return PackageServiceProvider.GetService(typeof(T)) as Ret; + return Package.GetGlobalService(typeof(T)) as Ret; } public static IComponentModel ComponentModel => GetGlobalService(); + public static ExportProvider DefaultExportProvider => ComponentModel.DefaultExportProvider; public static IVsWebBrowsingService GetWebBrowsingService(this IServiceProvider provider) { @@ -92,13 +97,7 @@ public static UriString GetRepoUrlFromSolution(IVsSolution solution) return null; if (solutionDir == null) return null; - var repoPath = Repository.Discover(solutionDir); - if (repoPath == null) - return null; - using (var repo = new Repository(repoPath)) - { - return GetUri(repo); - } + return GitService.GitServiceHelper.GetUri(solutionDir); } public static IRepository GetRepoFromSolution(this IVsSolution solution) @@ -108,15 +107,7 @@ public static IRepository GetRepoFromSolution(this IVsSolution solution) return null; if (solutionDir == null) return null; - var repoPath = Repository.Discover(solutionDir); - if (repoPath == null) - return null; - return new Repository(repoPath); - } - static UriString GetUri(IRepository repo) - { - return UriString.ToUriString(GitService.GetUriFromRepository(repo)?.ToRepositoryUrl()); + return GitService.GitServiceHelper.GetRepo(solutionDir); } - public static IGitService IGitService => PackageServiceProvider.GetService(); } } diff --git a/src/UnitTests/GitHub.App/Models/RepositoryModelTests.cs b/src/UnitTests/GitHub.App/Models/RepositoryModelTests.cs index 6dc3a7e0d3..64577d00b8 100644 --- a/src/UnitTests/GitHub.App/Models/RepositoryModelTests.cs +++ b/src/UnitTests/GitHub.App/Models/RepositoryModelTests.cs @@ -60,8 +60,7 @@ public void NoRemoteUrl() { var provider = Substitutes.ServiceProvider; Services.PackageServiceProvider = provider; - var gitservice = Substitutes.IGitService; - provider.GetService(typeof(IGitService)).Returns(gitservice); + var gitservice = provider.GetGitService(); var repo = Substitute.For(); var path = Directory.CreateSubdirectory("repo-name"); gitservice.GetUri(path.FullName).Returns((UriString)null); @@ -74,8 +73,7 @@ public void WithRemoteUrl() { var provider = Substitutes.ServiceProvider; Services.PackageServiceProvider = provider; - var gitservice = Substitutes.IGitService; - provider.GetService(typeof(IGitService)).Returns(gitservice); + var gitservice = provider.GetGitService(); var repo = Substitute.For(); var path = Directory.CreateSubdirectory("repo-name"); gitservice.GetUri(path.FullName).Returns(new UriString("https://github.com/user/repo-name")); diff --git a/src/UnitTests/Substitutes.cs b/src/UnitTests/Substitutes.cs index 2a6de0d842..003587c60e 100644 --- a/src/UnitTests/Substitutes.cs +++ b/src/UnitTests/Substitutes.cs @@ -2,10 +2,13 @@ using GitHub.Models; using GitHub.Services; using Microsoft.TeamFoundation.Git.Controls.Extensibility; +using Microsoft.VisualStudio.ComponentModelHost; using NSubstitute; using Rothko; using System; using System.Collections.Generic; +using System.ComponentModel.Composition; +using System.ComponentModel.Composition.Hosting; using System.Linq; using System.Text; using System.Threading.Tasks; @@ -82,13 +85,21 @@ public static IServiceProvider GetServiceProvider( IAvatarProvider avatarProvider = null) { var ret = Substitute.For(); + + var gitservice = IGitService; + var cm = Substitute.For(); + var cc = new CompositionContainer(CompositionOptions.IsThreadSafe | CompositionOptions.DisableSilentRejection); + cc.ComposeExportedValue(gitservice); + ((IComponentModel)cm).DefaultExportProvider.Returns(cc); + ret.GetService(typeof(SComponentModel)).Returns(cm); + var os = OperatingSystem; var vs = IVSServices; var clone = cloneService ?? new RepositoryCloneService(os, vs); var create = creationService ?? new RepositoryCreationService(clone); avatarProvider = avatarProvider ?? Substitute.For(); ret.GetService(typeof(IGitRepositoriesExt)).Returns(IGitRepositoriesExt); - ret.GetService(typeof(IGitService)).Returns(IGitService); + ret.GetService(typeof(IGitService)).Returns(gitservice); ret.GetService(typeof(IVSServices)).Returns(vs); ret.GetService(typeof(IOperatingSystem)).Returns(os); ret.GetService(typeof(IRepositoryCloneService)).Returns(clone); From be83fce9d310e8989eb0da80adc2d5852a5b21a7 Mon Sep 17 00:00:00 2001 From: Haacked Date: Wed, 23 Sep 2015 13:58:35 -0700 Subject: [PATCH 2/2] :art: The most anal of cleanup Also disabled a R# warning so I stop seeing it since it doesn't apply. --- src/GitHub.Exports/Services/GitService.cs | 2 -- src/GitHub.Exports/Services/Services.cs | 5 +++-- 2 files changed, 3 insertions(+), 4 deletions(-) diff --git a/src/GitHub.Exports/Services/GitService.cs b/src/GitHub.Exports/Services/GitService.cs index 4a9d9d63a2..2800aa869a 100644 --- a/src/GitHub.Exports/Services/GitService.cs +++ b/src/GitHub.Exports/Services/GitService.cs @@ -4,7 +4,6 @@ using GitHub.Primitives; using LibGit2Sharp; using Microsoft.VisualStudio.TeamFoundation.Git.Extensibility; -using GitHub.Extensions; namespace GitHub.Services { @@ -21,7 +20,6 @@ public class GitService : IGitService public UriString GetUri(IRepository repository) { return UriString.ToUriString(GetOriginUri(repository)?.ToRepositoryUrl()); - } /// diff --git a/src/GitHub.Exports/Services/Services.cs b/src/GitHub.Exports/Services/Services.cs index 1af09b5e54..717df6c6a8 100644 --- a/src/GitHub.Exports/Services/Services.cs +++ b/src/GitHub.Exports/Services/Services.cs @@ -50,7 +50,7 @@ public static IVsWebBrowsingService GetWebBrowsingService(this IServiceProvider public static IVsOutputWindow OutputWindow => GetGlobalService(); - static IVsOutputWindowPane outputWindowPane = null; + static IVsOutputWindowPane outputWindowPane; public static IVsOutputWindowPane OutputWindowPane { get @@ -61,7 +61,7 @@ public static IVsOutputWindowPane OutputWindowPane var uiShell = GetGlobalService(); // Get the frame of the output window var outputWindowGuid = new Guid("{34e76e81-ee4a-11d0-ae2e-00a0c90fffc3}"); - IVsWindowFrame outputWindowFrame = null; + IVsWindowFrame outputWindowFrame; ErrorHandler.ThrowOnFailure(uiShell.FindToolWindow((uint)__VSCREATETOOLWIN.CTW_fForceCreate, ref outputWindowGuid, out outputWindowFrame)); // Show the output window if (outputWindowFrame != null) @@ -78,6 +78,7 @@ public static IVsOutputWindowPane OutputWindowPane public static DTE Dte => GetGlobalService(); + // ReSharper disable once SuspiciousTypeConversion.Global public static DTE2 Dte2 => Dte as DTE2; public static IVsActivityLog GetActivityLog(this IServiceProvider provider)