Merge branch 'pl-error-tracking-limit' into 'master'

Resolve all RuboCop offenses for Sentry Client code and specs

See merge request gitlab-org/gitlab!84190

.rubocop_todo/layout/first_hash_element_indentation.yml

@@ -256,7 +256,6 @@ Layout/FirstHashElementIndentation:
     - 'lib/banzai/filter/commit_trailers_filter.rb'
     - 'lib/banzai/filter/playable_link_filter.rb'
     - 'lib/banzai/reference_parser/user_parser.rb'
-    - 'lib/error_tracking/sentry_client/issue.rb'
     - 'lib/gitlab/analytics/cycle_analytics/aggregated/records_fetcher.rb'
     - 'lib/gitlab/analytics/cycle_analytics/records_fetcher.rb'
     - 'lib/gitlab/asciidoc.rb'

.rubocop_todo/rspec/verified_doubles.yml

@@ -437,7 +437,6 @@ RSpec/VerifiedDoubles:
     - spec/lib/error_tracking/collector/dsn_spec.rb
     - spec/lib/error_tracking/collector/sentry_auth_parser_spec.rb
     - spec/lib/error_tracking/collector/sentry_request_parser_spec.rb
-    - spec/lib/error_tracking/sentry_client/issue_spec.rb
     - spec/lib/extracts_path_spec.rb
     - spec/lib/feature_spec.rb
     - spec/lib/gitaly/server_spec.rb

lib/error_tracking/sentry_client.rb

@@ -59,10 +59,8 @@ module ErrorTracking
       end
     end
 
-    def http_request
-      response = handle_request_exceptions do
-        yield
-      end
+    def http_request(&block)
+      response = handle_request_exceptions(&block)
 
       handle_response(response)
     end
@@ -86,9 +84,7 @@ module ErrorTracking
     end
 
     def handle_response(response)
-      unless response.code.between?(200, 204)
-        raise_error "Sentry response status code: #{response.code}"
-      end
+      raise_error "Sentry response status code: #{response.code}" unless response.code.between?(200, 204)
 
       { body: response.parsed_response, headers: response.headers }
     end

lib/error_tracking/sentry_client/event.rb

@@ -15,14 +15,14 @@ module ErrorTracking
         stack_trace = parse_stack_trace(event)
 
         Gitlab::ErrorTracking::ErrorEvent.new(
-          issue_id: event.dig('groupID'),
-          date_received: event.dig('dateReceived'),
+          issue_id: event['groupID'],
+          date_received: event['dateReceived'],
           stack_trace_entries: stack_trace
         )
       end
 
       def parse_stack_trace(event)
-        exception_entry = event.dig('entries')&.detect { |h| h['type'] == 'exception' }
+        exception_entry = event['entries']&.detect { |h| h['type'] == 'exception' }
         return [] unless exception_entry
 
         exception_values = exception_entry.dig('data', 'values')

lib/error_tracking/sentry_client/issue.rb

@@ -54,9 +54,7 @@ module ErrorTracking
       end
 
       def list_issue_sentry_query(issue_status:, limit:, sort: nil, search_term: '', cursor: nil)
-        unless SENTRY_API_SORT_VALUE_MAP.key?(sort)
-          raise BadRequestError, 'Invalid value for sort param'
-        end
+        raise BadRequestError, 'Invalid value for sort param' unless SENTRY_API_SORT_VALUE_MAP.key?(sort)
 
         {
           query: "is:#{issue_status} #{search_term}".strip,
@@ -69,7 +67,8 @@ module ErrorTracking
       def validate_size(issues)
         return if Gitlab::Utils::DeepSize.new(issues).valid?
 
-        raise ResponseInvalidSizeError, "Sentry API response is too big. Limit is #{Gitlab::Utils::DeepSize.human_default_max_size}."
+        message = "Sentry API response is too big. Limit is #{Gitlab::Utils::DeepSize.human_default_max_size}."
+        raise ResponseInvalidSizeError, message
       end
 
       def get_issue(issue_id:)
@@ -117,7 +116,7 @@ module ErrorTracking
       end
 
       def map_to_errors(issues)
-        issues.map(&method(:map_to_error))
+        issues.map { map_to_error(_1) }
       end
 
       def map_to_error(issue)
@@ -142,7 +141,7 @@ module ErrorTracking
       end
 
       def map_to_detailed_error(issue)
-        Gitlab::ErrorTracking::DetailedError.new({
+        Gitlab::ErrorTracking::DetailedError.new(
           id: issue.fetch('id'),
           first_seen: issue.fetch('firstSeen', nil),
           last_seen: issue.fetch('lastSeen', nil),
@@ -169,7 +168,7 @@ module ErrorTracking
           last_release_short_version: issue.dig('lastRelease', 'shortVersion'),
           last_release_version: issue.dig('lastRelease', 'version'),
           integrated: false
-        })
+        )
       end
 
       def extract_tags(issue)

lib/error_tracking/sentry_client/pagination_parser.rb

@@ -3,7 +3,7 @@
 module ErrorTracking
   class SentryClient
     module PaginationParser
-      PATTERN = /rel=\"(?<direction>\w+)\";\sresults=\"(?<results>\w+)\";\scursor=\"(?<cursor>.+)\"/.freeze
+      PATTERN = /rel="(?<direction>\w+)";\sresults="(?<results>\w+)";\scursor="(?<cursor>.+)"/.freeze
 
       def self.parse(headers)
         links = headers['link'].to_s.split(',')

lib/error_tracking/sentry_client/projects.rb

@@ -18,7 +18,7 @@ module ErrorTracking
       end
 
       def map_to_projects(projects)
-        projects.map(&method(:map_to_project))
+        projects.map { map_to_project(_1) }
       end
 
       def map_to_project(project)
@@ -28,7 +28,7 @@ module ErrorTracking
           id: project.fetch('id', nil),
           name: project.fetch('name'),
           slug: project.fetch('slug'),
-          status: project.dig('status'),
+          status: project['status'],
           organization_name: organization.fetch('name'),
           organization_id: organization.fetch('id', nil),
           organization_slug: organization.fetch('slug')

lib/error_tracking/sentry_client/repo.rb

@@ -23,7 +23,7 @@ module ErrorTracking
       end
 
       def map_to_repos(repos)
-        repos.map(&method(:map_to_repo))
+        repos.map { map_to_repo(_1) }
       end
 
       def map_to_repo(repo)

spec/lib/error_tracking/sentry_client/issue_spec.rb

@@ -13,7 +13,7 @@ RSpec.describe ErrorTracking::SentryClient::Issue do
   describe '#list_issues' do
     shared_examples 'issues have correct return type' do |klass|
       it "returns objects of type #{klass}" do
-        expect(subject[:issues]).to all( be_a(klass) )
+        expect(subject[:issues]).to all(be_a(klass))
       end
     end
 
@@ -41,10 +41,18 @@ RSpec.describe ErrorTracking::SentryClient::Issue do
     let(:cursor) { nil }
     let(:sort) { 'last_seen' }
     let(:sentry_api_response) { issues_sample_response }
-    let(:sentry_request_url) { sentry_url + '/issues/?limit=20&query=is:unresolved' }
+    let(:sentry_request_url) { "#{sentry_url}/issues/?limit=20&query=is:unresolved" }
     let!(:sentry_api_request) { stub_sentry_request(sentry_request_url, body: sentry_api_response) }
 
-    subject { client.list_issues(issue_status: issue_status, limit: limit, search_term: search_term, sort: sort, cursor: cursor) }
+    subject do
+      client.list_issues(
+        issue_status: issue_status,
+        limit: limit,
+        search_term: search_term,
+        sort: sort,
+        cursor: cursor
+      )
+    end
 
     it_behaves_like 'calls sentry api'
 
@@ -52,7 +60,7 @@ RSpec.describe ErrorTracking::SentryClient::Issue do
     it_behaves_like 'issues have correct length', 3
 
     shared_examples 'has correct external_url' do
-      context 'external_url' do
+      describe '#external_url' do
         it 'is constructed correctly' do
           expect(subject[:issues][0].external_url).to eq('https://sentrytest.gitlab.com/sentry-org/sentry-project/issues/11')
         end
@@ -62,7 +70,8 @@ RSpec.describe ErrorTracking::SentryClient::Issue do
     context 'when response has a pagination info' do
       let(:headers) do
         {
-          link: '<https://sentrytest.gitlab.com>; rel="previous"; results="true"; cursor="1573556671000:0:1", <https://sentrytest.gitlab.com>; rel="next"; results="true"; cursor="1572959139000:0:0"'
+          link: '<https://sentrytest.gitlab.com>; rel="previous"; results="true"; cursor="1573556671000:0:1",' \
+                '<https://sentrytest.gitlab.com>; rel="next"; results="true"; cursor="1572959139000:0:0"'
         }
       end
 
@@ -76,7 +85,7 @@ RSpec.describe ErrorTracking::SentryClient::Issue do
       end
     end
 
-    context 'error object created from sentry response' do
+    context 'when error object created from sentry response' do
       using RSpec::Parameterized::TableSyntax
 
       where(:error_object, :sentry_response) do
@@ -104,13 +113,13 @@ RSpec.describe ErrorTracking::SentryClient::Issue do
       it_behaves_like 'has correct external_url'
     end
 
-    context 'redirects' do
-      let(:sentry_api_url) { sentry_url + '/issues/?limit=20&query=is:unresolved' }
+    context 'with redirects' do
+      let(:sentry_api_url) { "#{sentry_url}/issues/?limit=20&query=is:unresolved" }
 
       it_behaves_like 'no Sentry redirects'
     end
 
-    context 'requests with sort parameter in sentry api' do
+    context 'with sort parameter in sentry api' do
       let(:sentry_request_url) do
         'https://sentrytest.gitlab.com/api/0/projects/sentry-org/sentry-project/' \
           'issues/?limit=20&query=is:unresolved&sort=freq'
@@ -140,7 +149,7 @@ RSpec.describe ErrorTracking::SentryClient::Issue do
       end
     end
 
-    context 'Older sentry versions where keys are not present' do
+    context 'with older sentry versions where keys are not present' do
       let(:sentry_api_response) do
         issues_sample_response[0...1].map do |issue|
           issue[:project].delete(:id)
@@ -156,7 +165,7 @@ RSpec.describe ErrorTracking::SentryClient::Issue do
       it_behaves_like 'has correct external_url'
     end
 
-    context 'essential keys missing in API response' do
+    context 'when essential keys are missing in API response' do
       let(:sentry_api_response) do
         issues_sample_response[0...1].map do |issue|
           issue.except(:id)
@@ -164,16 +173,18 @@ RSpec.describe ErrorTracking::SentryClient::Issue do
       end
 
       it 'raises exception' do
-        expect { subject }.to raise_error(ErrorTracking::SentryClient::MissingKeysError, 'Sentry API response is missing keys. key not found: "id"')
+        expect { subject }.to raise_error(ErrorTracking::SentryClient::MissingKeysError,
+                                          'Sentry API response is missing keys. key not found: "id"')
       end
     end
 
-    context 'sentry api response too large' do
+    context 'when sentry api response is too large' do
       it 'raises exception' do
-        deep_size = double('Gitlab::Utils::DeepSize', valid?: false)
+        deep_size = instance_double(Gitlab::Utils::DeepSize, valid?: false)
         allow(Gitlab::Utils::DeepSize).to receive(:new).with(sentry_api_response).and_return(deep_size)
 
-        expect { subject }.to raise_error(ErrorTracking::SentryClient::ResponseInvalidSizeError, 'Sentry API response is too big. Limit is 1 MB.')
+        expect { subject }.to raise_error(ErrorTracking::SentryClient::ResponseInvalidSizeError,
+                                          'Sentry API response is too big. Limit is 1 MB.')
       end
     end
 
@@ -212,7 +223,7 @@ RSpec.describe ErrorTracking::SentryClient::Issue do
 
     subject { client.issue_details(issue_id: issue_id) }
 
-    context 'error object created from sentry response' do
+    context 'with error object created from sentry response' do
       using RSpec::Parameterized::TableSyntax
 
       where(:error_object, :sentry_response) do
@@ -298,17 +309,16 @@ RSpec.describe ErrorTracking::SentryClient::Issue do
   describe '#update_issue' do
     let(:sentry_url) { 'https://sentrytest.gitlab.com/api/0' }
     let(:sentry_request_url) { "#{sentry_url}/issues/#{issue_id}/" }
-
-    before do
-      stub_sentry_request(sentry_request_url, :put)
-    end
-
     let(:params) do
       {
         status: 'resolved'
       }
     end
 
+    before do
+      stub_sentry_request(sentry_request_url, :put)
+    end
+
     subject { client.update_issue(issue_id: issue_id, params: params) }
 
     it_behaves_like 'calls sentry api' do
@@ -319,7 +329,7 @@ RSpec.describe ErrorTracking::SentryClient::Issue do
       expect(subject).to be_truthy
     end
 
-    context 'error encountered' do
+    context 'when error is encountered' do
       let(:error) { StandardError.new('error') }
 
       before do