From 34ff6480a44b804b3fe8cf21b90b8afffb21ecc8 Mon Sep 17 00:00:00 2001 From: AndrewSisley Date: Thu, 28 Mar 2024 11:57:29 -0400 Subject: [PATCH] feat: Force explicit primary decl. in SDL for one-ones (#2462) ## Relevant issue(s) Resolves #2461 ## Description Forces the explicit declaration of the primary side of one-one relation fields in SDL. PatchSchema already forces this and needed no change. --- client/errors.go | 9 +++ db/collection.go | 2 +- db/errors.go | 8 --- request/graphql/schema/descriptions_test.go | 4 +- request/graphql/schema/relations.go | 8 +-- tests/gen/gen_auto_test.go | 8 +-- .../backup/one_to_one/export_test.go | 8 +-- .../backup/one_to_one/import_test.go | 4 +- tests/integration/explain/fixture.go | 4 +- .../index/query_with_relation_filter_test.go | 4 +- .../field_kinds/one_to_one_to_one/utils.go | 2 +- .../field_kinds/one_to_one_to_one/utils.go | 2 +- tests/integration/schema/one_one_test.go | 61 +++++++++++++++++++ tests/integration/schema/relations_test.go | 2 +- .../view/one_to_one/with_transform_test.go | 2 +- tests/predefined/gen_predefined_test.go | 2 +- 16 files changed, 93 insertions(+), 37 deletions(-) create mode 100644 tests/integration/schema/one_one_test.go diff --git a/client/errors.go b/client/errors.go index 71a111d431..7e18e9566c 100644 --- a/client/errors.go +++ b/client/errors.go @@ -34,6 +34,7 @@ const ( errCanNotNormalizeValue string = "can not normalize value" errCanNotTurnNormalValueIntoArray string = "can not turn normal value into array" errCanNotMakeNormalNilFromFieldKind string = "can not make normal nil from field kind" + errPrimarySideNotDefined string = "primary side of relation not defined" ) // Errors returnable from this package. @@ -57,6 +58,7 @@ var ( ErrCanNotNormalizeValue = errors.New(errCanNotNormalizeValue) ErrCanNotTurnNormalValueIntoArray = errors.New(errCanNotTurnNormalValueIntoArray) ErrCanNotMakeNormalNilFromFieldKind = errors.New(errCanNotMakeNormalNilFromFieldKind) + ErrPrimarySideNotDefined = errors.New(errPrimarySideNotDefined) ) // NewErrFieldNotExist returns an error indicating that the given field does not exist. @@ -178,3 +180,10 @@ func NewErrCRDTKindMismatch(cType, kind string) error { func NewErrInvalidJSONPaylaod(payload string) error { return errors.New(errInvalidJSONPayload, errors.NewKV("Payload", payload)) } + +func NewErrPrimarySideNotDefined(relationName string) error { + return errors.New( + errPrimarySideNotDefined, + errors.NewKV("RelationName", relationName), + ) +} diff --git a/db/collection.go b/db/collection.go index 49bdf01e71..2ad2cf2ca5 100644 --- a/db/collection.go +++ b/db/collection.go @@ -484,7 +484,7 @@ func validateUpdateSchemaFields( } if !(proposedField.IsPrimaryRelation || relatedField.IsPrimaryRelation) { - return false, NewErrPrimarySideNotDefined(proposedField.RelationName) + return false, client.NewErrPrimarySideNotDefined(proposedField.RelationName) } if proposedField.IsPrimaryRelation && relatedField.IsPrimaryRelation { diff --git a/db/errors.go b/db/errors.go index c32da44671..b854ad2d3d 100644 --- a/db/errors.go +++ b/db/errors.go @@ -31,7 +31,6 @@ const ( errRelationalFieldInvalidRelationType string = "invalid RelationType" errRelationalFieldMissingIDField string = "missing id field for relation object field" errRelationalFieldMissingRelationName string = "missing relation name" - errPrimarySideNotDefined string = "primary side of relation not defined" errPrimarySideOnMany string = "cannot set the many side of a relation as primary" errBothSidesPrimary string = "both sides of a relation cannot be primary" errRelatedFieldKindMismatch string = "invalid Kind of the related field" @@ -273,13 +272,6 @@ func NewErrRelationalFieldMissingRelationName(name string) error { ) } -func NewErrPrimarySideNotDefined(relationName string) error { - return errors.New( - errPrimarySideNotDefined, - errors.NewKV("RelationName", relationName), - ) -} - func NewErrPrimarySideOnMany(name string) error { return errors.New( errPrimarySideOnMany, diff --git a/request/graphql/schema/descriptions_test.go b/request/graphql/schema/descriptions_test.go index 1540037a8d..354109965c 100644 --- a/request/graphql/schema/descriptions_test.go +++ b/request/graphql/schema/descriptions_test.go @@ -157,7 +157,7 @@ func TestSingleSimpleType(t *testing.T) { type Author { name: String age: Int - published: Book + published: Book @primary } `, targetDescs: []client.CollectionDefinition{ @@ -330,7 +330,7 @@ func TestSingleSimpleType(t *testing.T) { type Author { name: String age: Int - published: Book @relation(name:"book_authors") + published: Book @relation(name:"book_authors") @primary } `, targetDescs: []client.CollectionDefinition{ diff --git a/request/graphql/schema/relations.go b/request/graphql/schema/relations.go index e6d2af8b09..6d548ceebe 100644 --- a/request/graphql/schema/relations.go +++ b/request/graphql/schema/relations.go @@ -124,13 +124,7 @@ func (r *Relation) finalize() error { if aBit.isSet(relation_Type_Primary) { return ErrMultipleRelationPrimaries } else if !xBit.isSet(relation_Type_Primary) { - // neither type has primary set, auto add to - // lexicographically first one by schema type name - if strings.Compare(r.schemaTypes[0], r.schemaTypes[1]) < 1 { - r.types[1] = r.types[1] | relation_Type_Primary - } else { - r.types[0] = r.types[0] | relation_Type_Primary - } + return client.NewErrPrimarySideNotDefined(r.name) } } diff --git a/tests/gen/gen_auto_test.go b/tests/gen/gen_auto_test.go index 52ee7eb58d..54212509e0 100644 --- a/tests/gen/gen_auto_test.go +++ b/tests/gen/gen_auto_test.go @@ -338,7 +338,7 @@ func TestAutoGenerateFromSchema_RelationOneToOne(t *testing.T) { } type Device { - owner: User + owner: User @primary model: String }` @@ -792,7 +792,7 @@ func TestAutoGenerateFromSchema_ConfigThatCanNotBySupplied(t *testing.T) { type Device { model: String - owner: User + owner: User @primary }`, options: []Option{WithTypeDemand("User", 10), WithTypeDemand("Device", 30)}, }, @@ -801,12 +801,12 @@ func TestAutoGenerateFromSchema_ConfigThatCanNotBySupplied(t *testing.T) { type User { name: String device: Device - orders: Order + orders: Order @primary } type Device { model: String - owner: User + owner: User @primary } type Order { diff --git a/tests/integration/backup/one_to_one/export_test.go b/tests/integration/backup/one_to_one/export_test.go index 4ae32cbebc..b52e0bb02f 100644 --- a/tests/integration/backup/one_to_one/export_test.go +++ b/tests/integration/backup/one_to_one/export_test.go @@ -78,8 +78,8 @@ func TestBackupExport_DoubleReletionship_NoError(t *testing.T) { } type Book { name: String - author: User @relation(name: "written_books") - favourite: User @relation(name: "favourite_books") + author: User @relation(name: "written_books") @primary + favourite: User @relation(name: "favourite_books") @primary } `, }, @@ -122,8 +122,8 @@ func TestBackupExport_DoubleReletionshipWithUpdate_NoError(t *testing.T) { } type Book { name: String - author: User @relation(name: "written_books") - favourite: User @relation(name: "favourite_books") + author: User @relation(name: "written_books") @primary + favourite: User @relation(name: "favourite_books") @primary } `, }, diff --git a/tests/integration/backup/one_to_one/import_test.go b/tests/integration/backup/one_to_one/import_test.go index 5405dd4225..d7ca39ea55 100644 --- a/tests/integration/backup/one_to_one/import_test.go +++ b/tests/integration/backup/one_to_one/import_test.go @@ -205,8 +205,8 @@ func TestBackupImport_DoubleRelationshipWithUpdate_NoError(t *testing.T) { } type Book { name: String - author: User @relation(name: "written_books") - favourite: User @relation(name: "favourite_books") + author: User @relation(name: "written_books") @primary + favourite: User @relation(name: "favourite_books") @primary } `, }, diff --git a/tests/integration/explain/fixture.go b/tests/integration/explain/fixture.go index c531d95a84..83db5ff926 100644 --- a/tests/integration/explain/fixture.go +++ b/tests/integration/explain/fixture.go @@ -38,14 +38,14 @@ var SchemaForExplainTests = testUtils.SchemaUpdate{ verified: Boolean books: [Book] articles: [Article] - contact: AuthorContact + contact: AuthorContact @primary } type AuthorContact { cell: String email: String author: Author - address: ContactAddress + address: ContactAddress @primary } type ContactAddress { diff --git a/tests/integration/index/query_with_relation_filter_test.go b/tests/integration/index/query_with_relation_filter_test.go index e3ae71429e..db2f351ae7 100644 --- a/tests/integration/index/query_with_relation_filter_test.go +++ b/tests/integration/index/query_with_relation_filter_test.go @@ -167,7 +167,7 @@ func TestQueryWithIndexOnOneToOnesSecondaryRelation_IfFilterOnIndexedRelation_Sh } type Address { - user: User + user: User @primary city: String @index }`, }, @@ -348,7 +348,7 @@ func TestQueryWithIndexOnOneToTwoRelation_IfFilterOnIndexedRelation_ShouldFilter } type Address { - user: User + user: User @primary city: String @index }`, }, diff --git a/tests/integration/mutation/create/field_kinds/one_to_one_to_one/utils.go b/tests/integration/mutation/create/field_kinds/one_to_one_to_one/utils.go index 9fce31fdb2..896fb1c5eb 100644 --- a/tests/integration/mutation/create/field_kinds/one_to_one_to_one/utils.go +++ b/tests/integration/mutation/create/field_kinds/one_to_one_to_one/utils.go @@ -29,7 +29,7 @@ func execute(t *testing.T, test testUtils.TestCase) { name: String rating: Float author: Author - publisher: Publisher + publisher: Publisher @primary } type Author { diff --git a/tests/integration/mutation/delete/field_kinds/one_to_one_to_one/utils.go b/tests/integration/mutation/delete/field_kinds/one_to_one_to_one/utils.go index 89f0e497f4..131a7194fe 100644 --- a/tests/integration/mutation/delete/field_kinds/one_to_one_to_one/utils.go +++ b/tests/integration/mutation/delete/field_kinds/one_to_one_to_one/utils.go @@ -29,7 +29,7 @@ func execute(t *testing.T, test testUtils.TestCase) { name: String rating: Float author: Author - publisher: Publisher + publisher: Publisher @primary } type Author { diff --git a/tests/integration/schema/one_one_test.go b/tests/integration/schema/one_one_test.go new file mode 100644 index 0000000000..e14792f75e --- /dev/null +++ b/tests/integration/schema/one_one_test.go @@ -0,0 +1,61 @@ +// Copyright 2024 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 schema + +import ( + "testing" + + testUtils "github.com/sourcenetwork/defradb/tests/integration" +) + +func TestSchemaOneOne_NoPrimary_Errors(t *testing.T) { + test := testUtils.TestCase{ + Actions: []any{ + testUtils.SchemaUpdate{ + Schema: ` + type User { + name: String + dog: Dog + } + type Dog { + name: String + owner: User + } + `, + ExpectedError: "primary side of relation not defined. RelationName: dog_user", + }, + }, + } + + testUtils.ExecuteTestCase(t, test) +} + +func TestSchemaOneOne_TwoPrimaries_Errors(t *testing.T) { + test := testUtils.TestCase{ + Actions: []any{ + testUtils.SchemaUpdate{ + Schema: ` + type User { + name: String + dog: Dog @primary + } + type Dog { + name: String + owner: User @primary + } + `, + ExpectedError: "relation can only have a single field set as primary", + }, + }, + } + + testUtils.ExecuteTestCase(t, test) +} diff --git a/tests/integration/schema/relations_test.go b/tests/integration/schema/relations_test.go index d1b420afb6..ade67c689a 100644 --- a/tests/integration/schema/relations_test.go +++ b/tests/integration/schema/relations_test.go @@ -23,7 +23,7 @@ func TestSchemaRelationOneToOne(t *testing.T) { Schema: ` type Dog { name: String - user: User + user: User @primary } type User { dog: Dog diff --git a/tests/integration/view/one_to_one/with_transform_test.go b/tests/integration/view/one_to_one/with_transform_test.go index cc638596e0..e6da410ee1 100644 --- a/tests/integration/view/one_to_one/with_transform_test.go +++ b/tests/integration/view/one_to_one/with_transform_test.go @@ -32,7 +32,7 @@ func TestView_OneToOneWithTransformOnOuter(t *testing.T) { } type Book { name: String - author: Author + author: Author @primary } `, }, diff --git a/tests/predefined/gen_predefined_test.go b/tests/predefined/gen_predefined_test.go index c5e863a51c..94b261059e 100644 --- a/tests/predefined/gen_predefined_test.go +++ b/tests/predefined/gen_predefined_test.go @@ -80,7 +80,7 @@ func TestGeneratePredefinedFromSchema_OneToOne(t *testing.T) { } type Device { model: String - owner: User + owner: User @primary }` docs, err := CreateFromSDL(schema, DocsList{