Don't save CounterField values by default when saving models.
Review Request #9234 — Created Sept. 29, 2017 and submitted
We had a long-standing, hard-to-catch bug where counters would sometimes
end up with incorrect values, requiring a reset of the counters in order
to recompute. This turned out to be due to code
incrementing/decrementing a counter and then another instance somewhere
saving the old counter values back to the database.The only time a counter's value should be included as part of a standard
save operation is if the value is being reset (savingNone
) or if it's
explicitly requested inupdate_fields
.While Django does allow for some control over which fields are saved to
the database, there's no way for fields themselves to opt out of saving
a value, which is what we'd prefer. So instead,CounterField
now adds
a new version of_do_update()
(the model's function that actually
kicks off computing the SQL to update a set of values) that filters out
the values first. This ensures that we never accidentally save a
CounterField
or any of its subclasses.Unit tests were added to check on this new behavior.
All Djblets and Review Board unit tests pass.
Manually tested with a solid repro case and saw that the old stale values
were no longer being stored duringsave()
.