Skip to content
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

\Expose\Converter\Short: Support UUID v7 #15

Open
asgraf opened this issue Aug 31, 2023 · 15 comments
Open

\Expose\Converter\Short: Support UUID v7 #15

asgraf opened this issue Aug 31, 2023 · 15 comments
Labels
enhancement New feature or request help wanted Extra attention is needed

Comments

@asgraf
Copy link

asgraf commented Aug 31, 2023

Here is test code:

$uuidOrginal = \Ramsey\Uuid\Uuid::uuid7()->toString();
$uuidShort = \Expose\Converter\ConverterFactory::getConverter()->encode($uuidOrginal);
$uuidDecoded = \Expose\Converter\ConverterFactory::getConverter()->decode($uuidShort);
debug($uuidOrginal === $uuidDecoded);

debug is returning false when default \Expose\Converter\Short is used

Changing to \Expose\Converter\KeikoShort by adding line below makes debug returns true as expected

//in config
'Expose.converter' => \Expose\Converter\KeikoShort::class,
@dereuromark dereuromark mentioned this issue Aug 31, 2023
@dereuromark
Copy link
Owner

Yeah I dont know what the issue is, but I documented it for now. See PR.
If someone finds a way to fix it up, that would be awesome.

@dereuromark
Copy link
Owner

@asgraf
Copy link
Author

asgraf commented Aug 31, 2023

The issue is that the strings are shifted

'018a4bd3-0218-727e-ad08-33ac76dd1593' <-uuidOrginal
'18a4bd30-2187-27ea-d083-3ac76dd15930' <-uuidDecoded

uuidOrginal has leading zero
uuidDecoded has trailing zero

@asgraf
Copy link
Author

asgraf commented Aug 31, 2023

Look at those two strings without dashes:

018a4bd30218727ead0833ac76dd1593
 18a4bd30218727ead0833ac76dd15930

if you ignore the hyphens and those two zeros then both strings are identical

@dereuromark dereuromark changed the title \Expose\Converter\Short is bugged Support for UUID v7 Aug 31, 2023
@dereuromark dereuromark added enhancement New feature or request help wanted Extra attention is needed labels Aug 31, 2023
@dereuromark
Copy link
Owner

I documented the issue now, see merged PR, I will keep this open for now, maybe someone has an idea for support.
I wonder if a separated issue should be made for keiko converter, as they might face a similar issue there ( https://github.com/mgrajcarek/uuid-shortener/issues ) from the test cases that prove it also fails for them.
Maybe double check before posting there if their native converter doesnt support yet v7 either.

@asgraf
Copy link
Author

asgraf commented Aug 31, 2023

I don't think this has anything to do with v7 but with leading zeros.
Any uuid with leading zeros would be problematic
Uuid v7 just happens to have unix timestamp encoded in leading bytes and we are many years away to have non zero value in first byte
Other uuid types that have random bytes in leading bytes just have low probability to have leading zero (1 in 16)

@dereuromark
Copy link
Owner

Yeah, but it is weird that both converters have this issue, not aware of this problem

@dereuromark
Copy link
Owner

At least in the tests, how did u make Keiko work?

@asgraf
Copy link
Author

asgraf commented Sep 1, 2023

At least in the tests, how did u make Keiko work?

You are just testing wrong converter in your test
You should replace ConverterFactory::getConverter() with $this->converter in KeikoShortTest.php and ShortTest.php

ConverterFactory::getConverter() returns whatever converter you have defined in your config file instead of the one you want to test

@dereuromark
Copy link
Owner

Copy and paste^^ thx, will fix laters

@asgraf
Copy link
Author

asgraf commented Sep 1, 2023

As i expected source of problem are leading zeros

$uuidOrginal = '00000000-1111-2222-3333-444444444444';
$uuidShort = \Expose\Converter\ConverterFactory::getConverter()->encode($uuidOrginal);
$uuidDecoded = \Expose\Converter\ConverterFactory::getConverter()->decode($uuidShort);
debug($uuidDecoded);
//  \Expose\Converter\Short outputs: 11112222-3333-4444-4444-444400000000

I think this happens because you are converting hex string to hex number and converting numeric string to number always drops leading zeroes (base 16 does not matter). So you are going from 00000000111122223333444444444444 string to 111122223333444444444444 number
And then you convert this number back to fixed length string that results in extra trailing zeroes

@asgraf asgraf changed the title Support for UUID v7 \Expose\Converter\Short is bugged when uuid has leading zeros Sep 1, 2023
@asgraf
Copy link
Author

asgraf commented Sep 1, 2023

Another example:

$uuidOrginal = '00000000-0000-0000-0000-00000000000F';
$uuidShort = \Expose\Converter\ConverterFactory::getConverter()->encode($uuidOrginal);
$uuidDecoded = \Expose\Converter\ConverterFactory::getConverter()->decode($uuidShort);
debug($uuidDecoded);
//  \Expose\Converter\Short outputs: f0000000-0000-0000-0000-000000000000

In this example '00000000-0000-0000-0000-00000000000F hex string will be converted to single digit hex number f

@dereuromark
Copy link
Owner

Do u have an idea for a fix? Currently traveling for vacation

@asgraf
Copy link
Author

asgraf commented Sep 4, 2023

Sorry i currently do not have any free cycles for this

@dereuromark dereuromark changed the title \Expose\Converter\Short is bugged when uuid has leading zeros \Expose\Converter\Short: Support UUID v7 Sep 10, 2023
@dereuromark
Copy link
Owner

OK, I documented for now to use KeikoShort in this case. I will leave this ticket open in case anyone wants to implement a minimal solution/fix for it.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request help wanted Extra attention is needed
Projects
None yet
Development

No branches or pull requests

2 participants