Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix: Correctly handle serialisation of nil field values #1872

Merged
merged 1 commit into from
Sep 14, 2023
Merged

fix: Correctly handle serialisation of nil field values #1872

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 2 additions & 6 deletions client/document.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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
}
Expand Down Expand Up @@ -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) {
Expand Down
3 changes: 2 additions & 1 deletion client/document_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

question: Just to confirm the following is what we would expect if we tested it right?

assert.Equal(t, doc.values[doc.fields["Address"]].IsDelete(), false)

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

yes


//subdoc fields
// subDoc := doc.values[doc.fields["Address"]].Value().(*Document)
Comment on lines 138 to 139
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nitpick: suggest removal

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

don't further tempt me to remove this file completely :)

Expand Down
52 changes: 0 additions & 52 deletions tests/integration/mutation/update/field_kinds/array_bool_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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: `
Expand Down Expand Up @@ -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",
Expand Down
52 changes: 0 additions & 52 deletions tests/integration/mutation/update/field_kinds/array_float_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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: `
Expand Down Expand Up @@ -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",
Expand Down
52 changes: 0 additions & 52 deletions tests/integration/mutation/update/field_kinds/array_int_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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: `
Expand Down Expand Up @@ -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",
Expand Down
52 changes: 0 additions & 52 deletions tests/integration/mutation/update/field_kinds/array_string_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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: `
Expand Down Expand Up @@ -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",
Expand Down