validate_response() fails when 'pk' path parameter coerced by DRF

By default, DRF seems to change pk path parameters to the actual primary key field name for the schema. This leads to a strange error when the SchemaTester:

>>> response = client.get("/api/names/123/")
>>> schema_tester.validate_response(response)
openapi_tester.exceptions.UndocumentedSchemaSectionError: Error: Unsuccessfully tried to index the OpenAPI schema by `/api/names/{pk}/`. 

For debugging purposes, other valid routes include: 

    • /api/names/
    • /api/names/{id}/
    • ...

When I set REST_FRAMEWORK["SCHEMA_COERCE_PATH_PK"] = False in my settings, it works, but then the schema uses the path parameter {pk} instead of {id}, which is not very nice.

My Name model's primary key field name is id, and my ViewSet generating the routes looks like this:

class NameViewSet(viewsets.ReadOnlyModelViewSet):
    queryset = models.Name.objects.all()
    serializer_class = serializers.NameSerializer

router = SimpleRouter()
router.register(r"names", NameViewSet)

(Also possible that I've configured something incorrectly, since a lot has changed in the package recently...)

Thanks for reporting this issue.

I see we need to take some DRF settings into account.

You are generating schema with the drf management command?

I'm using drf-spectacular, but the same setting seems to apply.

If there's any way I can help, let me know.

Thanks for letting us know @saeub! @Goldziher and I discussed this earlier and I think we have all the information we need to implement a fix, most likely sometime early next week.

On an unrelated note, do you still have the same requirement for i18n-normalization that you raised in #115?

With @Goldziher as the driving force, we've more or less rewritten the library over the last few weeks, and the i18n handling is the one feature that we haven't yet implemented. If you no longer have any need for this feature we'll probably drop it and focus our attention towards writing tests and ironing out little wrinkles like this issue😊 The reason I ask is, I seem to recall you were documenting your schema manually, and that's how you originally found a need for the feature

Thanks again for the report 👌

Thanks a lot for your hard work @Goldziher and @sondrelg! :)

The project I mentioned in #115 still uses the i18n feature, so we haven't upgraded the package there yet. We're not able to use schema generators in any reasonable way there, so I'm pretty sure our manually written schema will stay on that one. It would be nice if the i18n feature returned, but it's not exactly urgent.

The project I'm working on at the moment uses drf-spectacular and will probably not need i18n in the API.

Question @saeub - if we simply generate the schema and automatically turn off the DRF schema settings, just generating vanilla specs. Will that work?

I mean if we temporarily override the drf settings within the test before we generate the schema:

from rest_framework.settings import api_settings

ORIGINAL_SCHEMA_COERCE_PATH_PK = api_settings.SCHEMA_COERCE_PATH_PK

api_settings.SCHEMA_COERCE_PATH_PK = False

<run code that generates schema>

api_settings.SCHEMA_COERCE_PATH_PK = ORIGINAL_SCHEMA_COERCE_PATH_PK

if we simply generate the schema and automatically turn off the DRF schema settings, just generating vanilla specs. Will that work?

Sounds like it could work. I was just trying to figure out at which point the setting is actually applied in rest_framework, and it appears to be pretty early (pytest gets stuck already in the collection phase when I add a breakpoint here). I wasn't able go get it to work by temporarily changing the setting in my test function.

@saeub - can you try using this: https://docs.djangoproject.com/en/3.1/topics/testing/tools/#django.test.override_settings?

I tried using a django-pytest fixture like this (https://pytest-django.readthedocs.io/en/latest/configuring_django.html#overriding-individual-settings):

@pytest.fixture(autouse=True)
def schema_settings(settings):
    settings.REST_FRAMEWORK["SCHEMA_COERCE_PATH_PK"] = False

When I apply this, I can see that the django.conf.settings are modified inside the test function, but the error still remains. :(

@saeub all good. I merged a fix for this issue. Can you test using our master branch? If not we will release soon and you would be able to test 1.3.

@saeub - v1.3.1 was release, please let us know if this resolves your issue (we also added support for the i18n parameter).

That's awesome, thanks a lot!

I updated to 1.3.1, and now I get a ValueError at this line:

drf-openapi-tester/openapi_tester/loaders.py

Line 151 in 81a8479

imported_module = import_module(".".join(split_view_path))

venv/lib/python3.8/site-packages/openapi_tester/schema_tester.py:368: in validate_response
    response_schema = self.get_response_schema_section(response)
venv/lib/python3.8/site-packages/openapi_tester/schema_tester.py:118: in get_response_schema_section
    parameterized_path, _ = self.loader.resolve_path(response.request["PATH_INFO"], method=response_method)
venv/lib/python3.8/site-packages/openapi_tester/loaders.py:210: in resolve_path
    de_parameterized_path, resolved_path = super().resolve_path(endpoint_path=endpoint_path, method=method)
venv/lib/python3.8/site-packages/openapi_tester/loaders.py:136: in resolve_path
    path, resolved_route = self.handle_pk_parameter(
venv/lib/python3.8/site-packages/openapi_tester/loaders.py:153: in handle_pk_parameter
    imported_module = import_module(".".join(split_view_path))
/usr/lib/python3.8/importlib/__init__.py:127: in import_module
    return _bootstrap._gcd_import(name[level:], package, level)
<frozen importlib._bootstrap>:1011: in _gcd_import
    ???
_ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _

name = '', package = None, level = 0

>   ???
E   ValueError: Empty module name

<frozen importlib._bootstrap>:950: ValueError

The resolved_route looks like this: ResolverMatch(func=pench.api.views.NameViewSet, args=(), kwargs={'pk': '{id}'}, url_name=name-detail, app_names=[], namespaces=[], route=api/names/(?P<pk>[^/.]+)/$) (view_name is also 'name-detail')

If it's of any help, I could try reproducing the error in the test_project and writing a test for it? Also haven't tried the i18n parameter yet, but I will as soon as possible.

I'll let @Goldziher confirm, but I think it's safe to assume that submitting a PR reproducing the error would be very helpful 👏

Yes, a reproduction would be awesome.

@saeub - 1.3.2 was just released with a hotfix. I hope this fixes the issue (it fixed the test) :). Thanks for your assistance.

Like a charm! 🥳 Thank you so much!