Commit abe44ef4 authored by Nick Thomas's avatar Nick Thomas

Merge branch 'id-improve-raw-blob-downloading' into 'master'

Improve raw blobs downloading

See merge request gitlab-org/gitlab!67155
parents e58a4257 91f0a077
...@@ -19,7 +19,7 @@ class Projects::RawController < Projects::ApplicationController ...@@ -19,7 +19,7 @@ class Projects::RawController < Projects::ApplicationController
feature_category :source_code_management feature_category :source_code_management
def show def show
@blob = @repository.blob_at(@ref, @path) @blob = @repository.blob_at(@ref, @path, limit: Gitlab::Git::Blob::LFS_POINTER_MAX_SIZE)
send_blob(@repository, @blob, inline: (params[:inline] != 'false'), allow_caching: Guest.can?(:download_code, @project)) send_blob(@repository, @blob, inline: (params[:inline] != 'false'), allow_caching: Guest.can?(:download_code, @project))
end end
......
...@@ -502,8 +502,8 @@ class Repository ...@@ -502,8 +502,8 @@ class Repository
end end
end end
def blob_at(sha, path) def blob_at(sha, path, limit: Gitlab::Git::Blob::MAX_DATA_DISPLAY_SIZE)
Blob.decorate(raw_repository.blob_at(sha, path), container) Blob.decorate(raw_repository.blob_at(sha, path, limit: limit), container)
rescue Gitlab::Git::Repository::NoRepository rescue Gitlab::Git::Repository::NoRepository
nil nil
end end
......
...@@ -29,14 +29,13 @@ module API ...@@ -29,14 +29,13 @@ module API
not_found! not_found!
end end
def assign_blob_vars! def assign_blob_vars!(limit:)
authorize! :download_code, user_project authorize! :download_code, user_project
@repo = user_project.repository @repo = user_project.repository
begin begin
@blob = Gitlab::Git::Blob.raw(@repo, params[:sha]) @blob = Gitlab::Git::Blob.raw(@repo, params[:sha], limit: limit)
@blob.load_all_data!(@repo)
rescue StandardError rescue StandardError
not_found! 'Blob' not_found! 'Blob'
end end
...@@ -71,7 +70,8 @@ module API ...@@ -71,7 +70,8 @@ module API
requires :sha, type: String, desc: 'The commit hash' requires :sha, type: String, desc: 'The commit hash'
end end
get ':id/repository/blobs/:sha/raw' do get ':id/repository/blobs/:sha/raw' do
assign_blob_vars! # Load metadata enough to ask Workhorse to load the whole blob
assign_blob_vars!(limit: 0)
no_cache_headers no_cache_headers
...@@ -83,7 +83,7 @@ module API ...@@ -83,7 +83,7 @@ module API
requires :sha, type: String, desc: 'The commit hash' requires :sha, type: String, desc: 'The commit hash'
end end
get ':id/repository/blobs/:sha' do get ':id/repository/blobs/:sha' do
assign_blob_vars! assign_blob_vars!(limit: -1)
{ {
size: @blob.size, size: @blob.size,
......
...@@ -77,8 +77,8 @@ module Gitlab ...@@ -77,8 +77,8 @@ module Gitlab
end end
end end
def raw(repository, sha) def raw(repository, sha, limit: MAX_DATA_DISPLAY_SIZE)
repository.gitaly_blob_client.get_blob(oid: sha, limit: MAX_DATA_DISPLAY_SIZE) repository.gitaly_blob_client.get_blob(oid: sha, limit: limit)
end end
# Returns an array of Blob instances, specified in blob_references as # Returns an array of Blob instances, specified in blob_references as
......
...@@ -827,8 +827,8 @@ module Gitlab ...@@ -827,8 +827,8 @@ module Gitlab
end end
end end
def blob_at(sha, path) def blob_at(sha, path, limit: Gitlab::Git::Blob::MAX_DATA_DISPLAY_SIZE)
Gitlab::Git::Blob.find(self, sha, path) unless Gitlab::Git.blank_ref?(sha) Gitlab::Git::Blob.find(self, sha, path, limit: limit) unless Gitlab::Git.blank_ref?(sha)
end end
# Items should be of format [[commit_id, path], [commit_id1, path1]] # Items should be of format [[commit_id, path], [commit_id1, path1]]
......
...@@ -33,15 +33,25 @@ RSpec.describe Projects::RawController do ...@@ -33,15 +33,25 @@ RSpec.describe Projects::RawController do
end end
context 'regular filename' do context 'regular filename' do
let(:filepath) { 'master/README.md' } let(:filepath) { 'master/CONTRIBUTING.md' }
it 'delivers ASCII file' do it 'delivers ASCII file' do
allow(Gitlab::Workhorse).to receive(:send_git_blob).and_call_original
subject subject
expect(response).to have_gitlab_http_status(:ok) expect(response).to have_gitlab_http_status(:ok)
expect(response.header['Content-Type']).to eq('text/plain; charset=utf-8') expect(response.header['Content-Type']).to eq('text/plain; charset=utf-8')
expect(response.header[Gitlab::Workhorse::DETECT_HEADER]).to eq 'true' expect(response.header[Gitlab::Workhorse::DETECT_HEADER]).to eq 'true'
expect(response.header[Gitlab::Workhorse::SEND_DATA_HEADER]).to start_with('git-blob:') expect(response.header[Gitlab::Workhorse::SEND_DATA_HEADER]).to start_with('git-blob:')
expect(Gitlab::Workhorse).to have_received(:send_git_blob) do |repository, blob|
expected_blob = project.repository.blob_at('master', 'CONTRIBUTING.md')
expect(repository).to eq(project.repository)
expect(blob.id).to eq(expected_blob.id)
expect(blob).to be_truncated
end
end end
it_behaves_like 'project cache control headers' it_behaves_like 'project cache control headers'
......
...@@ -107,13 +107,18 @@ RSpec.describe API::Repositories do ...@@ -107,13 +107,18 @@ RSpec.describe API::Repositories do
shared_examples_for 'repository blob' do shared_examples_for 'repository blob' do
it 'returns blob attributes as json' do it 'returns blob attributes as json' do
stub_const("Gitlab::Git::Blob::MAX_DATA_DISPLAY_SIZE", 5)
get api(route, current_user) get api(route, current_user)
expect(response).to have_gitlab_http_status(:ok) expect(response).to have_gitlab_http_status(:ok)
expect(json_response['size']).to eq(111) expect(json_response['size']).to eq(111)
expect(json_response['encoding']).to eq("base64") expect(json_response['encoding']).to eq("base64")
expect(Base64.decode64(json_response['content']).lines.first).to eq("class Commit\n")
expect(json_response['sha']).to eq(sample_blob.oid) expect(json_response['sha']).to eq(sample_blob.oid)
content = Base64.decode64(json_response['content'])
expect(content.lines.first).to eq("class Commit\n")
expect(content).to eq(project.repository.gitaly_blob_client.get_blob(oid: sample_blob.oid, limit: -1).data)
end end
context 'when sha does not exist' do context 'when sha does not exist' do
...@@ -164,7 +169,10 @@ RSpec.describe API::Repositories do ...@@ -164,7 +169,10 @@ RSpec.describe API::Repositories do
shared_examples_for 'repository raw blob' do shared_examples_for 'repository raw blob' do
it 'returns the repository raw blob' do it 'returns the repository raw blob' do
expect(Gitlab::Workhorse).to receive(:send_git_blob) expect(Gitlab::Workhorse).to receive(:send_git_blob) do |_, blob|
expect(blob.id).to eq(sample_blob.oid)
expect(blob.loaded_size).to eq(0)
end
get api(route, current_user) get api(route, current_user)
......
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