Merge branch '347389-migrate-WebHookWorker-to-use-params-arg-step-2' into 'master'

Migrate WebHookWorker to use params: step 2

See merge request gitlab-org/gitlab!79173

app/services/web_hook_service.rb

@@ -102,9 +102,11 @@ class WebHookService
       break log_rate_limited if rate_limited?
       break log_recursion_blocked if recursion_blocked?
 
-      data[:_gitlab_recursion_detection_request_uuid] = Gitlab::WebHooks::RecursionDetection::UUID.instance.request_uuid
+      params = {
+        recursion_detection_request_uuid: Gitlab::WebHooks::RecursionDetection::UUID.instance.request_uuid
+      }.compact
 
-      WebHookWorker.perform_async(hook.id, data, hook_name)
+      WebHookWorker.perform_async(hook.id, data, hook_name, params)
     end
   end
 

app/workers/web_hook_worker.rb

@@ -6,14 +6,14 @@ class WebHookWorker
   include ApplicationWorker
 
   feature_category :integrations
-  loggable_arguments 2
+  loggable_arguments 2, 3
   data_consistency :delayed
   sidekiq_options retry: 4, dead: false
   urgency :low
 
   worker_has_external_dependencies!
 
-  # Webhook recursion detection properties are passed through the `data` arg.
+  # Webhook recursion detection properties may be passed through the `data` arg.
   # This will be migrated to the `params` arg over the next few releases.
   # See https://gitlab.com/gitlab-org/gitlab/-/issues/347389.
   def perform(hook_id, data, hook_name, params = {})
@@ -21,12 +21,14 @@ class WebHookWorker
     return unless hook
 
     data = data.with_indifferent_access
+    params.symbolize_keys!
 
-    # Before executing the hook, reapply any recursion detection UUID that was
-    # initially present in the request header so the hook can pass this same header
-    # value in its request.
-    recursion_detection_uuid = data.delete(:_gitlab_recursion_detection_request_uuid)
-    Gitlab::WebHooks::RecursionDetection.set_request_uuid(recursion_detection_uuid)
+    # TODO: Remove in 14.9 https://gitlab.com/gitlab-org/gitlab/-/issues/347389
+    params[:recursion_detection_request_uuid] ||= data.delete(:_gitlab_recursion_detection_request_uuid)
+
+    # Before executing the hook, reapply any recursion detection UUID that was initially
+    # present in the request header so the hook can pass this same header value in its request.
+    Gitlab::WebHooks::RecursionDetection.set_request_uuid(params[:recursion_detection_request_uuid])
 
     WebHookService.new(hook, data, hook_name, jid).execute
   end

spec/services/feature_flags/update_service_spec.rb

@@ -130,7 +130,7 @@ RSpec.describe FeatureFlags::UpdateService do
 
       it 'executes hooks' do
         hook = create(:project_hook, :all_events_enabled, project: project)
-        expect(WebHookWorker).to receive(:perform_async).with(hook.id, an_instance_of(Hash), 'feature_flag_hooks')
+        expect(WebHookWorker).to receive(:perform_async).with(hook.id, an_instance_of(Hash), 'feature_flag_hooks', an_instance_of(Hash))
 
         subject
       end

spec/services/web_hook_service_spec.rb

@@ -404,7 +404,7 @@ RSpec.describe WebHookService, :request_store, :clean_gitlab_redis_shared_state
 
   describe '#async_execute' do
     def expect_to_perform_worker(hook)
-      expect(WebHookWorker).to receive(:perform_async).with(hook.id, data, 'push_hooks')
+      expect(WebHookWorker).to receive(:perform_async).with(hook.id, data, 'push_hooks', an_instance_of(Hash))
     end
 
     def expect_to_rate_limit(hook, threshold:, throttled: false)

spec/workers/web_hook_worker_spec.rb

@@ -19,7 +19,16 @@ RSpec.describe WebHookWorker do
       expect { subject.perform(non_existing_record_id, data, hook_name) }.not_to raise_error
     end
 
-    it 'retrieves recursion detection data, reinstates it, and cleans it from payload', :request_store, :aggregate_failures do
+    it 'retrieves recursion detection data and reinstates it', :request_store, :aggregate_failures do
+      uuid = SecureRandom.uuid
+      params = { recursion_detection_request_uuid: uuid }
+
+      expect_next(WebHookService, project_hook, data.with_indifferent_access, hook_name, anything).to receive(:execute)
+      expect { subject.perform(project_hook.id, data, hook_name, params) }
+        .to change { Gitlab::WebHooks::RecursionDetection::UUID.instance.request_uuid }.to(uuid)
+    end
+
+    it 'retrieves recursion detection data, reinstates it, and cleans it from payload when passed through as data', :request_store, :aggregate_failures do
       uuid = SecureRandom.uuid
       full_data = data.merge({ _gitlab_recursion_detection_request_uuid: uuid })