Skip to content

Commit

Permalink
Add IsZero() and fix empty suffix bug (#352)
Browse files Browse the repository at this point in the history
## Summary
- Adds an `IsZero` method that returns true if the typeid has the zero
suffix
- Fixes bug where parsing allowed for the empty string as a suffix, and
then generated a random suffix in it's place.

## How was it tested?
Wrote unit tests and ran them.
  • Loading branch information
loreto authored Jul 23, 2024
1 parent f81dd70 commit bd6a65c
Show file tree
Hide file tree
Showing 5 changed files with 65 additions and 7 deletions.
12 changes: 10 additions & 2 deletions constructors.go
Original file line number Diff line number Diff line change
Expand Up @@ -60,7 +60,7 @@ func FromSuffix[T Subtype, PT SubtypePtr[T]](suffix string) (T, error) {
}

prefix := defaultType[T]()
return from[T, PT](prefix, suffix)
return parse[T, PT](prefix, suffix)
}

// FromString parses a TypeID from a string of the form <prefix>_<suffix>
Expand All @@ -83,7 +83,7 @@ func Parse[T Subtype, PT SubtypePtr[T]](s string) (T, error) {
var id T
return id, err
}
return from[T, PT](prefix, suffix)
return parse[T, PT](prefix, suffix)
}

func split(id string) (string, string, error) {
Expand Down Expand Up @@ -146,6 +146,14 @@ func fromUUID[T Subtype, PT SubtypePtr[T]](prefix, uidStr string) (T, error) {
return nilID, err
}
suffix := base32.Encode(uid)
return parse[T, PT](prefix, suffix)
}

func parse[T Subtype, PT SubtypePtr[T]](prefix string, suffix string) (T, error) {
if suffix == "" {
var id T
return id, errors.New("suffix can't be the empty string")
}
return from[T, PT](prefix, suffix)
}

Expand Down
4 changes: 2 additions & 2 deletions subtype.go
Original file line number Diff line number Diff line change
Expand Up @@ -32,8 +32,8 @@ func (tid *TypeID[P]) init(prefix string, suffix string) {
tid.prefix = prefix
}

// If we're dealing with the "nil" suffix, we don't need to store it.
if suffix != nilSuffix {
// If we're dealing with the "zero" suffix, we don't need to store it.
if suffix != zeroSuffix {
tid.suffix = suffix
}
}
Expand Down
10 changes: 10 additions & 0 deletions testdata/invalid.yml
Original file line number Diff line number Diff line change
Expand Up @@ -97,3 +97,13 @@
- name: prefix-underscore-end
typeid: "prefix__00000000000000000000000000"
description: "The prefix can't end with an underscore"

# Tests specific to our golang implementation, consider promoting to the
# spec tests
- name: empty
typeid: ""
description: "The empty string is not a valid typeid"

- name: prefix-empty
typeid: "prefix_"
description: "The suffix can't be the empty string"
18 changes: 15 additions & 3 deletions typeid.go
Original file line number Diff line number Diff line change
Expand Up @@ -19,13 +19,13 @@ func (tid TypeID[P]) Prefix() string {
return defaultPrefix[P]()
}

const nilSuffix = "00000000000000000000000000"
const zeroSuffix = "00000000000000000000000000"

// Suffix returns the suffix of the TypeID in it's canonical base32 representation.
func (tid TypeID[P]) Suffix() string {
// We want to treat the "empty" TypeID as equivalent to the Nil typeid
// We want to treat the "empty" TypeID as equivalent to the 'zero' typeid
if tid.suffix == "" {
return nilSuffix
return zeroSuffix
}
return tid.suffix
}
Expand Down Expand Up @@ -59,6 +59,18 @@ func (tid TypeID[P]) UUID() string {
return uuid.FromBytesOrNil(tid.UUIDBytes()).String()
}

// IsZero returns true if the suffix of the TypeID is the zero suffix:
// "00000000000000000000000000"
//
// Note that IsZero() returns true regardless of the prefix value. All
// of these ids would return `IsZero == true`:
// + "prefix_00000000000000000000000000"
// + "test_00000000000000000000000000"
// + "00000000000000000000000000"
func (tid TypeID[P]) IsZero() bool {
return tid.suffix == "" || tid.suffix == zeroSuffix
}

// Must returns a TypeID if the error is nil, otherwise panics.
// Often used with New() to create a TypeID in a single line as follows:
// tid := Must(New("prefix"))
Expand Down
28 changes: 28 additions & 0 deletions typeid_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -39,6 +39,34 @@ func TestNilIsEmpty(t *testing.T) {
assert.Equal(t, nilID.UUIDBytes(), emptyID.UUIDBytes())
}

func TestIsZero(t *testing.T) {
testdata := []struct {
input string
output bool
}{
// IsZero == true values
{"00000000000000000000000000", true},
{"prefix_00000000000000000000000000", true},
{"other_00000000000000000000000000", true},
// IsZero == false values
{"00000000000000000000000001", false},
{"prefix_00000000000000000000000001", false},
{"other_00000000000000000000000001", false},
}

for _, td := range testdata {
t.Run(td.input, func(t *testing.T) {
tid, err := typeid.FromString(td.input)
if err != nil {
t.Error(err)
}
if tid.IsZero() != td.output {
t.Errorf("TypeId.IsZero should be %v for id %s", td.output, td.input)
}
})
}
}

func TestInvalidPrefix(t *testing.T) {
testdata := []struct {
name string
Expand Down

0 comments on commit bd6a65c

Please sign in to comment.