summaryrefslogtreecommitdiff
diff options
context:
space:
mode:
authorReuben Pereira <rpereira@gitlab.com>2019-01-09 21:04:27 +0000
committerKamil Trzciński <ayufan@ayufan.eu>2019-01-09 21:04:27 +0000
commitd69074fc722351fef313b09255a7e734c01618ac (patch)
treec2e9c2c15bd87eac4673428be687676167f1c1f0
parent47698eec4086b82047a677e218a5299f73c46109 (diff)
downloadgitlab-ce-d69074fc722351fef313b09255a7e734c01618ac.tar.gz
Service for calling Sentry issues api
-rw-r--r--app/controllers/projects/error_tracking_controller.rb54
-rw-r--r--app/models/error_tracking/project_error_tracking_setting.rb45
-rw-r--r--app/policies/project_policy.rb1
-rw-r--r--app/serializers/error_tracking/error_entity.rb10
-rw-r--r--app/serializers/error_tracking/error_serializer.rb7
-rw-r--r--app/services/error_tracking/list_issues_service.rb49
-rw-r--r--app/views/projects/error_tracking/index.html.haml1
-rw-r--r--config/routes/project.rb2
-rw-r--r--lib/gitlab/error_tracking/error.rb14
-rw-r--r--lib/sentry/client.rb104
-rw-r--r--locale/gitlab.pot3
-rw-r--r--spec/controllers/projects/error_tracking_controller_spec.rb142
-rw-r--r--spec/factories/error_tracking/error.rb24
-rw-r--r--spec/factories/project_error_tracking_settings.rb2
-rw-r--r--spec/fixtures/api/schemas/error_tracking/error.json21
-rw-r--r--spec/fixtures/api/schemas/error_tracking/index.json15
-rw-r--r--spec/fixtures/sentry/issues_sample_response.json42
-rw-r--r--spec/lib/sentry/client_spec.rb119
-rw-r--r--spec/models/error_tracking/project_error_tracking_setting_spec.rb91
-rw-r--r--spec/policies/project_policy_spec.rb2
-rw-r--r--spec/services/error_tracking/list_issues_service_spec.rb87
-rw-r--r--spec/services/projects/operations/update_service_spec.rb8
22 files changed, 828 insertions, 15 deletions
diff --git a/app/controllers/projects/error_tracking_controller.rb b/app/controllers/projects/error_tracking_controller.rb
new file mode 100644
index 00000000000..4596b6c91f2
--- /dev/null
+++ b/app/controllers/projects/error_tracking_controller.rb
@@ -0,0 +1,54 @@
+# frozen_string_literal: true
+
+class Projects::ErrorTrackingController < Projects::ApplicationController
+ before_action :check_feature_flag!
+ before_action :authorize_read_sentry_issue!
+ before_action :push_feature_flag_to_frontend
+
+ POLLING_INTERVAL = 10_000
+
+ def index
+ respond_to do |format|
+ format.html
+ format.json do
+ set_polling_interval
+ render_index_json
+ end
+ end
+ end
+
+ private
+
+ def render_index_json
+ service = ErrorTracking::ListIssuesService.new(project, current_user)
+ result = service.execute
+
+ unless result[:status] == :success
+ return render json: { message: result[:message] },
+ status: result[:http_status] || :bad_request
+ end
+
+ render json: {
+ errors: serialize_errors(result[:issues]),
+ external_url: service.external_url
+ }
+ end
+
+ def set_polling_interval
+ Gitlab::PollingInterval.set_header(response, interval: POLLING_INTERVAL)
+ end
+
+ def serialize_errors(errors)
+ ErrorTracking::ErrorSerializer
+ .new(project: project, user: current_user)
+ .represent(errors)
+ end
+
+ def check_feature_flag!
+ render_404 unless Feature.enabled?(:error_tracking, project)
+ end
+
+ def push_feature_flag_to_frontend
+ push_frontend_feature_flag(:error_tracking, current_user)
+ end
+end
diff --git a/app/models/error_tracking/project_error_tracking_setting.rb b/app/models/error_tracking/project_error_tracking_setting.rb
index 632c64c2f1c..7f4947ba27a 100644
--- a/app/models/error_tracking/project_error_tracking_setting.rb
+++ b/app/models/error_tracking/project_error_tracking_setting.rb
@@ -2,13 +2,58 @@
module ErrorTracking
class ProjectErrorTrackingSetting < ActiveRecord::Base
+ include ReactiveCaching
+
+ self.reactive_cache_key = ->(setting) { [setting.class.model_name.singular, setting.project_id] }
+
belongs_to :project
validates :api_url, length: { maximum: 255 }, public_url: true, url: { enforce_sanitization: true }
+ validate :validate_api_url_path
+
attr_encrypted :token,
mode: :per_attribute_iv,
key: Settings.attr_encrypted_db_key_base_truncated,
algorithm: 'aes-256-gcm'
+
+ after_save :clear_reactive_cache!
+
+ def sentry_client
+ Sentry::Client.new(api_url, token)
+ end
+
+ def sentry_external_url
+ self.class.extract_sentry_external_url(api_url)
+ end
+
+ def list_sentry_issues(opts = {})
+ with_reactive_cache('list_issues', opts.stringify_keys) do |result|
+ { issues: result }
+ end
+ end
+
+ def calculate_reactive_cache(request, opts)
+ case request
+ when 'list_issues'
+ sentry_client.list_issues(**opts.symbolize_keys)
+ end
+ end
+
+ # http://HOST/api/0/projects/ORG/PROJECT
+ # ->
+ # http://HOST/ORG/PROJECT
+ def self.extract_sentry_external_url(url)
+ url.sub('api/0/projects/', '')
+ end
+
+ private
+
+ def validate_api_url_path
+ unless URI(api_url).path.starts_with?('/api/0/projects')
+ errors.add(:api_url, 'path needs to start with /api/0/projects')
+ end
+ rescue URI::InvalidURIError
+ end
end
end
diff --git a/app/policies/project_policy.rb b/app/policies/project_policy.rb
index d70417e710e..12f9f29dcc1 100644
--- a/app/policies/project_policy.rb
+++ b/app/policies/project_policy.rb
@@ -200,6 +200,7 @@ class ProjectPolicy < BasePolicy
enable :read_environment
enable :read_deployment
enable :read_merge_request
+ enable :read_sentry_issue
end
# We define `:public_user_access` separately because there are cases in gitlab-ee
diff --git a/app/serializers/error_tracking/error_entity.rb b/app/serializers/error_tracking/error_entity.rb
new file mode 100644
index 00000000000..91388e7c3ad
--- /dev/null
+++ b/app/serializers/error_tracking/error_entity.rb
@@ -0,0 +1,10 @@
+# frozen_string_literal: true
+
+module ErrorTracking
+ class ErrorEntity < Grape::Entity
+ expose :id, :title, :type, :user_count, :count,
+ :first_seen, :last_seen, :message, :culprit,
+ :external_url, :project_id, :project_name, :project_slug,
+ :short_id, :status, :frequency
+ end
+end
diff --git a/app/serializers/error_tracking/error_serializer.rb b/app/serializers/error_tracking/error_serializer.rb
new file mode 100644
index 00000000000..ff9a645eb16
--- /dev/null
+++ b/app/serializers/error_tracking/error_serializer.rb
@@ -0,0 +1,7 @@
+# frozen_string_literal: true
+
+module ErrorTracking
+ class ErrorSerializer < BaseSerializer
+ entity ErrorEntity
+ end
+end
diff --git a/app/services/error_tracking/list_issues_service.rb b/app/services/error_tracking/list_issues_service.rb
new file mode 100644
index 00000000000..4cc35cfa4a8
--- /dev/null
+++ b/app/services/error_tracking/list_issues_service.rb
@@ -0,0 +1,49 @@
+# frozen_string_literal: true
+
+module ErrorTracking
+ class ListIssuesService < ::BaseService
+ DEFAULT_ISSUE_STATUS = 'unresolved'
+ DEFAULT_LIMIT = 20
+
+ def execute
+ return error('not enabled') unless enabled?
+ return error('access denied') unless can_read?
+
+ result = project_error_tracking_setting
+ .list_sentry_issues(issue_status: issue_status, limit: limit)
+
+ # our results are not yet ready
+ unless result
+ return error('not ready', :no_content)
+ end
+
+ success(issues: result[:issues])
+ end
+
+ def external_url
+ project_error_tracking_setting&.sentry_external_url
+ end
+
+ private
+
+ def project_error_tracking_setting
+ project.error_tracking_setting
+ end
+
+ def issue_status
+ params[:issue_status] || DEFAULT_ISSUE_STATUS
+ end
+
+ def limit
+ params[:limit] || DEFAULT_LIMIT
+ end
+
+ def enabled?
+ project_error_tracking_setting&.enabled?
+ end
+
+ def can_read?
+ can?(current_user, :read_sentry_issue, project)
+ end
+ end
+end
diff --git a/app/views/projects/error_tracking/index.html.haml b/app/views/projects/error_tracking/index.html.haml
new file mode 100644
index 00000000000..a3e0dc75f6f
--- /dev/null
+++ b/app/views/projects/error_tracking/index.html.haml
@@ -0,0 +1 @@
+- page_title _('Errors')
diff --git a/config/routes/project.rb b/config/routes/project.rb
index cf5a57300cf..e6ecb4bc9d8 100644
--- a/config/routes/project.rb
+++ b/config/routes/project.rb
@@ -442,6 +442,8 @@ constraints(::Constraints::ProjectUrlConstrainer.new) do
end
end
+ resources :error_tracking, only: [:index], controller: :error_tracking
+
# Since both wiki and repository routing contains wildcard characters
# its preferable to keep it below all other project routes
draw :wiki
diff --git a/lib/gitlab/error_tracking/error.rb b/lib/gitlab/error_tracking/error.rb
new file mode 100644
index 00000000000..4af5192aa6a
--- /dev/null
+++ b/lib/gitlab/error_tracking/error.rb
@@ -0,0 +1,14 @@
+# frozen_string_literal: true
+
+module Gitlab
+ module ErrorTracking
+ class Error
+ include ActiveModel::Model
+
+ attr_accessor :id, :title, :type, :user_count, :count,
+ :first_seen, :last_seen, :message, :culprit,
+ :external_url, :project_id, :project_name, :project_slug,
+ :short_id, :status, :frequency
+ end
+ end
+end
diff --git a/lib/sentry/client.rb b/lib/sentry/client.rb
new file mode 100644
index 00000000000..343f2c49a7f
--- /dev/null
+++ b/lib/sentry/client.rb
@@ -0,0 +1,104 @@
+# frozen_string_literal: true
+
+module Sentry
+ class Client
+ Error = Class.new(StandardError)
+
+ attr_accessor :url, :token
+
+ def initialize(api_url, token)
+ @url = api_url
+ @token = token
+ end
+
+ def list_issues(issue_status:, limit:)
+ issues = get_issues(issue_status: issue_status, limit: limit)
+ map_to_errors(issues)
+ end
+
+ private
+
+ def request_params
+ {
+ headers: {
+ 'Authorization' => "Bearer #{@token}"
+ },
+ follow_redirects: false
+ }
+ end
+
+ def get_issues(issue_status:, limit:)
+ resp = Gitlab::HTTP.get(
+ issues_api_url,
+ **request_params.merge(query: {
+ query: "is:#{issue_status}",
+ limit: limit
+ })
+ )
+
+ handle_response(resp)
+ end
+
+ def handle_response(response)
+ unless response.code == 200
+ raise Client::Error, "Sentry response error: #{response.code}"
+ end
+
+ response.as_json
+ end
+
+ def issues_api_url
+ issues_url = URI(@url + '/issues/')
+ issues_url.path.squeeze!('/')
+
+ issues_url
+ end
+
+ def map_to_errors(issues)
+ issues.map do |issue|
+ map_to_error(issue)
+ end
+ end
+
+ def issue_url(id)
+ issues_url = @url + "/issues/#{id}"
+ issues_url = ErrorTracking::ProjectErrorTrackingSetting.extract_sentry_external_url(issues_url)
+
+ uri = URI(issues_url)
+ uri.path.squeeze!('/')
+
+ uri.to_s
+ end
+
+ def map_to_error(issue)
+ id = issue.fetch('id')
+ project = issue.fetch('project')
+
+ count = issue.fetch('count', nil)
+
+ frequency = issue.dig('stats', '24h')
+ message = issue.dig('metadata', 'value')
+
+ external_url = issue_url(id)
+
+ Gitlab::ErrorTracking::Error.new(
+ id: id,
+ first_seen: issue.fetch('firstSeen', nil),
+ last_seen: issue.fetch('lastSeen', nil),
+ title: issue.fetch('title', nil),
+ type: issue.fetch('type', nil),
+ user_count: issue.fetch('userCount', nil),
+ count: count,
+ message: message,
+ culprit: issue.fetch('culprit', nil),
+ external_url: external_url,
+ short_id: issue.fetch('shortId', nil),
+ status: issue.fetch('status', nil),
+ frequency: frequency,
+ project_id: project.fetch('id'),
+ project_name: project.fetch('name', nil),
+ project_slug: project.fetch('slug', nil)
+ )
+ end
+ end
+end
diff --git a/locale/gitlab.pot b/locale/gitlab.pot
index 501acb6947a..bb7c58ae244 100644
--- a/locale/gitlab.pot
+++ b/locale/gitlab.pot
@@ -2959,6 +2959,9 @@ msgstr ""
msgid "Error while loading the merge request. Please try again."
msgstr ""
+msgid "Errors"
+msgstr ""
+
msgid "Estimated"
msgstr ""
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