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

Add TypeID community implementation for Erlang #396

Open
wants to merge 2 commits into
base: main
Choose a base branch
from

Conversation

eproxus
Copy link

@eproxus eproxus commented Sep 25, 2024

Summary

Adds keysmith, a new TypeID Erlang community implementation.

How was it tested?

Markdown has been manually reviewed.

Adds keysmith, a new TypeID Erlang community implementation.

Signed-off-by: Adam Lindberg <[email protected]>
@loreto
Copy link
Contributor

loreto commented Sep 25, 2024

@eproxus Thanks for submitting!

Could you point me to the tests validating conformance with the typeid spec? Basically the tests that test these cases:

@eproxus
Copy link
Author

eproxus commented Sep 26, 2024

@loreto 👍 I'm working on adding those. I'll let you know when it is done.

@eproxus
Copy link
Author

eproxus commented Sep 26, 2024

@loreto Looking into the test data, it seems the test data does not follow the specification. The specification says:

  1. A UUID suffix: a 128-bit UUIDv7 encoded as a 26-character string in base32.

But the test data contains (among others) the UUID 00000000-0000-0000-0000-000000000000 which is not a valid UUID version 7 (the valid version with all zeroes would be 00000000-0000-7000-8000-000000000000).

Since my implementation does strict parsing and generation of UUIDs it cannot use this test data for automated tests.

@eproxus
Copy link
Author

eproxus commented Sep 26, 2024

I realize now that any custom UUID is technically allowed by the spec. I've added validation for the invalid IDs for now, and will work on adding support for validating the valid IDs as well.

@loreto
Copy link
Contributor

loreto commented Sep 26, 2024

I realize now that any custom UUID is technically allowed by the spec. I've added validation for the invalid IDs for now, and will work on adding support for validating the valid IDs as well.

Thanks! Yeah the way I see it is:

  • By default a library should always generate new TypeIDs using UUIDv7
  • However, if the user of the library so chooses, they can encode any UUID as a TypeID

@eproxus
Copy link
Author

eproxus commented Dec 4, 2024

I've now added support for a more general UUID format so both the valid and invalid test files now pass. You can find the test code here: https://github.com/eproxus/keysmith/blob/main/test/keysmith_tests.erl#L347-L383

@loreto
Copy link
Contributor

loreto commented Dec 4, 2024

LGTM

@loreto loreto enabled auto-merge (squash) December 4, 2024 21:27
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

2 participants