From 42909c602344c0cf2a7d3e4e29b0d71e5ae892ff Mon Sep 17 00:00:00 2001 From: Andrew Sisley Date: Fri, 28 Jun 2024 11:49:17 -0400 Subject: [PATCH 1/3] Add test documenting issue --- tests/integration/schema/one_many_test.go | 39 +++++++++++++++++++++++ 1 file changed, 39 insertions(+) create mode 100644 tests/integration/schema/one_many_test.go diff --git a/tests/integration/schema/one_many_test.go b/tests/integration/schema/one_many_test.go new file mode 100644 index 0000000000..0f80537d25 --- /dev/null +++ b/tests/integration/schema/one_many_test.go @@ -0,0 +1,39 @@ +// 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 TestSchemaOneMany_Primary(t *testing.T) { + test := testUtils.TestCase{ + Actions: []any{ + testUtils.SchemaUpdate{ + Schema: ` + type User { + name: String + dogs: [Dog] + } + type Dog { + name: String + owner: User @primary + } + `, + ExpectedError: "duplicate field. Name: owner", + }, + }, + } + + testUtils.ExecuteTestCase(t, test) +} From abe73d9a74f6d5eb0c67d1b116d50a59606122a5 Mon Sep 17 00:00:00 2001 From: Andrew Sisley Date: Fri, 28 Jun 2024 12:05:16 -0400 Subject: [PATCH 2/3] Only (re) add primary field if it hasnt already been created --- internal/request/graphql/schema/collection.go | 23 +++++---- tests/integration/schema/one_many_test.go | 48 ++++++++++++++++++- 2 files changed, 60 insertions(+), 11 deletions(-) diff --git a/internal/request/graphql/schema/collection.go b/internal/request/graphql/schema/collection.go index 7c4e44593b..c4180be0f4 100644 --- a/internal/request/graphql/schema/collection.go +++ b/internal/request/graphql/schema/collection.go @@ -676,16 +676,19 @@ func finalizeRelations( } if !otherColFieldDescription.HasValue() || otherColFieldDescription.Value().Kind.Value().IsArray() { - // Relations only defined on one side of the object are possible, and so if this is one of them - // or if the other side is an array, we need to add the field to the schema (is primary side). - definition.Schema.Fields = append( - definition.Schema.Fields, - client.SchemaFieldDescription{ - Name: field.Name, - Kind: field.Kind.Value(), - Typ: cTypeByFieldNameByObjName[definition.Schema.Name][field.Name], - }, - ) + if _, exists := definition.Schema.GetFieldByName(field.Name); !exists { + // Relations only defined on one side of the object are possible, and so if this is one of them + // or if the other side is an array, we need to add the field to the schema (is primary side) + // if the field has not been explicitly declared by the user. + definition.Schema.Fields = append( + definition.Schema.Fields, + client.SchemaFieldDescription{ + Name: field.Name, + Kind: field.Kind.Value(), + Typ: cTypeByFieldNameByObjName[definition.Schema.Name][field.Name], + }, + ) + } } otherIsEmbedded := len(otherColDefinition.Value().Description.Fields) == 0 diff --git a/tests/integration/schema/one_many_test.go b/tests/integration/schema/one_many_test.go index 0f80537d25..bab84b3d40 100644 --- a/tests/integration/schema/one_many_test.go +++ b/tests/integration/schema/one_many_test.go @@ -13,6 +13,9 @@ package schema import ( "testing" + "github.com/sourcenetwork/immutable" + + "github.com/sourcenetwork/defradb/client" testUtils "github.com/sourcenetwork/defradb/tests/integration" ) @@ -30,7 +33,50 @@ func TestSchemaOneMany_Primary(t *testing.T) { owner: User @primary } `, - ExpectedError: "duplicate field. Name: owner", + ExpectedResults: []client.CollectionDescription{ + { + Name: immutable.Some("User"), + Fields: []client.CollectionFieldDescription{ + { + Name: "_docID", + }, + { + Name: "dogs", + ID: 1, + Kind: immutable.Some[client.FieldKind](client.ObjectArrayKind("Dog")), + RelationName: immutable.Some("dog_user"), + }, + { + Name: "name", + ID: 2, + }, + }, + }, + { + Name: immutable.Some("Dog"), + Fields: []client.CollectionFieldDescription{ + { + Name: "_docID", + }, + { + Name: "name", + ID: 1, + }, + { + Name: "owner", + ID: 2, + Kind: immutable.Some[client.FieldKind](client.ObjectKind("User")), + RelationName: immutable.Some("dog_user"), + }, + { + Name: "owner_id", + ID: 3, + Kind: immutable.Some[client.FieldKind](client.ScalarKind(client.FieldKind_DocID)), + RelationName: immutable.Some("dog_user"), + }, + }, + }, + }, }, }, } From 01caa854b2e20dce75dd914ee0009e68c0188aa3 Mon Sep 17 00:00:00 2001 From: Andrew Sisley Date: Fri, 28 Jun 2024 12:55:08 -0400 Subject: [PATCH 3/3] Fix typo Unrelated to this PR and spotted by Fred, but it wont get fixed for a while if it doesn't get snuck in here --- tests/integration/results.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/tests/integration/results.go b/tests/integration/results.go index 61561a48e1..755608394d 100644 --- a/tests/integration/results.go +++ b/tests/integration/results.go @@ -211,13 +211,13 @@ func assertCollectionDescriptions( if expected.Indexes != nil || len(actual.Indexes) != 0 { // Dont bother asserting this if the expected is nil and the actual is nil/empty. - // This is to say each test action from having to bother declaring an empty slice (if there are no indexes) + // This is to save each test action from having to bother declaring an empty slice (if there are no indexes) require.Equal(s.t, expected.Indexes, actual.Indexes) } if expected.Sources != nil || len(actual.Sources) != 0 { // Dont bother asserting this if the expected is nil and the actual is nil/empty. - // This is to say each test action from having to bother declaring an empty slice (if there are no sources) + // This is to save each test action from having to bother declaring an empty slice (if there are no sources) require.Equal(s.t, expected.Sources, actual.Sources) }