Non-nullable blank string-based model field validation doesn't prevent or clean `None`

[Simon Charette]

Since ​we discourage the use of NULLable string-based fields the assumed way of holding text that might be empty would be to use the blank option.

In this case I would expect full_clean to raise a ValidationError when an empty non-nullable field is assigned None:

from django.db import models

class A(models.Model):
    b = models.CharField(blank=True)

a = A(b=None)
a.full_clean()  # Passes
a.save()  # Integrity error

Unfortunately specifying blank=True disable all validation against the b field, leading to database integrity error upon calling save(). The obvious solution here would be to override A.clean to convert b to an empty string when it equals None but it strikes me as a non trivial change for a quite common use case: correctly allowing a string-based field to be empty.

This is not an issue when cleaning data retrieved from the usual application/form-url-encoded request.GET or request.POST since an empty value is always represented by the empty string. However, in the case of an application/json encoded payload one may either provide None or '' (null or '') to express emptiness. Which will trigger the issue described above.

Attaching a patch that special case the empty_values of non-nullable string based fields based on the empty_strings_allowed flag. All tests pass on Py2.7/SQLite3.

[Simon Charette]

See ​https://github.com/django/django/pull/2409

[Simon Charette]

As pointed out by Loïc this issue is related to #20205 which propose to clean '' to None for fields disallowing empty strings.

Maybe the correct solution here would be to clean None to '' for non-null fields allowing empty strings?

[Simon Charette]

Another solution might be to special case get_db_prep_value to convert None to '' when the field is not NULL'able and empty_strings_allowed.

This would be completely backward compatible in the case of a model specific clean() with a self.non_null_string_based_field is None conditional.

[Anssi Kääriäinen]

It seems the problem here is that for HTML forms, "" is the natural None value, so for example IntegerField="" should validate and set the field to Zero. But for model validation this isn't the case. In that use case providing "" for IntegerField doesn't make sense. We could maybe make forms treat "" specially (convert to None unless the underlying field accepts empty strings), and then remove "" from empty_values for practically every field. Unfortunately changes here seem problematic as the problem isn't that large currently, but the backwards compatibility issues will be large.

Also, maybe we could just remove "" from empty values for all fields that accept "" directly as a value. The logic is that while "" is kinda special value for CharField, it is just one value in its domain. However, for IntegerField (and other fields that do not accept "" directly) the empty string is not in their domain, and thus it can be treated as None.

I'm marking patch needs improvement for lack of better marker. What is needed here is a design decision of how we want to treat "" in CharFields.

[Jacob Walls]

Well, I came here to suggest wontfix, but Simon's batteries-included proposal is starting to tempt me.

This test case from 2010 indicates the supported design pattern that the naive fix to this ticket (run model validation on blankable/not-nullable fields instead of short-circuiting, as now) would break:

    def test_validation_with_empty_blank_field(self):
        # Since a value for pub_date wasn't provided and the field is
        # blank=True, model-validation should pass.
        # Also, Article.clean() should be run, so pub_date will be filled after
        # validation, so the form should save cleanly even though pub_date is
        # not allowed to be null.
        data = {
            'title': 'The state of model validation',
        }
        article = Article(author_id=self.author.id)
        form = ArticleForm(data, instance=article)
        self.assertEqual(list(form.errors), [])
        self.assertIsNotNone(form.instance.pub_date)
        article = form.save()

The preface to this test case says developers should be implementing an Article.clean(), which looks like this:

    def clean(self):
        if self.pub_date is None:
            self.pub_date = datetime.now()

If we run model validation on blankable not-nullable fields, we lose this design pattern.

This is also in agreement with Carlton's assessment here for BooleanField:

The validation weirdness when blank=True is a red-herring. It causes the field to be skipped during full_clean(). At that point developers should implementing clean_* to ensure they have an acceptable value. It's quite feasible that blank=True doesn't really make sense for a BooleanField, but I'm not sure we can tighten it up at this latter stage (no doubt there's some use-case…)

I think the test case at the top demonstrates the use case Carlton was reluctant to break for BooleanField, so I'm also reluctant to break it for string-based fields.

But Simon's proposal is another side of the same coin: if you didn't write a clean(), or your clean() missed a case, we could bail you out instead of propagating db failures.

Simon, do you have an opinion about whether your proposal still makes sense as a friendly guardrail? If so, then we should do something similar for #27697, by providing a nice {} if JSONField is not nullable.

#20205 presents a similar set of issues for PositiveIntegerField, but I think there *is* a fixable change there I plan to submit a patch for shortly.

[Jacob Walls]

​Per mailing list:

It does come up. It _is_ surprising that blank has this effect, and I'm not convinced it says clearly anywhere how you should approach it…

Having weighed the possible behavior changes and found them wanting, we're looking at documenting the blank=True and null=False idiom.

[Jacob Walls]

​PR

[Mariusz Felisiak <felisiak.mariusz@…>]

In cd6bddd:

Fixed #22224 -- Added note about supplying missing values for non-nullable fields with blank=True.

[Mariusz Felisiak <felisiak.mariusz@…>]

In 515d3c59:

[4.0.x] Fixed #22224 -- Added note about supplying missing values for non-nullable fields with blank=True.

Backport of cd6bddd44e0a1c3c6a96a3677f8366ef0e4b6782 from main