karanlyons / django-save-the-change

Your DB Got It the First Time.
http://django-save-the-change.readthedocs.io
Other
114 stars 29 forks source link

Support for abstract model classes? #23

Open gregorypease280 opened 7 years ago

gregorypease280 commented 7 years ago

Can I attach the STC Decorator to an abstract base class? I have over 20 or so models that all inherit from a base class and I would like to implement STC for all of them.

Currently receiving def save(self, *args, **kwargs): for save_hook in self._meta._stc_save_hooks: E AttributeError: 'Options' object has no attribute '_stc_save_hooks'

karanlyons commented 7 years ago

This looks like a result of how the Options (Model.meta) object is created for models. Some bookkeeping for STC is put in that object since it’s the sanest place for it to be, but that object isn’t inherited.

There’s definitely some way to make this work, but I’m not sure how best to do it without looking into it some more. Ideally we could have the needed attributes copied during class creation so you’ll only pay the cost on startup.

oTree-org commented 7 years ago

If you have many subclass models, a workaround is to apply the decorator in the metaclass. This works for me with Python 3.6 and Django 1.8.8:

from django.db import models
from save_the_change.decorators import SaveTheChange
from django.db.models.base import ModelBase

class SaveTheChangeModelBase(ModelBase):
    def __new__(cls, name, bases, attrs):
        meta = attrs.get("Meta")
        is_abstract = getattr(meta, "abstract", False)

        new_class = super().__new__(cls, name, bases, attrs)

        if not is_abstract:
            new_class = SaveTheChange(new_class)

        return new_class

class BaseKnight(models.Model, metaclass=SaveTheChangeModelBase):
    class Meta:
        abstract = True

class Knight(BaseKnight):
    name = models.CharField(max_length=100)
    age = models.PositiveIntegerField()

Verified with test case:

from django.db import models
from django.test import TestCase
from .models import Knight
from unittest.mock import patch
# Create your tests here.

class TestSTC(TestCase):

    def test_stc(self):
        kn = Knight.objects.create(name='Lancelot', age=20)

        with patch.object(models.Model, 'save') as patched_save:
            kn.save()
            patched_save.assert_not_called()

            kn.age = 21
            kn.save()
            patched_save.assert_called_once_with(update_fields=['age'])
karanlyons commented 7 years ago

What @oTree-org recommends is definitely what I would say to do as well given STC as it currently stands. STC used to use mixins as opposed to generators, but that required metaclass inheritance (as in the examples above) which becomes a problem for interoperability with other packages that want to do the same. The move to decorators seemed the best way out of these issues (I’m not a diehard fan of my current solution here, but it was the least bad one I could come up with. This history is also why the decorators are @SaveTheChange instead of @save_the_change).

Of course the decorators are just being passed the actual constructed class object, which is why they don’t work properly through descendant classes.

Honestly I’d probably just recommend that developers define their own custom metaclass when they want to use STC with model inheritance. It’s not pretty, but I can’t come up with another solution that has a clean syntax, and having STC do metaclass stuff by default again may introduce more problems than it solves.