diff --git a/docs/changelog.rst b/docs/changelog.rst index d19b1441..82cce5b9 100644 --- a/docs/changelog.rst +++ b/docs/changelog.rst @@ -1,6 +1,11 @@ Changelog ========= +v5.0.0 (202x-XX-XX) +------------------- + +* Implemented `Fix proxy model support. `_ + v4.3.0 (202X-XX-XX) ------------------- diff --git a/docs/index.rst b/docs/index.rst index e9f69f72..3adf9fc2 100644 --- a/docs/index.rst +++ b/docs/index.rst @@ -119,6 +119,7 @@ Advanced topics formsets migrating managers + proxy advanced changelog api/index diff --git a/docs/proxy.rst b/docs/proxy.rst new file mode 100644 index 00000000..eb508cc0 --- /dev/null +++ b/docs/proxy.rst @@ -0,0 +1,66 @@ +Proxy Models +============ + +:pypi:`django-polymorphic` has supported :ref:`proxy models ` since they were +introduced in Django but the default implementation is unintuitive. If a row is created from the +proxy model the :class:`~django.contrib.contenttypes.models.ContentType` of the proxy class is +recorded. This allows patterns that need to alter behavior based on which class was used to create +the row. + +**By default the queryset on proxy models will filter on instance_of(ProxyModel) which will exclude +any rows that were not created from the proxy.** + +.. code-block:: python + + from polymorphic.models import PolymorphicModel + + class MyModel(PolymorphicModel): + ... + + class MyProxy(MyModel): + class Meta: + proxy = True + + + MyModel.objects.create() + MyProxy.objects.create() + + assert MyModel.objects.count() == 2 + assert MyProxy.objects.count() == 1 + +This behavior may be unexpected for typical uses of proxy models which involves creating from the +concrete class then accessing from a proxy in the context where you need the modified proxy +interface. There is a +`discussion `_ if this should +continue to be the default behavior in version 5+. + +Polymorphic Proxy Queries +------------------------- + +.. versionadded:: 4.3 + +If you wish for your proxy model querysets to behave polymorphically by default +(include all rows created by the proxy and concrete models in the class's inheritance tree) then +instead of (or in additon to) :attr:`~django.db.models.Options.proxy` set a +:attr:`Meta.polymorphic_proxy` attribute to True: + +.. code-block:: python + + from polymorphic.models import PolymorphicModel + + class MyModel(PolymorphicModel): + ... + + class MyProxy(MyModel): + class Meta: + polymorphc_proxy = True + + + MyModel.objects.create() + MyProxy.objects.create() + + assert MyModel.objects.count() == 2 + assert MyProxy.objects.count() == 2 + + for proxy in MyProxy.objects.all(): + assert isinstance(proxy, MyProxy) # \ No newline at end of file diff --git a/src/polymorphic/base.py b/src/polymorphic/base.py index 2a9783fa..643d6f33 100644 --- a/src/polymorphic/base.py +++ b/src/polymorphic/base.py @@ -53,6 +53,14 @@ class PolymorphicModelBase(ModelBase): """ def __new__(self, model_name, bases, attrs, **kwargs): + polymorphic__proxy = None + if "Meta" in attrs: + if hasattr(attrs["Meta"], "polymorphic_proxy"): + polymorphic__proxy = attrs["Meta"].polymorphic_proxy + if polymorphic__proxy: + attrs["Meta"].proxy = True + del attrs["Meta"].polymorphic_proxy + # Workaround compatibility issue with six.with_metaclass() and custom Django model metaclasses: if not attrs and model_name == "NewBase": return super().__new__(self, model_name, bases, attrs, **kwargs) @@ -77,6 +85,11 @@ def __new__(self, model_name, bases, attrs, **kwargs): # for __init__ function of this class (monkeypatching inheritance accessors) new_class.polymorphic_super_sub_accessors_replaced = False + if polymorphic__proxy is not None: + new_class._meta.polymorphic_proxy = polymorphic__proxy + else: + new_class._meta.polymorphic_proxy = not new_class._meta.proxy + # determine the name of the primary key field and store it into the class variable # polymorphic_primary_key_name (it is needed by query.py) if new_class._meta.pk: diff --git a/src/polymorphic/managers.py b/src/polymorphic/managers.py index 0f8fd71c..918a329d 100644 --- a/src/polymorphic/managers.py +++ b/src/polymorphic/managers.py @@ -28,7 +28,7 @@ def from_queryset(cls, queryset_class, class_name=None): def get_queryset(self): qs = self.queryset_class(self.model, using=self._db, hints=self._hints) - if self.model._meta.proxy: + if not self.model._meta.polymorphic_proxy: qs = qs.instance_of(self.model) return qs diff --git a/src/polymorphic/tests/migrations/0001_initial.py b/src/polymorphic/tests/migrations/0001_initial.py index 47e1c50d..25e2ac8e 100644 --- a/src/polymorphic/tests/migrations/0001_initial.py +++ b/src/polymorphic/tests/migrations/0001_initial.py @@ -985,6 +985,17 @@ class Migration(migrations.Migration): 'swappable': 'POLYMORPHIC_TEST_SWAPPABLE', }, ), + migrations.CreateModel( + name='AliasProxyChild', + fields=[ + ], + options={ + 'proxy': True, + 'indexes': [], + 'constraints': [], + }, + bases=('tests.proxybase',), + ), migrations.CreateModel( name='ProxyChild', fields=[ @@ -1040,6 +1051,17 @@ class Migration(migrations.Migration): }, bases=('tests.subclassselectorproxybasemodel',), ), + migrations.CreateModel( + name='TradProxyChild', + fields=[ + ], + options={ + 'proxy': True, + 'indexes': [], + 'constraints': [], + }, + bases=('tests.proxybase',), + ), migrations.CreateModel( name='Bottom', fields=[ @@ -1198,6 +1220,39 @@ class Migration(migrations.Migration): }, bases=(polymorphic.showfields.ShowFieldTypeAndContent, models.Model), ), + migrations.CreateModel( + name='AliasOfNonProxyChild', + fields=[ + ], + options={ + 'proxy': True, + 'indexes': [], + 'constraints': [], + }, + bases=('tests.nonproxychild',), + ), + migrations.CreateModel( + name='NonAliasNonProxyChild', + fields=[ + ], + options={ + 'proxy': True, + 'indexes': [], + 'constraints': [], + }, + bases=('tests.nonproxychild',), + ), + migrations.CreateModel( + name='ProxyChildAliasProxy', + fields=[ + ], + options={ + 'proxy': True, + 'indexes': [], + 'constraints': [], + }, + bases=('tests.tradproxychild',), + ), migrations.CreateModel( name='PurpleHeadDuck', fields=[ @@ -1209,6 +1264,17 @@ class Migration(migrations.Migration): }, bases=('tests.blueheadduck', models.Model), ), + migrations.CreateModel( + name='TradProxyOnProxyChild', + fields=[ + ], + options={ + 'proxy': True, + 'indexes': [], + 'constraints': [], + }, + bases=('tests.proxychild',), + ), migrations.CreateModel( name='Model2D', fields=[ diff --git a/src/polymorphic/tests/models.py b/src/polymorphic/tests/models.py index b9b201c7..e67fea8f 100644 --- a/src/polymorphic/tests/models.py +++ b/src/polymorphic/tests/models.py @@ -354,7 +354,7 @@ class UUIDPlainC(UUIDPlainB): field3 = models.CharField(max_length=30) -# base -> proxy +# base(poly) -> proxy class ProxyBase(PolymorphicModel): @@ -370,7 +370,59 @@ class NonProxyChild(ProxyBase): name = models.CharField(max_length=30) -# base -> proxy -> real models +# A traditional django proxy models. ie proxy'ed class is alias class +# but in django_polymorphic this is not so. +# +# We have model types :- +# base(poly) / child(poly) : A concrete polymorphic model 1+ fields +# base(non poly) : A concrete django model 1+ fields +# proxy(poly) : A proxy model where it is considered different +# : from it superclasses +# proxy(Traditional Django) : A proxy model where it is an alias for the +# : underlying model + + +# base(poly) -> proxy(poly) -> proxy(Traditional Django) +class TradProxyOnProxyChild(ProxyChild): + class Meta: + polymorphic_proxy = True + + +# base(poly) -> proxy(Traditional Django) +class TradProxyChild(ProxyBase): + class Meta: + proxy = True + polymorphic_proxy = True + + +# base(poly) -> proxy(Traditional Django) -> proxy(poly) +# Not really helpful model as reduces to base(poly) -> proxy(poly) + + +# base(poly) -> child(poly) -> proxy(Traditional Django) +class AliasOfNonProxyChild(NonProxyChild): + class Meta: + proxy = True + polymorphic_proxy = True + + +# base(poly) -> proxy(Traditional Django) -> proxy(poly) +class ProxyChildAliasProxy(TradProxyChild): + class Meta: + proxy = True + + +# base(poly) -> proxy(poly) +class AliasProxyChild(ProxyBase): + class Meta: + polymorphic_proxy = True + + +# child(poly) -> proxy(poly) +class NonAliasNonProxyChild(NonProxyChild): + class Meta: + proxy = True + polymorphic_proxy = False class ProxiedBase(ShowFieldTypeAndContent, PolymorphicModel): diff --git a/src/polymorphic/tests/test_orm.py b/src/polymorphic/tests/test_orm.py index 671df97e..963a858b 100644 --- a/src/polymorphic/tests/test_orm.py +++ b/src/polymorphic/tests/test_orm.py @@ -14,6 +14,7 @@ from polymorphic.managers import PolymorphicManager from polymorphic.models import PolymorphicTypeInvalid, PolymorphicTypeUndefined from polymorphic.tests.models import ( + AliasProxyChild, ArtProject, Base, BlogA, @@ -99,6 +100,11 @@ SpecialAccount1, SpecialAccount1_1, SpecialAccount2, + NonAliasNonProxyChild, + TradProxyOnProxyChild, + TradProxyChild, + AliasOfNonProxyChild, + ProxyChildAliasProxy, ) @@ -871,6 +877,99 @@ def test_queryset_on_proxy_model_does_not_return_superclasses(self): assert ProxyBase.objects.count() == 5 assert ProxyChild.objects.count() == 3 + def test_queryset_on_polymorphic_proxy_model_returns_superclasses(self): + ProxyBase.objects.create(some_data="Base1") + AliasProxyChild.objects.create(some_data="ProxyChild1") + AliasProxyChild.objects.create(some_data="ProxyChild2") + ProxyChild.objects.create(some_data="PolyChild1") + NonAliasNonProxyChild.objects.create(some_data="SubChild1") + NonAliasNonProxyChild.objects.create(some_data="SubChild2") + NonProxyChild.objects.create(some_data="NonProxyChild1", name="t1") + + with self.subTest("superclasses"): + self.assertEqual(7, ProxyBase.objects.count()) + self.assertEqual(7, AliasProxyChild.objects.count()) + with self.subTest("only complete classes"): + # Non proxy models should not return the proxy siblings + self.assertEqual(1, ProxyChild.objects.count()) + self.assertEqual(2, NonAliasNonProxyChild.objects.count()) + self.assertEqual(3, NonProxyChild.objects.count()) + + def test_proxied_instances_are_proxies(self): + ProxyBase.objects.create(some_data="ProxyBase") + AliasProxyChild.objects.create(some_data="AliasProxyChild") + AliasProxyChild.objects.create(some_data="AliasProxyChild") + ProxyChild.objects.create(some_data="ProxyChild") + NonAliasNonProxyChild.objects.create(some_data="NonAliasNonProxyChild", name="nanpc1") + NonAliasNonProxyChild.objects.create(some_data="NonAliasNonProxyChild", name="nanpc1") + NonProxyChild.objects.create(some_data="NonProxyChild", name="t1") + TradProxyChild.objects.create(some_data="TradProxyChild") + TradProxyChild.objects.create(some_data="TradProxyChild") + ProxyChildAliasProxy.objects.create(some_data="ProxyChildAliasProxy") + AliasOfNonProxyChild.objects.create(some_data="AliasOfNonProxyChild", name="aonpc1") + TradProxyOnProxyChild.objects.create(some_data="TradProxyOnProxyChild") + TradProxyOnProxyChild.objects.create(some_data="TradProxyOnProxyChild") + + self.assertEqual(13, ProxyBase.objects.count()) + self.assertEqual(13, AliasProxyChild.objects.count()) + self.assertEqual(3, ProxyChild.objects.count()) + self.assertEqual(2, NonAliasNonProxyChild.objects.count()) + self.assertEqual(4, AliasOfNonProxyChild.objects.count()) + self.assertEqual(13, TradProxyChild.objects.count()) + self.assertEqual(1, ProxyChildAliasProxy.objects.count()) + self.assertEqual(13, TradProxyOnProxyChild.objects.count()) + self.assertEqual(4, NonProxyChild.objects.count()) + + # for row in ProxyBase.objects.all(): + # if row.some_data in ["ProxyBase", "ProxyChild"]: + # assert row.__class__ is ProxyBase, row.__class__.__name__ + # elif row.some_data == "TradProxyOnProxyChild": + # assert row.__class__ is TradProxyOnProxyChild, row.__class__.__name__ + # elif row.some_data in ["NonProxyChild", "NonAliasNonProxyChild"]: + # assert row.__class__ is NonProxyChild, row.__class__.__name__ + # elif row.some_data == "AliasOfNonProxyChild": + # assert row.__class__ is AliasOfNonProxyChild, row.__class__.__name__ + # elif row.some_data in ["TradProxyChild", "ProxyChildAliasProxy"]: + # assert row.__class__ is TradProxyChild, row.__class__.__name__ + # elif row.some_data == "AliasProxyChild": + # assert row.__class__ is AliasProxyChild, row.__class__.__name__ + # else: + # assert False, f"Unexpected row: {row.some_data}" + + def test_polymorphic_proxy_object_has_different_ctype_from_base(self): + obj1 = ProxyBase.objects.create(some_data="Base1") + obj2 = AliasProxyChild.objects.create(some_data="ProxyChild1") + obj1_ctype = ContentType.objects.get_for_model(obj1, for_concrete_model=False) + obj2_ctype = ContentType.objects.get_for_model(obj2, for_concrete_model=False) + self.assertNotEqual(obj1_ctype, obj2_ctype) + + def test_can_create_django_style_proxy_classes_alias(self): + ProxyBase.objects.create(some_data="Base1") + TradProxyChild.objects.create(some_data="Base2") + self.assertEqual(2, ProxyBase.objects.count()) + self.assertEqual(2, TradProxyChild.objects.count()) + TradProxyOnProxyChild.objects.create() + + def test_convert_back_to_django_style_from_polymorphic(self): + ProxyBase.objects.create(some_data="Base1") + ProxyChild.objects.create(some_data="Base1") + TradProxyOnProxyChild.objects.create(some_data="Base3") + self.assertEqual(3, ProxyBase.objects.count()) + self.assertEqual(2, ProxyChild.objects.count()) + self.assertEqual(3, TradProxyOnProxyChild.objects.count()) + + def test_convert_back_to_django_style_from_polymorphic_stops_at_concrete(self): + ProxyBase.objects.create(some_data="Base1") + NonProxyChild.objects.create(some_data="Base1") + AliasOfNonProxyChild.objects.create(some_data="Base1") + + self.assertEqual(3, ProxyBase.objects.count()) + self.assertEqual(2, NonProxyChild.objects.count()) + self.assertEqual(2, AliasOfNonProxyChild.objects.count()) + + def test_revert_back_to_polymorphic_proxy(self): + self.assertFalse(ProxyChildAliasProxy._meta.polymorphic_proxy) + def test_proxy_get_real_instance_class(self): """ The call to ``get_real_instance()`` also checks whether the returned model is of the correct type. @@ -880,12 +979,12 @@ def test_proxy_get_real_instance_class(self): name = "Item1" nonproxychild = NonProxyChild.objects.create(name=name) - pb = ProxyBase.objects.get(id=1) + pb = ProxyBase.objects.get(id=nonproxychild.pk) assert pb.get_real_instance_class() == NonProxyChild assert pb.get_real_instance() == nonproxychild assert pb.name == name - pbm = NonProxyChild.objects.get(id=1) + pbm = NonProxyChild.objects.get(id=nonproxychild.pk) assert pbm.get_real_instance_class() == NonProxyChild assert pbm.get_real_instance() == nonproxychild assert pbm.name == name