Small improvement to deletion log handling

When an exercise was replaced by another upstream, the sync would correctly update
existing routines and logs, but would not update the cache, causing the structure
data to point at stale exercise data.

Closes wger-project/react#1253

Author: rolandgeider

wger/exercises/models/base.py

@@ -272,7 +272,11 @@ def delete(
         from wger.exercises.models.image import ExerciseImage
         from wger.exercises.models.translation import Translation
         from wger.exercises.models.video import ExerciseVideo
-        from wger.manager.models import WorkoutLog
+        from wger.manager.helpers import reset_routine_cache
+        from wger.manager.models import (
+            Routine,
+            WorkoutLog,
+        )
         from wger.manager.models.slot_entry import SlotEntry
 
         replacement = None
@@ -299,9 +303,27 @@ def delete(
             # Replace references in workout logs and routines before deleting,
             # so that user data is not lost on this instance
             if replacement:
+                # Repointing the references with a bulk .update() bypasses the
+                # signals that keep the routine caches fresh. Collect the
+                # affected ones first, then reset their caches explicitly,
+                # otherwise the cached routine structure keeps pointing at this
+                # now-deleted exercise.
+                routine_ids = set(
+                    SlotEntry.objects.filter(exercise=self).values_list(
+                        'slot__day__routine_id', flat=True
+                    )
+                )
+                routine_ids.update(
+                    WorkoutLog.objects.filter(exercise=self).values_list('routine_id', flat=True)
+                )
+                routine_ids.discard(None)
+
                 SlotEntry.objects.filter(exercise=self).update(exercise=replacement)
                 WorkoutLog.objects.filter(exercise=self).update(exercise=replacement)
 
+                for routine in Routine.objects.filter(id__in=routine_ids):
+                    reset_routine_cache(routine)
+
                 if transfer_media:
                     ExerciseImage.objects.filter(exercise=self).update(exercise=replacement)
                     ExerciseVideo.objects.filter(exercise=self).update(exercise=replacement)

wger/exercises/sync.py

@@ -320,17 +320,17 @@ def print_fn(_):
             try:
                 old_exercise = Exercise.objects.get(uuid=uuid)
 
-                # Replace exercise in routines and logs
+                # Count the references before they are repointed, so we can
+                # report how many were moved to the replacement
                 if obj_replaced:
-                    nr_slot_entries = SlotEntry.objects.filter(exercise=old_exercise).update(
-                        exercise=obj_replaced
-                    )
+                    nr_slot_entries = SlotEntry.objects.filter(exercise=old_exercise).count()
+                    nr_logs = WorkoutLog.objects.filter(exercise=old_exercise).count()
 
-                    nr_logs = WorkoutLog.objects.filter(exercise=old_exercise).update(
-                        exercise=obj_replaced
-                    )
+                # Let the model repoint the references in routines and logs to
+                # the replacement (and reset the affected routine caches) before
+                # deleting, so that user data is not lost on this instance
+                old_exercise.delete(replace_by=replaced_by_uuid if obj_replaced else None)
 
-                old_exercise.delete()
                 replaced_by_info = f' (replaced by {obj_replaced.uuid})' if obj_replaced else ''
                 print_fn(f'Deleted exercise {uuid}{replaced_by_info}')
                 if nr_slot_entries:

wger/exercises/tests/test_deletion_log.py

@@ -16,6 +16,9 @@
 from unittest.mock import patch
 from uuid import UUID
 
+# Django
+from django.core.cache import cache
+
 # wger
 from wger.core.tests.base_testcase import WgerTestCase
 from wger.exercises.models import (
@@ -27,6 +30,7 @@
 from wger.exercises.models.video import ExerciseVideo
 from wger.manager.models import WorkoutLog
 from wger.manager.models.slot_entry import SlotEntry
+from wger.utils.cache import CacheKeyMapper
 
 
 class DeletionLogTestCase(WgerTestCase):
@@ -116,6 +120,28 @@ def test_exercise_replace_by_updates_slot_entries(self):
         # The slot entry should now point to the replacement
         self.assertEqual(SlotEntry.objects.filter(exercise=replacement).count(), 2)
 
+    def test_exercise_replace_by_resets_routine_cache(self):
+        """
+        Test that replacing a deleted exercise invalidates the cache of every
+        routine that referenced it.
+
+        The references are repointed with a bulk update that does not fire the
+        signals which normally refresh the routine caches. If the cache is not
+        reset explicitly, the cached routine structure keeps pointing at the
+        now-deleted exercise.
+        """
+        exercise_to_delete = Exercise.objects.get(pk=1)
+        replacement = Exercise.objects.get(pk=2)
+
+        routine = SlotEntry.objects.get(exercise=exercise_to_delete).slot.day.routine
+        cache_key = CacheKeyMapper.routine_api_structure_key(routine.id, routine.user_id)
+        cache.set(cache_key, {'stale': 'data'})
+        self.assertIsNotNone(cache.get(cache_key))
+
+        exercise_to_delete.delete(replace_by=str(replacement.uuid))
+
+        self.assertIsNone(cache.get(cache_key))
+
     def test_exercise_replace_by_delete_is_atomic_on_failure(self):
         """
         Test that a failure during the delete rolls back the deletion log and

wger/exercises/tests/test_sync.py

@@ -17,6 +17,7 @@
 from unittest.mock import patch
 
 # Django
+from django.core.cache import cache
 from django.core.exceptions import ValidationError
 
 # Third Party
@@ -52,6 +53,7 @@
     SlotEntry,
     WorkoutLog,
 )
+from wger.utils.cache import CacheKeyMapper
 from wger.utils.requests import wger_headers
 
 
@@ -822,6 +824,23 @@ def test_deletion_log(self, mock_request):
         for pk in logs:
             self.assertEqual(WorkoutLog.objects.get(pk=pk).exercise_id, 2)
 
+    @patch('requests.get', return_value=MockDeletionLogResponse())
+    def test_deletion_log_resets_routine_cache(self, mock_request):
+        """
+        Test that syncing a deletion that replaces an exercise invalidates the
+        cache of every routine that referenced it. Otherwise the cached routine
+        structure keeps pointing at the now-deleted exercise.
+        """
+        exercise1 = Exercise.objects.get(pk=1)
+        routine = SlotEntry.objects.filter(exercise=exercise1).first().slot.day.routine
+        cache_key = CacheKeyMapper.routine_api_structure_key(routine.id, routine.user_id)
+        cache.set(cache_key, {'stale': 'data'})
+        self.assertIsNotNone(cache.get(cache_key))
+
+        handle_deleted_entries(print)
+
+        self.assertIsNone(cache.get(cache_key))
+
     @patch('requests.get', return_value=MockExerciseResponse())
     def test_exercise_sync(self, mock_request):
         self.assertEqual(Exercise.objects.count(), 8)