diff --git a/example/settings/dev.py b/example/settings/dev.py index 80482ffa..8e13ec15 100644 --- a/example/settings/dev.py +++ b/example/settings/dev.py @@ -68,7 +68,7 @@ REST_FRAMEWORK = { "PAGE_SIZE": 5, "EXCEPTION_HANDLER": "rest_framework_json_api.exceptions.exception_handler", - "DEFAULT_PAGINATION_CLASS": "rest_framework_json_api.pagination.JsonApiPageNumberPagination", + "DEFAULT_PAGINATION_CLASS": "rest_framework_json_api.pagination.JsonApiPageNumberPagination", # noqa: B950 "DEFAULT_PARSER_CLASSES": ( "rest_framework_json_api.parsers.JSONParser", "rest_framework.parsers.FormParser", diff --git a/example/tests/integration/test_non_paginated_responses.py b/example/tests/integration/test_non_paginated_responses.py index 5a2e59c8..3483b7f4 100644 --- a/example/tests/integration/test_non_paginated_responses.py +++ b/example/tests/integration/test_non_paginated_responses.py @@ -29,7 +29,7 @@ def test_multiple_entries_no_pagination(multiple_entries, client): "blogHyperlinked": { "links": { "related": "http://testserver/entries/1/blog", - "self": "http://testserver/entries/1/relationships/blog_hyperlinked", + "self": "http://testserver/entries/1/relationships/blog_hyperlinked", # noqa: B950 } }, "authors": { @@ -43,7 +43,7 @@ def test_multiple_entries_no_pagination(multiple_entries, client): "commentsHyperlinked": { "links": { "related": "http://testserver/entries/1/comments", - "self": "http://testserver/entries/1/relationships/comments_hyperlinked", + "self": "http://testserver/entries/1/relationships/comments_hyperlinked", # noqa: B950 } }, "suggested": { @@ -63,7 +63,7 @@ def test_multiple_entries_no_pagination(multiple_entries, client): "featuredHyperlinked": { "links": { "related": "http://testserver/entries/1/featured", - "self": "http://testserver/entries/1/relationships/featured_hyperlinked", + "self": "http://testserver/entries/1/relationships/featured_hyperlinked", # noqa: B950 } }, "tags": {"data": [], "meta": {"count": 0}}, @@ -84,7 +84,7 @@ def test_multiple_entries_no_pagination(multiple_entries, client): "blogHyperlinked": { "links": { "related": "http://testserver/entries/2/blog", - "self": "http://testserver/entries/2/relationships/blog_hyperlinked", + "self": "http://testserver/entries/2/relationships/blog_hyperlinked", # noqa: B950 } }, "authors": { @@ -98,7 +98,7 @@ def test_multiple_entries_no_pagination(multiple_entries, client): "commentsHyperlinked": { "links": { "related": "http://testserver/entries/2/comments", - "self": "http://testserver/entries/2/relationships/comments_hyperlinked", + "self": "http://testserver/entries/2/relationships/comments_hyperlinked", # noqa: B950 } }, "suggested": { @@ -118,7 +118,7 @@ def test_multiple_entries_no_pagination(multiple_entries, client): "featuredHyperlinked": { "links": { "related": "http://testserver/entries/2/featured", - "self": "http://testserver/entries/2/relationships/featured_hyperlinked", + "self": "http://testserver/entries/2/relationships/featured_hyperlinked", # noqa: B950 } }, "tags": {"data": [], "meta": {"count": 0}}, diff --git a/example/tests/integration/test_pagination.py b/example/tests/integration/test_pagination.py index aedf8c6c..c723fce1 100644 --- a/example/tests/integration/test_pagination.py +++ b/example/tests/integration/test_pagination.py @@ -29,7 +29,7 @@ def test_pagination_with_single_entry(single_entry, client): "blogHyperlinked": { "links": { "related": "http://testserver/entries/1/blog", - "self": "http://testserver/entries/1/relationships/blog_hyperlinked", + "self": "http://testserver/entries/1/relationships/blog_hyperlinked", # noqa: B950 } }, "authors": { @@ -43,7 +43,7 @@ def test_pagination_with_single_entry(single_entry, client): "commentsHyperlinked": { "links": { "related": "http://testserver/entries/1/comments", - "self": "http://testserver/entries/1/relationships/comments_hyperlinked", + "self": "http://testserver/entries/1/relationships/comments_hyperlinked", # noqa: B950 } }, "suggested": { @@ -63,7 +63,7 @@ def test_pagination_with_single_entry(single_entry, client): "featuredHyperlinked": { "links": { "related": "http://testserver/entries/1/featured", - "self": "http://testserver/entries/1/relationships/featured_hyperlinked", + "self": "http://testserver/entries/1/relationships/featured_hyperlinked", # noqa: B950 } }, "tags": { diff --git a/example/tests/integration/test_polymorphism.py b/example/tests/integration/test_polymorphism.py index 69a97220..116243f1 100644 --- a/example/tests/integration/test_polymorphism.py +++ b/example/tests/integration/test_polymorphism.py @@ -33,7 +33,7 @@ def test_polymorphism_on_detail_relations(single_company, client): def test_polymorphism_on_included_relations(single_company, client): response = client.get( reverse("company-detail", kwargs={"pk": single_company.pk}) - + "?include=current_project,future_projects,current_art_project,current_research_project" + + "?include=current_project,future_projects,current_art_project,current_research_project" # noqa: B950 ) content = response.json() assert ( @@ -169,14 +169,14 @@ def test_invalid_type_on_polymorphic_model(client): try: assert ( content["errors"][0]["detail"] - == "The resource object's type (invalidProjects) is not the type that constitute the " + == "The resource object's type (invalidProjects) is not the type that constitute the " # noqa: B950 "collection represented by the endpoint (one of [researchProjects, artProjects])." ) except AssertionError: # Available type list order isn't enforced assert ( content["errors"][0]["detail"] - == "The resource object's type (invalidProjects) is not the type that constitute the " + == "The resource object's type (invalidProjects) is not the type that constitute the " # noqa: B950 "collection represented by the endpoint (one of [artProjects, researchProjects])." ) diff --git a/example/tests/test_filters.py b/example/tests/test_filters.py index 5bb12121..ab74dd0b 100644 --- a/example/tests/test_filters.py +++ b/example/tests/test_filters.py @@ -482,7 +482,7 @@ def test_search_keywords(self): "blog": {"data": {"type": "blogs", "id": "1"}}, "blogHyperlinked": { "links": { - "self": "http://testserver/entries/7/relationships/blog_hyperlinked", # noqa: E501 + "self": "http://testserver/entries/7/relationships/blog_hyperlinked", # noqa: B950 "related": "http://testserver/entries/7/blog", } }, @@ -490,7 +490,7 @@ def test_search_keywords(self): "comments": {"meta": {"count": 0}, "data": []}, "commentsHyperlinked": { "links": { - "self": "http://testserver/entries/7/relationships/comments_hyperlinked", # noqa: E501 + "self": "http://testserver/entries/7/relationships/comments_hyperlinked", # noqa: B950 "related": "http://testserver/entries/7/comments", } }, @@ -515,14 +515,14 @@ def test_search_keywords(self): }, "suggestedHyperlinked": { "links": { - "self": "http://testserver/entries/7/relationships/suggested_hyperlinked", # noqa: E501 + "self": "http://testserver/entries/7/relationships/suggested_hyperlinked", # noqa: B950 "related": "http://testserver/entries/7/suggested/", } }, "tags": {"data": [], "meta": {"count": 0}}, "featuredHyperlinked": { "links": { - "self": "http://testserver/entries/7/relationships/featured_hyperlinked", # noqa: E501 + "self": "http://testserver/entries/7/relationships/featured_hyperlinked", # noqa: B950 "related": "http://testserver/entries/7/featured", } }, @@ -550,7 +550,7 @@ def test_search_multiple_keywords(self): 1. For each keyword, search the 4 search_fields for a match and then get the result set which is the union of all results for the given keyword. 2. Intersect those results sets such that *all* keywords are represented. - See `example/fixtures/blogentry.json` for the test content that the searches are based on. + See `example/fixtures/blogentry.json` for what test content the searches are based on. The searches test for both direct entries and related blogs across multiple fields. """ for searches in ( diff --git a/example/tests/test_views.py b/example/tests/test_views.py index f6947fef..ad3fe124 100644 --- a/example/tests/test_views.py +++ b/example/tests/test_views.py @@ -562,8 +562,8 @@ def test_if_returns_error_on_bad_endpoint_name(self): expected = [ { "detail": ( - "The resource object's type (bad) is not the type that constitute the collection " - "represented by the endpoint (blogs)." + "The resource object's type (bad) is not the type that constitute the " + "collection represented by the endpoint (blogs)." ), "source": {"pointer": "/data"}, "status": "409", diff --git a/example/views.py b/example/views.py index edb49ba8..b0d92811 100644 --- a/example/views.py +++ b/example/views.py @@ -144,8 +144,8 @@ class NoPagination(JsonApiPageNumberPagination): class NonPaginatedEntryViewSet(EntryViewSet): pagination_class = NoPagination - # override the default filter backends in order to test QueryParameterValidationFilter without - # breaking older usage of non-standard query params like `page_size`. + # override the default filter backends in order to test QueryParameterValidationFilter + # without breaking older usage of non-standard query params like `page_size`. filter_backends = ( QueryParameterValidationFilter, OrderingFilter, diff --git a/requirements/requirements-codestyle.txt b/requirements/requirements-codestyle.txt index 2b00c11f..f60aab51 100644 --- a/requirements/requirements-codestyle.txt +++ b/requirements/requirements-codestyle.txt @@ -1,4 +1,5 @@ black==22.10.0 flake8==5.0.4 +flake8-bugbear==22.10.27 flake8-isort==5.0.0 isort==5.10.1 diff --git a/rest_framework_json_api/django_filters/backends.py b/rest_framework_json_api/django_filters/backends.py index 365831c7..83495004 100644 --- a/rest_framework_json_api/django_filters/backends.py +++ b/rest_framework_json_api/django_filters/backends.py @@ -44,7 +44,9 @@ class DjangoFilterBackend(DjangoFilterBackend): - A related resource path can be used: - ``?filter[inventory.item.partNum]=123456`` (where `inventory.item` is the relationship path) + ``?filter[inventory.item.partNum]=123456`` + + where `inventory.item` is the relationship path. If you are also using rest_framework.filters.SearchFilter you'll want to customize the name of the query parameter for searching to make sure it doesn't conflict diff --git a/rest_framework_json_api/exceptions.py b/rest_framework_json_api/exceptions.py index b13ccad5..1c7acbdb 100644 --- a/rest_framework_json_api/exceptions.py +++ b/rest_framework_json_api/exceptions.py @@ -18,8 +18,9 @@ def rendered_with_json_api(view): def exception_handler(exc, context): # Import this here to avoid potential edge-case circular imports, which # crashes with: - # "ImportError: Could not import 'rest_framework_json_api.parsers.JSONParser' for API setting - # 'DEFAULT_PARSER_CLASSES'. ImportError: cannot import name 'exceptions'.'" + # "ImportError: Could not import 'rest_framework_json_api.parsers.JSONParser' for + # API setting 'DEFAULT_PARSER_CLASSES'. + # ImportError: cannot import name 'exceptions'.'" # # Also see: https://github.com/django-json-api/django-rest-framework-json-api/issues/158 from rest_framework.views import exception_handler as drf_exception_handler diff --git a/rest_framework_json_api/metadata.py b/rest_framework_json_api/metadata.py index 1906de71..14b9f69b 100644 --- a/rest_framework_json_api/metadata.py +++ b/rest_framework_json_api/metadata.py @@ -113,7 +113,7 @@ def get_field_info(self, field): field_info["type"] = self.type_lookup[field] try: - serializer_model = getattr(serializer.Meta, "model") + serializer_model = serializer.Meta.model field_info["relationship_type"] = self.relation_type_lookup[ getattr(serializer_model, field.field_name) ] diff --git a/rest_framework_json_api/parsers.py b/rest_framework_json_api/parsers.py index 2aef1aa1..e26f6028 100644 --- a/rest_framework_json_api/parsers.py +++ b/rest_framework_json_api/parsers.py @@ -85,8 +85,8 @@ def parse_data(self, result, parser_context): from rest_framework_json_api.views import RelationshipView if isinstance(view, RelationshipView): - # We skip parsing the object as JSON:API Resource Identifier Object and not a regular - # Resource Object + # We skip parsing the object as JSON:API Resource Identifier Object and not + # a regular Resource Object if isinstance(data, list): for resource_identifier_object in data: if not ( diff --git a/rest_framework_json_api/relations.py b/rest_framework_json_api/relations.py index 6cca15c8..6eb69bdb 100644 --- a/rest_framework_json_api/relations.py +++ b/rest_framework_json_api/relations.py @@ -123,7 +123,7 @@ def get_links(self, obj=None, lookup_field="pk"): # AuthorViewSet.as_view({'get': 'retrieve_related'})) # 2. url(r'^authors/(?P[^/.]+)/bio/$', # AuthorBioViewSet.as_view({'get': 'retrieve'})) - # So, if related_link_url_kwarg == 'pk' it will add 'related_field' parameter to reverse() + # So, if related_link_url_kwarg == 'pk' it adds 'related_field' parameter to reverse() if self.related_link_url_kwarg == "pk": related_kwargs = self_kwargs else: diff --git a/rest_framework_json_api/schemas/openapi.py b/rest_framework_json_api/schemas/openapi.py index 99fc2462..5ddc8143 100644 --- a/rest_framework_json_api/schemas/openapi.py +++ b/rest_framework_json_api/schemas/openapi.py @@ -115,11 +115,11 @@ class SchemaGenerator(drf_openapi.SchemaGenerator): "uniqueItems": True, }, # A RelationshipView uses a ResourceIdentifierObjectSerializer (hence the name - # ResourceIdentifierObject returned by get_component_name()) which serializes type and - # id. These can be lists or individual items depending on whether the relationship is - # toMany or toOne so offer both options since we are not iterating over all the - # possible {related_field}'s but rather rendering one path schema which may represent - # toMany and toOne relationships. + # ResourceIdentifierObject returned by get_component_name()) which serializes type + # and id. These can be lists or individual items depending on whether the + # relationship is toMany or toOne so offer both options since we are not iterating + # over all the possible {related_field}'s but rather rendering one path schema + # which may represent toMany and toOne relationships. "ResourceIdentifierObject": { "oneOf": [ {"$ref": "#/components/schemas/relationshipToOne"}, @@ -181,10 +181,12 @@ class SchemaGenerator(drf_openapi.SchemaGenerator): "properties": { "pointer": { "type": "string", - "description": "A [JSON Pointer](https://tools.ietf.org/html/rfc6901) " - "to the associated entity in the request document " - "[e.g. `/data` for a primary data object, or " - "`/data/attributes/title` for a specific attribute.", + "description": ( + "A [JSON Pointer](https://tools.ietf.org/html/rfc6901) " + "to the associated entity in the request document " + "[e.g. `/data` for a primary data object, or " + "`/data/attributes/title` for a specific attribute.", + ), }, "parameter": { "type": "string", @@ -273,7 +275,8 @@ def get_schema(self, request=None, public=False): _, view_endpoints = self._get_paths_and_endpoints(None if public else request) #: `expanded_endpoints` is like view_endpoints with one extra field tacked on: - #: - 'action' copy of current view.action (list/fetch) as this gets reset for each request. + #: - 'action' copy of current view.action (list/fetch) as this gets reset for + # each request. expanded_endpoints = [] for path, method, view in view_endpoints: if hasattr(view, "action") and view.action == "retrieve_related": @@ -371,14 +374,15 @@ def _expand_related(self, path, method, view, view_endpoints): def _find_related_view(self, view_endpoints, related_serializer, parent_view): """ - For a given related_serializer, try to find it's "parent" view instance in view_endpoints. + For a given related_serializer, try to find it's "parent" view instance. + :param view_endpoints: list of all view endpoints :param related_serializer: the related serializer for a given related field :param parent_view: the parent view (used to find toMany vs. toOne). TODO: not actually used. :return:view """ - for path, method, view in view_endpoints: + for _path, _method, view in view_endpoints: view_serializer = view.get_serializer() if isinstance(view_serializer, related_serializer): return view @@ -468,7 +472,7 @@ def _get_fields_parameters(self, path, method): # TODO: See if able to identify the specific types for fields[type]=... and return this: # name: fields # in: query - # description: '[sparse fieldsets](https://jsonapi.org/format/#fetching-sparse-fieldsets)' + # description: '[sparse fieldsets](https://jsonapi.org/format/#fetching-sparse-fieldsets)' # noqa: B950 # required: true # style: deepObject # schema: @@ -609,10 +613,11 @@ def get_request_body(self, path, method): # DRF uses a $ref to the component schema definition, but this # doesn't work for JSON:API due to the different required fields based on # the method, so make those changes and inline another copy of the schema. + # TODO: A future improvement could make this DRYer with multiple component schemas: - # A base schema for each viewset that has no required fields - # One subclassed from the base that requires some fields (`type` but not `id` for POST) - # Another subclassed from base with required type/id but no required attributes (PATCH) + # A base schema for each viewset that has no required fields + # One subclassed from the base that requires some fields (`type` but not `id` for POST) + # Another subclassed from base with required type/id but no required attributes (PATCH) if is_relationship: item_schema = {"$ref": "#/components/schemas/ResourceIdentifierObject"} @@ -653,7 +658,9 @@ def get_request_body(self, path, method): def map_serializer(self, serializer): """ Custom map_serializer that serializes the schema using the JSON:API spec. - Non-attributes like related and identity fields, are move to 'relationships' and 'links'. + + Non-attributes like related and identity fields, are moved to 'relationships' + and 'links'. """ # TODO: remove attributes, etc. for relationshipView?? required = [] @@ -830,7 +837,7 @@ def _add_delete_responses(self, operation): } self._add_async_response(operation) operation["responses"]["204"] = { - "description": "[no content](https://jsonapi.org/format/#crud-deleting-responses-204)", + "description": "[no content](https://jsonapi.org/format/#crud-deleting-responses-204)", # noqa: B950 } # the 4xx errors: self._add_generic_failure_responses(operation) diff --git a/rest_framework_json_api/serializers.py b/rest_framework_json_api/serializers.py index bfdfec71..91f86464 100644 --- a/rest_framework_json_api/serializers.py +++ b/rest_framework_json_api/serializers.py @@ -98,7 +98,7 @@ def __init__(self, *args, **kwargs): # iterate over a *copy* of self.fields' underlying OrderedDict, because we may # modify the original during the iteration. # self.fields is a `rest_framework.utils.serializer_helpers.BindingDict` - for field_name, field in self.fields.fields.copy().items(): + for field_name, _field in self.fields.fields.copy().items(): if ( field_name == api_settings.URL_FIELD_NAME ): # leave self link there @@ -208,17 +208,13 @@ def __new__(cls, name, bases, attrs): serializer = super().__new__(cls, name, bases, attrs) if attrs.get("included_serializers", None): - setattr( - serializer, - "included_serializers", - LazySerializersDict(serializer, attrs["included_serializers"]), + serializer.included_serializers = LazySerializersDict( + serializer, attrs["included_serializers"] ) if attrs.get("related_serializers", None): - setattr( - serializer, - "related_serializers", - LazySerializersDict(serializer, attrs["related_serializers"]), + serializer.related_serializers = LazySerializersDict( + serializer, attrs["related_serializers"] ) return serializer @@ -334,10 +330,9 @@ def __new__(cls, name, bases, attrs): return new_class polymorphic_serializers = getattr(new_class, "polymorphic_serializers", None) - if not polymorphic_serializers: - raise NotImplementedError( - "A PolymorphicModelSerializer must define a `polymorphic_serializers` attribute." - ) + assert ( + polymorphic_serializers is not None + ), "A PolymorphicModelSerializer must define a `polymorphic_serializers` attribute." serializer_to_model = { serializer: serializer.Meta.model for serializer in polymorphic_serializers } diff --git a/rest_framework_json_api/utils.py b/rest_framework_json_api/utils.py index 2e86e762..c9114128 100644 --- a/rest_framework_json_api/utils.py +++ b/rest_framework_json_api/utils.py @@ -48,7 +48,7 @@ def get_resource_name(context, expand_polymorphic_types=False): return "errors" try: - resource_name = getattr(view, "resource_name") + resource_name = view.resource_name except AttributeError: try: if "kwargs" in context and "related_field" in context["kwargs"]: @@ -80,10 +80,10 @@ def get_resource_name(context, expand_polymorphic_types=False): def get_serializer_fields(serializer): fields = None if hasattr(serializer, "child"): - fields = getattr(serializer.child, "fields") + fields = serializer.child.fields meta = getattr(serializer.child, "Meta", None) if hasattr(serializer, "fields"): - fields = getattr(serializer, "fields") + fields = serializer.fields meta = getattr(serializer, "Meta", None) if fields is not None: @@ -159,8 +159,8 @@ def format_link_segment(value): def undo_format_link_segment(value): """ - Takes a link segment and undos format link segment to underscore which is the Python convention - but only in case `JSON_API_FORMAT_RELATED_LINKS` is actually configured. + Takes a link segment and undos format link segment to underscore which is the Python + convention but only in case `JSON_API_FORMAT_RELATED_LINKS` is actually configured. """ if json_api_settings.FORMAT_RELATED_LINKS: @@ -331,7 +331,7 @@ def get_relation_instance(resource_instance, source, serializer): # if the field is not defined on the model then we check the serializer # and if no value is there we skip over the field completely serializer_method = getattr(serializer, source, None) - if serializer_method and hasattr(serializer_method, "__call__"): + if serializer_method and callable(serializer_method): relation_instance = serializer_method(resource_instance) else: return False, None @@ -356,8 +356,8 @@ class Hyperlink(str): https://github.com/tomchristie/django-rest-framework """ - def __new__(self, url, name): - ret = str.__new__(self, url) + def __new__(cls, url, name): + ret = str.__new__(cls, url) ret.name = name return ret diff --git a/rest_framework_json_api/views.py b/rest_framework_json_api/views.py index c53864a0..de7e8aa6 100644 --- a/rest_framework_json_api/views.py +++ b/rest_framework_json_api/views.py @@ -129,7 +129,10 @@ def get_queryset(self, *args, **kwargs): class RelatedMixin: """ - This mixin handles all related entities, whose Serializers are declared in "related_serializers" + Mixing handling related links. + + This mixin handles all related entities, whose Serializers are declared + in "related_serializers". """ def retrieve_related(self, request, *args, **kwargs): @@ -162,6 +165,10 @@ def get_related_serializer_class(self): if "related_field" in self.kwargs: field_name = self.get_related_field_name() + assert hasattr(parent_serializer_class, "included_serializers") or hasattr( + parent_serializer_class, "related_serializers" + ), 'Either "included_serializers" or "related_serializers" should be configured' + # Try get the class from related_serializers if hasattr(parent_serializer_class, "related_serializers"): _class = parent_serializer_class.related_serializers.get( @@ -177,11 +184,6 @@ def get_related_serializer_class(self): if _class is None: raise NotFound - else: - assert ( - False - ), 'Either "included_serializers" or "related_serializers" should be configured' - return _class return parent_serializer_class diff --git a/setup.cfg b/setup.cfg index 2eb90b9b..c9d88f15 100644 --- a/setup.cfg +++ b/setup.cfg @@ -9,8 +9,13 @@ max-line-length = 88 extend-ignore = # whitespace before ':' - disabled as not PEP8 compliant E203, - # line too long (managed by black) + # line too long, as using bugbear E501 +extend-select = + # Line too long. This is a pragmatic equivalent of pycodestyle's E501 + B950, + # Invalid first argument used for method. + B902 exclude = build/lib, .eggs