Merge branch 'reduce_redundent_api_call' into 'master'

Avoid unnecessary Sidekiq retries for Security::TokenRevocationService

See merge request gitlab-org/gitlab!48636

ee/app/services/security/token_revocation_service.rb

@@ -11,7 +11,10 @@ module Security
     end
 
     def execute
+      raise RevocationFailedError, 'Missing revocation token data' if missing_token_data?
+
       return error('Token revocation is disabled') unless token_revocation_enabled?
+      return success if revoke_token_body.blank?
 
       response = revoke_tokens
       response.success? ? success : error('Failed to revoke tokens')
@@ -29,11 +32,9 @@ module Security
     end
 
     def revoke_tokens
-      raise RevocationFailedError, 'Missing revocation tokens data' if missing_token_data?
-
       ::Gitlab::HTTP.post(
         token_revocation_url,
-        body: message,
+        body: revoke_token_body,
         headers: {
          'Content-Type' => 'application/json',
          'Authorization' => revocation_api_token
@@ -54,7 +55,8 @@ module Security
       )
     end
 
-    def message
+    def revoke_token_body
+      @revoke_token_body ||= begin
          response = ::Gitlab::HTTP.get(
            token_types_url,
            headers: {
@@ -65,13 +67,14 @@ module Security
          raise RevocationFailedError, 'Failed to get revocation token types' unless response.success?
 
          token_types = ::Gitlab::Json.parse(response.body)['types']
-      raise RevocationFailedError, 'No token type is available' if token_types.blank?
+         return if token_types.blank?
 
          @revocable_keys.filter! { |key| token_types.include?(key[:type]) }
-      raise RevocationFailedError, 'No revocable key is present' if @revocable_keys.blank?
+         return if @revocable_keys.blank?
 
          @revocable_keys.to_json
        end
+    end
 
     def token_types_url
       ::Gitlab::CurrentSettings.secret_detection_revocation_token_types_url

ee/changelogs/unreleased/reduce_redundent_api_call.yml

+---
+title: Avoid unnecessary Sidekiq retries for Security::TokenRevocationService
+merge_request: 48636
+author:
+type: performance

ee/spec/services/security/token_revocation_service_spec.rb

@@ -49,13 +49,13 @@ RSpec.describe Security::TokenRevocationService, '#execute' do
     end
   end
 
-  context 'when revocation token API returns invalid token types' do
+  context 'when revocation token types API returns empty list of types' do
     before do
       stub_application_setting(secret_detection_token_revocation_enabled: true)
       stub_invalid_token_types_api_with_success
     end
 
-    specify { expect(subject).to eql({ message: 'No token type is available', status: :error }) }
+    specify { expect(subject).to eql({ status: :success }) }
   end
 
   context 'when revocation service is disabled' do
@@ -84,7 +84,7 @@ RSpec.describe Security::TokenRevocationService, '#execute' do
           end
         end
 
-        specify { expect(subject).to eql({ message: 'Missing revocation tokens data', status: :error }) }
+        specify { expect(subject).to eql({ message: 'Missing revocation token data', status: :error }) }
       end
 
       context 'when token_types_url is missing' do
@@ -94,7 +94,7 @@ RSpec.describe Security::TokenRevocationService, '#execute' do
           end
         end
 
-        specify { expect(subject).to eql({ message: 'Missing revocation tokens data', status: :error }) }
+        specify { expect(subject).to eql({ message: 'Missing revocation token data', status: :error }) }
       end
 
       context 'when revocation_api_token is missing' do
@@ -104,7 +104,7 @@ RSpec.describe Security::TokenRevocationService, '#execute' do
           end
         end
 
-        specify { expect(subject).to eql({ message: 'Missing revocation tokens data', status: :error }) }
+        specify { expect(subject).to eql({ message: 'Missing revocation token data', status: :error }) }
       end
 
       context 'when there is no token to be revoked' do
@@ -112,7 +112,7 @@ RSpec.describe Security::TokenRevocationService, '#execute' do
           { 'types': %w() }
         end
 
-        specify { expect(subject).to eql({ message: 'No token type is available', status: :error }) }
+        specify { expect(subject).to eql({ status: :success }) }
       end
     end