Don't save CounterField values by default when saving models.

Review Request #9234 — Created Sept. 29, 2017 and submitted

Information

Djblets
release-0.10.x
e52e572...

Reviewers

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().

brennie
  1. 
      
  2. djblets/db/fields.py (Diff revision 1)
     
     

    We need to go deeper

  3. djblets/db/fields.py (Diff revision 1)
     
     

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

  4. 
      
brennie
  1. Ship It!
  2. 
      
chipx86
Review request changed

Status: Closed (submitted)

Change Summary:

Pushed to release-0.10.x (c082c60)
Loading...