Merge branch 'extract-api-urls' into 'master'

Extract api urls from Sentry Client

See merge request gitlab-org/gitlab!22790

lib/sentry/api_urls.rb

+# frozen_string_literal: true
+
+module Sentry
+  class ApiUrls
+    def initialize(url_base)
+      @uri = URI(url_base).freeze
+    end
+
+    def issues_url
+      with_path(File.join(@uri.path, '/issues/'))
+    end
+
+    def issue_url(issue_id)
+      with_path("/api/0/issues/#{escape(issue_id)}/")
+    end
+
+    def projects_url
+      with_path('/api/0/projects/')
+    end
+
+    def issue_latest_event_url(issue_id)
+      with_path("/api/0/issues/#{escape(issue_id)}/events/latest/")
+    end
+
+    private
+
+    def with_path(new_path)
+      new_uri = @uri.dup
+      # Sentry API returns 404 if there are extra slashes in the URL
+      new_uri.path = new_path.squeeze('/')
+
+      new_uri
+    end
+
+    def escape(param)
+      CGI.escape(param.to_s)
+    end
+  end
+end

lib/sentry/client.rb

@@ -19,6 +19,10 @@ module Sentry
 
     private
 
+    def api_urls
+      @api_urls ||= Sentry::ApiUrls.new(@url)
+    end
+
     def handle_mapping_exceptions(&block)
       yield
     rescue KeyError => e

lib/sentry/client/event.rb

@@ -4,20 +4,13 @@ module Sentry
   class Client
     module Event
       def issue_latest_event(issue_id:)
-        latest_event = http_get(issue_latest_event_api_url(issue_id))[:body]
+        latest_event = http_get(api_urls.issue_latest_event_url(issue_id))[:body]
 
         map_to_event(latest_event)
       end
 
       private
 
-      def issue_latest_event_api_url(issue_id)
-        latest_event_url = URI(url)
-        latest_event_url.path = "/api/0/issues/#{issue_id}/events/latest/"
-
-        latest_event_url
-      end
-
       def map_to_event(event)
         stack_trace = parse_stack_trace(event)
 

lib/sentry/client/issue.rb

@@ -35,14 +35,14 @@ module Sentry
       end
 
       def update_issue(issue_id:, params:)
-        http_put(issue_api_url(issue_id), params)[:body]
+        http_put(api_urls.issue_url(issue_id), params)[:body]
       end
 
       private
 
       def get_issues(**keyword_args)
         response = http_get(
-          issues_api_url,
+          api_urls.issues_url,
           query: list_issue_sentry_query(keyword_args)
         )
 
@@ -72,21 +72,7 @@ module Sentry
       end
 
       def get_issue(issue_id:)
-        http_get(issue_api_url(issue_id))[:body]
-      end
-
-      def issues_api_url
-        issues_url = URI("#{url}/issues/")
-        issues_url.path.squeeze!('/')
-
-        issues_url
-      end
-
-      def issue_api_url(issue_id)
-        issue_url = URI(url)
-        issue_url.path = "/api/0/issues/#{CGI.escape(issue_id.to_s)}/"
-
-        issue_url
+        http_get(api_urls.issue_url(issue_id))[:body]
       end
 
       def parse_gitlab_issue(plugin_issues)

lib/sentry/client/projects.rb

@@ -14,14 +14,7 @@ module Sentry
       private
 
       def get_projects
-        http_get(projects_api_url)[:body]
-      end
-
-      def projects_api_url
-        projects_url = URI(url)
-        projects_url.path = '/api/0/projects/'
-
-        projects_url
+        http_get(api_urls.projects_url)[:body]
       end
 
       def map_to_projects(projects)

spec/lib/sentry/api_urls_spec.rb

+# frozen_string_literal: true
+
+require 'spec_helper'
+
+describe Sentry::ApiUrls do
+  let(:sentry_url) { 'https://sentrytest.gitlab.com/api/0/projects/sentry-org/sentry-project/' }
+  let(:token) { 'test-token' }
+  let(:issue_id) { '123456' }
+  let(:issue_id_with_reserved_chars) { '123$%' }
+  let(:escaped_issue_id) { '123%24%25' }
+  let(:api_urls) { Sentry::ApiUrls.new(sentry_url) }
+
+  # Sentry API returns 404 if there are extra slashes in the URL!
+  shared_examples 'correct url with extra slashes' do
+    let(:sentry_url) { 'https://sentrytest.gitlab.com/api/0/projects//sentry-org/sentry-project/' }
+
+    it_behaves_like 'correct url'
+  end
+
+  shared_examples 'correctly escapes issue ID' do
+    context 'with param a string with reserved chars' do
+      let(:issue_id) { issue_id_with_reserved_chars }
+
+      it { expect(subject.to_s).to include(escaped_issue_id) }
+    end
+
+    context 'with param a symbol with reserved chars' do
+      let(:issue_id) { issue_id_with_reserved_chars.to_sym }
+
+      it { expect(subject.to_s).to include(escaped_issue_id) }
+    end
+
+    context 'with param an integer' do
+      let(:issue_id) { 12345678 }
+
+      it { expect(subject.to_s).to include(issue_id.to_s) }
+    end
+  end
+
+  describe '#issues_url' do
+    subject { api_urls.issues_url }
+
+    shared_examples 'correct url' do
+      it { is_expected.to eq_uri('https://sentrytest.gitlab.com/api/0/projects/sentry-org/sentry-project/issues/') }
+    end
+
+    it_behaves_like 'correct url'
+    it_behaves_like 'correct url with extra slashes'
+  end
+
+  describe '#issue_url' do
+    subject { api_urls.issue_url(issue_id) }
+
+    shared_examples 'correct url' do
+      it { is_expected.to eq_uri("https://sentrytest.gitlab.com/api/0/issues/#{issue_id}/") }
+    end
+
+    it_behaves_like 'correct url'
+    it_behaves_like 'correct url with extra slashes'
+    it_behaves_like 'correctly escapes issue ID'
+  end
+
+  describe '#projects_url' do
+    subject { api_urls.projects_url }
+
+    shared_examples 'correct url' do
+      it { is_expected.to eq_uri('https://sentrytest.gitlab.com/api/0/projects/') }
+    end
+
+    it_behaves_like 'correct url'
+    it_behaves_like 'correct url with extra slashes'
+  end
+
+  describe '#issue_latest_event_url' do
+    subject { api_urls.issue_latest_event_url(issue_id) }
+
+    shared_examples 'correct url' do
+      it { is_expected.to eq_uri("https://sentrytest.gitlab.com/api/0/issues/#{issue_id}/events/latest/") }
+    end
+
+    it_behaves_like 'correct url'
+    it_behaves_like 'correct url with extra slashes'
+    it_behaves_like 'correctly escapes issue ID'
+  end
+end

spec/lib/sentry/client/issue_spec.rb

@@ -109,28 +109,6 @@ describe Sentry::Client::Issue do
       it_behaves_like 'no Sentry redirects'
     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(:sentry_request_url) do
-        '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(client.url).to eq(sentry_url)
-        expect(Gitlab::HTTP).to receive(:get).with(
-          URI('https://sentrytest.gitlab.com/api/0/projects/sentry-org/sentry-project/issues/'),
-          anything
-        ).and_call_original
-
-        subject
-
-        expect(sentry_api_request).to have_been_requested
-      end
-    end
-
     context 'requests with sort parameter in sentry api' do
       let(:sentry_request_url) do
         'https://sentrytest.gitlab.com/api/0/projects/sentry-org/sentry-project/' \
@@ -232,14 +210,6 @@ describe Sentry::Client::Issue do
 
     subject { client.issue_details(issue_id: issue_id) }
 
-    it 'escapes issue ID' do
-      allow(CGI).to receive(:escape).and_call_original
-
-      subject
-
-      expect(CGI).to have_received(:escape).with(issue_id.to_s)
-    end
-
     context 'error object created from sentry response' do
       using RSpec::Parameterized::TableSyntax
 

spec/lib/sentry/client/projects_spec.rb

@@ -91,25 +91,6 @@ describe Sentry::Client::Projects do
       it_behaves_like 'no Sentry redirects'
     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//' }
-      let!(:valid_req_stub) do
-        stub_sentry_request(sentry_list_projects_url)
-      end
-
-      it 'removes extra slashes in api url' do
-        expect(Gitlab::HTTP).to receive(:get).with(
-          URI(sentry_list_projects_url),
-          anything
-        ).and_call_original
-
-        subject
-
-        expect(valid_req_stub).to have_been_requested
-      end
-    end
-
     context 'when exception is raised' do
       let(:sentry_request_url) { sentry_list_projects_url }
 

spec/support/matchers/eq_uri.rb

+# frozen_string_literal: true
+
+# Assert the result matches a URI object initialized with the expectation variable.
+#
+# Success:
+# ```
+# expect(URI('www.fish.com')).to eq_uri('www.fish.com')
+# ```
+#
+# Failure:
+# ```
+# expect(URI('www.fish.com')).to eq_uri('www.dog.com')
+# ```
+#
+RSpec::Matchers.define :eq_uri do |expected|
+  match do |actual|
+    actual == URI(expected)
+  end
+end