Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Integrations interface #1136

Closed
wants to merge 1 commit into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
10 changes: 9 additions & 1 deletion app/models/apple/episode.rb
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
# frozen_string_literal: true

module Apple
class Episode
class Episode < Integrations::Base::Episode
include Apple::ApiWaiting
include Apple::ApiResponse
attr_accessor :show,
Expand Down Expand Up @@ -221,6 +221,14 @@ def initialize(show:, feeder_episode:, api:)
@api = api || Apple::Api.from_env
end

def synced_with_integration?
synced_with_apple?
end

def integration_new?
apple_new?
end

def api_response
feeder_episode.apple_sync_log&.api_response
end
Expand Down
48 changes: 1 addition & 47 deletions app/models/apple/publisher.rb
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
# frozen_string_literal: true

module Apple
class Publisher
class Publisher < Integrations::Base::Publisher
PUBLISH_CHUNK_LEN = 25

attr_reader :public_feed,
Expand Down Expand Up @@ -35,52 +35,6 @@ def podcast
public_feed.podcast
end

def filter_episodes_to_sync(eps)
# Reject episodes if the audio is marked as uploaded/complete
# or if the episode is a video
eps
.reject(&:synced_with_apple?)
.reject(&:video_content_type?)
end

def episodes_to_sync
# only look at the private delegated delivery feed
filter_episodes_to_sync(show.apple_private_feed_episodes)
end

def filter_episodes_to_archive(eps)
eps_in_private_feed = Set.new(show.apple_private_feed_episodes)

# Episodes to archive can include:
# - episodes that are now excluded from the feed
# - episodes that are deleted or unpublished
# - episodes that have fallen off the end of the feed (Feed#display_episodes_count)
eps
.reject { |ep| eps_in_private_feed.include?(ep) }
.reject(&:apple_new?)
.reject(&:archived?)
end

def episodes_to_archive
# look at the global list of episodes, not just the private feed
filter_episodes_to_archive(show.podcast_episodes)
end

def filter_episodes_to_unarchive(eps)
eps.filter(&:archived?)
end

def episodes_to_unarchive
# only look at the private delegated delivery feed
filter_episodes_to_unarchive(show.apple_private_feed_episodes)
end

def only_episodes_with_apple_state(eps)
# Only select episodes that have an remote apple state,
# as determined by the sync log
eps.reject(&:apple_new?)
end

def poll_all_episodes!
poll_episodes!(show.podcast_episodes)
end
Expand Down
53 changes: 3 additions & 50 deletions app/models/apple/show.rb
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
# frozen_string_literal: true

module Apple
class Show
class Show < Integrations::Base::Show
include Apple::ApiResponse

attr_reader :public_feed,
Expand Down Expand Up @@ -165,55 +165,8 @@ def get_show
self.class.get_show(api, apple_id)
end

# In the case where there are duplicate guids in the feeds, we want to make
# sure that the most "current" episode is the one that maps to the remote state.
def sort_by_episode_properties(eps)
# Sort the episodes by:
# 1. Non-deleted episodes first
# 2. Published episodes first
# 3. Published date most recent first
# 4. Created date most recent first
eps =
eps.sort_by do |e|
[
e.deleted_at.nil? ? 1 : -1,
e.published_at.present? ? 1 : -1,
e.published_at || e.created_at,
e.created_at
]
end

# return sorted list, reversed
# modeling a priority queue -- most important first
eps.reverse
end

def podcast_feeder_episodes
@podcast_feeder_episodes ||=
podcast.episodes
.reset
.with_deleted
.group_by(&:item_guid)
.values
.map { |eps| sort_by_episode_properties(eps) }
.map(&:first)
end

# All the episodes -- including deleted and unpublished
def podcast_episodes
@podcast_episodes ||= podcast_feeder_episodes.map { |e| Apple::Episode.new(api: api, show: self, feeder_episode: e) }
end

# Does not include deleted episodes
def episodes
raise "Missing apple show id" unless apple_id.present?

@episodes ||= begin
feed_episode_ids = Set.new(private_feed.feed_episodes.feed_ready.map(&:id))

podcast_episodes
.filter { |e| feed_episode_ids.include?(e.feeder_episode.id) }
end
def build_integration_episode(feeder_episode)
Apple::Episode.new(api: api, show: self, feeder_episode: feeder_episode)
end

def apple_private_feed_episodes
Expand Down
40 changes: 40 additions & 0 deletions app/models/integrations/base/episode.rb
Original file line number Diff line number Diff line change
@@ -0,0 +1,40 @@
module Integrations
module Base
class Episode
attr_reader :feeder_episode

def initialize(feeder_episode)
@feeder_episode = feeder_episode
end

def synced_with_integration?
raise NotImplementedError, "Subclasses must implement synced_with_integration?"
end

def integration_new?
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this means not persisted to the integration?

it would be helpful of there were some common way for integrations to save external ids and state of persistence to the 3rd party

raise NotImplementedError, "Subclasses must implement integration_new?"
end

def archived?
raise NotImplementedError, "Subclasses must implement archived?"
end

def video_content_type?
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Megaphone doesn't support video at all, so this is also not applicable

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

my bad, that is how this is used! perfect

feeder_episode.video_content_type?
end

# Delegate methods to feeder_episode
def method_missing(method_name, *arguments, &block)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The episode integration written for megaphone act as their own entity, passing through methods to the underlying feeder episode is likely to cause issues, this is not needed.

if feeder_episode.respond_to?(method_name)
feeder_episode.send(method_name, *arguments, &block)
else
super
end
end

def respond_to_missing?(method_name, include_private = false)
feeder_episode.respond_to?(method_name) || super
end
end
end
end
56 changes: 56 additions & 0 deletions app/models/integrations/base/episode_set_operations.rb
Original file line number Diff line number Diff line change
@@ -0,0 +1,56 @@
module Integrations
module Base
module EpisodeSetOperations
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The root of the episode set manipulation. The consumption side: Deriving the sets of episodes and filtering them for CRUD sync

# In the case where there are duplicate guids in the feeds, we want to make
# sure that the most "current" episode is the one that maps to the remote state.
def sort_by_episode_properties(eps)
# Sort the episodes by:
# 1. Non-deleted episodes first
# 2. Published episodes first
# 3. Published date most recent first
# 4. Created date most recent first
eps =
eps.sort_by do |e|
[
e.deleted_at.nil? ? 1 : -1,
e.published_at.present? ? 1 : -1,
e.published_at || e.created_at,
e.created_at
]
end

# return sorted list, reversed
# modeling a priority queue -- most important first
eps.reverse
end

def filter_episodes_to_sync(eps)
# Reject episodes if the audio is marked as uploaded/complete
# or if the episode is a video
eps
.reject(&:synced_with_integration?)
.reject(&:video_content_type?)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ah, that is common to megaphone as well, but that's sorta luck - other integrations may have no issue with video, so this method may not be common/reusable

end

def filter_episodes_to_archive(eps, eps_in_feed)
# Episodes to archive can include:
# - episodes that are now excluded from the feed
# - episodes that are deleted or unpublished
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maybe? megaphone doesn't have an "archived" status for episodes, that is an apple concept
It has a notion of publishing or unpublishing- but the status is one of "published", "scheduled", "not_ready"

# - episodes that have fallen off the end of the feed (Feed#display_episodes_count)
eps
.reject { |ep| eps_in_feed.include?(ep) }
.reject(&:integration_new?)
.reject(&:archived?)
end

def filter_episodes_to_unarchive(eps)
eps.filter(&:archived?)
end

# Only select episodes that have an remote integration state
def only_episodes_with_integration_state(eps)
eps.reject(&:integration_new?)
end
end
end
end
29 changes: 29 additions & 0 deletions app/models/integrations/base/publisher.rb
Original file line number Diff line number Diff line change
@@ -0,0 +1,29 @@
module Integrations
module Base
class Publisher
include EpisodeSetOperations

attr_reader :show

def initialize(show:)
@show = show
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

A "show" is an apple concept, not in megaphone, in megaphone it is a Podcast.
The api podcast object for megaphone could maybe implement some of this, we'll see, but at a minimum it's a misnomer, and not how I organized the megaphone code

end

def episodes_to_sync
filter_episodes_to_sync(show.episodes)
end

def episodes_to_archive
filter_episodes_to_archive(show.podcast_episodes, Set.new(show.episodes))
end

def episodes_to_unarchive
filter_episodes_to_unarchive(show.episodes)
end

def publish!
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This makes sense as it is the only part of this that I see called by another class/module, this seems like the interface of the publisher

raise NotImplementedError, "Subclasses must implement publish!"
end
end
end
end
50 changes: 50 additions & 0 deletions app/models/integrations/base/show.rb
Original file line number Diff line number Diff line change
@@ -0,0 +1,50 @@
module Integrations
module Base
class Show
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The show gives the handle on the episodes. The production side: baseline unfiltered, sorted set of episodes.

include EpisodeSetOperations

attr_reader :feed

def initialize(public_feed:, private_feed:)
@public_feed = public_feed
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

public feed not used in this

@private_feed = private_feed
end

def podcast
private_feed.podcast
end

def podcast_feeder_episodes
@podcast_feeder_episodes ||=
podcast.episodes
.reset
.with_deleted
.group_by(&:item_guid)
.values
.map { |eps| sort_by_episode_properties(eps) }
.map(&:first)
end

# All the episodes -- including deleted and unpublished
def podcast_episodes
@podcast_episodes ||= podcast_feeder_episodes.map { |e| build_integration_episode(e) }
end

# Does not include deleted episodes
def episodes
@episodes ||= begin
feed_episode_ids = Set.new(private_feed.feed_episodes.feed_ready.map(&:id))

podcast_episodes
.filter { |e| feed_episode_ids.include?(e.feeder_episode.id) }
end
end

private

def build_integration_episode(feeder_episode)
raise NotImplementedError, "Subclasses must implement create_integration_episode"
end
end
end
end
4 changes: 2 additions & 2 deletions test/models/apple/publisher_test.rb
Original file line number Diff line number Diff line change
Expand Up @@ -38,10 +38,10 @@

it "should only return episodes that have an apple state" do
episode.stub(:apple_new?, true) do
assert_equal apple_publisher.only_episodes_with_apple_state([episode]), []
assert_equal apple_publisher.only_episodes_with_integration_state([episode]), []
end
episode.stub(:apple_new?, false) do
assert_equal apple_publisher.only_episodes_with_apple_state([episode]), [episode]
assert_equal apple_publisher.only_episodes_with_integration_state([episode]), [episode]
end
end
end
Expand Down
Loading