tfranzel / drf-spectacular

Sane and flexible OpenAPI 3 schema generation for Django REST framework.
https://drf-spectacular.readthedocs.io
BSD 3-Clause "New" or "Revised" License
2.39k stars 264 forks source link

get_queryset with an abstract model #1171

Open alexandernst opened 9 months ago

alexandernst commented 9 months ago

Describe the bug I have a use case in which I have a ListAPIView that connects to a 3rd party (Stripe) API, fetches data (invoices) and returns that data to the user. I have a serializer, but I don't have a model.

The entire code looks something like this:

class InvoicesList(generics.ListAPIView):
  serializer_class = InvoiceSerializer

  def get_queryset(self):
    if getattr(self, 'swagger_fake_view', False):
      return   # <----  ¿?¿?¿?¿?¿?¿?¿?¿?

    return StripeWrapper().get_invoices()

class InvoiceSerializer(serializers.Serializer):
  ...fields..
  ...fields...
  ...fields

class StripeWrapper():
  def get_invoices():
   return requests.get(......)

Since I don't have a model, drf-spectacular refuses to generate the proper openapi specs. It expects to receive an EmptyQuerySet (SomeModel.objects.none()), but I can't provide it any since I don't have an Invoice model. I could create an abstract model like this:

class Invoice(models.Model):
  class Meta:
    abstract = True

but I still won't be able to provide drf-spectacular with a Invoice.objects.none() since there is no manager in that class (and there can't be since it's abstract).

How should I proceed in this scenario? Is there some workaround?

To Reproduce N/A

Expected behavior N/A

tfranzel commented 9 months ago

Hi, so 2 observations. I put your example in a test case and it works for me without error or warnings.

    if getattr(self, 'swagger_fake_view', False):
      return 

Should be enough. returning None will be handled gracefully internally. The given serializer will be used anyway since this call is merely to find potential path parameters (which there are none). Outside of spectacular, this if will never be entered, so no issue there for DRF.

We could, in case there are no path parameters, skip the get_queryset call. That would be a small fix, but I still don't understand where your code exactly breaks. What exactly is the problem? stacktrace?

alexandernst commented 9 months ago

If I return None, I get this:

/code/business/apiviews/invoices.py: Warning [InvoicesL]: Failed to obtain model through view's queryset due to raised exception. Prevent this either by setting "queryset = Model.objects.none()" on the view, checking for "getattr(self, "swagger_fake_view", False)" in get_queryset() or by simply using @extend_schema. (Exception: 'NoneType' object has no attribute 'model')
tfranzel commented 9 months ago

Something is weird here. That warning cannot be explained with your example.

get_view_model is only used twice. The first is with parameter extraction, but the warning is disabled for that usage. This exception is expected and does happen, however it is suppressed with that if statement.

The second usage is with django_filter. Are you using django-filter here too?

https://github.com/tfranzel/drf-spectacular/blob/473d2d803b9e8ae986ea60292ec0ebfb0ff2d601/drf_spectacular/openapi.py#L483

https://github.com/tfranzel/drf-spectacular/blob/473d2d803b9e8ae986ea60292ec0ebfb0ff2d601/drf_spectacular/plumbing.py#L215-L217


this testcase works as expected for me:

def test_foo(no_warnings):
    class InvoiceSerializer(serializers.Serializer):
        field = serializers.IntegerField()

    class InvoicesList(generics.ListAPIView):
        serializer_class = InvoiceSerializer

        def get_queryset(self):
            if getattr(self, 'swagger_fake_view', False):
                return
            raise RuntimeError("scheme gen should not get here")

    schema = generate_schema('/x/', view=InvoicesList)
    assert schema
alexandernst commented 9 months ago

Does this help?

> /code/business/apiviews/invoices.py(28)get_queryset()
-> return None
(Pdb) bt
  /usr/local/lib/python3.10/threading.py(973)_bootstrap()
-> self._bootstrap_inner()
  /usr/local/lib/python3.10/threading.py(1016)_bootstrap_inner()
-> self.run()
  /usr/local/lib/python3.10/threading.py(953)run()
-> self._target(*self._args, **self._kwargs)
  /usr/local/lib/python3.10/socketserver.py(683)process_request_thread()
-> self.finish_request(request, client_address)
  /usr/local/lib/python3.10/socketserver.py(360)finish_request()
-> self.RequestHandlerClass(request, client_address, self)
  /usr/local/lib/python3.10/socketserver.py(747)__init__()
-> self.handle()
  /usr/local/lib/python3.10/site-packages/django/core/servers/basehttp.py(227)handle()
-> self.handle_one_request()
  /usr/local/lib/python3.10/site-packages/django/core/servers/basehttp.py(252)handle_one_request()
-> handler.run(self.server.get_app())
  /usr/local/lib/python3.10/wsgiref/handlers.py(137)run()
-> self.result = application(self.environ, self.start_response)
  /usr/local/lib/python3.10/site-packages/django/contrib/staticfiles/handlers.py(80)__call__()
-> return self.application(environ, start_response)
  /usr/local/lib/python3.10/site-packages/django/core/handlers/wsgi.py(124)__call__()
-> response = self.get_response(request)
  /usr/local/lib/python3.10/site-packages/django/core/handlers/base.py(140)get_response()
-> response = self._middleware_chain(request)
  /usr/local/lib/python3.10/site-packages/django/core/handlers/exception.py(55)inner()
-> response = get_response(request)
  /usr/local/lib/python3.10/site-packages/django/utils/deprecation.py(134)__call__()
-> response = response or self.get_response(request)
  /usr/local/lib/python3.10/site-packages/django/core/handlers/exception.py(55)inner()
-> response = get_response(request)
  /usr/local/lib/python3.10/site-packages/silk/middleware.py(72)__call__()
-> response = self.get_response(request)
  /usr/local/lib/python3.10/site-packages/django/core/handlers/exception.py(55)inner()
-> response = get_response(request)
  /usr/local/lib/python3.10/site-packages/django/utils/deprecation.py(134)__call__()
-> response = response or self.get_response(request)
  /usr/local/lib/python3.10/site-packages/django/core/handlers/exception.py(55)inner()
-> response = get_response(request)
  /usr/local/lib/python3.10/site-packages/corsheaders/middleware.py(56)__call__()
-> result = self.get_response(request)
  /usr/local/lib/python3.10/site-packages/django/core/handlers/exception.py(55)inner()
-> response = get_response(request)
  /usr/local/lib/python3.10/site-packages/django/utils/deprecation.py(134)__call__()
-> response = response or self.get_response(request)
  /usr/local/lib/python3.10/site-packages/django/core/handlers/exception.py(55)inner()
-> response = get_response(request)
  /code/ambiance/middleware.py(14)__call__()
-> response = self.get_response(request)
  /usr/local/lib/python3.10/site-packages/django/core/handlers/exception.py(55)inner()
-> response = get_response(request)
  /usr/local/lib/python3.10/site-packages/django/utils/deprecation.py(134)__call__()
-> response = response or self.get_response(request)
  /usr/local/lib/python3.10/site-packages/django/core/handlers/exception.py(55)inner()
-> response = get_response(request)
  /usr/local/lib/python3.10/site-packages/django/utils/deprecation.py(134)__call__()
-> response = response or self.get_response(request)
  /usr/local/lib/python3.10/site-packages/django/core/handlers/exception.py(55)inner()
-> response = get_response(request)
  /usr/local/lib/python3.10/site-packages/django/utils/deprecation.py(134)__call__()
-> response = response or self.get_response(request)
  /usr/local/lib/python3.10/site-packages/django/core/handlers/exception.py(55)inner()
-> response = get_response(request)
  /usr/local/lib/python3.10/site-packages/django/core/handlers/base.py(197)_get_response()
-> response = wrapped_callback(request, *callback_args, **callback_kwargs)
  /usr/local/lib/python3.10/site-packages/django/views/decorators/csrf.py(56)wrapper_view()
-> return view_func(*args, **kwargs)
  /usr/local/lib/python3.10/site-packages/django/views/generic/base.py(104)view()
-> return self.dispatch(request, *args, **kwargs)
  /usr/local/lib/python3.10/site-packages/rest_framework/views.py(506)dispatch()
-> response = handler(request, *args, **kwargs)
  /usr/local/lib/python3.10/site-packages/drf_spectacular/views.py(84)get()
-> return self._get_schema_response(request)
  /usr/local/lib/python3.10/site-packages/drf_spectacular/views.py(92)_get_schema_response()
-> data=generator.get_schema(request=request, public=self.serve_public),
  /usr/local/lib/python3.10/site-packages/drf_spectacular/generators.py(281)get_schema()
-> paths=self.parse(request, public),
  /usr/local/lib/python3.10/site-packages/drf_spectacular/generators.py(252)parse()
-> operation = view.schema.get_operation(
  /usr/local/lib/python3.10/site-packages/drf_spectacular/openapi.py(91)get_operation()
-> parameters = self._get_parameters()
  /usr/local/lib/python3.10/site-packages/drf_spectacular/openapi.py(258)_get_parameters()
-> **dict_helper(self._resolve_path_parameters(path_variables)),
  /usr/local/lib/python3.10/site-packages/drf_spectacular/openapi.py(483)_resolve_path_parameters()
-> model = get_view_model(self.view, emit_warnings=False)
  /usr/local/lib/python3.10/site-packages/drf_spectacular/plumbing.py(213)get_view_model()
-> return view.get_queryset().model
> /code/business/apiviews/invoices.py(28)get_queryset()
(Pdb) s
--Return--
> /code/business/apiviews/invoices.py(28)get_queryset()->None
-> return None
(Pdb) s
AttributeError: 'NoneType' object has no attribute 'model'
> /usr/local/lib/python3.10/site-packages/drf_spectacular/plumbing.py(213)get_view_model()
-> return view.get_queryset().model
(Pdb) ll
202     def get_view_model(view, emit_warnings=True):
203         """
204         obtain model from view via view's queryset. try safer view attribute first
205         before going through get_queryset(), which may perform arbitrary operations.
206         """
207         model = getattr(getattr(view, 'queryset', None), 'model', None)
208  
209         if model is not None:
210             return model
211  
212         try:
213  ->         return view.get_queryset().model
214         except Exception as exc:
215             if emit_warnings:
216                 warn(
217                     f'Failed to obtain model through view\'s queryset due to raised exception. '
218                     f'Prevent this either by setting "queryset = Model.objects.none()" on the '
219                     f'view, checking for "getattr(self, "swagger_fake_view", False)" in '
220                     f'get_queryset() or by simply using @extend_schema. (Exception: {exc})'
221                 )
(Pdb) process_request
alexandernst commented 9 months ago

BTW, yes, I'm using django-filters

tfranzel commented 9 months ago

your stacktrace cannot lead to that warning. emit_warnings=False does not allow it.

  /usr/local/lib/python3.10/site-packages/drf_spectacular/openapi.py(483)_resolve_path_parameters()
-> model = get_view_model(self.view, emit_warnings=False)

213  ->         return view.get_queryset().model
214         except Exception as exc:
215             if emit_warnings:
216                 warn(

The warning must come from your usage of django-filter.

Since I don't have a model, drf-spectacular refuses to generate the proper openapi specs.

what exactly do you mean with that? just a wrong schema or does it completely break with an exception?

alexandernst commented 9 months ago

The warning must come from your usage of django-filter.

Yet the backtrace that I posted says otherwise, right?

You're checking

207         model = getattr(getattr(view, 'queryset', None), 'model', None)
208  
209         if model is not None:
210             return model

but I don't have queryset set to anything, which I believe will skip that if. Then you try return view.get_queryset().model, which is what is causing the Exception.

just a wrong schema or does it completely break with an exception?

It generates the rest of the endpoints, it just skips this particular endpoint.

tfranzel commented 9 months ago

but I don't have queryset set to anything, which I believe will skip that if. Then you try return view.get_queryset().model, which is what is causing the Exception.

yes. 207 will be None. 209 will be then skipped 213 will raise the attribute error 214 will catch that exception 215 will skip the warning because -> model = get_view_model(self.view, emit_warnings=False) is how this is called in the stacktrace and if emit_warnings: thus cannot be entered like that.

this will all happen under /usr/local/lib/python3.10/site-packages/drf_spectacular/openapi.py(483)_resolve_path_parameters(), which has nothing to do with the serializer extraction.

Yet the backtrace that I posted says otherwise, right?

I'm just saying I cannot explain what you are observing with the code here. How can that warn() call happen with emit_warnings=False? It makes no sense to me.

tfranzel commented 9 months ago

I should also add that test_foo test case above does not produce the warning and properly generates the Invoice component, which is correctly put in the operation.

So that test case exactly behaves as it is supposed to: no warning and the correct operation and component.

Even if I explain the warning away with django-filter (and thus a missing endpoint query parameter), it still does not explain the missing Invoice component as query param extraction also behaves independently.

alexandernst commented 9 months ago

In contrib/django_filters.py:54, in get_schema_operation_parameters, you're calling get_view_model(auto_schema.view) without passing the second parameter (which is True by default), which is why the warning is getting printed.

tfranzel commented 9 months ago

That is what I said before! get_view_model() is used twice and only once with the warning enabled (django-filter).

Your given stacktrace did not produce that warning because it was not in the django-filter code section, but rather in the path param extraction section, where the warning is turned off.

However, all of this does not explain why the Invoice serializer was not detected. That warning you got there from the django-filter plugin has nothing to do with the response serializer not showing up.

So, I cannot help you any further like that as I cannot reproduce your issue with the given information. The test case I have shown above behaves perfectly, so something else must be going on in your setup. You need to provide me a with a reproduction of your issue if you want futher assistance.

alexandernst commented 9 months ago

Fair enough! I'll try to isolate the problem and provide a minimum reproducible demo. In the meantime, do you think passing False as second parameter to that get_view_model call should be added?

tfranzel commented 9 months ago

Fair enough! I'll try to isolate the problem and provide a minimum reproducible demo.

yes, please do.

In the meantime, do you think passing False as second parameter to that get_view_model call should be added?

No I don't think so. It is deliberately made like that. The path parameter extraction has fallbacks that may work and so the warning may be unnessecary or misleading. For django-filter this method is all there is and so if there is no model available, the parameter cannot be properly extracted, thus the warning.