Movatterモバイル変換


[0]ホーム

URL:


Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly

Sign up
Appearance settings

[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 ourterms of service andprivacy statement. We’ll occasionally send you account related emails.

Already on GitHub?Sign in to your account

Closed

Conversation

@guillbdx
Copy link

QA
Branch?master
Bug fix?no
New feature?yes
Deprecations?no
Tickets#36102
LicenseMIT

UID constraint.

@guillbdx
Copy link
Author

guillbdx commentedApr 9, 2020
edited
Loading

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 astrict option:https://symfony.com/doc/current/reference/constraints/Uuid.html#strict There is no notion ofstrict orloose 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 singleIp constraint validates IPv4 and IPv6, which are completely different between each other
  • A singleCardScheme constraint validates all kinds of credit card numbers, which are completely different between each other
gennadigennadigennadi reacted with thumbs up emoji

@wouterj
Copy link
Member

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-grekasnicolas-grekas added this to thenext milestoneApr 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

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

@guillbdx
Copy link
Author

guillbdx commentedAug 16, 2020
edited
Loading

@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

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

Sign up for freeto join this conversation on GitHub. Already have an account?Sign in to comment

Reviewers

No reviews

Assignees

No one assigned

Projects

None yet

Milestone

5.2

Development

Successfully merging this pull request may close these issues.

6 participants

@guillbdx@javiereguiluz@wouterj@nicolas-grekas@fabpot@carsonbot

[8]ページ先頭

©2009-2025 Movatter.jp