edelooff / sqlalchemy-json

Full-featured JSON type with mutation tracking for SQLAlchemy
http://variable-scope.com/posts/mutation-tracking-in-nested-json-structures-using-sqlalchemy
BSD 2-Clause "Simplified" License
189 stars 34 forks source link

setdefault() on TrackedDict doesn't convert the default parameter to a TrackedObject #19

Closed brianmaissy closed 5 years ago

brianmaissy commented 5 years ago

In the following code:

foo = nested_mutable_json.setdefault('foo', {})
foo['bar'] = 'baz'

the __setitem__ will convert the dict to a TrackedDict, but setdefaultwill still return the original dict, so operations on foowill not be saved to the TrackedDict.

I'd be happy to open a PR with a fix (just override setdefault in TrackedDict to convert the default parameter and then call super().setdefault)

edelooff commented 5 years ago

That's definitely undesired behavior, and the suggested fix sounds appropriate, a PR would be welcome.

This does point out a general shortcoming in __setitem__ though, where it doesn't simple set in all cases, sometimes storing a copy instead, causing pretty much the same unexpected behavior as you highlighted with setdefault:

>>> outer = TrackedDict()
>>> inner = {}
>>> outer['inner'] = inner
>>> inner['foo'] = 'bar'
>>> outer
{'inner': {}}

Ideas on how to deal with this inconsistency would also be appreciated.