Store api_url, token as key in list_projects cache55199-reactive-caching

Store api_url and token as part of key for reactive cache of
list_projects. This is needed because the list_projects API allows
getting Sentry projects using an api_url and token passed in POST
params without saving those values into DB.

4 files changed, 103 insertions, 22 deletions

diff --git a/app/models/error_tracking/project_error_tracking_setting.rb b/app/models/error_tracking/project_error_tracking_setting.rb
index 78eddfdb497..7c13faae511 100644
--- a/app/models/error_tracking/project_error_tracking_setting.rb
+++ b/app/models/error_tracking/project_error_tracking_setting.rb
@@ -63,7 +63,11 @@ module ErrorTracking
     end
 
     def list_sentry_projects
-      with_reactive_cache('list_projects', {}) do |result|
+      opts = {
+        'api_url' => self.api_url,
+        'token' => self.token
+      }
+      with_reactive_cache('list_projects', opts) do |result|
         result
       end
     end
@@ -73,6 +77,11 @@ module ErrorTracking
       when 'list_issues'
         { issues: sentry_client.list_issues(**opts.symbolize_keys) }
       when 'list_projects'
+        opts.symbolize_keys!
+        self.api_url = opts[:api_url]
+        self.token = opts[:token]
+        self.enabled = true
+
         { projects: sentry_client.list_projects }
       end
     rescue Sentry::Client::SentryError => e
diff --git a/app/services/error_tracking/list_projects_service.rb b/app/services/error_tracking/list_projects_service.rb
index f2d0904a41f..510692895d9 100644
--- a/app/services/error_tracking/list_projects_service.rb
+++ b/app/services/error_tracking/list_projects_service.rb
@@ -15,7 +15,7 @@ module ErrorTracking
 
       # our results are not yet ready
       if result.nil?
-        return error('not ready', :no_content)
+        return error('Calculating cache entry, try again later', :no_content)
       end
 
       if result[:error].present?
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 c28b3aa88dd..c2e62f07e1e 100644
--- a/spec/models/error_tracking/project_error_tracking_setting_spec.rb
+++ b/spec/models/error_tracking/project_error_tracking_setting_spec.rb
@@ -175,12 +175,19 @@ describe ErrorTracking::ProjectErrorTrackingSetting do
     context 'when cached' do
       let(:sentry_client) { spy(:sentry_client) }
 
+      let(:opts) do
+        {
+          api_url: subject.api_url,
+          token: subject.token
+        }
+      end
+
       before do
-        stub_reactive_cache(subject, projects, {})
+        stub_reactive_cache(subject, projects, opts)
         synchronous_reactive_cache(subject)
 
-        expect(subject).to receive(:sentry_client).and_return(sentry_client)
-        expect(sentry_client).to receive(:list_projects)
+        allow(subject).to receive(:sentry_client).and_return(sentry_client)
+        allow(sentry_client).to receive(:list_projects)
           .and_return(projects)
       end
 
@@ -225,6 +232,57 @@ describe ErrorTracking::ProjectErrorTrackingSetting do
         )
       end
     end
+
+    context 'different cache entries' do
+      let(:sentry_client1) { spy(:sentry_client) }
+      let(:sentry_client2) { spy(:sentry_client) }
+      let(:new_api_url) { 'http://sentry.some_new_server.com/api/0/projects/org/proj' }
+      let(:new_token) { 'new-token' }
+      let(:new_projects) { [:new, :projects] }
+
+      let(:opts) do
+        {
+          api_url: subject.api_url,
+          token: subject.token
+        }
+      end
+
+      let(:new_opts) do
+        {
+          api_url: new_api_url,
+          token: new_token
+        }
+      end
+
+      before do
+        stub_reactive_cache(subject, projects, opts)
+        stub_reactive_cache(subject, new_projects, new_opts)
+        synchronous_reactive_cache(subject)
+
+        allow(subject).to receive(:sentry_client).and_call_original
+
+        allow(Sentry::Client).to receive(:new).with(subject.api_url, subject.token)
+          .and_return(sentry_client1)
+        allow(sentry_client1).to receive(:list_projects).and_return(projects)
+
+        allow(Sentry::Client).to receive(:new).with(new_api_url, new_token)
+          .and_return(sentry_client2)
+        allow(sentry_client2).to receive(:list_projects).and_return(new_projects)
+      end
+
+      it 'returns correct cached projects' do
+        result = subject.list_sentry_projects
+        expect(result).to eq(projects: projects)
+        expect(Sentry::Client).to have_received(:new).with(subject.api_url, subject.token)
+
+        subject.api_url = new_api_url
+        subject.token = new_token
+
+        result = subject.list_sentry_projects
+        expect(result).to eq(projects: new_projects)
+        expect(Sentry::Client).to have_received(:new).with(new_api_url, new_token)
+      end
+    end
   end
 
   context 'slugs' do
diff --git a/spec/services/error_tracking/list_projects_service_spec.rb b/spec/services/error_tracking/list_projects_service_spec.rb
index 0f8dff9f2b2..06509038ff1 100644
--- a/spec/services/error_tracking/list_projects_service_spec.rb
+++ b/spec/services/error_tracking/list_projects_service_spec.rb
@@ -54,6 +54,37 @@ describe ErrorTracking::ListProjectsService do
         end
       end
 
+      context 'error tracking setting returns error' do
+        before do
+          allow(error_tracking_setting).to receive(:list_sentry_projects)
+            .and_return({
+              error: 'Sentry response error: 500',
+              http_status: :bad_request
+            })
+        end
+
+        it 'returns error response' do
+          expect(result[:message]).to eq('Sentry response error: 500')
+          expect(result[:http_status]).to eq(:bad_request)
+          expect(result[:status]).to eq(:error)
+          expect(error_tracking_setting).to have_received(:list_sentry_projects)
+        end
+      end
+
+      context 'error tracking setting returns nil' do
+        before do
+          allow(error_tracking_setting).to receive(:list_sentry_projects)
+            .and_return(nil)
+        end
+
+        it 'returns 204' do
+          expect(result[:message]).to eq('Calculating cache entry, try again later')
+          expect(result[:http_status]).to eq(:no_content)
+          expect(result[:status]).to eq(:error)
+          expect(error_tracking_setting).to have_received(:list_sentry_projects)
+        end
+      end
+
       context 'with invalid url' do
         let(:params) do
           ActionController::Parameters.new(
@@ -85,23 +116,6 @@ describe ErrorTracking::ListProjectsService do
         end
       end
 
-      context 'when list_sentry_projects returns nil' do
-        before do
-          expect(error_tracking_setting)
-            .to receive(:list_sentry_projects).and_return(nil)
-        end
-
-        it 'result is not ready' do
-          result = subject.execute
-
-          expect(result).to eq(
-            status: :error,
-            http_status: :no_content,
-            message: 'not ready'
-          )
-        end
-      end
-
       context 'when list_sentry_projects returns empty array' do
         before do
           expect(error_tracking_setting)