diff --git a/app/controllers/projects/repositories_controller.rb b/app/controllers/projects/repositories_controller.rb index 112365ba91a..96defb0c721 100644 --- a/app/controllers/projects/repositories_controller.rb +++ b/app/controllers/projects/repositories_controller.rb @@ -11,14 +11,10 @@ class Projects::RepositoriesController < Projects::ApplicationController end def archive - unless can?(current_user, :download_code, @project) - render_404 and return - end - begin file_path = ArchiveRepositoryService.new(@project, params[:ref], params[:format]).execute rescue - return render_404 + return head :not_found end if file_path diff --git a/app/services/archive_repository_service.rb b/app/services/archive_repository_service.rb index 40b0a64fb73..e1b41527d8d 100644 --- a/app/services/archive_repository_service.rb +++ b/app/services/archive_repository_service.rb @@ -6,7 +6,7 @@ class ArchiveRepositoryService @project, @ref, @format = project, ref, format.downcase end - def execute + def execute(options = {}) project.repository.clean_old_archives raise "No archive file path" unless file_path @@ -17,7 +17,7 @@ class ArchiveRepositoryService RepositoryArchiveWorker.perform_async(project.id, ref, format) end - archived = wait_until_archived + archived = wait_until_archived(options[:timeout] || 5.0) file_path if archived end @@ -45,6 +45,8 @@ class ArchiveRepositoryService end def wait_until_archived(timeout = 5.0) + return archived? if timeout == 0.0 + t1 = Time.now begin diff --git a/app/workers/repository_archive_worker.rb b/app/workers/repository_archive_worker.rb index 42ac77c588e..021c1139568 100644 --- a/app/workers/repository_archive_worker.rb +++ b/app/workers/repository_archive_worker.rb @@ -13,6 +13,7 @@ class RepositoryArchiveWorker repository.clean_old_archives + return unless file_path return if archived? || archiving? repository.archive_repo(ref, storage_path, format) diff --git a/spec/controllers/projects/repositories_controller_spec.rb b/spec/controllers/projects/repositories_controller_spec.rb new file mode 100644 index 00000000000..91856ed0cc0 --- /dev/null +++ b/spec/controllers/projects/repositories_controller_spec.rb @@ -0,0 +1,65 @@ +require "spec_helper" + +describe Projects::RepositoriesController do + let(:project) { create(:project) } + let(:user) { create(:user) } + + describe "GET archive" do + before do + sign_in(user) + project.team << [user, :developer] + + allow(ArchiveRepositoryService).to receive(:new).and_return(service) + end + + let(:service) { ArchiveRepositoryService.new(project, "master", "zip") } + + it "executes ArchiveRepositoryService" do + expect(ArchiveRepositoryService).to receive(:new).with(project, "master", "zip") + expect(service).to receive(:execute) + + get :archive, namespace_id: project.namespace.path, project_id: project.path, ref: "master", format: "zip" + end + + context "when the service raises an error" do + + before do + allow(service).to receive(:execute).and_raise("Archive failed") + end + + it "renders Not Found" do + get :archive, namespace_id: project.namespace.path, project_id: project.path, ref: "master", format: "zip" + + expect(response.status).to eq(404) + end + end + + context "when the service doesn't return a path" do + + before do + allow(service).to receive(:execute).and_return(nil) + end + + it "reloads the page" do + get :archive, namespace_id: project.namespace.path, project_id: project.path, ref: "master", format: "zip" + + expect(response).to redirect_to(archive_namespace_project_repository_path(project.namespace, project, ref: "master", format: "zip")) + end + end + + context "when the service returns a path" do + + let(:path) { Rails.root.join("spec/fixtures/dk.png").to_s } + + before do + allow(service).to receive(:execute).and_return(path) + end + + it "sends the file" do + get :archive, namespace_id: project.namespace.path, project_id: project.path, ref: "master", format: "zip" + + expect(response.body).to eq(File.binread(path)) + end + end + end +end diff --git a/spec/services/archive_repository_service_spec.rb b/spec/services/archive_repository_service_spec.rb new file mode 100644 index 00000000000..f168a913976 --- /dev/null +++ b/spec/services/archive_repository_service_spec.rb @@ -0,0 +1,93 @@ +require 'spec_helper' + +describe ArchiveRepositoryService do + let(:project) { create(:project) } + subject { ArchiveRepositoryService.new(project, "master", "zip") } + + describe "#execute" do + it "cleans old archives" do + expect(project.repository).to receive(:clean_old_archives) + + subject.execute(timeout: 0.0) + end + + context "when the repository doesn't have an archive file path" do + before do + allow(project.repository).to receive(:archive_file_path).and_return(nil) + end + + it "raises an error" do + expect { + subject.execute(timeout: 0.0) + }.to raise_error + end + end + + context "when the repository has an archive file path" do + let(:file_path) { "/archive.zip" } + let(:pid_file_path) { "/archive.zip.pid" } + + before do + allow(project.repository).to receive(:archive_file_path).and_return(file_path) + allow(project.repository).to receive(:archive_pid_file_path).and_return(pid_file_path) + end + + context "when the archive file already exists" do + before do + allow(File).to receive(:exist?).with(file_path).and_return(true) + end + + it "returns the file path" do + expect(subject.execute(timeout: 0.0)).to eq(file_path) + end + end + + context "when the archive file doesn't exist yet" do + before do + allow(File).to receive(:exist?).with(file_path).and_return(false) + allow(File).to receive(:exist?).with(pid_file_path).and_return(true) + end + + context "when the archive pid file doesn't exist yet" do + before do + allow(File).to receive(:exist?).with(pid_file_path).and_return(false) + end + + it "queues the RepositoryArchiveWorker" do + expect(RepositoryArchiveWorker).to receive(:perform_async) + + subject.execute(timeout: 0.0) + end + end + + context "when the archive pid file already exists" do + it "doesn't queue the RepositoryArchiveWorker" do + expect(RepositoryArchiveWorker).not_to receive(:perform_async) + + subject.execute(timeout: 0.0) + end + end + + context "when the archive file exists after a little while" do + before do + Thread.new do + sleep 0.1 + allow(File).to receive(:exist?).with(file_path).and_return(true) + end + end + + it "returns the file path" do + expect(subject.execute(timeout: 0.2)).to eq(file_path) + end + end + + context "when the archive file doesn't exist after the timeout" do + it "returns nil" do + expect(subject.execute(timeout: 0.0)).to eq(nil) + end + end + end + end + end +end + diff --git a/spec/workers/repository_archive_worker_spec.rb b/spec/workers/repository_archive_worker_spec.rb new file mode 100644 index 00000000000..c2362058cfd --- /dev/null +++ b/spec/workers/repository_archive_worker_spec.rb @@ -0,0 +1,80 @@ +require 'spec_helper' + +describe RepositoryArchiveWorker do + let(:project) { create(:project) } + subject { RepositoryArchiveWorker.new } + + before do + allow(Project).to receive(:find).and_return(project) + end + + describe "#perform" do + it "cleans old archives" do + expect(project.repository).to receive(:clean_old_archives) + + subject.perform(project.id, "master", "zip") + end + + context "when the repository doesn't have an archive file path" do + before do + allow(project.repository).to receive(:archive_file_path).and_return(nil) + end + + it "doesn't archive the repo" do + expect(project.repository).not_to receive(:archive_repo) + + subject.perform(project.id, "master", "zip") + end + end + + context "when the repository has an archive file path" do + let(:file_path) { "/archive.zip" } + let(:pid_file_path) { "/archive.zip.pid" } + + before do + allow(project.repository).to receive(:archive_file_path).and_return(file_path) + allow(project.repository).to receive(:archive_pid_file_path).and_return(pid_file_path) + end + + context "when the archive file already exists" do + before do + allow(File).to receive(:exist?).with(file_path).and_return(true) + end + + it "doesn't archive the repo" do + expect(project.repository).not_to receive(:archive_repo) + + subject.perform(project.id, "master", "zip") + end + end + + context "when the archive file doesn't exist yet" do + before do + allow(File).to receive(:exist?).with(file_path).and_return(false) + allow(File).to receive(:exist?).with(pid_file_path).and_return(true) + end + + context "when the archive pid file doesn't exist yet" do + before do + allow(File).to receive(:exist?).with(pid_file_path).and_return(false) + end + + it "archives the repo" do + expect(project.repository).to receive(:archive_repo) + + subject.perform(project.id, "master", "zip") + end + end + + context "when the archive pid file already exists" do + it "doesn't archive the repo" do + expect(project.repository).not_to receive(:archive_repo) + + subject.perform(project.id, "master", "zip") + end + end + end + end + end +end +