8000 [Uid] add AbstractUid and interop with base-58/32/RFC4122 encodings by nicolas-grekas · Pull Request #36074 · symfony/symfony · GitHub
[go: up one dir, main page]
Skip to content

[Uid] add AbstractUid and interop with base-58/32/RFC4122 encodings #36074

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.

Sign up for GitHub

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

Jump to bottom
Merged
merged 1 commit into from
Mar 15, 2020
Merged

[Uid] add AbstractUid and interop with base-58/32/RFC4122 encodings #36074

merged 1 commit into from
Mar 15, 2020

Conversation

nicolas-grekas
Copy link
Member
@nicolas-grekas nicolas-grekas commented Mar 14, 2020 •
edited
Loading
Q A
Branch? master
Bug fix? no
New feature? yes
Deprecations? no
Tickets -
License MIT
Doc PR -

This PR provides a base AbstractUid class that is shared by Uuid and Ulid.

It adds new methods that provide interoperability between all types of UIDs but also between different encodings of UIDs:

  • toBase58() using the bitcoin alphabet - that's 22 chars aka "short-ids" - case sensitive
  • toBase32() using Crockford's alphabet - 26 chars ids - case insensitive
  • toRfc4122() to represent as a UUID-formatted string - 36 chars

This adds to toBinary() and to fromString(), the latter being able to cope with any of the 4 representations to create any kind of UIDs.

@nicolas-grekas nicolas-grekas added this to the next milestone Mar 14, 2020
@javiereguiluz
Copy link
Member

I don't know the future plans of this component (will we keep adding other popular UIDs?) but this doesn't look future-proof to me. Any method that we add here could look outdated or unused in other future UID generators.

This reminds me to the salt property of password encoders. It was obvious to add it to an interface because every encoder uses it ... until some stopped using it and the interface no longer looked good for modern encoders.

@nicolas-grekas
Copy link
Member Author
nicolas-grekas commented Mar 14, 2020 •
edited
Loading

All serious id schemes use 128-bit identifiers, which is wide enough to deal with any use cases for the foreseeable future. Then, base58, base32, uuid-format are 3 widely used representation of 128-bit numbers. They fit various needs: 58 bit for compactness, 32 bit for case insensitivity, uuid-format for interop with e.g. DB storages.

My opinion is that all three of them are generic enough to land in the component.

Your comparison with salted-encoders is good: we did add them to the component because that's what ppl were widely using at the time. We don't know the future but we know these encoding schemes are widely used and have good reasons to remain so.

@nicolas-grekas
Copy link
Member Author

will we keep adding other popular UIDs?

UUIDv6 would make sense. I'm not sure about others. I think we should work on integrating with Doctrine before considering other schemes. Then future will tell, there are no plans...

@fabpot
Copy link
Member
fabpot commented Mar 15, 2020

Can you rebase?

@nicolas-grekas
Copy link
Member Author

Now rebased.

@fabpot
Copy link
Member
fabpot commented Mar 15, 2020

Thank you @nicolas-grekas.

@fabpot fabpot merged commit ade47dd into symfony:master Mar 15, 2020
@fabpot fabpot mentioned this pull request Mar 15, 2020
Closed
@nicolas-grekas nicolas-grekas deleted the uuid-abstract branch March 15, 2020 10:41
Tobion added a commit to Tobion/symfony that referenced this pull request Mar 15, 2020
@Tobion
remove unused uuid property
901e62a 
leftover from symfony#36074
@Tobion Tobion mentioned this pull request Mar 15, 2020
Merged
nicolas-grekas added a commit that referenced this pull request Mar 15, 2020
@nicolas-grekas
minor #36086 [UID] remove unused property (Tobion)
f37d901 
This PR was merged into the 5.1-dev branch.

Discussion
----------

[UID] remove unused property

| Q             | A
| ------------- | ---
| Branch?       | master
| Bug fix?      | no
| New feature?  | no <!-- please update src/**/CHANGELOG.md files -->
| Deprecations? | no <!-- please update UPGRADE-*.md and src/**/CHANGELOG.md files -->
| Tickets       |
| License       | MIT
| Doc PR        |

leftover from #36074

Commits
-------

901e62a remove unused uuid property
symfony-splitter pushed a commit to symfony/uid that referenced this pull request Mar 15, 2020
@Tobion
remove unused uuid property
acd0fc8 
leftover from symfony/symfony#36074
@nicolas-grekas nicolas-grekas modified the milestones: next, 5.1 May 4, 2020
@fabpot fabpot mentioned this pull request May 5, 2020
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@fabpot fabpot fabpot approved these changes

Assignees
No one assigned
Labels
Feature Status: Reviewed
Projects
None yet
Milestone
5.1
Development

Successfully merging this pull request may close these issues.
4 participants
@nicolas-grekas @javiereguiluz @fabpot @carsonbot
0