Test initial changes on short-circuiting SHA fetchingosw-gitaly-find-commit-n-plus-1-fix

4 files changed, 50 insertions, 31 deletions

diff --git a/app/models/compare.rb b/app/models/compare.rb
index feb4b89c781..c0f8225a3fb 100644
--- a/app/models/compare.rb
+++ b/app/models/compare.rb
@@ -24,37 +24,37 @@ class Compare
     @commits ||= Commit.decorate(@compare.commits, project)
   end
 
-  def start_commit
-    strong_memoize(:start_commit) do
-      commit = @compare.base
-
-      ::Commit.new(commit, project) if commit
-    end
-  end
-
-  def head_commit
-    strong_memoize(:head_commit) do
-      commit = @compare.head
-
-      ::Commit.new(commit, project) if commit
-    end
-  end
-  alias_method :commit, :head_commit
+  # def start_commit
+  #   strong_memoize(:start_commit) do
+  #     commit = @compare.base
+  #
+  #     ::Commit.new(commit, project) if commit
+  #   end
+  # end
+
+  # def head_commit
+  #   strong_memoize(:head_commit) do
+  #     commit = @compare.head
+  #
+  #     ::Commit.new(commit, project) if commit
+  #   end
+  # end
+  # alias_method :commit, :head_commit
 
   def start_commit_sha
-    start_commit&.sha
+    @compare.base
   end
 
   def base_commit_sha
     strong_memoize(:base_commit) do
-      next unless start_commit && head_commit
+      next unless start_commit_sha && head_commit_sha
 
-      @base_sha || project.merge_base_commit(start_commit.id, head_commit.id)&.sha
+      @base_sha || project.merge_base_commit(start_commit_sha, head_commit_sha)&.sha
     end
   end
 
   def head_commit_sha
-    commit&.sha
+    @compare.head
   end
 
   def raw_diffs(*args)
diff --git a/lib/gitlab/diff/diff_refs.rb b/lib/gitlab/diff/diff_refs.rb
index 81df47964be..d6de5b7a923 100644
--- a/lib/gitlab/diff/diff_refs.rb
+++ b/lib/gitlab/diff/diff_refs.rb
@@ -46,8 +46,8 @@ module Gitlab
           straight = start_sha == base_sha
 
           CompareService.new(project, head_sha).execute(project,
-                                                        start_sha,
-                                                        base_sha: base_sha,
+                                                        start_sha, # target_ref
+                                                        base_sha: base_sha, # base_sha
                                                         straight: straight)
         end
       end
diff --git a/lib/gitlab/git/compare.rb b/lib/gitlab/git/compare.rb
index 7cb842256d0..eb1864ea93a 100644
--- a/lib/gitlab/git/compare.rb
+++ b/lib/gitlab/git/compare.rb
@@ -8,27 +8,26 @@ module Gitlab
       def initialize(repository, base, head, straight: false)
         @repository = repository
         @straight = straight
+        @base = base
+        @head = head
 
         unless base && head
           @commits = []
           return
         end
 
-        @base = Gitlab::Git::Commit.find(repository, base.try(:strip))
-        @head = Gitlab::Git::Commit.find(repository, head.try(:strip))
-
         @commits = [] unless @base && @head
         @commits = [] if same
       end
 
       def same
-        @base && @head && @base.id == @head.id
+        @base && @head && @base == @head
       end
 
       def commits
         return @commits if defined?(@commits)
 
-        @commits = Gitlab::Git::Commit.between(@repository, @base.id, @head.id)
+        @commits = Gitlab::Git::Commit.between(@repository, @base, @head)
       end
 
       def diffs(options = {})
@@ -38,7 +37,7 @@ module Gitlab
 
         paths = options.delete(:paths) || []
         options[:straight] = @straight
-        Gitlab::Git::Diff.between(@repository, @head.id, @base.id, options, *paths)
+        Gitlab::Git::Diff.between(@repository, @head, @base, options, *paths)
       end
     end
   end
diff --git a/lib/gitlab/git/repository.rb b/lib/gitlab/git/repository.rb
index b145001a024..b7ac4b05403 100644
--- a/lib/gitlab/git/repository.rb
+++ b/lib/gitlab/git/repository.rb
@@ -1130,6 +1130,7 @@ module Gitlab
         end
       end
 
+      # What is it trying to fetch?
       def with_repo_branch_commit(start_repository, start_branch_name)
         Gitlab::Git.check_namespace!(start_repository)
         start_repository = RemoteRepository.new(start_repository) unless start_repository.is_a?(RemoteRepository)
@@ -1183,15 +1184,34 @@ module Gitlab
       end
 
       def compare_source_branch(target_branch_name, source_repository, source_branch_name, straight:)
-        with_repo_branch_commit(source_repository, source_branch_name) do |commit|
-          break unless commit
+        return unless source_branch_name
 
+        # Copying a few things from with_repo_branch_commit method.
+        # TODO: Refactor
+        #
+        Gitlab::Git.check_namespace!(source_repository)
+        source_repository = RemoteRepository.new(source_repository) unless source_repository.is_a?(RemoteRepository)
+
+        return if source_repository.empty?
+
+        if source_repository.same_repository?(self) && ::Commit.valid_hash?(source_branch_name)
           Gitlab::Git::Compare.new(
             self,
             target_branch_name,
-            commit.sha,
+            source_branch_name,
             straight: straight
           )
+        else
+          with_repo_branch_commit(source_repository, source_branch_name) do |commit|
+            break unless commit
+
+            Gitlab::Git::Compare.new(
+              self,
+              target_branch_name,
+              commit.sha,
+              straight: straight
+            )
+          end
         end
       end