Skip to content

Commit

Permalink
fix: Infinite loop when updating one-one relation (#1915)
Browse files Browse the repository at this point in the history
## Relevant issue(s)

Resolves #1914 

## Description

Fixes an a bug where an infinite loop may be created when updating a
self-referencing one-one relation from the secondary side.
  • Loading branch information
AndrewSisley authored Sep 27, 2023
1 parent 2c862ac commit e952be5
Show file tree
Hide file tree
Showing 5 changed files with 218 additions and 11 deletions.
16 changes: 9 additions & 7 deletions client/descriptions.go
Original file line number Diff line number Diff line change
Expand Up @@ -52,13 +52,15 @@ func (col CollectionDescription) GetFieldByID(id FieldID) (FieldDescription, boo
return FieldDescription{}, false
}

// 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
}
// GetFieldByRelation returns the field that supports the relation of the given name.
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
Expand Down
2 changes: 1 addition & 1 deletion db/collection.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
}
Expand Down
7 changes: 6 additions & 1 deletion db/collection_update.go
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand All @@ -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)
}
Expand Down
13 changes: 11 additions & 2 deletions planner/type_join.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)
}
Expand Down Expand Up @@ -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)
}
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,191 @@
// Copyright 2023 Democratized Data Foundation
//
// Use of this software is governed by the Business Source License
// included in the file licenses/BSL.txt.
//
// As of the Change Date specified in that file, in accordance with
// the Business Source License, use of this software will be governed
// by the Apache License, Version 2.0, included in the file
// licenses/APL.txt.

package one_to_one

import (
"fmt"
"testing"

testUtils "github.com/sourcenetwork/defradb/tests/integration"
)

func TestMutationUpdateOneToOne_SelfReferencingFromPrimary(t *testing.T) {
user1ID := "bae-decf6467-4c7c-50d7-b09d-0a7097ef6bad"

test := testUtils.TestCase{
Description: "One to one update mutation, self referencing from primary",
Actions: []any{
testUtils.SchemaUpdate{
Schema: `
type User {
name: String
boss: User @primary
underling: User
}
`,
},
testUtils.CreateDoc{
Doc: `{
"name": "John"
}`,
},
testUtils.CreateDoc{
Doc: `{
"name": "Fred"
}`,
},
testUtils.UpdateDoc{
DocID: 1,
Doc: fmt.Sprintf(
`{
"boss_id": "%s"
}`,
user1ID,
),
},
testUtils.Request{
Request: `
query {
User {
name
boss {
name
}
}
}`,
Results: []map[string]any{
{
"name": "Fred",
"boss": map[string]any{
"name": "John",
},
},
{
"name": "John",
"boss": nil,
},
},
},
testUtils.Request{
Request: `
query {
User {
name
underling {
name
}
}
}`,
Results: []map[string]any{
{
"name": "Fred",
"underling": nil,
},
{
"name": "John",
"underling": map[string]any{
"name": "Fred",
},
},
},
},
},
}

testUtils.ExecuteTestCase(t, test)
}

func TestMutationUpdateOneToOne_SelfReferencingFromSecondary(t *testing.T) {
user1ID := "bae-decf6467-4c7c-50d7-b09d-0a7097ef6bad"

test := testUtils.TestCase{
Description: "One to one update mutation, self referencing from secondary",

Actions: []any{
testUtils.SchemaUpdate{
Schema: `
type User {
name: String
boss: User
underling: User @primary
}
`,
},
testUtils.CreateDoc{
Doc: `{
"name": "John"
}`,
},
testUtils.CreateDoc{
Doc: `{
"name": "Fred"
}`,
},
testUtils.UpdateDoc{
DocID: 1,
Doc: fmt.Sprintf(
`{
"boss_id": "%s"
}`,
user1ID,
),
},
testUtils.Request{
Request: `
query {
User {
name
boss {
name
}
}
}`,
Results: []map[string]any{
{
"name": "Fred",
"boss": map[string]any{
"name": "John",
},
},
{
"name": "John",
"boss": nil,
},
},
},
testUtils.Request{
Request: `
query {
User {
name
underling {
name
}
}
}`,
Results: []map[string]any{
{
"name": "Fred",
"underling": nil,
},
{
"name": "John",
"underling": map[string]any{
"name": "Fred",
},
},
},
},
},
}

testUtils.ExecuteTestCase(t, test)
}

0 comments on commit e952be5

Please sign in to comment.