From bfa1bdc37f4bf33c76d767ed815e30a384b369e0 Mon Sep 17 00:00:00 2001 From: Anna Buianova Date: Sat, 22 Jun 2019 04:31:15 +0300 Subject: [PATCH] More specs and refactoring podcasts fetching (#3245) * More specs and refactoring podcasts fetching * Fixed PodcastFeed references in tests * Pass podcast_id instead of podcast to CreateEpisode --- app/jobs/podcasts/get_episodes_job.rb | 2 +- app/labor/podcast_feed.rb | 88 --------------- app/services/podcasts/create_episode.rb | 49 +++++++++ app/services/podcasts/feed.rb | 18 ++++ app/services/podcasts/get_episode.rb | 30 ++++++ app/services/podcasts/update_episode.rb | 35 ++++++ spec/requests/podcast_episodes_api_spec.rb | 2 +- spec/services/podcasts/create_episode_spec.rb | 61 +++++++++++ .../podcasts/feed_spec.rb} | 15 ++- spec/services/podcasts/update_episode_spec.rb | 38 +++++++ spec/support/fixtures/awayfromthekeyboard.rss | 51 +++++++++ spec/support/fixtures/developertea.rss | 100 ++++++++++++++++++ 12 files changed, 394 insertions(+), 95 deletions(-) delete mode 100644 app/labor/podcast_feed.rb create mode 100644 app/services/podcasts/create_episode.rb create mode 100644 app/services/podcasts/feed.rb create mode 100644 app/services/podcasts/get_episode.rb create mode 100644 app/services/podcasts/update_episode.rb create mode 100644 spec/services/podcasts/create_episode_spec.rb rename spec/{labor/podcast_feed_spec.rb => services/podcasts/feed_spec.rb} (75%) create mode 100644 spec/services/podcasts/update_episode_spec.rb create mode 100644 spec/support/fixtures/awayfromthekeyboard.rss create mode 100644 spec/support/fixtures/developertea.rss diff --git a/app/jobs/podcasts/get_episodes_job.rb b/app/jobs/podcasts/get_episodes_job.rb index 5f9ed54a5..fa8732f1e 100644 --- a/app/jobs/podcasts/get_episodes_job.rb +++ b/app/jobs/podcasts/get_episodes_job.rb @@ -2,7 +2,7 @@ module Podcasts class GetEpisodesJob < ApplicationJob queue_as :podcasts_get_episodes - def perform(podcast_id, limit = 1000, feed = PodcastFeed.new) + def perform(podcast_id, limit = 1000, feed = Podcasts::Feed.new) podcast = Podcast.find_by(id: podcast_id) return unless podcast diff --git a/app/labor/podcast_feed.rb b/app/labor/podcast_feed.rb deleted file mode 100644 index 926c92a9e..000000000 --- a/app/labor/podcast_feed.rb +++ /dev/null @@ -1,88 +0,0 @@ -require "rss" -require "rss/itunes" - -class PodcastFeed - def get_episodes(podcast, num = 1000) - rss = HTTParty.get(podcast.feed_url).body - feed = RSS::Parser.parse(rss, false) - feed.items.first(num).each do |item| - ep = existing_episode(item, podcast).first - if ep - update_existing_episode(ep, item, podcast) - else - create_new_episode(item, podcast) - end - end - feed.items.size - rescue StandardError => e - Rails.logger.error(e) - end - - private - - # returns empty array if an episode doesn't exist - def existing_episode(item, podcast) - # presence returns nil if the query is an empty array, otherwise returns the array - podcasts = PodcastEpisode.where(media_url: item.enclosure.url).presence || - PodcastEpisode.where(title: item.title).presence || - PodcastEpisode.where(guid: item.guid.to_s).presence || - (podcast.unique_website_url? && PodcastEpisode.where(website_url: item.link).presence) - podcasts.to_a - end - - def create_new_episode(item, podcast) - ep = PodcastEpisode.new - ep.title = item.title - ep.podcast_id = podcast.id - ep.slug = item.title.parameterize - ep.subtitle = item.itunes_subtitle - ep.summary = item.itunes_summary - ep.website_url = item.link - ep.guid = item.guid - get_media_url(ep, item, podcast) - begin - ep.published_at = item.pubDate.to_date - rescue StandardError => e - Rails.logger.error("not a valid date: #{e}") - end - ep.body = item.content_encoded || item.itunes_summary || item.description - ep.save! - end - - def update_existing_episode(episode, item, _podcast) - if episode.published_at.nil? - begin - episode.published_at = item.pubDate.to_date - episode.save - rescue StandardError => e - Rails.logger.error("not a valid date: #{e}") - end - end - update_media_url(episode, item) - end - - def get_media_url(episode, item, podcast) - episode.media_url = if Rails.env.test? || - HTTParty.head(item.enclosure.url.gsub(/http:/, "https:")).code == 200 - item.enclosure.url.gsub(/http:/, "https:") - else - item.enclosure.url - end - rescue StandardError - # podcast episode must have a media_url - episode.media_url = item.enclosure.url - podcast.update(status_notice: "This podcast may not be playable in the browser") if podcast.status_notice.empty? - end - - def update_media_url(episode, item) - if episode.media_url.include?("https") - nil - elsif !episode.media_url.include?("https") && - item.enclosure.url.include?("https") - episode.update!(media_url: item.enclosure.url) - end - rescue StandardError - message = "something went wrong with #{podcast.title}, #{episode.title} -- #{episode.media_url}" - Rails.logger.error(message) - end -end diff --git a/app/services/podcasts/create_episode.rb b/app/services/podcasts/create_episode.rb new file mode 100644 index 000000000..1ecc9e08e --- /dev/null +++ b/app/services/podcasts/create_episode.rb @@ -0,0 +1,49 @@ +module Podcasts + class CreateEpisode + def initialize(podcast_id, item) + @podcast_id = podcast_id + @item = item + end + + def self.call(*args) + new(*args).call + end + + def call + ep = PodcastEpisode.new + ep.title = item.title + ep.podcast_id = podcast_id + ep.slug = item.title.parameterize + ep.subtitle = item.itunes_subtitle + ep.summary = item.itunes_summary + ep.website_url = item.link + ep.guid = item.guid + get_media_url(ep) + begin + ep.published_at = item.pubDate.to_date + rescue StandardError => e + Rails.logger.error("not a valid date: #{e}") + end + ep.body = item.content_encoded || item.itunes_summary || item.description + ep.save! + ep + end + + private + + attr_reader :podcast_id, :item + + # checking url when it is https is useless, the url is set to the enclosure url anyway + def get_media_url(episode) + episode.media_url = if HTTParty.head(item.enclosure.url.gsub(/http:/, "https:")).code == 200 + item.enclosure.url.gsub(/http:/, "https:") + else + item.enclosure.url + end + rescue StandardError + # podcast episode must have a media_url + episode.media_url = item.enclosure.url + episode.podcast.update(status_notice: "This podcast may not be playable in the browser") if episode.podcast.status_notice.empty? + end + end +end diff --git a/app/services/podcasts/feed.rb b/app/services/podcasts/feed.rb new file mode 100644 index 000000000..7d6af7ba8 --- /dev/null +++ b/app/services/podcasts/feed.rb @@ -0,0 +1,18 @@ +require "rss" +require "rss/itunes" + +module Podcasts + class Feed + def get_episodes(podcast, num = 1000) + rss = HTTParty.get(podcast.feed_url).body + feed = RSS::Parser.parse(rss, false) + get_episode = Podcasts::GetEpisode.new(podcast) + feed.items.first(num).each do |item| + get_episode.call(item) + end + feed.items.size + rescue StandardError => e + Rails.logger.error(e) + end + end +end diff --git a/app/services/podcasts/get_episode.rb b/app/services/podcasts/get_episode.rb new file mode 100644 index 000000000..8d78315d8 --- /dev/null +++ b/app/services/podcasts/get_episode.rb @@ -0,0 +1,30 @@ +module Podcasts + class GetEpisode + def initialize(podcast) + @podcast = podcast + end + + def call(item) + ep = existing_episode(item, podcast).first + if ep + Podcasts::UpdateEpisode.call(ep, item) + else + Podcasts::CreateEpisode.call(podcast.id, item) + end + end + + private + + attr_reader :podcast + + # returns empty array if an episode doesn't exist + def existing_episode(item, podcast) + # presence returns nil if the query is an empty array, otherwise returns the array + podcasts = PodcastEpisode.where(media_url: item.enclosure.url).presence || + PodcastEpisode.where(title: item.title).presence || + PodcastEpisode.where(guid: item.guid.to_s).presence || + (podcast.unique_website_url? && PodcastEpisode.where(website_url: item.link).presence) + podcasts.to_a + end + end +end diff --git a/app/services/podcasts/update_episode.rb b/app/services/podcasts/update_episode.rb new file mode 100644 index 000000000..c11ed3aa6 --- /dev/null +++ b/app/services/podcasts/update_episode.rb @@ -0,0 +1,35 @@ +module Podcasts + class UpdateEpisode + def initialize(episode, item) + @episode = episode + @item = item + end + + def self.call(*args) + new(*args).call + end + + def call + update_published_at unless episode.published_at? + update_media_url if !episode.media_url.include?("https") && item.enclosure.url.include?("https") + end + + private + + attr_reader :episode, :item + + def update_published_at + episode.published_at = item.pubDate.to_date + episode.save + rescue StandardError => e + Rails.logger.error("not a valid date: #{e}") + end + + def update_media_url + episode.update!(media_url: item.enclosure.url) + rescue StandardError + message = "something went wrong with #{episode.podcast_title}, #{episode.title} -- #{episode.media_url}" + Rails.logger.error(message) + end + end +end diff --git a/spec/requests/podcast_episodes_api_spec.rb b/spec/requests/podcast_episodes_api_spec.rb index dd0fa81e5..cea15438a 100644 --- a/spec/requests/podcast_episodes_api_spec.rb +++ b/spec/requests/podcast_episodes_api_spec.rb @@ -9,7 +9,7 @@ RSpec.describe "ArticlesApi", type: :request, vcr: vcr_option do let(:podcast) { create(:podcast, feed_url: "http://softwareengineeringdaily.com/feed/podcast/") } before do - PodcastFeed.new.get_episodes(podcast, 2) + Podcasts::Feed.new.get_episodes(podcast, 2) end describe "GET /api/articles" do diff --git a/spec/services/podcasts/create_episode_spec.rb b/spec/services/podcasts/create_episode_spec.rb new file mode 100644 index 000000000..2bc7f1b2d --- /dev/null +++ b/spec/services/podcasts/create_episode_spec.rb @@ -0,0 +1,61 @@ +require "rails_helper" +require "rss" +require "rss/itunes" + +RSpec.describe Podcasts::CreateEpisode, type: :service do + let!(:podcast) { create(:podcast) } + + context "when item has an https media_url" do + let!(:item) { RSS::Parser.parse("spec/support/fixtures/developertea.rss", false).items.first } + + before do + stub_request(:head, item.enclosure.url).to_return(status: 200) + end + + it "creates an episode" do + expect do + described_class.call(podcast.id, item) + end.to change(PodcastEpisode, :count).by(1) + end + + it "creates an episode with correct data" do + episode = described_class.call(podcast.id, item) + expect(episode.title).to eq("Individual Contributor Career Growth w/ Matt Klein (part 1)") + expect(episode.podcast_id).to eq(podcast.id) + expect(episode.website_url).to eq("http://developertea.simplecast.fm/50464d4b") + expect(episode.guid).to include("53b17a1e-271b-40e3-a084-a67b4fcba562") + end + + it "rescues an exception when pubDate is invalid" do + allow(item).to receive(:pubDate).and_return("not a date, haha") + episode = described_class.call(podcast.id, item) + expect(episode).to be_persisted + expect(episode.published_at).to eq(nil) + end + end + + context "when item has an http media url" do + let!(:item) { RSS::Parser.parse("spec/support/fixtures/awayfromthekeyboard.rss", false).items.first } + let(:https_url) { "https://awayfromthekeyboard.com/wp-content/uploads/2018/02/Episode_075_Lara_Hogan_Demystifies_Public_Speaking.mp3" } + + it "sets media_url to https version when it is available" do + stub_request(:head, https_url).to_return(status: 200) + episode = described_class.call(podcast.id, item) + expect(episode.media_url).to eq(https_url) + end + + it "keeps an http media url when https version is not available" do + stub_request(:head, https_url).to_return(status: 404) + episode = described_class.call(podcast.id, item) + expect(episode.media_url).to eq(item.enclosure.url) + end + + # enable when the logic will not rely solely on exception + xit "sets status notice when https version is not available" do + stub_request(:head, https_url).to_return(status: 404) + described_class.call(podcast.id, item) + podcast.reload + expect(podcast.status_notice).to include("may not be playable") + end + end +end diff --git a/spec/labor/podcast_feed_spec.rb b/spec/services/podcasts/feed_spec.rb similarity index 75% rename from spec/labor/podcast_feed_spec.rb rename to spec/services/podcasts/feed_spec.rb index 2d128ca26..f24c75c4c 100644 --- a/spec/labor/podcast_feed_spec.rb +++ b/spec/services/podcasts/feed_spec.rb @@ -5,7 +5,7 @@ vcr_option = { allow_playback_repeats: "true" } -RSpec.describe PodcastFeed, vcr: vcr_option do +RSpec.describe Podcasts::Feed, vcr: vcr_option do let(:feed_url) { "http://softwareengineeringdaily.com/feed/podcast/" } let(:podcast) { create(:podcast, feed_url: feed_url) } @@ -14,14 +14,19 @@ RSpec.describe PodcastFeed, vcr: vcr_option do end context "when creating" do + before do + stub_request(:head, "https://traffic.libsyn.com/sedaily/AnalyseAsia.mp3").to_return(status: 200) + stub_request(:head, "https://traffic.libsyn.com/sedaily/IFTTT.mp3").to_return(status: 200) + end + it "fetches podcast episodes" do expect do - PodcastFeed.new.get_episodes(podcast, 2) + described_class.new.get_episodes(podcast, 2) end.to change(PodcastEpisode, :count).by(2) end it "fetches correct podcasts" do - PodcastFeed.new.get_episodes(podcast, 2) + described_class.new.get_episodes(podcast, 2) episodes = podcast.podcast_episodes expect(episodes.pluck(:title).sort).to eq(["Analyse Asia with Bernard Leong", "IFTTT Architecture with Nicky Leach"]) expect(episodes.pluck(:media_url).sort).to eq(%w[https://traffic.libsyn.com/sedaily/AnalyseAsia.mp3 https://traffic.libsyn.com/sedaily/IFTTT.mp3]) @@ -34,12 +39,12 @@ RSpec.describe PodcastFeed, vcr: vcr_option do it "does not refetch already fetched episodes" do expect do - PodcastFeed.new.get_episodes(podcast, 2) + described_class.new.get_episodes(podcast, 2) end.not_to change(PodcastEpisode, :count) end it "updates published_at for existing episodes" do - PodcastFeed.new.get_episodes(podcast, 2) + described_class.new.get_episodes(podcast, 2) episode.reload episode2.reload expect(episode.published_at).to be_truthy diff --git a/spec/services/podcasts/update_episode_spec.rb b/spec/services/podcasts/update_episode_spec.rb new file mode 100644 index 000000000..cc1d1b99f --- /dev/null +++ b/spec/services/podcasts/update_episode_spec.rb @@ -0,0 +1,38 @@ +require "rails_helper" + +RSpec.describe Podcasts::UpdateEpisode, type: :service do + let(:podcast) { create(:podcast) } + let(:enclosure) { instance_double("RSS::Rss::Channel::Item::Enclosure", url: "https://audio.simplecast.com/2330f132.mp3") } + let(:item) { instance_double("RSS::Rss::Channel::Item", pubDate: "2019-06-19", enclosure: enclosure) } + + it "updates published_at if it's nil" do + episode = create(:podcast_episode, podcast: podcast, published_at: nil) + described_class.call(episode, item) + episode.reload + expect(episode.published_at).to be_truthy + end + + it "updates media_url if the item url contains https" do + episode = create(:podcast_episode, podcast: podcast, media_url: "http://audio.simplecast.com/2330f132.mp3") + described_class.call(episode, item) + episode.reload + expect(episode.media_url).to eq("https://audio.simplecast.com/2330f132.mp3") + end + + it "catches expception when pubDate is invalid" do + invalid_item = instance_double("RSS::Rss::Channel::Item", pubDate: "i'm not a date", enclosure: enclosure) + + episode = create(:podcast_episode, podcast: podcast, published_at: nil) + described_class.call(episode, invalid_item) + expect(episode.published_at).to be_nil + end + + it "does fine when there's nothing to update" do + published_at = Time.current - 1.day + episode = create(:podcast_episode, podcast: podcast, media_url: "https://audio.simplecast.com/100.mp3", published_at: published_at) + described_class.call(episode, item) + episode.reload + expect(episode.media_url).to eq("https://audio.simplecast.com/100.mp3") + expect(episode.published_at.strftime("%Y-%m-%d")).to eq(published_at.strftime("%Y-%m-%d")) + end +end diff --git a/spec/support/fixtures/awayfromthekeyboard.rss b/spec/support/fixtures/awayfromthekeyboard.rss new file mode 100644 index 000000000..4318da49c --- /dev/null +++ b/spec/support/fixtures/awayfromthekeyboard.rss @@ -0,0 +1,51 @@ + + + + Away From The Keyboard + + http://awayfromthekeyboard.com + Away From The Keyboard is where technologists tell their stories of how they started, how they grew, how they learned, and how they unwind. Hosted by software developers Cecil Phillip and Richie Rump, Away From The Keyboard looks at the human side of technology in a fun and informative way. New episodes are released every Tuesday. + Sun, 30 Sep 2018 10:00:52 +0000 + en-us + ℗ & © 2019 Away From The Keyboard + A podcast exploring the other side of the technical community + Away From The Keyboard + + Away From The Keyboard + jorriss@gmail.com + + + + http://awayfromthekeyboard.com/wp-content/uploads/2015/04/Away_from_the_Keyboard_iTunes.jpg + Away From The Keyboard + http://awayfromthekeyboard.com + + no + + + + hourly + 1 + https://wordpress.org/?v=5.1.1 + 89028513 + + Episode 75: Lara Hogan Demystifies Public Speaking + The conversation starts with Lara discussing her start in technology which believe it or not started with Lord of the Rings, international studies, a semester in Prague, and photography. Lara then goes in and discusses how she started speaking publicly, how she prepares to for a presentation, and why she wrote the book "Demystifying Public Speaking". The panel then talks travel and their tips for traveling. Lara then shares about her trip to New Zealand and what she does when she's away from the keyboard. + + + http://awayfromthekeyboard.com/2018/02/28/episode-75-lara-hogan-demystifies-public-speaking/ + Wed, 28 Feb 2018 11:00:41 +0000 + Richie Rump + http://awayfromthekeyboard.com/?p=1092 + The conversation starts with Lara discussing her start in technology which believe it or not started with Lord of the Rings, international studies, a semester in Prague, and photography. Lara then goes in and discusses how she started speaking publicly, how she prepares to for a presentation, and why she wrote the book "Demystifying Public Speaking". The panel then talks travel and their tips for traveling. Lara then shares about her trip to New Zealand and what she does when she's away from the keyboard. + + The conversation starts with Lara discussing her start in technology which believe it or not started with Lord of the Rings, international studies, a semester in Prague, and photography. Lara then goes in and discusses how she started speaking publicly, how she prepares to for a presentation, and why she wrote the book "Demystifying Public Speaking". The panel then talks travel and their tips for traveling. Lara then shares about her trip to New Zealand and what she does when she's away from the keyboard. + + + 43:09 + No + Richie Rump + 1092 + + + \ No newline at end of file diff --git a/spec/support/fixtures/developertea.rss b/spec/support/fixtures/developertea.rss new file mode 100644 index 000000000..57576f7ec --- /dev/null +++ b/spec/support/fixtures/developertea.rss @@ -0,0 +1,100 @@ + + + + + Developer Tea + https://simplecast.com + Developer Tea exists to help driven developers connect to their ultimate purpose and excel at their work so that they can positively impact the people they influence. + +With over 13 million downloads to date, Developer Tea is a short podcast hosted by Jonathan Cutrell (@jcutrell), co-founder of Spec and developer at @Clearbit. We hope you'll take the topics from this podcast and continue the conversation, either online or in person with your peers. Twitter: @developertea :: Email: developertea@gmail.com + © 2018 Spec Network, Inc. + en-us + Wed, 19 Jun 2019 05:00:00 -0400 + Wed, 19 Jun 2019 19:24:51 -0400 + http://www.developertea.com + + https://media.simplecast.com/podcast/image/363/1471485029-artwork.jpg + Developer Tea + http://www.developertea.com + + https://rss.simplecast.com/podcasts/363/rss + Spec + + Developer Tea exists to help driven developers connect to their ultimate purpose and excel at their work so that they can positively impact the people they influence. + +With over 13 million downloads to date, Developer Tea is a short podcast hosted by Jonathan Cutrell (@jcutrell), co-founder of Spec and developer at @Clearbit. We hope you'll take the topics from this podcast and continue the conversation, either online or in person with your peers. Twitter: @developertea :: Email: developertea@gmail.com + Developer Tea exists to help driven developers connect to their ultimate purpose and excel at their work so that they can positively impact the people they influence. + no + short podcasts for developers, development, programming, web development + episodic + + Spec Network, Inc. + shows@spec.fm + + + + + + + + Individual Contributor Career Growth w/ Matt Klein (part 1) + 53b17a1e-271b-40e3-a084-a67b4fcba562 + http://developertea.simplecast.fm/50464d4b + What does a long career as an individual contributor look like? The answer isn't always clear cut, especially if you're give the option of becoming a manager. Today, we'll talk to Matt Klein about how he approaches that. + + + What does a long career as an individual contributor look like? The answer isn't always clear cut, especially if you're given the option of becoming a manager. Today, we'll talk to Matt Klein about how he approaches this in part 1 of our two part interview.

+ + +

Matt on the Web

+ +

Matt is on the engineering team at Lyft and one of the main contributors to the open source project, Envoy.

+ + + + + +

Get in touch

+ +

If you have questions about today's episode, want to start a conversation about today's topic or just want to let us know if you found this episode valuable I encourage you to join the conversation or start your own on our community platform Spectrum.chat/specfm/developer-tea

+ + +

🧡 Leave a Review

+ +

If you're enjoying the show and want to support the content head over to iTunes and leave a review! It helps other developers discover the show and keep us focused on what matters to you.

+ + +

🍵 Subscribe to the Tea Break Challenge

+ +

This is a daily challenge designed help you become more self-aware and be a better developer so you can have a positive impact on the people around you. Check it out and give it a try at https://www.teabreakchallenge.com/.

+ + +

🙏 Thanks to today's sponsor: Sentry

+ +

Sentry tells you about errors in your code before your customers have a chance to encounter them.

+ +

Not only do we tell you about them, we also give you all the details you’ll need to be able to fix them. You’ll see exactly how many users have been impacted by a bug, the stack trace, the commit that the error was released as part of, the engineer who wrote the line of code that is currently busted, and a lot more.

+ +

Give it a try for yourself at Sentry.io

+]]> + + Wed, 19 Jun 2019 05:00:00 -0400 + shows@spec.fm (Spec) + + Spec + + 00:31:35 + What does a long career as an individual contributor look like? The answer isn't always clear cut, especially if you're give the option of becoming a manager. Today, we'll talk to Matt Klein about how he approaches that. + + What does a long career as an individual contributor look like? The answer isn't always clear cut, especially if you're give the option of becoming a manager. Today, we'll talk to Matt Klein about how he approaches that. + + + no + full + 696 + + +