Invalidate cache

CHANGELOG.md

@@ -17,6 +17,7 @@ and this project adheres to [Semantic Versioning](http://semver.org/).
 - Validate trip start date to be earlier than end date. #2057
 - Fog of war radius slider in map v2 settings is now being respected correctly. #2041
 - Applying changes in map v2 settings now works correctly. #2041
+- Invalidate stats cache on recalculation and other operations that change stats data.
 
 
 # [0.36.4] - 2025-12-26

app/jobs/points/nightly_reverse_geocoding_job.rb

@@ -6,8 +6,16 @@ class Points::NightlyReverseGeocodingJob < ApplicationJob
   def perform
     return unless DawarichSettings.reverse_geocoding_enabled?
 
+    processed_user_ids = Set.new
+
     Point.not_reverse_geocoded.find_each(batch_size: 1000) do |point|
       point.async_reverse_geocode
+      processed_user_ids.add(point.user_id)
+    end
+
+    # Invalidate caches for all users who had points reverse geocoded
+    processed_user_ids.each do |user_id|
+      Cache::InvalidateUserCaches.new(user_id).call
     end
   end
 end

app/services/cache/clean.rb

@@ -36,8 +36,8 @@ class Cache::Clean
 
     def delete_countries_cities_cache
       User.find_each do |user|
-        Rails.cache.delete("dawarich/user_#{user.id}_countries")
-        Rails.cache.delete("dawarich/user_#{user.id}_cities")
+        Rails.cache.delete("dawarich/user_#{user.id}_countries_visited")
+        Rails.cache.delete("dawarich/user_#{user.id}_cities_visited")
       end
     end
   end

app/services/cache/invalidate_user_caches.rb

@@ -0,0 +1,34 @@
+# frozen_string_literal: true
+
+class Cache::InvalidateUserCaches
+  # Invalidates user-specific caches that depend on point data.
+  # This should be called after:
+  # - Reverse geocoding operations (updates country/city data)
+  # - Stats calculations (updates geocoding stats)
+  # - Bulk point imports/updates
+  def initialize(user_id)
+    @user_id = user_id
+  end
+
+  def call
+    invalidate_countries_visited
+    invalidate_cities_visited
+    invalidate_points_geocoded_stats
+  end
+
+  def invalidate_countries_visited
+    Rails.cache.delete("dawarich/user_#{user_id}_countries_visited")
+  end
+
+  def invalidate_cities_visited
+    Rails.cache.delete("dawarich/user_#{user_id}_cities_visited")
+  end
+
+  def invalidate_points_geocoded_stats
+    Rails.cache.delete("dawarich/user_#{user_id}_points_geocoded_stats")
+  end
+
+  private
+
+  attr_reader :user_id
+end

app/services/stats/calculate_month.rb

@@ -42,6 +42,9 @@ class Stats::CalculateMonth
       )
 
       stat.save!
+
+      # Invalidate user caches after stats calculation since they may have changed
+      Cache::InvalidateUserCaches.new(user.id).call
     end
   end
 

spec/jobs/imports/destroy_job_spec.rb

@@ -0,0 +1,192 @@
+# frozen_string_literal: true
+
+require 'rails_helper'
+
+RSpec.describe Imports::DestroyJob, type: :job do
+  describe '#perform' do
+    let(:user) { create(:user) }
+    let(:import) { create(:import, user: user, status: :completed) }
+
+    describe 'queue configuration' do
+      it 'uses the default queue' do
+        expect(described_class.queue_name).to eq('default')
+      end
+    end
+
+    context 'when import exists' do
+      before do
+        # Create some points for the import
+        create_list(:point, 3, user: user, import: import)
+      end
+
+      it 'changes import status to deleting and deletes it' do
+        expect(import).not_to be_deleting
+
+        import_id = import.id
+        described_class.perform_now(import_id)
+
+        expect(Import.find_by(id: import_id)).to be_nil
+      end
+
+      it 'calls the Imports::Destroy service' do
+        destroy_service = instance_double(Imports::Destroy)
+        allow(Imports::Destroy).to receive(:new).with(user, import).and_return(destroy_service)
+        allow(destroy_service).to receive(:call)
+
+        described_class.perform_now(import.id)
+
+        expect(Imports::Destroy).to have_received(:new).with(user, import)
+        expect(destroy_service).to have_received(:call)
+      end
+
+      it 'broadcasts status update to the user' do
+        allow(ImportsChannel).to receive(:broadcast_to)
+
+        described_class.perform_now(import.id)
+
+        expect(ImportsChannel).to have_received(:broadcast_to).with(
+          user,
+          hash_including(
+            action: 'status_update',
+            import: hash_including(
+              id: import.id,
+              status: 'deleting'
+            )
+          )
+        ).at_least(:once)
+      end
+
+      it 'broadcasts deletion complete to the user' do
+        allow(ImportsChannel).to receive(:broadcast_to)
+
+        described_class.perform_now(import.id)
+
+        expect(ImportsChannel).to have_received(:broadcast_to).with(
+          user,
+          hash_including(
+            action: 'delete',
+            import: hash_including(id: import.id)
+          )
+        ).at_least(:once)
+      end
+
+      it 'broadcasts both status update and deletion messages' do
+        allow(ImportsChannel).to receive(:broadcast_to)
+
+        described_class.perform_now(import.id)
+
+        expect(ImportsChannel).to have_received(:broadcast_to).twice
+      end
+
+      it 'deletes the import and its points' do
+        import_id = import.id
+        point_ids = import.points.pluck(:id)
+
+        described_class.perform_now(import_id)
+
+        expect(Import.find_by(id: import_id)).to be_nil
+        expect(Point.where(id: point_ids)).to be_empty
+      end
+    end
+
+    context 'when import does not exist' do
+      let(:non_existent_id) { 999_999 }
+
+      it 'does not raise an error' do
+        expect { described_class.perform_now(non_existent_id) }.not_to raise_error
+      end
+
+      it 'does not call the Imports::Destroy service' do
+        expect(Imports::Destroy).not_to receive(:new)
+
+        described_class.perform_now(non_existent_id)
+      end
+
+      it 'does not broadcast any messages' do
+        expect(ImportsChannel).not_to receive(:broadcast_to)
+
+        described_class.perform_now(non_existent_id)
+      end
+
+      it 'returns early without logging' do
+        allow(Rails.logger).to receive(:warn)
+
+        described_class.perform_now(non_existent_id)
+
+        # Early return at line 8 doesn't log, only rescue block does
+        expect(Rails.logger).not_to have_received(:warn)
+      end
+    end
+
+    context 'when import is deleted during job execution' do
+      it 'handles RecordNotFound gracefully' do
+        allow(Import).to receive(:find_by).with(id: import.id).and_return(import)
+        allow(import).to receive(:deleting!).and_raise(ActiveRecord::RecordNotFound)
+
+        expect { described_class.perform_now(import.id) }.not_to raise_error
+      end
+
+      it 'logs a warning when RecordNotFound is raised' do
+        allow(Import).to receive(:find_by).with(id: import.id).and_return(import)
+        allow(import).to receive(:deleting!).and_raise(ActiveRecord::RecordNotFound)
+        allow(Rails.logger).to receive(:warn)
+
+        described_class.perform_now(import.id)
+
+        expect(Rails.logger).to have_received(:warn).with(/Import #{import.id} not found/)
+      end
+    end
+
+    context 'when broadcast fails' do
+      before do
+        allow(ImportsChannel).to receive(:broadcast_to).and_raise(StandardError, 'Broadcast error')
+      end
+
+      it 'allows the error to propagate' do
+        expect { described_class.perform_now(import.id) }.to raise_error(StandardError, 'Broadcast error')
+      end
+    end
+
+    context 'when Imports::Destroy service fails' do
+      before do
+        allow_any_instance_of(Imports::Destroy).to receive(:call).and_raise(StandardError, 'Destroy failed')
+      end
+
+      it 'allows the error to propagate' do
+        expect { described_class.perform_now(import.id) }.to raise_error(StandardError, 'Destroy failed')
+      end
+
+      it 'has already set status to deleting before service is called' do
+        expect do
+          described_class.perform_now(import.id) rescue StandardError
+        end.to change { import.reload.status }.to('deleting')
+      end
+    end
+
+    context 'with multiple imports for different users' do
+      let(:user2) { create(:user) }
+      let(:import2) { create(:import, user: user2, status: :completed) }
+
+      it 'only broadcasts to the correct user' do
+        expect(ImportsChannel).to receive(:broadcast_to).with(user, anything).twice
+        expect(ImportsChannel).not_to receive(:broadcast_to).with(user2, anything)
+
+        described_class.perform_now(import.id)
+      end
+    end
+
+    context 'job enqueuing' do
+      it 'can be enqueued' do
+        expect do
+          described_class.perform_later(import.id)
+        end.to have_enqueued_job(described_class).with(import.id)
+      end
+
+      it 'can be performed later with correct arguments' do
+        expect do
+          described_class.perform_later(import.id)
+        end.to have_enqueued_job(described_class).on_queue('default').with(import.id)
+      end
+    end
+  end
+end

spec/jobs/points/nightly_reverse_geocoding_job_spec.rb

@@ -62,18 +62,22 @@ RSpec.describe Points::NightlyReverseGeocodingJob, type: :job do
       end
 
       context 'with points needing reverse geocoding' do
+        let(:user2) { create(:user) }
         let!(:point_without_geocoding1) do
           create(:point, user: user, reverse_geocoded_at: nil)
         end
         let!(:point_without_geocoding2) do
           create(:point, user: user, reverse_geocoded_at: nil)
         end
+        let!(:point_without_geocoding3) do
+          create(:point, user: user2, reverse_geocoded_at: nil)
+        end
         let!(:geocoded_point) do
           create(:point, user: user, reverse_geocoded_at: 1.day.ago)
         end
 
         it 'processes all points that need reverse geocoding' do
-          expect { described_class.perform_now }.to have_enqueued_job(ReverseGeocodingJob).exactly(2).times
+          expect { described_class.perform_now }.to have_enqueued_job(ReverseGeocodingJob).exactly(3).times
         end
 
         it 'enqueues jobs with correct parameters' do
@@ -82,6 +86,8 @@ RSpec.describe Points::NightlyReverseGeocodingJob, type: :job do
             .with('Point', point_without_geocoding1.id)
             .and have_enqueued_job(ReverseGeocodingJob)
             .with('Point', point_without_geocoding2.id)
+            .and have_enqueued_job(ReverseGeocodingJob)
+            .with('Point', point_without_geocoding3.id)
         end
 
         it 'uses find_each with correct batch size' do
@@ -93,6 +99,44 @@ RSpec.describe Points::NightlyReverseGeocodingJob, type: :job do
 
           expect(relation_mock).to have_received(:find_each).with(batch_size: 1000)
         end
+
+        it 'invalidates caches for all affected users' do
+          allow(Cache::InvalidateUserCaches).to receive(:new).and_call_original
+
+          described_class.perform_now
+
+          # Verify that cache invalidation service was instantiated for both users
+          expect(Cache::InvalidateUserCaches).to have_received(:new).with(user.id)
+          expect(Cache::InvalidateUserCaches).to have_received(:new).with(user2.id)
+        end
+
+        it 'invalidates caches for the correct users' do
+          cache_service1 = instance_double(Cache::InvalidateUserCaches)
+          cache_service2 = instance_double(Cache::InvalidateUserCaches)
+
+          allow(Cache::InvalidateUserCaches).to receive(:new).with(user.id).and_return(cache_service1)
+          allow(Cache::InvalidateUserCaches).to receive(:new).with(user2.id).and_return(cache_service2)
+          allow(cache_service1).to receive(:call)
+          allow(cache_service2).to receive(:call)
+
+          described_class.perform_now
+
+          expect(cache_service1).to have_received(:call)
+          expect(cache_service2).to have_received(:call)
+        end
+
+        it 'does not invalidate caches multiple times for the same user' do
+          # user has 2 points, but cache should only be invalidated once
+          cache_service = instance_double(Cache::InvalidateUserCaches)
+
+          allow(Cache::InvalidateUserCaches).to receive(:new).with(user.id).and_return(cache_service)
+          allow(Cache::InvalidateUserCaches).to receive(:new).with(user2.id).and_return(instance_double(Cache::InvalidateUserCaches, call: nil))
+          allow(cache_service).to receive(:call)
+
+          described_class.perform_now
+
+          expect(cache_service).to have_received(:call).once
+        end
       end
     end
 

spec/requests/imports_spec.rb

@@ -223,9 +223,10 @@ RSpec.describe 'Imports', type: :request do
       it 'deletes the import' do
         expect do
           delete import_path(import)
-        end.to change(user.imports, :count).by(-1)
+        end.to have_enqueued_job(Imports::DestroyJob).with(import.id)
 
         expect(response).to redirect_to(imports_path)
+        expect(import.reload).to be_deleting
       end
     end
   end

spec/services/cache/clean_spec.rb

@@ -12,10 +12,10 @@ RSpec.describe Cache::Clean do
     let(:user_2_years_tracked_key) { "dawarich/user_#{user2.id}_years_tracked" }
     let(:user_1_points_geocoded_stats_key) { "dawarich/user_#{user1.id}_points_geocoded_stats" }
     let(:user_2_points_geocoded_stats_key) { "dawarich/user_#{user2.id}_points_geocoded_stats" }
-    let(:user_1_countries_key) { "dawarich/user_#{user1.id}_countries" }
-    let(:user_2_countries_key) { "dawarich/user_#{user2.id}_countries" }
-    let(:user_1_cities_key) { "dawarich/user_#{user1.id}_cities" }
-    let(:user_2_cities_key) { "dawarich/user_#{user2.id}_cities" }
+    let(:user_1_countries_key) { "dawarich/user_#{user1.id}_countries_visited" }
+    let(:user_2_countries_key) { "dawarich/user_#{user2.id}_countries_visited" }
+    let(:user_1_cities_key) { "dawarich/user_#{user1.id}_cities_visited" }
+    let(:user_2_cities_key) { "dawarich/user_#{user2.id}_cities_visited" }
 
     before do
       # Set up cache entries that should be cleaned

spec/services/cache/invalidate_user_caches_spec.rb

@@ -0,0 +1,60 @@
+# frozen_string_literal: true
+
+require 'rails_helper'
+
+RSpec.describe Cache::InvalidateUserCaches do
+  let(:user) { create(:user) }
+  let(:service) { described_class.new(user.id) }
+
+  describe '#call' do
+    it 'invalidates all user-related caches' do
+      # Pre-populate the caches
+      Rails.cache.write("dawarich/user_#{user.id}_countries_visited", ['USA', 'Canada'])
+      Rails.cache.write("dawarich/user_#{user.id}_cities_visited", ['New York', 'Toronto'])
+      Rails.cache.write("dawarich/user_#{user.id}_points_geocoded_stats", { geocoded: 100, without_data: 10 })
+
+      # Verify caches are populated
+      expect(Rails.cache.read("dawarich/user_#{user.id}_countries_visited")).to eq(['USA', 'Canada'])
+      expect(Rails.cache.read("dawarich/user_#{user.id}_cities_visited")).to eq(['New York', 'Toronto'])
+      expect(Rails.cache.read("dawarich/user_#{user.id}_points_geocoded_stats")).to eq({ geocoded: 100, without_data: 10 })
+
+      # Invalidate caches
+      service.call
+
+      # Verify caches are cleared
+      expect(Rails.cache.read("dawarich/user_#{user.id}_countries_visited")).to be_nil
+      expect(Rails.cache.read("dawarich/user_#{user.id}_cities_visited")).to be_nil
+      expect(Rails.cache.read("dawarich/user_#{user.id}_points_geocoded_stats")).to be_nil
+    end
+  end
+
+  describe '#invalidate_countries_visited' do
+    it 'deletes the countries_visited cache' do
+      Rails.cache.write("dawarich/user_#{user.id}_countries_visited", ['USA', 'Canada'])
+
+      service.invalidate_countries_visited
+
+      expect(Rails.cache.read("dawarich/user_#{user.id}_countries_visited")).to be_nil
+    end
+  end
+
+  describe '#invalidate_cities_visited' do
+    it 'deletes the cities_visited cache' do
+      Rails.cache.write("dawarich/user_#{user.id}_cities_visited", ['New York', 'Toronto'])
+
+      service.invalidate_cities_visited
+
+      expect(Rails.cache.read("dawarich/user_#{user.id}_cities_visited")).to be_nil
+    end
+  end
+
+  describe '#invalidate_points_geocoded_stats' do
+    it 'deletes the points_geocoded_stats cache' do
+      Rails.cache.write("dawarich/user_#{user.id}_points_geocoded_stats", { geocoded: 100, without_data: 10 })
+
+      service.invalidate_points_geocoded_stats
+
+      expect(Rails.cache.read("dawarich/user_#{user.id}_points_geocoded_stats")).to be_nil
+    end
+  end
+end

spec/services/stats/calculate_month_spec.rb

@@ -201,6 +201,29 @@ RSpec.describe Stats::CalculateMonth do
           end
         end
       end
+
+      context 'when invalidating caches' do
+        it 'invalidates user caches after updating stats' do
+          # Use existing points from the parent context
+          cache_service = instance_double(Cache::InvalidateUserCaches)
+          allow(Cache::InvalidateUserCaches).to receive(:new).with(user.id).and_return(cache_service)
+          allow(cache_service).to receive(:call)
+
+          calculate_stats
+
+          expect(cache_service).to have_received(:call)
+        end
+
+        it 'does not invalidate caches when there are no points' do
+          # Create a new user without points
+          new_user = create(:user)
+          service = described_class.new(new_user.id, year, month)
+
+          expect(Cache::InvalidateUserCaches).not_to receive(:new)
+
+          service.call
+        end
+      end
     end
   end
 end