From f8dfa6b0ab49ac4d69df693bc0aa60995ff99096 Mon Sep 17 00:00:00 2001 From: jokiefer Date: Thu, 2 Dec 2021 19:51:49 +0100 Subject: [PATCH 1/6] add error message to tell the user which serializer has the error --- rest_framework_json_api/utils.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/rest_framework_json_api/utils.py b/rest_framework_json_api/utils.py index 65cbc645..bbf9d269 100644 --- a/rest_framework_json_api/utils.py +++ b/rest_framework_json_api/utils.py @@ -320,7 +320,7 @@ def get_resource_type_from_serializer(serializer): return meta.resource_name elif hasattr(meta, "model"): return get_resource_type_from_model(meta.model) - raise AttributeError() + raise AttributeError(f"can not detect 'resource_name' on serializer ' {serializer.__class__}'") def get_included_resources(request, serializer=None): From 8572529045242296b91558df11d26040ad0930d7 Mon Sep 17 00:00:00 2001 From: jokiefer Date: Thu, 2 Dec 2021 21:29:12 +0100 Subject: [PATCH 2/6] * change error message with more details about module and line number of the given serializer * add test case to check the correct error message --- rest_framework_json_api/utils.py | 5 ++++- tests/test_utils.py | 15 +++++++++++++++ 2 files changed, 19 insertions(+), 1 deletion(-) diff --git a/rest_framework_json_api/utils.py b/rest_framework_json_api/utils.py index bbf9d269..dd7aadf3 100644 --- a/rest_framework_json_api/utils.py +++ b/rest_framework_json_api/utils.py @@ -320,7 +320,10 @@ def get_resource_type_from_serializer(serializer): return meta.resource_name elif hasattr(meta, "model"): return get_resource_type_from_model(meta.model) - raise AttributeError(f"can not detect 'resource_name' on serializer ' {serializer.__class__}'") + # inspect.currentframe().f_code.co_firstlineno + raise AttributeError( + f"can not detect 'resource_name' on serializer '{serializer.__class__.__name__}'" + f" in module '{serializer.__class__.__module__}:{inspect.getsourcelines(serializer.__class__)[-1]}'") def get_included_resources(request, serializer=None): diff --git a/tests/test_utils.py b/tests/test_utils.py index 43c12dcf..e3f0a6bd 100644 --- a/tests/test_utils.py +++ b/tests/test_utils.py @@ -1,6 +1,7 @@ import pytest from django.db import models from rest_framework import status +from rest_framework.fields import Field from rest_framework.generics import GenericAPIView from rest_framework.response import Response from rest_framework.views import APIView @@ -15,6 +16,7 @@ get_included_serializers, get_related_resource_type, get_resource_name, + get_resource_type_from_serializer, undo_format_field_name, undo_format_field_names, undo_format_link_segment, @@ -377,3 +379,16 @@ class Meta: } assert included_serializers == expected_included_serializers + +def test_get_resource_type_from_serializer_error_message(): + + class SerializerWithoutResourceName(serializers.Serializer): + something = Field() + + try: + get_resource_type_from_serializer( + SerializerWithoutResourceName() + ) + raise AssertionError('no AttributeError was raised') + except AttributeError as ex: + assert str(ex) == f"can not detect 'resource_name' on serializer 'SerializerWithoutResourceName' in module 'tests.test_utils:385'" From 7c3bb8bbffe77746cd8a7c945216de89dc36b30b Mon Sep 17 00:00:00 2001 From: jokiefer Date: Thu, 2 Dec 2021 21:34:36 +0100 Subject: [PATCH 3/6] add dynamic line calculation on test case --- tests/test_utils.py | 9 +++++---- 1 file changed, 5 insertions(+), 4 deletions(-) diff --git a/tests/test_utils.py b/tests/test_utils.py index e3f0a6bd..3f5b5e7f 100644 --- a/tests/test_utils.py +++ b/tests/test_utils.py @@ -1,4 +1,5 @@ import pytest +import inspect from django.db import models from rest_framework import status from rest_framework.fields import Field @@ -384,11 +385,11 @@ def test_get_resource_type_from_serializer_error_message(): class SerializerWithoutResourceName(serializers.Serializer): something = Field() + + serializer = SerializerWithoutResourceName() try: - get_resource_type_from_serializer( - SerializerWithoutResourceName() - ) + get_resource_type_from_serializer(serializer=serializer) raise AssertionError('no AttributeError was raised') except AttributeError as ex: - assert str(ex) == f"can not detect 'resource_name' on serializer 'SerializerWithoutResourceName' in module 'tests.test_utils:385'" + assert str(ex) == f"can not detect 'resource_name' on serializer 'SerializerWithoutResourceName' in module 'tests.test_utils:{inspect.getsourcelines(serializer.__class__)[-1]}'" From e07834783b7e3a62ccbaf80922bf33f90e0aacf5 Mon Sep 17 00:00:00 2001 From: jokiefer Date: Fri, 3 Dec 2021 13:01:42 +0100 Subject: [PATCH 4/6] fixes all suggestions from comment https://github.com/django-json-api/django-rest-framework-json-api/pull/1020#pullrequestreview-822524352 --- AUTHORS | 1 + CHANGELOG.md | 1 + rest_framework_json_api/utils.py | 5 ++--- tests/test_utils.py | 12 +++++------- 4 files changed, 9 insertions(+), 10 deletions(-) diff --git a/AUTHORS b/AUTHORS index 35d959c6..7fde73ac 100644 --- a/AUTHORS +++ b/AUTHORS @@ -45,3 +45,4 @@ Tim Selman Tom Glowka Ulrich Schuster Yaniv Peer +Jonas Kiefer \ No newline at end of file diff --git a/CHANGELOG.md b/CHANGELOG.md index 10559ddb..af467377 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -22,6 +22,7 @@ any parts of the framework not mentioned in the documentation should generally b * Raise comprehensible error when reserved field names `meta` and `results` are used. * Use `relationships` in the error object `pointer` when the field is actually a relationship. * Added missing inflection to the generated OpenAPI schema. +* Added error message if AttributeError is raised by `get_resource_type_from_serializer()` ### Changed diff --git a/rest_framework_json_api/utils.py b/rest_framework_json_api/utils.py index dd7aadf3..c0cd5ab9 100644 --- a/rest_framework_json_api/utils.py +++ b/rest_framework_json_api/utils.py @@ -320,10 +320,9 @@ def get_resource_type_from_serializer(serializer): return meta.resource_name elif hasattr(meta, "model"): return get_resource_type_from_model(meta.model) - # inspect.currentframe().f_code.co_firstlineno raise AttributeError( - f"can not detect 'resource_name' on serializer '{serializer.__class__.__name__}'" - f" in module '{serializer.__class__.__module__}:{inspect.getsourcelines(serializer.__class__)[-1]}'") + f"can not detect 'resource_name' on serializer '{serializer.__class__.__name__}'" + f" in module '{serializer.__class__.__module__}'") def get_included_resources(request, serializer=None): diff --git a/tests/test_utils.py b/tests/test_utils.py index 3f5b5e7f..861692f7 100644 --- a/tests/test_utils.py +++ b/tests/test_utils.py @@ -1,5 +1,4 @@ import pytest -import inspect from django.db import models from rest_framework import status from rest_framework.fields import Field @@ -381,15 +380,14 @@ class Meta: assert included_serializers == expected_included_serializers + def test_get_resource_type_from_serializer_error_message(): class SerializerWithoutResourceName(serializers.Serializer): something = Field() - + serializer = SerializerWithoutResourceName() - - try: + + with pytest.raises(AttributeError) as excinfo: get_resource_type_from_serializer(serializer=serializer) - raise AssertionError('no AttributeError was raised') - except AttributeError as ex: - assert str(ex) == f"can not detect 'resource_name' on serializer 'SerializerWithoutResourceName' in module 'tests.test_utils:{inspect.getsourcelines(serializer.__class__)[-1]}'" + assert str(excinfo.value) == "can not detect 'resource_name' on serializer 'SerializerWithoutResourceName' in module 'tests.test_utils'" From befb15fdb13ea2ce0ca0d04da7575fcf77e7ddc4 Mon Sep 17 00:00:00 2001 From: jokiefer Date: Fri, 3 Dec 2021 13:49:55 +0100 Subject: [PATCH 5/6] formatting with black --- rest_framework_json_api/utils.py | 3 ++- tests/test_utils.py | 6 ++++-- 2 files changed, 6 insertions(+), 3 deletions(-) diff --git a/rest_framework_json_api/utils.py b/rest_framework_json_api/utils.py index c0cd5ab9..6189093f 100644 --- a/rest_framework_json_api/utils.py +++ b/rest_framework_json_api/utils.py @@ -322,7 +322,8 @@ def get_resource_type_from_serializer(serializer): return get_resource_type_from_model(meta.model) raise AttributeError( f"can not detect 'resource_name' on serializer '{serializer.__class__.__name__}'" - f" in module '{serializer.__class__.__module__}'") + f" in module '{serializer.__class__.__module__}'" + ) def get_included_resources(request, serializer=None): diff --git a/tests/test_utils.py b/tests/test_utils.py index 861692f7..c6851136 100644 --- a/tests/test_utils.py +++ b/tests/test_utils.py @@ -382,7 +382,6 @@ class Meta: def test_get_resource_type_from_serializer_error_message(): - class SerializerWithoutResourceName(serializers.Serializer): something = Field() @@ -390,4 +389,7 @@ class SerializerWithoutResourceName(serializers.Serializer): with pytest.raises(AttributeError) as excinfo: get_resource_type_from_serializer(serializer=serializer) - assert str(excinfo.value) == "can not detect 'resource_name' on serializer 'SerializerWithoutResourceName' in module 'tests.test_utils'" + assert ( + str(excinfo.value) + == "can not detect 'resource_name' on serializer 'SerializerWithoutResourceName' in module 'tests.test_utils'" + ) From b2fdf2eb21d075839957795b058db61c0307566d Mon Sep 17 00:00:00 2001 From: Oliver Sauder Date: Sun, 5 Dec 2021 20:12:36 +0400 Subject: [PATCH 6/6] Minor cleanup --- AUTHORS | 2 +- CHANGELOG.md | 2 +- tests/test_utils.py | 8 ++++---- 3 files changed, 6 insertions(+), 6 deletions(-) diff --git a/AUTHORS b/AUTHORS index 7fde73ac..fa91e9b2 100644 --- a/AUTHORS +++ b/AUTHORS @@ -15,6 +15,7 @@ Jamie Bliss Jason Housley Jeppe Fihl-Pearson Jerel Unruh +Jonas Kiefer Jonas Metzener Jonathan Senecal Joseba Mendivil @@ -45,4 +46,3 @@ Tim Selman Tom Glowka Ulrich Schuster Yaniv Peer -Jonas Kiefer \ No newline at end of file diff --git a/CHANGELOG.md b/CHANGELOG.md index af467377..1fa3d92e 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -22,7 +22,7 @@ any parts of the framework not mentioned in the documentation should generally b * Raise comprehensible error when reserved field names `meta` and `results` are used. * Use `relationships` in the error object `pointer` when the field is actually a relationship. * Added missing inflection to the generated OpenAPI schema. -* Added error message if AttributeError is raised by `get_resource_type_from_serializer()` +* Added missing error message when `resource_name` is not properly configured. ### Changed diff --git a/tests/test_utils.py b/tests/test_utils.py index c6851136..d8810c0b 100644 --- a/tests/test_utils.py +++ b/tests/test_utils.py @@ -381,7 +381,7 @@ class Meta: assert included_serializers == expected_included_serializers -def test_get_resource_type_from_serializer_error_message(): +def test_get_resource_type_from_serializer_without_resource_name_raises_error(): class SerializerWithoutResourceName(serializers.Serializer): something = Field() @@ -389,7 +389,7 @@ class SerializerWithoutResourceName(serializers.Serializer): with pytest.raises(AttributeError) as excinfo: get_resource_type_from_serializer(serializer=serializer) - assert ( - str(excinfo.value) - == "can not detect 'resource_name' on serializer 'SerializerWithoutResourceName' in module 'tests.test_utils'" + assert str(excinfo.value) == ( + "can not detect 'resource_name' on serializer " + "'SerializerWithoutResourceName' in module 'tests.test_utils'" )