From 2e635f94f626c9b1dcc1f7614e2fbef97449505a Mon Sep 17 00:00:00 2001 From: Maikel Linke Date: Thu, 13 Sep 2018 17:18:03 +1000 Subject: [PATCH 1/3] Make job queuing more robust and efficient --- .../products_cache_refreshment.rb | 26 +++++++------------ .../products_cache_refreshment_spec.rb | 1 + 2 files changed, 11 insertions(+), 16 deletions(-) diff --git a/lib/open_food_network/products_cache_refreshment.rb b/lib/open_food_network/products_cache_refreshment.rb index 276734570f9..17efd2acd52 100644 --- a/lib/open_food_network/products_cache_refreshment.rb +++ b/lib/open_food_network/products_cache_refreshment.rb @@ -17,31 +17,25 @@ module OpenFoodNetwork class ProductsCacheRefreshment def self.refresh(distributor, order_cycle) - unless pending_job? distributor, order_cycle - enqueue_job distributor, order_cycle - end + job = refresh_job(distributor, order_cycle) + enqueue_job(job) unless pending_job?(job) end - private - def self.pending_job?(distributor, order_cycle) - # To inspect each job, we need to deserialize the payload. - # This is slow, and if it's a problem in practice, we could pre-filter in SQL - # for handlers matching the class name, distributor id and order cycle id. + def self.refresh_job(distributor, order_cycle) + RefreshProductsCacheJob.new(distributor.id, order_cycle.id) + end + def self.pending_job?(job) Delayed::Job. where(locked_at: nil). - map(&:payload_object). - select { |j| - j.class == RefreshProductsCacheJob && - j.distributor_id == distributor.id && - j.order_cycle_id == order_cycle.id - }.any? + where(handler: job.to_yaml). + exists? end - def self.enqueue_job(distributor, order_cycle) - Delayed::Job.enqueue RefreshProductsCacheJob.new(distributor.id, order_cycle.id), priority: 10 + def self.enqueue_job(job) + Delayed::Job.enqueue job, priority: 10 end end end diff --git a/spec/lib/open_food_network/products_cache_refreshment_spec.rb b/spec/lib/open_food_network/products_cache_refreshment_spec.rb index f65b81346e7..74998928d5f 100644 --- a/spec/lib/open_food_network/products_cache_refreshment_spec.rb +++ b/spec/lib/open_food_network/products_cache_refreshment_spec.rb @@ -1,3 +1,4 @@ +require 'spec_helper' require 'open_food_network/products_cache_refreshment' module OpenFoodNetwork From ec953e1db0a8f704bd5fbde5d50a06b4dd967a91 Mon Sep 17 00:00:00 2001 From: Maikel Linke Date: Thu, 13 Sep 2018 17:22:01 +1000 Subject: [PATCH 2/3] Style cache refreshment class --- .rubocop_todo.yml | 6 ---- .../products_cache_refreshment.rb | 32 ++++++++++--------- 2 files changed, 17 insertions(+), 21 deletions(-) diff --git a/.rubocop_todo.yml b/.rubocop_todo.yml index eaff617a863..7798d41309e 100644 --- a/.rubocop_todo.yml +++ b/.rubocop_todo.yml @@ -185,7 +185,6 @@ Layout/EmptyLines: - 'lib/open_food_network/order_cycle_permissions.rb' - 'lib/open_food_network/products_cache.rb' - 'lib/open_food_network/products_cache_integrity_checker.rb' - - 'lib/open_food_network/products_cache_refreshment.rb' - 'lib/open_food_network/products_renderer.rb' - 'lib/open_food_network/property_merge.rb' - 'lib/open_food_network/reports/bulk_coop_report.rb' @@ -560,7 +559,6 @@ Layout/MultilineOperationIndentation: - 'app/models/variant_override_set.rb' - 'lib/open_food_network/accounts_and_billing_settings_validator.rb' - 'lib/open_food_network/order_cycle_permissions.rb' - - 'lib/open_food_network/products_cache_refreshment.rb' - 'lib/open_food_network/sales_tax_report.rb' - 'lib/open_food_network/users_and_enterprises_report.rb' @@ -986,7 +984,6 @@ Lint/IneffectiveAccessModifier: - 'app/models/variant_override.rb' - 'lib/open_food_network/feature_toggle.rb' - 'lib/open_food_network/products_cache.rb' - - 'lib/open_food_network/products_cache_refreshment.rb' - 'lib/open_food_network/property_merge.rb' - 'spec/lib/open_food_network/reports/report_spec.rb' @@ -1096,7 +1093,6 @@ Lint/UselessAccessModifier: - 'app/models/column_preference.rb' - 'lib/open_food_network/feature_toggle.rb' - 'lib/open_food_network/products_cache.rb' - - 'lib/open_food_network/products_cache_refreshment.rb' - 'lib/open_food_network/property_merge.rb' - 'lib/open_food_network/reports/bulk_coop_report.rb' - 'spec/lib/open_food_network/reports/report_spec.rb' @@ -1505,7 +1501,6 @@ Rails/TimeZone: - 'lib/open_food_network/users_and_enterprises_report.rb' - 'spec/controllers/api/statuses_controller_spec.rb' - 'spec/jobs/heartbeat_job_spec.rb' - - 'spec/lib/open_food_network/products_cache_refreshment_spec.rb' - 'spec/lib/open_food_network/products_cache_spec.rb' - 'spec/models/enterprise_relationship_spec.rb' - 'spec/models/variant_override_spec.rb' @@ -1861,7 +1856,6 @@ Style/GuardClause: - 'lib/open_food_network/accounts_and_billing_settings_validator.rb' - 'lib/open_food_network/order_cycle_form_applicator.rb' - 'lib/open_food_network/products_cache.rb' - - 'lib/open_food_network/products_cache_refreshment.rb' - 'lib/open_food_network/products_renderer.rb' - 'lib/open_food_network/rack_request_blocker.rb' - 'lib/open_food_network/variant_and_line_item_naming.rb' diff --git a/lib/open_food_network/products_cache_refreshment.rb b/lib/open_food_network/products_cache_refreshment.rb index 17efd2acd52..e9ebc670291 100644 --- a/lib/open_food_network/products_cache_refreshment.rb +++ b/lib/open_food_network/products_cache_refreshment.rb @@ -21,21 +21,23 @@ def self.refresh(distributor, order_cycle) enqueue_job(job) unless pending_job?(job) end - private - - def self.refresh_job(distributor, order_cycle) - RefreshProductsCacheJob.new(distributor.id, order_cycle.id) - end - - def self.pending_job?(job) - Delayed::Job. - where(locked_at: nil). - where(handler: job.to_yaml). - exists? - end - - def self.enqueue_job(job) - Delayed::Job.enqueue job, priority: 10 + class << self + private + + def refresh_job(distributor, order_cycle) + RefreshProductsCacheJob.new(distributor.id, order_cycle.id) + end + + def pending_job?(job) + Delayed::Job. + where(locked_at: nil). + where(handler: job.to_yaml). + exists? + end + + def enqueue_job(job) + Delayed::Job.enqueue job, priority: 10 + end end end end From ac85b9031573fae1f84b060e9381c9bfa2878e7f Mon Sep 17 00:00:00 2001 From: Maikel Linke Date: Sat, 15 Sep 2018 10:18:54 +1000 Subject: [PATCH 3/3] Clarify private class method declaration --- .../products_cache_refreshment.rb | 29 +++++++++---------- 1 file changed, 14 insertions(+), 15 deletions(-) diff --git a/lib/open_food_network/products_cache_refreshment.rb b/lib/open_food_network/products_cache_refreshment.rb index e9ebc670291..3bcde00f8de 100644 --- a/lib/open_food_network/products_cache_refreshment.rb +++ b/lib/open_food_network/products_cache_refreshment.rb @@ -21,23 +21,22 @@ def self.refresh(distributor, order_cycle) enqueue_job(job) unless pending_job?(job) end - class << self - private - - def refresh_job(distributor, order_cycle) - RefreshProductsCacheJob.new(distributor.id, order_cycle.id) - end + def self.refresh_job(distributor, order_cycle) + RefreshProductsCacheJob.new(distributor.id, order_cycle.id) + end + private_class_method :refresh_job - def pending_job?(job) - Delayed::Job. - where(locked_at: nil). - where(handler: job.to_yaml). - exists? - end + def self.pending_job?(job) + Delayed::Job. + where(locked_at: nil). + where(handler: job.to_yaml). + exists? + end + private_class_method :pending_job? - def enqueue_job(job) - Delayed::Job.enqueue job, priority: 10 - end + def self.enqueue_job(job) + Delayed::Job.enqueue job, priority: 10 end + private_class_method :enqueue_job end end