From c4f84dcc843bdff1a5e01248b1edd231af0a5ef3 Mon Sep 17 00:00:00 2001 From: Andrew Sisley Date: Wed, 27 Sep 2023 16:07:24 -0400 Subject: [PATCH] Ensure other field is returned from GetRelation If the relationship references the same (host) collection, the function would in some circumstances return the known field. In collection.save, when updating via a secondary one-one field this leads to an infinite loop. --- client/descriptions.go | 14 ++++++++------ db/collection.go | 2 +- db/collection_update.go | 7 ++++++- planner/type_join.go | 13 +++++++++++-- .../field_kinds/one_to_one/with_self_ref_test.go | 3 --- 5 files changed, 26 insertions(+), 13 deletions(-) diff --git a/client/descriptions.go b/client/descriptions.go index 0b44f36b83..f25fc40772 100644 --- a/client/descriptions.go +++ b/client/descriptions.go @@ -53,12 +53,14 @@ func (col CollectionDescription) GetFieldByID(id FieldID) (FieldDescription, boo } // GetRelation returns the field that supports the relation of the given name. -func (col CollectionDescription) GetRelation(name string) (FieldDescription, bool) { - if !col.Schema.IsEmpty() { - for _, field := range col.Schema.Fields { - if field.RelationName == name { - return field, true - } +func (col CollectionDescription) GetFieldByRelation( + relationName string, + otherCollectionName string, + otherFieldName string, +) (FieldDescription, bool) { + for _, field := range col.Schema.Fields { + if field.RelationName == relationName && !(col.Name == otherCollectionName && otherFieldName == field.Name) { + return field, true } } return FieldDescription{}, false diff --git a/db/collection.go b/db/collection.go index cda5cbf584..f5dacccfb1 100644 --- a/db/collection.go +++ b/db/collection.go @@ -1062,7 +1062,7 @@ func (c *collection) save( if isSecondaryRelationID { primaryId := val.Value().(string) - err = c.patchPrimaryDoc(ctx, txn, relationFieldDescription, primaryKey.DocKey, primaryId) + err = c.patchPrimaryDoc(ctx, txn, c.Name(), relationFieldDescription, primaryKey.DocKey, primaryId) if err != nil { return cid.Undef, err } diff --git a/db/collection_update.go b/db/collection_update.go index 1a15482935..2e353dd0d3 100644 --- a/db/collection_update.go +++ b/db/collection_update.go @@ -350,6 +350,7 @@ func (c *collection) isSecondaryIDField(fieldDesc client.FieldDescription) (clie func (c *collection) patchPrimaryDoc( ctx context.Context, txn datastore.Txn, + secondaryCollectionName string, relationFieldDescription client.FieldDescription, docKey string, fieldValue string, @@ -365,7 +366,11 @@ func (c *collection) patchPrimaryDoc( } primaryCol = primaryCol.WithTxn(txn) - primaryField, ok := primaryCol.Description().GetRelation(relationFieldDescription.RelationName) + primaryField, ok := primaryCol.Description().GetFieldByRelation( + relationFieldDescription.RelationName, + secondaryCollectionName, + relationFieldDescription.Name, + ) if !ok { return client.NewErrFieldNotExist(relationFieldDescription.RelationName) } diff --git a/planner/type_join.go b/planner/type_join.go index f37437089e..ee771b01fc 100644 --- a/planner/type_join.go +++ b/planner/type_join.go @@ -259,7 +259,11 @@ func (p *Planner) makeTypeJoinOne( return nil, err } - subTypeField, subTypeFieldNameFound := subTypeCollectionDesc.GetRelation(subTypeFieldDesc.RelationName) + subTypeField, subTypeFieldNameFound := subTypeCollectionDesc.GetFieldByRelation( + subTypeFieldDesc.RelationName, + parent.sourceInfo.collectionDescription.Name, + subTypeFieldDesc.Name, + ) if !subTypeFieldNameFound { return nil, client.NewErrFieldNotExist(subTypeFieldDesc.RelationName) } @@ -481,7 +485,12 @@ func (p *Planner) makeTypeJoinMany( return nil, err } - rootField, rootNameFound := subTypeCollectionDesc.GetRelation(subTypeFieldDesc.RelationName) + rootField, rootNameFound := subTypeCollectionDesc.GetFieldByRelation( + subTypeFieldDesc.RelationName, + parent.sourceInfo.collectionDescription.Name, + subTypeFieldDesc.Name, + ) + if !rootNameFound { return nil, client.NewErrFieldNotExist(subTypeFieldDesc.RelationName) } diff --git a/tests/integration/mutation/update/field_kinds/one_to_one/with_self_ref_test.go b/tests/integration/mutation/update/field_kinds/one_to_one/with_self_ref_test.go index 9f95b91408..16225f4ab3 100644 --- a/tests/integration/mutation/update/field_kinds/one_to_one/with_self_ref_test.go +++ b/tests/integration/mutation/update/field_kinds/one_to_one/with_self_ref_test.go @@ -103,8 +103,6 @@ func TestMutationUpdateOneToOne_SelfReferencingFromPrimary(t *testing.T) { testUtils.ExecuteTestCase(t, test) } -/* -This test will enter an infinite loop func TestMutationUpdateOneToOne_SelfReferencingFromSecondary(t *testing.T) { user1ID := "bae-decf6467-4c7c-50d7-b09d-0a7097ef6bad" @@ -191,4 +189,3 @@ func TestMutationUpdateOneToOne_SelfReferencingFromSecondary(t *testing.T) { testUtils.ExecuteTestCase(t, test) } -*/