Merge pull request #105 from github/shana/fix-gitservice-null

Fix IGitService

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;
         }
 

src/GitHub.Exports/GitHub.Exports.csproj

@@ -99,7 +99,6 @@
     <None Include="..\..\script\Key.snk" Condition="$(Buildtype) == 'Internal'">
       <Link>Key.snk</Link>
     </None>
-    <Compile Include="Extensions\GitHelpers.cs" />
     <Compile Include="Helpers\INotifyPropertySource.cs" />
     <Compile Include="Extensions\PropertyNotifierExtensions.cs" />
     <Compile Include="Extensions\SimpleRepositoryModelExtensions.cs" />

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;
         }

src/GitHub.Exports/Services/GitService.cs

@@ -7,52 +7,93 @@
 
 namespace GitHub.Services
 {
-    [Export(typeof(IVSServices))]
-    [PartCreationPolicy(CreationPolicy.Shared)]
+    [Export(typeof(IGitService))]
+    [PartCreationPolicy(CreationPolicy.NonShared)]
     public class GitService : IGitService
     {
         /// <summary>
         /// Returns the URL of the remote named "origin" for the specified <see cref="repository"/>. If the repository
         /// is null or no remote named origin exists, this method returns null
         /// </summary>
         /// <param name="repository">The repository to look at for the remote.</param>
-        /// <returns>A <see cref="UriString"/> representing the origin or null if none found.</returns>
+        /// <returns>Returns a <see cref="UriString"/> representing the uri of the "origin" remote normalized to a GitHub repository url or null if none found.</returns>
         public UriString GetUri(IRepository repository)
         {
-            return UriString.ToUriString(GetUriFromRepository(repository)?.ToRepositoryUrl());
+            return UriString.ToUriString(GetOriginUri(repository)?.ToRepositoryUrl());
         }
 
         /// <summary>
-        /// Probes for a git repository and if one is found, returns a <see cref="UriString"/> for the repository's
-        /// remote named "origin" if one is found
+        /// Returns a <see cref="UriString"/> representing the uri of the "origin" remote normalized to a GitHub repository url or null if none found.
+        /// </summary>
+        /// <param name="repository"></param>
+        /// <returns>Returns a <see cref="UriString"/> representing the uri of the "origin" remote normalized to a GitHub repository url or null if none found.</returns>
+        public static UriString GetGitHubUri(IRepository repository)
+        {
+            return GitServiceHelper.GetUri(repository);
+        }
+
+        /// <summary>
+        /// Probes for a git repository and if one is found, returns a normalized GitHub uri <see cref="UriString"/>
+        /// for the repository's remote named "origin" if one is found
         /// </summary>
         /// <remarks>
         /// The lookup checks to see if the specified <paramref name="path"/> 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.
         /// </remarks>
         /// <param name="path">The path to start probing</param>
-        /// <returns>A <see cref="UriString"/> representing the origin or null if none found.</returns>
+        /// <returns>Returns a <see cref="UriString"/> representing the uri of the "origin" remote normalized to a GitHub repository url or null if none found.</returns>
         public UriString GetUri(string path)
         {
             return GetUri(GetRepo(path));
         }
 
         /// <summary>
-        /// Probes for a git repository and if one is found, returns a <see cref="UriString"/> 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 <see cref="UriString"/>
+        /// for the repository's remote named "origin" if one is found
+        /// </summary>
+        /// <remarks>
+        /// The lookup checks to see if the specified <paramref name="path"/> 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.
+        /// </remarks>
+        /// <param name="path">The path to start probing</param>
+        /// <returns>Returns a <see cref="UriString"/> representing the uri of the "origin" remote normalized to a GitHub repository url or null if none found.</returns>
+        public static UriString GetUriFromPath(string path)
+        {
+            return GitServiceHelper.GetUri(path);
+        }
+
+        /// <summary>
+        /// Probes for a git repository and if one is found, returns a normalized GitHub uri
+        /// <see cref="UriString"/> for the repository's remote named "origin" if one is found
         /// </summary>
         /// <remarks>
         /// The lookup checks to see if the path specified by the RepositoryPath property of the specified
         /// <see cref="repoInfo"/> 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.
         /// </remarks>
         /// <param name="repoInfo">The repository information containing the path to start probing</param>
-        /// <returns>A <see cref="UriString"/> representing the origin or null if none found.</returns>
+        /// <returns>Returns a <see cref="UriString"/> representing the uri of the "origin" remote normalized to a GitHub repository url or null if none found.</returns>
         public UriString GetUri(IGitRepositoryInfo repoInfo)
         {
             return GetUri(GetRepo(repoInfo));
         }
 
+        /// <summary>
+        /// Probes for a git repository and if one is found, returns a normalized GitHub uri
+        /// <see cref="UriString"/> for the repository's remote named "origin" if one is found
+        /// </summary>
+        /// <remarks>
+        /// The lookup checks to see if the path specified by the RepositoryPath property of the specified
+        /// <see cref="repoInfo"/> 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.
+        /// </remarks>
+        /// <param name="repoInfo">The repository information containing the path to start probing</param>
+        /// <returns>Returns a <see cref="UriString"/> representing the uri of the "origin" remote normalized to a GitHub repository url or null if none found.</returns>
+        public static UriString GetUriFromVSGit(IGitRepositoryInfo repoInfo)
+        {
+            return GitServiceHelper.GetUri(repoInfo);
+        }
+
         /// <summary>
         /// Probes for a git repository and if one is found, returns a <see cref="IRepository"/> instance for the
         /// repository.
@@ -70,6 +111,22 @@ public IRepository GetRepo(IGitRepositoryInfo repoInfo)
             return GetRepo(repoInfo?.RepositoryPath);
         }
 
+        /// <summary>
+        /// Probes for a git repository and if one is found, returns a <see cref="IRepository"/> instance for the
+        /// repository.
+        /// </summary>
+        /// <remarks>
+        /// The lookup checks to see if the path specified by the RepositoryPath property of the specified
+        /// <see cref="repoInfo"/> 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.
+        /// </remarks>
+        /// <param name="repoInfo">The repository information containing the path to start probing</param>
+        /// <returns>An instance of <see cref="IRepository"/> or null</returns>
+        public static IRepository GetRepoFromVSGit(IGitRepositoryInfo repoInfo)
+        {
+            return GitServiceHelper.GetRepo(repoInfo);
+        }
+
         /// <summary>
         /// Probes for a git repository and if one is found, returns a <see cref="IRepository"/> instance for the
         /// repository.
@@ -86,13 +143,35 @@ public IRepository GetRepo(string path)
             return repoPath == null ? null : new Repository(repoPath);
         }
 
-        internal static UriString GetUriFromRepository(IRepository repo)
+        /// <summary>
+        /// Probes for a git repository and if one is found, returns a <see cref="IRepository"/> instance for the
+        /// repository.
+        /// </summary>
+        /// <remarks>
+        /// The lookup checks to see if the specified <paramref name="path"/> 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.
+        /// </remarks>
+        /// <param name="path">The path to start probing</param>
+        /// <returns>An instance of <see cref="IRepository"/> or null</returns>
+        public static IRepository GetRepoFromPath(string path)
+        {
+            return GitServiceHelper.GetRepo(path);
+        }
+
+        /// <summary>
+        /// Returns a <see cref="UriString"/> representing the uri of the "origin" remote with no modifications.
+        /// </summary>
+        /// <param name="repo"></param>
+        /// <returns></returns>
+        public static UriString GetOriginUri(IRepository repo)
         {
             return repo
                 ?.Network
                 .Remotes
                 .FirstOrDefault(x => x.Name.Equals("origin", StringComparison.Ordinal))
                 ?.Url;
         }
+
+        public static IGitService GitServiceHelper => VisualStudio.Services.DefaultExportProvider.GetExportedValueOrDefault<IGitService>() ?? new GitService();
     }
 }

src/GitHub.Exports/Services/Services.cs

@@ -1,14 +1,15 @@
 using System;
 using EnvDTE;
 using EnvDTE80;
-using GitHub.Extensions;
 using GitHub.Info;
 using GitHub.Primitives;
 using GitHub.Services;
 using LibGit2Sharp;
 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; }
 
         /// <summary>
-        /// Two ways of getting a service. First, trying the passed-in <paramref name="provider"/>,
-        /// then <see cref="PackageServiceProvider"/>
-        /// 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 <paramref name="provider"/>,
+        /// then <see cref="PackageServiceProvider"/>, then <see cref="T:Microsoft.VisualStudio.Shell.Package"/>
+        /// 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.
         /// </summary>
         /// <typeparam name="T"></typeparam>
         /// <typeparam name="Ret"></typeparam>
@@ -33,10 +35,13 @@ static Ret GetGlobalService<T, Ret>(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<SComponentModel, IComponentModel>();
+        public static ExportProvider DefaultExportProvider => ComponentModel.DefaultExportProvider;
 
         public static IVsWebBrowsingService GetWebBrowsingService(this IServiceProvider provider)
         {
@@ -45,7 +50,7 @@ public static IVsWebBrowsingService GetWebBrowsingService(this IServiceProvider
 
         public static IVsOutputWindow OutputWindow => GetGlobalService<SVsOutputWindow, IVsOutputWindow>();
 
-        static IVsOutputWindowPane outputWindowPane = null;
+        static IVsOutputWindowPane outputWindowPane;
         public static IVsOutputWindowPane OutputWindowPane
         {
             get
@@ -56,7 +61,7 @@ public static IVsOutputWindowPane OutputWindowPane
                     var uiShell = GetGlobalService<SVsUIShell, IVsUIShell>();
                     // 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)
@@ -73,6 +78,7 @@ public static IVsOutputWindowPane OutputWindowPane
 
         public static DTE Dte => GetGlobalService<DTE, DTE>();
 
+        // ReSharper disable once SuspiciousTypeConversion.Global
         public static DTE2 Dte2 => Dte as DTE2;
 
         public static IVsActivityLog GetActivityLog(this IServiceProvider provider)
@@ -92,13 +98,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 +108,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<IGitService>();
     }
 }

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<IRepository>();
             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<IRepository>();
             var path = Directory.CreateSubdirectory("repo-name");
             gitservice.GetUri(path.FullName).Returns(new UriString("https://github.com/user/repo-name"));