From 5d56f615533b09ae476a6c6181d583b4aa0ed51e Mon Sep 17 00:00:00 2001 From: Andrew Sisley Date: Wed, 25 Sep 2024 14:22:20 -0400 Subject: [PATCH] Fix issue There were two problems, firstly `==` was used which always resulted in false as a property on the object was a pointer. Secondly related, but not mutated, objects were not included in the validation set making them appear to be deleted. --- internal/db/definition_validation.go | 4 ++- internal/db/schema.go | 25 ++++++++++++++----- .../updates/add/field/with_relation_test.go | 2 +- 3 files changed, 23 insertions(+), 8 deletions(-) diff --git a/internal/db/definition_validation.go b/internal/db/definition_validation.go index 47c222bf9c..2d178f1e07 100644 --- a/internal/db/definition_validation.go +++ b/internal/db/definition_validation.go @@ -818,7 +818,9 @@ func validateFieldNotMutated( for _, newField := range newSchema.Fields { oldField, exists := oldFieldsByName[newField.Name] - if exists && oldField != newField { + + // DeepEqual is temporary, as this validation is temporary + if exists && !reflect.DeepEqual(oldField, newField) { return NewErrCannotMutateField(newField.Name) } } diff --git a/internal/db/schema.go b/internal/db/schema.go index 391d5e515a..f1906feb54 100644 --- a/internal/db/schema.go +++ b/internal/db/schema.go @@ -376,11 +376,6 @@ func (db *db) updateSchema( return err } - oldDefs := make([]client.CollectionDefinition, 0, len(allExistingCols)) - for _, col := range allExistingCols { - oldDefs = append(oldDefs, col.Definition()) - } - for _, schema := range proposedDescriptionsByName { previousSchema := existingSchemaByName[schema.Name] @@ -498,7 +493,25 @@ func (db *db) updateSchema( return err } - err = db.validateSchemaUpdate(ctx, oldDefs, definitions) + oldDefs := make([]client.CollectionDefinition, 0, len(allExistingCols)) + for _, col := range allExistingCols { + oldDefs = append(oldDefs, col.Definition()) + } + + defNames := make(map[string]struct{}, len(definitions)) + for _, def := range definitions { + defNames[def.GetName()] = struct{}{} + } + + newDefs := make([]client.CollectionDefinition, 0, len(definitions)) + newDefs = append(newDefs, definitions...) + for _, existing := range allExistingCols { + if _, ok := defNames[existing.Definition().GetName()]; !ok { + newDefs = append(newDefs, existing.Definition()) + } + } + + err = db.validateSchemaUpdate(ctx, oldDefs, newDefs) if err != nil { return err } diff --git a/tests/integration/schema/updates/add/field/with_relation_test.go b/tests/integration/schema/updates/add/field/with_relation_test.go index b23729d48b..9adb5659ac 100644 --- a/tests/integration/schema/updates/add/field/with_relation_test.go +++ b/tests/integration/schema/updates/add/field/with_relation_test.go @@ -16,6 +16,7 @@ import ( testUtils "github.com/sourcenetwork/defradb/tests/integration" ) +// This test ensures that nearby relation fields are not failing validation during a schema patch. func TestSchemaUpdatesAddField_DoesNotAffectExistingRelation(t *testing.T) { test := testUtils.TestCase{ Actions: []any{ @@ -38,7 +39,6 @@ func TestSchemaUpdatesAddField_DoesNotAffectExistingRelation(t *testing.T) { { "op": "add", "path": "/Book/Fields/-", "value": {"Name": "rating", "Kind": 4} } ] `, - ExpectedError: "mutating an existing field is not supported.", }, }, }