Commit fcc87070 authored by rpereira2's avatar rpereira2

Refactor status API controller

* Due to the removal of the SelfMonitoringProjectCreateWorker.status
method, refactor the controller.
parent e009ae1b
...@@ -87,13 +87,7 @@ class Admin::ApplicationSettingsController < Admin::ApplicationController ...@@ -87,13 +87,7 @@ class Admin::ApplicationSettingsController < Admin::ApplicationController
def status_create_self_monitoring_project def status_create_self_monitoring_project
return self_monitoring_project_not_implemented unless Feature.enabled?(:self_monitoring_project) return self_monitoring_project_not_implemented unless Feature.enabled?(:self_monitoring_project)
job_id = params[:job_id] job_id = params[:job_id].to_s
unless job_id.present? && job_id.is_a?(String)
return render status: :bad_request, json: {
message: _('"job_id" must be an alphanumeric value')
}
end
unless job_id.length <= PARAM_JOB_ID_MAX_SIZE unless job_id.length <= PARAM_JOB_ID_MAX_SIZE
return render status: :bad_request, json: { return render status: :bad_request, json: {
...@@ -102,29 +96,19 @@ class Admin::ApplicationSettingsController < Admin::ApplicationController ...@@ -102,29 +96,19 @@ class Admin::ApplicationSettingsController < Admin::ApplicationController
} }
end end
::Gitlab::PollingInterval.set_header(response, interval: 3_000) if Gitlab::CurrentSettings.instance_administration_project_id.present?
render status: :ok, json: self_monitoring_data
result = SelfMonitoringProjectCreateWorker.status(job_id)
if [:in_progress, :unknown].include?(result[:status]) elsif SelfMonitoringProjectCreateWorker.in_progress?(job_id)
render status: :accepted, json: result ::Gitlab::PollingInterval.set_header(response, interval: 3_000)
elsif result[:status] == :completed render status: :accepted, json: { message: _('Job is in progress') }
data = result[:output].merge(self_monitoring_data)
render status: :ok, json: data
else else
message = _('SelfMonitoringProjectCreateWorker#status returned unknown status "%{status}"') % render status: :bad_request, json: {
{ status: result[:status] } message: _('Self-monitoring project does not exist. Please check logs ' \
raise_exception_for_dev( 'for any error messages')
message, }
{ return_value: result }
)
render(
status: :internal_server_error,
json: { message: message }
)
end end
end end
...@@ -137,13 +121,6 @@ class Admin::ApplicationSettingsController < Admin::ApplicationController ...@@ -137,13 +121,6 @@ class Admin::ApplicationSettingsController < Admin::ApplicationController
} }
end end
def raise_exception_for_dev(message, extra = {})
Gitlab::ErrorTracking.track_and_raise_for_dev_exception(
StandardError.new(message),
extra
)
end
def self_monitoring_project_not_implemented def self_monitoring_project_not_implemented
render( render(
status: :not_implemented, status: :not_implemented,
......
...@@ -65,9 +65,6 @@ msgstr "" ...@@ -65,9 +65,6 @@ msgstr ""
msgid "\"%{path}\" did not exist on \"%{ref}\"" msgid "\"%{path}\" did not exist on \"%{ref}\""
msgstr "" msgstr ""
msgid "\"job_id\" must be an alphanumeric value"
msgstr ""
msgid "%d comment" msgid "%d comment"
msgid_plural "%d comments" msgid_plural "%d comments"
msgstr[0] "" msgstr[0] ""
...@@ -10194,6 +10191,9 @@ msgstr "" ...@@ -10194,6 +10191,9 @@ msgstr ""
msgid "Job has wrong arguments format." msgid "Job has wrong arguments format."
msgstr "" msgstr ""
msgid "Job is in progress"
msgstr ""
msgid "Job is missing the `model_type` argument." msgid "Job is missing the `model_type` argument."
msgstr "" msgstr ""
...@@ -16220,7 +16220,7 @@ msgstr "" ...@@ -16220,7 +16220,7 @@ msgstr ""
msgid "Self-monitoring is not enabled on this GitLab server, contact your administrator." msgid "Self-monitoring is not enabled on this GitLab server, contact your administrator."
msgstr "" msgstr ""
msgid "SelfMonitoringProjectCreateWorker#status returned unknown status \"%{status}\"" msgid "Self-monitoring project does not exist. Please check logs for any error messages"
msgstr "" msgstr ""
msgid "Send a separate email notification to Developers." msgid "Send a separate email notification to Developers."
......
...@@ -85,23 +85,7 @@ describe 'Self-Monitoring project requests' do ...@@ -85,23 +85,7 @@ describe 'Self-Monitoring project requests' do
end end
context 'with feature flag enabled' do context 'with feature flag enabled' do
it 'calls .status' do
expect(worker_class).to receive(:status).with(job_id).and_call_original
subject
end
it 'sets polling header' do
expect(::Gitlab::PollingInterval).to receive(:set_header)
subject
end
context 'with invalid job_id' do context 'with invalid job_id' do
it_behaves_like 'handles missing or invalid job_id', nil
it_behaves_like 'handles missing or invalid job_id', job_id: nil
it_behaves_like 'handles missing or invalid job_id', job_id: [2]
it 'returns bad_request if job_id too long' do it 'returns bad_request if job_id too long' do
get status_create_self_monitoring_project_admin_application_settings_path, get status_create_self_monitoring_project_admin_application_settings_path,
params: { job_id: 'a' * 51 } params: { job_id: 'a' * 51 }
...@@ -114,89 +98,74 @@ describe 'Self-Monitoring project requests' do ...@@ -114,89 +98,74 @@ describe 'Self-Monitoring project requests' do
end end
end end
context 'different status values' do context 'when self-monitoring project exists' do
let(:project) { build(:project) }
before do before do
allow(worker_class).to receive(:status).and_return(status) stub_application_setting(instance_administration_project_id: 1)
stub_application_setting(instance_administration_project: project)
end end
context 'when status returns in_progress' do it 'does not need job_id' do
let(:status) { { status: :in_progress } } get status_create_self_monitoring_project_admin_application_settings_path
it 'returns status accepted' do aggregate_failures do
subject expect(response).to have_gitlab_http_status(:success)
expect(json_response).to eq(
aggregate_failures do 'project_id' => 1,
expect(response).to have_gitlab_http_status(:accepted) 'project_full_path' => project.full_path
expect(json_response).to eq('status' => 'in_progress') )
end
end end
end end
context 'when status returns unknown' do it 'returns success' do
let(:status) { { status: :unknown, message: 'message' } } subject
it 'returns accepted' do
subject
aggregate_failures do aggregate_failures do
expect(response).to have_gitlab_http_status(:accepted) expect(response).to have_gitlab_http_status(:success)
expect(json_response).to eq( expect(json_response).to eq(
'status' => 'unknown', 'project_id' => 1,
'message' => 'message' 'project_full_path' => project.full_path
) )
end
end end
end end
end
context 'when status returns completed' do context 'when job is in progress' do
let(:status) { { status: :completed, output: { status: :success } } } before do
let(:project) { build(:project) } allow(worker_class).to receive(:in_progress?)
.with(job_id)
.and_return(true)
end
before do it 'sets polling header' do
stub_application_setting(instance_administration_project_id: 2) expect(::Gitlab::PollingInterval).to receive(:set_header)
stub_application_setting(instance_administration_project: project)
end
it 'returns output' do subject
subject
aggregate_failures do
expect(response).to have_gitlab_http_status(:success)
expect(json_response).to eq(
'status' => 'success',
'project_id' => 2,
'project_full_path' => project.full_path
)
end
end
end end
context 'when unexpected status is returned' do it 'returns accepted' do
let(:status) { { status: :unexpected_status } } subject
it 'raises error' do aggregate_failures do
expect { subject }.to raise_error( expect(response).to have_gitlab_http_status(:accepted)
StandardError, 'SelfMonitoringProjectCreateWorker#status returned ' \ expect(json_response).to eq('message' => 'Job is in progress')
'unknown status "unexpected_status"'
)
end end
end
end
context 'when self-monitoring project and job do not exist' do
let(:job_id) { nil }
it 'returns bad_request' do
subject
context 'in production' do aggregate_failures do
before do expect(response).to have_gitlab_http_status(:bad_request)
stub_rails_env('production') expect(json_response).to eq(
end 'message' => 'Self-monitoring project does not exist. Please check ' \
'logs for any error messages'
it 'returns internal_server_error' do )
subject
aggregate_failures do
expect(response).to have_gitlab_http_status(:internal_server_error)
expect(json_response).to eq(
'message' => 'SelfMonitoringProjectCreateWorker#status returned ' \
'unknown status "unexpected_status"'
)
end
end
end end
end end
end end
......
...@@ -18,17 +18,6 @@ RSpec.shared_examples 'not accessible if feature flag is disabled' do ...@@ -18,17 +18,6 @@ RSpec.shared_examples 'not accessible if feature flag is disabled' do
end end
end end
RSpec.shared_examples 'handles missing or invalid job_id' do |job_id_param|
it "returns bad_request for #{job_id_param}" do
get status_create_self_monitoring_project_admin_application_settings_path, params: job_id_param
aggregate_failures do
expect(response).to have_gitlab_http_status(:bad_request)
expect(json_response).to eq('message' => '"job_id" must be an alphanumeric value')
end
end
end
RSpec.shared_examples 'not accessible to non-admin users' do RSpec.shared_examples 'not accessible to non-admin users' do
context 'with unauthenticated user' do context 'with unauthenticated user' do
it 'redirects to signin page' do it 'redirects to signin page' do
......
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