Merge remote-tracking branches 'origin/apple-config-tracks-apple-data…

…', 'origin/increase-audio-asset-state-wait-timeout', 'origin/show-skips-updates' and 'origin/fix-guard-for-podcast-container'

app/models/apple/api_waiting.rb

@@ -1,24 +1,49 @@
+API_WAIT_INTERVAL = 2.seconds
+API_WAIT_TIMEOUT = 5.minutes
+
 module Apple
   module ApiWaiting
-    API_WAIT_INTERVAL = 2.seconds
-    API_WAIT_TIMEOUT = 5.minutes
-
     extend ActiveSupport::Concern
     included do
-      def self.wait_for(remaining_records)
-        t_beg = Time.now.utc
-        loop do
-          Rails.logger.info(".wait_for", {remaining_record_count: remaining_records.count, have_waited: Time.now.utc - t_beg})
+      def self.current_time
+        Time.now.utc
+      end
 
-          # Return `timeout == true` if we've waited too long
-          break [true, remaining_records] if Time.now.utc - t_beg > self::API_WAIT_TIMEOUT
+      def self.wait_timed_out?(waited, wait_timeout)
+        if waited > wait_timeout
+          Rails.logger.info("Timed out waiting for Apple API to process", waited: waited, wait_timeout: wait_timeout)
+          true
+        else
+          false
+        end
+      end
 
-          remaining_records = yield(remaining_records)
+      def self.work_done?(remaining_records, waited, wait_timeout)
+        if remaining_records.empty?
+          Rails.logger.info("Done waiting for Apple Api work", waited: waited, wait_timeout: wait_timeout)
+          true
+        else
+          false
+        end
+      end
+
+      def self.wait_for(remaining_records, wait_timeout: API_WAIT_TIMEOUT, wait_interval: API_WAIT_INTERVAL)
+        t_beg = current_time
 
+        waited = 0
+        loop do
           # All done, return `timeout == false`
-          break [false, []] if remaining_records.empty?
+          break [false, []] if work_done?(remaining_records, waited, wait_timeout)
+          # Return `timeout == true` if we've waited too long
 
-          sleep(self::API_WAIT_INTERVAL)
+          break [true, remaining_records] if wait_timed_out?(waited, wait_timeout)
+
+          sleep(wait_interval)
+
+          waited = current_time - t_beg
+          Rails.logger.info(".wait_for", {remaining_record_count: remaining_records.count, have_waited: waited})
+
+          remaining_records = yield(remaining_records)
         end
       end
     end

app/models/apple/config.rb

@@ -40,5 +40,21 @@ def build_publisher
     def apple_key
       Base64.decode64(apple_key_pem_b64)
     end
+
+    def apple_data
+      episode_data =
+        ::Episode.where(podcast: podcast).map do |episode|
+          [episode.apple_sync_log, episode.podcast_container]
+        end.flatten.compact
+
+      podcast_delivery_data = [
+        Apple::PodcastDelivery.with_deleted.where(episode: podcast.episodes),
+        Apple::PodcastDeliveryFile.with_deleted.where(episode: podcast.episodes)
+      ]
+
+      feed_data = [public_feed.apple_sync_log, private_feed.apple_sync_log].compact
+
+      [podcast_delivery_data, episode_data, feed_data]
+    end
   end
 end

app/models/apple/episode.rb

@@ -12,11 +12,14 @@ class Episode
     AUDIO_ASSET_FAILURE = "FAILURE"
     AUDIO_ASSET_SUCCESS = "SUCCESS"
 
+    EPISODE_ASSET_WAIT_TIMEOUT = 8.minutes.freeze
+    EPISODE_ASSET_WAIT_INTERVAL = 10.seconds.freeze
+
     # In the case where the episodes state is not yet ready to publish, but the
     # underlying models are ready. Poll the episodes audio asset state but
     # guard against waiting for episode assets that will never be processed.
     def self.wait_for_asset_state(api, eps)
-      wait_for(eps) do |remaining_eps|
+      wait_for(eps, wait_timeout: EPISODE_ASSET_WAIT_TIMEOUT, wait_interval: EPISODE_ASSET_WAIT_INTERVAL) do |remaining_eps|
         Rails.logger.info("Probing for episode audio asset state")
         unwrapped = get_episodes(api, remaining_eps)
 
@@ -386,7 +389,9 @@ def drafting?
     end
 
     def container_upload_complete?
-      feeder_episode.apple_podcast_container.container_upload_satisfied?
+      return false if missing_container?
+
+      podcast_container.container_upload_satisfied?
     end
 
     def audio_asset_vendor_id

app/models/apple/podcast_container.rb

@@ -8,7 +8,7 @@ class PodcastContainer < ApplicationRecord
 
     default_scope { includes(:apple_sync_log) }
 
-    has_one :apple_sync_log, -> { podcast_containers }, foreign_key: :feeder_id, class_name: "SyncLog"
+    has_one :apple_sync_log, -> { podcast_containers }, foreign_key: :feeder_id, class_name: "SyncLog", dependent: :destroy
     has_many :podcast_deliveries, dependent: :destroy
     has_many :podcast_delivery_files, through: :podcast_deliveries
     belongs_to :episode, class_name: "::Episode"
@@ -249,7 +249,8 @@ def missing_podcast_audio?
     end
 
     def delivered?
-      return false if podcast_delivery_files.length == 0
+      # because we cannot infer if the podcast delivery files have expired
+      return true if podcast_delivery_files.length == 0
 
       (podcast_delivery_files.all?(&:delivered?) &&
         podcast_delivery_files.all?(&:processed?))
@@ -262,15 +263,9 @@ def processed_errors?
     end
 
     def delivery_settled?
-      return false if podcast_delivery_files.length == 0
-
       delivered? && !processed_errors?
     end
 
-    def skip_delivery?
-      container_upload_satisfied?
-    end
-
     def container_upload_satisfied?
       # Sets us up for a retry if something prevented the audio from being
       # marked as uploaded and then processed and validated. Assuming that we
@@ -279,6 +274,10 @@ def container_upload_satisfied?
       has_podcast_audio? && delivery_settled?
     end
 
+    def skip_delivery?
+      container_upload_satisfied?
+    end
+
     def needs_delivery?
       !skip_delivery?
     end

app/models/apple/podcast_delivery.rb

@@ -8,7 +8,7 @@ class PodcastDelivery < ApplicationRecord
 
     default_scope { includes(:apple_sync_log) }
 
-    has_one :apple_sync_log, -> { podcast_deliveries }, foreign_key: :feeder_id, class_name: "SyncLog"
+    has_one :apple_sync_log, -> { podcast_deliveries }, foreign_key: :feeder_id, class_name: "SyncLog", dependent: :destroy
     has_many :podcast_delivery_files, dependent: :destroy
     belongs_to :episode, class_name: "::Episode"
     belongs_to :podcast_container, class_name: "::Apple::PodcastContainer"

app/models/apple/podcast_delivery_file.rb

@@ -9,7 +9,7 @@ class PodcastDeliveryFile < ApplicationRecord
 
     default_scope { includes(:apple_sync_log) }
 
-    has_one :apple_sync_log, -> { podcast_delivery_files }, foreign_key: :feeder_id, class_name: "SyncLog", autosave: true
+    has_one :apple_sync_log, -> { podcast_delivery_files }, foreign_key: :feeder_id, class_name: "SyncLog", autosave: true, dependent: :delete
     belongs_to :podcast_delivery
     has_one :podcast_container, through: :podcast_delivery
     belongs_to :episode, class_name: "::Episode"

app/models/apple/show.rb

@@ -63,21 +63,32 @@ def category_data
       [{"type" => "categories", "id" => "1301"}]
     end
 
-    def show_data
+    def update_attributes
+      create_attributes.except(:releaseFrequency)
+    end
+
+    def create_attributes
       {
-        data: {
-          type: "shows",
-          relationships: {
-            allowedCountriesAndRegions: {data: api.countries_and_regions}
-          },
-          attributes: {
-            kind: "RSS",
-            rssUrl: feed_published_url,
-            releaseFrequency: "OPTOUT",
-            thirdPartyRights: "HAS_RIGHTS_TO_THIRD_PARTY_CONTENT"
+        kind: "RSS",
+        rssUrl: feed_published_url,
+        releaseFrequency: "WEEKLY",
+        thirdPartyRights: "HAS_RIGHTS_TO_THIRD_PARTY_CONTENT"
+      }
+    end
+
+    def show_data(attributes, id: nil)
+      res =
+        {
+          data: {
+            type: "shows",
+            relationships: {
+              allowedCountriesAndRegions: {data: api.countries_and_regions}
+            },
+            attributes: attributes
           }
         }
-      }
+      res[:data][:id] = id if id.present?
+      res
     end
 
     def apple_id
@@ -106,19 +117,23 @@ def sync!
     end
 
     def create_show!
-      data = show_data
+      data = show_data(create_attributes)
       Rails.logger.info("Creating show", show_data: data)
       resp = api.post("shows", data)
 
       api.response(resp)
     end
 
     def update_show!(sync)
-      show_data_with_id = show_data
-      show_data_with_id[:data][:id] = sync.external_id
-      Rails.logger.info("Updating show", show_data: show_data_with_id)
-      resp = api.patch("shows/#{sync.external_id}", show_data_with_id)
-
+      Rails.logger.warn("Skipping update for existing show!")
+      # TODO, map out the cases where we'd actually need to update a show
+      # data = show_data(update_attributes, id: apple_id)
+      # Rails.logger.info("Updating show", show_data: data)
+      # resp = api.patch("shows/#{sync.external_id}", data)
+      #
+      # api.response(resp)
+
+      resp = api.get("shows/#{sync.external_id}")
       api.response(resp)
     end
 

test/models/apple/api_waiting_test.rb

@@ -2,15 +2,8 @@
 
 require "test_helper"
 
-class TestWait
+class Test
   include Apple::ApiWaiting
-  API_WAIT_INTERVAL = 0.seconds
-end
-
-class TestTimeout
-  include Apple::ApiWaiting
-  API_WAIT_INTERVAL = 0.seconds
-  API_WAIT_TIMEOUT = 0.seconds
 end
 
 describe Apple::ApiWaiting do
@@ -25,7 +18,7 @@ class TestTimeout
 
       step = 0
 
-      (timed_out, remaining) = TestWait.wait_for(records) do |remaining|
+      (timed_out, remaining) = Test.wait_for(records, wait_interval: 0.seconds) do |remaining|
         rem = remaining.dup
 
         assert_equal intervals[step], rem
@@ -42,12 +35,29 @@ class TestTimeout
     end
 
     it "times out" do
-      (timed_out, remaining) = TestTimeout.wait_for(["a", "b", "c"]) do |remaining|
+      (timed_out, remaining) = Test.wait_for(["a", "b", "c"], wait_interval: 0.seconds, wait_timeout: 0.seconds) do |remaining|
         remaining
       end
 
       assert_equal timed_out, true
       assert_equal remaining, ["a", "b", "c"]
     end
+
+    it "times out by default after 5 minutes" do
+      current_time = Time.utc(2021, 1, 1, 0, 0, 0)
+      index = 0
+
+      Test.stub(:current_time, -> {
+                                 current_time += index.minutes
+                                 index += 1
+                               }) do
+        (timed_out, remaining) = Test.wait_for(["a", "b", "c"], wait_interval: 0.seconds) do |remaining|
+          remaining
+        end
+
+        assert_equal timed_out, true
+        assert_equal remaining, ["a", "b", "c"]
+      end
+    end
   end
 end

test/models/apple/episode_test.rb

@@ -91,9 +91,9 @@
       assert_equal true, apple_episode.waiting_for_asset_state?
     end
 
-    it "should be false if there are no podcast delivery files" do
+    it "should be true if there are no podcast delivery files and the asset state is UNSPECIFIED" do
       apple_episode.podcast_container.stub(:podcast_delivery_files, []) do
-        assert_equal false, apple_episode.waiting_for_asset_state?
+        assert_equal true, apple_episode.waiting_for_asset_state?
       end
     end
 
@@ -147,6 +147,18 @@
         assert_equal false, ep.synced_with_apple?
       end
     end
+
+    it "should be false when the podcast_container is nil" do
+      ep = build(:apple_episode)
+      assert ep.podcast_container.nil?
+
+      # it returns early via the guard
+      ep.stub(:podcast_container, -> { raise "shouldn't happen" }) do
+        ep.stub(:has_container?, false) do
+          refute ep.container_upload_complete?
+        end
+      end
+    end
   end
 
   describe "#enclosure_filename" do

test/models/apple/podcast_container_test.rb

@@ -256,6 +256,31 @@ class Apple::PodcastContainerTest < ActiveSupport::TestCase
     end
   end
 
+  describe "retry sentinals and gatekeeping" do
+    let(:container) { Apple::PodcastContainer.new }
+    describe "#delivery_settled?" do
+      it "should be settled if there are no delivery files" do
+        container.stub(:podcast_delivery_files, []) do
+          assert container.delivery_settled?
+          assert container.delivered?
+        end
+      end
+    end
+
+    describe "#container_upload_satisfied?" do
+      it "should be satisfied with podcast files and a settled delivery" do
+        container.stub(:files, [
+          {status: Apple::PodcastContainer::FILE_STATUS_SUCCESS,
+           assetRole: Apple::PodcastContainer::FILE_ASSET_ROLE_PODCAST_AUDIO}.with_indifferent_access
+        ]) do
+          container.stub(:podcast_delivery_files, []) do
+            assert container.container_upload_satisfied?
+          end
+        end
+      end
+    end
+  end
+
   describe "#destroy" do
     it "should destroy the podcast container and cascade to the delivery and delivery file" do
       apple_episode.stub(:apple_id, apple_episode_id) do

test/models/apple/show_test.rb

@@ -107,7 +107,9 @@
 
   describe "#sync!" do
     it "runs sync!" do
-      apple_show.api.stub(:patch, OpenStruct.new(body: {"data" => {"id" => "123", "attributes" => {"foo" => "bar"}}}.to_json, code: "200")) do
+      # TODO, do we need update the show?
+      # apple_show.api.stub(:patch, OpenStruct.new(body: {"data" => {"id" => "123", "attributes" => {"foo" => "bar"}}}.to_json, code: "200")) do
+      apple_show.api.stub(:get, OpenStruct.new(body: {"data" => {"id" => "123", "attributes" => {"foo" => "bar"}}}.to_json, code: "200")) do
         assert apple_show.sync_log.present?
         apple_show.sync_log.update!(api_response: {})
 
@@ -122,7 +124,7 @@
     end
 
     it "creates a sync log if one does not exist" do
-      apple_show.api.stub(:patch, OpenStruct.new(body: {"data" => {"id" => "123", "attributes" => {"foo" => "bar"}}}.to_json, code: "200")) do
+      apple_show.api.stub(:get, OpenStruct.new(body: {"data" => {"id" => "123", "attributes" => {"foo" => "bar"}}}.to_json, code: "200")) do
         assert apple_show.sync_log.present?
 
         sync = apple_show.sync!
@@ -161,7 +163,7 @@
 
   describe "#show_data" do
     it "returns a hash" do
-      assert_equal apple_show.show_data.class, Hash
+      assert_equal apple_show.show_data({}).class, Hash
     end
   end