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

feat: add UUIDv7 support with encoding options #65

Open
wants to merge 3 commits into
base: master
Choose a base branch
from

Conversation

ashwingopalsamy
Copy link

Changes

Addressing the issue #61, this PR introduces UUIDv7 support to shortuuid, bringing time-ordered UUID generation to our library.

What does the PR implement?

  • NewV7: Generates a UUIDv7 using the default base57 alphabet.
  • NewV7WithEncoder: Allows encoding UUIDv7 with a custom encoder.
  • NewV7WithNamespace: Creates a UUIDv5 (or UUIDv7 if the namespace is empty) and encodes it.
  • NewV7WithAlphabet: Encodes UUIDv7 with a custom alphabet.

Adds Unit Tests including Benchmarks. No breaking changes; everything is backward-compatible.

@ashwingopalsamy ashwingopalsamy mentioned this pull request Jan 11, 2025
shortuuid.go Outdated Show resolved Hide resolved
@anatoly-kussul
Copy link
Contributor

I think it's possible to reuse namespace logic like so:

func NewWithNamespace(name string) string {
	return newWithNamespace(name, uuid.NewRandom)
}

func NewV7WithNamespace(name string) string {
	return NewWithNamespace(name, uuid.NewV7)
}

func newWithNamespace(name string, defaultUUID func() (uuid.UUID, error)) string {
	var u uuid.UUID

	switch {
	case name == "":
		u = uuid.Must(defaultUUID())
	case hasPrefixCaseInsensitive(name, "https://"):
		u = hashedUUID(uuid.NameSpaceURL, name)
	case hasPrefixCaseInsensitive(name, "http://"):
		u = hashedUUID(uuid.NameSpaceURL, name)
	default:
		u = hashedUUID(uuid.NameSpaceDNS, name)
	}

	return DefaultEncoder.Encode(u)
}

Or maybe, it would be even better, to just introduce exported (naming is up for discussion)
MustNewWithNamespace(name string, defaultUUID func() (uuid.UUID, error)) string
so that it then can be used externally with any version as:

MustNewWithNamespace(name, uuid.NewV6)
MustNewWithNamespace(name, uuid.NewV7)

@ashwingopalsamy
Copy link
Author

Hey, thanks for reviewing. Its a good idea, but I feel that we should keep these functions separate. Here’s why:

Separate functions like NewWithNamespace and NewV7WithNamespace are easy to understand and follow the patterns developers expect from libraries like google/uuid and gofrs/uuid. Abstracting them into a single helper works just fine, but comes at the cost of clarity, especially for newcomers trying to understand or contribute to the codebase.

Also, I think this abstraction does not solve a real problem. The namespace logic is simple and unlikely to change often. Adding an exported function like MustNewWithNamespace feels like added complexity without much practical benefit for users.

Simplicity has its value here. Keeping the functions distinct makes the library clean and predictable; it does what users expect, with no extra layers to parse or understand.

That said, I get the intention behind the suggestion. Let me know if there’s something that I'm overlooking.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants