Issue with upgrade from 3.14 to 3.15.1, null values no longer allowed in POST

[Zolton]

Checklist

3.15.1 is causing problems in prod with a change to how null values are handled.
Currently running Django. 5.0.6, django-cors-headers 4.3.1, and gunicorn 22.0.0

As an example, I have a table that keeps track of a person, an organization, and some small tidbit of info about them, with the constraint that at least 2 fields must always be filled out and have unique information:

ContactOrganizationInformation
  contact=models.ForeignKey(Contact, on_delete=models.CASCADE, blank=True, null=True)
  organization=models.ForeignKey(Organization, on_delete=models.CASCADE, blank=True, null=True)
  information=models.ForeignKey(Information, on_delete=models.CASCADE, blank=True, null=True)

class Meta:
  constraints = [
    models.UniqueConstraint(fields = ['contact', 'organization', 'information', name='UniqueThree'),
    models.UniqueConstraint(fields = ['contact', 'organization', name='UniqueCO'),
    models.UniqueConstraint(fields = [ 'organization', 'information', name='UniqueOI'),
    models.UniqueConstraint(fields = ['contact', 'information', name='UniqueCI')
  ]

models.CheckConstraint(
name="NotMoreThanTwoNulls',
violation_error_message= "At least 2 fields must be filled out",
check = 
  models.Q(contact__isnull=False, organization__isnull=False, information__isnull=True |
 models.Q(contact__isnull=False, organization__isnull=True, information__isnull=False |
 models.Q(contact__isnull=True, organization__isnull=False, information__isnull=False |
 models.Q(contact__isnull=False, organization__isnull=False, information__isnull=False
)
]

The idea is, of the 3 fields, at least 2 must be filled out. So in 3.14, I could send a POST request like

{ 
contact: "A Swell Guy",
organization: "A Great Org"
}

or

{
contact: "A Swell Guy",
information: "He's actually not a swell guy, only an OK one"
}

And just leave out one of the fields out, it'd just be null in the database - which is exactly what we want; if there's no info, just leave it null

Now in 3.15.1, this results in a 400 Bad Request, and the error message, "This field is required".

This is causing problems because now all our front end requests have to be rewritten as

{ 
contact: "A Swell Guy",
organization: "A Great Org",
information: ""
}

In the above case, Information would still reflected in the database as Null in the database.

Our issue is having to include a blank string in POST requests, when previously, just omitting the key-value pair was enough to cause a null value. We'd prefer not having to change every single POST format, and keep the old habit of "If a key-value is not present, it's just null", instead of having to write in an empty string

[peterthomassen]

Can you pinpoint which commit causes this? (e.g. via git bisect)

[Zolton]

Tracked it down after a few hours of installing and uninstalling various commits.
My tests run fine as of commit 9882207 and start erroring out on commit b7523f4.

To confirm, I started up my backend database locally and tested; sure enough, commit b7523 resulted in

This field is required when I omitted 1 of the 3 possible values in a POST request, while it worked as-expected when running commit 98822

It seems this pull request did something:
#7438

[stumpylog]

We're also experiencing this with paperless-ngx. The API now returns {'owner': ['This field is required.']}. I would suggest this is essentially the same as #9378 too.

Roughly with these fields and constraints:

    owner = models.ForeignKey(
        User,
        blank=True,
        null=True,
        on_delete=models.SET_NULL,
        verbose_name=_("owner"),
    )
    name = models.CharField(_("name"), max_length=128)

    class Meta:
        constraints = [
            models.UniqueConstraint(
                fields=["name", "owner"],
                name="%(app_label)s_%(class)s_unique_name_owner",
            ),
            models.UniqueConstraint(
                name="%(app_label)s_%(class)s_name_uniq",
                fields=["name"],
                condition=models.Q(owner__isnull=True),
            ),
        ]

which I think are pretty un-complicated constraints.

[BPC-AnonymousBeast]

@peterthomassen
Will this be considered an issue or does everyone have to change their models and add a default value to all the fields?
I would like to start work on this. Hence, confirming before beginning.

[Zolton]

I'm hoping this is considered an issue that needs to be fixed, not just because changing the front end is a PITA, but mainly because I don't think an API should require users to know and define literally every possible key-value pair when hitting an endpoint.
It should be perfectly fine to omit key-value pairs they simply don't care about if our backend allows for it - if it was truly important, we'd slap a Require on it and not even allow front ends to omit it

[BPC-AnonymousBeast]

I'm hoping this is considered an issue that needs to be fixed, not just because changing the front end is a PITA, but mainly because I don't think an API should require users to know and define literally every possible key-value pair when hitting an endpoint.

@Zolton
I don't think you have to make changes to your frontend at all.
Referring to the code you have provided, you just need to add a keyword argument called default in your model class. This makes sure that a null value is stored if you don't provide data for a particular parameter. i.e if you omit a key-value pair.

ex : -

ContactOrganizationInformation
  contact=models.ForeignKey(Contact, on_delete=models.CASCADE,blank=True, null=True,default=None )
  organization=models.ForeignKey(Organization, on_delete=models.CASCADE, blank=True, null=True,default=None)

Also please go through my comment in #9378. I have tried to reproduce the error. I used foreign key like it was done by you but I'm able to send a POST request without a parameter which is also a foreign key. I didn't receive any error.
I have also provided my model and serializer code there, if you feel like I should make changes and try again then let me know.

[shamoon]

While its true that the solution above default=None does partially mitigate things, it is 1) a breaking change and 2) still leaves the following problem, where something like this now fails where it used to work:

def create(self, validated_data):
    if "owner" not in validated_data and self.user:
        validated_data["owner"] = self.user

The above snippet allowed us to distinguish between the situation where the owner field wasnt specified at all (in which case we set a value) vs when it is specified, including when it was specified as None.

Edit: I can workaround this with e.g. self.context["request"].data, but still...

[BPC-AnonymousBeast]

While its true that the solution above default=None does partially mitigate things, it is 1) a breaking change and 2) still leaves the following problem, where something like this now fails where it used to work:

def create(self, validated_data):

    if "owner" not in validated_data and self.user:

        validated_data["owner"] = self.user

Edit: I can workaround this with e.g. `self.context["request"].data`, but still...

I understand. But none of the contributors have confirmed if this will be considered an issue and will be worked upon. Waiting if someone responds.

[shamoon]

I understand. But none of the contributors have confirmed if this will be considered an issue and will be worked upon. Waiting if someone responds.

oh yes, I completely agree. Hope I didn’t sound argumentative or anything like that, just adding some input. Also I really appreciate you pointing out the workaround, it’s at least a path forward for us for now! Thank you.

[Zolton]

@BPC-AnonymousBeast

Thanks so much for trying it!

I went through my code and compared it to the PR that started this (#7438), which mostly centered around a change in the get_unique_together serializer.
I didn't think it was relevant when I made the OP, but I'm overriding perform_create for this table, and using get_or_create to tie it together with another table, so when one updates, the other does too. I'll tinker around with my views.py and serializers, maybe I can get things to work if I take some stuff out, which might help pinpoint where the issue is and why it's not accepting null values

Update: overriding perform_create and using get_or_create has no effect; I get the same error when I don't override and just rely on the default/built-in create functions

[auvipy]

I think it was done in a major version from 3.14 to 3.15 and the reason was valid behind the change. but I am willing to know counter logic /fixes

[Zolton]

@Zolton I don't think you have to make changes to your frontend at all. Referring to the code you have provided, you just need to add a keyword argument called default in your model class. This makes sure that a null value is stored if you don't provide data for a particular parameter. i.e if you omit a key-value pair.

Also please go through my comment in #9378. I have tried to reproduce the error. I used foreign key like it was done by you but I'm able to send a POST request without a parameter which is also a foreign key. I didn't receive any error. I have also provided my model and serializer code there, if you feel like I should make changes and try again then let me know.

To follow up on this, setting default=None fixed the issue; but this is fairly unexpected since I had blank=True and null=True as well, which has been working for quite awhile now.

I think the reason your code isn't able to reproduce the issue is it doesn't have any constraints. In my original post, if I remove the class Meta: constraints array, then missing key-value pairs are accepted just fine in my tests, so the real issue seems to be that applying constraints causes those fields to become required, and omissions aren't treated as null.

To the issue of the bug, I found this tidbit under https://www.django-rest-framework.org/api-guide/validators/,

Note: The UniqueTogetherValidator class always imposes an implicit constraint that all the fields it applies to are always treated as required

So the UniqueTogetherValidator makes things required, but the docs also say this:
https://www.django-rest-framework.org/api-guide/fields/#required

If you're using Model Serializer default value will be False if you have specified blank=True or default or null=True at your field in your Model

And then there's this:
https://www.django-rest-framework.org/api-guide/fields/#default

Note that setting a default value implies that the field is not required. Including both the default and required keyword arguments is invalid and will raise an error.

So:

• setting a unique constraint makes a field implicitly required
• but using null=True or blank=True implicitly sets a default value, and
• default values are implicitly not required.

It seems there's a tension between these statements, and I'm guessing a kind of hierarchy between which one wins existed before, but was changed by the PR I mentioned above

I'm not particularly well-versed in Django code and could easily be wrong, but my best guess is that this is why null=True was fine before - default was being set automatically and was higher in the hierarchy, and with the PR change, it's now lower in the undocumented hierarchy

[znotdead]

UniqueConstraint can be conditional with check if field is not null (this case is not fully supported with recent changes). In case when model have a field with null=True and blank=True, serializer should not change this field to required. Also some fields can be set automatically in pre_save signal of model for example, but with new changes serializer validation will fail first... this changes definitely should be in major release as lot's of code could be broken during upgrade from 3.14 to 3.15

[SorianoMarmol]

Is a new version expected with this fix and others related toUniqueConstraint ? The support for UniqueConstraint is not complete and may cause issues…