From b53a2f74edf7402b0e10193d7496184641ba8e70 Mon Sep 17 00:00:00 2001 From: Pavel Savchenko Date: Thu, 1 Nov 2012 08:36:26 +0200 Subject: [PATCH 1/8] Do not rely that `primary_key` will always be first field, Add support for specifying `fields = ('pk',)` to serialize the primary key field (no matter how the field is actually called) --- rest_framework/serializers.py | 10 +++++++--- 1 file changed, 7 insertions(+), 3 deletions(-) diff --git a/rest_framework/serializers.py b/rest_framework/serializers.py index 3d134a741..9fd1a4c95 100644 --- a/rest_framework/serializers.py +++ b/rest_framework/serializers.py @@ -126,11 +126,16 @@ class BaseSerializer(Field): for key, val in fields.items(): if key not in ret: ret[key] = val + if value.source.primary_key: + pk_field = key # If 'fields' is specified, use those fields, in that order. if self.opts.fields: new = SortedDict() for key in self.opts.fields: + if key == 'pk': + # User requested the 'pk', use the primary key found + new[key] = ret[pk_field] new[key] = ret[key] ret = new @@ -344,12 +349,11 @@ class ModelSerializer(Serializer): fields += [field for field in opts.many_to_many if field.serialize] ret = SortedDict() - is_pk = True # First field in the list is the pk for model_field in fields: - if is_pk: + if model_field.primary_key: + # Django requires models to have only one primary_key so this should be safe field = self.get_pk_field(model_field) - is_pk = False elif model_field.rel and nested: field = self.get_nested_field(model_field) elif model_field.rel: From 7a127415246803ca2bf6cf073d3b9aa567841dc5 Mon Sep 17 00:00:00 2001 From: Pavel Savchenko Date: Thu, 1 Nov 2012 08:41:37 +0200 Subject: [PATCH 2/8] correct value to val --- 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 9fd1a4c95..47c544b39 100644 --- a/rest_framework/serializers.py +++ b/rest_framework/serializers.py @@ -126,7 +126,7 @@ class BaseSerializer(Field): for key, val in fields.items(): if key not in ret: ret[key] = val - if value.source.primary_key: + if val.source.primary_key: pk_field = key # If 'fields' is specified, use those fields, in that order. From 19bcc3c86539e215e5668fac93cfc25e63166a3e Mon Sep 17 00:00:00 2001 From: Pavel Savchenko Date: Thu, 1 Nov 2012 08:51:55 +0200 Subject: [PATCH 3/8] use getattr to choose the pk field --- rest_framework/serializers.py | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/rest_framework/serializers.py b/rest_framework/serializers.py index 47c544b39..5443d2d67 100644 --- a/rest_framework/serializers.py +++ b/rest_framework/serializers.py @@ -126,8 +126,11 @@ class BaseSerializer(Field): for key, val in fields.items(): if key not in ret: ret[key] = val - if val.source.primary_key: - pk_field = key + try: + if getattr(val.source, primary_key): + pk_field = key + except AttributeError: + pass # If 'fields' is specified, use those fields, in that order. if self.opts.fields: From bd24d83333698bc8f4b923d1b45879dc9541c638 Mon Sep 17 00:00:00 2001 From: Pavel Savchenko Date: Thu, 1 Nov 2012 08:53:13 +0200 Subject: [PATCH 4/8] actually look for the right attribute --- 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 5443d2d67..67e0cc114 100644 --- a/rest_framework/serializers.py +++ b/rest_framework/serializers.py @@ -127,7 +127,7 @@ class BaseSerializer(Field): if key not in ret: ret[key] = val try: - if getattr(val.source, primary_key): + if getattr(val.source, 'primary_key'): pk_field = key except AttributeError: pass From e01b0d9ec036d300c3091009d7bfe979380b63e1 Mon Sep 17 00:00:00 2001 From: Pavel Savchenko Date: Thu, 1 Nov 2012 08:59:19 +0200 Subject: [PATCH 5/8] Comments --- rest_framework/serializers.py | 9 +++++++-- 1 file changed, 7 insertions(+), 2 deletions(-) diff --git a/rest_framework/serializers.py b/rest_framework/serializers.py index 67e0cc114..8f2e315ee 100644 --- a/rest_framework/serializers.py +++ b/rest_framework/serializers.py @@ -127,7 +127,8 @@ class BaseSerializer(Field): if key not in ret: ret[key] = val try: - if getattr(val.source, 'primary_key'): + # Test if field was marked as pk_field + if getattr(val, 'is_pk_field'): pk_field = key except AttributeError: pass @@ -139,6 +140,7 @@ class BaseSerializer(Field): if key == 'pk': # User requested the 'pk', use the primary key found new[key] = ret[pk_field] + continue new[key] = ret[key] ret = new @@ -376,7 +378,10 @@ class ModelSerializer(Serializer): """ Returns a default instance of the pk field. """ - return Field() + field = Field() + # Mark field as primary key + field.is_pk_field = True + return field def get_nested_field(self, model_field): """ From c973a3b8ae6e81d9ce452c039f4816849e2551a3 Mon Sep 17 00:00:00 2001 From: Pavel Savchenko Date: Tue, 6 Nov 2012 22:04:09 +0200 Subject: [PATCH 6/8] add tests and doc changes for 'pk' #350, modify get_fields to allow excluding using 'pk' shorctut as well --- docs/api-guide/serializers.md | 2 ++ docs/tutorial/1-serialization.md | 2 +- docs/tutorial/4-authentication-and-permissions.md | 2 +- rest_framework/serializers.py | 7 ++++++- rest_framework/tests/serializer.py | 12 ++++++++++-- setup.py | 2 +- 6 files changed, 21 insertions(+), 6 deletions(-) diff --git a/docs/api-guide/serializers.md b/docs/api-guide/serializers.md index ee7f72dd7..c76329e50 100644 --- a/docs/api-guide/serializers.md +++ b/docs/api-guide/serializers.md @@ -236,6 +236,8 @@ For example: model = Account exclude = ('id',) +**Note**: All fields names in either `fields` or `exclude` must already be defined in the model or set explicitly, with the exception of `pk` which is a shortcut field to the actual primary key field of the model (by default Django sets this to `id` but can be overidden). + ## Specifiying nested serialization The default `ModelSerializer` uses primary keys for relationships, but you can also easily generate nested representations using the `depth` option: diff --git a/docs/tutorial/1-serialization.md b/docs/tutorial/1-serialization.md index ba64f2aa7..bdddc4d80 100644 --- a/docs/tutorial/1-serialization.md +++ b/docs/tutorial/1-serialization.md @@ -201,7 +201,7 @@ Open the file `snippets/serializers.py` again, and edit the `SnippetSerializer` class SnippetSerializer(serializers.ModelSerializer): class Meta: model = Snippet - fields = ('id', 'title', 'code', 'linenos', 'language', 'style') + fields = ('pk', 'title', 'code', 'linenos', 'language', 'style') diff --git a/docs/tutorial/4-authentication-and-permissions.md b/docs/tutorial/4-authentication-and-permissions.md index f85250bea..ba44edda4 100644 --- a/docs/tutorial/4-authentication-and-permissions.md +++ b/docs/tutorial/4-authentication-and-permissions.md @@ -59,7 +59,7 @@ Now that we've got some users to work with, we'd better add representations of t class Meta: model = User - fields = ('id', 'username', 'snippets') + fields = ('pk', 'username', 'snippets') Because `'snippets'` is a *reverse* relationship on the User model, it will not be included by default when using the `ModelSerializer` class, so we've needed to add an explicit field for it. diff --git a/rest_framework/serializers.py b/rest_framework/serializers.py index 8985c8189..29aa202af 100644 --- a/rest_framework/serializers.py +++ b/rest_framework/serializers.py @@ -130,6 +130,8 @@ class BaseSerializer(Field): # Set up the field field.initialize(parent=self, field_name=key) + pk_field = None + # Add in the default fields fields = self.default_fields(nested) for key, val in fields.items(): @@ -147,7 +149,7 @@ class BaseSerializer(Field): new = SortedDict() for key in self.opts.fields: if key == 'pk': - # User requested the 'pk', use the primary key found + # 'pk' shortcut: use the primary key found new[key] = ret[pk_field] continue new[key] = ret[key] @@ -156,6 +158,9 @@ class BaseSerializer(Field): # Remove anything in 'exclude' if self.opts.exclude: for key in self.opts.exclude: + if key == 'pk': + # 'pk' shortcut: remove the primary key found + ret.pop(pk_field) ret.pop(key, None) return ret diff --git a/rest_framework/tests/serializer.py b/rest_framework/tests/serializer.py index 8d1de4298..7b4c9f0ec 100644 --- a/rest_framework/tests/serializer.py +++ b/rest_framework/tests/serializer.py @@ -50,7 +50,7 @@ class PersonSerializer(serializers.ModelSerializer): class Meta: model = Person - fields = ('name', 'age', 'info') + fields = ('pk', 'name', 'age', 'info') class BasicTests(TestCase): @@ -112,7 +112,7 @@ class BasicTests(TestCase): """ serializer = PersonSerializer(self.person) self.assertEquals(set(serializer.data.keys()), - set(['name', 'age', 'info'])) + set(['pk', 'name', 'age', 'info'])) def test_field_with_dictionary(self): """ Make sure that dictionaries from fields are left intact @@ -121,6 +121,14 @@ class BasicTests(TestCase): expected = self.person_data self.assertEquals(serializer.data['info'], expected) + def test_pk_field_exists(self): + """ + Verify the `pk` shortcut for primary key (by default: `id`) + """ + + serializer = PersonSerializer(self.person) + self.assertEquals(serializer.data['pk'], self.person.id) + class ValidationTests(TestCase): def setUp(self): diff --git a/setup.py b/setup.py index 26d072837..95b4b9144 100755 --- a/setup.py +++ b/setup.py @@ -62,7 +62,7 @@ setup( author_email='tom@tomchristie.com', packages=get_packages('rest_framework'), package_data=get_package_data('rest_framework'), - test_suite='rest_framework.runtests.runtests.main', + test_suite='rest_framework.runtests.runcoverage.main', install_requires=[], classifiers=[ 'Development Status :: 4 - Beta', From 2b264cc3e82d4b1e5abb63d89052b91d73512676 Mon Sep 17 00:00:00 2001 From: Pavel Savchenko Date: Tue, 6 Nov 2012 22:04:09 +0200 Subject: [PATCH 7/8] add tests and doc changes for 'pk' #350, modify get_fields to allow excluding using 'pk' shorctut as well **Ammend**: remove un-intended change in setup.py --- docs/api-guide/serializers.md | 2 ++ docs/tutorial/1-serialization.md | 2 +- docs/tutorial/4-authentication-and-permissions.md | 2 +- rest_framework/serializers.py | 7 ++++++- rest_framework/tests/serializer.py | 12 ++++++++++-- 5 files changed, 20 insertions(+), 5 deletions(-) diff --git a/docs/api-guide/serializers.md b/docs/api-guide/serializers.md index ee7f72dd7..c76329e50 100644 --- a/docs/api-guide/serializers.md +++ b/docs/api-guide/serializers.md @@ -236,6 +236,8 @@ For example: model = Account exclude = ('id',) +**Note**: All fields names in either `fields` or `exclude` must already be defined in the model or set explicitly, with the exception of `pk` which is a shortcut field to the actual primary key field of the model (by default Django sets this to `id` but can be overidden). + ## Specifiying nested serialization The default `ModelSerializer` uses primary keys for relationships, but you can also easily generate nested representations using the `depth` option: diff --git a/docs/tutorial/1-serialization.md b/docs/tutorial/1-serialization.md index ba64f2aa7..bdddc4d80 100644 --- a/docs/tutorial/1-serialization.md +++ b/docs/tutorial/1-serialization.md @@ -201,7 +201,7 @@ Open the file `snippets/serializers.py` again, and edit the `SnippetSerializer` class SnippetSerializer(serializers.ModelSerializer): class Meta: model = Snippet - fields = ('id', 'title', 'code', 'linenos', 'language', 'style') + fields = ('pk', 'title', 'code', 'linenos', 'language', 'style') diff --git a/docs/tutorial/4-authentication-and-permissions.md b/docs/tutorial/4-authentication-and-permissions.md index f85250bea..ba44edda4 100644 --- a/docs/tutorial/4-authentication-and-permissions.md +++ b/docs/tutorial/4-authentication-and-permissions.md @@ -59,7 +59,7 @@ Now that we've got some users to work with, we'd better add representations of t class Meta: model = User - fields = ('id', 'username', 'snippets') + fields = ('pk', 'username', 'snippets') Because `'snippets'` is a *reverse* relationship on the User model, it will not be included by default when using the `ModelSerializer` class, so we've needed to add an explicit field for it. diff --git a/rest_framework/serializers.py b/rest_framework/serializers.py index 8985c8189..29aa202af 100644 --- a/rest_framework/serializers.py +++ b/rest_framework/serializers.py @@ -130,6 +130,8 @@ class BaseSerializer(Field): # Set up the field field.initialize(parent=self, field_name=key) + pk_field = None + # Add in the default fields fields = self.default_fields(nested) for key, val in fields.items(): @@ -147,7 +149,7 @@ class BaseSerializer(Field): new = SortedDict() for key in self.opts.fields: if key == 'pk': - # User requested the 'pk', use the primary key found + # 'pk' shortcut: use the primary key found new[key] = ret[pk_field] continue new[key] = ret[key] @@ -156,6 +158,9 @@ class BaseSerializer(Field): # Remove anything in 'exclude' if self.opts.exclude: for key in self.opts.exclude: + if key == 'pk': + # 'pk' shortcut: remove the primary key found + ret.pop(pk_field) ret.pop(key, None) return ret diff --git a/rest_framework/tests/serializer.py b/rest_framework/tests/serializer.py index 8d1de4298..7b4c9f0ec 100644 --- a/rest_framework/tests/serializer.py +++ b/rest_framework/tests/serializer.py @@ -50,7 +50,7 @@ class PersonSerializer(serializers.ModelSerializer): class Meta: model = Person - fields = ('name', 'age', 'info') + fields = ('pk', 'name', 'age', 'info') class BasicTests(TestCase): @@ -112,7 +112,7 @@ class BasicTests(TestCase): """ serializer = PersonSerializer(self.person) self.assertEquals(set(serializer.data.keys()), - set(['name', 'age', 'info'])) + set(['pk', 'name', 'age', 'info'])) def test_field_with_dictionary(self): """ Make sure that dictionaries from fields are left intact @@ -121,6 +121,14 @@ class BasicTests(TestCase): expected = self.person_data self.assertEquals(serializer.data['info'], expected) + def test_pk_field_exists(self): + """ + Verify the `pk` shortcut for primary key (by default: `id`) + """ + + serializer = PersonSerializer(self.person) + self.assertEquals(serializer.data['pk'], self.person.id) + class ValidationTests(TestCase): def setUp(self): From b6841df16a356faf240440718412e2ac9cfb3b5c Mon Sep 17 00:00:00 2001 From: Pavel Savchenko Date: Tue, 6 Nov 2012 23:16:46 +0200 Subject: [PATCH 8/8] remove un-intended change in setup.py --- setup.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/setup.py b/setup.py index 95b4b9144..26d072837 100755 --- a/setup.py +++ b/setup.py @@ -62,7 +62,7 @@ setup( author_email='tom@tomchristie.com', packages=get_packages('rest_framework'), package_data=get_package_data('rest_framework'), - test_suite='rest_framework.runtests.runcoverage.main', + test_suite='rest_framework.runtests.runtests.main', install_requires=[], classifiers=[ 'Development Status :: 4 - Beta',