From 84021fd98f25f3b3957f0154a39b70c7b9c934a0 Mon Sep 17 00:00:00 2001 From: Ryan McKinley Date: Wed, 11 Sep 2024 09:32:33 +0300 Subject: [PATCH] JSON: Ensure all fields types work as expected when they are the first field in a frame (#1080) --- data/field_type.go | 2 ++ data/frame_json.go | 60 ++++++++++++++++++++++++++++++----- data/frame_json_test.go | 70 +++++++++++++++++++++++++++++++++++++++++ 3 files changed, 125 insertions(+), 7 deletions(-) diff --git a/data/field_type.go b/data/field_type.go index fdec0503a..3304071a5 100644 --- a/data/field_type.go +++ b/data/field_type.go @@ -81,7 +81,9 @@ const ( // FieldTypeJSON indicates the underlying primitive is a []json.RawMessage. FieldTypeJSON + // FieldTypeNullableJSON indicates the underlying primitive is a []*json.RawMessage. + // Deprecated: Use FieldTypeJSON, an array can be null anyway FieldTypeNullableJSON // FieldTypeEnum indicates the underlying primitive is a []data.EnumItemIndex, with field mapping metadata diff --git a/data/frame_json.go b/data/frame_json.go index f5403938a..6842c8e2e 100644 --- a/data/frame_json.go +++ b/data/frame_json.go @@ -420,6 +420,8 @@ func int64FromJSON(v interface{}) (int64, error) { return 0, fmt.Errorf("unable to convert int64 in json [%T]", v) } +// in this path, we do not yet know the length and must discover it from the array +// nolint:gocyclo func jsonValuesToVector(iter *jsoniter.Iterator, ft FieldType) (vector, error) { itere := sdkjsoniter.NewIterator(iter) // we handle Uint64 differently because the regular method for unmarshalling to []any does not work for uint64 correctly @@ -435,6 +437,7 @@ func jsonValuesToVector(iter *jsoniter.Iterator, ft FieldType) (vector, error) { return nil, err } return newUint64VectorWithValues(u), nil + case FieldTypeNullableUint64: parseUint64 := func(s string) (*uint64, error) { u, err := strconv.ParseUint(s, 0, 64) @@ -448,9 +451,57 @@ func jsonValuesToVector(iter *jsoniter.Iterator, ft FieldType) (vector, error) { return nil, err } return newNullableUint64VectorWithValues(u), nil + + case FieldTypeInt64: + vals := newInt64Vector(0) + for iter.ReadArray() { + v := iter.ReadInt64() + vals.Append(v) + } + return vals, nil + + case FieldTypeNullableInt64: + vals := newNullableInt64Vector(0) + for iter.ReadArray() { + t := iter.WhatIsNext() + if t == sdkjsoniter.NilValue { + iter.ReadNil() + vals.Append(nil) + } else { + v := iter.ReadInt64() + vals.Append(&v) + } + } + return vals, nil + + case FieldTypeJSON, FieldTypeNullableJSON: + vals := newJsonRawMessageVector(0) + for iter.ReadArray() { + var v json.RawMessage + t := iter.WhatIsNext() + if t == sdkjsoniter.NilValue { + iter.ReadNil() + } else { + iter.ReadVal(&v) + } + vals.Append(v) + } + + // Convert this to the pointer flavor + if ft == FieldTypeNullableJSON { + size := vals.Len() + nullable := newNullableJsonRawMessageVector(size) + for i := 0; i < size; i++ { + v := vals.At(i).(json.RawMessage) + nullable.Set(i, &v) + } + return nullable, nil + } + + return vals, nil } - // if it's not uint64 field, handle the array the old way + // if it's not uint64 field, handle the array the old way convert := func(v interface{}) (interface{}, error) { return v, nil } @@ -506,11 +557,6 @@ func jsonValuesToVector(iter *jsoniter.Iterator, ft FieldType) (vector, error) { return int32(iV), err } - case FieldTypeInt64: - convert = func(v interface{}) (interface{}, error) { - return int64FromJSON(v) - } - case FieldTypeFloat32: convert = func(v interface{}) (interface{}, error) { fV, err := float64FromJSON(v) @@ -1293,7 +1339,7 @@ func readTimeVectorJSON(iter *jsoniter.Iterator, nullable bool, size int) (vecto } else { ms := iter.ReadInt64() - tv := time.Unix(ms/int64(1e+3), (ms%int64(1e+3))*int64(1e+6)) + tv := time.Unix(ms/int64(1e+3), (ms%int64(1e+3))*int64(1e+6)).UTC() arr.SetConcrete(i, tv) } } diff --git a/data/frame_json_test.go b/data/frame_json_test.go index 6ff2978c3..9deee2850 100644 --- a/data/frame_json_test.go +++ b/data/frame_json_test.go @@ -65,6 +65,76 @@ func TestGoldenFrameJSON(t *testing.T) { if diff := cmp.Diff(f, out, data.FrameTestCompareOptions()...); diff != "" { t.Errorf("Result mismatch (-want +got):\n%s", diff) } + + // Now try each field as the only field in the frame + for idx, field := range f.Fields { + expected := out.Fields[idx] + size := expected.Len() + + frame := data.NewFrame("test-"+field.Name, field) + b, err := data.FrameToJSON(frame, data.IncludeAll) // json.Marshal(f2) + require.NoError(t, err) + + f2 := &data.Frame{} + err = json.Unmarshal(b, f2) + require.NoError(t, err) + + found := f2.Fields[0] + require.Equal(t, size, found.Len()) + for i := 0; i < size; i++ { + // Lots of NaN flavors that are really the same + if expected.Type().Numeric() { + fA, _ := expected.NullableFloatAt(i) + fB, _ := found.NullableFloatAt(i) + if fA != nil && fB != nil { + if math.IsNaN(*fA) && math.IsNaN(*fB) { + continue // many flavors of NaN! + } + } + } + + switch expected.Type() { + case data.FieldTypeJSON, data.FieldTypeNullableJSON: + msgA, okA := expected.ConcreteAt(i) + msgB, okB := found.ConcreteAt(i) + if okA && okB { + a := string(msgA.(json.RawMessage)) + b := string(msgB.(json.RawMessage)) + require.JSONEq(t, a, b, field.Name) // pretty print does not matter + continue + } else if expected.Type() == data.FieldTypeNullableJSON { + // The pointer conversions get too weird for this to be reasonable + continue + } + } + + assert.EqualValues(t, expected.At(i), found.At(i), "field: %s, row: %d", field.Name, i) + } + } +} + +func TestMarshalFieldTypeJSON(t *testing.T) { + testJSON := func(ft data.FieldType) { + msg := json.RawMessage([]byte(`{"l1":"v1"}`)) + f1 := data.NewFieldFromFieldType(ft, 1) + f1.SetConcrete(0, msg) + + f := data.NewFrame("frame", f1) + + fbytes, err := json.Marshal(f) + require.NoError(t, err) + + // fmt.Println("frame in json:", string(fbytes)) + + f2 := &data.Frame{} + err = json.Unmarshal(fbytes, f2) + require.NoError(t, err) + val, ok := f2.Fields[0].ConcreteAt(0) + require.True(t, ok) + require.Equal(t, msg, val) + } + testJSON(data.FieldTypeJSON) + testJSON(data.FieldTypeNullableJSON) // this version is silly } type simpleTestObj struct {