-
Notifications
You must be signed in to change notification settings - Fork 11.1k
Commit
This commit does not belong to any branch on this repository, and may belong to a fork outside of the repository.
- Loading branch information
1 parent
7ba0c22
commit 3d39604
Showing
3 changed files
with
43 additions
and
0 deletions.
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
3d39604
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.
@taylorotwell There is a pretty big problem here. People are expecting
orderedUuid
to return an ID that takes time into account. However, this is not possible by the spec of UUID's.First, simply take a look at the implementation of the v4 generator to see it never uses the codec for time. It just gets random seed data, then hex's it, and then generates the UUID from that. The
setCodec
call here to try and make it use time, is never accounted for in the pathway to make a v4 id.Second, the UUID specification itself (section 4.4) supports this notion.
Introducing a component of time, makes it into a v1 ID. Which is determined by time where v4's are supposed to be as completely random as possible. With no deterministic input.
You need to return a v1 UUID to get an output that can be ordered. V4's can't and shouldn't fulfill this need.
Fix paths:
First, we could add a new method that is called
uuidOrdered
. This wouldreturn Uuid::v1();
and be done. Then the current method would just return a v4 without all the extra logic, but trigger a warning to let developers know to convert to v1 types using the new method to get what they want and that the current method will be removed in the future since it isn't useful.Second, Simply let the current method trigger an error about the impending v1 switch. Then switch in 5.8 to return v1 types.
The first method is preferable for a few reasons:
The second method allows incorrect code to continue to manifest with no direct resolution path until multiple releases to come. This is not good for applications that are depending on the timestamp being included in the identifier.
So, this is a "comb uuid" type thing. Which means it isn't a true UUID but something that looks like one, can validate as one, but has some deterministic seeded data.
Also, the v1 idea for sorting doesn't work at all. Looking back into that, the timestamp is seeded across the bits. So it makes them unsortable since they aren't placed together. Since that helps create the problem this does which is sections that can be determined.
So this is fine technically. Just a confusing docblock since it didn't explain right away that it isn't generating a true v4 UUID but an alternate type that emulates the same structure.