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.38k stars 264 forks source link

Ability to disable enums postprocessing for components under specific status codes #753

Closed ghazi-git closed 2 years ago

ghazi-git commented 2 years ago

I’m working on a drf package that provides an exception handler to standardize error responses (same format for all 4xx and 5xx responses). The default format looks like this:

{
  "type": "validation_error",
  "errors": [
    {
      "code": "required",
      "detail": "This field is required.",
      "attr": "name"
    }
  ]
}

Then, I used drf-spectacular to generate the schema for 4xx/5xx status codes. This is still a work in progress here (schema generation for 4xx and 5xx) and here (accurate schema generation for validation errors i.e. determine for each field being validated all possible error codes)

In the latter PR, I was able to implement the necessary changes that would result in a schema like this. However, after the enums postprocessing hook runs I’m left with quite a high number of warnings like this

Warning #46: enum naming encountered a non-optimally resolvable collision for fields named "attr". The same name has been used for multiple choice sets in multiple components. The collision was resolved with "AttrBb3Enum". add an entry to ENUM_NAME_OVERRIDES to fix the naming.
Warning #71: enum naming encountered a non-optimally resolvable collision for fields named "code". The same name has been used for multiple choice sets in multiple components. The collision was resolved with "CodeDaaEnum". add an entry to ENUM_NAME_OVERRIDES to fix the naming.

That is understandable because, in the response serializer, attr and code are set as ChoiceFields which can have the same choices across different serializers. The usual solution is to add the enums with the same choices to ENUM_NAME_OVERRIDES. However, error response serializers are generated dynamically depending on the request serializer fields. So, the suggested fix does not apply and that’s why I’m asking if it is possible to disable enums postprocessing for components under specific status codes (in my case 400).

It would be great to hear your opinion about implementing this in drf-spectacular or if it’s a very specific use case that should remain outside it or if you have a suggestion that avoids the problem in the first place. Thanks in advance.

tfranzel commented 2 years ago

Hi @ghazi-git, that is quite interesting. Just a view details to get you oriented:

In essence, DRF is simply under-defined in this regard and adding extra info is harder than it looks. I'm weary of introducing special cases unless it is absolutely necessary as the system is already quite hard to understand from the outside. Special handling for specific fields will likely confuse more people than actually benefit from it. Not strictly against changing anything, but it has to be balanced.

The one thing I could imagine is creating a postprocessing hook that explicitly deals with your handful of enums manually. It must precede the default hook in the list, which means that once our code is reached, those changes were already made and there are no new collisions. Not the most elegant solution, but it would have the least impact at the moment.

ghazi-git commented 2 years ago

@tfranzel thank you for the details. I'll probably go with the postprocessing hook approach. I've actually considered sth similar before along with few other options (last paragraph in the PR comment). It's good to hear this from you as well.

Hopefully, once I wrap up the work (still need to test and properly document the work), I'll take a stab at #101 since I'm currently working on the same issue but a different response format.

One more thing, if there is sth I learned from generating error responses and reading drf-spectacular code (specifically plumbing.py/drainage.py and openapi.py) is that nothing is "simple". Maybe the idea is, but the implementation is lines & lines of code for the different scenarios and edge cases. So, huge thanks for all the efforts you've put into making schema generation easy 👏