[Uid] Add support for binary, base-32 and base-58 representations in Uuid::isValid()
#57940
Conversation
7c6a361 to
4fffe31
Compare
src/Symfony/Component/Uid/Uuid.php
Outdated
| @@ -130,8 +121,14 @@ final public static function v8(string $uuid): UuidV8 | |||
| return new UuidV8($uuid); | |||
| } | |||
|
|
|||
| public static function isValid(string $uuid): bool | |||
| public static function isValid(string $uuid, int $format = self::FORMAT_RFC_4122): bool | |||
There was a problem hiding this comment.
I'm not a big fan of bitfields in options (e.g. Uuid::FORMAT_BASE_32 | Uuid::FORMAT_RFC_4122). So, I wonder if we could replace this int by a string|array (maybe renamed the option too to $allowedFormats).
This would be similar to:
- The formats option in Css constraint
- The schemes option in CardScheme constraint
There was a problem hiding this comment.
I like bit-fields because it reads well, it is quick and easy to test a flag, but also because it offers nice auto-completion and static analysis thanks to @param int-mask-of.
Those seems a bit more difficult with a string|array. I'm curious to understand why you're not a big fan of bit-field and why'd you prefer an array of options? 🙂
There was a problem hiding this comment.
A bit-field is more appropriate to me also.
Note that adding an argument is a BC break, we should comment it and use func_get_arg instead
There was a problem hiding this comment.
Oh yes, I got confused with private methods. It's updated now
4fffe31 to
9bdefd5
Compare
9bdefd5 to
ca995f4
Compare
…`Uuid::isValid()`
ca995f4 to
64aa006
Compare
|
Thank you @alexandre-daubois. |
| if (36 === \strlen($uuid) && !($format & self::FORMAT_RFC_4122)) { | ||
| return false; | ||
| } | ||
|
|
||
| $uuid = self::transformToRfc4122($uuid, $format); | ||
|
|
There was a problem hiding this comment.
Hi @alexandre-daubois sorry to dig this up, but while 7.2 is not release..
I find this early condition non "optimal", as default case will be in fact $format = FORMAT_RFC_4122..
What if we did something like this here (before? instead?), avoiding the regexp if the string is not 36 chars long ?
if (($format & self::FORMAT_RFC_4122) && 36 !== \strlen($uuid)) {
return false;
}What do you think ?
There was a problem hiding this comment.
(before line 131, i messed up my selection 😅 )
There was a problem hiding this comment.
I did try a few things but it would require some refactoring because of how the control flow is currently. I'm not sure it would worth it and if performances would greatly improve 🤔
| * | ||
| * @param int-mask-of<Uuid::FORMAT_*> $format | ||
| * | ||
| * @return non-empty-string |
There was a problem hiding this comment.
@alexandre-daubois this is currently not true, afaik ... should we enforce it (or remove it) ?
There was a problem hiding this comment.
Indeed, but more than that, it's a bit confusing. I proposed this update: #58089
Allows to use a bitfield in
Uuid::isValid()to accept one or many formats: