diff --git a/app/models/facility.rb b/app/models/facility.rb index b308352699..49e26e2d60 100644 --- a/app/models/facility.rb +++ b/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") diff --git a/app/models/facility_district.rb b/app/models/facility_district.rb index c9a402afc0..eaf7e02b4c 100644 --- a/app/models/facility_district.rb +++ b/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 diff --git a/app/models/facility_group.rb b/app/models/facility_group.rb index d9381d4ce5..57265ffb64 100644 --- a/app/models/facility_group.rb +++ b/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 diff --git a/app/models/region.rb b/app/models/region.rb index ca1dab1f7b..8f6c496115 100644 --- a/app/models/region.rb +++ b/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 diff --git a/app/services/control_rate_service.rb b/app/services/control_rate_service.rb index 4cc32b4ac0..e43ea2baca 100644 --- a/app/services/control_rate_service.rb +++ b/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? diff --git a/app/services/no_bp_measure_service.rb b/app/services/no_bp_measure_service.rb index eb98c86c1a..238427551e 100644 --- a/app/services/no_bp_measure_service.rb +++ b/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? diff --git a/app/services/reports/region_cache_warmer.rb b/app/services/reports/region_cache_warmer.rb index 0036c0f04c..362f6b34a6 100644 --- a/app/services/reports/region_cache_warmer.rb +++ b/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 diff --git a/app/services/reports/region_service.rb b/app/services/reports/region_service.rb index 63f7f19bf1..841052a8b5 100644 --- a/app/services/reports/region_service.rb +++ b/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 diff --git a/spec/lib/seed/facility_seeder_spec.rb b/spec/lib/seed/facility_seeder_spec.rb index 70b380a3e8..1d6b283573 100644 --- a/spec/lib/seed/facility_seeder_spec.rb +++ b/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 diff --git a/spec/models/facility_district_spec.rb b/spec/models/facility_district_spec.rb index 0c0f9559ef..e68ecccb91 100644 --- a/spec/models/facility_district_spec.rb +++ b/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") diff --git a/spec/models/region_spec.rb b/spec/models/region_spec.rb index 238a75362e..4933536966 100644 --- a/spec/models/region_spec.rb +++ b/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) diff --git a/spec/services/reports/region_cache_warmer_spec.rb b/spec/services/reports/region_cache_warmer_spec.rb index f70df15048..de52473fdb 100644 --- a/spec/services/reports/region_cache_warmer_spec.rb +++ b/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