-
Notifications
You must be signed in to change notification settings - Fork 0
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
Changes for the Ramsey Uuid v5 generator #1
Changes for the Ramsey Uuid v5 generator #1
Conversation
// Use default namespace if none is provided. | ||
if (empty($namespace)) { | ||
$namespace = $this->namespace; | ||
} | ||
|
||
return Uuid::uuid5($namespace, $str)->toString(); | ||
return Uuid::uuid5(Uuid::NAMESPACE_DNS, $namespace)->toString(); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why wouldn't we allow people to provide a namespace?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We do allow it. The $namespace
variable is passed in. But based on the examples here (https://github.com/ramsey/uuid) it appears that we don't pass in a second string
// Generate a version 5 (name-based and hashed with SHA1) UUID object
$uuid5 = Uuid::uuid5(Uuid::NAMESPACE_DNS, 'php.net');
echo $uuid5->toString() . "\n"; // i.e. c4a760a8-dbcf-5254-a0d9-6a4474bd1b62
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Dear people, i think you have that uuid v5 implementation wrong.
UUIDv5 depends on a previous generated UUID. So there are two ones, the first uses Uuid::NAMESPACE_DNS
, the second one is based on the first. Please look at:
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I checked through the Ramsey code because I was curious, and it looks like it takes care of that for us behind the scenes. I'm scrambling right now with a hundred other things, but when I get time, the best way I can check is to see if it keeps giving use the EXACT same uuid, or a new one every time. If it's the same every time, then you're probably right, @DiegoPino.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Dani, just to be sure, without the string you get always the same if using Uuid::NAMESPACE_DNS + domain
So this is a salt. and makes unique base UUID for your own domain. Uuid::NAMESPACE_DNS = is a constant UUID defined in RFC.
If you pass this output to the generator as first input and then the string (lets say an dc identifier) you get the final one. If not you will get always the same. So both of you are right, you need to run both. (but the first one should be static because if the config is not changing neither that one, the reason i put it so on the old silex version)
https://www.ietf.org/rfc/rfc4122.txt (page 18)
Changes for the Ramsey Uuid v5 generator
No description provided.