From f2452936e9a33ce6eb95b11bd89b34fc6d2991a0 Mon Sep 17 00:00:00 2001 From: Ryan Hiebert Date: Thu, 5 Nov 2015 13:07:40 -0600 Subject: [PATCH 1/5] Allow no queryset when get_queryset overridden The user may wish to provide a dynamic queryset on a `RelatedField` based on the `context`. The way to do that is to create a subclass of `RelatedField` (or a child) and override the `get_queryset` method. However, this is undocumented, and instantiating that field without a `queryset` argument (because it's not needed) will raise an assertion error. Document `.get_queryset(self)` as an official part of the API of `RelatedField`, and don't enforce the use of `queryset` when `get_queryset` is overridden. --- docs/api-guide/relations.md | 2 ++ rest_framework/relations.py | 8 ++++---- tests/test_relations.py | 8 ++++++++ 3 files changed, 14 insertions(+), 4 deletions(-) diff --git a/docs/api-guide/relations.md b/docs/api-guide/relations.md index 51e5f78c4..d5a7bfc7f 100644 --- a/docs/api-guide/relations.md +++ b/docs/api-guide/relations.md @@ -330,6 +330,8 @@ To implement a custom relational field, you should override `RelatedField`, and If you want to implement a read-write relational field, you must also implement the `.to_internal_value(self, data)` method. +To provide a dynamic queryset based on the `context`, you can also override `.get_queryset(self)` instead of specifying `.queryset` on the class or when initializing the field. + ## Example For example, we could define a relational field to serialize a track to a custom string representation, using its ordering, title, and duration. diff --git a/rest_framework/relations.py b/rest_framework/relations.py index 40261f3f3..74e0f3e03 100644 --- a/rest_framework/relations.py +++ b/rest_framework/relations.py @@ -62,10 +62,6 @@ class RelatedField(Field): self.queryset = kwargs.pop('queryset', self.queryset) self.html_cutoff = kwargs.pop('html_cutoff', self.html_cutoff) self.html_cutoff_text = kwargs.pop('html_cutoff_text', self.html_cutoff_text) - assert self.queryset is not None or kwargs.get('read_only', None), ( - 'Relational field must provide a `queryset` argument, ' - 'or set read_only=`True`.' - ) assert not (self.queryset is not None and kwargs.get('read_only', None)), ( 'Relational fields should not provide a `queryset` argument, ' 'when setting read_only=`True`.' @@ -112,6 +108,10 @@ class RelatedField(Field): def get_queryset(self): queryset = self.queryset + assert queryset is not None, ( + 'Relational field must provide a `queryset` argument, ' + 'or set read_only=`True`.' + ) if isinstance(queryset, (QuerySet, Manager)): # Ensure queryset is re-evaluated whenever used. # Note that actually a `Manager` class may also be used as the diff --git a/tests/test_relations.py b/tests/test_relations.py index fd37e63e3..9703c3b9d 100644 --- a/tests/test_relations.py +++ b/tests/test_relations.py @@ -176,6 +176,14 @@ class TestSlugRelatedField(APISimpleTestCase): representation = self.field.to_representation(self.instance) assert representation == self.instance.name + def test_no_queryset_init(self): + class NoQuerySetSlugRelatedField(serializers.SlugRelatedField): + def get_queryset(this): + return self.queryset + + field = NoQuerySetSlugRelatedField(slug_field='name') + field.to_internal_value(self.instance.name) + class TestManyRelatedField(APISimpleTestCase): def setUp(self): From fe12816b829a5b73648ab367a9c3ddbdf292abaa Mon Sep 17 00:00:00 2001 From: Ryan Hiebert Date: Fri, 13 Nov 2015 15:16:27 -0600 Subject: [PATCH 2/5] Move default validation back into init method --- rest_framework/relations.py | 12 ++++++++---- 1 file changed, 8 insertions(+), 4 deletions(-) diff --git a/rest_framework/relations.py b/rest_framework/relations.py index 74e0f3e03..e2b46c121 100644 --- a/rest_framework/relations.py +++ b/rest_framework/relations.py @@ -62,6 +62,14 @@ class RelatedField(Field): self.queryset = kwargs.pop('queryset', self.queryset) self.html_cutoff = kwargs.pop('html_cutoff', self.html_cutoff) self.html_cutoff_text = kwargs.pop('html_cutoff_text', self.html_cutoff_text) + + default_get_queryset = getattr(RelatedField.get_queryset, '__func__', + RelatedField.get_queryset) # Python 2/3 + if self.get_queryset.__func__ == default_get_queryset: + assert self.queryset is not None or kwargs.get('read_only', None), ( + 'Relational field must provide a `queryset` argument, ' + 'or set read_only=`True`.' + ) assert not (self.queryset is not None and kwargs.get('read_only', None)), ( 'Relational fields should not provide a `queryset` argument, ' 'when setting read_only=`True`.' @@ -108,10 +116,6 @@ class RelatedField(Field): def get_queryset(self): queryset = self.queryset - assert queryset is not None, ( - 'Relational field must provide a `queryset` argument, ' - 'or set read_only=`True`.' - ) if isinstance(queryset, (QuerySet, Manager)): # Ensure queryset is re-evaluated whenever used. # Note that actually a `Manager` class may also be used as the From dca2de3a5ca3d4e4593628e2fdbe72ec7d6b6c8b Mon Sep 17 00:00:00 2001 From: Ryan Hiebert Date: Mon, 16 Nov 2015 12:21:58 -0600 Subject: [PATCH 3/5] Make the override check a utility function --- rest_framework/relations.py | 6 ++---- rest_framework/utils/overridden.py | 7 +++++++ 2 files changed, 9 insertions(+), 4 deletions(-) create mode 100644 rest_framework/utils/overridden.py diff --git a/rest_framework/relations.py b/rest_framework/relations.py index e2b46c121..00ca8ecbf 100644 --- a/rest_framework/relations.py +++ b/rest_framework/relations.py @@ -18,7 +18,7 @@ from rest_framework.fields import ( Field, empty, get_attribute, is_simple_callable, iter_options ) from rest_framework.reverse import reverse -from rest_framework.utils import html +from rest_framework.utils import html, overridden class Hyperlink(six.text_type): @@ -63,9 +63,7 @@ class RelatedField(Field): self.html_cutoff = kwargs.pop('html_cutoff', self.html_cutoff) self.html_cutoff_text = kwargs.pop('html_cutoff_text', self.html_cutoff_text) - default_get_queryset = getattr(RelatedField.get_queryset, '__func__', - RelatedField.get_queryset) # Python 2/3 - if self.get_queryset.__func__ == default_get_queryset: + if not overridden.method_overridden('get_queryset', RelatedField, self): assert self.queryset is not None or kwargs.get('read_only', None), ( 'Relational field must provide a `queryset` argument, ' 'or set read_only=`True`.' diff --git a/rest_framework/utils/overridden.py b/rest_framework/utils/overridden.py new file mode 100644 index 000000000..5402916c7 --- /dev/null +++ b/rest_framework/utils/overridden.py @@ -0,0 +1,7 @@ +def method_overridden(method_name, klass, instance): + """ + Determine if a method has been overridden. + """ + method = getattr(klass, method_name) + default_method = getattr(method, '__func__', method) # Python 3 compat + return default_method is not getattr(instance, method_name).__func__ From 389b48e3945b5e9b4f37593d57266f3226c5f87e Mon Sep 17 00:00:00 2001 From: Ryan Hiebert Date: Wed, 18 Nov 2015 08:19:27 -0600 Subject: [PATCH 4/5] Avoid making a new module for this function --- rest_framework/relations.py | 13 +++++++++++-- rest_framework/utils/overridden.py | 7 ------- 2 files changed, 11 insertions(+), 9 deletions(-) delete mode 100644 rest_framework/utils/overridden.py diff --git a/rest_framework/relations.py b/rest_framework/relations.py index 00ca8ecbf..996fa9306 100644 --- a/rest_framework/relations.py +++ b/rest_framework/relations.py @@ -18,7 +18,16 @@ from rest_framework.fields import ( Field, empty, get_attribute, is_simple_callable, iter_options ) from rest_framework.reverse import reverse -from rest_framework.utils import html, overridden +from rest_framework.utils import html + + +def method_overridden(method_name, klass, instance): + """ + Determine if a method has been overridden. + """ + method = getattr(klass, method_name) + default_method = getattr(method, '__func__', method) # Python 3 compat + return default_method is not getattr(instance, method_name).__func__ class Hyperlink(six.text_type): @@ -63,7 +72,7 @@ class RelatedField(Field): self.html_cutoff = kwargs.pop('html_cutoff', self.html_cutoff) self.html_cutoff_text = kwargs.pop('html_cutoff_text', self.html_cutoff_text) - if not overridden.method_overridden('get_queryset', RelatedField, self): + if not method_overridden('get_queryset', RelatedField, self): assert self.queryset is not None or kwargs.get('read_only', None), ( 'Relational field must provide a `queryset` argument, ' 'or set read_only=`True`.' diff --git a/rest_framework/utils/overridden.py b/rest_framework/utils/overridden.py deleted file mode 100644 index 5402916c7..000000000 --- a/rest_framework/utils/overridden.py +++ /dev/null @@ -1,7 +0,0 @@ -def method_overridden(method_name, klass, instance): - """ - Determine if a method has been overridden. - """ - method = getattr(klass, method_name) - default_method = getattr(method, '__func__', method) # Python 3 compat - return default_method is not getattr(instance, method_name).__func__ From a19f1520659299762200f43bcb63d66f18f6841a Mon Sep 17 00:00:00 2001 From: Ryan Hiebert Date: Wed, 18 Nov 2015 10:25:51 -0600 Subject: [PATCH 5/5] Note possibility of overriding get_queryset --- rest_framework/relations.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/rest_framework/relations.py b/rest_framework/relations.py index 996fa9306..feafd4930 100644 --- a/rest_framework/relations.py +++ b/rest_framework/relations.py @@ -75,7 +75,7 @@ class RelatedField(Field): if not method_overridden('get_queryset', RelatedField, self): assert self.queryset is not None or kwargs.get('read_only', None), ( 'Relational field must provide a `queryset` argument, ' - 'or set read_only=`True`.' + 'override `get_queryset`, or set read_only=`True`.' ) assert not (self.queryset is not None and kwargs.get('read_only', None)), ( 'Relational fields should not provide a `queryset` argument, '