Composite cv chaining

CLAUDE.md

@@ -269,6 +269,12 @@ Don't write unnecessary comments in tests. When writing a new test, look at surr
 - test length, where possible
 - length and quantity of comments (don't be too wordy)
 
+### Code Documentation
+
+Avoid YARD-style documentation (`@param`, `@return`) in Ruby code. Use single-line comments describing what the method does, not how.
+
+Example: `# Returns :scheduled, :running, or nil based on task status`
+
 ### Test Commands Reference
 
 **CRITICAL: Never use `bundle exec rake test TEST=...` for individual tests. Always use `ktest`.**

app/lib/actions/katello/content_view/publish.rb

@@ -37,13 +37,15 @@ def plan(content_view, description = "", options = {importing: false, syncable:
           version = version_for_publish(content_view, options)
           self.version = version
           library = content_view.organization.library
+          triggered_by_id = options[:triggered_by_id] ||
+                            (options[:triggered_by].is_a?(Integer) ? options[:triggered_by] : options[:triggered_by]&.id)
           history = ::Katello::ContentViewHistory.create!(:content_view_version => version,
                                                           :user => ::User.current.login,
                                                           :status => ::Katello::ContentViewHistory::IN_PROGRESS,
                                                           :action => ::Katello::ContentViewHistory.actions[:publish],
                                                           :task => self.task,
                                                           :notes => description,
-                                                          :triggered_by => options[:triggered_by]
+                                                          :triggered_by_id => triggered_by_id
                                                          )
           source_repositories = []
           content_view.publish_repositories(options[:override_components]) do |repositories|
@@ -113,7 +115,9 @@ def humanized_name
 
         def run
           version = ::Katello::ContentViewVersion.find(input[:content_view_version_id])
-          version.auto_publish_composites!
+          # Pass the current execution plan ID so auto_publish can coordinate
+          # with other component CV publishes using Dynflow chaining
+          version.auto_publish_composites!(execution_plan_id)
 
           output[:content_view_id] = input[:content_view_id]
           output[:content_view_version_id] = input[:content_view_version_id]

app/lib/actions/katello/content_view_version/incremental_update.rb

@@ -275,7 +275,7 @@ def finalize
           end
 
           if version.latest? && !version.content_view.composite?
-            version.auto_publish_composites!
+            version.auto_publish_composites!(task.external_id)
           end
         end
 

app/models/katello/content_view_version.rb

@@ -361,18 +361,137 @@ def update_content_counts!
       save!
     end
 
-    def auto_publish_composites!
-      metadata = {
-        description: _("Auto Publish - Triggered by '%s'") % self.name,
-        triggered_by: self.id,
-      }
+    def auto_publish_composites!(component_task_id)
+      description = _("Auto Publish - Triggered by '%s'") % self.name
+
       self.content_view.auto_publish_components.pluck(:composite_content_view_id).each do |composite_id|
-        ::Katello::EventQueue.push_event(::Katello::Events::AutoPublishCompositeView::EVENT_TYPE, composite_id) do |attrs|
-          attrs[:metadata] = metadata
+        composite_cv = ::Katello::ContentView.find(composite_id)
+
+        composite_cv.with_lock do
+          status = composite_publish_status(composite_cv)
+
+          case status
+          when :scheduled
+            Rails.logger.info("Composite CV #{composite_cv.name} publish already scheduled, skipping duplicate")
+            next
+          when :running, nil
+            # Either composite is running or no composite activity detected
+            # Schedule event to trigger composite publish with proper coordination
+            Rails.logger.info("Composite CV #{composite_cv.name} scheduling auto-publish event")
+            schedule_auto_publish_event(composite_cv, description, component_task_id)
+            next
+          end
+        end
+      end
+    end
+
+    class << self
+      # Trigger a composite publish with coordination for sibling tasks.
+      # Checks for running component CV publishes and chains if necessary.
+      def trigger_composite_publish_with_coordination(composite_cv, description, triggered_by_version_id, calling_task_id: nil)
+        # Find currently running component CV publish tasks
+        component_cv_ids = composite_cv.components.pluck(:content_view_id)
+        running_tasks = ForemanTasks::Task::DynflowTask
+          .for_action(::Actions::Katello::ContentView::Publish)
+          .where(state: ['planning', 'planned', 'running'])
+          .select do |task|
+            task_input = task.input
+            task_input && component_cv_ids.include?(task_input.dig('content_view', 'id'))
+          end
+
+        sibling_task_ids = running_tasks.map(&:external_id)
+        # Exclude the calling component task to avoid self-dependency
+        sibling_task_ids.reject! { |id| id == calling_task_id } if calling_task_id
+
+        trigger_publish_with_sibling_tasks(composite_cv, sibling_task_ids, description, triggered_by_version_id)
+      rescue ForemanTasks::Lock::LockConflict => e
+        Rails.logger.info("Composite CV #{composite_cv.name} publish lock conflict: #{e.class} - #{e.message}")
+        ::Katello::UINotifications::ContentView::AutoPublishFailure.deliver!(composite_cv)
+        raise
+      rescue StandardError => e
+        Rails.logger.error("Failed to auto-publish composite CV #{composite_cv.name}: #{e.class} - #{e.message}")
+        Rails.logger.debug(e.backtrace.join("\n")) if e.backtrace
+        ::Katello::UINotifications::ContentView::AutoPublishFailure.deliver!(composite_cv)
+        raise
+      end
+
+      # Trigger a composite publish, chaining to sibling tasks if any exist
+      def trigger_publish_with_sibling_tasks(composite_cv, sibling_task_ids, description, triggered_by_version_id)
+        if sibling_task_ids.any?
+          ForemanTasks.dynflow.world.chain(
+            sibling_task_ids,
+            ::Actions::Katello::ContentView::Publish,
+            composite_cv,
+            description,
+            triggered_by_id: triggered_by_version_id
+          )
+        else
+          ForemanTasks.async_task(
+            ::Actions::Katello::ContentView::Publish,
+            composite_cv,
+            description,
+            triggered_by_id: triggered_by_version_id
+          )
         end
       end
     end
 
+    private
+
+    # Returns :scheduled, :running, or nil based on composite CV publish task status
+    def composite_publish_status(composite_cv)
+      # Check scheduled tasks first (they don't have input populated yet)
+      scheduled_tasks = ForemanTasks::Task::DynflowTask
+        .for_action(::Actions::Katello::ContentView::Publish)
+        .where(state: 'scheduled')
+
+      if scheduled_tasks.any? { |task| scheduled_task_for_composite?(task, composite_cv) }
+        return :scheduled
+      end
+
+      # Check running tasks (these have input populated)
+      if find_active_composite_publish_tasks(composite_cv).any?
+        return :running
+      end
+
+      nil
+    end
+
+    # Schedule an event to retry composite publish after current one finishes
+    def schedule_auto_publish_event(composite_cv, description, component_task_id)
+      ::Katello::EventQueue.push_event(::Katello::Events::AutoPublishCompositeView::EVENT_TYPE, composite_cv.id) do |attrs|
+        attrs[:metadata] = { description: description, version_id: self.id, calling_task_id: component_task_id }
+      end
+    end
+
+    # Check if a scheduled task is for the given composite CV by inspecting delayed plan args
+    def scheduled_task_for_composite?(task, composite_cv)
+      delayed_plan = ForemanTasks.dynflow.world.persistence.load_delayed_plan(task.external_id)
+      return false if delayed_plan.nil?
+
+      args = delayed_plan.args
+      args.first.is_a?(::Katello::ContentView) && args.first.id == composite_cv.id
+    rescue NoMethodError, TypeError, Dynflow::Error => e
+      Rails.logger.error("Failed to check scheduled task for composite CV #{composite_cv.name}: #{e.message}")
+      false
+    end
+
+    # Find active (planning/planned/running) composite publish tasks (does NOT check scheduled tasks)
+    def find_active_composite_publish_tasks(composite_cv)
+      relevant_tasks = ForemanTasks::Task::DynflowTask
+        .for_action(::Actions::Katello::ContentView::Publish)
+        .where(state: ['planning', 'planned', 'running'])
+        .select do |task|
+          # Check if task is publishing the composite CV
+          task_input = task.input
+          task_input && task_input.dig('content_view', 'id') == composite_cv.id
+        end
+
+      relevant_tasks.map(&:external_id)
+    end
+
+    public
+
     def repository_type_counts_map
       counts = {}
       Katello::RepositoryTypeManager.enabled_repository_types.keys.each do |repo_type|

app/models/katello/events/auto_publish_composite_view.rb

@@ -1,5 +1,10 @@
 module Katello
   module Events
+    # Event handler for retrying composite content view auto-publish when a lock conflict occurs.
+    # This is used in conjunction with Dynflow chaining:
+    # - Dynflow chaining coordinates sibling component CV publishes to avoid race conditions
+    # - Event-based retry handles the case when a composite CV publish is already running
+    # See: ContentViewVersion#auto_publish_composites!
     class AutoPublishCompositeView
       EVENT_TYPE = 'auto_publish_composite_view'.freeze
 
@@ -20,10 +25,14 @@ def run
         return unless composite_view
 
         begin
-          ForemanTasks.async_task(::Actions::Katello::ContentView::Publish,
-                              composite_view,
-                              metadata[:description],
-                              triggered_by: metadata[:version_id])
+          # Use the same coordination logic as auto_publish_composites! to check for
+          # running component tasks and chain if necessary
+          ::Katello::ContentViewVersion.trigger_composite_publish_with_coordination(
+            composite_view,
+            metadata[:description],
+            metadata[:version_id],
+            calling_task_id: metadata[:calling_task_id]
+          )
         rescue => e
           self.retry = true if e.is_a?(ForemanTasks::Lock::LockConflict)
           deliver_failure_notification

test/actions/katello/content_view_test.rb

@@ -249,7 +249,7 @@ class PublishTest < TestBase
     end
 
     context 'run phase' do
-      it 'creates auto-publish events for non-composite views' do
+      it 'schedules event for composite views' do
         composite_view = katello_content_views(:composite_view)
         action.stubs(:task).returns(success_task)
 
@@ -258,23 +258,29 @@ class PublishTest < TestBase
                           composite_content_view: composite_view,
                           content_view: content_view)
 
-        plan_action action, content_view
-        run_action action
+        # Mock the task relations to simulate no scheduled composite
+        ForemanTasks::Task::DynflowTask.stubs(:for_action)
+          .returns(stub(where: stub(any?: false))) # Scheduled check: no scheduled tasks
+          .then.returns(stub(where: stub(select: []))) # Running composite check: none
 
-        event = Katello::Event.find_by(event_type: Katello::Events::AutoPublishCompositeView::EVENT_TYPE, object_id: composite_view.id)
-        version = content_view.versions.last
+        # Expect event to be scheduled
+        ::Katello::EventQueue.expects(:push_event).with(
+          ::Katello::Events::AutoPublishCompositeView::EVENT_TYPE,
+          composite_view.id
+        )
 
-        assert_equal event.metadata[:triggered_by], version.id
-        assert_equal event.metadata[:description], "Auto Publish - Triggered by '#{version.name}'"
+        plan_action action, content_view
+        run_action action
       end
 
       it 'does nothing for non-composite view' do
         action.stubs(:task).returns(success_task)
 
+        # Should not trigger any auto-publish events
+        ::Katello::EventQueue.expects(:push_event).never
+
         plan_action action, katello_content_views(:no_environment_view)
         run_action action
-
-        assert_empty Katello::Event.all
       end
     end
 

test/models/content_view_version_auto_publish_test.rb

@@ -0,0 +1,119 @@
+require 'katello_test_helper'
+
+module Katello
+  class ContentViewVersionAutoPublishTest < ActiveSupport::TestCase
+    def setup
+      User.current = User.find(users(:admin).id)
+      @org = FactoryBot.create(:katello_organization)
+
+      # Create two component content views
+      @component_cv1 = FactoryBot.create(:katello_content_view, :organization => @org, :name => "Component CV 1")
+      @component_cv2 = FactoryBot.create(:katello_content_view, :organization => @org, :name => "Component CV 2")
+
+      # Create a composite content view with auto-publish enabled
+      @composite_cv = FactoryBot.create(:katello_content_view,
+                                         :organization => @org,
+                                         :composite => true,
+                                         :auto_publish => true,
+                                         :name => "Composite CV")
+
+      # Add components to composite
+      @component1_version = FactoryBot.create(:katello_content_view_version,
+                                               :content_view => @component_cv1,
+                                               :major => 1,
+                                               :minor => 0)
+      @component2_version = FactoryBot.create(:katello_content_view_version,
+                                               :content_view => @component_cv2,
+                                               :major => 1,
+                                               :minor => 0)
+
+      # For latest: true, set content_view (not content_view_version)
+      # Validation requires: either (latest=true + content_view) OR (content_view_version)
+      FactoryBot.create(:katello_content_view_component,
+                        :composite_content_view => @composite_cv,
+                        :content_view => @component_cv1,
+                        :latest => true)
+      FactoryBot.create(:katello_content_view_component,
+                        :composite_content_view => @composite_cv,
+                        :content_view => @component_cv2,
+                        :latest => true)
+    end
+
+    def test_auto_publish_schedules_event_when_no_composite_activity
+      task_id = SecureRandom.uuid
+
+      # Stub to return no scheduled, no running composite
+      ForemanTasks::Task::DynflowTask.stubs(:for_action)
+        .returns(stub(where: stub(any?: false))) # Scheduled check: no scheduled tasks
+        .then.returns(stub(where: stub(select: []))) # Running composite check: none
+
+      ::Katello::EventQueue.expects(:push_event).with(
+        ::Katello::Events::AutoPublishCompositeView::EVENT_TYPE,
+        @composite_cv.id
+      )
+
+      @component1_version.auto_publish_composites!(task_id)
+    end
+
+    def test_auto_publish_schedules_event_when_composite_running
+      task_id = SecureRandom.uuid
+      running_task = stub(external_id: SecureRandom.uuid, input: { 'content_view' => { 'id' => @composite_cv.id } })
+
+      # Stub to return no scheduled but a running composite
+      ForemanTasks::Task::DynflowTask.stubs(:for_action)
+        .returns(stub(where: stub(any?: false))) # Scheduled check: no scheduled tasks
+        .then.returns(stub(where: stub(select: [running_task]))) # Running composite check: found running
+
+      ::Katello::EventQueue.expects(:push_event).with(
+        ::Katello::Events::AutoPublishCompositeView::EVENT_TYPE,
+        @composite_cv.id
+      )
+
+      @component1_version.auto_publish_composites!(task_id)
+    end
+
+    def test_auto_publish_skips_when_composite_already_scheduled
+      task_id = SecureRandom.uuid
+      composite_task_id = SecureRandom.uuid
+
+      # Create mock scheduled composite publish task with delayed plan args
+      composite_task = stub(external_id: composite_task_id)
+      delayed_plan = stub(args: [@composite_cv, "description", {}])
+
+      # Mock the delayed plan lookup - need to allow the real dynflow world through
+      # but intercept the persistence.load_delayed_plan call
+      world_stub = ForemanTasks.dynflow.world
+      persistence_stub = stub(load_delayed_plan: delayed_plan)
+      world_stub.stubs(:persistence).returns(persistence_stub)
+
+      # Stub scheduled check to return the composite task
+      scheduled_relation = mock
+      scheduled_relation.expects(:any?).yields(composite_task).returns(true)
+
+      ForemanTasks::Task::DynflowTask.stubs(:for_action)
+        .returns(stub(where: scheduled_relation))
+
+      # Should not schedule event when already scheduled
+      ::Katello::EventQueue.expects(:push_event).never
+
+      @component1_version.auto_publish_composites!(task_id)
+    end
+
+    def test_scheduled_task_for_composite_handles_errors_gracefully
+      task_id = SecureRandom.uuid
+      composite_task = stub(external_id: task_id)
+
+      # Mock dynflow world to raise an error when loading delayed plan
+      world_stub = ForemanTasks.dynflow.world
+      persistence_stub = stub
+      persistence_stub.stubs(:load_delayed_plan).raises(Dynflow::Error, "Delayed plan not found")
+      world_stub.stubs(:persistence).returns(persistence_stub)
+
+      # Should log error and return false instead of raising
+      Rails.logger.expects(:error).with(regexp_matches(/Failed to check scheduled task/))
+
+      result = @component1_version.send(:scheduled_task_for_composite?, composite_task, @composite_cv)
+      refute result
+    end
+  end
+end