diff options
author | Reuben Pereira <rpereira@gitlab.com> | 2019-01-10 00:04:27 +0300 |
---|---|---|
committer | Kamil TrzciĆski <ayufan@ayufan.eu> | 2019-01-10 00:04:27 +0300 |
commit | d69074fc722351fef313b09255a7e734c01618ac (patch) | |
tree | c2e9c2c15bd87eac4673428be687676167f1c1f0 /spec | |
parent | 47698eec4086b82047a677e218a5299f73c46109 (diff) |
Service for calling Sentry issues api
Diffstat (limited to 'spec')
-rw-r--r-- | spec/controllers/projects/error_tracking_controller_spec.rb | 142 | ||||
-rw-r--r-- | spec/factories/error_tracking/error.rb | 24 | ||||
-rw-r--r-- | spec/factories/project_error_tracking_settings.rb | 2 | ||||
-rw-r--r-- | spec/fixtures/api/schemas/error_tracking/error.json | 21 | ||||
-rw-r--r-- | spec/fixtures/api/schemas/error_tracking/index.json | 15 | ||||
-rw-r--r-- | spec/fixtures/sentry/issues_sample_response.json | 42 | ||||
-rw-r--r-- | spec/lib/sentry/client_spec.rb | 119 | ||||
-rw-r--r-- | spec/models/error_tracking/project_error_tracking_setting_spec.rb | 91 | ||||
-rw-r--r-- | spec/policies/project_policy_spec.rb | 2 | ||||
-rw-r--r-- | spec/services/error_tracking/list_issues_service_spec.rb | 87 | ||||
-rw-r--r-- | spec/services/projects/operations/update_service_spec.rb | 8 |
11 files changed, 538 insertions, 15 deletions
diff --git a/spec/controllers/projects/error_tracking_controller_spec.rb b/spec/controllers/projects/error_tracking_controller_spec.rb new file mode 100644 index 00000000000..729e71b87a6 --- /dev/null +++ b/spec/controllers/projects/error_tracking_controller_spec.rb @@ -0,0 +1,142 @@ +# frozen_string_literal: true + +require 'rails_helper' + +describe Projects::ErrorTrackingController do + set(:project) { create(:project) } + set(:user) { create(:user) } + + before do + sign_in(user) + project.add_maintainer(user) + end + + describe 'GET #index' do + describe 'html' do + it 'renders index with 200 status code' do + get :index, params: project_params + + expect(response).to have_gitlab_http_status(:ok) + expect(response).to render_template(:index) + end + + context 'with feature flag disabled' do + before do + stub_feature_flags(error_tracking: false) + end + + it 'returns 404' do + get :index, params: project_params + + expect(response).to have_gitlab_http_status(:not_found) + end + end + + context 'with insufficient permissions' do + before do + project.add_guest(user) + end + + it 'returns 404' do + get :index, params: project_params + + expect(response).to have_gitlab_http_status(:not_found) + end + end + + context 'with an anonymous user' do + before do + sign_out(user) + end + + it 'redirects to sign-in page' do + get :index, params: project_params + + expect(response).to redirect_to(new_user_session_path) + end + end + end + + describe 'format json' do + shared_examples 'no data' do + it 'returns no data' do + get :index, params: project_params(format: :json) + + expect(response).to have_gitlab_http_status(:ok) + expect(response).to match_response_schema('error_tracking/index') + expect(json_response['external_url']).to be_nil + expect(json_response['errors']).to eq([]) + end + end + + let(:list_issues_service) { spy(:list_issues_service) } + let(:external_url) { 'http://example.com' } + + before do + expect(ErrorTracking::ListIssuesService) + .to receive(:new).with(project, user) + .and_return(list_issues_service) + end + + context 'service result is successful' do + before do + expect(list_issues_service).to receive(:execute) + .and_return(status: :success, issues: [error]) + expect(list_issues_service).to receive(:external_url) + .and_return(external_url) + end + + let(:error) { build(:error_tracking_error) } + + it 'returns a list of errors' do + get :index, params: project_params(format: :json) + + expect(response).to have_gitlab_http_status(:ok) + expect(response).to match_response_schema('error_tracking/index') + expect(json_response['external_url']).to eq(external_url) + expect(json_response['errors']).to eq([error].as_json) + end + end + + context 'service result is erroneous' do + let(:error_message) { 'error message' } + + context 'without http_status' do + before do + expect(list_issues_service).to receive(:execute) + .and_return(status: :error, message: error_message) + end + + it 'returns 400 with message' do + get :index, params: project_params(format: :json) + + expect(response).to have_gitlab_http_status(:bad_request) + expect(json_response['message']).to eq(error_message) + end + end + + context 'with explicit http_status' do + let(:http_status) { :no_content } + + before do + expect(list_issues_service).to receive(:execute) + .and_return(status: :error, message: error_message, http_status: http_status) + end + + it 'returns http_status with message' do + get :index, params: project_params(format: :json) + + expect(response).to have_gitlab_http_status(http_status) + expect(json_response['message']).to eq(error_message) + end + end + end + end + end + + private + + def project_params(opts = {}) + opts.reverse_merge(namespace_id: project.namespace, project_id: project) + end +end diff --git a/spec/factories/error_tracking/error.rb b/spec/factories/error_tracking/error.rb new file mode 100644 index 00000000000..ff883a3d22c --- /dev/null +++ b/spec/factories/error_tracking/error.rb @@ -0,0 +1,24 @@ +# frozen_string_literal: true + +FactoryBot.define do + factory :error_tracking_error, class: Gitlab::ErrorTracking::Error do + id 'id' + title 'title' + type 'error' + user_count 1 + count 2 + first_seen { Time.now } + last_seen { Time.now } + message 'message' + culprit 'culprit' + external_url 'http://example.com/id' + project_id 'project1' + project_name 'project name' + project_slug 'project_name' + short_id 'ID' + status 'unresolved' + frequency [] + + skip_create + end +end diff --git a/spec/factories/project_error_tracking_settings.rb b/spec/factories/project_error_tracking_settings.rb index f044cbe8755..fbd8dfd395c 100644 --- a/spec/factories/project_error_tracking_settings.rb +++ b/spec/factories/project_error_tracking_settings.rb @@ -3,7 +3,7 @@ FactoryBot.define do factory :project_error_tracking_setting, class: ErrorTracking::ProjectErrorTrackingSetting do project - api_url 'https://gitlab.com' + api_url 'https://gitlab.com/api/0/projects/sentry-org/sentry-project' enabled true token 'access_token_123' end diff --git a/spec/fixtures/api/schemas/error_tracking/error.json b/spec/fixtures/api/schemas/error_tracking/error.json new file mode 100644 index 00000000000..df2c02d7d5d --- /dev/null +++ b/spec/fixtures/api/schemas/error_tracking/error.json @@ -0,0 +1,21 @@ +{ + "type": "object", + "required" : [ + "external_url", + "last_seen", + "message", + "type" + ], + "properties" : { + "id": { "type": "string"}, + "first_seen": { "type": "string", "format": "date-time" }, + "last_seen": { "type": "string", "format": "date-time" }, + "type": { "type": "string" }, + "message": { "type": "string" }, + "culprit": { "type": "string" }, + "count": { "type": "integer"}, + "external_url": { "type": "string" }, + "user_count": { "type": "integer"} + }, + "additionalProperties": true +} diff --git a/spec/fixtures/api/schemas/error_tracking/index.json b/spec/fixtures/api/schemas/error_tracking/index.json new file mode 100644 index 00000000000..d3abc29ffa7 --- /dev/null +++ b/spec/fixtures/api/schemas/error_tracking/index.json @@ -0,0 +1,15 @@ +{ + "type": "object", + "required": [ + "external_url", + "errors" + ], + "properties": { + "external_url": { "type": ["string", "null"] }, + "errors": { + "type": "array", + "items": { "$ref": "error.json" } + } + }, + "additionalProperties": false +} diff --git a/spec/fixtures/sentry/issues_sample_response.json b/spec/fixtures/sentry/issues_sample_response.json new file mode 100644 index 00000000000..ed22499cfa1 --- /dev/null +++ b/spec/fixtures/sentry/issues_sample_response.json @@ -0,0 +1,42 @@ +[{ + "lastSeen": "2018-12-31T12:00:11Z", + "numComments": 0, + "userCount": 0, + "stats": { + "24h": [ + [ + 1546437600, + 0 + ] + ] + }, + "culprit": "sentry.tasks.reports.deliver_organization_user_report", + "title": "gaierror: [Errno -2] Name or service not known", + "id": "11", + "assignedTo": null, + "logger": null, + "type": "error", + "annotations": [], + "metadata": { + "type": "gaierror", + "value": "[Errno -2] Name or service not known" + }, + "status": "unresolved", + "subscriptionDetails": null, + "isPublic": false, + "hasSeen": false, + "shortId": "INTERNAL-4", + "shareId": null, + "firstSeen": "2018-12-17T12:00:14Z", + "count": "21", + "permalink": "35.228.54.90/sentry/internal/issues/11/", + "level": "error", + "isSubscribed": true, + "isBookmarked": false, + "project": { + "slug": "internal", + "id": "1", + "name": "Internal" + }, + "statusDetails": {} + }] diff --git a/spec/lib/sentry/client_spec.rb b/spec/lib/sentry/client_spec.rb new file mode 100644 index 00000000000..b36be0fd9c1 --- /dev/null +++ b/spec/lib/sentry/client_spec.rb @@ -0,0 +1,119 @@ +# frozen_string_literal: true + +require 'spec_helper' + +describe Sentry::Client do + let(:issue_status) { 'unresolved' } + let(:limit) { 20 } + let(:sentry_url) { 'https://sentrytest.gitlab.com/api/0/projects/sentry-org/sentry-project' } + let(:token) { 'test-token' } + + let(:sample_response) do + Gitlab::Utils.deep_indifferent_access( + JSON.parse(File.read(Rails.root.join('spec/fixtures/sentry/issues_sample_response.json'))) + ) + end + + subject(:client) { described_class.new(sentry_url, token) } + + describe '#list_issues' do + subject { client.list_issues(issue_status: issue_status, limit: limit) } + + before do + stub_sentry_request(sentry_url + '/issues/?limit=20&query=is:unresolved', body: sample_response) + end + + it 'returns objects of type ErrorTracking::Error' do + expect(subject.length).to eq(1) + expect(subject[0]).to be_a(Gitlab::ErrorTracking::Error) + end + + context 'error object created from sentry response' do + using RSpec::Parameterized::TableSyntax + + where(:error_object, :sentry_response) do + :id | :id + :first_seen | :firstSeen + :last_seen | :lastSeen + :title | :title + :type | :type + :user_count | :userCount + :count | :count + :message | [:metadata, :value] + :culprit | :culprit + :short_id | :shortId + :status | :status + :frequency | [:stats, '24h'] + :project_id | [:project, :id] + :project_name | [:project, :name] + :project_slug | [:project, :slug] + end + + with_them do + it { expect(subject[0].public_send(error_object)).to eq(sample_response[0].dig(*sentry_response)) } + end + + context 'external_url' do + it 'is constructed correctly' do + expect(subject[0].external_url).to eq('https://sentrytest.gitlab.com/sentry-org/sentry-project/issues/11') + end + end + end + + context 'redirects' do + let(:redirect_to) { 'https://redirected.example.com' } + let(:other_url) { 'https://other.example.org' } + + let!(:redirected_req_stub) { stub_sentry_request(other_url) } + + let!(:redirect_req_stub) do + stub_sentry_request( + sentry_url + '/issues/?limit=20&query=is:unresolved', + status: 302, + headers: { location: redirect_to } + ) + end + + it 'does not follow redirects' do + expect { subject }.to raise_exception(Sentry::Client::Error, 'Sentry response error: 302') + expect(redirect_req_stub).to have_been_requested + expect(redirected_req_stub).not_to have_been_requested + end + end + + # Sentry API returns 404 if there are extra slashes in the URL! + context 'extra slashes in URL' do + let(:sentry_url) { 'https://sentrytest.gitlab.com/api/0/projects//sentry-org/sentry-project/' } + let(:client) { described_class.new(sentry_url, token) } + + let!(:valid_req_stub) do + stub_sentry_request( + 'https://sentrytest.gitlab.com/api/0/projects/sentry-org/sentry-project/' \ + 'issues/?limit=20&query=is:unresolved' + ) + end + + it 'removes extra slashes in api url' do + expect(Gitlab::HTTP).to receive(:get).with( + URI('https://sentrytest.gitlab.com/api/0/projects/sentry-org/sentry-project/issues/'), + anything + ).and_call_original + + client.list_issues(issue_status: issue_status, limit: limit) + + expect(valid_req_stub).to have_been_requested + end + end + end + + private + + def stub_sentry_request(url, body: {}, status: 200, headers: {}) + WebMock.stub_request(:get, url) + .to_return( + status: status, + headers: { 'Content-Type' => 'application/json' }.merge(headers), + body: body.to_json + ) + end +end diff --git a/spec/models/error_tracking/project_error_tracking_setting_spec.rb b/spec/models/error_tracking/project_error_tracking_setting_spec.rb index 83f29718eda..2f8ab21d4b2 100644 --- a/spec/models/error_tracking/project_error_tracking_setting_spec.rb +++ b/spec/models/error_tracking/project_error_tracking_setting_spec.rb @@ -3,33 +3,106 @@ require 'spec_helper' describe ErrorTracking::ProjectErrorTrackingSetting do + include ReactiveCachingHelpers + set(:project) { create(:project) } + subject { create(:project_error_tracking_setting, project: project) } + describe 'Associations' do it { is_expected.to belong_to(:project) } end describe 'Validations' do - subject { create(:project_error_tracking_setting, project: project) } - context 'when api_url is over 255 chars' do - before do + it 'fails validation' do subject.api_url = 'https://' + 'a' * 250 - end - it 'fails validation' do expect(subject).not_to be_valid expect(subject.errors.messages[:api_url]).to include('is too long (maximum is 255 characters)') end end context 'With unsafe url' do - let(:project_error_tracking_setting) { create(:project_error_tracking_setting, project: project) } - it 'fails validation' do - project_error_tracking_setting.api_url = "https://replaceme.com/'><script>alert(document.cookie)</script>" + subject.api_url = "https://replaceme.com/'><script>alert(document.cookie)</script>" + + expect(subject).not_to be_valid + end + end + + context 'URL path' do + it 'fails validation with wrong path' do + subject.api_url = 'http://gitlab.com/project1/something' + + expect(subject).not_to be_valid + expect(subject.errors.messages[:api_url]).to include('path needs to start with /api/0/projects') + end + + it 'passes validation with correct path' do + subject.api_url = 'http://gitlab.com/api/0/projects/project1/something' + + expect(subject).to be_valid + end + end + end + + describe '#sentry_external_url' do + let(:sentry_url) { 'https://sentrytest.gitlab.com/api/0/projects/sentry-org/sentry-project' } + + before do + subject.api_url = sentry_url + end + + it 'returns the correct url' do + expect(subject.class).to receive(:extract_sentry_external_url).with(sentry_url).and_call_original + + result = subject.sentry_external_url + + expect(result).to eq('https://sentrytest.gitlab.com/sentry-org/sentry-project') + end + end + + describe '#sentry_client' do + it 'returns sentry client' do + expect(subject.sentry_client).to be_a(Sentry::Client) + end + end + + describe '#list_sentry_issues' do + let(:issues) { [:list, :of, :issues] } + + let(:opts) do + { issue_status: 'unresolved', limit: 10 } + end + + let(:result) do + subject.list_sentry_issues(**opts) + end + + context 'when cached' do + let(:sentry_client) { spy(:sentry_client) } + + before do + stub_reactive_cache(subject, issues, opts) + synchronous_reactive_cache(subject) + + expect(subject).to receive(:sentry_client).and_return(sentry_client) + end + + it 'returns cached issues' do + expect(sentry_client).to receive(:list_issues).with(opts) + .and_return(issues) + + expect(result).to eq(issues: issues) + end + end + + context 'when not cached' do + it 'returns nil' do + expect(subject).not_to receive(:sentry_client) - expect(project_error_tracking_setting).not_to be_valid + expect(result).to be_nil end end end diff --git a/spec/policies/project_policy_spec.rb b/spec/policies/project_policy_spec.rb index 9cb20854f6e..2a4030de998 100644 --- a/spec/policies/project_policy_spec.rb +++ b/spec/policies/project_policy_spec.rb @@ -24,7 +24,7 @@ describe ProjectPolicy do download_code fork_project create_project_snippet update_issue admin_issue admin_label admin_list read_commit_status read_build read_container_image read_pipeline read_environment read_deployment - read_merge_request download_wiki_code + read_merge_request download_wiki_code read_sentry_issue ] end diff --git a/spec/services/error_tracking/list_issues_service_spec.rb b/spec/services/error_tracking/list_issues_service_spec.rb new file mode 100644 index 00000000000..d9dab1d705c --- /dev/null +++ b/spec/services/error_tracking/list_issues_service_spec.rb @@ -0,0 +1,87 @@ +# frozen_string_literal: true + +require 'spec_helper' + +describe ErrorTracking::ListIssuesService do + set(:user) { create(:user) } + set(:project) { create(:project) } + + let(:sentry_url) { 'https://sentrytest.gitlab.com/api/0/projects/sentry-org/sentry-project' } + let(:token) { 'test-token' } + let(:result) { subject.execute } + + let(:error_tracking_setting) do + create(:project_error_tracking_setting, api_url: sentry_url, token: token, project: project) + end + + subject { described_class.new(project, user) } + + before do + expect(project).to receive(:error_tracking_setting).at_least(:once).and_return(error_tracking_setting) + + project.add_reporter(user) + end + + describe '#execute' do + context 'with authorized user' do + context 'when list_sentry_issues returns issues' do + let(:issues) { [:list, :of, :issues] } + + before do + expect(error_tracking_setting) + .to receive(:list_sentry_issues).and_return(issues: issues) + end + + it 'returns the issues' do + expect(result).to eq(status: :success, issues: issues) + end + end + + context 'when list_sentry_issues returns nil' do + before do + expect(error_tracking_setting) + .to receive(:list_sentry_issues).and_return(nil) + end + + it 'result is not ready' do + expect(result).to eq( + status: :error, http_status: :no_content, message: 'not ready') + end + end + end + + context 'with unauthorized user' do + let(:unauthorized_user) { create(:user) } + + subject { described_class.new(project, unauthorized_user) } + + it 'returns error' do + result = subject.execute + + expect(result).to include(status: :error, message: 'access denied') + end + end + + context 'with error tracking disabled' do + before do + error_tracking_setting.enabled = false + end + + it 'raises error' do + result = subject.execute + + expect(result).to include(status: :error, message: 'not enabled') + end + end + end + + describe '#sentry_external_url' do + let(:external_url) { 'https://sentrytest.gitlab.com/sentry-org/sentry-project' } + + it 'calls ErrorTracking::ProjectErrorTrackingSetting' do + expect(error_tracking_setting).to receive(:sentry_external_url).and_call_original + + subject.external_url + end + end +end diff --git a/spec/services/projects/operations/update_service_spec.rb b/spec/services/projects/operations/update_service_spec.rb index 731be907453..6afae3da80c 100644 --- a/spec/services/projects/operations/update_service_spec.rb +++ b/spec/services/projects/operations/update_service_spec.rb @@ -17,7 +17,7 @@ describe Projects::Operations::UpdateService do { error_tracking_setting_attributes: { enabled: false, - api_url: 'http://url', + api_url: 'http://gitlab.com/api/0/projects/org/project', token: 'token' } } @@ -32,7 +32,7 @@ describe Projects::Operations::UpdateService do project.reload expect(project.error_tracking_setting).not_to be_enabled - expect(project.error_tracking_setting.api_url).to eq('http://url') + expect(project.error_tracking_setting.api_url).to eq('http://gitlab.com/api/0/projects/org/project') expect(project.error_tracking_setting.token).to eq('token') end end @@ -42,7 +42,7 @@ describe Projects::Operations::UpdateService do { error_tracking_setting_attributes: { enabled: true, - api_url: 'http://url', + api_url: 'http://gitlab.com/api/0/projects/org/project', token: 'token' } } @@ -52,7 +52,7 @@ describe Projects::Operations::UpdateService do expect(result[:status]).to eq(:success) expect(project.error_tracking_setting).to be_enabled - expect(project.error_tracking_setting.api_url).to eq('http://url') + expect(project.error_tracking_setting.api_url).to eq('http://gitlab.com/api/0/projects/org/project') expect(project.error_tracking_setting.token).to eq('token') end end |