Commit ca696175 authored by Alejandro Rodríguez's avatar Alejandro Rodríguez

Optimize Banzai::Filter::RelativeLinkFilter

    A lot of git operations were being repeated, for example, to build a url
    you would ask if the path was a Tree, which would call a recursive routine
    in Gitlab::Git::Tree#where, then ask if the path was a Blob, which would
    call a recursive routine at Gitlab::Git::Blob#find, making reference to
    the same git objects several times. Now we call Rugged::Tree#path, which
    allows us to determine the type of the path in one pass.

    Some other minor improvement added, like saving commonly used references
    instead of calculating them each time.
parent 5804b6a0
...@@ -8,6 +8,7 @@ v 8.9.0 (unreleased) ...@@ -8,6 +8,7 @@ v 8.9.0 (unreleased)
- Bulk assign/unassign labels to issues. - Bulk assign/unassign labels to issues.
- Ability to prioritize labels !4009 / !3205 (Thijs Wouters) - Ability to prioritize labels !4009 / !3205 (Thijs Wouters)
- Show Star and Fork buttons on mobile. - Show Star and Fork buttons on mobile.
- Performance improvements on RelativeLinkFilter
- Fix endless redirections when accessing user OAuth applications when they are disabled - Fix endless redirections when accessing user OAuth applications when they are disabled
- Allow enabling wiki page events from Webhook management UI - Allow enabling wiki page events from Webhook management UI
- Bump rouge to 1.11.0 - Bump rouge to 1.11.0
......
...@@ -271,6 +271,32 @@ class Commit ...@@ -271,6 +271,32 @@ class Commit
merged_merge_request ? 'merge request' : 'commit' merged_merge_request ? 'merge request' : 'commit'
end end
# Get the URI type of the given path
#
# Used to build URLs to files in the repository in GFM.
#
# path - String path to check
#
# Examples:
#
# uri_type('doc/README.md') # => :blob
# uri_type('doc/logo.png') # => :raw
# uri_type('doc/api') # => :tree
# uri_type('not/found') # => :nil
#
# Returns a symbol
def uri_type(path)
entry = @raw.tree.path(path)
if entry[:type] == :blob
blob = Gitlab::Git::Blob.new(name: entry[:name])
blob.image? ? :raw : :blob
else
entry[:type]
end
rescue Rugged::TreeError
nil
end
private private
def repo_changes def repo_changes
......
...@@ -14,6 +14,8 @@ module Banzai ...@@ -14,6 +14,8 @@ module Banzai
def call def call
return doc unless linkable_files? return doc unless linkable_files?
@uri_types = {}
doc.search('a:not(.gfm)').each do |el| doc.search('a:not(.gfm)').each do |el|
process_link_attr el.attribute('href') process_link_attr el.attribute('href')
end end
...@@ -48,7 +50,7 @@ module Banzai ...@@ -48,7 +50,7 @@ module Banzai
uri.path = [ uri.path = [
relative_url_root, relative_url_root,
context[:project].path_with_namespace, context[:project].path_with_namespace,
path_type(file_path), uri_type(file_path),
ref || context[:project].default_branch, # if no ref exists, point to the default branch ref || context[:project].default_branch, # if no ref exists, point to the default branch
file_path file_path
].compact.join('/').squeeze('/').chomp('/') ].compact.join('/').squeeze('/').chomp('/')
...@@ -87,7 +89,7 @@ module Banzai ...@@ -87,7 +89,7 @@ module Banzai
return path unless request_path return path unless request_path
parts = request_path.split('/') parts = request_path.split('/')
parts.pop if path_type(request_path) != 'tree' parts.pop if uri_type(request_path) != :tree
while path.start_with?('../') while path.start_with?('../')
parts.pop parts.pop
...@@ -98,45 +100,20 @@ module Banzai ...@@ -98,45 +100,20 @@ module Banzai
end end
def file_exists?(path) def file_exists?(path)
return false if path.nil? path.present? && !!uri_type(path)
repository.blob_at(current_sha, path).present? ||
repository.tree(current_sha, path).entries.any?
end end
# Get the type of the given path def uri_type(path)
# @uri_types[path] ||= begin
# path - String path to check
#
# Examples:
#
# path_type('doc/README.md') # => 'blob'
# path_type('doc/logo.png') # => 'raw'
# path_type('doc/api') # => 'tree'
#
# Returns a String
def path_type(path)
unescaped_path = Addressable::URI.unescape(path) unescaped_path = Addressable::URI.unescape(path)
if tree?(unescaped_path) current_commit.uri_type(unescaped_path)
'tree'
elsif image?(unescaped_path)
'raw'
else
'blob'
end
end end
def tree?(path)
repository.tree(current_sha, path).entries.any?
end
def image?(path)
repository.blob_at(current_sha, path).try(:image?)
end end
def current_sha def current_commit
context[:commit].try(:id) || @current_commit ||= context[:commit] ||
ref ? repository.commit(ref).try(:sha) : repository.head_commit.sha ref ? repository.commit(ref) : repository.head_commit
end end
def relative_url_root def relative_url_root
...@@ -148,7 +125,7 @@ module Banzai ...@@ -148,7 +125,7 @@ module Banzai
end end
def repository def repository
context[:project].try(:repository) @repository ||= context[:project].try(:repository)
end end
end end
end end
......
...@@ -132,11 +132,8 @@ describe Banzai::Filter::RelativeLinkFilter, lib: true do ...@@ -132,11 +132,8 @@ describe Banzai::Filter::RelativeLinkFilter, lib: true do
path = 'files/images/한글.png' path = 'files/images/한글.png'
escaped = Addressable::URI.escape(path) escaped = Addressable::URI.escape(path)
# Stub these methods so the file doesn't actually need to be in the repo # Stub this method so the file doesn't actually need to be in the repo
allow_any_instance_of(described_class). allow_any_instance_of(described_class).to receive(:uri_type).and_return(:raw)
to receive(:file_exists?).and_return(true)
allow_any_instance_of(described_class).
to receive(:image?).with(path).and_return(true)
doc = filter(image(escaped)) doc = filter(image(escaped))
expect(doc.at_css('img')['src']).to match '/raw/' expect(doc.at_css('img')['src']).to match '/raw/'
......
...@@ -207,4 +207,16 @@ eos ...@@ -207,4 +207,16 @@ eos
expect(commit.participants).to include(note1.author, note2.author) expect(commit.participants).to include(note1.author, note2.author)
end end
end end
describe '#uri_type' do
it 'returns the URI type at the given path' do
expect(commit.uri_type('files/html')).to be(:tree)
expect(commit.uri_type('files/images/logo-black.png')).to be(:raw)
expect(commit.uri_type('files/js/application.js')).to be(:blob)
end
it "returns nil if the path doesn't exists" do
expect(commit.uri_type('this/path/doesnt/exist')).to be_nil
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