Replace OrderedDict #1625
Replace OrderedDict #1625
Conversation
Since Python 3.7, regular dicts preserve insertion order, meaning most usages of OrderedDict can be replaced with regular dicts. Add a comment for the one case that cannot in the DebugToolbar class.
| return signing.Signer(salt=cls.salt).sign( | ||
| json.dumps({key: force_str(value) for key, value in items}) | ||
| json.dumps({key: force_str(value) for key, value in data.items()}) |
There was a problem hiding this comment.
I was a bit worried about this. I thought that maybe the string was hashed and that changing the order of keys may lead to invalid hashes but this doesn't seem to be the case since we're not hashing, only signing and unsigning.
There was a problem hiding this comment.
Another consideration is that when the signing code was originally added, DjDT still supported Python 3.6, which although in practice maintained dict insertion order, did not guarantee it. So to guarantee that the tests were correct, it was probably necessary to sort the data before signing at that point in time (no longer necessary now that Python 3.7 is the minimum supported version).
tests/test_forms.py
Outdated
|
|
||
| DATA = {"value": "foo", "date": datetime(2020, 1, 1, tzinfo=timezone.utc)} | ||
| SIGNED_DATA = f'{{"date": "2020-01-01 00:00:00+00:00", "value": "foo"}}:{SIGNATURE}' | ||
| SIGNED_DATA = f'{{"value": "foo", "date": "2020-01-01 00:00:00+00:00"}}:{SIGNATURE}' |
There was a problem hiding this comment.
Detail: I'd prefer it if you reversed the order of value and date in the dictionary below; this would neatly show that the signature stays the same.
The signature does not depend on a canonical representation of the data, so don't bother sorting it. Tweak the data for the tests as a consequence. However, note that the tests are still deterministic since as of version 3.7, Python guarantees dictionary order.
Passing a key argument of the form `key=lambda item: item[0]` to sorted() when sorting dict items is unneeded, because tuples already compare element-wise, and the first elements are known to be unique since they are dictionary keys.
living180
force-pushed
the
replace_ordereddict
branch
from
5e6ae18 to
bd4132f
Compare
|
Thanks! |
Replace most usages of
OrderedDictwith plain Python dicts since as of Python 3.7, dicts are guaranteed to maintain insertion order. Also make a couple of small improvements related to sorting dicts.