From 52db4eadc204c3e5ec234729c24275c8ed803634 Mon Sep 17 00:00:00 2001 From: Dustin Farris Date: Wed, 8 Jan 2014 16:14:27 -0500 Subject: [PATCH 1/5] Testing nested serializers with models that have str foreign key references. --- rest_framework/runtests/settings.py | 3 +++ rest_framework/tests/accounts/__init__.py | 0 rest_framework/tests/accounts/models.py | 8 ++++++++ rest_framework/tests/accounts/serializers.py | 11 +++++++++++ rest_framework/tests/records/__init__.py | 0 rest_framework/tests/records/models.py | 6 ++++++ rest_framework/tests/test_serializer_nested.py | 5 +++++ rest_framework/tests/users/__init__.py | 0 rest_framework/tests/users/models.py | 6 ++++++ rest_framework/tests/users/serializers.py | 8 ++++++++ 10 files changed, 47 insertions(+) create mode 100644 rest_framework/tests/accounts/__init__.py create mode 100644 rest_framework/tests/accounts/models.py create mode 100644 rest_framework/tests/accounts/serializers.py create mode 100644 rest_framework/tests/records/__init__.py create mode 100644 rest_framework/tests/records/models.py create mode 100644 rest_framework/tests/users/__init__.py create mode 100644 rest_framework/tests/users/models.py create mode 100644 rest_framework/tests/users/serializers.py diff --git a/rest_framework/runtests/settings.py b/rest_framework/runtests/settings.py index be7216580..3fc0eb2f4 100644 --- a/rest_framework/runtests/settings.py +++ b/rest_framework/runtests/settings.py @@ -100,6 +100,9 @@ INSTALLED_APPS = ( 'rest_framework', 'rest_framework.authtoken', 'rest_framework.tests', + 'rest_framework.tests.accounts', + 'rest_framework.tests.records', + 'rest_framework.tests.users', ) # OAuth is optional and won't work if there is no oauth_provider & oauth2 diff --git a/rest_framework/tests/accounts/__init__.py b/rest_framework/tests/accounts/__init__.py new file mode 100644 index 000000000..e69de29bb diff --git a/rest_framework/tests/accounts/models.py b/rest_framework/tests/accounts/models.py new file mode 100644 index 000000000..525e601ba --- /dev/null +++ b/rest_framework/tests/accounts/models.py @@ -0,0 +1,8 @@ +from django.db import models + +from rest_framework.tests.users.models import User + + +class Account(models.Model): + owner = models.ForeignKey(User, related_name='accounts_owned') + admins = models.ManyToManyField(User, blank=True, null=True, related_name='accounts_administered') diff --git a/rest_framework/tests/accounts/serializers.py b/rest_framework/tests/accounts/serializers.py new file mode 100644 index 000000000..a27b9ca6f --- /dev/null +++ b/rest_framework/tests/accounts/serializers.py @@ -0,0 +1,11 @@ +from rest_framework import serializers + +from rest_framework.tests.accounts.models import Account +from rest_framework.tests.users.serializers import UserSerializer + + +class AccountSerializer(serializers.ModelSerializer): + admins = UserSerializer(many=True) + + class Meta: + model = Account diff --git a/rest_framework/tests/records/__init__.py b/rest_framework/tests/records/__init__.py new file mode 100644 index 000000000..e69de29bb diff --git a/rest_framework/tests/records/models.py b/rest_framework/tests/records/models.py new file mode 100644 index 000000000..769548074 --- /dev/null +++ b/rest_framework/tests/records/models.py @@ -0,0 +1,6 @@ +from django.db import models + + +class Record(models.Model): + account = models.ForeignKey('accounts.Account', blank=True, null=True) + owner = models.ForeignKey('users.User', blank=True, null=True) diff --git a/rest_framework/tests/test_serializer_nested.py b/rest_framework/tests/test_serializer_nested.py index 7114a0603..686a1f5f4 100644 --- a/rest_framework/tests/test_serializer_nested.py +++ b/rest_framework/tests/test_serializer_nested.py @@ -346,3 +346,8 @@ class NestedModelSerializerUpdateTests(TestCase): result.save() self.assertEqual(result.id, john.id) + +class ImportingModelSerializerWithStrForeignKeys(TestCase): + def test_import_model_serializer(self): + from rest_framework.tests.accounts.serializers import AccountSerializer + self.assertIsInstance(AccountSerializer(), serializers.ModelSerializer) diff --git a/rest_framework/tests/users/__init__.py b/rest_framework/tests/users/__init__.py new file mode 100644 index 000000000..e69de29bb diff --git a/rest_framework/tests/users/models.py b/rest_framework/tests/users/models.py new file mode 100644 index 000000000..128bac90b --- /dev/null +++ b/rest_framework/tests/users/models.py @@ -0,0 +1,6 @@ +from django.db import models + + +class User(models.Model): + account = models.ForeignKey('accounts.Account', blank=True, null=True, related_name='users') + active_record = models.ForeignKey('records.Record', blank=True, null=True) diff --git a/rest_framework/tests/users/serializers.py b/rest_framework/tests/users/serializers.py new file mode 100644 index 000000000..da4965540 --- /dev/null +++ b/rest_framework/tests/users/serializers.py @@ -0,0 +1,8 @@ +from rest_framework import serializers + +from rest_framework.tests.users.models import User + + +class UserSerializer(serializers.ModelSerializer): + class Meta: + model = User From bf5b77ce6d171723f2d187aadd29c8ee4cdc3870 Mon Sep 17 00:00:00 2001 From: Dustin Farris Date: Thu, 9 Jan 2014 11:42:41 -0500 Subject: [PATCH 2/5] Move serializer import to top-level causes test error --- rest_framework/tests/test_serializer_nested.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/rest_framework/tests/test_serializer_nested.py b/rest_framework/tests/test_serializer_nested.py index 686a1f5f4..d0a773feb 100644 --- a/rest_framework/tests/test_serializer_nested.py +++ b/rest_framework/tests/test_serializer_nested.py @@ -6,6 +6,7 @@ Doesn't cover model serializers. from __future__ import unicode_literals from django.test import TestCase from rest_framework import serializers +from rest_framework.tests.accounts.serializers import AccountSerializer from . import models @@ -349,5 +350,4 @@ class NestedModelSerializerUpdateTests(TestCase): class ImportingModelSerializerWithStrForeignKeys(TestCase): def test_import_model_serializer(self): - from rest_framework.tests.accounts.serializers import AccountSerializer self.assertIsInstance(AccountSerializer(), serializers.ModelSerializer) From 2332382b5109939238801e7d4c018455d159fe91 Mon Sep 17 00:00:00 2001 From: Dustin Farris Date: Sun, 12 Jan 2014 20:28:19 -0500 Subject: [PATCH 3/5] Add a sanity check to avoid running into unresolved related models. --- rest_framework/models.py | 23 ++++++++++++++++++++++- rest_framework/serializers.py | 3 ++- rest_framework/tests/test_models.py | 28 ++++++++++++++++++++++++++++ 3 files changed, 52 insertions(+), 2 deletions(-) create mode 100644 rest_framework/tests/test_models.py diff --git a/rest_framework/models.py b/rest_framework/models.py index 5b53a5264..249cdd828 100644 --- a/rest_framework/models.py +++ b/rest_framework/models.py @@ -1 +1,22 @@ -# Just to keep things like ./manage.py test happy +import inspect + +from django.db import models + + +def resolve_model(obj): + """ + Resolve supplied `obj` to a Django model class. + + `obj` must be a Django model class, or a string representation + of one. + + String representations should have the format: + 'appname.ModelName' + """ + if type(obj) == str and len(obj.split('.')) == 2: + app_name, model_name = obj.split('.') + return models.get_model(app_name, model_name) + elif inspect.isclass(obj) and issubclass(obj, models.Model): + return obj + else: + raise ValueError("{0} is not a valid Django model".format(obj)) diff --git a/rest_framework/serializers.py b/rest_framework/serializers.py index b22ca5783..6b31c3043 100644 --- a/rest_framework/serializers.py +++ b/rest_framework/serializers.py @@ -20,6 +20,7 @@ from django.db import models from django.forms import widgets from django.utils.datastructures import SortedDict from rest_framework.compat import get_concrete_model, six +from rest_framework.models import resolve_model # Note: We do the following so that users of the framework can use this style: # @@ -656,7 +657,7 @@ class ModelSerializer(Serializer): if model_field.rel: to_many = isinstance(model_field, models.fields.related.ManyToManyField) - related_model = model_field.rel.to + related_model = resolve_model(model_field.rel.to) if to_many and not model_field.rel.through._meta.auto_created: has_through_model = True diff --git a/rest_framework/tests/test_models.py b/rest_framework/tests/test_models.py new file mode 100644 index 000000000..5e92d48ae --- /dev/null +++ b/rest_framework/tests/test_models.py @@ -0,0 +1,28 @@ +from django.db import models +from django.test import TestCase + +from rest_framework.models import resolve_model +from rest_framework.tests.models import BasicModel + + +class ResolveModelTests(TestCase): + """ + `resolve_model` should return a Django model class given the + provided argument is a Django model class itself, or a properly + formatted string representation of one. + """ + def test_resolve_django_model(self): + resolved_model = resolve_model(BasicModel) + self.assertEqual(resolved_model, BasicModel) + + def test_resolve_string_representation(self): + resolved_model = resolve_model('tests.BasicModel') + self.assertEqual(resolved_model, BasicModel) + + def test_resolve_non_django_model(self): + with self.assertRaises(ValueError): + resolve_model(TestCase) + + def test_resolve_with_improper_string_representation(self): + with self.assertRaises(ValueError): + resolve_model('BasicModel') From b1b58762a3d84ac4cdc6553e8ed06983fd3502ca Mon Sep 17 00:00:00 2001 From: Dustin Farris Date: Mon, 13 Jan 2014 11:47:44 -0500 Subject: [PATCH 4/5] Move models.resolve_model to serializers._resolve_model --- rest_framework/models.py | 23 +-------------- rest_framework/serializers.py | 29 +++++++++++++++++-- .../{test_models.py => test_serializers.py} | 14 ++++----- 3 files changed, 35 insertions(+), 31 deletions(-) rename rest_framework/tests/{test_models.py => test_serializers.py} (62%) diff --git a/rest_framework/models.py b/rest_framework/models.py index 249cdd828..5b53a5264 100644 --- a/rest_framework/models.py +++ b/rest_framework/models.py @@ -1,22 +1 @@ -import inspect - -from django.db import models - - -def resolve_model(obj): - """ - Resolve supplied `obj` to a Django model class. - - `obj` must be a Django model class, or a string representation - of one. - - String representations should have the format: - 'appname.ModelName' - """ - if type(obj) == str and len(obj.split('.')) == 2: - app_name, model_name = obj.split('.') - return models.get_model(app_name, model_name) - elif inspect.isclass(obj) and issubclass(obj, models.Model): - return obj - else: - raise ValueError("{0} is not a valid Django model".format(obj)) +# Just to keep things like ./manage.py test happy diff --git a/rest_framework/serializers.py b/rest_framework/serializers.py index 6b31c3043..0ea2cadbf 100644 --- a/rest_framework/serializers.py +++ b/rest_framework/serializers.py @@ -13,14 +13,15 @@ response content is handled by parsers and renderers. from __future__ import unicode_literals import copy import datetime +import inspect import types from decimal import Decimal +from django.core.exceptions import ImproperlyConfigured from django.core.paginator import Page from django.db import models from django.forms import widgets from django.utils.datastructures import SortedDict from rest_framework.compat import get_concrete_model, six -from rest_framework.models import resolve_model # Note: We do the following so that users of the framework can use this style: # @@ -33,6 +34,27 @@ from rest_framework.relations import * from rest_framework.fields import * +def _resolve_model(obj): + """ + Resolve supplied `obj` to a Django model class. + + `obj` must be a Django model class itself, or a string + representation of one. Useful in situtations like GH #1225 where + Django may not have resolved a string-based reference to a model in + another model's foreign key definition. + + String representations should have the format: + 'appname.ModelName' + """ + if type(obj) == str and len(obj.split('.')) == 2: + app_name, model_name = obj.split('.') + return models.get_model(app_name, model_name) + elif inspect.isclass(obj) and issubclass(obj, models.Model): + return obj + else: + raise ValueError("{0} is not a Django model".format(obj)) + + def pretty_name(name): """Converts 'first_name' to 'First name'""" if not name: @@ -657,7 +679,10 @@ class ModelSerializer(Serializer): if model_field.rel: to_many = isinstance(model_field, models.fields.related.ManyToManyField) - related_model = resolve_model(model_field.rel.to) + try: + related_model = _resolve_model(model_field.rel.to) + except ValueError as error_message: + raise ImproperlyConfigured(error_message) if to_many and not model_field.rel.through._meta.auto_created: has_through_model = True diff --git a/rest_framework/tests/test_models.py b/rest_framework/tests/test_serializers.py similarity index 62% rename from rest_framework/tests/test_models.py rename to rest_framework/tests/test_serializers.py index 5e92d48ae..082a400ca 100644 --- a/rest_framework/tests/test_models.py +++ b/rest_framework/tests/test_serializers.py @@ -1,28 +1,28 @@ from django.db import models from django.test import TestCase -from rest_framework.models import resolve_model +from rest_framework.serializers import _resolve_model from rest_framework.tests.models import BasicModel class ResolveModelTests(TestCase): """ - `resolve_model` should return a Django model class given the + `_resolve_model` should return a Django model class given the provided argument is a Django model class itself, or a properly formatted string representation of one. """ def test_resolve_django_model(self): - resolved_model = resolve_model(BasicModel) + resolved_model = _resolve_model(BasicModel) self.assertEqual(resolved_model, BasicModel) def test_resolve_string_representation(self): - resolved_model = resolve_model('tests.BasicModel') + resolved_model = _resolve_model('tests.BasicModel') self.assertEqual(resolved_model, BasicModel) def test_resolve_non_django_model(self): with self.assertRaises(ValueError): - resolve_model(TestCase) + _resolve_model(TestCase) - def test_resolve_with_improper_string_representation(self): + def test_resolve_improper_string_representation(self): with self.assertRaises(ValueError): - resolve_model('BasicModel') + _resolve_model('BasicModel') From c4d77667cf80588a2195fdc025bda53a5b977105 Mon Sep 17 00:00:00 2001 From: Dustin Farris Date: Mon, 13 Jan 2014 12:03:13 -0500 Subject: [PATCH 5/5] Move ImportingModelSerializerTests and add comments. --- .../tests/test_serializer_import.py | 19 +++++++++++++++++++ .../tests/test_serializer_nested.py | 6 ------ 2 files changed, 19 insertions(+), 6 deletions(-) create mode 100644 rest_framework/tests/test_serializer_import.py diff --git a/rest_framework/tests/test_serializer_import.py b/rest_framework/tests/test_serializer_import.py new file mode 100644 index 000000000..9f30a7ffa --- /dev/null +++ b/rest_framework/tests/test_serializer_import.py @@ -0,0 +1,19 @@ +from django.test import TestCase + +from rest_framework import serializers +from rest_framework.tests.accounts.serializers import AccountSerializer + + +class ImportingModelSerializerTests(TestCase): + """ + In some situations like, GH #1225, it is possible, especially in + testing, to import a serializer who's related models have not yet + been resolved by Django. `AccountSerializer` is an example of such + a serializer (imported at the top of this file). + """ + def test_import_model_serializer(self): + """ + The serializer at the top of this file should have been + imported successfully, and we should be able to instantiate it. + """ + self.assertIsInstance(AccountSerializer(), serializers.ModelSerializer) diff --git a/rest_framework/tests/test_serializer_nested.py b/rest_framework/tests/test_serializer_nested.py index d0a773feb..6d69ffbd0 100644 --- a/rest_framework/tests/test_serializer_nested.py +++ b/rest_framework/tests/test_serializer_nested.py @@ -6,7 +6,6 @@ Doesn't cover model serializers. from __future__ import unicode_literals from django.test import TestCase from rest_framework import serializers -from rest_framework.tests.accounts.serializers import AccountSerializer from . import models @@ -346,8 +345,3 @@ class NestedModelSerializerUpdateTests(TestCase): result = deserialize.object result.save() self.assertEqual(result.id, john.id) - - -class ImportingModelSerializerWithStrForeignKeys(TestCase): - def test_import_model_serializer(self): - self.assertIsInstance(AccountSerializer(), serializers.ModelSerializer)