Add regions to daily cache warming (#1917)

* Cache regions in the warmer as well

* Move cache_key behavior to Region

this should get the cache keys consistent between a facility and
district group and their associated Regions, because they should have
the same data and therefore the same cache keys

* Fix test

* linting

* looser expectation

* delegate cache_version also

app/models/facility.rb

@@ -99,6 +99,8 @@ class Facility < ApplicationRecord
   delegate :protocol, to: :facility_group, allow_nil: true
   delegate :organization, :organization_id, to: :facility_group, allow_nil: true
   delegate :follow_ups_by_period, to: :patients, prefix: :patient
+  delegate :district_region?, :block_region?, :facility_region?, to: :region
+  delegate :cache_key, :cache_version, to: :region
 
   def self.parse_facilities_from_file(file_contents)
     Csv::FacilitiesParser.parse(file_contents)
@@ -207,8 +209,6 @@ def discardable?
     registered_patients.none? && blood_pressures.none? && blood_sugars.none? && appointments.none?
   end
 
-  delegate :district_region?, :block_region?, :facility_region?, to: :region
-
   def valid_block
     unless facility_group.region.block_regions.pluck(:name).include?(block)
       errors.add(:zone, "not present in the facility group")

app/models/facility_district.rb

@@ -61,6 +61,14 @@ def region
     self
   end
 
+  def cache_key
+    ["facility_districts", id].join("/")
+  end
+
+  def cache_version
+    updated_at.utc.to_s(:usec)
+  end
+
   # For regions compatibility
   def facility_region?
     false

app/models/facility_group.rb

@@ -29,6 +29,10 @@ class FacilityGroup < ApplicationRecord
   auto_strip_attributes :name, squish: true, upcase_first: true
   attribute :enable_diabetes_management, :boolean
 
+  # For Regions compatibility
+  delegate :district_region?, :block_region?, :facility_region?, to: :region
+  delegate :cache_key, :cache_version, to: :region
+
   # FacilityGroups don't actually have a state
   # This virtual attr exists simply to simulate the State -> FG/District hierarchy for Regions.
   attr_writer :state
@@ -59,9 +63,6 @@ def create_state_region!
     Region.state_regions.create!(name: state, reparent_to: organization.region)
   end
 
-  # For Regions compatibility
-  delegate :district_region?, :block_region?, :facility_region?, to: :region
-
   def child_region_type
     "facility"
   end

app/models/region.rb

@@ -151,6 +151,14 @@ def region
     self
   end
 
+  def cache_key
+    [model_name.cache_key, region_type, id].join("/")
+  end
+
+  def cache_version
+    updated_at.utc.to_s(:usec)
+  end
+
   private
 
   def _set_path_for_seeds

app/services/control_rate_service.rb

@@ -1,5 +1,5 @@
 class ControlRateService
-  CACHE_VERSION = 8
+  CACHE_VERSION = 9
 
   # Can be initialized with _either_ a Period range or a single Period to calculate
   # control rates. We need to handle a single period for calculating point in time benchmarks.
@@ -115,11 +115,11 @@ def quarterly_report?
   end
 
   def cache_key
-    "#{self.class}/#{region.model_name}/#{region.id}/#{report_range.begin.type}/#{Date.current}"
+    "#{self.class}/#{region.cache_key}/#{report_range.begin.type}/#{Date.current}"
   end
 
   def cache_version
-    "#{region.updated_at.utc.to_s(:usec)}/#{CACHE_VERSION}"
+    "#{region.cache_version}/#{CACHE_VERSION}"
   end
 
   def force_cache?

app/services/no_bp_measure_service.rb

@@ -1,5 +1,5 @@
 class NoBPMeasureService
-  CACHE_VERSION = 2
+  CACHE_VERSION = 3
   CACHE_TTL = 7.days
 
   def initialize(region, periods:)
@@ -77,11 +77,11 @@ def execute_sql(period)
   end
 
   def cache_key(period)
-    "#{self.class}/#{region.model_name}/#{region.id}/#{period}"
+    "#{self.class}/#{region.cache_key}/#{period}"
   end
 
   def cache_version
-    "#{region.updated_at.utc.to_s(:usec)}/#{CACHE_VERSION}"
+    "#{region.cache_version}/#{CACHE_VERSION}"
   end
 
   def force_cache?

app/services/reports/region_cache_warmer.rb

@@ -53,14 +53,14 @@ def notify(msg, extra = {})
 
     def cache_facility_groups
       FacilityGroup.find_each(batch_size: BATCH_SIZE).each do |region|
-        RegionService.new(region: region, period: period).call
+        RegionService.call(region: region, period: period)
         Statsd.instance.increment("region_cache_warmer.facility_groups.cache")
       end
     end
 
     def cache_facilities
       Facility.find_each(batch_size: BATCH_SIZE).each do |region|
-        RegionService.new(region: region, period: period).call
+        RegionService.call(region: region, period: period)
         Statsd.instance.increment("region_cache_warmer.facilities.cache")
       end
     end

app/services/reports/region_service.rb

@@ -7,6 +7,10 @@ def self.default_period
       Period.month(Date.current.beginning_of_month)
     end
 
+    def self.call(*args)
+      new(*args).call
+    end
+
     def initialize(region:, period:)
       @current_user = current_user
       @region = region

spec/lib/seed/facility_seeder_spec.rb

@@ -5,7 +5,7 @@
     expect {
       Seed::FacilitySeeder.call(config: Seed::Config.new)
     }.to change { FacilityGroup.count }.by(2)
-      .and change { Facility.count }.by(8)
+      .and change { Facility.count }.by_at_least(7)
   end
 
   it "creates facility groups and facilities with regions" do

spec/models/facility_district_spec.rb

@@ -8,6 +8,11 @@
   let!(:mansa_facility) { create(:facility, district: "Mansa", facility_group: facility_group) }
   let!(:other_bathinda_facility) { create(:facility, district: "Bathinda", facility_group: other_facility_group) }
 
+  it "has a cache_key" do
+    facility_district = FacilityDistrict.new(name: "Bathinda")
+    expect(facility_district.cache_key).to eq("facility_districts/Bathinda")
+  end
+
   describe "#facilities" do
     it "returns facilities with matching district name" do
       facility_district = FacilityDistrict.new(name: "Bathinda")

spec/models/region_spec.rb

@@ -47,6 +47,15 @@
     end
   end
 
+  context "cache_key" do
+    it "contains class name, region type, and id" do
+      facility_group = create(:facility_group)
+      region = facility_group.region
+      expect(region.cache_key).to eq("regions/district/#{region.id}")
+      expect(facility_group.cache_key).to eq(region.cache_key)
+    end
+  end
+
   context "facilities" do
     it "returns the source facilities" do
       facility_group = create(:facility_group)

spec/services/reports/region_cache_warmer_spec.rb

@@ -22,7 +22,7 @@ def refresh_views
   it "skips caching if disabled via Flipper" do
     Flipper.enable(:disable_region_cache_warmer)
     expect(Reports::RegionService).to receive(:new).never
-    Reports::RegionCacheWarmer.new
+    Reports::RegionCacheWarmer.call
   end
 
   it "sets force_cache to true on creation" do
@@ -31,41 +31,16 @@ def refresh_views
     expect(RequestStore.store[:force_cache]).to be true
   end
 
+  it "completes successfully" do
+    facilities = FactoryBot.create_list(:facility, 2, facility_group: facility_group_1)
+    Reports::RegionCacheWarmer.call
+  end
+
   it "warms the cache for all regions" do
     facilities = FactoryBot.create_list(:facility, 5, facility_group: facility_group_1)
-    facility = facilities.first
-    facility_2 = create(:facility)
-
-    controlled_in_jan_and_june = create_list(:patient, 2, full_name: "controlled", recorded_at: jan_2019, registration_facility: facility, registration_user: user)
-    controlled_just_for_june = create(:patient, full_name: "just for june", recorded_at: jan_2019, registration_facility: facility, registration_user: user)
-    patient_from_other_facility = create(:patient, full_name: "other facility", recorded_at: jan_2019, registration_facility: facility_2, registration_user: user)
-
-    Timecop.freeze(jan_2020) do
-      controlled_in_jan_and_june.map do |patient|
-        create(:blood_pressure, :under_control, facility: facility, patient: patient, recorded_at: 2.days.ago)
-        create(:blood_pressure, :hypertensive, facility: facility, patient: patient, recorded_at: 4.days.ago)
-      end
-      create(:blood_pressure, :under_control, facility: facility, patient: patient_from_other_facility, recorded_at: 2.days.ago)
-    end
 
-    Timecop.freeze(june_1) do
-      controlled_in_jan_and_june.map do |patient|
-        create(:blood_pressure, :under_control, facility: facility, patient: patient, recorded_at: 2.days.ago)
-        create(:blood_pressure, :hypertensive, facility: facility, patient: patient, recorded_at: 4.days.ago)
-      end
-
-      create(:blood_pressure, :under_control, facility: facility, patient: controlled_just_for_june, recorded_at: 4.days.ago)
-
-      uncontrolled = create_list(:patient, 2, recorded_at: Time.current, registration_facility: facility, registration_user: user)
-      uncontrolled.map do |patient|
-        create(:blood_pressure, :hypertensive, facility: facility, patient: patient, recorded_at: 1.days.ago)
-        create(:blood_pressure, :under_control, facility: facility, patient: patient, recorded_at: 2.days.ago)
-      end
-    end
-
-    refresh_views
-    Timecop.travel(june_1) do
-      Reports::RegionCacheWarmer.call
-    end
+    expect(Reports::RegionService).to receive(:call).with(hash_including(region: instance_of(FacilityGroup))).exactly(1).times
+    expect(Reports::RegionService).to receive(:call).with(hash_including(region: instance_of(Facility))).exactly(5).times
+    Reports::RegionCacheWarmer.call
   end
 end