Proposal - ``Model.reset_state``

677 views
Skip to first unread message

Yo-Yo Ma

unread,
Oct 15, 2012, 12:16:41 PM10/15/12
to django-d...@googlegroups.com
Problem:

a = A.objects.get(...)
form = AModelForm(data={...}, instance=a)
if form.is_valid():
    a = form.save()
else:
    a.calculate_foo_field()
    a.last_attempt = datetime.now()
    a.save()  # Oops, now the instance has the bad data provided to the form


Workarounds:

    # Get a fresh copy of ``a``
    a = A.objects.get(pk=a.pk)
    # Wasted query
    # Also, this won't work in the context of a ``select_for_update`` on the original instance

    # Use ``update`` instead
    A.objects.filter(pk=a.pk).update(last_attempt=datetime.now(), ...)
    # What about ``calculate_foo_field``?
    # Also has the ``select_for_update`` problem


Solution:

    a.reset_state()  # Resets the instance's field state to the point of creation (using data stored in a ``_state_reset_cache`` dict)
    a.reset_foo()  # etc.
    a.latest_attempt = datetime.now()
    a.save()

Problem: Uses more memory per instance
Solution: Add ``QuerySet.cache_for_reset()`` allowing opt-in usage of the feature, treating ``reset_state`` as noop when instance doesn't have the state reset cache.

Alex Ogier

unread,
Oct 15, 2012, 1:11:24 PM10/15/12
to django-d...@googlegroups.com
That seems like a heavy-weight solution for something that probably isn't all that common. The same thing could be achieved by adding a method to checkpoint the current state, or even clone the instance, which would only incur the overhead when you know you need it. For example:

a = A.objects.get(...)
form = AModelForm(data={...}, instance=a.clone()) ## copy the state here

if form.is_valid():
    a = form.save()
else:
    a.calculate_foo_field()
    a.last_attempt = datetime.now()
    a.save()

This is easy enough to do piecemeal by adding .clone() methods to your models, and trying to do it generically sends us even farther towards Django models having all the bureaucracy of a C++ class with copy-constructors, so I wouldn't advocate adding it to core. Hopefully it addresses your use case though.

Here's an example of a similar snippet someone has written, though obviously you want to keep the same PK instead of auto-generating a new one: http://djangosnippets.org/snippets/904/

Best,
Alex Ogier
Reply all
Reply to author
Forward
0 new messages