Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Improve performance by supporting repo.lstree with a specific path #457

Open
wants to merge 3 commits into
base: master
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions lib/gollum-lib.rb
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,7 @@ module Gollum; end
require File.expand_path('../gollum-lib/committer', __FILE__)
require File.expand_path('../gollum-lib/pagination', __FILE__)
require File.expand_path('../gollum-lib/blob_entry', __FILE__)
require File.expand_path('../gollum-lib/tree_entry', __FILE__)
require File.expand_path('../gollum-lib/wiki', __FILE__)
require File.expand_path('../gollum-lib/redirects', __FILE__)
require File.expand_path('../gollum-lib/file', __FILE__)
Expand All @@ -31,6 +32,7 @@ module Gollum; end
require File.expand_path('../gollum-lib/markup', __FILE__)
require File.expand_path('../gollum-lib/markups', __FILE__)
require File.expand_path('../gollum-lib/sanitization', __FILE__)

require File.expand_path('../gollum-lib/filter', __FILE__)

module Gollum
Expand Down
23 changes: 13 additions & 10 deletions lib/gollum-lib/git_access.rb
Original file line number Diff line number Diff line change
Expand Up @@ -48,9 +48,9 @@ def ref_to_sha(ref)
# ref - A String Git reference or Git SHA to a commit.
#
# Returns an Array of BlobEntry instances.
def tree(ref)
def tree(ref, path = '/', recursive = true)
if (sha = ref_to_sha(ref))
get_cache(:tree, sha) { tree!(sha) }
get_cache(:tree, "#{sha}#{path}") { tree!(sha, path, recursive) }
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Cache key currently doesn't differentiate between recursive being true or false.

else
[]
end
Expand Down Expand Up @@ -156,16 +156,19 @@ def ref_to_sha!(ref)
# sha - String commit SHA.
#
# Returns an Array of BlobEntry instances.
def tree!(sha)
tree = @repo.lstree(sha, { :recursive => true })
items = []
tree.each do |entry|
if entry[:type] == 'blob'
next if @page_file_dir && !entry[:path].start_with?("#{@page_file_dir}/")
items << BlobEntry.new(entry[:sha], entry[:path], entry[:size], entry[:mode])
def tree!(sha, path = '/', recursive = true)
tree = @repo.lstree(sha, (Pathname.new(@page_file_dir.to_s) + path).to_s, recursive: recursive )
tree.reduce([]) do |accumulator, entry|
if !@page_file_dir || entry[:path].start_with?("#{@page_file_dir}/") # guard against directory traversal
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not sure this line is needed, if we're already passing @page_file_dir as the base path.

accumulator << case entry[:type]
when 'blob'
BlobEntry.new(entry[:sha], entry[:path], entry[:size], entry[:mode])
when 'tree'
TreeEntry.new(entry[:sha], entry[:path])
end
end
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We could also opt to return blobs and trees in separate Arrays, so callers who need only the blobs can ignore the trees.

Either way, I guess initializing the TreeEntries has minor performance impact when looping into the entire wiki.

accumulator
end
items
end

# Reads the content from the Git db at the given SHA.
Expand Down
16 changes: 16 additions & 0 deletions lib/gollum-lib/tree_entry.rb
Original file line number Diff line number Diff line change
@@ -0,0 +1,16 @@
# ~*~ encoding: utf-8 ~*~
module Gollum
# Represents a Tree (directory) that can be contained in another Tree.
# We do not really care about any information other than the Tree's path and name:
# If we want to know a Tree `foo`'s'contents, this can be achieved by e.g. calling wiki.path_list(foo.path)
class TreeEntry
attr_reader :sha
attr_reader :path

def initialize(sha, path)
@sha = sha
@path = path
@name = ::File.basename(@path)
end
end
end
47 changes: 37 additions & 10 deletions lib/gollum-lib/wiki.rb
Original file line number Diff line number Diff line change
Expand Up @@ -488,7 +488,7 @@ def files(treeish = nil)
# Returns a Fixnum
def size(ref = nil)
tree_map_for(ref || @ref).inject(0) do |num, entry|
num + (::Gollum::Page.valid_page_name?(entry.name) ? 1 : 0)
num + (entry.is_a?(BlobEntry) && ::Gollum::Page.valid_page_name?(entry.name) ? 1 : 0)
end
rescue Gollum::Git::NoSuchShaFound
0
Expand Down Expand Up @@ -637,24 +637,47 @@ def page_file_name(name, format)
format.nil? ? name : "#{name}.#{::Gollum::Page.format_to_ext(format)}"
end

# Fill an array with a list of pages and files in the wiki.
# Fill an array with a list of all pages and files in the wiki.
#
# ref - A String ref that is either a commit SHA or references one.
#
# Returns a flat Array of Gollum::Page and Gollum::File instances.
def tree_list(ref = @ref, pages=true, files=true)
if (sha = @access.ref_to_sha(ref))
commit = @access.commit(sha)
tree_map_for(sha).inject([]) do |list, entry|
wrap_tree_entries(tree_map_for(sha), commit, pages, files, false)
else
[]
end
end

# Fill an array with a list of pages, files, and directories at a specific path in the wiki.
#
# path - A String path in the wiki.
# ref - A String ref that is either a commit SHA or references one.
#
# Returns a flat Array of Gollum::Page and Gollum::File instances.
def path_list(path, ref = @ref)
if (sha = @access.ref_to_sha(ref))
commit = @access.commit(sha)
wrap_tree_entries(tree_map_for(sha, false, path, false), commit, true, true, true)
else
[]
end
end

def wrap_tree_entries(entries, commit, pages, files, trees)
entries.inject([]) do |list, entry|
if entry.is_a?(TreeEntry)
list << entry if trees
else
if ::Gollum::Page.valid_page_name?(entry.name)
list << entry.page(self, commit) if pages
elsif files && !entry.name.start_with?('_') && !::Gollum::Page.protected_files.include?(entry.name)
list << entry.file(self, commit)
end
list
end
else
[]
list
end
end

Expand Down Expand Up @@ -690,17 +713,21 @@ def commit_for(ref)
#
# ref - A String ref that is either a commit SHA or references one.
# ignore_page_file_dir - Boolean, if true, searches all files within the git repo, regardless of dir/subdir
# path - A String path to a directory that should be listed.
# recursive - Boolean. Whether to recursively list all contents or not.
#
# Returns an Array of BlobEntry instances.
def tree_map_for(ref, ignore_page_file_dir = false)
def tree_map_for(ref, ignore_page_file_dir = false, path = '/', recursive = true)
if ignore_page_file_dir && !@page_file_dir.nil?
@root_access ||= GitAccess.new(path, nil, @repo_is_bare)
@root_access.tree(ref)
else
@access.tree(ref)
begin
@access.tree(ref, path, recursive)
rescue Gollum::Git::NoSuchShaFound
[]
end
end
rescue Gollum::Git::NoSuchShaFound
[]
end

def inspect
Expand Down
5 changes: 5 additions & 0 deletions test/test_wiki.rb
Original file line number Diff line number Diff line change
Expand Up @@ -48,6 +48,11 @@
assert_equal commits, @wiki.log(:page_num => 2).map(&:id)
end

test "list specific directory in the wiki" do
puts @wiki.path_list('Mordor').inspect
assert false
end

test "list files and pages" do
contents = @wiki.tree_list
pages = contents.select {|x| x.is_a?(::Gollum::Page)}
Expand Down
Loading