Skip to content

Commit 99c0b7f

Browse files
authored
[typeid] Remove NullableID in favor of sql.Null[TypeID], remove Must() (#519)
## Summary Replace NullableID with sql.Null[TypeID] for nullable columns - Remove custom NullableID type in favor of Go's standard sql.Null[TypeID] - Update all tests to use sql.Null[TypeID] instead of NullableID - Update examples to demonstrate sql.Null[TypeID] usage - Add documentation recommending sql.Null[TypeID] for nullable columns - Maintain full backward compatibility for database operations Remove Must function from typeid.go to keep API surface small. Introduce MustParse as a helper only in tests (not part of public API) ## How was it tested? Updated unit tests and ran them. ## Community Contribution License All community contributions in this pull request are licensed to the project maintainers under the terms of the [Apache 2 License](https://www.apache.org/licenses/LICENSE-2.0). By creating this pull request I represent that I have the right to license the contributions to the project maintainers under the Apache 2 License as stated in the [Community Contribution License](https://github.com/jetify-com/opensource/blob/main/CONTRIBUTING.md#community-contribution-license).
1 parent 3f47986 commit 99c0b7f

File tree

7 files changed

+58
-92
lines changed

7 files changed

+58
-92
lines changed

typeid/typeid-go/bench_test.go

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -52,7 +52,7 @@ var testTypeIDs = func() []typeid.TypeID {
5252
var ids []typeid.TypeID
5353
for _, prefix := range prefixPatterns {
5454
for i := 0; i < 10; i++ {
55-
ids = append(ids, typeid.Must(typeid.Generate(prefix)))
55+
ids = append(ids, typeid.MustGenerate(prefix))
5656
}
5757
}
5858
return ids
@@ -431,8 +431,8 @@ func BenchmarkTypicalUsage(b *testing.B) {
431431
// Client pattern: few creates, many string conversions
432432
b.Run("client", func(b *testing.B) {
433433
// Pre-create TypeIDs
434-
userID := typeid.Must(typeid.Generate("user"))
435-
sessionID := typeid.Must(typeid.Generate("session"))
434+
userID := typeid.MustGenerate("user")
435+
sessionID := typeid.MustGenerate("session")
436436

437437
b.ReportAllocs()
438438
b.ResetTimer()
@@ -457,11 +457,11 @@ func BenchmarkTypicalUsage(b *testing.B) {
457457
var tid typeid.TypeID
458458
for b.Loop() {
459459
// Create new request ID
460-
tid = typeid.Must(typeid.Generate("req"))
460+
tid = typeid.MustGenerate("req")
461461
s = tid.String()
462462

463463
// Create response ID
464-
tid = typeid.Must(typeid.Generate("resp"))
464+
tid = typeid.MustGenerate("resp")
465465
s = tid.String()
466466
}
467467

typeid/typeid-go/encoding_test.go

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -25,7 +25,7 @@ func TestJSONValid(t *testing.T) {
2525
for _, td := range testdata {
2626
t.Run(td.Name, func(t *testing.T) {
2727
// Test MarshalText via JSON encoding
28-
tid := typeid.Must(typeid.Parse(td.Tid))
28+
tid := typeid.MustParse(td.Tid)
2929
encoded, err := json.Marshal(tid)
3030
assert.NoError(t, err)
3131
assert.Equal(t, `"`+td.Tid+`"`, string(encoded))
@@ -47,7 +47,7 @@ func TestAppendTextValid(t *testing.T) {
4747

4848
for _, td := range testdata {
4949
t.Run(td.Name, func(t *testing.T) {
50-
tid := typeid.Must(typeid.Parse(td.Tid))
50+
tid := typeid.MustParse(td.Tid)
5151

5252
// Test AppendText with nil slice (equivalent to MarshalText)
5353
result, err := tid.AppendText(nil)
@@ -121,21 +121,21 @@ func TestJSONOmitZero(t *testing.T) {
121121
},
122122
{
123123
name: "constructed zero ID",
124-
typeID: typeid.Must(typeid.Parse("00000000000000000000000000")),
124+
typeID: typeid.MustParse("00000000000000000000000000"),
125125
expectedWithout: `{"id":"00000000000000000000000000"}`,
126126
expectedWith: `{}`,
127127
description: "constructed zero ID should omit with omitzero tag",
128128
},
129129
{
130130
name: "prefixed zero ID",
131-
typeID: typeid.Must(typeid.Parse("user_00000000000000000000000000")),
131+
typeID: typeid.MustParse("user_00000000000000000000000000"),
132132
expectedWithout: `{"id":"user_00000000000000000000000000"}`,
133133
expectedWith: `{"id":"user_00000000000000000000000000"}`,
134134
description: "prefixed zero ID should not omit because IsZero() returns false",
135135
},
136136
{
137137
name: "non-zero ID",
138-
typeID: typeid.Must(typeid.Parse("prefix_01h455vb4pex5vsknk084sn02q")),
138+
typeID: typeid.MustParse("prefix_01h455vb4pex5vsknk084sn02q"),
139139
expectedWithout: `{"id":"prefix_01h455vb4pex5vsknk084sn02q"}`,
140140
expectedWith: `{"id":"prefix_01h455vb4pex5vsknk084sn02q"}`,
141141
description: "non-zero ID should always be included",

typeid/typeid-go/examples_test.go

Lines changed: 7 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,7 @@
11
package typeid_test
22

33
import (
4+
"database/sql"
45
"encoding/json"
56
"fmt"
67

@@ -47,7 +48,7 @@ func ExampleTypeID_MarshalText() {
4748

4849
// Create a product with TypeID
4950
product := Product{
50-
ID: typeid.Must(typeid.Parse("product_00041061050r3gg28a1c60t3gf")),
51+
ID: typeid.MustParse("product_00041061050r3gg28a1c60t3gf"),
5152
Name: "Widget",
5253
Price: 29.99,
5354
}
@@ -124,9 +125,10 @@ func ExampleTypeID_Scan() {
124125
// Retrieved user user_00041061050r3gg28a1c60t3gf from database
125126
}
126127

127-
// ExampleNullableID demonstrates using NullableID for nullable database columns
128-
func ExampleNullableID() {
129-
var managerID typeid.NullableID
128+
// Example_nullableColumns demonstrates using sql.Null[TypeID] for nullable database columns.
129+
// This is the recommended approach for handling nullable TypeID columns in Go applications.
130+
func Example_nullableColumns() {
131+
var managerID sql.Null[typeid.TypeID]
130132

131133
// Scan NULL value from database
132134
err := managerID.Scan(nil)
@@ -141,7 +143,7 @@ func ExampleNullableID() {
141143
panic(err)
142144
}
143145
fmt.Printf("Is valid: %v\n", managerID.Valid)
144-
fmt.Printf("Manager: %s\n", managerID.TypeID.String())
146+
fmt.Printf("Manager: %s\n", managerID.V.String())
145147

146148
// Output:
147149
// Is valid: false

typeid/typeid-go/shared_test.go

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,12 @@
1+
package typeid
2+
3+
// MustParse returns a TypeID if the error is nil, otherwise panics.
4+
// Used in tests to create a TypeID in a single line as follows:
5+
// tid := MustParse("prefix_abc123")
6+
func MustParse(s string) TypeID {
7+
tid, err := Parse(s)
8+
if err != nil {
9+
panic(err)
10+
}
11+
return tid
12+
}

typeid/typeid-go/sql.go

Lines changed: 1 addition & 38 deletions
Original file line numberDiff line numberDiff line change
@@ -5,7 +5,7 @@ import (
55
"fmt"
66
)
77

8-
// TODO: decide if we want nullable (or just use pointers)
8+
// For nullable TypeID columns, use sql.Null[TypeID].
99

1010
// Scan implements the sql.Scanner interface so the TypeIDs can be read from
1111
// databases transparently. Currently database types that map to string are
@@ -35,40 +35,3 @@ func (tid *TypeID) Scan(src any) error {
3535
func (tid TypeID) Value() (driver.Value, error) {
3636
return tid.String(), nil
3737
}
38-
39-
// NullableID is wrapper for nullable columns.
40-
type NullableID struct {
41-
TypeID TypeID
42-
Valid bool
43-
}
44-
45-
func (n NullableID) Value() (driver.Value, error) {
46-
if !n.Valid {
47-
return nil, nil // SQL NULL
48-
}
49-
return n.TypeID.Value() // Delegate to TypeID
50-
}
51-
52-
func (n *NullableID) Scan(src any) error {
53-
if src == nil {
54-
n.TypeID, n.Valid = zeroID, false
55-
return nil
56-
}
57-
58-
// Empty string is invalid even for nullable columns - force explicit NULL usage
59-
if str, ok := src.(string); ok && str == "" {
60-
return &validationError{
61-
Message: "empty string is invalid TypeID",
62-
}
63-
}
64-
65-
// Try to scan the TypeID, only set Valid=true if successful
66-
err := n.TypeID.Scan(src)
67-
if err != nil {
68-
n.Valid = false
69-
return err
70-
}
71-
72-
n.Valid = true
73-
return nil
74-
}

typeid/typeid-go/sql_test.go

Lines changed: 28 additions & 29 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,7 @@
11
package typeid_test
22

33
import (
4+
"database/sql"
45
_ "embed"
56
"errors"
67
"testing"
@@ -30,7 +31,7 @@ func TestScanValid(t *testing.T) {
3031
err := scanned.Scan(td.Tid)
3132
assert.NoError(t, err)
3233

33-
expected := typeid.Must(typeid.Parse(td.Tid))
34+
expected := typeid.MustParse(td.Tid)
3435
assert.Equal(t, expected, scanned)
3536
assert.Equal(t, td.Tid, scanned.String())
3637
})
@@ -113,70 +114,71 @@ func TestValue(t *testing.T) {
113114

114115
for _, td := range testdata {
115116
t.Run(td.Name, func(t *testing.T) {
116-
tid := typeid.Must(typeid.Parse(td.Tid))
117+
tid := typeid.MustParse(td.Tid)
117118
actual, err := tid.Value()
118119
assert.NoError(t, err)
119120
assert.Equal(t, td.Tid, actual)
120121
})
121122
}
122123
}
123124

124-
func TestNullableIDScanValid(t *testing.T) {
125+
// Test sql.Null[TypeID] to verify it works for nullable database columns
126+
func TestSQLNullScanValid(t *testing.T) {
125127
var testdata []ValidExample
126128
err := yaml.Unmarshal(validSQLYML, &testdata)
127129
require.NoError(t, err)
128130

129131
for _, td := range testdata {
130132
t.Run(td.Name, func(t *testing.T) {
131-
// Test NullableID.Scan with valid TypeID strings
132-
var scanned typeid.NullableID
133+
// Test sql.Null[TypeID].Scan with valid TypeID strings
134+
var scanned sql.Null[typeid.TypeID]
133135
err := scanned.Scan(td.Tid)
134136
assert.NoError(t, err)
135137

136-
expected := typeid.Must(typeid.Parse(td.Tid))
137-
assert.True(t, scanned.Valid, "NullableID should be valid for valid typeid")
138-
assert.Equal(t, expected, scanned.TypeID)
139-
assert.Equal(t, td.Tid, scanned.TypeID.String())
138+
expected := typeid.MustParse(td.Tid)
139+
assert.True(t, scanned.Valid, "sql.Null[TypeID] should be valid for valid typeid")
140+
assert.Equal(t, expected, scanned.V)
141+
assert.Equal(t, td.Tid, scanned.V.String())
140142
})
141143
}
142144
}
143145

144-
func TestNullableIDScanSpecialCases(t *testing.T) {
146+
func TestSQLNullScanSpecialCases(t *testing.T) {
145147
testdata := []struct {
146148
name string
147149
input any
148-
expected typeid.NullableID
150+
expected sql.Null[typeid.TypeID]
149151
expectError bool
150152
}{
151-
{"nil", nil, typeid.NullableID{Valid: false}, false},
152-
{"empty string", "", typeid.NullableID{}, true},
153+
{"nil", nil, sql.Null[typeid.TypeID]{Valid: false}, false},
154+
{"empty string", "", sql.Null[typeid.TypeID]{}, true},
153155
}
154156

155157
for _, td := range testdata {
156158
t.Run(td.name, func(t *testing.T) {
157-
var scanned typeid.NullableID
159+
var scanned sql.Null[typeid.TypeID]
158160
err := scanned.Scan(td.input)
159161

160162
if td.expectError {
161163
assert.Error(t, err)
162-
assert.Contains(t, err.Error(), "empty string is invalid TypeID")
164+
assert.Contains(t, err.Error(), "cannot scan empty string into TypeID")
163165
// Verify that scan errors are validation errors
164166
assert.True(t, errors.Is(err, typeid.ErrValidation), "expected validation error")
165167
} else {
166168
assert.NoError(t, err)
167169
assert.Equal(t, td.expected.Valid, scanned.Valid)
168170
if td.expected.Valid {
169-
assert.Equal(t, td.expected.TypeID, scanned.TypeID)
171+
assert.Equal(t, td.expected.V, scanned.V)
170172
}
171173
}
172174
})
173175
}
174176
}
175177

176-
func TestNullableIDValue(t *testing.T) {
178+
func TestSQLNullValue(t *testing.T) {
177179
// Test the invalid case (Valid: false)
178180
t.Run("invalid", func(t *testing.T) {
179-
invalid := typeid.NullableID{Valid: false}
181+
invalid := sql.Null[typeid.TypeID]{Valid: false}
180182
actual, err := invalid.Value()
181183
assert.NoError(t, err)
182184
assert.Equal(t, nil, actual)
@@ -189,32 +191,31 @@ func TestNullableIDValue(t *testing.T) {
189191

190192
for _, td := range testdata {
191193
t.Run(td.Name, func(t *testing.T) {
192-
tid := typeid.Must(typeid.Parse(td.Tid))
193-
nullable := typeid.NullableID{TypeID: tid, Valid: true}
194+
tid := typeid.MustParse(td.Tid)
195+
nullable := sql.Null[typeid.TypeID]{V: tid, Valid: true}
194196
actual, err := nullable.Value()
195197
assert.NoError(t, err)
196198
assert.Equal(t, td.Tid, actual)
197199
})
198200
}
199201
}
200202

201-
func TestNullableIDScanInvalid(t *testing.T) {
203+
func TestSQLNullScanInvalid(t *testing.T) {
202204
var testdata []InvalidExample
203205
err := yaml.Unmarshal(invalidSQLYML, &testdata)
204206
require.NoError(t, err)
205207

206208
for _, td := range testdata {
207209
t.Run(td.Name, func(t *testing.T) {
208-
// Test NullableID.Scan with invalid TypeID strings
209-
var scanned typeid.NullableID
210+
// Test sql.Null[TypeID].Scan with invalid TypeID strings
211+
var scanned sql.Null[typeid.TypeID]
210212
err := scanned.Scan(td.Tid)
211-
assert.Error(t, err, "NullableID.Scan should fail for invalid typeid: %s", td.Tid)
212-
assert.False(t, scanned.Valid, "NullableID should not be valid after scan error")
213+
assert.Error(t, err, "sql.Null[TypeID].Scan should fail for invalid typeid: %s", td.Tid)
213214
})
214215
}
215216
}
216217

217-
func TestNullableIDScanUnsupportedType(t *testing.T) {
218+
func TestSQLNullScanUnsupportedType(t *testing.T) {
218219
testdata := []struct {
219220
name string
220221
input any
@@ -233,12 +234,10 @@ func TestNullableIDScanUnsupportedType(t *testing.T) {
233234

234235
for _, td := range testdata {
235236
t.Run(td.name, func(t *testing.T) {
236-
var scanned typeid.NullableID
237+
var scanned sql.Null[typeid.TypeID]
237238
err := scanned.Scan(td.input)
238239
assert.Error(t, err)
239240
assert.Contains(t, err.Error(), "unsupported scan type")
240-
assert.False(t, scanned.Valid, "NullableID should not be valid after scan error")
241-
// Verify that scan errors are validation errors
242241
assert.True(t, errors.Is(err, typeid.ErrValidation), "expected validation error")
243242
})
244243
}

typeid/typeid-go/typeid.go

Lines changed: 0 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -98,13 +98,3 @@ func (tid TypeID) HasSuffix() bool {
9898
func (tid TypeID) IsZero() bool {
9999
return tid.value == ""
100100
}
101-
102-
// Must returns a TypeID if the error is nil, otherwise panics.
103-
// Often used with Parse() to create a TypeID in a single line as follows:
104-
// tid := Must(Parse("prefix_abc123"))
105-
func Must(tid TypeID, err error) TypeID {
106-
if err != nil {
107-
panic(err)
108-
}
109-
return tid
110-
}

0 commit comments

Comments
 (0)