Service for calling Sentry issues api

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