uuid icon indicating copy to clipboard operation
uuid copied to clipboard

Added factory method for UUIDv4 with monotonic prefix

Open Bilge opened this issue 3 years ago • 6 comments

Added a factory method for UUIDv4 with monotonic prefix (COMB codec).

Description

The documentation describes a method of augmenting the standard UUIDv4 generation with an algorithm that substitutes a monotonic prefix. This PR wraps up that code in a static factory method that's easier to use.

Motivation and context

A weakness of the current implementation is that it requires the user to fiddle with a number of internal structures, including codecs and random number generators, which should be considered an implementation detail that users don't want to be burdened with. In particular, this becomes problematic if the code breaks in future due to upgrades and changes to implementation details because users cannot be expected to know how to fix this. By moving the invocation to a static factory method, it is the library's responsibility to make sure this functionality continues to work via a simpler construction mechanism, which is much easier for users to consume and reduces the chance that their code breaks in future.

How has this been tested?

Ran tests locally.

Types of changes

  • [ ] Bug fix (non-breaking change which fixes an issue)
  • [x] New feature (non-breaking change which adds functionality)
  • [ ] Breaking change (fix or feature that would cause existing functionality to change)

Checklist:

  • [x] My code follows the code style of this project.
  • [x] My change requires a change to the documentation.
  • [ ] I have updated the documentation accordingly.
  • [x] I have read the CONTRIBUTING.md document.
  • [x] I have added tests to cover my changes.
  • [x] All new and existing tests passed.
  • [x] I have run composer run-script test locally, and there were no failures or errors.

Bilge avatar Aug 23 '20 19:08 Bilge

I rejected a similar PR (#205) a few years ago. I'll have to give this some thought. I want to encourage folks to start using v6 (and v7, when implemented) UUIDs, instead of the COMB UUIDs.

ramsey avatar Aug 23 '20 20:08 ramsey

Why is v6 preferred over COMB?

Bilge avatar Aug 23 '20 20:08 Bilge

If I may share my thoughts... this comes down to one of two things: either COMB is part of this library now and for the future or it is not and will be deprecated and scheduled for removal. If it's going to be removed then this PR is redundant but if it's going to remain available in future versions then this factory method is necessary. This is in spite of whether you prefer v6 over COMB because making COMB hard to use is not the correct way to discourage people from using it. If v6 is strictly better than COMB then this should be documented with an explanation as to why, but that is a separate issue.

On the topic of v6 vs COMB, it seems to me that for users who just want a random UUID with monotonic prefix, COMB is a better fit than v6, because v6 requires more code to achieve the same thing, i.e. it requires the user to configure node randomizing.

tl;dr: if COMB has a future as part of this library, it should be easy to use for those whom wish to use it.

Bilge avatar Aug 23 '20 20:08 Bilge

Right. v6 is not technically the same. However, v7 (not yet implemented) is effectively the same as a timestamp-first COMB. v6 and v7 are also non-standard, but I'm hoping to make them standard.

ramsey avatar Aug 23 '20 20:08 ramsey

OK, but v7 doesn't exist yet, so for the time being doesn't it warrant a static factory method for COMB?

Bilge avatar Aug 23 '20 20:08 Bilge

Codecov Report

Merging #336 (aec4c87) into 4.x (01433b5) will decrease coverage by 0.10%. The diff coverage is 81.81%.

Impacted file tree graph

@@             Coverage Diff              @@
##                4.x     #336      +/-   ##
============================================
- Coverage     95.53%   95.43%   -0.11%     
- Complexity      554      556       +2     
============================================
  Files            64       64              
  Lines          1479     1490      +11     
============================================
+ Hits           1413     1422       +9     
- Misses           66       68       +2     
Impacted Files Coverage Δ
src/Uuid.php 94.31% <0.00%> (-2.20%) :arrow_down:
src/UuidFactory.php 98.37% <100.00%> (+0.12%) :arrow_up:

codecov[bot] avatar Oct 29 '20 23:10 codecov[bot]

Thank you for your work on this.

Since this library now supports v6 and v7 UUIDs. I will deprecate both the ordered-time and timestamp-first COMB codecs, since they are no longer necessary.

ramsey avatar Dec 19 '22 20:12 ramsey