Skip to content

Commit

Permalink
Delegated delivery reupload (#853)
Browse files Browse the repository at this point in the history
* Episode delivery depends on status column

* Update episode delivery flag

* Incorporate the delivery flag into synced_with_apple?

* Mark the episode as needing delivery

* Add convenience method

* Episode needs delivery to reach this

* Annotate the episode publish logging methods

* Extract the apple delivery status off the episode model

* Include id in index, reset on mutate
  • Loading branch information
svevang authored Oct 4, 2023
1 parent aa112a0 commit 5f6b7b6
Show file tree
Hide file tree
Showing 11 changed files with 166 additions and 17 deletions.
15 changes: 13 additions & 2 deletions app/models/apple/episode.rb
Original file line number Diff line number Diff line change
Expand Up @@ -450,11 +450,18 @@ def reset_for_upload!
end

def needs_delivery?
podcast_container&.needs_delivery? || apple_hosted_audio_asset_container_id.blank?
return true if missing_container?

# TODO: probe for episode media version
podcast_container&.needs_delivery? || feeder_episode.apple_needs_delivery?
end

def has_delivery?
!needs_delivery?
end

def synced_with_apple?
audio_asset_state_success? && container_upload_complete? && !drafting?
audio_asset_state_success? && has_delivery? && !drafting?
end

def waiting_for_asset_state?
Expand Down Expand Up @@ -496,5 +503,9 @@ def apple_sync_log
def apple_sync_log=(sl)
feeder_episode.apple_sync_log = sl
end

def apple_mark_for_reupload!
feeder_episode.apple_mark_for_reupload!
end
end
end
5 changes: 5 additions & 0 deletions app/models/apple/episode_delivery_status.rb
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
module Apple
class EpisodeDeliveryStatus < ApplicationRecord
belongs_to :episode, class_name: "::Episode"
end
end
14 changes: 10 additions & 4 deletions app/models/apple/podcast_container.rb
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,7 @@ class PodcastContainer < ApplicationRecord

def self.reset_source_file_metadata(episodes)
episodes = episodes.select { |ep| ep.podcast_container.present? }
episodes = episodes.select { |ep| ep.podcast_container.needs_delivery? }
episodes = episodes.select { |ep| ep.needs_delivery? }

episodes.map do |episode|
container = episode.container
Expand All @@ -34,7 +34,6 @@ def self.reset_source_file_metadata(episodes)

# Back to DTR to pick up fresh arrangements:
container.reset_source_metadata!(episode)
episode.feeder_episode.apple_mark_for_reupload!
end.compact
end

Expand Down Expand Up @@ -271,7 +270,7 @@ def container_upload_satisfied?
# marked as uploaded and then processed and validated. Assuming that we
# get to that point and the audio is still missing, we should be able to
# retry.
has_podcast_audio? && delivery_settled?
has_podcast_audio?
end

def skip_delivery?
Expand All @@ -282,11 +281,18 @@ def needs_delivery?
!skip_delivery?
end

def filename_prefix(ct)
ct.zero? ? "" : "#{ct}_"
end

def reset_source_metadata!(apple_ep)
count = source_fetch_count + 1

update!(
source_fetch_count: count,
source_url: nil,
source_size: nil,
source_filename: apple_ep.enclosure_filename,
source_filename: filename_prefix(count) + apple_ep.enclosure_filename,
enclosure_url: apple_ep.enclosure_url
)
end
Expand Down
7 changes: 7 additions & 0 deletions app/models/apple/publisher.rb
Original file line number Diff line number Diff line change
Expand Up @@ -264,7 +264,14 @@ def mark_delivery_files_uploaded!(eps)
pdfs = eps.map(&:podcast_delivery_files).flatten
::Apple::PodcastDeliveryFile.mark_uploaded(api, pdfs)

# link the podcast container with the audio to the episode
res = Apple::Episode.update_audio_container_reference(api, eps)
# update the feeder episode to indicate that delivery is no longer needed
eps.each do |ep|
Rails.logger.info("Marking episode as no longer needing delivery", {episode_id: ep.feeder_episode.id})
ep.feeder_episode.apple_has_delivery!
end

Rails.logger.info("Updated remote container references for episodes.", {count: res.length})
end
end
Expand Down
31 changes: 29 additions & 2 deletions app/models/episode.rb
Original file line number Diff line number Diff line change
Expand Up @@ -39,6 +39,7 @@ class Episode < ApplicationRecord
class_name: "Apple::PodcastDelivery"
has_many :apple_podcast_delivery_files, through: :apple_podcast_deliveries, source: :podcast_delivery_files,
class_name: "Apple::PodcastDeliveryFile"
has_many :apple_episode_delivery_statuses, -> { order(created_at: :desc) }, dependent: :destroy, class_name: "Apple::EpisodeDeliveryStatus"

validates :podcast_id, :guid, presence: true
validates :title, presence: true
Expand Down Expand Up @@ -114,6 +115,28 @@ def apple_delivery_file_errors
apple_delivery_files.map { |p| p.asset_processing_state["errors"] }.flatten
end

def apple_episode_delivery_status
apple_episode_delivery_statuses.order(created_at: :desc).first
end

def apple_needs_delivery?
return true if apple_episode_delivery_status.nil?

apple_episode_delivery_status.delivered == false
end

def apple_needs_delivery!
apple_episode_delivery_statuses.create!(delivered: false)
apple_episode_delivery_statuses.reset
apple_episode_delivery_status
end

def apple_has_delivery!
apple_episode_delivery_statuses.create!(delivered: true)
apple_episode_delivery_statuses.reset
apple_episode_delivery_status
end

def self.generate_item_guid(podcast_id, episode_guid)
"prx_#{podcast_id}_#{episode_guid}"
end
Expand Down Expand Up @@ -231,14 +254,18 @@ def copy_media(force = false)
end

def apple_mark_for_reupload!
# remove the previous delivery attempt (soft delete)
apple_podcast_deliveries.map(&:destroy)
apple_podcast_deliveries.reset
apple_podcast_container&.podcast_deliveries&.reset
apple_needs_delivery!
end

def publish!
apple_mark_for_reupload!
podcast&.publish!
Rails.logger.tagged("Episode#publish!") do
apple_mark_for_reupload!
podcast&.publish!
end
end

def podcast_feed_url
Expand Down
37 changes: 37 additions & 0 deletions db/migrate/20230920153421_add_needs_apple_delivery_to_episode.rb
Original file line number Diff line number Diff line change
@@ -0,0 +1,37 @@
class AddNeedsAppleDeliveryToEpisode < ActiveRecord::Migration[7.0]
def change
create_table :apple_episode_delivery_statuses do |t|
t.references :episode, null: false, foreign_key: true
t.boolean :delivered, default: false

t.datetime :created_at, null: false
end

execute('CREATE INDEX "index_apple_episode_delivery_statuses_on_episode_id_created_at" ON "apple_episode_delivery_statuses" ("episode_id", "created_at") INCLUDE (delivered, id);')

reversible do |dir|
dir.up do
episode_ids = SyncLog.where(feeder_type: :episodes).pluck(:feeder_id)
apple_episodes =
Episode.where(id: episode_ids).map do |e|
Apple::Episode.new(show: true, feeder_episode: e, api: true)
end

needs_delivery_episodes = []
apple_episodes.each do |apple_episode|
if (apple_episode.podcast_container.nil? || apple_episode.podcast_container.needs_delivery?) || apple_episode.apple_hosted_audio_asset_container_id.blank?
apple_episode.feeder_episode.apple_needs_delivery!
needs_delivery_episodes << apple_episode.feeder_episode
else
apple_episode.feeder_episode.apple_has_delivery!
end
end

Rails.logger.info("Needs delivery count: #{needs_delivery_episodes.length}")
needs_delivery_episodes.sort_by(&:podcast_id).each do |ep|
Rails.logger.info("Needs delivery episode: #{ep.id} podcast: #{ep.podcast_id}")
end
end
end
end
end
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
class AddFetchCountToPodcastContainer < ActiveRecord::Migration[7.0]
def change
add_column :apple_podcast_containers, :source_fetch_count, :integer, default: 0, null: false
end
end
12 changes: 11 additions & 1 deletion db/schema.rb

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

2 changes: 1 addition & 1 deletion test/factories/apple_episode_factory.rb
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@

# set a complete episode factory varient
factory :uploaded_apple_episode do
feeder_episode { create(:episode) }
feeder_episode { create(:episode, apple_episode_delivery_statuses: [Apple::EpisodeDeliveryStatus.new(delivered: true)]) }
transient do
api_response do
build(:apple_episode_api_response,
Expand Down
33 changes: 26 additions & 7 deletions test/models/apple/podcast_container_test.rb
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,8 @@ class Apple::PodcastContainerTest < ActiveSupport::TestCase
end
describe "#reset_source_metadata!" do
it "clears out the fields" do
assert_equal 0, pc.source_fetch_count
assert pc.source_filename.present?
assert pc.source_filename.present?
assert pc.source_url.present?
assert pc.source_size.present?
Expand All @@ -41,13 +43,31 @@ class Apple::PodcastContainerTest < ActiveSupport::TestCase
end

pc.reload
assert_equal 1, pc.source_fetch_count
assert pc.source_filename.present?
refute pc.source_url.present?
refute pc.source_size.present?

assert_equal "transistor", pc.source_filename
assert_equal "1_transistor", pc.source_filename
assert_equal "http://something/transistor", pc.enclosure_url
end

it "increments the source filename" do
apple_episode.stub(:enclosure_filename, "new") do
apple_episode.stub(:enclosure_url, "http://this-is-new") do
assert_equal "foo", pc.source_filename
assert_equal 0, pc.source_fetch_count

pc.reset_source_metadata!(apple_episode)
assert_equal "1_new", pc.source_filename
assert_equal 1, pc.source_fetch_count

pc.reset_source_metadata!(apple_episode)
assert_equal "2_new", pc.source_filename
assert_equal 2, pc.source_fetch_count
end
end
end
end

describe "#needs_file_metadata?" do
Expand All @@ -62,18 +82,19 @@ class Apple::PodcastContainerTest < ActiveSupport::TestCase

describe ".reset_source_file_metadata" do
it "needs delivery in order to be reset" do
apple_episode.podcast_container.stub(:needs_delivery?, false) do
apple_episode.stub(:needs_delivery?, false) do
Apple::PodcastContainer.reset_source_file_metadata([apple_episode])
end

apple_episode.podcast_container.reload
refute apple_episode.podcast_container.source_url.nil?
refute apple_episode.podcast_container.source_size.nil?
refute apple_episode.podcast_container.source_filename.nil?
assert_equal "foo", apple_episode.podcast_container.source_filename

apple_episode.stub(:enclosure_filename, "new") do
apple_episode.stub(:enclosure_url, "http://this-is-new") do
apple_episode.podcast_container.stub(:needs_delivery?, true) do
apple_episode.stub(:needs_delivery?, true) do
Apple::PodcastContainer.reset_source_file_metadata([apple_episode])
end
end
Expand All @@ -83,7 +104,7 @@ class Apple::PodcastContainerTest < ActiveSupport::TestCase
assert apple_episode.podcast_container.source_url.nil?
assert apple_episode.podcast_container.source_size.nil?
assert_equal "http://this-is-new", apple_episode.podcast_container.enclosure_url
assert_equal "new", apple_episode.podcast_container.source_filename
assert_equal "1_new", apple_episode.podcast_container.source_filename
end
end
end
Expand Down Expand Up @@ -273,9 +294,7 @@ class Apple::PodcastContainerTest < ActiveSupport::TestCase
{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
assert container.container_upload_satisfied?
end
end
end
Expand Down
22 changes: 22 additions & 0 deletions test/models/episode_test.rb
Original file line number Diff line number Diff line change
Expand Up @@ -431,4 +431,26 @@
assert_empty episode.apple_podcast_deliveries
end
end

describe "#apple_needs_delivery?" do
let(:episode) { create(:episode) }
it "is true by default" do
assert_nil episode.apple_episode_delivery_status
assert episode.apple_needs_delivery?
end

it "can be set to false" do
episode.apple_has_delivery!
refute episode.apple_needs_delivery?
end

it "can be set to true" do
episode.apple_has_delivery!
refute episode.apple_needs_delivery?

# now set it to true
episode.apple_needs_delivery!
assert episode.apple_needs_delivery?
end
end
end

0 comments on commit 5f6b7b6

Please sign in to comment.