From a32e689d3ab7a7802f457743f2ab84cbff414470 Mon Sep 17 00:00:00 2001 From: Andrew Sisley Date: Wed, 13 Sep 2023 13:15:16 -0400 Subject: [PATCH] Handle serialisation of nil field values Nil json values were being treated as delete operations. Setting a value to nil is not the same as deleting it. --- client/document.go | 8 +-- client/document_test.go | 3 +- .../update/field_kinds/array_bool_test.go | 52 ------------------- .../update/field_kinds/array_float_test.go | 52 ------------------- .../update/field_kinds/array_int_test.go | 52 ------------------- .../update/field_kinds/array_string_test.go | 52 ------------------- 6 files changed, 4 insertions(+), 215 deletions(-) diff --git a/client/document.go b/client/document.go index a2860c371a..c48ccfce88 100644 --- a/client/document.go +++ b/client/document.go @@ -209,11 +209,7 @@ func (doc *Document) SetWithJSON(patch []byte) error { } for k, v := range patchObj { - if v == nil { - err = doc.Delete(k) - } else { - err = doc.Set(k, v) - } + err = doc.Set(k, v) if err != nil { return err } @@ -273,7 +269,7 @@ func (doc *Document) setObject(t CType, field string, val *Document) error { // @todo: Update with document schemas func (doc *Document) setAndParseType(field string, value any) error { if value == nil { - return nil + return doc.setCBOR(LWW_REGISTER, field, value) } switch val := value.(type) { diff --git a/client/document_test.go b/client/document_test.go index c2e9b406c0..9073373cd3 100644 --- a/client/document_test.go +++ b/client/document_test.go @@ -132,7 +132,8 @@ func TestSetWithJSON(t *testing.T) { assert.Equal(t, doc.values[doc.fields["Name"]].IsDocument(), false) assert.Equal(t, doc.values[doc.fields["Age"]].Value(), int64(27)) assert.Equal(t, doc.values[doc.fields["Age"]].IsDocument(), false) - assert.Equal(t, doc.values[doc.fields["Address"]].IsDelete(), true) + assert.Equal(t, doc.values[doc.fields["Address"]].Value(), nil) + assert.Equal(t, doc.values[doc.fields["Address"]].IsDocument(), false) //subdoc fields // subDoc := doc.values[doc.fields["Address"]].Value().(*Document) diff --git a/tests/integration/mutation/update/field_kinds/array_bool_test.go b/tests/integration/mutation/update/field_kinds/array_bool_test.go index 831c47c6cb..b1a5500242 100644 --- a/tests/integration/mutation/update/field_kinds/array_bool_test.go +++ b/tests/integration/mutation/update/field_kinds/array_bool_test.go @@ -13,19 +13,12 @@ package field_kinds import ( "testing" - "github.com/sourcenetwork/immutable" - testUtils "github.com/sourcenetwork/defradb/tests/integration" ) func TestMutationUpdate_WithArrayOfBooleansToNil(t *testing.T) { test := testUtils.TestCase{ Description: "Simple update mutation with boolean array, replace with nil", - // This restriction should be removed when we can, it is here because of - // https://github.com/sourcenetwork/defradb/issues/1842 - SupportedMutationTypes: immutable.Some([]testUtils.MutationType{ - testUtils.GQLRequestMutationType, - }), Actions: []any{ testUtils.SchemaUpdate{ Schema: ` @@ -66,51 +59,6 @@ func TestMutationUpdate_WithArrayOfBooleansToNil(t *testing.T) { testUtils.ExecuteTestCase(t, test) } -func TestMutationUpdate_WithArrayOfBooleansToNil_Errors(t *testing.T) { - test := testUtils.TestCase{ - Description: "Simple update mutation with boolean array, replace with nil", - // This is a bug, this test should be removed in - // https://github.com/sourcenetwork/defradb/issues/1842 - SupportedMutationTypes: immutable.Some([]testUtils.MutationType{ - testUtils.CollectionNamedMutationType, - testUtils.CollectionSaveMutationType, - }), - Actions: []any{ - testUtils.SchemaUpdate{ - Schema: ` - type Users { - name: String - likedIndexes: [Boolean!] - } - `, - }, - testUtils.CreateDoc{ - Doc: `{ - "name": "John", - "likedIndexes": [true, true, false, true] - }`, - }, - testUtils.UpdateDoc{ - Doc: `{ - "likedIndexes": null - }`, - }, - testUtils.Request{ - Request: ` - query { - Users { - likedIndexes - } - } - `, - ExpectedError: "EOF", - }, - }, - } - - testUtils.ExecuteTestCase(t, test) -} - func TestMutationUpdate_WithArrayOfBooleansToEmpty(t *testing.T) { test := testUtils.TestCase{ Description: "Simple update mutation with boolean array, replace with empty", diff --git a/tests/integration/mutation/update/field_kinds/array_float_test.go b/tests/integration/mutation/update/field_kinds/array_float_test.go index 255d5f8538..a1806eadf0 100644 --- a/tests/integration/mutation/update/field_kinds/array_float_test.go +++ b/tests/integration/mutation/update/field_kinds/array_float_test.go @@ -13,19 +13,12 @@ package field_kinds import ( "testing" - "github.com/sourcenetwork/immutable" - testUtils "github.com/sourcenetwork/defradb/tests/integration" ) func TestMutationUpdate_WithArrayOfFloatsToNil(t *testing.T) { test := testUtils.TestCase{ Description: "Simple update mutation with float array, replace with nil", - // This restriction should be removed when we can, it is here because of - // https://github.com/sourcenetwork/defradb/issues/1842 - SupportedMutationTypes: immutable.Some([]testUtils.MutationType{ - testUtils.GQLRequestMutationType, - }), Actions: []any{ testUtils.SchemaUpdate{ Schema: ` @@ -66,51 +59,6 @@ func TestMutationUpdate_WithArrayOfFloatsToNil(t *testing.T) { testUtils.ExecuteTestCase(t, test) } -func TestMutationUpdate_WithArrayOfFloatsToNil_Errors(t *testing.T) { - test := testUtils.TestCase{ - Description: "Simple update mutation with float array, replace with nil", - // This is a bug, this test should be removed in - // https://github.com/sourcenetwork/defradb/issues/1842 - SupportedMutationTypes: immutable.Some([]testUtils.MutationType{ - testUtils.CollectionNamedMutationType, - testUtils.CollectionSaveMutationType, - }), - Actions: []any{ - testUtils.SchemaUpdate{ - Schema: ` - type Users { - name: String - favouriteFloats: [Float!] - } - `, - }, - testUtils.CreateDoc{ - Doc: `{ - "name": "John", - "favouriteFloats": [3.1425, 0.00000000001, 10] - }`, - }, - testUtils.UpdateDoc{ - Doc: `{ - "favouriteFloats": null - }`, - }, - testUtils.Request{ - Request: ` - query { - Users { - favouriteFloats - } - } - `, - ExpectedError: "EOF", - }, - }, - } - - testUtils.ExecuteTestCase(t, test) -} - func TestMutationUpdate_WithArrayOfFloatsToEmpty(t *testing.T) { test := testUtils.TestCase{ Description: "Simple update mutation with float array, replace with empty", diff --git a/tests/integration/mutation/update/field_kinds/array_int_test.go b/tests/integration/mutation/update/field_kinds/array_int_test.go index 7278b15e23..63ecfc0969 100644 --- a/tests/integration/mutation/update/field_kinds/array_int_test.go +++ b/tests/integration/mutation/update/field_kinds/array_int_test.go @@ -13,19 +13,12 @@ package field_kinds import ( "testing" - "github.com/sourcenetwork/immutable" - testUtils "github.com/sourcenetwork/defradb/tests/integration" ) func TestMutationUpdate_WithArrayOfIntsToNil(t *testing.T) { test := testUtils.TestCase{ Description: "Simple update mutation with integer array, replace with nil", - // This restriction should be removed when we can, it is here because of - // https://github.com/sourcenetwork/defradb/issues/1842 - SupportedMutationTypes: immutable.Some([]testUtils.MutationType{ - testUtils.GQLRequestMutationType, - }), Actions: []any{ testUtils.SchemaUpdate{ Schema: ` @@ -66,51 +59,6 @@ func TestMutationUpdate_WithArrayOfIntsToNil(t *testing.T) { testUtils.ExecuteTestCase(t, test) } -func TestMutationUpdate_WithArrayOfIntsToNil_Errors(t *testing.T) { - test := testUtils.TestCase{ - Description: "Simple update mutation with integer array, replace with nil", - // This is a bug, this test should be removed in - // https://github.com/sourcenetwork/defradb/issues/1842 - SupportedMutationTypes: immutable.Some([]testUtils.MutationType{ - testUtils.CollectionNamedMutationType, - testUtils.CollectionSaveMutationType, - }), - Actions: []any{ - testUtils.SchemaUpdate{ - Schema: ` - type Users { - name: String - favouriteIntegers: [Int!] - } - `, - }, - testUtils.CreateDoc{ - Doc: `{ - "name": "John", - "favouriteIntegers": [1, 2, 3, 5, 8] - }`, - }, - testUtils.UpdateDoc{ - Doc: `{ - "favouriteIntegers": null - }`, - }, - testUtils.Request{ - Request: ` - query { - Users { - favouriteIntegers - } - } - `, - ExpectedError: "EOF", - }, - }, - } - - testUtils.ExecuteTestCase(t, test) -} - func TestMutationUpdate_WithArrayOfIntsToEmpty(t *testing.T) { test := testUtils.TestCase{ Description: "Simple update mutation with integer array, replace with empty", diff --git a/tests/integration/mutation/update/field_kinds/array_string_test.go b/tests/integration/mutation/update/field_kinds/array_string_test.go index b15003072a..3481fb7912 100644 --- a/tests/integration/mutation/update/field_kinds/array_string_test.go +++ b/tests/integration/mutation/update/field_kinds/array_string_test.go @@ -13,19 +13,12 @@ package field_kinds import ( "testing" - "github.com/sourcenetwork/immutable" - testUtils "github.com/sourcenetwork/defradb/tests/integration" ) func TestMutationUpdate_WithArrayOfStringsToNil(t *testing.T) { test := testUtils.TestCase{ Description: "Simple update mutation with string array, replace with nil", - // This restriction should be removed when we can, it is here because of - // https://github.com/sourcenetwork/defradb/issues/1842 - SupportedMutationTypes: immutable.Some([]testUtils.MutationType{ - testUtils.GQLRequestMutationType, - }), Actions: []any{ testUtils.SchemaUpdate{ Schema: ` @@ -66,51 +59,6 @@ func TestMutationUpdate_WithArrayOfStringsToNil(t *testing.T) { testUtils.ExecuteTestCase(t, test) } -func TestMutationUpdate_WithArrayOfStringsToNil_Errors(t *testing.T) { - test := testUtils.TestCase{ - Description: "Simple update mutation with string array, replace with nil", - // This is a bug, this test should be removed in - // https://github.com/sourcenetwork/defradb/issues/1842 - SupportedMutationTypes: immutable.Some([]testUtils.MutationType{ - testUtils.CollectionNamedMutationType, - testUtils.CollectionSaveMutationType, - }), - Actions: []any{ - testUtils.SchemaUpdate{ - Schema: ` - type Users { - name: String - preferredStrings: [String!] - } - `, - }, - testUtils.CreateDoc{ - Doc: `{ - "name": "John", - "preferredStrings": ["", "the previous", "the first", "empty string"] - }`, - }, - testUtils.UpdateDoc{ - Doc: `{ - "preferredStrings": null - }`, - }, - testUtils.Request{ - Request: ` - query { - Users { - preferredStrings - } - } - `, - ExpectedError: "EOF", - }, - }, - } - - testUtils.ExecuteTestCase(t, test) -} - func TestMutationUpdate_WithArrayOfStringsToEmpty(t *testing.T) { test := testUtils.TestCase{ Description: "Simple update mutation with string array, replace with empty",