Skip to content

[Validator] UID Constraint. #36407

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

Closed
wants to merge 1 commit into from

Conversation

guillbdx
Copy link

@guillbdx guillbdx commented Apr 9, 2020

Q A
Branch? master
Bug fix? no
New feature? yes
Deprecations? no
Tickets #36102
License MIT

UID constraint.

@guillbdx
Copy link
Author

guillbdx commented Apr 9, 2020

This PR proposes a single Uid constraint and allows the user to choose the UID types between ULID, UUID_V1, UUID_V3, UUID_V4, UUID_V5 and UUID_V6 (multiple values possible).

According to the feedbacks received so far, it looks like some people would prefer to have not only one single Uid Constraint, but 2 constraints:

  • a Ulid Constraint
  • a Uuid Constraint

It's ok and i will update my code accordingly. But we have to take a few decisions about the Uuid Constraint. Indeed, there is already a Uuid Constraint in Symfony: https://symfony.com/doc/current/reference/constraints/Uuid.html So the purpose would be to update this existing contraint in order to use the new Uid Component.

But this existing constraint has a strict option: https://symfony.com/doc/current/reference/constraints/Uuid.html#strict There is no notion of strict or loose in the new Uid Component. Should we just deprecate this option?

By the way, the existing constraint can valid V2 UUID versions. V2 is not handled in the new Uid Component. Should we juste use the Uuid::isValid() method and check for number 2 at proper position to valid UUID V2 uuids?

Should we also accept UUIDs encoded in base 58 and 32?

@javiereguiluz
Copy link
Member

I'd vote to create a single constraint for all kinds of Uids.

That would be consistent with what we already do in other constraints:

  • A single Ip constraint validates IPv4 and IPv6, which are completely different between each other
  • A single CardScheme constraint validates all kinds of credit card numbers, which are completely different between each other

@wouterj
Copy link
Member

wouterj commented Apr 10, 2020

IPv4 and IPv6 are both different versions of the IP adres. "UID" is just an abbreviation for a fancy name, it does not define a specific string or format. E.g. uniqid() can also be called a UID. Anything that is an identifier and is unique is a UID.

That's why I don't think a single constraint makes sense. Also, as said in the comment of @guillbdx, there is already a UUID constraint. So it makes sense to focus this PR on updating the UUID constraint and introducing a ULID one.


About loose/strict: We can keep it imho. Loose will validate just the string as it's doing atm, the strict variant will use the UID component.

@nicolas-grekas nicolas-grekas added this to the next milestone Apr 10, 2020
@guillbdx
Copy link
Author

Thanks @javiereguiluz and @wouterj I will wait for other feedbacks to see what are the other pro and cons.

@nicolas-grekas
Copy link
Member

I agree with @wouterj
I also think we need a way to accept short representations of these ids (in URLs typically.)
I don't know exactly how this would all work together (with entities esp.) so let's iterate and figure out.

@fabpot
Copy link
Member

fabpot commented Aug 12, 2020

@guillbdx Still interested in working on this PR? @wouterj and @nicolas-grekas agree on a way to move forward.

@guillbdx
Copy link
Author

guillbdx commented Aug 16, 2020

@fabpot I won't have time to work on it for a while, would be better that someone take it over.

@fabpot
Copy link
Member

fabpot commented Aug 17, 2020

Thanks for the head up. As there is an associated issue, let’s close here.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants