From c0dbcda6f106b9c60c0b3e3a48f06e77cc66524e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Sergen=20Yal=C3=A7=C4=B1n?= Date: Tue, 6 Feb 2024 19:10:54 +0300 Subject: [PATCH 1/3] Add a nil case to the getValueFromInterface function MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Sergen Yalçın (cherry picked from commit 5cd11c97cb15ff3500747c838c99cd17d93d306b) --- pkg/fieldpath/paved.go | 27 ++++++++++++++++----------- pkg/fieldpath/paved_test.go | 8 ++++++++ 2 files changed, 24 insertions(+), 11 deletions(-) diff --git a/pkg/fieldpath/paved.go b/pkg/fieldpath/paved.go index 908187dd5..3283e5676 100644 --- a/pkg/fieldpath/paved.go +++ b/pkg/fieldpath/paved.go @@ -112,7 +112,9 @@ func (p *Paved) getValue(s Segments) (any, error) { return getValueFromInterface(p.object, s) } -func getValueFromInterface(it any, s Segments) (any, error) { +func getValueFromInterface(it any, s Segments) (any, error) { //nolint:gocyclo // See note below. + // Although the complexity of the function may seem high, in fact the same + // operation is performed in different cases. for i, current := range s { final := i == len(s)-1 switch current.Type { @@ -129,18 +131,21 @@ func getValueFromInterface(it any, s Segments) (any, error) { } it = array[current.Index] case SegmentField: - object, ok := it.(map[string]any) - if !ok { + switch object := it.(type) { + case map[string]any: + v, ok := object[current.Field] + if !ok { + return nil, errNotFound{errors.Errorf("%s: no such field", s[:i+1])} + } + if final { + return v, nil + } + it = object[current.Field] + case nil: + return nil, errNotFound{errors.Errorf("field %q is not found in the path", s[:i])} + default: return nil, errors.Errorf("%s: not an object", s[:i]) } - v, ok := object[current.Field] - if !ok { - return nil, errNotFound{errors.Errorf("%s: no such field", s[:i+1])} - } - if final { - return v, nil - } - it = object[current.Field] } } diff --git a/pkg/fieldpath/paved_test.go b/pkg/fieldpath/paved_test.go index 91bc5e256..e98dad89d 100644 --- a/pkg/fieldpath/paved_test.go +++ b/pkg/fieldpath/paved_test.go @@ -161,6 +161,14 @@ func TestGetValue(t *testing.T) { err: errors.Wrap(errors.New("unexpected ']' at position 5"), "cannot parse path \"spec[]\""), }, }, + "NilValue": { + reason: "Requesting for an object that has nil value", + path: "spec.containers[*].name", + data: []byte(`{"spec":{"containers": null}}`), + want: want{ + err: errNotFound{errors.Errorf("field %q is not found in the path", "spec.containers")}, + }, + }, } for name, tc := range cases { From c3042fb18c6a5c06fd3a0f08415091fb595552c3 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Sergen=20Yal=C3=A7=C4=B1n?= Date: Wed, 7 Feb 2024 11:18:55 +0300 Subject: [PATCH 2/3] Change the error message MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Sergen Yalçın (cherry picked from commit b1cd25a73b12cedda47fdc895d049b1f923cb8a4) --- pkg/fieldpath/paved.go | 2 +- pkg/fieldpath/paved_test.go | 6 +++--- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/pkg/fieldpath/paved.go b/pkg/fieldpath/paved.go index 3283e5676..e6b974ffa 100644 --- a/pkg/fieldpath/paved.go +++ b/pkg/fieldpath/paved.go @@ -142,7 +142,7 @@ func getValueFromInterface(it any, s Segments) (any, error) { //nolint:gocyclo / } it = object[current.Field] case nil: - return nil, errNotFound{errors.Errorf("field %q is not found in the path", s[:i])} + return nil, errNotFound{errors.Errorf("path %q is not found in the paved object", s[:i])} default: return nil, errors.Errorf("%s: not an object", s[:i]) } diff --git a/pkg/fieldpath/paved_test.go b/pkg/fieldpath/paved_test.go index e98dad89d..bf768a74e 100644 --- a/pkg/fieldpath/paved_test.go +++ b/pkg/fieldpath/paved_test.go @@ -161,12 +161,12 @@ func TestGetValue(t *testing.T) { err: errors.Wrap(errors.New("unexpected ']' at position 5"), "cannot parse path \"spec[]\""), }, }, - "NilValue": { - reason: "Requesting for an object that has nil value", + "NilParent": { + reason: "Request for a path with a nil parent value", path: "spec.containers[*].name", data: []byte(`{"spec":{"containers": null}}`), want: want{ - err: errNotFound{errors.Errorf("field %q is not found in the path", "spec.containers")}, + err: errNotFound{errors.Errorf("path %q is not found in the paved object", "spec.containers")}, }, }, } From 94b26aa8efffd8e72f2b205ac8262c995ca8dae1 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Sergen=20Yal=C3=A7=C4=B1n?= Date: Tue, 13 Feb 2024 16:01:25 +0300 Subject: [PATCH 3/3] Change the error message with a more consistent with the other errors MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Sergen Yalçın (cherry picked from commit 195a02da319e4663c0949938d1cb33d88ab4912a) --- pkg/fieldpath/paved.go | 2 +- pkg/fieldpath/paved_test.go | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/pkg/fieldpath/paved.go b/pkg/fieldpath/paved.go index e6b974ffa..be3ab3581 100644 --- a/pkg/fieldpath/paved.go +++ b/pkg/fieldpath/paved.go @@ -142,7 +142,7 @@ func getValueFromInterface(it any, s Segments) (any, error) { //nolint:gocyclo / } it = object[current.Field] case nil: - return nil, errNotFound{errors.Errorf("path %q is not found in the paved object", s[:i])} + return nil, errNotFound{errors.Errorf("%s: expected map, got nil", s[:i])} default: return nil, errors.Errorf("%s: not an object", s[:i]) } diff --git a/pkg/fieldpath/paved_test.go b/pkg/fieldpath/paved_test.go index bf768a74e..e6781b37f 100644 --- a/pkg/fieldpath/paved_test.go +++ b/pkg/fieldpath/paved_test.go @@ -166,7 +166,7 @@ func TestGetValue(t *testing.T) { path: "spec.containers[*].name", data: []byte(`{"spec":{"containers": null}}`), want: want{ - err: errNotFound{errors.Errorf("path %q is not found in the paved object", "spec.containers")}, + err: errNotFound{errors.Errorf("%s: expected map, got nil", "spec.containers")}, }, }, }