Skip to content

Commit c10fd97

Browse files
authored
structaccess.Get: return NotFoundError for nil case (#4209)
Follow up to #4204 NotFoundError is meant for cases where path is correct for a given type but value is not there. nil struct is such case. This also simplifies test cases. We don't need hasTypeError flag anymore, because notFoundError already implies hasTypeError=false
1 parent a0aa88c commit c10fd97

File tree

2 files changed

+26
-32
lines changed

2 files changed

+26
-32
lines changed

libs/structs/structaccess/get.go

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -53,7 +53,9 @@ func getValue(v any, path *structpath.PathNode) (reflect.Value, error) {
5353
cur, ok = deref(cur)
5454
if !ok {
5555
// cannot proceed further due to nil encountered at current location
56-
return reflect.Value{}, fmt.Errorf("%s: cannot access nil value", node.Parent().String())
56+
// There could be 2 cases: the type is correct but value is not found due to nil, in this case NotFoundError is 100% correct.
57+
// It could also be that path up to nil is correct, but not after. We don't know because we stop there. In this case NotFoundError refers to path up to nil.
58+
return reflect.Value{}, &NotFoundError{node.Parent().String() + ": cannot access nil value"}
5759
}
5860

5961
if idx, isIndex := node.Index(); isIndex {

libs/structs/structaccess/get_test.go

Lines changed: 23 additions & 31 deletions
Original file line numberDiff line numberDiff line change
@@ -11,13 +11,12 @@ import (
1111

1212
// unexported global test case type
1313
type testCase struct {
14-
name string
15-
path string
16-
want any
17-
wantSelf bool
18-
errFmt string
19-
notFound string // if set, expect NotFoundError with this message
20-
typeHasPath bool
14+
name string
15+
path string
16+
want any
17+
wantSelf bool
18+
errFmt string
19+
notFound string // if set, expect NotFoundError with this message
2120
}
2221

2322
func testGet(t *testing.T, obj any, path string, want any) {
@@ -197,10 +196,9 @@ func runCommonTests(t *testing.T, obj any) {
197196
errFmt: "connection[0]: cannot index struct",
198197
},
199198
{
200-
name: "out of range index",
201-
path: "items[5]",
202-
notFound: "items[5]: index out of range, length is 2",
203-
typeHasPath: true,
199+
name: "out of range index",
200+
path: "items[5]",
201+
notFound: "items[5]: index out of range, length is 2",
204202
},
205203
{
206204
name: "no json tag field should not be accessible",
@@ -213,21 +211,19 @@ func runCommonTests(t *testing.T, obj any) {
213211
errFmt: "items.id: cannot access key \"id\" on slice",
214212
},
215213
{
216-
name: "nil pointer access",
217-
path: "connection_not_set.id",
218-
errFmt: "connection_not_set: cannot access nil value",
219-
typeHasPath: true,
214+
name: "nil pointer access",
215+
path: "connection_not_set.id",
216+
notFound: "connection_not_set: cannot access nil value",
220217
},
221218
{
222219
name: "map non-string key type",
223220
path: "map_int.any",
224221
errFmt: "map_int.any: map key must be string, got int",
225222
},
226223
{
227-
name: "map missing key",
228-
path: "labels.missing",
229-
notFound: "labels.missing: key \"missing\" not found in map",
230-
typeHasPath: true,
224+
name: "map missing key",
225+
path: "labels.missing",
226+
notFound: "labels.missing: key \"missing\" not found in map",
231227
},
232228
{
233229
name: "json dash ignored",
@@ -247,33 +243,29 @@ func runCommonTests(t *testing.T, obj any) {
247243
want: "first",
248244
},
249245
{
250-
name: "key-value no match",
251-
path: "items[id='missing']",
252-
notFound: "items[id='missing']: no element found with id=\"missing\"",
253-
typeHasPath: true,
246+
name: "key-value no match",
247+
path: "items[id='missing']",
248+
notFound: "items[id='missing']: no element found with id=\"missing\"",
254249
},
255250
{
256251
name: "key-value on non-slice",
257252
path: "connection[id='abc']",
258253
errFmt: "connection[id='abc']: cannot use key-value syntax on struct",
259254
},
260255
{
261-
name: "key-value field not found",
262-
path: "items[missing='value']",
263-
notFound: "items[missing='value']: no element found with missing=\"value\"",
264-
typeHasPath: true,
256+
name: "key-value field not found",
257+
path: "items[missing='value']",
258+
notFound: "items[missing='value']: no element found with missing=\"value\"",
265259
},
266260
}
267261

268262
for _, tt := range tests {
269263
t.Run(tt.name, func(t *testing.T) {
270264
hasPathError := ValidateByString(reflect.TypeOf(obj), tt.path)
271-
if tt.errFmt == "" && tt.notFound == "" || tt.typeHasPath {
265+
if tt.errFmt == "" {
272266
require.NoError(t, hasPathError)
273-
} else if tt.errFmt != "" {
267+
} else {
274268
require.EqualError(t, hasPathError, tt.errFmt)
275-
} else if tt.notFound != "" {
276-
require.EqualError(t, hasPathError, tt.notFound)
277269
}
278270

279271
got, err := GetByString(obj, tt.path)

0 commit comments

Comments
 (0)