From 8c3ba8d6c9021f250fb1597f6b597d817af46b38 Mon Sep 17 00:00:00 2001 From: Douwe Maan Date: Mon, 6 Jun 2016 13:16:30 +0200 Subject: [PATCH 1/4] Add workhorse controller and API helpers --- app/controllers/application_controller.rb | 1 + app/controllers/projects/avatars_controller.rb | 5 +---- app/controllers/projects/raw_controller.rb | 5 +---- .../projects/repositories_controller.rb | 3 +-- app/helpers/workhorse_helper.rb | 17 +++++++++++++++++ db/schema.rb | 1 + lib/api/helpers.rb | 10 ++++++++++ lib/api/repositories.rb | 10 +++------- lib/gitlab/workhorse.rb | 8 ++++---- .../controllers/projects/raw_controller_spec.rb | 2 ++ .../projects/repositories_controller_spec.rb | 5 +++-- spec/lib/gitlab/workhorse_spec.rb | 2 +- 12 files changed, 45 insertions(+), 24 deletions(-) create mode 100644 app/helpers/workhorse_helper.rb diff --git a/app/controllers/application_controller.rb b/app/controllers/application_controller.rb index 62f63701799..cd6ae507cf1 100644 --- a/app/controllers/application_controller.rb +++ b/app/controllers/application_controller.rb @@ -6,6 +6,7 @@ class ApplicationController < ActionController::Base include Gitlab::GonHelper include GitlabRoutingHelper include PageLayoutHelper + include WorkhorseHelper before_action :authenticate_user_from_token! before_action :authenticate_user! diff --git a/app/controllers/projects/avatars_controller.rb b/app/controllers/projects/avatars_controller.rb index 72921b3aa14..5962f74c39b 100644 --- a/app/controllers/projects/avatars_controller.rb +++ b/app/controllers/projects/avatars_controller.rb @@ -10,10 +10,7 @@ class Projects::AvatarsController < Projects::ApplicationController return if cached_blob? - headers.store(*Gitlab::Workhorse.send_git_blob(@repository, @blob)) - headers['Content-Disposition'] = 'inline' - headers['Content-Type'] = safe_content_type(@blob) - head :ok # 'render nothing: true' messes up the Content-Type + send_git_blob @repository, @blob else render_404 end diff --git a/app/controllers/projects/raw_controller.rb b/app/controllers/projects/raw_controller.rb index 10de0e60530..10d24da16d7 100644 --- a/app/controllers/projects/raw_controller.rb +++ b/app/controllers/projects/raw_controller.rb @@ -18,10 +18,7 @@ class Projects::RawController < Projects::ApplicationController if @blob.lfs_pointer? send_lfs_object else - headers.store(*Gitlab::Workhorse.send_git_blob(@repository, @blob)) - headers['Content-Disposition'] = 'inline' - headers['Content-Type'] = safe_content_type(@blob) - head :ok # 'render nothing: true' messes up the Content-Type + send_git_blob @repository, @blob end else render_404 diff --git a/app/controllers/projects/repositories_controller.rb b/app/controllers/projects/repositories_controller.rb index bb7a6b6a5ab..d5af0341d18 100644 --- a/app/controllers/projects/repositories_controller.rb +++ b/app/controllers/projects/repositories_controller.rb @@ -11,8 +11,7 @@ class Projects::RepositoriesController < Projects::ApplicationController end def archive - headers.store(*Gitlab::Workhorse.send_git_archive(@project, params[:ref], params[:format])) - head :ok + send_git_archive @repository, ref: params[:ref], format: params[:format] rescue => ex logger.error("#{self.class.name}: #{ex}") return git_not_found! diff --git a/app/helpers/workhorse_helper.rb b/app/helpers/workhorse_helper.rb new file mode 100644 index 00000000000..9d306c9096e --- /dev/null +++ b/app/helpers/workhorse_helper.rb @@ -0,0 +1,17 @@ +# Helpers to send Git blobs or archives through Workhorse. +# Workhorse will also serve files when using `send_file`. +module WorkhorseHelper + # Send a Git blob through Workhorse + def send_git_blob(repository, blob) + headers.store(*Gitlab::Workhorse.send_git_blob(repository, blob)) + headers['Content-Disposition'] = 'inline' + headers['Content-Type'] = safe_content_type(blob) + head :ok # 'render nothing: true' messes up the Content-Type + end + + # Archive a Git repository and send it through Workhorse + def send_git_archive(repository, ref:, format:) + headers.store(*Gitlab::Workhorse.send_git_archive(repository, ref: ref, format: format)) + head :ok + end +end diff --git a/db/schema.rb b/db/schema.rb index 9b991f347a9..659ddc6df04 100644 --- a/db/schema.rb +++ b/db/schema.rb @@ -12,6 +12,7 @@ # It's strongly recommended that you check this file into your version control system. ActiveRecord::Schema.define(version: 20160530150109) do + # These are extensions that must be enabled in order to support this database enable_extension "plpgsql" enable_extension "pg_trgm" diff --git a/lib/api/helpers.rb b/lib/api/helpers.rb index 2aaa0557ea3..0e47bb0b8ad 100644 --- a/lib/api/helpers.rb +++ b/lib/api/helpers.rb @@ -408,5 +408,15 @@ module API error!(errors[:access_level], 422) if errors[:access_level].any? not_found!(errors) end + + def send_git_blob(repository, blob) + env['api.format'] = :txt + content_type 'text/plain' + header *Gitlab::Workhorse.send_git_blob(repository, blob) + end + + def send_git_archive(repository, ref:, format:) + header *Gitlab::Workhorse.send_git_archive(repository, ref: ref, format: format) + end end end diff --git a/lib/api/repositories.rb b/lib/api/repositories.rb index 9cb14e95ebc..f55aceed92c 100644 --- a/lib/api/repositories.rb +++ b/lib/api/repositories.rb @@ -56,8 +56,7 @@ module API blob = Gitlab::Git::Blob.find(repo, commit.id, params[:filepath]) not_found! "File" unless blob - content_type 'text/plain' - header(*Gitlab::Workhorse.send_git_blob(repo, blob)) + send_git_blob repo, blob end # Get a raw blob contents by blob sha @@ -80,10 +79,7 @@ module API not_found! 'Blob' unless blob - env['api.format'] = :txt - - content_type blob.mime_type - header(*Gitlab::Workhorse.send_git_blob(repo, blob)) + send_git_blob repo, blob end # Get a an archive of the repository @@ -98,7 +94,7 @@ module API authorize! :download_code, user_project begin - header(*Gitlab::Workhorse.send_git_archive(user_project, params[:sha], params[:format])) + send_git_archive user_project.repository, ref: params[:sha], format: params[:format] rescue not_found!('File') end diff --git a/lib/gitlab/workhorse.rb b/lib/gitlab/workhorse.rb index c3ddd4c2680..96e99dc0088 100644 --- a/lib/gitlab/workhorse.rb +++ b/lib/gitlab/workhorse.rb @@ -18,10 +18,10 @@ module Gitlab ] end - def send_git_archive(project, ref, format) + def send_git_archive(repository, ref:, format:) format ||= 'tar.gz' format.downcase! - params = project.repository.archive_metadata(ref, Gitlab.config.gitlab.repository_downloads_path, format) + params = repository.archive_metadata(ref, Gitlab.config.gitlab.repository_downloads_path, format) raise "Repository or ref not found" if params.empty? [ @@ -29,9 +29,9 @@ module Gitlab "git-archive:#{encode(params)}", ] end - + protected - + def encode(hash) Base64.urlsafe_encode64(JSON.dump(hash)) end diff --git a/spec/controllers/projects/raw_controller_spec.rb b/spec/controllers/projects/raw_controller_spec.rb index fb29274c687..33c35161da3 100644 --- a/spec/controllers/projects/raw_controller_spec.rb +++ b/spec/controllers/projects/raw_controller_spec.rb @@ -17,6 +17,7 @@ describe Projects::RawController do expect(response.header['Content-Type']).to eq('text/plain; charset=utf-8') expect(response.header['Content-Disposition']). to eq("inline") + expect(response.header[Gitlab::Workhorse::SEND_DATA_HEADER]).to start_with("git-blob:") end end @@ -31,6 +32,7 @@ describe Projects::RawController do expect(response.status).to eq(200) expect(response.header['Content-Type']).to eq('image/jpeg') + expect(response.header[Gitlab::Workhorse::SEND_DATA_HEADER]).to start_with("git-blob:") end end diff --git a/spec/controllers/projects/repositories_controller_spec.rb b/spec/controllers/projects/repositories_controller_spec.rb index 0ddbec9eac2..aad62cf20e3 100644 --- a/spec/controllers/projects/repositories_controller_spec.rb +++ b/spec/controllers/projects/repositories_controller_spec.rb @@ -20,10 +20,11 @@ describe Projects::RepositoriesController do project.team << [user, :developer] sign_in(user) end - it "uses Gitlab::Workhorse" do - expect(Gitlab::Workhorse).to receive(:send_git_archive).with(project, "master", "zip") + it "uses Gitlab::Workhorse" do get :archive, namespace_id: project.namespace.path, project_id: project.path, ref: "master", format: "zip" + + expect(response.header[Gitlab::Workhorse::SEND_DATA_HEADER]).to start_with("git-archive:") end context "when the service raises an error" do diff --git a/spec/lib/gitlab/workhorse_spec.rb b/spec/lib/gitlab/workhorse_spec.rb index d940bf05061..c5c1402e8fc 100644 --- a/spec/lib/gitlab/workhorse_spec.rb +++ b/spec/lib/gitlab/workhorse_spec.rb @@ -11,7 +11,7 @@ describe Gitlab::Workhorse, lib: true do end it "raises an error" do - expect { subject.send_git_archive(project, "master", "zip") }.to raise_error(RuntimeError) + expect { subject.send_git_archive(project.repository, ref: "master", format: "zip") }.to raise_error(RuntimeError) end end end From ae011aff63bd6b20043d8ee533f07a65db0b75ff Mon Sep 17 00:00:00 2001 From: Douwe Maan Date: Mon, 6 Jun 2016 13:35:26 +0200 Subject: [PATCH 2/4] Add changelog item --- CHANGELOG | 1 + 1 file changed, 1 insertion(+) diff --git a/CHANGELOG b/CHANGELOG index 72fe32a01a3..4594d9d554f 100644 --- a/CHANGELOG +++ b/CHANGELOG @@ -63,6 +63,7 @@ v 8.8.3 - Fix serious performance bug with rendering Markdown with InlineDiffFilter. !4392 - Fix missing number on generated ordered list element. !4437 - Prevent disclosure of notes on confidential issues in search results. + - Add workhorse controller and API helpers v 8.8.2 - Added remove due date button. !4209 From 3855e33cf8fe95d6de83b18698a958095fa49df5 Mon Sep 17 00:00:00 2001 From: Douwe Maan Date: Mon, 6 Jun 2016 15:34:50 +0200 Subject: [PATCH 3/4] Move changelog item --- CHANGELOG | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/CHANGELOG b/CHANGELOG index 4594d9d554f..03e7e5b1e56 100644 --- a/CHANGELOG +++ b/CHANGELOG @@ -38,6 +38,7 @@ v 8.9.0 (unreleased) - Improve error handling importing projects - Put project Files and Commits tabs under Code tab - Replace Colorize with Rainbow for coloring console output in Rake tasks. + - Add workhorse controller and API helpers v 8.8.4 (unreleased) - Ensure branch cleanup regardless of whether the GitHub import process succeeds @@ -63,7 +64,6 @@ v 8.8.3 - Fix serious performance bug with rendering Markdown with InlineDiffFilter. !4392 - Fix missing number on generated ordered list element. !4437 - Prevent disclosure of notes on confidential issues in search results. - - Add workhorse controller and API helpers v 8.8.2 - Added remove due date button. !4209 From 701e2df7e55113dafd48c570baad44bf7f24f863 Mon Sep 17 00:00:00 2001 From: Douwe Maan Date: Fri, 10 Jun 2016 12:29:15 +0200 Subject: [PATCH 4/4] Satisfy Rubocop --- lib/api/helpers.rb | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/lib/api/helpers.rb b/lib/api/helpers.rb index 0e47bb0b8ad..e1d3bbcc02d 100644 --- a/lib/api/helpers.rb +++ b/lib/api/helpers.rb @@ -412,11 +412,11 @@ module API def send_git_blob(repository, blob) env['api.format'] = :txt content_type 'text/plain' - header *Gitlab::Workhorse.send_git_blob(repository, blob) + header(*Gitlab::Workhorse.send_git_blob(repository, blob)) end def send_git_archive(repository, ref:, format:) - header *Gitlab::Workhorse.send_git_archive(repository, ref: ref, format: format) + header(*Gitlab::Workhorse.send_git_archive(repository, ref: ref, format: format)) end end end