Improve performance by supporting repo.lstree with a specific path #457
Conversation
|
Still getting some tests errors. Most are just due to changed expectations in the Git Access tests, as the results now contain In general, we should make sure the default case ( |
| if (sha = ref_to_sha(ref)) | ||
| get_cache(:tree, sha) { tree!(sha) } | ||
| get_cache(:tree, "#{sha}#{path}") { tree!(sha, path, recursive) } |
There was a problem hiding this comment.
Cache key currently doesn't differentiate between recursive being true or false.
| 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 |
There was a problem hiding this comment.
Not sure this line is needed, if we're already passing @page_file_dir as the base path.
| BlobEntry.new(entry[:sha], entry[:path], entry[:size], entry[:mode]) | ||
| when 'tree' | ||
| TreeEntry.new(entry[:sha], entry[:path]) | ||
| end | ||
| end |
There was a problem hiding this comment.
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.
This PR adds support for new adapter features which allow us to call
lstreefor a specific directory in the repo, and non-recursively:gollum/rugged_adapter#67
gollum/rjgit_adapter#26
Specs: gollum/adapter_specs#19
It depends on those changes.
This will eventually allow us to improve performance of the Overview route, as we'll no longer need to get and loop over the entire contents of the repository.