diff --git a/.gitignore b/.gitignore index fef6d0b6..e4059f03 100644 --- a/.gitignore +++ b/.gitignore @@ -4,6 +4,7 @@ /log/*.log /tmp /config/*.yml +!/config/schedule.yml /.rvmrc /coverage **.orig diff --git a/Gemfile b/Gemfile index cc1348e6..5adabda4 100644 --- a/Gemfile +++ b/Gemfile @@ -29,7 +29,7 @@ gem 'redis', '~> 3.3.0' gem 'restpack_serializer', git: 'https://github.com/zooniverse/restpack_serializer.git', branch: 'talk-api-version', ref: '637aaaf85e' gem 'sidekiq', '< 6' gem 'sidekiq-congestion', '~> 0.1.0' -gem 'sidetiq', '~> 0.7.2' +gem 'sidekiq-cron', '<=1.8.0' # can remove this version constraint once we are on Rails 5+ gem 'zoo_stream', '~> 1.0' group :test, :development do @@ -51,6 +51,7 @@ end group :test do gem 'codeclimate-test-reporter', '~> 0.5' + gem 'mock_redis' gem 'simplecov', '~> 0.11.2' gem 'webmock', '~> 3.4' end diff --git a/Gemfile.lock b/Gemfile.lock index 9f26b453..71b85b60 100644 --- a/Gemfile.lock +++ b/Gemfile.lock @@ -57,23 +57,6 @@ GEM aws-sdk-core (= 2.3.7) benchmark-ips (2.6.1) builder (3.2.4) - celluloid (0.17.3) - celluloid-essentials - celluloid-extras - celluloid-fsm - celluloid-pool - celluloid-supervision - timers (>= 4.1.1) - celluloid-essentials (0.20.5) - timers (>= 4.1.1) - celluloid-extras (0.20.5) - timers (>= 4.1.1) - celluloid-fsm (0.20.5) - timers (>= 4.1.1) - celluloid-pool (0.20.5) - timers (>= 4.1.1) - celluloid-supervision (0.20.6) - timers (>= 4.1.1) codeclimate-test-reporter (0.5.0) simplecov (>= 0.7.1, < 1.0.0) coderay (1.1.2) @@ -91,6 +74,8 @@ GEM domain_name (0.5.20190701) unf (>= 0.0.5, < 1.0.0) erubis (2.7.0) + et-orbi (1.2.11) + tzinfo factory_girl (4.7.0) activesupport (>= 3.0.0) factory_girl_rails (4.7.0) @@ -102,6 +87,9 @@ GEM faraday (>= 0.7.4, < 1.0) ffi (1.9.25) formatador (0.2.5) + fugit (1.11.0) + et-orbi (~> 1, >= 1.2.11) + raabro (~> 1.4) globalid (0.4.2) activesupport (>= 4.2.0) guard (2.14.0) @@ -119,14 +107,12 @@ GEM guard-compat (~> 1.1) rspec (>= 2.99.0, < 4.0) hashdiff (0.3.7) - hitimes (1.3.0) honeybadger (4.5.6) http-accept (1.7.0) http-cookie (1.0.5) domain_name (~> 0.5) i18n (0.9.5) concurrent-ruby (~> 1.0) - ice_cube (0.14.0) its-it (1.3.0) jmespath (1.6.1) json (1.8.6) @@ -171,6 +157,8 @@ GEM mini_mime (1.1.2) mini_portile2 (2.6.1) minitest (5.14.4) + mock_redis (0.36.0) + ruby2_keywords modware (0.1.3) key_struct (~> 0.4) multipart-post (2.1.1) @@ -193,6 +181,7 @@ GEM nio4r (~> 2.0) pundit (1.1.0) activesupport (>= 3.0.0) + raabro (1.4.0) racc (1.6.0) rack (1.6.13) rack-cors (1.0.6) @@ -260,6 +249,7 @@ GEM rspec-mocks (~> 3.4.0) rspec-support (~> 3.4.0) rspec-support (3.4.1) + ruby2_keywords (0.0.5) ruby_dep (1.3.1) safe_yaml (1.0.4) schema_monkey (2.1.5) @@ -287,10 +277,9 @@ GEM sidekiq-congestion (0.1.1) congestion (~> 0.1) sidekiq (>= 3.0) - sidetiq (0.7.2) - celluloid (>= 0.17.3) - ice_cube (~> 0.14.0) - sidekiq (>= 4.1.0) + sidekiq-cron (1.4.0) + fugit (~> 1) + sidekiq (>= 4.2.1) simplecov (0.11.2) docile (~> 1.1.0) json (~> 1.8) @@ -313,9 +302,7 @@ GEM rest-client (>= 2.0.2) thor (1.1.0) thread_safe (0.3.6) - timecop (0.9.8) - timers (4.1.2) - hitimes + timecop (0.8.1) tzinfo (1.2.10) thread_safe (~> 0.1) unf (0.1.4) @@ -344,6 +331,7 @@ DEPENDENCIES json-schema (~> 2.8) json-schema_builder (~> 0.0.8) logstasher (~> 0.9.0) + mock_redis newrelic_rpm pg (~> 0.21) pry (~> 0.11.3) @@ -358,7 +346,7 @@ DEPENDENCIES schema_plus_pg_indexes (~> 0.1.12) sidekiq (< 6) sidekiq-congestion (~> 0.1.0) - sidetiq (~> 0.7.2) + sidekiq-cron (<= 1.8.0) simplecov (~> 0.11.2) spring (~> 1.7.1) spring-commands-rspec (~> 1.0.4) diff --git a/Gemfile.next.lock b/Gemfile.next.lock index 4f3b8415..7d9c5bb6 100644 --- a/Gemfile.next.lock +++ b/Gemfile.next.lock @@ -62,8 +62,6 @@ GEM benchmark-ips (2.13.0) bigdecimal (3.1.8) builder (3.2.4) - celluloid (0.18.0) - timers (~> 4) codeclimate-test-reporter (0.6.0) simplecov (>= 0.7.1, < 1.0.0) coderay (1.1.3) @@ -83,6 +81,8 @@ GEM domain_name (0.5.20190701) unf (>= 0.0.5, < 1.0.0) erubis (2.7.0) + et-orbi (1.2.11) + tzinfo factory_girl (4.7.0) activesupport (>= 3.0.0) factory_girl_rails (4.7.0) @@ -115,6 +115,9 @@ GEM faraday (~> 1.0) ffi (1.16.3) formatador (1.1.0) + fugit (1.11.0) + et-orbi (~> 1, >= 1.2.11) + raabro (~> 1.4) globalid (1.1.0) activesupport (>= 5.0) guard (2.14.2) @@ -138,7 +141,6 @@ GEM domain_name (~> 0.5) i18n (1.14.5) concurrent-ruby (~> 1.0) - ice_cube (0.14.0) its-it (1.3.0) jmespath (1.6.2) json (1.8.6) @@ -185,6 +187,8 @@ GEM mini_mime (1.1.5) mini_portile2 (2.8.6) minitest (5.23.0) + mock_redis (0.36.0) + ruby2_keywords modware (0.1.3) key_struct (~> 0.4) multipart-post (2.4.1) @@ -216,6 +220,7 @@ GEM nio4r (~> 2.0) pundit (1.1.0) activesupport (>= 3.0.0) + raabro (1.4.0) racc (1.8.0) rack (2.2.9) rack-cors (1.0.6) @@ -313,10 +318,9 @@ GEM sidekiq-congestion (0.1.1) congestion (~> 0.1) sidekiq (>= 3.0) - sidetiq (0.7.2) - celluloid (>= 0.17.3) - ice_cube (~> 0.14.0) - sidekiq (>= 4.1.0) + sidekiq-cron (1.8.0) + fugit (~> 1) + sidekiq (>= 4.2.1) simplecov (0.11.2) docile (~> 1.1.0) json (~> 1.8) @@ -343,7 +347,6 @@ GEM thread_safe (0.3.6) timecop (0.9.8) timeout (0.4.1) - timers (4.3.5) tzinfo (1.2.11) thread_safe (~> 0.1) unf (0.1.4) @@ -375,6 +378,7 @@ DEPENDENCIES json-schema (~> 2.8) json-schema_builder (~> 0.0.8) logstasher (~> 0.9.0) + mock_redis newrelic_rpm pg (~> 0.21) pry (~> 0.11.3) @@ -389,7 +393,7 @@ DEPENDENCIES schema_plus_pg_indexes (~> 0.2.1) sidekiq (< 6) sidekiq-congestion (~> 0.1.0) - sidetiq (~> 0.7.2) + sidekiq-cron (<= 1.8.0) simplecov (~> 0.11.2) spring (~> 2.0.2) spring-commands-rspec (~> 1.0.4) diff --git a/app/workers/concerns/digest_email_worker.rb b/app/workers/concerns/digest_email_worker.rb index 8e061c29..8f7975e5 100644 --- a/app/workers/concerns/digest_email_worker.rb +++ b/app/workers/concerns/digest_email_worker.rb @@ -3,7 +3,6 @@ module DigestEmailWorker included do include Sidekiq::Worker - include Sidetiq::Schedulable sidekiq_options retry: false, backtrace: true diff --git a/app/workers/concerns/expiry_worker.rb b/app/workers/concerns/expiry_worker.rb index 53db1788..f429ed9e 100644 --- a/app/workers/concerns/expiry_worker.rb +++ b/app/workers/concerns/expiry_worker.rb @@ -3,14 +3,12 @@ module ExpiryWorker included do include Sidekiq::Worker - include Sidetiq::Schedulable class << self attr_accessor :model end sidekiq_options retry: false, backtrace: true - recurrence{ hourly } end def perform diff --git a/app/workers/daily_digest_email_worker.rb b/app/workers/daily_digest_email_worker.rb index d9d590c0..d412b9d7 100644 --- a/app/workers/daily_digest_email_worker.rb +++ b/app/workers/daily_digest_email_worker.rb @@ -1,8 +1,4 @@ class DailyDigestEmailWorker include DigestEmailWorker self.frequency = :daily - - recurrence do - daily.hour_of_day 6 - end end diff --git a/app/workers/weekly_digest_email_worker.rb b/app/workers/weekly_digest_email_worker.rb index 678ddae8..64e0b6c2 100644 --- a/app/workers/weekly_digest_email_worker.rb +++ b/app/workers/weekly_digest_email_worker.rb @@ -1,8 +1,4 @@ class WeeklyDigestEmailWorker include DigestEmailWorker self.frequency = :weekly - - recurrence do - weekly.day(:monday).hour_of_day 12 - end end diff --git a/config/initializers/sidekiq.rb b/config/initializers/sidekiq.rb index 3be80ac8..49f481f5 100644 --- a/config/initializers/sidekiq.rb +++ b/config/initializers/sidekiq.rb @@ -13,6 +13,12 @@ def self.redis_url config.server_middleware do |chain| chain.add Sidekiq::Congestion::Limiter end + + # Sidekiq-cron: loads recurring jobs from config/schedule.yml + schedule_file = 'config/schedule.yml' + if File.exist?(schedule_file) + Sidekiq::Cron::Job.load_from_hash YAML.load_file(schedule_file) + end end require 'sidekiq/web' @@ -23,13 +29,6 @@ def self.redis_url password == ENV.fetch('SIDEKIQ_ADMIN_PASSWORD') end unless Rails.env.test? || Rails.env.development? -require 'sidetiq' -Sidetiq.configure do |config| - config.utc = true -end - -require 'sidetiq/web' - # preload autoloaded workers Dir[Rails.root.join('app/workers/**/*.rb')].sort.each do |path| name = path.match(/workers\/(.+)\.rb$/)[1] diff --git a/config/routes.rb b/config/routes.rb index a230d16d..b00431ed 100644 --- a/config/routes.rb +++ b/config/routes.rb @@ -1,4 +1,5 @@ require 'sidekiq/web' +require 'sidekiq/cron/web' Rails.application.routes.draw do defaults format: 'json' do diff --git a/config/schedule.yml b/config/schedule.yml new file mode 100644 index 00000000..ba67177a --- /dev/null +++ b/config/schedule.yml @@ -0,0 +1,36 @@ +# Here is a list of jobs that are scheduled to run periodically. +# We use a UNIX cron notation to specify execution schedule. +# +# Please read here for more information: +# https://github.com/ondrejbartas/sidekiq-cron#adding-cron-job + +announcement_expiry_worker: + cron: "0 * * * *" + class: "AnnouncementExpiryWorker" + queue: "default" + description: "Hourly worker that destroys all expired Announcements. Announcements expires_at set to 1 month from creation" +daily_digest_email_worker: + cron: "0 6 * * *" + class: "DailyDigestEmailWorker" + queue: "default" + description: "Daily worker that sends notifications to users who have a Daily Subscription Preference. Runs at 6th hour of the day." +data_request_expiry_worker: + cron: "0 * * * *" + class: "DataRequestExpiryWorker" + queue: "default" + description: "Hourly worker that destroys all expired Data Requests. Data Request expires_at is set to 1 day from request creation." +notification_expiry_worker: + cron: "0 * * * *" + class: "NotificationExpiryWorker" + queue: "default" + description: "Hourly worker that destroys all expired Notifications. Notifications expires in 3 months of creation" +unsubscribe_token_expiry_worker: + cron: "0 * * * *" + class: "UnsubscribeTokenExpiryWorker" + queue: "default" + description: "Hourly worker that destroys all unsubscribe_tokens. Unsubscribe_token expires_at set to 1 month of creation" +weekly_digest_expiry_worker: + cron: "0 12 * * 1" + class: "WeeklyDigestEmailWorker" + queue: "default" + description: "Weekly worker that sends notifications to users who have a subscription preference set to weekly. Runs on Mondays on the 12th hour of the day." \ No newline at end of file diff --git a/spec/spec_helper.rb b/spec/spec_helper.rb index c28999af..ef836a43 100644 --- a/spec/spec_helper.rb +++ b/spec/spec_helper.rb @@ -31,10 +31,22 @@ RSpec.configure do |config| config.include FactoryGirl::Syntax::Methods config.include JSON::SchemaBuilder::RSpecHelper, type: :schema + MOCK_REDIS ||= MockRedis.new config.before(:suite){ WebMock.disable_net_connect! } config.after(:suite){ WebMock.allow_net_connect! } + config.before(:each) do |example| + allow(Redis).to receive(:new).and_return(MOCK_REDIS) + + MOCK_REDIS.keys.each do |key| + MOCK_REDIS.del(key) + end + + # Clears out the jobs for tests using the fake testing + Sidekiq::Worker.clear_all + end + config.expect_with :rspec do |expectations| expectations.include_chain_clauses_in_custom_matcher_descriptions = true end diff --git a/spec/support/shared_examples_for_expirable.rb b/spec/support/shared_examples_for_expirable.rb index 31e2031e..9264f99b 100644 --- a/spec/support/shared_examples_for_expirable.rb +++ b/spec/support/shared_examples_for_expirable.rb @@ -18,9 +18,17 @@ RSpec.shared_examples_for 'an expiry worker' do |model:| it{ is_expected.to be_a Sidekiq::Worker } - describe 'schedule' do - subject{ described_class.schedule.to_s } - it{ is_expected.to eql 'Hourly' } + describe 'hourly schedule' do + it_behaves_like 'is schedulable' do + let(:cron_sched) { '0 * * * *' } + let(:enqueue_time) { Fugit::Cron.parse(cron_sched).previous_time } + let(:class_name) { described_class.name } + let(:enqueued_times) { + [ + enqueue_time.utc + ] + } + end end describe '.model' do diff --git a/spec/support/shared_examples_for_schedulable.rb b/spec/support/shared_examples_for_schedulable.rb new file mode 100644 index 00000000..f4f151e9 --- /dev/null +++ b/spec/support/shared_examples_for_schedulable.rb @@ -0,0 +1,17 @@ +shared_examples 'is schedulable' do + let(:job) { Sidekiq::Cron::Job.new(name: 'cron_job_name', cron: cron_sched, class: class_name) } + + it 'gets queued on enqueued_times' do + enqueued_times.each do |enqueued_time| + # update to formatted_enqueue_time when updating sidekiq-cron to 1.9+ (sidekiq 7 support) + job_enqueue_time = Time.at(job.formated_enqueue_time.to_f).utc + expect(job_enqueue_time).to eq(enqueued_time) + end + end + + it 'does not get enqueued outside of enqueued_times' do + outside_time = enqueued_times.first + 3 * 60 + + expect(job.should_enque?(outside_time)).to be false + end +end \ No newline at end of file diff --git a/spec/workers/daily_digest_email_worker_spec.rb b/spec/workers/daily_digest_email_worker_spec.rb index c19b2e60..86a477a0 100644 --- a/spec/workers/daily_digest_email_worker_spec.rb +++ b/spec/workers/daily_digest_email_worker_spec.rb @@ -2,5 +2,14 @@ RSpec.describe DailyDigestEmailWorker, type: :worker do it_behaves_like 'a digest email worker', scope: :daily - its('class.schedule.to_s'){ is_expected.to eql 'Daily on the 6th hour of the day' } + it_behaves_like 'is schedulable' do + let(:cron_sched) { '0 6 * * *' } + let(:enqueue_time) { Fugit::Cron.parse(cron_sched).previous_time } + let(:class_name) { described_class.name } + let(:enqueued_times) { + [ + enqueue_time.utc + ] + } + end end diff --git a/spec/workers/weekly_digest_email_worker_spec.rb b/spec/workers/weekly_digest_email_worker_spec.rb index 98c66ad3..a5670b2e 100644 --- a/spec/workers/weekly_digest_email_worker_spec.rb +++ b/spec/workers/weekly_digest_email_worker_spec.rb @@ -2,5 +2,14 @@ RSpec.describe WeeklyDigestEmailWorker, type: :worker do it_behaves_like 'a digest email worker', scope: :weekly - its('class.schedule.to_s'){ is_expected.to eql 'Weekly on Mondays on the 12th hour of the day' } + it_behaves_like 'is schedulable' do + let(:cron_sched) { '0 12 * * 1' } + let(:enqueue_time) { Fugit::Cron.parse(cron_sched).previous_time } + let(:class_name) { described_class.name } + let(:enqueued_times) { + [ + enqueue_time.utc + ] + } + end end