Unexpected ValueError instead of ValidationError

[penenkel]

Given the following model

class MyModel(models.Model):
    class MyChoices(models.TextChoices):
        A = "a"
        B = "b"
        C = "c"

    my_field = EnumField(MyChoices, null=True, blank=True, default=None)

I would have expected MyModel(my_field="").full_clean() to throw a ValidationError, but instead it throws a ValueError in the coercion step.

---

Apart from that, kudos to you and your library, that you throw an error at all. Im quite happy that I have found it.

I was growing quite frustrated with the Django CharField based approach wrt. to nullable enum fields. It is infuriating that one is required to set blank=true to allow setting those fields to None/Null, which then automatically implies that an empty string is also allowed and it even sets default="" by default. I mean why?!

[bckohan]

Thanks for the report! I'll look into it.

[bckohan]

Can confirm this is a bug triggered by blank=True, if blank=False a validation error is raised instead

[bckohan]

Whats happening here is that Model.full_clean runs all validation routines on all fields and collects the validation errors and then at the end if there are any it throws a master ValidationError with all the collected validation errors. In this case a ValidationError is being thrown in the EnumField.clean method, but the model full_clean proceeds until it his the check constraint validations that eventually call get_db_prep_value which throws the ValueError during type coercion. This exception escapes full_clean causing the problem.

This will be a little tricky to correct without changing all errors to ValidationErrors - which may be ok. Its not clear to me if Model.get_db_prep_value and Model.get_prep_value are expected to raise ValidationErrors or not.

[bckohan]

ok. I finally found some concrete guidance in the django docs:

For to_python(), if anything goes wrong during value conversion, you should raise a ValidationError exception.

[bckohan]

Our to_python() function does honor this though. I don't see the harm in extending the ValidationErrors to the db_prep calls though. I will make the change.

There's no official documentation I can find about what get_db_prep_value and get_prep_value are supposed to raise, but most of the Django fields call to_python in those functions which is spec'ed to throw ValidationErrors. I think this change should be minimally invasive - I'll go ahead and make it and issue a 3.0.1 release.

[bckohan]

However, can confirm that some django fields will happily throw a TypeError or ValueError out of get_prep_value. Look at IntegerField for example:

    def get_prep_value(self, value):
        value = super().get_prep_value(value)
        if value is None:
            return None
        try:
            return int(value)
        except (TypeError, ValueError) as e:
           raise e.__class__(
                "Field '%s' expected a number but got %r." % (self.name, value),
            ) from e

[bckohan]

I'm now pretty convinced this is a bug in Django's CheckConstraint.validate. There seems to be no attempt to catch ValueError/TypeError and recast to ValidationError - which could result in 500s instead of 400s when full_clean() is invoked in certain paths when using IntegerField and CharField.

Instead of modifying EnumField.get_prep_value I'm going to subclass CheckConstraint and report this issue upstream.

[penenkel]

I'm not quite sure if it is relevant here, but for the original Django CharField I figured out that .full_clean() acutally does not even try to validate an empty string if blank=True. Thats because it apperantly assumes all empty values to be valid in that case. And what an empty value is, is defined by the static attribute Field.empty_values. I managed to derive my own TextChoiceField from CharField and overwrote empty_values, now it is working as I want it to and declines empty strings because they are not part of the valid choices of my TextChoices type.

[bckohan]

Thanks for the tip! That led me down a better path. I was trying to recreate this issue with non-blank invalid enums and couldnt because I do see now that full_clean doesnt run check constraint validation on those.

So yes this is very specific to stric enum character fields when blank=True and strict=True where the empty string is also not a valid enumeration value. I will update the empty_values list for fields in that specific case.