Rework unique_together data in the signature for comparison and storage.

Review Request #11885 — Created Nov. 29, 2021 and submitted

Information

Django Evolution
release-2.x

Reviewers

A unique_together is a list of tuples of fields, or a list of lists of
fields. When saving these to a signature, we normalize them to a list of
tuples, to ease comparison, but there were still comparison issues for
data not yet stored.

We now normalize whenever the property is set (either in the constructor
or when applying a unique_together value), ensuring we're comparing
the same types of data, and ensuring we're working with copies of the
original data. That also allows us to remove a bunch of deepcopy()
calls in other places.

Some changes were also made around the _unique_together_applied flag.
First off, while still a private variable, there's now a public function
(apply_unique_together()) that will set it, so that the mutation class
doesn't have to reach into private variables.

More importantly, comparison now works differently.
ModelSignature.__eq__ calls has_unique_together_changed, which only
considered the old model signature's value for
_unique_together_applied. The problem is, if objects are compared in
the other direction (which can and does happen), the wrong result will
be returned.

That's been updated to instead compare the old/new flags directly, which
is more correct and avoids an issue where evolutions get skipped if the
only change to a model is a unique_together baseline change.

Unit tests pass on all versions of Python and Django.

Summary ID
Rework unique_together data in the signature for comparison and storage.
A `unique_together` is a list of tuples of fields, or a list of lists of fields. When saving these to a signature, we normalize them to a list of tuples, to ease comparison, but there were still comparison issues for data not yet stored. We now normalize whenever the property is set (either in the constructor or when applying a `unique_together` value), ensuring we're comparing the same types of data, and ensuring we're working with copies of the original data. That also allows us to remove a bunch of `deepcopy()` calls in other places. Some changes were also made around the `_unique_together_applied` flag. First off, while still a private variable, there's now a public function (`apply_unique_together()`) that will set it, so that the mutation class doesn't have to reach into private variables. More importantly, comparison now works differently. `ModelSignature.__eq__` calls `has_unique_together_changed`, which only considered the old model signature's value for `_unique_together_applied`. The problem is, if objects are compared in the other direction (which can and does happen), the wrong result will be returned. That's been updated to instead compare the old/new flags directly, which is more correct and avoids an issue where evolutions get skipped if the only change to a model is a `unique_together` baseline change.
c8f90a11977db068b7250098e62c33168541b9d1
david
  1. Ship It!
  2. 
      
chipx86
Review request changed

Status: Closed (submitted)

Change Summary:

Pushed to release-2.x (1aef74c)
Loading...