Verify episode publishing state (#1377)

* Guard for remote publishing state

* Refactor

* Fixup return

* Fixup coment

* Remove dupe logging

* Fixup merge

Author: svevang

app/models/apple/publisher.rb

@@ -397,11 +397,47 @@ def mark_as_uploaded!(eps)
       end
     end
 
+    def verify_publishing_state!(eps)
+      # Capture initial state before polling to detect drift
+      initial_states = eps.to_h { |ep| [ep.feeder_id, ep.publishing_state] }
+
+      poll_episodes!(eps)
+
+      drifted_count = eps.count do |ep|
+        initial_state = initial_states[ep.feeder_id]
+        (ep.publishing_state != initial_state).tap do |drifted|
+          if drifted
+            Rails.logger.warn("Episode publishing state found to be out of sync", {
+              episode_id: ep.feeder_id,
+              local_expected_state: initial_state,
+              remote_actual_state: ep.publishing_state
+            })
+          end
+        end
+      end
+
+      if drifted_count.positive?
+        raise Apple::RetryPublishingError.new("Detected #{drifted_count} episodes with publishing state drift")
+      end
+
+      true
+    end
+
     def publish_drafting!(eps)
       Rails.logger.tagged("##{__method__}") do
-        eps = eps.select { |ep| ep.drafting? && ep.container_upload_complete? }
+        # Guard: verify all episodes are in a consistent local and remote state
+        verify_publishing_state!(eps)
+
+        drafting_eps, non_ready_eps = eps.partition { |ep| ep.drafting? && ep.container_upload_complete? }
+        non_ready_eps.each do |ep|
+          Rails.logger.info("Skipping publish for non-ready episode", {episode_id: ep.feeder_id,
+                                                                     publishing_state: ep.publishing_state,
+                                                                     container_upload_complete: ep.container_upload_complete?})
+        end
+
+        Rails.logger.info("Publishing #{drafting_eps.length} drafting episodes.", {drafting_episode_ids: drafting_eps.map(&:feeder_id)}) if drafting_eps.any?
 
-        res = Apple::Episode.publish(api, show, eps)
+        res = Apple::Episode.publish(api, show, drafting_eps)
 
         Rails.logger.info("Published #{res.length} drafting episodes.")
       end

test/models/apple/publisher_test.rb

@@ -398,6 +398,57 @@
     end
   end
 
+  describe "#verify_publishing_state!" do
+    let(:episode1) { build(:uploaded_apple_episode, show: apple_publisher.show, api_response: build(:apple_episode_api_response, publishing_state: "DRAFTING")) }
+    let(:episode2) { build(:uploaded_apple_episode, show: apple_publisher.show, api_response: build(:apple_episode_api_response, publishing_state: "DRAFTING")) }
+    let(:episodes) { [episode1, episode2] }
+
+    it "should not raise error when episodes remain in same state" do
+      apple_publisher.stub(:poll_episodes!, nil) do
+        result = apple_publisher.verify_publishing_state!(episodes)
+        assert result == true
+      end
+    end
+
+    it "should raise RetryPublishingError when state drift is detected" do
+      # Simulate state change during poll - episode1 starts DRAFTING but becomes PUBLISHED after poll
+      apple_publisher.stub(:poll_episodes!, proc {
+        episode1.api_response["api_response"]["val"]["data"]["attributes"]["publishingState"] = "PUBLISHED"
+      }) do
+        error = assert_raises(Apple::RetryPublishingError) do
+          apple_publisher.verify_publishing_state!(episodes)
+        end
+        assert_match(/Detected 1 episodes with publishing state drift/, error.message)
+      end
+    end
+
+    it "should detect drift and raise error even with non-DRAFTING episodes" do
+      # episode1 starts in PUBLISHED state, then drifts to ARCHIVED
+      episode1.api_response["api_response"]["val"]["data"]["attributes"]["publishingState"] = "PUBLISHED"
+
+      apple_publisher.stub(:poll_episodes!, proc {
+        episode1.api_response["api_response"]["val"]["data"]["attributes"]["publishingState"] = "ARCHIVED"
+      }) do
+        error = assert_raises(Apple::RetryPublishingError) do
+          apple_publisher.verify_publishing_state!(episodes)
+        end
+        assert_match(/Detected 1 episodes with publishing state drift/, error.message)
+      end
+    end
+
+    it "should raise error with count when multiple episodes drift" do
+      apple_publisher.stub(:poll_episodes!, proc {
+        episode1.api_response["api_response"]["val"]["data"]["attributes"]["publishingState"] = "PUBLISHED"
+        episode2.api_response["api_response"]["val"]["data"]["attributes"]["publishingState"] = "ARCHIVED"
+      }) do
+        error = assert_raises(Apple::RetryPublishingError) do
+          apple_publisher.verify_publishing_state!(episodes)
+        end
+        assert_match(/Detected 2 episodes with publishing state drift/, error.message)
+      end
+    end
+  end
+
   describe "#publish_drafting!" do
     let(:episode1) { build(:uploaded_apple_episode, show: apple_publisher.show, api_response: build(:apple_episode_api_response, publishing_state: "DRAFTING")) }
     let(:episode2) { build(:uploaded_apple_episode, show: apple_publisher.show, api_response: build(:apple_episode_api_response, publishing_state: "DRAFTING")) }
@@ -407,8 +458,10 @@
       mock = Minitest::Mock.new
       mock.expect(:call, [], [Apple::Api, Apple::Show, episodes])
 
-      Apple::Episode.stub(:publish, mock) do
-        apple_publisher.publish_drafting!(episodes)
+      apple_publisher.stub(:verify_publishing_state!, nil) do
+        Apple::Episode.stub(:publish, mock) do
+          apple_publisher.publish_drafting!(episodes)
+        end
       end
 
       assert mock.verify
@@ -656,7 +709,9 @@
 
       apple_publisher.stub(:upload_media!, upload_mock) do
         apple_publisher.stub(:process_delivery!, ->(*) {}) do
-          apple_publisher.upload_and_process!([episode])
+          apple_publisher.stub(:verify_publishing_state!, nil) do
+            apple_publisher.upload_and_process!([episode])
+          end
         end
       end
 
@@ -670,7 +725,9 @@
       mock.expect(:call, nil, [[episode]])
       apple_publisher.stub(:upload_media!, mock) do
         apple_publisher.stub(:process_delivery!, ->(*) {}) do
-          apple_publisher.upload_and_process!([episode])
+          apple_publisher.stub(:verify_publishing_state!, nil) do
+            apple_publisher.upload_and_process!([episode])
+          end
         end
       end
 
@@ -815,9 +872,11 @@
       apple_publisher.stub(:increment_asset_wait!, increment_mock) do
         apple_publisher.stub(:wait_for_upload_processing, wait_upload_mock) do
           apple_publisher.stub(:wait_for_asset_state, wait_for_asset_state_stub) do
-            Apple::Episode.stub(:probe_asset_state, probe_mock) do
-              apple_publisher.stub(:mark_as_delivered!, mark_delivered_mock) do
-                apple_publisher.process_delivery!([episode])
+            apple_publisher.stub(:verify_publishing_state!, ->(*) {}) do
+              Apple::Episode.stub(:probe_asset_state, probe_mock) do
+                apple_publisher.stub(:mark_as_delivered!, mark_delivered_mock) do
+                  apple_publisher.process_delivery!([episode])
+                end
               end
             end
           end
@@ -896,10 +955,12 @@
         apple_publisher.stub(:wait_for_upload_processing, ->(*) {}) do
           apple_publisher.stub(:wait_for_asset_state, wait_for_asset_state_stub) do
             apple_publisher.stub(:check_for_stuck_episodes, ->(*) {}) do
-              Apple::Episode.stub(:probe_asset_state, probe_mock) do
-                apple_publisher.stub(:mark_as_delivered!, ->(*) {}) do
-                  apple_publisher.stub(:log_asset_wait_duration!, ->(*) {}) do
-                    apple_publisher.process_delivery!([episode])
+              apple_publisher.stub(:verify_publishing_state!, ->(*) {}) do
+                Apple::Episode.stub(:probe_asset_state, probe_mock) do
+                  apple_publisher.stub(:mark_as_delivered!, ->(*) {}) do
+                    apple_publisher.stub(:log_asset_wait_duration!, ->(*) {}) do
+                      apple_publisher.process_delivery!([episode])
+                    end
                   end
                 end
               end
@@ -938,10 +999,12 @@
       apple_publisher.stub(:increment_asset_wait!, ->(*) {}) do
         apple_publisher.stub(:wait_for_upload_processing, ->(*) {}) do
           apple_publisher.stub(:wait_for_asset_state, wait_for_asset_state_stub) do
-            Apple::Episode.stub(:probe_asset_state, probe_mock) do
-              apple_publisher.stub(:mark_as_delivered!, ->(*) {}) do
-                apple_publisher.stub(:log_asset_wait_duration!, ->(*) {}) do
-                  apple_publisher.process_delivery!(episodes)
+            apple_publisher.stub(:verify_publishing_state!, ->(*) {}) do
+              Apple::Episode.stub(:probe_asset_state, probe_mock) do
+                apple_publisher.stub(:mark_as_delivered!, ->(*) {}) do
+                  apple_publisher.stub(:log_asset_wait_duration!, ->(*) {}) do
+                    apple_publisher.process_delivery!(episodes)
+                  end
                 end
               end
             end
@@ -996,9 +1059,11 @@
         apple_publisher.stub(:wait_for_upload_processing, ->(*) {}) do
           apple_publisher.stub(:wait_for_asset_state, wait_for_asset_state_stub) do
             apple_publisher.stub(:check_for_stuck_episodes, ->(*) {}) do
-              Apple::Episode.stub(:probe_asset_state, probe_mock) do
-                apple_publisher.stub(:mark_as_delivered!, mark_delivered_mock) do
-                  apple_publisher.process_delivery!([episode1, episode2, episode3])
+              apple_publisher.stub(:verify_publishing_state!, ->(*) {}) do
+                Apple::Episode.stub(:probe_asset_state, probe_mock) do
+                  apple_publisher.stub(:mark_as_delivered!, mark_delivered_mock) do
+                    apple_publisher.process_delivery!([episode1, episode2, episode3])
+                  end
                 end
               end
             end
@@ -1047,15 +1112,17 @@
             apple_publisher.stub(:wait_for_upload_processing, ->(*) {}) do
               apple_publisher.stub(:wait_for_asset_state, wait_for_asset_state_stub) do
                 apple_publisher.stub(:check_for_stuck_episodes, ->(*) {}) do
-                  Apple::Episode.stub(:probe_asset_state, probe_mock) do
-                    apple_publisher.stub(:mark_as_delivered!, mark_delivered_mock) do
-                      apple_publisher.stub(:log_asset_wait_duration!, ->(*) {}) do
-                        # Should raise timeout error when timeout is reached
-                        error = assert_raises(Apple::AssetStateTimeoutError) do
-                          apple_publisher.process_delivery!([episode1, episode2])
+                  apple_publisher.stub(:verify_publishing_state!, ->(*) {}) do
+                    Apple::Episode.stub(:probe_asset_state, probe_mock) do
+                      apple_publisher.stub(:mark_as_delivered!, mark_delivered_mock) do
+                        apple_publisher.stub(:log_asset_wait_duration!, ->(*) {}) do
+                          # Should raise timeout error when timeout is reached
+                          error = assert_raises(Apple::AssetStateTimeoutError) do
+                            apple_publisher.process_delivery!([episode1, episode2])
+                          end
+
+                          assert_equal 2, error.episodes.length, "Error should contain both episodes"
                         end
-
-                        assert_equal 2, error.episodes.length, "Error should contain both episodes"
                       end
                     end
                   end