Move Compare and CompareWith to IGitService

IGitService knows about IRepository but not remote servers. IGitClient knows about remote git repositories. IGitService is a more appropriate home for Compare and CompareWith.
github · StanleyGoldman · Mar 26, 2019 · Dec 18, 2018 · Dec 18, 2018 · Dec 18, 2018
commit 62ccc9bd341f9531b2929eb9c161a6d244e7042a
diff --git a/src/GitHub.App/SampleData/GitServiceDesigner.cs b/src/GitHub.App/SampleData/GitServiceDesigner.cs
@@ -15,5 +15,7 @@ class GitServiceDesigner : IGitService
         public IRepository GetRepository(string path) => null;
         public UriString GetUri(string path, string remote = "origin") => null;
         public UriString GetUri(IRepository repository, string remote = "origin") => null;
+        public Task<Patch> Compare(IRepository repository, string sha1, string sha2, string path) => null;
+        public Task<ContentChanges> CompareWith(IRepository repository, string sha1, string sha2, string path, byte[] contents) => null;
     }
 }
diff --git a/src/GitHub.App/Services/GitClient.cs b/src/GitHub.App/Services/GitClient.cs
@@ -231,66 +231,6 @@ public Task<TreeChanges> Compare(
             });
         }
 
-        public Task<Patch> Compare(
-            IRepository repository,
-            string sha1,
-            string sha2,
-            string path)
-        {
-            Guard.ArgumentNotNull(repository, nameof(repository));
-            Guard.ArgumentNotEmptyString(sha1, nameof(sha1));
-            Guard.ArgumentNotEmptyString(sha2, nameof(sha2));
-            Guard.ArgumentNotEmptyString(path, nameof(path));
-
-            return Task.Run(() =>
-            {
-                var commit1 = repository.Lookup<Commit>(sha1);
-                var commit2 = repository.Lookup<Commit>(sha2);
-
-                if (commit1 != null && commit2 != null)
-                {
-                    return repository.Diff.Compare<Patch>(
-                        commit1.Tree,
-                        commit2.Tree,
-                        new[] { path },
-                        compareOptions);
-                }
-                else
-                {
-                    return null;
-                }
-            });
-        }
-
-        public Task<ContentChanges> CompareWith(IRepository repository, string sha1, string sha2, string path, byte[] contents)
-        {
-            Guard.ArgumentNotNull(repository, nameof(repository));
-            Guard.ArgumentNotEmptyString(sha1, nameof(sha1));
-            Guard.ArgumentNotEmptyString(sha2, nameof(sha1));
-            Guard.ArgumentNotEmptyString(path, nameof(path));
-
-            return Task.Run(() =>
-            {
-                var commit1 = repository.Lookup<Commit>(sha1);
-                var commit2 = repository.Lookup<Commit>(sha2);
-
-                var treeChanges = repository.Diff.Compare<TreeChanges>(commit1.Tree, commit2.Tree, compareOptions);
-                var normalizedPath = path.Replace("/", "\\");
-                var renamed = treeChanges.FirstOrDefault(x => x.Path == normalizedPath);
-                var oldPath = renamed?.OldPath ?? path;
-
-                if (commit1 != null)
-                {
-                    var contentStream = contents != null ? new MemoryStream(contents) : new MemoryStream();
-                    var blob1 = commit1[oldPath]?.Target as Blob ?? repository.ObjectDatabase.CreateBlob(new MemoryStream());
-                    var blob2 = repository.ObjectDatabase.CreateBlob(contentStream, path);
-                    return repository.Diff.Compare(blob1, blob2, compareOptions);
-                }
-
-                return null;
-            });
-        }
-
         public Task<T> GetConfig<T>(IRepository repository, string key)
         {
             Guard.ArgumentNotNull(repository, nameof(repository));

diff --git a/src/GitHub.Exports.Reactive/Services/IGitClient.cs b/src/GitHub.Exports.Reactive/Services/IGitClient.cs
@@ -85,31 +85,6 @@ public interface IGitClient
         /// </returns>
         Task<TreeChanges> Compare(IRepository repository, string sha1, string sha2, bool detectRenames = false);
 
-        /// <summary>
-        /// Compares a file in two commits.
-        /// </summary>
-        /// <param name="repository">The repository</param>
-        /// <param name="sha1">The SHA of the first commit.</param>
-        /// <param name="sha2">The SHA of the second commit.</param>
-        /// <param name="path">The relative path to the file.</param>
-        /// <returns>
-        /// A <see cref="Patch"/> object or null if one of the commits could not be found in the repository.
-        /// </returns>
-        Task<Patch> Compare(IRepository repository, string sha1, string sha2, string path);
-
-        /// <summary>
-        /// Compares a file in a commit to a string.
-        /// </summary>
-        /// <param name="repository">The repository</param>
-        /// <param name="sha1">The SHA of the first commit.</param>
-        /// <param name="sha2">The SHA of the second commit.</param>
-        /// <param name="path">The relative path to the file.</param>
-        /// <param name="contents">The contents to compare with the file.</param>
-        /// <returns>
-        /// A <see cref="Patch"/> object or null if the commit could not be found in the repository.
-        /// </returns>
-        Task<ContentChanges> CompareWith(IRepository repository, string sha1, string sha2, string path, byte[] contents);
-
         /// <summary>
         /// Gets the value of a configuration key.
         /// </summary>

diff --git a/src/GitHub.Exports/Services/GitService.cs b/src/GitHub.Exports/Services/GitService.cs
@@ -2,7 +2,6 @@
 using System.IO;
 using System.Linq;
 using System.Threading.Tasks;
-using System.Collections.Generic;
 using System.ComponentModel.Composition;
 using GitHub.UI;
 using GitHub.Models;
@@ -17,11 +16,13 @@ namespace GitHub.Services
     public class GitService : IGitService
     {
         readonly IRepositoryFacade repositoryFacade;
+        readonly CompareOptions defaultCompareOptions;
 
         [ImportingConstructor]
         public GitService(IRepositoryFacade repositoryFacade)
         {
             this.repositoryFacade = repositoryFacade;
+            defaultCompareOptions = new CompareOptions();
         }
 
         /// <summary>
@@ -228,5 +229,65 @@ public Task<string> GetLatestPushedSha(string path, string remote = "origin")
                 }
             });
         }
+
+        public Task<Patch> Compare(
+            IRepository repository,
+            string sha1,
+            string sha2,
+            string path)
+        {
+            Guard.ArgumentNotNull(repository, nameof(repository));
+            Guard.ArgumentNotEmptyString(sha1, nameof(sha1));
+            Guard.ArgumentNotEmptyString(sha2, nameof(sha2));
+            Guard.ArgumentNotEmptyString(path, nameof(path));
+
+            return Task.Run(() =>
+            {
+                var commit1 = repository.Lookup<Commit>(sha1);
+                var commit2 = repository.Lookup<Commit>(sha2);
+
+                if (commit1 != null && commit2 != null)
+                {
+                    return repository.Diff.Compare<Patch>(
+                        commit1.Tree,
+                        commit2.Tree,
+                        new[] { path },
+                        defaultCompareOptions);
+                }
+                else
+                {
+                    return null;
+                }
+            });
+        }
+
+        public Task<ContentChanges> CompareWith(IRepository repository, string sha1, string sha2, string path, byte[] contents)
+        {
+            Guard.ArgumentNotNull(repository, nameof(repository));
+            Guard.ArgumentNotEmptyString(sha1, nameof(sha1));
+            Guard.ArgumentNotEmptyString(sha2, nameof(sha1));
+            Guard.ArgumentNotEmptyString(path, nameof(path));
+
+            return Task.Run(() =>
+            {
+                var commit1 = repository.Lookup<Commit>(sha1);
+                var commit2 = repository.Lookup<Commit>(sha2);
+
+                var treeChanges = repository.Diff.Compare<TreeChanges>(commit1.Tree, commit2.Tree, defaultCompareOptions);
+                var normalizedPath = path.Replace("/", "\\");
+                var renamed = treeChanges.FirstOrDefault(x => x.Path == normalizedPath);
+                var oldPath = renamed?.OldPath ?? path;
+
+                if (commit1 != null)
+                {
+                    var contentStream = contents != null ? new MemoryStream(contents) : new MemoryStream();
+                    var blob1 = commit1[oldPath]?.Target as Blob ?? repository.ObjectDatabase.CreateBlob(new MemoryStream());
+                    var blob2 = repository.ObjectDatabase.CreateBlob(contentStream, path);
+                    return repository.Diff.Compare(blob1, blob2, defaultCompareOptions);
+                }
+
+                return null;
+            });
+        }
     }
 }
diff --git a/src/GitHub.Exports/Services/IGitService.cs b/src/GitHub.Exports/Services/IGitService.cs
@@ -71,5 +71,30 @@ public interface IGitService
         /// <param name="remote">The remote name to look for</param>
         /// <returns></returns>
         Task<string> GetLatestPushedSha(string path, string remote = "origin");
+
+        /// <summary>
+        /// Compares a file in two commits.
+        /// </summary>
+        /// <param name="repository">The repository</param>
+        /// <param name="sha1">The SHA of the first commit.</param>
+        /// <param name="sha2">The SHA of the second commit.</param>
+        /// <param name="path">The relative path to the file.</param>
+        /// <returns>
+        /// A <see cref="Patch"/> object or null if one of the commits could not be found in the repository.
+        /// </returns>
+        Task<Patch> Compare(IRepository repository, string sha1, string sha2, string path);
+
+        /// <summary>
+        /// Compares a file in a commit to a string.
+        /// </summary>
+        /// <param name="repository">The repository</param>
+        /// <param name="sha1">The SHA of the first commit.</param>
+        /// <param name="sha2">The SHA of the second commit.</param>
+        /// <param name="path">The relative path to the file.</param>
+        /// <param name="contents">The contents to compare with the file.</param>
+        /// <returns>
+        /// A <see cref="Patch"/> object or null if the commit could not be found in the repository.
+        /// </returns>
+        Task<ContentChanges> CompareWith(IRepository repository, string sha1, string sha2, string path, byte[] contents);
     }
 }
diff --git a/src/GitHub.InlineReviews/Services/DiffService.cs b/src/GitHub.InlineReviews/Services/DiffService.cs
@@ -16,12 +16,12 @@ namespace GitHub.InlineReviews.Services
     [PartCreationPolicy(CreationPolicy.NonShared)]
     public class DiffService : IDiffService
     {
-        readonly IGitClient gitClient;
+        readonly IGitService gitService;
 
         [ImportingConstructor]
-        public DiffService(IGitClient gitClient)
+        public DiffService(IGitService gitService)
         {
-            this.gitClient = gitClient;
+            this.gitService = gitService;
         }
 
         /// <inheritdoc/>
@@ -31,15 +31,15 @@ public async Task<IReadOnlyList<DiffChunk>> Diff(
             string headSha,
             string path)
         {
-            var patch = await gitClient.Compare(repo, baseSha, headSha, path);
+            var patch = await gitService.Compare(repo, baseSha, headSha, path);
 
             if (patch != null)
             {
                 return DiffUtilities.ParseFragment(patch).ToList();
             }
             else
             {
-                return new DiffChunk[0];
+                return Array.Empty<DiffChunk>();
             }
         }
 
@@ -51,15 +51,15 @@ public async Task<IReadOnlyList<DiffChunk>> Diff(
             string path,
             byte[] contents)
         {
-            var changes = await gitClient.CompareWith(repo, baseSha, headSha, path, contents);
+            var changes = await gitService.CompareWith(repo, baseSha, headSha, path, contents);
 
             if (changes?.Patch != null)
             {
                 return DiffUtilities.ParseFragment(changes.Patch).ToList();
             }
             else
             {
-                return new DiffChunk[0];
+                return Array.Empty<DiffChunk>();
             }
         }
     }

diff --git a/test/GitHub.InlineReviews.UnitTests/Services/DiffServiceTests.cs b/test/GitHub.InlineReviews.UnitTests/Services/DiffServiceTests.cs
@@ -17,7 +17,7 @@ public class TheParseFragmentMethod
             [Test]
             public void ShouldParsePr960()
             {
-                var target = new DiffService(Substitute.For<IGitClient>());
+                var target = new DiffService(Substitute.For<IGitService>());
                 var result = DiffUtilities.ParseFragment(Properties.Resources.pr_960_diff).ToList();
 
                 Assert.That(4, Is.EqualTo(result.Count));

diff --git a/test/GitHub.InlineReviews.UnitTests/TestDoubles/FakeDiffService.cs b/test/GitHub.InlineReviews.UnitTests/TestDoubles/FakeDiffService.cs
@@ -22,13 +22,13 @@ sealed class FakeDiffService : IDiffService, IDisposable
         public FakeDiffService()
         {
             this.repository = CreateRepository();
-            this.inner = new DiffService(Substitute.For<IGitClient>());
+            this.inner = new DiffService(Substitute.For<IGitService>());
         }
 
         public FakeDiffService(string path, string contents)
         {
             this.repository = CreateRepository();
-            this.inner = new DiffService(Substitute.For<IGitClient>());
+            this.inner = new DiffService(Substitute.For<IGitService>());
             AddFile(path, contents);
         }
 

diff --git a/test/GitHub.TeamFoundation.UnitTests/VSGitExtTests.cs b/test/GitHub.TeamFoundation.UnitTests/VSGitExtTests.cs
@@ -319,5 +319,7 @@ public LocalRepositoryModel CreateLocalRepositoryModel(string localPath)
         public IRepository GetRepository(string path) => throw new NotImplementedException();
         public UriString GetUri(IRepository repository, string remote = "origin") => throw new NotImplementedException();
         public UriString GetUri(string path, string remote = "origin") => throw new NotImplementedException();
+        public Task<Patch> Compare(IRepository repository, string sha1, string sha2, string path) => throw new NotImplementedException();
+        public Task<ContentChanges> CompareWith(IRepository repository, string sha1, string sha2, string path, byte[] contents) => throw new NotImplementedException();
     }
 }