Merge branch 'zj-refs-hash' into 'master'

Don't use rugged in Repository#refs_hash

Closes gitaly#880

See merge request gitlab-org/gitlab-ce!16827

app/helpers/graph_helper.rb

@@ -1,10 +1,6 @@
 module GraphHelper
-  def get_refs(repo, commit)
-    refs = ""
-    # Commit::ref_names already strips the refs/XXX from important refs (e.g. refs/heads/XXX)
-    # so anything leftover is internally used by GitLab
-    commit_refs = commit.ref_names(repo).reject { |name| name.starts_with?('refs/') }
-    refs << commit_refs.join(' ')
+  def refs(repo, commit)
+    refs = commit.ref_names(repo).join(' ')
 
     # append note count
     notes_count = @graph.notes[commit.id]

app/views/projects/network/show.json.erb

@@ -13,7 +13,7 @@
         },
         time: c.time,
         space: c.spaces.first,
-        refs: get_refs(@graph.repo, c),
+        refs: refs(@graph.repo, c),
         id: c.sha,
         date: c.date,
         message: c.message,

lib/gitlab/git/commit.rb

@@ -402,15 +402,6 @@ module Gitlab
         end
       end
 
-      # Get a collection of Rugged::Reference objects for this commit.
-      #
-      # Ex.
-      #   commit.ref(repo)
-      #
-      def refs(repo)
-        repo.refs_hash[id]
-      end
-
       # Get ref names collection
       #
       # Ex.
@@ -418,7 +409,7 @@ module Gitlab
       #
       def ref_names(repo)
         refs(repo).map do |ref|
-          ref.name.sub(%r{^refs/(heads|remotes|tags)/}, "")
+          ref.sub(%r{^refs/(heads|remotes|tags)/}, "")
         end
       end
 
@@ -553,6 +544,15 @@ module Gitlab
           date: Google::Protobuf::Timestamp.new(seconds: author_or_committer[:time].to_i)
         )
       end
+
+      # Get a collection of Gitlab::Git::Ref objects for this commit.
+      #
+      # Ex.
+      #   commit.ref(repo)
+      #
+      def refs(repo)
+        repo.refs_hash[id]
+      end
     end
   end
 end

lib/gitlab/git/repository.rb

@@ -631,21 +631,18 @@ module Gitlab
         end
       end
 
-      # Get refs hash which key is SHA1
-      # and value is a Rugged::Reference
+      # Get refs hash which key is is the commit id
+      # and value is a Gitlab::Git::Tag or Gitlab::Git::Branch
+      # Note that both inherit from Gitlab::Git::Ref
       def refs_hash
-        # Initialize only when first call
-        if @refs_hash.nil?
-          @refs_hash = Hash.new { |h, k| h[k] = [] }
+        return @refs_hash if @refs_hash
 
-          rugged.references.each do |r|
-            # Symbolic/remote references may not have an OID; skip over them
-            target_oid = r.target.try(:oid)
-            if target_oid
-              sha = rev_parse_target(target_oid).oid
-              @refs_hash[sha] << r
-            end
-          end
+        @refs_hash = Hash.new { |h, k| h[k] = [] }
+
+        (tags + branches).each do |ref|
+          next unless ref.target && ref.name
+
+          @refs_hash[ref.dereferenced_target.id] << ref.name
         end
 
         @refs_hash

spec/helpers/graph_helper_spec.rb

@@ -7,10 +7,10 @@ describe GraphHelper do
     let(:graph) { Network::Graph.new(project, 'master', commit, '') }
 
     it 'filters our refs used by GitLab' do
-      allow(commit).to receive(:ref_names).and_return(['refs/merge-requests/abc', 'master', 'refs/tmp/xyz'])
       self.instance_variable_set(:@graph, graph)
-      refs = get_refs(project.repository, commit)
-      expect(refs).to eq('master')
+      refs = refs(project.repository, commit)
+
+      expect(refs).to match('master')
     end
   end
 end

spec/lib/gitlab/git/repository_spec.rb

@@ -600,12 +600,16 @@ describe Gitlab::Git::Repository, seed_helper: true do
   end
 
   describe "#refs_hash" do
-    let(:refs) { repository.refs_hash }
+    subject { repository.refs_hash }
 
     it "should have as many entries as branches and tags" do
       expected_refs = SeedRepo::Repo::BRANCHES + SeedRepo::Repo::TAGS
       # We flatten in case a commit is pointed at by more than one branch and/or tag
-      expect(refs.values.flatten.size).to eq(expected_refs.size)
+      expect(subject.values.flatten.size).to eq(expected_refs.size)
+    end
+
+    it 'has valid commit ids as keys' do
+      expect(subject.keys).to all( match(Commit::COMMIT_SHA_PATTERN) )
     end
   end