Skip to content

Commit

Permalink
Fix issue
Browse files Browse the repository at this point in the history
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.
  • Loading branch information
AndrewSisley committed Sep 26, 2024
1 parent 2824a1c commit 5d56f61
Show file tree
Hide file tree
Showing 3 changed files with 23 additions and 8 deletions.
4 changes: 3 additions & 1 deletion internal/db/definition_validation.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)
}
}
Expand Down
25 changes: 19 additions & 6 deletions internal/db/schema.go
Original file line number Diff line number Diff line change
Expand Up @@ -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]

Expand Down Expand Up @@ -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
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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{
Expand All @@ -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.",
},
},
}
Expand Down

0 comments on commit 5d56f61

Please sign in to comment.