Make commit lookups explicit

lib/gitlab/git/branch.rb

@@ -3,39 +3,8 @@
 module Gitlab
   module Git
     class Branch < Ref
-      def initialize(repository, name, target)
-        if target.is_a?(Gitaly::FindLocalBranchResponse)
-          target = target_from_gitaly_local_branches_response(target)
-        end
-
-        super(repository, name, target)
-      end
-
-      def target_from_gitaly_local_branches_response(response)
-        # Git messages have no encoding enforcements. However, in the UI we only
-        # handle UTF-8, so basically we cross our fingers that the message force
-        # encoded to UTF-8 is readable.
-        message = response.commit_subject.dup.force_encoding('UTF-8')
-
-        # NOTE: For ease of parsing in Gitaly, we have only the subject of
-        # the commit and not the full message. This is ok, since all the
-        # code that uses `local_branches` only cares at most about the
-        # commit message.
-        # TODO: Once gitaly "takes over" Rugged consider separating the
-        # subject from the message to make it clearer when there's one
-        # available but not the other.
-        hash = {
-          id: response.commit_id,
-          message: message,
-          authored_date: Time.at(response.commit_author.date.seconds),
-          author_name: response.commit_author.name,
-          author_email: response.commit_author.email,
-          committed_date: Time.at(response.commit_committer.date.seconds),
-          committer_name: response.commit_committer.name,
-          committer_email: response.commit_committer.email
-        }
-
-        Gitlab::Git::Commit.decorate(hash)
+      def initialize(repository, name, target, target_commit)
+        super(repository, name, target, target_commit)
       end
     end
   end

lib/gitlab/git/ref.rb

@@ -33,10 +33,10 @@ module Gitlab
         object
       end
-      def initialize(repository, name, target)
+      def initialize(repository, name, target, derefenced_target)
         encode! name
         @name = name.gsub(/\Arefs\/(tags|heads)\//, '')
-        @dereferenced_target = Gitlab::Git::Commit.find(repository, target)
+        @dereferenced_target = derefenced_target
         @target = if target.respond_to?(:oid)
                     target.oid
                   elsif target.respond_to?(:name)

lib/gitlab/git/repository.rb

@@ -99,7 +99,10 @@ module Gitlab
         reload_rugged if force_reload
         rugged_ref = rugged.branches[name]
-        Gitlab::Git::Branch.new(self, rugged_ref.name, rugged_ref.target) if rugged_ref
+        if rugged_ref
+          target_commit = Gitlab::Git::Commit.find(self, rugged_ref.target)
+          Gitlab::Git::Branch.new(self, rugged_ref.name, rugged_ref.target, target_commit)
+        end
       end
       def local_branches(sort_by: nil)
@@ -166,7 +169,8 @@ module Gitlab
             end
           end
-          Gitlab::Git::Tag.new(self, ref.name, ref.target, message)
+          target_commit = Gitlab::Git::Commit.find(self, ref.target)
+          Gitlab::Git::Tag.new(self, ref.name, ref.target, target_commit, message)
         end.sort_by(&:name)
       end
@@ -692,7 +696,8 @@ module Gitlab
       #   create_branch("other-feature", "master")
       def create_branch(ref, start_point = "HEAD")
         rugged_ref = rugged.branches.create(ref, start_point)
-        Gitlab::Git::Branch.new(self, rugged_ref.name, rugged_ref.target)
+        target_commit = Gitlab::Git::Commit.find(self, rugged_ref.target)
+        Gitlab::Git::Branch.new(self, rugged_ref.name, rugged_ref.target, target_commit)
       rescue Rugged::ReferenceError => e
         raise InvalidRef.new("Branch #{ref} already exists") if e.to_s =~ /'refs\/heads\/#{ref}'/
         raise InvalidRef.new("Invalid reference #{start_point}")
@@ -826,7 +831,8 @@ module Gitlab
       def branches_filter(filter: nil, sort_by: nil)
         branches = rugged.branches.each(filter).map do |rugged_ref|
           begin
-            Gitlab::Git::Branch.new(self, rugged_ref.name, rugged_ref.target)
+            target_commit = Gitlab::Git::Commit.find(self, rugged_ref.target)
+            Gitlab::Git::Branch.new(self, rugged_ref.name, rugged_ref.target, target_commit)
           rescue Rugged::ReferenceError
             # Omit invalid branch
           end

lib/gitlab/git/tag.rb

@@ -5,8 +5,8 @@ module Gitlab
     class Tag < Ref
       attr_reader :object_sha
-      def initialize(repository, name, target, message = nil)
-        super(repository, name, target)
+      def initialize(repository, name, target, target_commit, message = nil)
+        super(repository, name, target, target_commit)
         @message = message
       end

lib/gitlab/gitaly_client/ref.rb

@@ -72,11 +72,39 @@ module Gitlab
             Gitlab::Git::Branch.new(
               @repository,
               encode!(gitaly_branch.name.dup),
-              gitaly_branch.commit_id
+              gitaly_branch.commit_id,
+              commit_from_local_branches_response(gitaly_branch)
             )
           end
         end
       end
+
+      def commit_from_local_branches_response(response)
+        # Git messages have no encoding enforcements. However, in the UI we only
+        # handle UTF-8, so basically we cross our fingers that the message force
+        # encoded to UTF-8 is readable.
+        message = response.commit_subject.dup.force_encoding('UTF-8')
+
+        # NOTE: For ease of parsing in Gitaly, we have only the subject of
+        # the commit and not the full message. This is ok, since all the
+        # code that uses `local_branches` only cares at most about the
+        # commit message.
+        # TODO: Once gitaly "takes over" Rugged consider separating the
+        # subject from the message to make it clearer when there's one
+        # available but not the other.
+        hash = {
+          id: response.commit_id,
+          message: message,
+          authored_date: Time.at(response.commit_author.date.seconds),
+          author_name: response.commit_author.name,
+          author_email: response.commit_author.email,
+          committed_date: Time.at(response.commit_committer.date.seconds),
+          committer_name: response.commit_committer.name,
+          committer_email: response.commit_committer.email
+        }
+
+        Gitlab::Git::Commit.decorate(hash)
+      end
     end
   end
 end

spec/lib/gitlab/git/branch_spec.rb

@@ -7,51 +7,6 @@ describe Gitlab::Git::Branch, seed_helper: true do
   it { is_expected.to be_kind_of Array }
-  describe 'initialize' do
-    let(:commit_id) { 'f00' }
-    let(:commit_subject) { "My commit".force_encoding('ASCII-8BIT') }
-    let(:committer) do
-      Gitaly::FindLocalBranchCommitAuthor.new(
-        name: generate(:name),
-        email: generate(:email),
-        date: Google::Protobuf::Timestamp.new(seconds: 123)
-      )
-    end
-    let(:author) do
-      Gitaly::FindLocalBranchCommitAuthor.new(
-        name: generate(:name),
-        email: generate(:email),
-        date: Google::Protobuf::Timestamp.new(seconds: 456)
-      )
-    end
-    let(:gitaly_branch) do
-      Gitaly::FindLocalBranchResponse.new(
-        name: 'foo', commit_id: commit_id, commit_subject: commit_subject,
-        commit_author: author, commit_committer: committer
-      )
-    end
-    let(:attributes) do
-      {
-        id: commit_id,
-        message: commit_subject,
-        authored_date: Time.at(author.date.seconds),
-        author_name: author.name,
-        author_email: author.email,
-        committed_date: Time.at(committer.date.seconds),
-        committer_name: committer.name,
-        committer_email: committer.email
-      }
-    end
-    let(:branch) { described_class.new(repository, 'foo', gitaly_branch) }
-
-    it 'parses Gitaly::FindLocalBranchResponse correctly' do
-      expect(Gitlab::Git::Commit).to receive(:decorate)
-        .with(hash_including(attributes)).and_call_original
-
-      expect(branch.dereferenced_target.message).to be_utf8
-    end
-  end
-
   describe '#size' do
     subject { super().size }
     it { is_expected.to eq(SeedRepo::Repo::BRANCHES.size) }