This is inline with how other types classes handle invalid values.

Is this maybe something we want to harden/enforce in future cake versions? Because sure, normally we'd expect users to have validation present to make sure valid values are being passed down the entity, but having null as a save fallback seems a bit weird to me.

Just looked up a few cases:

• BinaryUUID type with a invalid UUID
• BoolType with an array or unknown string
• any Date or DateTime based type with an invalid string value
• Decimal with an invalid number or unparsable string or array

In general we always seem to coerse an empty string to null which is fine i guess, but any other "value" being passed down to a database type should still be valid imho.

..but any other "value" being passed down to a database type should still be valid imho.

So what to you want to happen instead, if an invalid value cannot be marshalled to an enum instance?

I'd throw an InvalidArgumentException based exception just like we do in BoolType, DecimalType, IntegerType.

If it should be try-catched upwards is up for debate, but in general I'd like to remove that kind of "soft fallback" for invalid values on a database type layer.

If I'd quote you: What is the usecase for having this soft fallback to a null value?

What is the usecase for having this soft fallback to a null value?

The use case is to not have the app blow up for invalid user provided data. You mentioned expecting users to have validation rules but a validation failure doesn't prevent marshalling, the marshaller will try try to set the value for the entity field/property. So throwing an exception would mean user will just can a general error page/response instead of the page/response with validation errors.

Makes sense.

Well thats what I meant by

If it should be try-catched upwards is up for debate

All I am referencing here is the fact, that we already do throw exceptions in certain database types if the given value can't be processed for that specific type.

I know validation is only being called, if you do ->patchEntity() and not if you manually try to save a manually adjusted entity, but this then makes it even worse imho.

If an entity is built via request data and its not using ->patchEntity() and therefore skipping the validation step it should break on saving without a nice warning, as thats not the recommended way to do.

If an entity is manually built via e.g. a CLI command sync and saved without going through validation its also the programmers choice of doing so, therefore it should also throw an exception if something goes wrong and not default to null for unknown values.

This feels like a hidden escape gateway for bugs to happen without anyone knowing why.

And if we need to have a separate exception for that which can be caught in between to do that, then why not add it.

This feels like a hidden escape gateway for bugs to happen without anyone knowing why.

Wouldn't those bugs often be caught by application validation? Developers have the ability to use not null columns and validation and domain rules to prevent any invalid data from being persisted.

I think it is useful to align the behavior of the EnumType with other types which by and large return null on invalid data.

We could, or users could, add options to always throw if they want that behavior.

If we can fail to marshal, you can fail to validate. Just depends where you want that last stand to be.

Wouldn't those bugs often be caught by application validation? Developers have the ability to use not null columns and validation and domain rules to prevent any invalid data from being persisted.

Yes, thats exactly my point. Validation would catch that, but validation only happens when you do patchEntity()

I think I am going to prepare a separate issue/PR explaining more in detail (with tests) what I am trying to achieve here.

For now I am fine with this as well to align the EnumType with the others.

Validation would catch that, but validation only happens when you do patchEntity()

There are application rules too if you want to safeguard against invalid values set using Entity::set().