-
Notifications
You must be signed in to change notification settings - Fork 110
Add support for symfony uuid #689
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
Conversation
8eaaa75
to
7acda1a
Compare
9c036d9
to
525dac5
Compare
525dac5
to
fe96e38
Compare
Thank you, works nicely here. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Same points apply to UuidTypeDescriptor.
Coding standard failures here |
Thank you. |
How would you recommend dealing with the different subtypes? |
Is there a benefit to typing to Uuid7 rather than Uuid ? Is it sure to be an Uuid7 ? Even if you migrate manually a value in database directly ? So far I would say an easy way is to ignore it in the baseline. |
The benefits of uuidv7 are in (time) sortability and indexability. Hence we want to be strict about it, as it's important in our systems. But it's a bit of a grey area, we obviously validate its v7 in our domain code where we generate it, so there's an argument to be made that the persistence layer only cares about the fact its a UUID and not about the specific version. The argument about manually migrated data seems irrelevant to me, that's exactly what these doctrine types are for, to be able to verify that during persistence/fetching. With the generic UUID type it also wouldn't guarantee that the database doesn't contain random strings. |
But the purpose of the EntityColumn rule from PHPStan is to avoid possible crash. PHPStan has right to say you could have manually inserted a non-uuid7 in the database and it will be loaded as another Uuid. The same way if you type a nullable field with And again if you think this error is unrelevant, the pattern is easy to ignore. |
Unless the Doctrine type guarantees UUIDv7 will be assigned to the property, this reported error is valid. |
Closes #682