Commit 73e78c4e authored by Zeger-Jan van de Weg's avatar Zeger-Jan van de Weg

Don't use rugged in Repository#refs_hash

The refs hash is used to determine what branches and tags have a commit
as head in the network graph. The previous implementation depended on
Rugged#references. The problem with this implementation was that it
depended on rugged, but also that it iterated over all references and
thus loading more data than needed if for example the project uses CI/CD
environments, Pipelines, or Merge Requests.

Given only refs are checked the network cares about the GraphHelper#refs
method has no need to reject those, simplifying the method.

Closes gitlab-org/gitaly#880
parent 86342966
module GraphHelper module GraphHelper
def get_refs(repo, commit) def refs(repo, commit)
refs = "" refs = commit.ref_names(repo).join(' ')
# 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(' ')
# append note count # append note count
notes_count = @graph.notes[commit.id] notes_count = @graph.notes[commit.id]
......
...@@ -13,7 +13,7 @@ ...@@ -13,7 +13,7 @@
}, },
time: c.time, time: c.time,
space: c.spaces.first, space: c.spaces.first,
refs: get_refs(@graph.repo, c), refs: refs(@graph.repo, c),
id: c.sha, id: c.sha,
date: c.date, date: c.date,
message: c.message, message: c.message,
......
...@@ -402,15 +402,6 @@ module Gitlab ...@@ -402,15 +402,6 @@ module Gitlab
end end
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 # Get ref names collection
# #
# Ex. # Ex.
...@@ -418,7 +409,7 @@ module Gitlab ...@@ -418,7 +409,7 @@ module Gitlab
# #
def ref_names(repo) def ref_names(repo)
refs(repo).map do |ref| refs(repo).map do |ref|
ref.name.sub(%r{^refs/(heads|remotes|tags)/}, "") ref.sub(%r{^refs/(heads|remotes|tags)/}, "")
end end
end end
...@@ -553,6 +544,15 @@ module Gitlab ...@@ -553,6 +544,15 @@ module Gitlab
date: Google::Protobuf::Timestamp.new(seconds: author_or_committer[:time].to_i) date: Google::Protobuf::Timestamp.new(seconds: author_or_committer[:time].to_i)
) )
end 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 end
end end
...@@ -627,21 +627,18 @@ module Gitlab ...@@ -627,21 +627,18 @@ module Gitlab
end end
end end
# Get refs hash which key is SHA1 # Get refs hash which key is is the commit id
# and value is a Rugged::Reference # and value is a Gitlab::Git::Tag or Gitlab::Git::Branch
# Note that both inherit from Gitlab::Git::Ref
def refs_hash def refs_hash
# Initialize only when first call return @refs_hash if @refs_hash
if @refs_hash.nil?
@refs_hash = Hash.new { |h, k| h[k] = [] } @refs_hash = Hash.new { |h, k| h[k] = [] }
rugged.references.each do |r| (tags + branches).each do |ref|
# Symbolic/remote references may not have an OID; skip over them next unless ref.target && ref.name
target_oid = r.target.try(:oid)
if target_oid @refs_hash[ref.dereferenced_target.id] << ref.name
sha = rev_parse_target(target_oid).oid
@refs_hash[sha] << r
end
end
end end
@refs_hash @refs_hash
......
...@@ -7,10 +7,10 @@ describe GraphHelper do ...@@ -7,10 +7,10 @@ describe GraphHelper do
let(:graph) { Network::Graph.new(project, 'master', commit, '') } let(:graph) { Network::Graph.new(project, 'master', commit, '') }
it 'filters our refs used by GitLab' do 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) self.instance_variable_set(:@graph, graph)
refs = get_refs(project.repository, commit) refs = refs(project.repository, commit)
expect(refs).to eq('master')
expect(refs).to match('master')
end end
end end
end end
...@@ -600,12 +600,16 @@ describe Gitlab::Git::Repository, seed_helper: true do ...@@ -600,12 +600,16 @@ describe Gitlab::Git::Repository, seed_helper: true do
end end
describe "#refs_hash" do describe "#refs_hash" do
let(:refs) { repository.refs_hash } subject { repository.refs_hash }
it "should have as many entries as branches and tags" do it "should have as many entries as branches and tags" do
expected_refs = SeedRepo::Repo::BRANCHES + SeedRepo::Repo::TAGS 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 # 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
end end
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment