From 091b15b7421374df4e06bfdb91bef6a0c36072cf Mon Sep 17 00:00:00 2001 From: Diego Silva Date: Sat, 4 May 2019 10:11:06 +0100 Subject: [PATCH] Change DetectRepositoryLanguagesWorker to not receive user Fixes #60425 --- app/models/repository.rb | 2 +- app/services/git/branch_push_service.rb | 2 +- .../projects/repository_languages_service.rb | 2 +- app/workers/detect_repository_languages_worker.rb | 7 +++---- ...-when-accessing-charts-with-anonymous-user.yml | 5 +++++ .../projects/graphs_controller_spec.rb | 15 +++++++++++++++ .../detect_repository_languages_service_spec.rb | 2 +- .../projects/repository_languages_service_spec.rb | 4 ++-- .../detect_repository_languages_worker_spec.rb | 11 ++--------- 9 files changed, 31 insertions(+), 19 deletions(-) create mode 100644 changelogs/unreleased/60425-fix-500-when-accessing-charts-with-anonymous-user.yml diff --git a/app/models/repository.rb b/app/models/repository.rb index be17b54ff12..1c02e68f2f6 100644 --- a/app/models/repository.rb +++ b/app/models/repository.rb @@ -465,7 +465,7 @@ class Repository def after_import expire_content_cache - DetectRepositoryLanguagesWorker.perform_async(project.id, project.owner.id) + DetectRepositoryLanguagesWorker.perform_async(project.id) end # Runs code after a new commit has been pushed. diff --git a/app/services/git/branch_push_service.rb b/app/services/git/branch_push_service.rb index da053ce80c7..abf11f253f6 100644 --- a/app/services/git/branch_push_service.rb +++ b/app/services/git/branch_push_service.rb @@ -48,7 +48,7 @@ module Git def enqueue_detect_repository_languages return unless default_branch? - DetectRepositoryLanguagesWorker.perform_async(project.id, current_user.id) + DetectRepositoryLanguagesWorker.perform_async(project.id) end # Only stop environments if the ref is a branch that is being deleted diff --git a/app/services/projects/repository_languages_service.rb b/app/services/projects/repository_languages_service.rb index e75851c7da4..05f43c2264b 100644 --- a/app/services/projects/repository_languages_service.rb +++ b/app/services/projects/repository_languages_service.rb @@ -11,7 +11,7 @@ module Projects def perform_language_detection if persisted_repository_languages.blank? - ::DetectRepositoryLanguagesWorker.perform_async(project.id, current_user.id) + ::DetectRepositoryLanguagesWorker.perform_async(project.id) else project.update_column(:detected_repository_languages, true) end diff --git a/app/workers/detect_repository_languages_worker.rb b/app/workers/detect_repository_languages_worker.rb index 64bc9776d48..838c3be78f0 100644 --- a/app/workers/detect_repository_languages_worker.rb +++ b/app/workers/detect_repository_languages_worker.rb @@ -12,13 +12,12 @@ class DetectRepositoryLanguagesWorker attr_reader :project # rubocop: disable CodeReuse/ActiveRecord - def perform(project_id, user_id) + def perform(project_id, user_id = nil) @project = Project.find_by(id: project_id) - user = User.find_by(id: user_id) - return unless project && user + return unless project try_obtain_lease do - ::Projects::DetectRepositoryLanguagesService.new(project, user).execute + ::Projects::DetectRepositoryLanguagesService.new(project).execute end end # rubocop: enable CodeReuse/ActiveRecord diff --git a/changelogs/unreleased/60425-fix-500-when-accessing-charts-with-anonymous-user.yml b/changelogs/unreleased/60425-fix-500-when-accessing-charts-with-anonymous-user.yml new file mode 100644 index 00000000000..4274dc5918c --- /dev/null +++ b/changelogs/unreleased/60425-fix-500-when-accessing-charts-with-anonymous-user.yml @@ -0,0 +1,5 @@ +--- +title: "Fix 500 error when accessing charts with an anonymous user" +merge_request: 28091 +author: Diego Silva +type: fixed diff --git a/spec/controllers/projects/graphs_controller_spec.rb b/spec/controllers/projects/graphs_controller_spec.rb index d390e84c9b0..b5248c7f0c8 100644 --- a/spec/controllers/projects/graphs_controller_spec.rb +++ b/spec/controllers/projects/graphs_controller_spec.rb @@ -28,6 +28,21 @@ describe Projects::GraphsController do end describe 'charts' do + context 'with an anonymous user' do + let(:project) { create(:project, :repository, :public) } + + before do + sign_out(user) + end + + it 'renders charts with 200 status code' do + get(:charts, params: { namespace_id: project.namespace.path, project_id: project.path, id: 'master' }) + + expect(response).to have_gitlab_http_status(:ok) + expect(response).to render_template(:charts) + end + end + context 'when languages were previously detected' do let(:project) { create(:project, :repository, detected_repository_languages: true) } let!(:repository_language) { create(:repository_language, project: project) } diff --git a/spec/services/projects/detect_repository_languages_service_spec.rb b/spec/services/projects/detect_repository_languages_service_spec.rb index e3e561c971c..df5eed18ac0 100644 --- a/spec/services/projects/detect_repository_languages_service_spec.rb +++ b/spec/services/projects/detect_repository_languages_service_spec.rb @@ -5,7 +5,7 @@ require 'spec_helper' describe Projects::DetectRepositoryLanguagesService, :clean_gitlab_redis_shared_state do set(:project) { create(:project, :repository) } - subject { described_class.new(project, project.owner) } + subject { described_class.new(project) } describe '#execute' do context 'without previous detection' do diff --git a/spec/services/projects/repository_languages_service_spec.rb b/spec/services/projects/repository_languages_service_spec.rb index 09c61363ad2..46c5095327d 100644 --- a/spec/services/projects/repository_languages_service_spec.rb +++ b/spec/services/projects/repository_languages_service_spec.rb @@ -10,7 +10,7 @@ describe Projects::RepositoryLanguagesService do context 'when a project is without detected programming languages' do it 'schedules a worker and returns the empty result' do - expect(::DetectRepositoryLanguagesWorker).to receive(:perform_async).with(project.id, project.owner.id) + expect(::DetectRepositoryLanguagesWorker).to receive(:perform_async).with(project.id) expect(service.execute).to eq([]) end end @@ -19,7 +19,7 @@ describe Projects::RepositoryLanguagesService do let!(:repository_language) { create(:repository_language, project: project) } it 'does not schedule a worker and returns the detected languages' do - expect(::DetectRepositoryLanguagesWorker).not_to receive(:perform_async).with(project.id, project.owner.id) + expect(::DetectRepositoryLanguagesWorker).not_to receive(:perform_async).with(project.id) languages = service.execute diff --git a/spec/workers/detect_repository_languages_worker_spec.rb b/spec/workers/detect_repository_languages_worker_spec.rb index dbf32555985..755eb8dbf6b 100644 --- a/spec/workers/detect_repository_languages_worker_spec.rb +++ b/spec/workers/detect_repository_languages_worker_spec.rb @@ -4,7 +4,6 @@ require 'spec_helper' describe DetectRepositoryLanguagesWorker do set(:project) { create(:project) } - let(:user) { project.owner } subject { described_class.new } @@ -14,19 +13,13 @@ describe DetectRepositoryLanguagesWorker do allow(::Projects::DetectRepositoryLanguagesService).to receive(:new).and_return(service) expect(service).to receive(:execute) - subject.perform(project.id, user.id) + subject.perform(project.id) end context 'when invalid ids are used' do it 'does not raise when the project could not be found' do expect do - subject.perform(-1, user.id) - end.not_to raise_error - end - - it 'does not raise when the user could not be found' do - expect do - subject.perform(project.id, -1) + subject.perform(-1) end.not_to raise_error end end -- GitLab