fix(mass_model_updater): read tracked fields via attribute access for skip_unchanged (#15114)

The skip-unchanged no-op check snapshotted each tracked field with
model.__dict__.get(f) to avoid triggering a deferred-field load. But when a
tracked field is deferred (the caller's queryset used .only()/.defer() and
omitted it), __dict__ has no entry and .get() returns None. If function then
recomputes that field to None, None == None marks the row unchanged and the
write is silently dropped, persisting a stale value.

Read the pre-function value with normal attribute access (getattr) instead.
Deferred tracked fields are loaded and compared against their real persisted
value, so a real change can never be mistaken for a no-op. The cost of a
deferred tracked field is now a visible per-row load rather than silent data
loss; callers needing the fast path must load the tracked fields in their
queryset (documented).

Adds regression tests: a deferred tracked field recomputed to None must issue
an UPDATE and persist; one left unchanged must still skip the write.

Author: valentijnscholten

dojo/utils.py

@@ -1646,8 +1646,10 @@ def mass_model_updater(model_type, models, function, fields, page_size=1000, ord
 
     When ``fields`` is given:
       - skip_unchanged (default True): rows whose tracked ``fields`` were not changed by
-        ``function`` are not written (compared against the values loaded from the page
-        query; deferred fields are read from ``__dict__`` so no extra query is issued).
+        ``function`` are not written. The pre-``function`` value of each tracked field is
+        read with normal attribute access, so callers must ensure the tracked ``fields``
+        are loaded by the queryset (i.e. not excluded via ``.only()``/``.defer()``);
+        otherwise accessing a deferred tracked field issues a per-row query.
       - writer (optional): a callable ``writer(model_type, batch, fields)`` used to persist
         each batch instead of Django's ``bulk_update`` (e.g. a backend-specific fast path).
         Defaults to ``bulk_update``.
@@ -1689,16 +1691,17 @@ def mass_model_updater(model_type, models, function, fields, page_size=1000, ord
             i += 1
             last_id = model.id
 
-            # snapshot tracked fields before mutation (read from __dict__ to avoid
-            # triggering a deferred-field load); used to skip no-op writes
+            # snapshot tracked fields before mutation; used to skip no-op writes.
+            # Read via normal attribute access so the real persisted value is compared
+            # (callers must load the tracked fields; see docstring).
             before = None
             if fields and skip_unchanged:
-                before = [model.__dict__.get(f) for f in fields]
+                before = [getattr(model, f) for f in fields]
 
             function(model)
 
             if fields and skip_unchanged and before is not None and all(
-                model.__dict__.get(f) == old for f, old in zip(fields, before, strict=True)
+                getattr(model, f) == old for f, old in zip(fields, before, strict=True)
             ):
                 # nothing changed for this row -> no write needed
                 pass

unittests/test_mass_model_updater.py

@@ -130,6 +130,54 @@ def test_skip_unchanged_can_be_disabled(self):
             msg="skip_unchanged=False must still write unchanged rows",
         )
 
+    def test_writes_deferred_tracked_field_recomputed_to_none(self):
+        # Regression (dojo-pro risk-mode SLA recalc): the before-snapshot reads each
+        # tracked field with normal attribute access, so a deferred tracked field is
+        # loaded and its real persisted value is compared -- never mistaken for an
+        # absent/None value. Recomputing a populated field to None must still persist.
+        ids = self._finding_ids()
+        # Seed a non-None value via a normally-loaded queryset.
+        mass_model_updater(
+            Finding, Finding.objects.filter(id__in=ids),
+            lambda f: setattr(f, "hash_code", f"seed-{f.id}"), fields=["hash_code"], order="asc",
+        )
+        # Reload deferring hash_code (the tracked field), then recompute it to None.
+        deferred_qs = Finding.objects.filter(id__in=ids).only("id")
+        with CaptureQueriesContext(connection) as ctx:
+            mass_model_updater(
+                Finding, deferred_qs,
+                lambda f: setattr(f, "hash_code", None), fields=["hash_code"], order="asc",
+            )
+        self.assertGreater(
+            len(_updates(ctx.captured_queries)), 0,
+            msg="deferred tracked field recomputed to None must still issue an UPDATE",
+        )
+        for fid in ids:
+            self.assertIsNone(
+                Finding.objects.get(id=fid).hash_code,
+                msg=f"finding {fid}: deferred field recomputed to None was not persisted",
+            )
+
+    def test_skips_deferred_tracked_field_left_unchanged(self):
+        # The flip side: a deferred tracked field that function() does NOT change must
+        # still be recognised as unchanged (real value loaded and compared equal) and
+        # issue no UPDATE -- the skip-unchanged optimization keeps working with .only().
+        ids = self._finding_ids()
+        mass_model_updater(
+            Finding, Finding.objects.filter(id__in=ids),
+            lambda f: setattr(f, "hash_code", f"keep-{f.id}"), fields=["hash_code"], order="asc",
+        )
+        deferred_qs = Finding.objects.filter(id__in=ids).only("id")
+        with CaptureQueriesContext(connection) as ctx:
+            mass_model_updater(
+                Finding, deferred_qs,
+                lambda f: setattr(f, "hash_code", f"keep-{f.id}"), fields=["hash_code"], order="asc",
+            )
+        self.assertEqual(
+            len(_updates(ctx.captured_queries)), 0,
+            msg="deferred tracked field left unchanged must not issue an UPDATE",
+        )
+
     def test_hashcode_values_writer_uses_values_sql_on_postgres(self):
         if connection.vendor != "postgresql":
             self.skipTest("VALUES fast path is postgres-only")