From e7e470739fc4d2694d1c0e2dbf3f6465b44ad069 Mon Sep 17 00:00:00 2001 From: Mjumbe Wawatu Ukweli Date: Mon, 11 Mar 2013 03:23:44 -0400 Subject: [PATCH 1/4] Use parent's primary key when model is derived via multitable inheritance --- rest_framework/serializers.py | 7 +++-- rest_framework/tests/models.py | 14 ++++++++++ rest_framework/tests/serializer.py | 42 +++++++++++++++++++++++++++++- 3 files changed, 60 insertions(+), 3 deletions(-) diff --git a/rest_framework/serializers.py b/rest_framework/serializers.py index 2ae7c215f..1b044c5fa 100644 --- a/rest_framework/serializers.py +++ b/rest_framework/serializers.py @@ -456,8 +456,11 @@ class ModelSerializer(Serializer): "Serializer class '%s' is missing 'model' Meta option" % self.__class__.__name__ opts = get_concrete_model(cls)._meta pk_field = opts.pk - # while pk_field.rel: - # pk_field = pk_field.rel.to._meta.pk + + # If model is a child via multitable inheritance, use parent's pk + while isinstance(pk_field, models.OneToOneField) and pk_field.rel.parent_link: + pk_field = pk_field.rel.to._meta.pk + fields = [pk_field] fields += [field for field in opts.fields if field.serialize] fields += [field for field in opts.many_to_many if field.serialize] diff --git a/rest_framework/tests/models.py b/rest_framework/tests/models.py index f2117538c..fcfe5a0c7 100644 --- a/rest_framework/tests/models.py +++ b/rest_framework/tests/models.py @@ -166,3 +166,17 @@ class NullableOneToOneSource(RESTFrameworkModel): name = models.CharField(max_length=100) target = models.OneToOneField(OneToOneTarget, null=True, blank=True, related_name='nullable_source') + + +# Inherited +class ParentModel(RESTFrameworkModel): + name1 = models.CharField(max_length=100) + + +class ChildModel(ParentModel): + name2 = models.CharField(max_length=100) + + +class AssociatedModel(RESTFrameworkModel): + ref = models.OneToOneField(ParentModel, primary_key=True) + name = models.CharField(max_length=100) diff --git a/rest_framework/tests/serializer.py b/rest_framework/tests/serializer.py index beb372c2b..93909b653 100644 --- a/rest_framework/tests/serializer.py +++ b/rest_framework/tests/serializer.py @@ -4,7 +4,8 @@ from django.test import TestCase from rest_framework import serializers from rest_framework.tests.models import (HasPositiveIntegerAsChoice, Album, ActionItem, Anchor, BasicModel, BlankFieldModel, BlogPost, Book, CallableDefaultValueModel, DefaultValueModel, - ManyToManyModel, Person, ReadOnlyManyToManyModel, Photo) + ManyToManyModel, Person, ReadOnlyManyToManyModel, Photo, ParentModel, ChildModel, + AssociatedModel) import datetime import pickle @@ -96,6 +97,16 @@ class BrokenModelSerializer(serializers.ModelSerializer): fields = ['some_field'] +class DerivedModelSerializer(serializers.ModelSerializer): + class Meta: + model = ChildModel + + +class AssociatedModelSerializer(serializers.ModelSerializer): + class Meta: + model = AssociatedModel + + class BasicTests(TestCase): def setUp(self): self.comment = Comment( @@ -170,6 +181,27 @@ class BasicTests(TestCase): self.assertEqual(set(serializer.data.keys()), set(['name', 'age', 'info'])) + def test_multitable_inherited_model_fields_as_expected(self): + """ + Assert that the parent pointer field is not included in the fields + serialized fields + """ + child = ChildModel(name1='parent name', name2='child name') + serializer = DerivedModelSerializer(child) + self.assertEqual(set(serializer.data.keys()), + set(['name1', 'name2', 'id'])) + + def test_onetoone_primary_key_model_fields_as_expected(self): + """ + Assert that a model with a onetoone field that is the primary key is + not treated like a derived model + """ + parent = ParentModel(name1='parent name') + associate = AssociatedModel(name='hello', ref=parent) + serializer = AssociatedModelSerializer(associate) + self.assertEqual(set(serializer.data.keys()), + set(['name', 'ref'])) + def test_field_with_dictionary(self): """ Make sure that dictionaries from fields are left intact @@ -250,6 +282,14 @@ class ValidationTests(TestCase): self.assertEqual(serializer.is_valid(), False) self.assertEqual(serializer.errors, {'email': ['This field is required.']}) + def test_multitable_inherited_model(self): + data = { + 'name1': 'parent name', + 'name2': 'child name', + } + serializer = DerivedModelSerializer(data=data) + self.assertEqual(serializer.is_valid(), True) + def test_missing_bool_with_default(self): """Make sure that a boolean value with a 'False' value is not mistaken for not having a default.""" From bdcecf48e3c0bd592d50b6b6fac6353a41c3cbfd Mon Sep 17 00:00:00 2001 From: Mjumbe Wawatu Ukweli Date: Mon, 11 Mar 2013 16:01:14 -0400 Subject: [PATCH 2/4] Simplify inherited child check to not use isinstance --- rest_framework/serializers.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/rest_framework/serializers.py b/rest_framework/serializers.py index 1b044c5fa..cd2bb8f1f 100644 --- a/rest_framework/serializers.py +++ b/rest_framework/serializers.py @@ -458,7 +458,7 @@ class ModelSerializer(Serializer): pk_field = opts.pk # If model is a child via multitable inheritance, use parent's pk - while isinstance(pk_field, models.OneToOneField) and pk_field.rel.parent_link: + while pk_field.rel and pk_field.rel.parent_link: pk_field = pk_field.rel.to._meta.pk fields = [pk_field] From 354fbc64ba5046ce49d58c8243a4f81caddf3823 Mon Sep 17 00:00:00 2001 From: Mjumbe Wawatu Ukweli Date: Mon, 11 Mar 2013 17:28:44 -0400 Subject: [PATCH 3/4] Group the model-inheritance-related tests together --- rest_framework/tests/models.py | 14 ---- .../tests/multitable_inheritance.py | 16 +++++ rest_framework/tests/serializer.py | 70 +++++++++++-------- 3 files changed, 55 insertions(+), 45 deletions(-) create mode 100644 rest_framework/tests/multitable_inheritance.py diff --git a/rest_framework/tests/models.py b/rest_framework/tests/models.py index fcfe5a0c7..f2117538c 100644 --- a/rest_framework/tests/models.py +++ b/rest_framework/tests/models.py @@ -166,17 +166,3 @@ class NullableOneToOneSource(RESTFrameworkModel): name = models.CharField(max_length=100) target = models.OneToOneField(OneToOneTarget, null=True, blank=True, related_name='nullable_source') - - -# Inherited -class ParentModel(RESTFrameworkModel): - name1 = models.CharField(max_length=100) - - -class ChildModel(ParentModel): - name2 = models.CharField(max_length=100) - - -class AssociatedModel(RESTFrameworkModel): - ref = models.OneToOneField(ParentModel, primary_key=True) - name = models.CharField(max_length=100) diff --git a/rest_framework/tests/multitable_inheritance.py b/rest_framework/tests/multitable_inheritance.py new file mode 100644 index 000000000..1cca7823e --- /dev/null +++ b/rest_framework/tests/multitable_inheritance.py @@ -0,0 +1,16 @@ +from __future__ import unicode_literals +from django.db import models +from rest_framework.tests.models import RESTFrameworkModel + + +class ParentModel(RESTFrameworkModel): + name1 = models.CharField(max_length=100) + + +class ChildModel(ParentModel): + name2 = models.CharField(max_length=100) + + +class AssociatedModel(RESTFrameworkModel): + ref = models.OneToOneField(ParentModel, primary_key=True) + name = models.CharField(max_length=100) diff --git a/rest_framework/tests/serializer.py b/rest_framework/tests/serializer.py index 93909b653..c4a8a8994 100644 --- a/rest_framework/tests/serializer.py +++ b/rest_framework/tests/serializer.py @@ -4,8 +4,9 @@ from django.test import TestCase from rest_framework import serializers from rest_framework.tests.models import (HasPositiveIntegerAsChoice, Album, ActionItem, Anchor, BasicModel, BlankFieldModel, BlogPost, Book, CallableDefaultValueModel, DefaultValueModel, - ManyToManyModel, Person, ReadOnlyManyToManyModel, Photo, ParentModel, ChildModel, - AssociatedModel) + ManyToManyModel, Person, ReadOnlyManyToManyModel, Photo) +from rest_framework.tests.multitable_inheritance import (ParentModel, + ChildModel, AssociatedModel) import datetime import pickle @@ -181,27 +182,6 @@ class BasicTests(TestCase): self.assertEqual(set(serializer.data.keys()), set(['name', 'age', 'info'])) - def test_multitable_inherited_model_fields_as_expected(self): - """ - Assert that the parent pointer field is not included in the fields - serialized fields - """ - child = ChildModel(name1='parent name', name2='child name') - serializer = DerivedModelSerializer(child) - self.assertEqual(set(serializer.data.keys()), - set(['name1', 'name2', 'id'])) - - def test_onetoone_primary_key_model_fields_as_expected(self): - """ - Assert that a model with a onetoone field that is the primary key is - not treated like a derived model - """ - parent = ParentModel(name1='parent name') - associate = AssociatedModel(name='hello', ref=parent) - serializer = AssociatedModelSerializer(associate) - self.assertEqual(set(serializer.data.keys()), - set(['name', 'ref'])) - def test_field_with_dictionary(self): """ Make sure that dictionaries from fields are left intact @@ -282,14 +262,6 @@ class ValidationTests(TestCase): self.assertEqual(serializer.is_valid(), False) self.assertEqual(serializer.errors, {'email': ['This field is required.']}) - def test_multitable_inherited_model(self): - data = { - 'name1': 'parent name', - 'name2': 'child name', - } - serializer = DerivedModelSerializer(data=data) - self.assertEqual(serializer.is_valid(), True) - def test_missing_bool_with_default(self): """Make sure that a boolean value with a 'False' value is not mistaken for not having a default.""" @@ -1150,3 +1122,39 @@ class DeserializeListTestCase(TestCase): self.assertFalse(serializer.is_valid()) expected = [{}, {'email': ['This field is required.']}, {}] self.assertEqual(serializer.errors, expected) + + +class IneritedModelSerializationTests(TestCase): + + def test_multitable_inherited_model_fields_as_expected(self): + """ + Assert that the parent pointer field is not included in the fields + serialized fields + """ + child = ChildModel(name1='parent name', name2='child name') + serializer = DerivedModelSerializer(child) + self.assertEqual(set(serializer.data.keys()), + set(['name1', 'name2', 'id'])) + + def test_onetoone_primary_key_model_fields_as_expected(self): + """ + Assert that a model with a onetoone field that is the primary key is + not treated like a derived model + """ + parent = ParentModel(name1='parent name') + associate = AssociatedModel(name='hello', ref=parent) + serializer = AssociatedModelSerializer(associate) + self.assertEqual(set(serializer.data.keys()), + set(['name', 'ref'])) + + def test_data_is_valid_without_parent_ptr(self): + """ + Assert that the pointer to the parent table is not a required field + for input data + """ + data = { + 'name1': 'parent name', + 'name2': 'child name', + } + serializer = DerivedModelSerializer(data=data) + self.assertEqual(serializer.is_valid(), True) From bd3fe75e1a41e45b0c9ff1e39707ee059ad0e06a Mon Sep 17 00:00:00 2001 From: Mjumbe Wawatu Ukweli Date: Mon, 11 Mar 2013 17:32:32 -0400 Subject: [PATCH 4/4] Further group model inheritance tests --- .../tests/multitable_inheritance.py | 51 +++++++++++++++++++ rest_framework/tests/serializer.py | 48 ----------------- 2 files changed, 51 insertions(+), 48 deletions(-) diff --git a/rest_framework/tests/multitable_inheritance.py b/rest_framework/tests/multitable_inheritance.py index 1cca7823e..00c153276 100644 --- a/rest_framework/tests/multitable_inheritance.py +++ b/rest_framework/tests/multitable_inheritance.py @@ -1,8 +1,11 @@ from __future__ import unicode_literals from django.db import models +from django.test import TestCase +from rest_framework import serializers from rest_framework.tests.models import RESTFrameworkModel +# Models class ParentModel(RESTFrameworkModel): name1 = models.CharField(max_length=100) @@ -14,3 +17,51 @@ class ChildModel(ParentModel): class AssociatedModel(RESTFrameworkModel): ref = models.OneToOneField(ParentModel, primary_key=True) name = models.CharField(max_length=100) + + +# Serializers +class DerivedModelSerializer(serializers.ModelSerializer): + class Meta: + model = ChildModel + + +class AssociatedModelSerializer(serializers.ModelSerializer): + class Meta: + model = AssociatedModel + + +# Tests +class IneritedModelSerializationTests(TestCase): + + def test_multitable_inherited_model_fields_as_expected(self): + """ + Assert that the parent pointer field is not included in the fields + serialized fields + """ + child = ChildModel(name1='parent name', name2='child name') + serializer = DerivedModelSerializer(child) + self.assertEqual(set(serializer.data.keys()), + set(['name1', 'name2', 'id'])) + + def test_onetoone_primary_key_model_fields_as_expected(self): + """ + Assert that a model with a onetoone field that is the primary key is + not treated like a derived model + """ + parent = ParentModel(name1='parent name') + associate = AssociatedModel(name='hello', ref=parent) + serializer = AssociatedModelSerializer(associate) + self.assertEqual(set(serializer.data.keys()), + set(['name', 'ref'])) + + def test_data_is_valid_without_parent_ptr(self): + """ + Assert that the pointer to the parent table is not a required field + for input data + """ + data = { + 'name1': 'parent name', + 'name2': 'child name', + } + serializer = DerivedModelSerializer(data=data) + self.assertEqual(serializer.is_valid(), True) diff --git a/rest_framework/tests/serializer.py b/rest_framework/tests/serializer.py index c4a8a8994..beb372c2b 100644 --- a/rest_framework/tests/serializer.py +++ b/rest_framework/tests/serializer.py @@ -5,8 +5,6 @@ from rest_framework import serializers from rest_framework.tests.models import (HasPositiveIntegerAsChoice, Album, ActionItem, Anchor, BasicModel, BlankFieldModel, BlogPost, Book, CallableDefaultValueModel, DefaultValueModel, ManyToManyModel, Person, ReadOnlyManyToManyModel, Photo) -from rest_framework.tests.multitable_inheritance import (ParentModel, - ChildModel, AssociatedModel) import datetime import pickle @@ -98,16 +96,6 @@ class BrokenModelSerializer(serializers.ModelSerializer): fields = ['some_field'] -class DerivedModelSerializer(serializers.ModelSerializer): - class Meta: - model = ChildModel - - -class AssociatedModelSerializer(serializers.ModelSerializer): - class Meta: - model = AssociatedModel - - class BasicTests(TestCase): def setUp(self): self.comment = Comment( @@ -1122,39 +1110,3 @@ class DeserializeListTestCase(TestCase): self.assertFalse(serializer.is_valid()) expected = [{}, {'email': ['This field is required.']}, {}] self.assertEqual(serializer.errors, expected) - - -class IneritedModelSerializationTests(TestCase): - - def test_multitable_inherited_model_fields_as_expected(self): - """ - Assert that the parent pointer field is not included in the fields - serialized fields - """ - child = ChildModel(name1='parent name', name2='child name') - serializer = DerivedModelSerializer(child) - self.assertEqual(set(serializer.data.keys()), - set(['name1', 'name2', 'id'])) - - def test_onetoone_primary_key_model_fields_as_expected(self): - """ - Assert that a model with a onetoone field that is the primary key is - not treated like a derived model - """ - parent = ParentModel(name1='parent name') - associate = AssociatedModel(name='hello', ref=parent) - serializer = AssociatedModelSerializer(associate) - self.assertEqual(set(serializer.data.keys()), - set(['name', 'ref'])) - - def test_data_is_valid_without_parent_ptr(self): - """ - Assert that the pointer to the parent table is not a required field - for input data - """ - data = { - 'name1': 'parent name', - 'name2': 'child name', - } - serializer = DerivedModelSerializer(data=data) - self.assertEqual(serializer.is_valid(), True)