fix: Prevent mutations from secondary side of relation

client/document.go

@@ -241,11 +241,32 @@
 		if err != nil {
 			return nil, err
 		}
+
+		// Validate that the given value is a valid docID
+		_, err = NewDocIDFromString(v)
+		if err != nil {
+			return nil, err
+		}
+
 		return NewNormalString(v), nil
 	}
 
 	switch field.Kind {
-	case FieldKind_DocID, FieldKind_NILLABLE_STRING, FieldKind_NILLABLE_BLOB:
+	case FieldKind_DocID:
+		v, err := getString(val)
+		if err != nil {
+			return nil, err
+		}
+
+		// Validate that the given value is a valid docID
+		_, err = NewDocIDFromString(v)
+		if err != nil {
+			return nil, err
+		}
+
+		return NewNormalString(v), nil
+
+	case FieldKind_NILLABLE_STRING, FieldKind_NILLABLE_BLOB:
 		v, err := getString(val)
 		if err != nil {
 			return nil, err
@@ -692,6 +713,17 @@
 	if !exists {
 		return NewErrFieldNotExist(field)
 	}
+
+	if fd.Kind == FieldKind_DocID && strings.HasSuffix(field, request.RelatedObjectID) {
+		objFieldName := strings.TrimSuffix(field, request.RelatedObjectID)
+		ofd, exists := doc.collectionDefinition.GetFieldByName(objFieldName)
+		if exists && !ofd.IsPrimaryRelation {
+			return NewErrCannotSetRelationFromSecondarySide(field)
+		}
+	} else if fd.Kind.IsObject() && !fd.IsPrimaryRelation {
+		return NewErrCannotSetRelationFromSecondarySide(field)
+	}
+
 	if fd.Kind.IsObject() && !fd.Kind.IsArray() {
 		if !strings.HasSuffix(field, request.RelatedObjectID) {
 			field = field + request.RelatedObjectID

client/errors.go

@@ -34,6 +34,7 @@ const (
 	errCanNotTurnNormalValueIntoArray      string = "can not turn normal value into array"
 	errCanNotMakeNormalNilFromFieldKind    string = "can not make normal nil from field kind"
 	errFailedToParseKind                   string = "failed to parse kind"
+	errCannotSetRelationFromSecondarySide  string = "cannot set relation from secondary side"
 )
 
 // Errors returnable from this package.
@@ -190,3 +191,7 @@ func ReviveError(message string) error {
 		return fmt.Errorf("%s", message)
 	}
 }
+
+func NewErrCannotSetRelationFromSecondarySide(name string) error {
+	return errors.New(errCannotSetRelationFromSecondarySide, errors.NewKV("Name", name))
+}

docs/data_format_changes/i3102-no-change-tests-updated.md

@@ -0,0 +1,3 @@
+# Prevent mutations from secondary side of relation
+
+The docIDs in some tests that relied on the removed behaviour have changed.

internal/db/collection.go

@@ -655,31 +655,6 @@ func (c *collection) save(
 			// that it's set to the same as the field description CRDT type.
 			val.SetType(fieldDescription.Typ)
 
-			relationFieldDescription, isSecondaryRelationID := fieldDescription.GetSecondaryRelationField(c.Definition())
-			if isSecondaryRelationID {
-				if val.Value() == nil {
-					// If the value (relation) is nil, we don't need to check for any documents already linked to it
-					continue
-				}
-
-				primaryId := val.Value().(string)
-
-				err = c.patchPrimaryDoc(
-					ctx,
-					c.Name().Value(),
-					relationFieldDescription,
-					primaryKey.DocID,
-					primaryId,
-				)
-				if err != nil {
-					return cid.Undef, err
-				}
-
-				// If this field was a secondary relation ID the related document will have been
-				// updated instead and we should discard this value
-				continue
-			}
-
 			err = c.validateOneToOneLinkDoesntAlreadyExist(
 				ctx,
 				doc.ID().String(),

internal/db/collection_update.go

@@ -13,13 +13,11 @@ package db
 import (
 	"context"
 
-	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/errors"
 	"github.com/sourcenetwork/defradb/internal/planner"
 )
 
@@ -133,91 +131,6 @@ func (c *collection) updateWithFilter(
 	return results, nil
 }
 
-// patchPrimaryDoc patches the (primary) document linked to from the document of the given DocID via the
-// given (secondary) relationship field description (hosted on the collection of the document matching the
-// given DocID).
-//
-// The given field value should be the string representation of the DocID of the primary document to be
-// patched.
-func (c *collection) patchPrimaryDoc(
-	ctx context.Context,
-	secondaryCollectionName string,
-	relationFieldDescription client.FieldDefinition,
-	docID string,
-	fieldValue string,
-) error {
-	primaryDocID, err := client.NewDocIDFromString(fieldValue)
-	if err != nil {
-		return err
-	}
-
-	primaryDef, _, err := client.GetDefinitionFromStore(ctx, c.db, c.Definition(), relationFieldDescription.Kind)
-	if err != nil {
-		return err
-	}
-
-	primaryField, ok := primaryDef.Description.GetFieldByRelation(
-		relationFieldDescription.RelationName,
-		secondaryCollectionName,
-		relationFieldDescription.Name,
-	)
-	if !ok {
-		return client.NewErrFieldNotExist(relationFieldDescription.RelationName)
-	}
-
-	primaryIDField, ok := primaryDef.GetFieldByName(primaryField.Name + request.RelatedObjectID)
-	if !ok {
-		return client.NewErrFieldNotExist(primaryField.Name + request.RelatedObjectID)
-	}
-
-	primaryCol := c.db.newCollection(primaryDef.Description, primaryDef.Schema)
-	doc, err := primaryCol.Get(
-		ctx,
-		primaryDocID,
-		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
-	}
-
-	err = primaryCol.validateOneToOneLinkDoesntAlreadyExist(
-		ctx,
-		primaryDocID.String(),
-		primaryIDField,
-		docID,
-	)
-	if err != nil {
-		return err
-	}
-
-	existingVal, err := doc.GetValue(primaryIDField.Name)
-	if err != nil && !errors.Is(err, client.ErrFieldNotExist) {
-		return err
-	}
-
-	if existingVal != nil && existingVal.Value() != "" && existingVal.Value() != docID {
-		return NewErrOneOneAlreadyLinked(docID, fieldValue, relationFieldDescription.RelationName)
-	}
-
-	err = doc.Set(primaryIDField.Name, docID)
-	if err != nil {
-		return err
-	}
-
-	err = primaryCol.Update(ctx, doc)
-	if err != nil {
-		return err
-	}
-
-	return nil
-}
-
 // makeSelectionPlan constructs a simple read-only plan of the collection using the given filter.
 // currently it doesn't support any other operations other than filters.
 // (IE: No limit, order, etc)

internal/request/graphql/schema/generate.go

@@ -566,6 +566,20 @@ func (g *Generator) buildMutationInputTypes(collections []client.CollectionDefin
 					continue
 				}
 
+				if field.Kind == client.FieldKind_DocID && strings.HasSuffix(field.Name, request.RelatedObjectID) {
+					objFieldName := strings.TrimSuffix(field.Name, request.RelatedObjectID)
+					ofd, exists := collection.GetFieldByName(objFieldName)
+					if exists && !ofd.IsPrimaryRelation {
+						// We do not allow the mutation of relations from the secondary side,
+						// they must not be included in the input type(s)
+						continue
+					}
+				} else if field.Kind.IsObject() && !field.IsPrimaryRelation {
+					// We do not allow the mutation of relations from the secondary side,
+					// they must not be included in the input type(s)
+					continue
+				}
+
 				var ttype gql.Type
 				if field.Kind.IsObject() {
 					if field.Kind.IsArray() {

tests/integration/mutation/create/field_kinds/one_to_many/with_alias_test.go

@@ -94,35 +94,6 @@ func TestMutationCreateOneToMany_AliasedRelationNameNonExistingRelationManySide_
 	}
 	executeTestCase(t, test)
 }
-func TestMutationCreateOneToMany_AliasedRelationNameInvalidIDManySide_CreatedDoc(t *testing.T) {
-	test := testUtils.TestCase{
-		Description: "One to many create mutation, invalid id, from the many side, with alias",
-		Actions: []any{
-			testUtils.CreateDoc{
-				CollectionID: 0,
-				Doc: `{
-					"name": "Painted House",
-					"author": "ValueDoesntMatter"
-				}`,
-			},
-			testUtils.Request{
-				Request: `query {
-					Book {
-						name
-					}
-				}`,
-				Results: map[string]any{
-					"Book": []map[string]any{
-						{
-							"name": "Painted House",
-						},
-					},
-				},
-			},
-		},
-	}
-	executeTestCase(t, test)
-}
 
 func TestMutationCreateOneToMany_AliasedRelationNameToLinkFromManySide(t *testing.T) {
 	test := testUtils.TestCase{

tests/integration/mutation/create/field_kinds/one_to_one/with_alias_test.go

@@ -73,23 +73,6 @@ func TestMutationCreateOneToOne_UseAliasWithNonExistingRelationPrimarySide_Creat
 	executeTestCase(t, test)
 }
 
-func TestMutationCreateOneToOne_UseAliasWithNonExistingRelationSecondarySide_Error(t *testing.T) {
-	test := testUtils.TestCase{
-		Description: "One to one create mutation, alias relation, from the secondary side",
-		Actions: []any{
-			testUtils.CreateDoc{
-				CollectionID: 0,
-				Doc: `{
-					"name": "Painted House",
-					"author": "bae-be6d8024-4953-5a92-84b4-f042d25230c6"
-				}`,
-				ExpectedError: "document not found or not authorized to access",
-			},
-		},
-	}
-	executeTestCase(t, test)
-}
-
 func TestMutationCreateOneToOne_UseAliasedRelationNameToLink_QueryFromPrimarySide(t *testing.T) {
 	test := testUtils.TestCase{
 		Description: "One to one create mutation with an alias relation.",
@@ -153,9 +136,13 @@ func TestMutationCreateOneToOne_UseAliasedRelationNameToLink_QueryFromPrimarySid
 	executeTestCase(t, test)
 }
 
-func TestMutationCreateOneToOne_UseAliasedRelationNameToLink_QueryFromSecondarySide(t *testing.T) {
+func TestMutationCreateOneToOne_UseAliasedRelationNameToLink_CollectionAPI_Errors(t *testing.T) {
 	test := testUtils.TestCase{
 		Description: "One to one create mutation from secondary side with alias relation.",
+		SupportedMutationTypes: immutable.Some([]testUtils.MutationType{
+			testUtils.CollectionSaveMutationType,
+			testUtils.CollectionNamedMutationType,
+		}),
 		Actions: []any{
 			testUtils.CreateDoc{
 				CollectionID: 1,
@@ -169,46 +156,34 @@ func TestMutationCreateOneToOne_UseAliasedRelationNameToLink_QueryFromSecondaryS
 					"name":   "Painted House",
 					"author": testUtils.NewDocIndex(1, 0),
 				},
+				ExpectedError: "cannot set relation from secondary side",
 			},
-			testUtils.Request{
-				Request: `query {
-					Author {
-						name
-						published {
-							name
-						}
-					}
+		},
+	}
+
+	executeTestCase(t, test)
+}
+
+func TestMutationCreateOneToOne_UseAliasedRelationNameToLink_GQL_Errors(t *testing.T) {
+	test := testUtils.TestCase{
+		Description: "One to one create mutation from secondary side with alias relation.",
+		SupportedMutationTypes: immutable.Some([]testUtils.MutationType{
+			testUtils.GQLRequestMutationType,
+		}),
+		Actions: []any{
+			testUtils.CreateDoc{
+				CollectionID: 1,
+				Doc: `{
+					"name": "John Grisham"
 				}`,
-				Results: map[string]any{
-					"Author": []map[string]any{
-						{
-							"name": "John Grisham",
-							"published": map[string]any{
-								"name": "Painted House",
-							},
-						},
-					},
-				},
 			},
-			testUtils.Request{
-				Request: `query {
-					Book {
-						name
-						author {
-							name
-						}
-					}
-				}`,
-				Results: map[string]any{
-					"Book": []map[string]any{
-						{
-							"name": "Painted House",
-							"author": map[string]any{
-								"name": "John Grisham",
-							},
-						},
-					},
+			testUtils.CreateDoc{
+				CollectionID: 0,
+				DocMap: map[string]any{
+					"name":   "Painted House",
+					"author": testUtils.NewDocIndex(1, 0),
 				},
+				ExpectedError: "Argument \"input\" has invalid value",
 			},
 		},
 	}