Merge pull request #993 from PRX/feat/simplify_apple_config_associations

Apple::Config only belongs to the private feed

app/jobs/publish_feed_job.rb

@@ -15,7 +15,8 @@ def perform(podcast, pub_item)
     Rails.logger.info("Starting publishing pipeline via PublishFeedJob", {podcast_id: podcast.id, publishing_queue_item_id: pub_item.id})
 
     PublishingPipelineState.start!(podcast)
-    podcast.feeds.each { |feed| publish_feed(podcast, feed) }
+    podcast.feeds.each { |feed| publish_apple(podcast, feed) }
+    podcast.feeds.each { |feed| publish_rss(podcast, feed) }
     PublishingPipelineState.complete!(podcast)
   rescue => e
     PublishingPipelineState.error!(podcast)
@@ -25,19 +26,14 @@ def perform(podcast, pub_item)
     PublishingPipelineState.settle_remaining!(podcast)
   end
 
-  def publish_feed(podcast, feed)
-    publish_apple(feed)
-    publish_rss(podcast, feed)
-  end
-
-  def publish_apple(feed)
+  def publish_apple(podcast, feed)
     return unless feed.publish_to_apple?
     res = PublishAppleJob.perform_now(feed.apple_config)
-    PublishingPipelineState.publish_apple!(feed.podcast)
+    PublishingPipelineState.publish_apple!(podcast)
     res
   rescue => e
     NewRelic::Agent.notice_error(e)
-    res = PublishingPipelineState.error_apple!(feed.podcast)
+    res = PublishingPipelineState.error_apple!(podcast)
     raise e if feed.apple_config.sync_blocks_rss?
     res
   end

app/models/apple/config.rb

@@ -6,32 +6,23 @@ class Config < ApplicationRecord
     DEFAULT_TITLE = "Apple Delegated Delivery Subscriptions"
     DEFAULT_AUDIO_FORMAT = {"f" => "flac", "b" => 16, "c" => 2, "s" => 44100}.freeze
 
-    belongs_to :podcast, class_name: "Podcast"
-    belongs_to :public_feed, class_name: "Feed"
-    belongs_to :private_feed, class_name: "Feed"
-    belongs_to :key, class_name: "Apple::Key", optional: true
+    belongs_to :feed
+    belongs_to :key, class_name: "Apple::Key", optional: true, validate: true, autosave: true
 
-    delegate :title, to: :podcast, prefix: "podcast"
+    validate :podcast_has_one_apple_config
+    validate :not_default_feed
 
+    # backwards-compatible "key" getters
     delegate :provider_id, to: :key
     delegate :key_id, to: :key
     delegate :key_pem, to: :key
     delegate :key_pem_b64, to: :key
 
-    validates_presence_of :podcast
-    validates_presence_of :public_feed
-    validates_presence_of :private_feed
-
-    validates_associated :public_feed
-    validates_associated :private_feed
-
-    validates :podcast, uniqueness: true, allow_nil: false
-
-    validates :public_feed, uniqueness: {scope: :private_feed,
-                                         message: "can only have one config per public and private feed"}
-    validates :public_feed, exclusion: {in: ->(apple_credential) { [apple_credential.private_feed] }}
-
-    validate :feed_podcasts_match
+    # backwards-compatible associations
+    delegate :podcast, to: :feed, allow_nil: true
+    delegate :id, :title, to: :podcast, prefix: true, allow_nil: true
+    delegate :public_feed, to: :podcast, allow_nil: true
+    alias_method :private_feed, :feed
 
     def self.find_or_build_private_feed(podcast)
       if (existing = podcast.feeds.find_by(slug: DEFAULT_FEED_SLUG, title: DEFAULT_TITLE))
@@ -64,12 +55,7 @@ def self.build_apple_config(podcast, key)
         raise "Stopping build_apple_config" if $stdin.gets.chomp.downcase != "y"
       end
 
-      Apple::Config.new(
-        podcast: podcast,
-        public_feed: podcast.default_feed,
-        private_feed: find_or_build_private_feed(podcast),
-        key: key
-      )
+      Apple::Config.new(feed: find_or_build_private_feed(podcast), key: key)
     end
 
     def self.mark_as_delivered!(apple_publisher)
@@ -99,18 +85,21 @@ def self.setup_delegated_delivery(podcast, key: nil, apple_config: nil, apple_sh
       mark_as_delivered!(pub)
     end
 
-    def feed_podcasts_match
-      return unless public_feed.present? && private_feed.present?
+    def not_default_feed
+      if feed&.default?
+        errors.add(:feed, "cannot use default feed")
+      end
+    end
 
-      if (public_feed.podcast_id != podcast_id) || (private_feed.podcast_id != podcast_id)
-        errors.add(:public_feed, "must belong to the same podcast as the private feed")
+    def podcast_has_one_apple_config
+      all_feeds = Feed.where(podcast_id: feed.podcast_id).pluck(:id)
+      if Apple::Config.where(feed_id: all_feeds).where.not(id: id).any?
+        errors.add(:feed, "podcast already has an apple config")
       end
     end
 
     def publish_to_apple?
-      return false unless key&.valid?
-
-      public_feed.publish_to_apple?
+      !!key&.valid? && publish_enabled?
     end
 
     def build_publisher

app/models/feed.rb

@@ -25,13 +25,7 @@ class Feed < ApplicationRecord
   alias_attribute :tokens, :feed_tokens
   accepts_nested_attributes_for :feed_tokens, allow_destroy: true, reject_if: ->(ft) { ft[:token].blank? }
 
-  has_one :apple_config,
-    autosave: true,
-    class_name: "::Apple::Config",
-    dependent: :destroy,
-    foreign_key: :public_feed_id,
-    inverse_of: :public_feed,
-    validate: true
+  has_one :apple_config, class_name: "::Apple::Config", dependent: :destroy, autosave: true, validate: true
 
   has_many :feed_images, -> { order("created_at DESC") }, autosave: true, dependent: :destroy, inverse_of: :feed
   has_many :itunes_images, -> { order("created_at DESC") }, autosave: true, dependent: :destroy, inverse_of: :feed
@@ -173,9 +167,7 @@ def normalize_category(cat)
   end
 
   def publish_to_apple?
-    apple_config.present? &&
-      apple_config.public_feed == self &&
-      apple_config.publish_enabled?
+    !!apple_config&.publish_to_apple?
   end
 
   def include_tags=(tags)

app/models/podcast.rb

@@ -20,7 +20,7 @@ class Podcast < ApplicationRecord
   serialize :restrictions, JSON
 
   has_one :default_feed, -> { default }, class_name: "Feed", validate: true, autosave: true, inverse_of: :podcast
-  has_one :apple_config, class_name: "::Apple::Config", validate: true, autosave: true, inverse_of: :podcast
+  alias_method :public_feed, :default_feed
 
   has_many :episodes, -> { order("published_at desc") }, dependent: :destroy
   has_many :feeds, dependent: :destroy
@@ -67,6 +67,19 @@ def default_feed
     super || build_default_feed(podcast: self, private: false)
   end
 
+  def apple_config
+    if defined?(@apple_config)
+      @apple_config
+    else
+      @apple_config = Apple::Config.where(feed_id: feeds.pluck(:id)).first
+    end
+  end
+
+  def reload(options = nil)
+    remove_instance_variable(:@apple_config) if defined?(@apple_config)
+    super
+  end
+
   def explicit=(value)
     super Podcast::EXPLICIT_ALIASES.fetch(value, value)
   end

db/migrate/20240410212602_simplify_apple_config_foreign_keys.rb

@@ -0,0 +1,32 @@
+class SimplifyAppleConfigForeignKeys < ActiveRecord::Migration[7.0]
+  def up
+    Apple::Config.find_each do |ac|
+      unless ac[:public_feed_id] == Podcast.find(ac[:podcast_id]).default_feed.id
+        raise "Apple::Config[#{ac.id} is not using the podcast.default_feed"
+      end
+    end
+
+    remove_column :apple_configs, :podcast_id
+    remove_column :apple_configs, :public_feed_id
+    rename_column :apple_configs, :private_feed_id, :feed_id
+  end
+
+  def down
+    add_column :apple_configs, :podcast_id, :integer
+    add_index :apple_configs, [:podcast_id], unique: true
+
+    add_reference :apple_configs, :public_feed, index: true
+    add_foreign_key :apple_configs, :feeds, column: :public_feed_id
+
+    rename_column :apple_configs, :feed_id, :private_feed_id
+
+    Apple::Config.find_each do |ac|
+      f = Feed.find(ac.private_feed_id)
+      ac.podcast_id = f.podcast.id
+      ac.public_feed_id = f.podcast.default_feed.id
+      ac.save!(validate: false)
+    end
+
+    change_column_null :apple_configs, :public_feed_id, false
+  end
+end

test/factories/apple_config_factory.rb

@@ -3,12 +3,6 @@
     publish_enabled { true }
     sync_blocks_rss { true }
     key { build(:apple_key) }
-    podcast
-
-    # set up the private and public feeds
-    before(:create) do |apple_config, evaluator|
-      apple_config.public_feed ||= create(:feed, podcast: evaluator.podcast)
-      apple_config.private_feed ||= create(:feed, podcast: evaluator.podcast)
-    end
+    feed
   end
 end

test/jobs/publish_apple_job_test.rb

@@ -3,9 +3,9 @@
 describe PublishAppleJob do
   let(:episode) { create(:episode, prx_uri: "/api/v1/stories/87683") }
   let(:podcast) { episode.podcast }
-  let(:feed) { create(:feed, podcast: podcast, slug: "adfree") }
+  let(:feed) { podcast.default_feed }
   let(:private_feed) { create(:private_feed, podcast: podcast) }
-  let(:apple_config) { create(:apple_config, podcast: podcast, public_feed: feed, private_feed: private_feed) }
+  let(:apple_config) { create(:apple_config, feed: private_feed) }
 
   describe "publishing to apple" do
     it "does not publish to apple unless publish_enabled?" do

test/jobs/publish_feed_job_test.rb

@@ -82,40 +82,40 @@
 
   describe "publishing to apple" do
     it "does not schedule publishing to apple if there is no apple config" do
-      assert_nil feed.apple_config
-      assert_nil job.publish_apple(feed)
+      assert_nil private_feed.apple_config
+      assert_nil job.publish_apple(podcast, private_feed)
     end
 
     describe "when the apple config is present" do
-      let(:apple_config) { create(:apple_config, podcast: podcast, public_feed: feed, private_feed: private_feed) }
+      let(:apple_config) { create(:apple_config, feed: private_feed) }
 
       it "does not schedule publishing to apple if the config is marked as not publishable" do
         apple_config.update!(publish_enabled: false)
-        assert_equal apple_config, feed.apple_config.reload
-        assert_nil job.publish_apple(feed)
+        assert_equal apple_config, private_feed.apple_config.reload
+        assert_nil job.publish_apple(podcast, private_feed)
       end
 
       it "does run the apple publishing if the config is present and marked as publishable" do
-        assert_equal apple_config, feed.apple_config.reload
+        assert_equal apple_config, private_feed.apple_config.reload
         PublishAppleJob.stub(:perform_now, :publishing_apple!) do
-          assert_equal :publishing_apple!, job.publish_apple(feed)
+          assert_equal :publishing_apple!, job.publish_apple(podcast, private_feed)
         end
       end
 
       it "Performs the apple publishing job based regardless of sync_blocks_rss flag" do
-        assert_equal apple_config, feed.apple_config.reload
+        assert_equal apple_config, private_feed.apple_config.reload
 
         # stub the two possible ways the job can be called
         # perform_later is not used.
         PublishAppleJob.stub(:perform_later, :perform_later) do
           PublishAppleJob.stub(:perform_now, :perform_now) do
             apple_config.update!(sync_blocks_rss: true)
 
-            assert_equal :perform_now, job.publish_apple(feed)
+            assert_equal :perform_now, job.publish_apple(podcast, private_feed)
 
             apple_config.update!(sync_blocks_rss: false)
             feed.reload
-            assert_equal :perform_now, job.publish_apple(feed)
+            assert_equal :perform_now, job.publish_apple(podcast, private_feed)
           end
         end
       end
@@ -128,26 +128,26 @@
           PublishingPipelineState.start!(feed.podcast)
         end
         it "raises an error if the apple publishing fails" do
-          assert_equal apple_config, feed.apple_config.reload
+          assert_equal apple_config, private_feed.apple_config.reload
 
           PublishAppleJob.stub(:perform_now, ->(*, **) { raise "some apple error" }) do
             # it raises
-            assert_raises(RuntimeError) { job.publish_apple(feed) }
+            assert_raises(RuntimeError) { job.publish_apple(podcast, private_feed) }
 
             assert_equal ["created", "started", "error_apple"].sort, PublishingPipelineState.where(podcast: feed.podcast).latest_pipelines.pluck(:status).sort
           end
         end
 
         it "does not raise an error if the apple publishing is not blocking RSS" do
-          assert_equal apple_config, feed.apple_config.reload
-          feed.apple_config.update!(sync_blocks_rss: false)
+          assert_equal apple_config, private_feed.apple_config.reload
+          private_feed.apple_config.update!(sync_blocks_rss: false)
 
           mock = Minitest::Mock.new
           mock.expect(:call, nil, [RuntimeError])
 
           PublishAppleJob.stub(:perform_now, ->(*, **) { raise "some apple error" }) do
             NewRelic::Agent.stub(:notice_error, mock) do
-              job.publish_apple(feed)
+              job.publish_apple(podcast, private_feed)
             end
           end
           assert_equal ["created", "started", "error_apple"].sort, PublishingPipelineState.where(podcast: feed.podcast).latest_pipelines.pluck(:status).sort