[UID] Added the component + Added support for UUID #35940

lyrixx · 2020-03-03T14:43:54Z

Q	A
Branch?	master
Bug fix?	no
New feature?	yes
Deprecations?	no
Tickets
License	MIT
Doc PR

<?php

require __DIR__.'/vendor/autoload.php';

use Symfony\Component\Uid\Uuid;

$u = Uuid::v1();
dump($u);
dump($u->isNull());
dump($u->getType());
dump($u->getVariant());
dump($u->getTime());
dump($u->getMac());
dump($u->isEqualsTo($u));
dump($u->compare($u));
dump(Uuid::fromBinary($u->toBinary()));

src/Symfony/Component/String/Tests/UuidTest.php

src/Symfony/Component/String/Uuid.php

nicolas-grekas

Nice thank you, very simple!

Why in the string component ?

Because that's where it fits bests to me: the String component gathers simple "string-like" value objects; UUID is an important one of them.

src/Symfony/Component/String/Uuid.php

wouterj · 2020-03-03T20:56:36Z

Given the number of UUID libraries in PHP/Symfony land (ref https://jolicode.com/blog/uuid-generation-in-php ), I would say this topic is oversaturated with libraries already. Is there a major pro for this new class instead of the Pecl extension/Symfony Polyfill or Ramsey UUID?

nicolas-grekas · 2020-03-03T21:12:04Z

Where do you see saturation here? pecl is low level: there need to be a value object in our OOP code. polyfill is ... a polyfill, and FFI+libuuid is a POC. Which means there is only one single available (and mainstream) implementation: ramsey/uuid.
The class proposed here provides the same benefits (a VO), with a very simple implementation.
Ppl that need to do fancy things with uuid - or that need guaranteed portability to 32b systems, will have to resort to ramsey/uuid. But the rest of the world will be happy to leverage a trivial implem.
At least I would.

src/Symfony/Component/String/Uuid.php

nicolas-grekas · 2020-03-04T10:22:30Z

this UUID class doesn't add any value on top of the PECL extension/polyfill

It does provide a lot: e.g a VO integrates with Doctrine types. The pecl extension doesn't.

About the doc for deps, there's nothing specific to this PR... The argument would apply to any new feature with optional deps...

src/Symfony/Component/String/Uuid.php

fabpot · 2020-03-05T13:26:03Z

Agreed on the new component proposal.

ro0NL · 2020-03-05T13:51:24Z

did we consider adding polyfill support for ramsey/uuid? esp. since it has 1st class pecl support already:

https://github.com/ramsey/uuid/blob/b31703e7c9752260ddc773c95110294fa9abc5ff/src/FeatureSet.php#L84
https://github.com/ramsey/uuid/blob/master/src/Generator/PeclUuidNameGenerator.php

Ppl that need to do fancy things with uuid - or that need guaranteed portability to 32b systems, will have to resort to ramsey/uuid

so, we shouldnt resort to ramsey/uuid for simple/non-fancy uuids? i feel like we're defeating it's UuidInterface + the fact most of the ecosystem is using it already.

It does provide a lot: e.g a VO integrates with Doctrine types.

so does ramsey/uuid.

im rather skeptical about vendor-a/uuid + vendor-b/uuid :)

localheinz

👍

Looks good to me code-wise.

However, I agree with @theofidry.

I believe the general question the core team need to discuss is whether Symfony wants to follow the same route Zend Framework did about a decade ago (pulling in a component for every single use case), or whether that is really necessary.

Pulling in a component for every single use case has the following down-sides:

more work required by maintainers
more maintainers required
more documentation required
more bug fixes required
more signal and noise on the central symfony/symfony repository
killing successful 3rd-party libraries

Not sure if symfony/symfony really needs to become that big black 🕳, sucking in everything around it.

dunglas

(I've no strong opinion regarding if we need a new component or if we can just rely on ramsey/uuid, but 👍 regarding the code).

.travis.yml

src/Symfony/Component/Uuid/LICENSE

wouterj · 2020-03-11T10:10:15Z

Just a random thought: Would the idea of the Uid component be stronger if we add a UidInterface contract, that can be used by any unique identifier introduced?

lyrixx · 2020-03-11T10:19:52Z

@wouterj What would be the interface ?

src/Symfony/Component/Uid/Tests/UuidTest.php

src/Symfony/Component/Uid/Uuid.php

OskarStark

2 minors

src/Symfony/Component/Uid/Uuid.php

nicolas-grekas

Still good to me after the recent changes. Good to go on my side.

fabpot · 2020-03-12T17:23:29Z

Thank you @lyrixx.

carsonbot added Status: Needs Review Feature labels Mar 3, 2020

lyrixx force-pushed the string-uuid branch from d5c1925 to f41cf74 Compare March 3, 2020 15:07

nicolas-grekas added this to the next milestone Mar 3, 2020

lyrixx requested a review from nicolas-grekas March 3, 2020 15:27

nicolas-grekas reviewed Mar 3, 2020

View reviewed changes

src/Symfony/Component/String/Tests/UuidTest.php Outdated Show resolved Hide resolved

nicolas-grekas reviewed Mar 3, 2020

View reviewed changes

src/Symfony/Component/String/Uuid.php Outdated Show resolved Hide resolved

This comment has been minimized.

Sign in to view

lyrixx force-pushed the string-uuid branch from f41cf74 to c700f30 Compare March 3, 2020 15:49

This comment has been minimized.

Sign in to view

lyrixx force-pushed the string-uuid branch 2 times, most recently from 546a7b9 to 232c30b Compare March 3, 2020 15:55

nicolas-grekas approved these changes Mar 3, 2020

View reviewed changes

carsonbot added Status: Reviewed and removed Status: Needs Review labels Mar 3, 2020

OskarStark reviewed Mar 3, 2020

View reviewed changes

src/Symfony/Component/String/Uuid.php Outdated Show resolved Hide resolved

This comment has been minimized.

Sign in to view

javiereguiluz reviewed Mar 4, 2020

View reviewed changes

src/Symfony/Component/String/Uuid.php Outdated Show resolved Hide resolved

src/Symfony/Component/String/Uuid.php Outdated Show resolved Hide resolved

localheinz reviewed Mar 4, 2020

View reviewed changes

src/Symfony/Component/String/Uuid.php Outdated Show resolved Hide resolved

localheinz reviewed Mar 4, 2020

View reviewed changes

src/Symfony/Component/String/Uuid.php Outdated Show resolved Hide resolved

This comment has been minimized.

Sign in to view

lyrixx force-pushed the string-uuid branch from 232c30b to 265eb01 Compare March 4, 2020 13:26

lyrixx force-pushed the string-uuid branch from 265eb01 to 67cb424 Compare March 5, 2020 13:52

lyrixx changed the title ~~[String] Added support for UUID~~ [UUID] Added the component Mar 5, 2020

localheinz reviewed Mar 6, 2020

View reviewed changes

dunglas approved these changes Mar 6, 2020

View reviewed changes

localheinz reviewed Mar 6, 2020

View reviewed changes

.travis.yml Outdated Show resolved Hide resolved

localheinz reviewed Mar 6, 2020

View reviewed changes

src/Symfony/Component/Uuid/LICENSE Outdated Show resolved Hide resolved

This comment has been minimized.

Sign in to view

lyrixx force-pushed the string-uuid branch from f193713 to d171180 Compare March 11, 2020 10:04

lyrixx changed the title ~~[UUID] Added the component~~ [Uid] Added the component + Added support for UUID Mar 11, 2020

lyrixx changed the title ~~[Uid] Added the component + Added support for UUID~~ [UID] Added the component + Added support for UUID Mar 11, 2020

lyrixx force-pushed the string-uuid branch from d171180 to 993ec72 Compare March 11, 2020 10:05

nicolas-grekas reviewed Mar 11, 2020

View reviewed changes

src/Symfony/Component/Uid/Tests/UuidTest.php Outdated Show resolved Hide resolved

src/Symfony/Component/Uid/Tests/UuidTest.php Outdated Show resolved Hide resolved

lyrixx force-pushed the string-uuid branch from 993ec72 to 386ca40 Compare March 11, 2020 14:42

OskarStark reviewed Mar 12, 2020

View reviewed changes

src/Symfony/Component/Uid/Uuid.php Show resolved Hide resolved

OskarStark approved these changes Mar 12, 2020

View reviewed changes

src/Symfony/Component/Uid/Uuid.php Outdated Show resolved Hide resolved

lyrixx force-pushed the string-uuid branch from 386ca40 to e89d2de Compare March 12, 2020 12:36

nicolas-grekas approved these changes Mar 12, 2020

View reviewed changes

[UID] Added the component + Added support for UUID

c3f14dd

lyrixx force-pushed the string-uuid branch from e89d2de to c3f14dd Compare March 12, 2020 17:21

fabpot approved these changes Mar 12, 2020

View reviewed changes

fabpot merged commit d108f7b into symfony:master Mar 12, 2020

fabpot mentioned this pull request Mar 12, 2020

[UID] Added the component + Added support for UUID symfony/symfony-docs#13333

Closed

lyrixx deleted the string-uuid branch March 13, 2020 16:03

nicolas-grekas modified the milestones: next, 5.1 May 4, 2020

fabpot mentioned this pull request May 5, 2020

Release v5.1.0-BETA1 #36713

Merged

Uh oh!

[UID] Added the component + Added support for UUID #35940

[UID] Added the component + Added support for UUID #35940

Uh oh!

Conversation

lyrixx commented Mar 3, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Uh oh!

Uh oh!

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

nicolas-grekas left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

wouterj commented Mar 3, 2020

Uh oh!

nicolas-grekas commented Mar 3, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

This comment has been minimized.

Uh oh!

Uh oh!

nicolas-grekas commented Mar 4, 2020

Uh oh!

Uh oh!

Uh oh!

This comment has been minimized.

fabpot commented Mar 5, 2020

Uh oh!

ro0NL commented Mar 5, 2020

Uh oh!

localheinz left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

dunglas left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

wouterj commented Mar 11, 2020

Uh oh!

lyrixx commented Mar 11, 2020

Uh oh!

Uh oh!

Uh oh!

Uh oh!

OskarStark left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

nicolas-grekas left a comment

Choose a reason for hiding this comment

Uh oh!

fabpot commented Mar 12, 2020

Uh oh!

Uh oh!

lyrixx commented Mar 3, 2020 •

edited

Loading

nicolas-grekas commented Mar 3, 2020 •

edited

Loading

localheinz left a comment •

edited

Loading