From 47e8fbc9ec251c82ad825a2b4a1e93549b48c140 Mon Sep 17 00:00:00 2001 From: Ben Halpern Date: Tue, 15 Jan 2019 13:29:11 -0500 Subject: [PATCH] Add feed_fetched_at for users to not refetch unnecessarily (#1555) * Fix RSS issues by not refetching users as often * Make rss fetch not forced in rake task * Remove unnecesary schema line --- app/services/rss_reader.rb | 5 ++++- ...90115155656_add_feed_fetched_at_to_users.rb | 5 +++++ db/schema.rb | 3 ++- lib/tasks/fetch.rake | 2 +- spec/services/rss_reader_spec.rb | 18 ++++++++++++++++++ 5 files changed, 30 insertions(+), 3 deletions(-) create mode 100644 db/migrate/20190115155656_add_feed_fetched_at_to_users.rb diff --git a/app/services/rss_reader.rb b/app/services/rss_reader.rb index 1ce817719..51044ed21 100644 --- a/app/services/rss_reader.rb +++ b/app/services/rss_reader.rb @@ -11,8 +11,10 @@ class RssReader @request_id = request_id end - def get_all_articles + def get_all_articles(force = true) User.where.not(feed_url: [nil, ""]).find_each do |user| + next if force == false && (rand(2) == 1 || user.feed_fetched_at > 15.minutes.ago) # Don't fetch every time. + create_articles_for_user(user) end end @@ -33,6 +35,7 @@ class RssReader def create_articles_for_user(user) with_span("create_articles_for_user", user_id: user.id, username: user.username) do |metadata| + user.update_column(:feed_fetched_at, Time.current) feed = fetch_rss(user.feed_url.strip) metadata[:feed_length] = feed.entries.length if feed&.entries feed.entries.reverse_each do |item| diff --git a/db/migrate/20190115155656_add_feed_fetched_at_to_users.rb b/db/migrate/20190115155656_add_feed_fetched_at_to_users.rb new file mode 100644 index 000000000..2ca2947db --- /dev/null +++ b/db/migrate/20190115155656_add_feed_fetched_at_to_users.rb @@ -0,0 +1,5 @@ +class AddFeedFetchedAtToUsers < ActiveRecord::Migration[5.1] + def change + add_column :users, :feed_fetched_at, :datetime, default: "2017-01-01 05:00:00" + end +end diff --git a/db/schema.rb b/db/schema.rb index 432b61f6c..5215b655d 100644 --- a/db/schema.rb +++ b/db/schema.rb @@ -10,7 +10,7 @@ # # It's strongly recommended that you check this file into your version control system. -ActiveRecord::Schema.define(version: 20190109212351) do +ActiveRecord::Schema.define(version: 20190115155656) do # These are extensions that must be enabled in order to support this database enable_extension "plpgsql" @@ -744,6 +744,7 @@ ActiveRecord::Schema.define(version: 20190109212351) do t.datetime "exported_at" t.string "facebook_url" t.boolean "feed_admin_publish_permission", default: true + t.datetime "feed_fetched_at", default: "2017-01-01 05:00:00" t.boolean "feed_mark_canonical", default: false t.string "feed_url" t.integer "following_orgs_count", default: 0, null: false diff --git a/lib/tasks/fetch.rake b/lib/tasks/fetch.rake index 42c6ff1a9..779f702a0 100644 --- a/lib/tasks/fetch.rake +++ b/lib/tasks/fetch.rake @@ -24,7 +24,7 @@ end task fetch_all_rss: :environment do Rails.application.eager_load! - RssReader.get_all_articles + RssReader.get_all_articles(false) # False means don't force fetch. Fetch "random" subset instead of all of them. end task resave_supported_tags: :environment do diff --git a/spec/services/rss_reader_spec.rb b/spec/services/rss_reader_spec.rb index 101258584..de9f40ca8 100644 --- a/spec/services/rss_reader_spec.rb +++ b/spec/services/rss_reader_spec.rb @@ -38,6 +38,24 @@ RSpec.describe RssReader, vcr: vcr_option do end end + it "sets time current" do + described_class.new.get_all_articles + expect(User.find_by(feed_url: nonpermanent_link).feed_fetched_at).to be > 2.minutes.ago + end + + it "does not refetch same user over and over" do + user = User.find_by(feed_url: nonpermanent_link) + user.update_column(:feed_fetched_at, Time.current) + fetched_at_time = user.feed_fetched_at + sleep(1) + described_class.new.get_all_articles + described_class.new.get_all_articles + described_class.new.get_all_articles + described_class.new.get_all_articles + described_class.new.get_all_articles + expect(user.feed_fetched_at).to eq(fetched_at_time) + end + it "gets articles for user" do # the result within the approval file depends on the feed described_class.new.fetch_user(User.first)