diff --git a/app/helpers/graph_helper.rb b/app/helpers/graph_helper.rb index 6d303ba857d..1022070ab6f 100644 --- a/app/helpers/graph_helper.rb +++ b/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] diff --git a/app/views/projects/network/show.json.erb b/app/views/projects/network/show.json.erb index 122e84b41b2..7491b37310d 100644 --- a/app/views/projects/network/show.json.erb +++ b/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, diff --git a/lib/gitlab/git/commit.rb b/lib/gitlab/git/commit.rb index 768617e2cae..d95561fe1b2 100644 --- a/lib/gitlab/git/commit.rb +++ b/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 diff --git a/lib/gitlab/git/repository.rb b/lib/gitlab/git/repository.rb index d7510061def..6761fb0937a 100644 --- a/lib/gitlab/git/repository.rb +++ b/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 diff --git a/spec/helpers/graph_helper_spec.rb b/spec/helpers/graph_helper_spec.rb index 400635abdde..1f8a38dc697 100644 --- a/spec/helpers/graph_helper_spec.rb +++ b/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 diff --git a/spec/lib/gitlab/git/repository_spec.rb b/spec/lib/gitlab/git/repository_spec.rb index ec1c7a96f92..edcf8889c27 100644 --- a/spec/lib/gitlab/git/repository_spec.rb +++ b/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