From 633d4eae8b8b91c66f023c87e4339147213039c2 Mon Sep 17 00:00:00 2001 From: Carlton Gibson Date: Tue, 28 Nov 2017 09:14:16 +0100 Subject: [PATCH 1/6] Extract method for `manual_fields` processing Allows reuse of logic to replace Field instances in a field list by `Field.name`. Adds a utility function for the logic plus a wrapper method on `AutoSchema`. Closes #5632 --- rest_framework/schemas/inspectors.py | 33 +++++++++++++++++++++++----- 1 file changed, 28 insertions(+), 5 deletions(-) diff --git a/rest_framework/schemas/inspectors.py b/rest_framework/schemas/inspectors.py index 47f5b9e13e..b1e461c54e 100644 --- a/rest_framework/schemas/inspectors.py +++ b/rest_framework/schemas/inspectors.py @@ -105,6 +105,25 @@ def get_pk_description(model, model_field): ) +def update_fields(fields, update_with): + """ + Update list of coreapi.Field instances, overwriting on `Field.name`. + + Utility function to handle replacing coreapi.Field fields + from a list by name. Used to handle `manual_fields`. + + Parameters: + + * `fields`: list of `coreapi.Field` instances to update + * `update_with: list of `coreapi.Field` instances to add or replace. + """ + by_name = {f.name: f for f in fields} + for f in update_with: + by_name[f.name] = f + fields = list(by_name.values()) + return fields + + class ViewInspector(object): """ Descriptor class on APIView. @@ -181,11 +200,7 @@ def get_link(self, path, method, base_url): fields += self.get_pagination_fields(path, method) fields += self.get_filter_fields(path, method) - if self._manual_fields is not None: - by_name = {f.name: f for f in fields} - for f in self._manual_fields: - by_name[f.name] = f - fields = list(by_name.values()) + fields = self.update_manual_fields(fields) if fields and any([field.location in ('form', 'body') for field in fields]): encoding = self.get_encoding(path, method) @@ -379,6 +394,14 @@ def get_filter_fields(self, path, method): fields += filter_backend().get_schema_fields(self.view) return fields + def update_manual_fields(self, fields): + """ + Adjust `fields` with `manual_fields` + """ + if self._manual_fields is not None: + fields = update_fields(fields, self._manual_fields) + return fields + def get_encoding(self, path, method): """ Return the 'encoding' parameter to use for a given endpoint. From 322cf8ebc99bc22669a2d76b775491bbd842ec98 Mon Sep 17 00:00:00 2001 From: Ryan P Kilby Date: Fri, 1 Dec 2017 04:10:36 -0500 Subject: [PATCH 2/6] Manual fields suggestions (#2) * Use OrderedDict in inspectors * Move empty check to 'update_fields()' * Make 'update_fields()' an AutoSchema staticmethod * Add 'AutoSchema.get_manual_fields()' * Conform '.get_manual_fields()' to other methods --- rest_framework/schemas/inspectors.py | 47 ++++++++++++++-------------- 1 file changed, 23 insertions(+), 24 deletions(-) diff --git a/rest_framework/schemas/inspectors.py b/rest_framework/schemas/inspectors.py index b1e461c54e..240b713aca 100644 --- a/rest_framework/schemas/inspectors.py +++ b/rest_framework/schemas/inspectors.py @@ -105,25 +105,6 @@ def get_pk_description(model, model_field): ) -def update_fields(fields, update_with): - """ - Update list of coreapi.Field instances, overwriting on `Field.name`. - - Utility function to handle replacing coreapi.Field fields - from a list by name. Used to handle `manual_fields`. - - Parameters: - - * `fields`: list of `coreapi.Field` instances to update - * `update_with: list of `coreapi.Field` instances to add or replace. - """ - by_name = {f.name: f for f in fields} - for f in update_with: - by_name[f.name] = f - fields = list(by_name.values()) - return fields - - class ViewInspector(object): """ Descriptor class on APIView. @@ -200,7 +181,8 @@ def get_link(self, path, method, base_url): fields += self.get_pagination_fields(path, method) fields += self.get_filter_fields(path, method) - fields = self.update_manual_fields(fields) + manual_fields = self.get_manual_fields(path, method) + fields = self.update_fields(fields, manual_fields) if fields and any([field.location in ('form', 'body') for field in fields]): encoding = self.get_encoding(path, method) @@ -394,12 +376,29 @@ def get_filter_fields(self, path, method): fields += filter_backend().get_schema_fields(self.view) return fields - def update_manual_fields(self, fields): + def get_manual_fields(self, path, method): + return self._manual_fields + + @staticmethod + def update_fields(fields, update_with): """ - Adjust `fields` with `manual_fields` + Update list of coreapi.Field instances, overwriting on `Field.name`. + + Utility function to handle replacing coreapi.Field fields + from a list by name. Used to handle `manual_fields`. + + Parameters: + + * `fields`: list of `coreapi.Field` instances to update + * `update_with: list of `coreapi.Field` instances to add or replace. """ - if self._manual_fields is not None: - fields = update_fields(fields, self._manual_fields) + if not update_with: + return fields + + by_name = OrderedDict((f.name, f) for f in fields) + for f in update_with: + by_name[f.name] = f + fields = list(by_name.values()) return fields def get_encoding(self, path, method): From 122d843e40beaa753408c3db200c50a2b65bc318 Mon Sep 17 00:00:00 2001 From: Carlton Gibson Date: Fri, 1 Dec 2017 11:01:56 +0100 Subject: [PATCH 3/6] Add test for update_fields --- tests/test_schemas.py | 40 ++++++++++++++++++++++++++++++++++++++-- 1 file changed, 38 insertions(+), 2 deletions(-) diff --git a/tests/test_schemas.py b/tests/test_schemas.py index 56692d4f59..ba561a9597 100644 --- a/tests/test_schemas.py +++ b/tests/test_schemas.py @@ -516,7 +516,7 @@ def test_4605_regression(self): assert prefix == '/' -class TestDescriptor(TestCase): +class TestAutoSchema(TestCase): def test_apiview_schema_descriptor(self): view = APIView() @@ -528,7 +528,43 @@ def test_get_link_requires_instance(self): with pytest.raises(AssertionError): descriptor.get_link(None, None, None) # ???: Do the dummy arguments require a tighter assert? - def test_manual_fields(self): + def test_update_fields(self): + """ + That updating fields by-name helper is correct + + Recall: `update_fields(fields, update_with)` + """ + schema = AutoSchema() + fields = [] + + # Adds a field... + fields = schema.update_fields(fields, [ + coreapi.Field( + "my_field", + required=True, + location="path", + schema=coreschema.String() + ), + ]) + + assert len(fields) == 1 + assert fields[0].name == "my_field" + + # Replaces a field... + fields = schema.update_fields(fields, [ + coreapi.Field( + "my_field", + required=False, + location="path", + schema=coreschema.String() + ), + ]) + + assert len(fields) == 1 + assert fields[0].required is False + + def test_get_manual_fields(self): + """That get_manual_fields is applied during get_link""" class CustomView(APIView): schema = AutoSchema(manual_fields=[ From 986469afbe4e72b95fbaa8b26d805865a5341888 Mon Sep 17 00:00:00 2001 From: Carlton Gibson Date: Fri, 1 Dec 2017 11:33:10 +0100 Subject: [PATCH 4/6] Make sure `manual_fields` is a list. (As documented to be) --- rest_framework/schemas/inspectors.py | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/rest_framework/schemas/inspectors.py b/rest_framework/schemas/inspectors.py index 240b713aca..008d7c0910 100644 --- a/rest_framework/schemas/inspectors.py +++ b/rest_framework/schemas/inspectors.py @@ -172,7 +172,8 @@ def __init__(self, manual_fields=None): * `manual_fields`: list of `coreapi.Field` instances that will be added to auto-generated fields, overwriting on `Field.name` """ - + if manual_fields is None: + manual_fields = [] self._manual_fields = manual_fields def get_link(self, path, method, base_url): From 49415ed640798e8ed316dbc29dc737ed19f932d0 Mon Sep 17 00:00:00 2001 From: Carlton Gibson Date: Fri, 1 Dec 2017 11:50:06 +0100 Subject: [PATCH 5/6] Add docs for new AutoSchema methods. * `get_manual_fields` * `update_fields` --- docs/api-guide/schemas.md | 25 +++++++++++++++++++++++++ 1 file changed, 25 insertions(+) diff --git a/docs/api-guide/schemas.md b/docs/api-guide/schemas.md index 22894a9782..2b83e0671b 100644 --- a/docs/api-guide/schemas.md +++ b/docs/api-guide/schemas.md @@ -603,6 +603,31 @@ Return a list of `coreapi.Link()` instances, as returned by the `get_schema_fiel Return a list of `coreapi.Link()` instances, as returned by the `get_schema_fields()` method of any filter classes used by the view. +### get_manual_fields(self, path, method) + +Return a list of `coreapi.Field()` instances to be added to or replace generated fields. Defaults to (optional) `manual_fields` passed to `AutoSchema` constructor. + +May be overridden to customise manual fields by `path` or `method`. For example, a per-method adjustment may look like this: + +```python +def get_manual_fields(self, path, method): + """Example adding per-method fields.""" + + extra_fields = [] + if method=='GET': + extra_fields = # ... list of extra fields for GET ... + if method=='POST': + extra_fields = # ... list of extra fields for POST ... + + manual_fields = super().get_manual_fields() + return manual_fields + extra_fields +``` + +### update_fields(fields, update_with) + +Utility `staticmethod`. Encapsulates logic to add or replace fields from a list +by `Field.name`. May be overridden to adjust replacement criteria. + ## ManualSchema From 7dc903fe2f081c603a942404dfcfc6c1c45f9cd6 Mon Sep 17 00:00:00 2001 From: Carlton Gibson Date: Fri, 1 Dec 2017 12:01:27 +0100 Subject: [PATCH 6/6] Add release notes for PR. --- docs/topics/release-notes.md | 19 +++++++++++++++++++ 1 file changed, 19 insertions(+) diff --git a/docs/topics/release-notes.md b/docs/topics/release-notes.md index 9ac3ab1005..600fef9c97 100644 --- a/docs/topics/release-notes.md +++ b/docs/topics/release-notes.md @@ -40,6 +40,25 @@ You can determine your currently installed version using `pip freeze`: ## 3.7.x series +### 3.7.4 + +**Date**: UNRELEASED + +* Extract method for `manual_fields` processing [#5633][gh5633] + + Allows for easier customisation of `manual_fields` processing, for example + to provide per-method manual fields. `AutoSchema` adds `get_manual_fields`, + as the intended override point, and a utility method `update_fields`, to + handle by-name field replacement from a list, which, in general, you are not + expected to override. + + Note: `AutoSchema.__init__` now ensures `manual_fields` is a list. + Previously may have been stored internally as `None`. + + +[gh5633]: https://github.com/encode/django-rest-framework/issues/5633 + + ### 3.7.3