-
Notifications
You must be signed in to change notification settings - Fork 270
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Model fields that contain a default
value are not marked as required in response API
#1299
Comments
User
Serializer makes many fields optionalUser
Serializer makes many response fields optional
Hi @johnthagen, One OpenAPI component is not able to fully cover what a DRF serializer can do. What you see is a carefully crafted compromise that would work both for the request and response. If you want to have a better representation, you need to enable https://drf-spectacular.readthedocs.io/en/latest/client_generation.html#component-issues This is especially important for client generation due to the stated reasons. You will then get 2 components that together very closely resemble what the serializer actually does. Regarding the Can't really say anything about the specific generator target, but with the |
@tfranzel Thanks for this response.
Sorry, I should have also mentioned my SPECTACULAR_SETTINGS = {
"COMPONENT_SPLIT_REQUEST": True,
# drf-spectacular-sidecar settings
"SWAGGER_UI_DIST": "SIDECAR",
"SWAGGER_UI_FAVICON_HREF": "SIDECAR",
"REDOC_DIST": "SIDECAR",
...
} In my first post, I tried to simplify my example by not including the generated I think this will help summarize the crux of my question. If I swap the built in from django.db import models
class MyUser(models.Model):
username = models.TextField()
is_staff = models.BooleanField()
is_superuser = models.BooleanField()
class UserSerializer(ModelSerializer):
class Meta:
model = MyUser
fields = "__all__" Then the generated OpenAPI spec is correctly generated as I expect. The response type in the API guarantees that This is the expected behaviour for my typical Serializers and ViewSets, but I'm trying to figure out why the built in |
User
Serializer makes many response fields optionaldefault
value are not marked as required in retrieve and list response APIs
default
value are not marked as required in retrieve and list response APIsdefault
value are not marked as required in response API
I think I get your point, but I believe we have a conceptual problem here. Foundational statements
We cannot really look at the model here, because DRF degenerates the Serializer from the Model for us. There is information loss there for sure. On top of that, the user can modify/patch onto that generated serializer. Trying to untangle what is intentional by the user or just auto-generated is at leased problematic. This is the serializer we get from DRF for the django standard user and "in theory" that would be all we are allowed to look at: class UserSerializer:
id = IntegerField(label='ID', read_only=True)
password = CharField(max_length=128)
last_login = DateTimeField(allow_null=True, required=False)
is_superuser = BooleanField(help_text='Designates that this user has all permissions without explicitly assigning them.', label='Superuser status', required=False)
username = CharField(help_text='Required. 150 characters or fewer. Letters, digits and @/./ /-/_ only.', max_length=150, validators=[...])
first_name = CharField(allow_blank=True, max_length=150, required=False)
last_name = CharField(allow_blank=True, max_length=150, required=False)
email = EmailField(allow_blank=True, label='Email address', max_length=254, required=False)
is_staff = BooleanField(help_text='Designates whether the user can log into this admin site.', label='Staff status', required=False)
is_active = BooleanField(help_text='Designates whether this user should be treated as active. Unselect this instead of deleting accounts.', label='Active', required=False)
date_joined = DateTimeField(required=False)
groups = PrimaryKeyRelatedField(help_text='The groups this user belongs to. A user will get all permissions granted to each of their groups.', many=True, queryset=Group.objects.all(), required=False)
user_permissions = PrimaryKeyRelatedField(help_text='Specific permissions for this user.', many=True, queryset=Permission.objects.all(), required=False) Regarding the field_without_default = models.BooleanField()
field_with_default = models.BooleanField(default=False)
# generates this
field_without_default = serializers.BooleanField()
field_with_default = serializers.BooleanField(required=False) # the default makes this not required So we do nothing special here and merely carry over the My conclusionIMHO, DRF's The fact that we consider the I came to that realization at a point where this would have been to big of a breaking change. However, I think we could add another setting to address this. Is it a valid assumption that all fields are always present on serializer responses? It is certainly true most of the time, but it is not guaranteed, I think. What do you think? |
Oh, right. I forgot that there is a
For a response I do think this is valid, or at least this should be the default and users can override this with I think we still need to allow the But the default, I think, should be that the API guarantees the fields the user defines will be returned (e.g., set as I'd be interested to hear a counter example where someone wouldn't want these marked as required for a response. The result is that clients couldn't be confident that any fields exist and would need to individually check that each exist before accessing them. To me that sounds like a very brittle way to use an API, especially as the default way to generate it. In TypeScript (probably the most widely used API client language), this results is lots of As always, I appreciate the thoughtful feedback and consideration. |
Describe the bug
Using the default Django
User
model, creating aSerializer
andViewSet
from this leads to many of the fields being optional in the response model that should always be "required"/assured to be there.Consider:
And
This creates a schema where many of the fields (such as
is_staff
) are optional even for theUser
response type:But I'm not sure why they would be marked optional in the response, as my API will always return a
bool
for them? I see in the Model definitions they have default values, perhaps that is related?I'm still not sure if this is a bug, or some kind of expected behaviour for this model.
The problem this causes is that in generated clients such as Typescript the code is:
And the
?
denotes it is optional and all clients then have to do anull
check even through the API will always return a value for this field.In a generated Python client, this looks like the following, with an extra
Unset
state:Environment
The text was updated successfully, but these errors were encountered: