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 (saving None) or if it's
explicitly requested in update_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 during save().

  2. djblets/db/ (Diff revision 1)

    We need to go deeper

  3. djblets/db/ (Diff revision 1)

    IMO type(model) reads nicer. Dunder methods/attrs are ugly. However type(model) only works on new style classes.

  1. Ship It!
Review request changed

Status: Closed (submitted)

Change Summary:

Pushed to release-0.10.x (c082c60)