fix: Prevent multiple docs from being linked in one one

db/collection.go

@@ -16,6 +16,7 @@ import (
 	"encoding/json"
 	"fmt"
 	"strconv"
+	"strings"
 
 	"github.com/fxamacker/cbor/v2"
 	"github.com/ipfs/go-cid"
@@ -1019,6 +1020,11 @@ func (c *collection) save(
 				continue
 			}
 
+			err = c.validateOneToOneLinkDoesntAlreadyExist(ctx, txn, doc.Key().String(), fieldDescription, val.Value())
+			if err != nil {
+				return cid.Undef, err
+			}
+
 			node, _, err := c.saveDocValue(ctx, txn, fieldKey, val)
 			if err != nil {
 				return cid.Undef, err
@@ -1082,6 +1088,108 @@ func (c *collection) save(
 	return headNode.Cid(), nil
 }
 
+func (c *collection) validateOneToOneLinkDoesntAlreadyExist(
+	ctx context.Context,
+	txn datastore.Txn,
+	docKey string,
+	fieldDescription client.FieldDescription,
+	value any,
+) error {
+	if !fieldDescription.RelationType.IsSet(client.Relation_Type_INTERNAL_ID) {
+		return nil
+	}
+
+	if value == nil {
+		return nil
+	}
+
+	objFieldDescription, ok := c.desc.Schema.GetField(strings.TrimSuffix(fieldDescription.Name, request.RelatedObjectID))
+	if !ok {
+		return client.NewErrFieldNotExist(strings.TrimSuffix(fieldDescription.Name, request.RelatedObjectID))
+	}
+	if !objFieldDescription.RelationType.IsSet(client.Relation_Type_ONEONE) {
+		return nil
+	}
+
+	fetcher := c.newFetcher()
+
+	err := fetcher.Init(ctx, txn, &c.desc, []client.FieldDescription{fieldDescription}, nil, nil, false, false)
+	if err != nil {
+		closeErr := fetcher.Close()
+		if closeErr != nil {
+			return errors.Wrap(err.Error(), closeErr)
+		}
+		return err
+	}
+
+	colPrefix := core.DataStoreKey{CollectionID: c.desc.IDString()}
+	err = fetcher.Start(ctx, core.NewSpans(core.NewSpan(colPrefix, colPrefix.PrefixEnd())))
+	if err != nil {
+		closeErr := fetcher.Close()
+		if closeErr != nil {
+			return errors.Wrap(err.Error(), closeErr)
+		}
+		return err
+	}
+
+	var alreadyLinked bool
+	var existingDocumentID string
+fetchLoop:
+	for {
+		doc, _, err := fetcher.FetchNext(ctx)
+		if err != nil {
+			closeErr := fetcher.Close()
+			if closeErr != nil {
+				return errors.Wrap(err.Error(), closeErr)
+			}
+			return err
+		}
+		if doc == nil {
+			err = fetcher.Close()
+			if err != nil {
+				return err
+			}
+			break
+		}
+
+		existingDocumentID = string(doc.Key())
+		if string(doc.Key()) == docKey {
+			continue
+		}
+
+		props, err := doc.Properties(false)
+		if err != nil {
+			closeErr := fetcher.Close()
+			if closeErr != nil {
+				return errors.Wrap(err.Error(), closeErr)
+			}
+			return err
+		}
+
+		for field, fetchedValue := range props {
+			if field.ID != fieldDescription.ID {
+				continue
+			}
+
+			if value == fetchedValue {
+				alreadyLinked = true
+				break fetchLoop
+			}
+		}
+	}
+
+	err = fetcher.Close()
+	if err != nil {
+		return err
+	}
+
+	if alreadyLinked {
+		return NewErrOneOneAlreadyLinked(docKey, existingDocumentID, objFieldDescription.RelationName)
+	}
+
+	return nil
+}
+
 // Delete will attempt to delete a document by key will return true if a deletion is successful,
 // and return false, along with an error, if it cannot.
 // If the document doesn't exist, then it will return false, and a ErrDocumentNotFound error.

db/collection_update.go

@@ -12,15 +12,16 @@ package db
 
 import (
 	"context"
-	"fmt"
 	"strings"
 
+	ds "github.com/ipfs/go-datastore"
 	"github.com/sourcenetwork/immutable"
 	"github.com/valyala/fastjson"
 
 	"github.com/sourcenetwork/defradb/client"
 	"github.com/sourcenetwork/defradb/client/request"
 	"github.com/sourcenetwork/defradb/datastore"
+	"github.com/sourcenetwork/defradb/errors"
 	"github.com/sourcenetwork/defradb/planner"
 )
 
@@ -364,13 +365,45 @@ func (c *collection) patchPrimaryDoc(
 	}
 	primaryCol = primaryCol.WithTxn(txn)
 
-	primaryField, _ := primaryCol.Description().GetRelation(relationFieldDescription.RelationName)
+	primaryField, ok := primaryCol.Description().GetRelation(relationFieldDescription.RelationName)
+	if !ok {
+		return client.NewErrFieldNotExist(relationFieldDescription.RelationName)
+	}
+
+	primaryIDField, ok := primaryCol.Description().Schema.GetField(primaryField.Name + request.RelatedObjectID)
+	if !ok {
+		return client.NewErrFieldNotExist(primaryField.Name + request.RelatedObjectID)
+	}
 
-	_, err = primaryCol.UpdateWithKey(
+	doc, err := primaryCol.Get(
 		ctx,
 		primaryDockey,
-		fmt.Sprintf(`{"%s": "%s"}`, primaryField.Name+request.RelatedObjectID, docKey),
+		false,
 	)
+	if err != nil && !errors.Is(err, ds.ErrNotFound) {
+		return err
+	}
+
+	// If the document doesn't exist then there is nothing to update.
+	if doc == nil {
+		return nil
+	}
+
+	existingVal, err := doc.GetValue(primaryIDField.Name)
+	if err != nil && !errors.Is(err, client.ErrFieldNotExist) {
+		return err
+	}
+
+	if existingVal != nil && existingVal.Value() != "" && existingVal.Value() != docKey {
+		return NewErrOneOneAlreadyLinked(docKey, fieldValue, relationFieldDescription.RelationName)
+	}
+
+	err = doc.Set(primaryIDField.Name, docKey)
+	if err != nil {
+		return err
+	}
+
+	err = primaryCol.Update(ctx, doc)
 	if err != nil {
 		return err
 	}

db/errors.go

@@ -83,6 +83,7 @@ const (
 	errDocUpdate                          string = "failed to update doc to collection"
 	errExpectedJSONObject                 string = "expected JSON object"
 	errExpectedJSONArray                  string = "expected JSON array"
+	errOneOneAlreadyLinked                string = "target document is already linked to another document"
 )
 
 var (
@@ -159,6 +160,7 @@ var (
 	ErrDocUpdate                          = errors.New(errDocUpdate)
 	ErrExpectedJSONObject                 = errors.New(errExpectedJSONObject)
 	ErrExpectedJSONArray                  = errors.New(errExpectedJSONArray)
+	ErrOneOneAlreadyLinked                = errors.New(errOneOneAlreadyLinked)
 )
 
 // NewErrFieldOrAliasToFieldNotExist returns an error indicating that the given field or an alias field does not exist.
@@ -600,3 +602,12 @@ func NewErrDocCreate(inner error) error {
 func NewErrDocUpdate(inner error) error {
 	return errors.Wrap(errDocUpdate, inner)
 }
+
+func NewErrOneOneAlreadyLinked(documentId, targetId, relationName string) error {
+	return errors.New(
+		errOneOneAlreadyLinked,
+		errors.NewKV("DocumentID", documentId),
+		errors.NewKV("TargetID", targetId),
+		errors.NewKV("RelationName", relationName),
+	)
+}

tests/integration/backup/one_to_one/export_test.go

@@ -65,41 +65,6 @@ func TestBackupExport_AllCollectionsMultipleDocsAndDocUpdate_NoError(t *testing.
 	executeTestCase(t, test)
 }
 
-// note: This test should fail at the second book creation since the relationship is 1-to-1 and this
-// effectively creates a 1-to-many relationship
-func TestBackupExport_AllCollectionsMultipleDocsAndMultipleDocUpdate_NoError(t *testing.T) {
-	test := testUtils.TestCase{
-		Actions: []any{
-			testUtils.CreateDoc{
-				CollectionID: 0,
-				Doc:          `{"name": "John", "age": 30}`,
-			},
-			testUtils.CreateDoc{
-				CollectionID: 0,
-				Doc:          `{"name": "Bob", "age": 31}`,
-			},
-			testUtils.CreateDoc{
-				CollectionID: 1,
-				Doc:          `{"name": "John and the sourcerers' stone", "author": "bae-e933420a-988a-56f8-8952-6c245aebd519"}`,
-			},
-			testUtils.CreateDoc{
-				CollectionID: 1,
-				Doc:          `{"name": "Game of chains", "author": "bae-e933420a-988a-56f8-8952-6c245aebd519"}`,
-			},
-			testUtils.UpdateDoc{
-				CollectionID: 0,
-				DocID:        0,
-				Doc:          `{"age": 31}`,
-			},
-			testUtils.BackupExport{
-				ExpectedContent: `{"Book":[{"_key":"bae-4399f189-138d-5d49-9e25-82e78463677b","_newKey":"bae-78a40f28-a4b8-5dca-be44-392b0f96d0ff","author_id":"bae-807ea028-6c13-5f86-a72b-46e8b715a162","name":"Game of chains"},{"_key":"bae-5cf2fec3-d8ed-50d5-8286-39109853d2da","_newKey":"bae-edeade01-2d21-5d6d-aadf-efc5a5279de5","author_id":"bae-807ea028-6c13-5f86-a72b-46e8b715a162","name":"John and the sourcerers' stone"}],"User":[{"_key":"bae-0648f44e-74e8-593b-a662-3310ec278927","_newKey":"bae-0648f44e-74e8-593b-a662-3310ec278927","age":31,"name":"Bob"},{"_key":"bae-e933420a-988a-56f8-8952-6c245aebd519","_newKey":"bae-807ea028-6c13-5f86-a72b-46e8b715a162","age":31,"name":"John"}]}`,
-			},
-		},
-	}
-
-	executeTestCase(t, test)
-}
-
 func TestBackupExport_DoubleReletionship_NoError(t *testing.T) {
 	test := testUtils.TestCase{
 		Actions: []any{
@@ -191,53 +156,3 @@ func TestBackupExport_DoubleReletionshipWithUpdate_NoError(t *testing.T) {
 
 	testUtils.ExecuteTestCase(t, test)
 }
-
-// note: This test should fail at the second book creation since the relationship is 1-to-1 and this
-// effectively creates a 1-to-many relationship
-func TestBackupExport_DoubleReletionshipWithUpdateAndDoublylinked_NoError(t *testing.T) {
-	test := testUtils.TestCase{
-		Actions: []any{
-			testUtils.SchemaUpdate{
-				Schema: `
-				type User {
-					name: String
-					age: Int
-					book: Book @relation(name: "written_books")
-					favouriteBook: Book @relation(name: "favourite_books")
-				}
-				type Book {
-					name: String
-					author: User @relation(name: "written_books")
-					favourite: User @relation(name: "favourite_books")
-				}
-				`,
-			},
-			testUtils.CreateDoc{
-				CollectionID: 0,
-				Doc:          `{"name": "John", "age": 30}`,
-			},
-			testUtils.CreateDoc{
-				CollectionID: 0,
-				Doc:          `{"name": "Bob", "age": 31}`,
-			},
-			testUtils.CreateDoc{
-				CollectionID: 1,
-				Doc:          `{"name": "John and the sourcerers' stone", "author": "bae-e933420a-988a-56f8-8952-6c245aebd519", "favourite": "bae-0648f44e-74e8-593b-a662-3310ec278927"}`,
-			},
-			testUtils.CreateDoc{
-				CollectionID: 1,
-				Doc:          `{"name": "Game of chains"}`,
-			},
-			testUtils.UpdateDoc{
-				CollectionID: 0,
-				DocID:        0,
-				Doc:          `{"age": 31, "book_id": "bae-da7f2d88-05c4-528a-846a-0d18ab26603b"}`,
-			},
-			testUtils.BackupExport{
-				ExpectedContent: `{"Book":[{"_key":"bae-45b1def4-4e63-5a93-a1b8-f7b08e682164","_newKey":"bae-add2ccfe-84a1-519c-ab7d-c54b43909532","author_id":"bae-807ea028-6c13-5f86-a72b-46e8b715a162","favourite_id":"bae-0648f44e-74e8-593b-a662-3310ec278927","name":"John and the sourcerers' stone"},{"_key":"bae-da7f2d88-05c4-528a-846a-0d18ab26603b","_newKey":"bae-78a40f28-a4b8-5dca-be44-392b0f96d0ff","author_id":"bae-807ea028-6c13-5f86-a72b-46e8b715a162","name":"Game of chains"}],"User":[{"_key":"bae-0648f44e-74e8-593b-a662-3310ec278927","_newKey":"bae-0648f44e-74e8-593b-a662-3310ec278927","age":31,"name":"Bob"},{"_key":"bae-e933420a-988a-56f8-8952-6c245aebd519","_newKey":"bae-807ea028-6c13-5f86-a72b-46e8b715a162","age":31,"name":"John"}]}`,
-			},
-		},
-	}
-
-	testUtils.ExecuteTestCase(t, test)
-}

tests/integration/backup/one_to_one/import_test.go

@@ -150,9 +150,6 @@ func TestBackupImport_WithMultipleNoKeyAndMultipleCollectionsAndUpdatedDocs_NoEr
 	executeTestCase(t, test)
 }
 
-// note: This test should fail at the second book creation since the relationship is 1-to-1 and this
-// effectively creates a 1-to-many relationship:
-// https://github.com/sourcenetwork/defradb/issues/1646
 func TestBackupImport_WithMultipleNoKeyAndMultipleCollectionsAndMultipleUpdatedDocs_NoError(t *testing.T) {
 	test := testUtils.TestCase{
 		Actions: []any{
@@ -187,50 +184,7 @@ func TestBackupImport_WithMultipleNoKeyAndMultipleCollectionsAndMultipleUpdatedD
 						}
 					]
 				}`,
-			},
-			testUtils.Request{
-				Request: `
-					query  {
-						User {
-							name
-							age
-						}
-					}`,
-				Results: []map[string]any{
-					{
-						"name": "Bob",
-						"age":  uint64(31),
-					},
-					{
-						"name": "John",
-						"age":  uint64(31),
-					},
-				},
-			},
-			testUtils.Request{
-				Request: `
-					query  {
-						Book {
-							name
-							author {
-								_key
-							}
-						}
-					}`,
-				Results: []map[string]any{
-					{
-						"name": "Game of chains",
-						"author": map[string]any{
-							"_key": "bae-807ea028-6c13-5f86-a72b-46e8b715a162",
-						},
-					},
-					{
-						"name": "John and the sourcerers' stone",
-						"author": map[string]any{
-							"_key": "bae-807ea028-6c13-5f86-a72b-46e8b715a162",
-						},
-					},
-				},
+				ExpectedError: "target document is already linked to another document.",
 			},
 		},
 	}