diff --git a/db/collection.go b/db/collection.go index 352d3fc8a1..f066c1d9fe 100644 --- a/db/collection.go +++ b/db/collection.go @@ -483,17 +483,7 @@ func (db *db) setDefaultSchemaVersion( } } - cols, err := db.getAllCollections(ctx, txn) - if err != nil { - return err - } - - definitions := make([]client.CollectionDefinition, len(cols)) - for i, col := range cols { - definitions[i] = col.Definition() - } - - return db.parser.SetSchema(ctx, txn, definitions) + return db.loadSchema(ctx, txn) } func (db *db) setDefaultSchemaVersionExplicit( @@ -633,6 +623,47 @@ func (db *db) getAllCollections(ctx context.Context, txn datastore.Txn) ([]clien return collections, nil } +// getAllActiveDefinitions returns all queryable collection/views and any embedded schema used by them. +func (db *db) getAllActiveDefinitions(ctx context.Context, txn datastore.Txn) ([]client.CollectionDefinition, error) { + cols, err := description.GetCollections(ctx, txn) + if err != nil { + return nil, err + } + + definitions := make([]client.CollectionDefinition, len(cols)) + for i, col := range cols { + schema, err := description.GetSchemaVersion(ctx, txn, col.SchemaVersionID) + if err != nil { + return nil, err + } + + collection := db.newCollection(col, schema) + + err = collection.loadIndexes(ctx, txn) + if err != nil { + return nil, err + } + + definitions[i] = collection.Definition() + } + + schemas, err := description.GetCollectionlessSchemas(ctx, txn) + if err != nil { + return nil, err + } + + for _, schema := range schemas { + definitions = append( + definitions, + client.CollectionDefinition{ + Schema: schema, + }, + ) + } + + return definitions, nil +} + // GetAllDocIDs returns all the document IDs that exist in the collection. // // @todo: We probably need a lock on the collection for this kind of op since diff --git a/db/description/schema.go b/db/description/schema.go index 06b129f3df..c486ee1a59 100644 --- a/db/description/schema.go +++ b/db/description/schema.go @@ -283,3 +283,45 @@ func GetSchemaVersionIDs( return schemaVersions, nil } + +// GetCollectionlessSchemas returns all schema that are not attached to a collection. +// +// Typically this means any schema embedded in a View. +// +// WARNING: This function does not currently account for multiple versions of collectionless schema, +// at the moment such a situation is impossible, but that is likely to change, at which point this +// function will need to account for that. +func GetCollectionlessSchemas( + ctx context.Context, + txn datastore.Txn, +) ([]client.SchemaDescription, error) { + cols, err := GetCollections(ctx, txn) + if err != nil { + return nil, err + } + + allSchemas, err := GetAllSchemas(ctx, txn) + if err != nil { + return nil, err + } + + schemaRootsByVersionID := map[string]string{} + for _, schema := range allSchemas { + schemaRootsByVersionID[schema.VersionID] = schema.Root + } + + colSchemaRoots := map[string]struct{}{} + for _, col := range cols { + schemaRoot := schemaRootsByVersionID[col.SchemaVersionID] + colSchemaRoots[schemaRoot] = struct{}{} + } + + collectionlessSchema := []client.SchemaDescription{} + for _, schema := range allSchemas { + if _, hasCollection := colSchemaRoots[schema.Root]; !hasCollection { + collectionlessSchema = append(collectionlessSchema, schema) + } + } + + return collectionlessSchema, nil +} diff --git a/db/schema.go b/db/schema.go index df95df60e2..988aea5e17 100644 --- a/db/schema.go +++ b/db/schema.go @@ -54,11 +54,6 @@ func (db *db) addSchema( return nil, err } - err = db.parser.SetSchema(ctx, txn, append(existingDefinitions, newDefinitions...)) - if err != nil { - return nil, err - } - returnDescriptions := make([]client.CollectionDescription, len(newDefinitions)) for i, definition := range newDefinitions { col, err := db.createCollection(ctx, txn, definition) @@ -68,20 +63,20 @@ func (db *db) addSchema( returnDescriptions[i] = col.Description() } + err = db.loadSchema(ctx, txn) + if err != nil { + return nil, err + } + return returnDescriptions, nil } func (db *db) loadSchema(ctx context.Context, txn datastore.Txn) error { - collections, err := db.getAllCollections(ctx, txn) + definitions, err := db.getAllActiveDefinitions(ctx, txn) if err != nil { return err } - definitions := make([]client.CollectionDefinition, len(collections)) - for i := range collections { - definitions[i] = collections[i].Definition() - } - return db.parser.SetSchema(ctx, txn, definitions) } @@ -150,17 +145,7 @@ func (db *db) patchSchema(ctx context.Context, txn datastore.Txn, patchString st } } - newCollections, err := db.getAllCollections(ctx, txn) - if err != nil { - return err - } - - definitions := make([]client.CollectionDefinition, len(newCollections)) - for i, col := range newCollections { - definitions[i] = col.Definition() - } - - return db.parser.SetSchema(ctx, txn, definitions) + return db.loadSchema(ctx, txn) } // substituteSchemaPatch handles any substitution of values that may be required before diff --git a/db/view.go b/db/view.go index dc04c83303..2b4666df22 100644 --- a/db/view.go +++ b/db/view.go @@ -60,21 +60,6 @@ func (db *db) addView( newDefinitions[i].Description.BaseQuery = baseQuery } - existingCollections, err := db.getAllCollections(ctx, txn) - if err != nil { - return nil, err - } - - existingDefinitions := make([]client.CollectionDefinition, len(existingCollections)) - for i := range existingCollections { - existingDefinitions[i] = existingCollections[i].Definition() - } - - err = db.parser.SetSchema(ctx, txn, append(existingDefinitions, newDefinitions...)) - if err != nil { - return nil, err - } - returnDescriptions := make([]client.CollectionDefinition, len(newDefinitions)) for i, definition := range newDefinitions { if definition.Description.Name == "" { @@ -95,5 +80,10 @@ func (db *db) addView( } } + err = db.loadSchema(ctx, txn) + if err != nil { + return nil, err + } + return returnDescriptions, nil } diff --git a/tests/integration/schema/simple_test.go b/tests/integration/schema/simple_test.go index ed8e05abf7..dccec9c4dd 100644 --- a/tests/integration/schema/simple_test.go +++ b/tests/integration/schema/simple_test.go @@ -78,7 +78,7 @@ func TestSchemaSimpleErrorsGivenDuplicateSchema(t *testing.T) { Schema: ` type Users {} `, - ExpectedError: "schema type already exists", + ExpectedError: "collection already exists", }, }, } @@ -94,7 +94,7 @@ func TestSchemaSimpleErrorsGivenDuplicateSchemaInSameSDL(t *testing.T) { type Users {} type Users {} `, - ExpectedError: "schema type already exists", + ExpectedError: "collection already exists", }, }, } diff --git a/tests/integration/view/one_to_one/identical_schema_test.go b/tests/integration/view/one_to_one/identical_schema_test.go index fb82303134..90248ede17 100644 --- a/tests/integration/view/one_to_one/identical_schema_test.go +++ b/tests/integration/view/one_to_one/identical_schema_test.go @@ -8,7 +8,7 @@ // by the Apache License, Version 2.0, included in the file // licenses/APL.txt. -package one_to_many +package one_to_one import ( "testing" @@ -92,3 +92,55 @@ func TestView_OneToOneSameSchema(t *testing.T) { testUtils.ExecuteTestCase(t, test) } + +func TestView_OneToOneEmbeddedSchemaIsNotLostOnNextUpdate(t *testing.T) { + test := testUtils.TestCase{ + Description: "One to one view followed by GQL type update", + Actions: []any{ + testUtils.SchemaUpdate{ + Schema: ` + type Author { + name: String + books: [Book] + } + type Book { + name: String + author: Author + } + `, + }, + testUtils.CreateView{ + Query: ` + Author { + name + books { + name + } + } + `, + SDL: ` + type AuthorView { + name: String + books: [BookView] + } + interface BookView { + name: String + } + `, + }, + // After creating the view, update the system's types again and ensure + // that `BookView` is not forgotten. A GQL error would appear if this + // was broken as `AuthorView.books` would reference a type that does + // not exist. + testUtils.SchemaUpdate{ + Schema: ` + type User { + name: String + } + `, + }, + }, + } + + testUtils.ExecuteTestCase(t, test) +} diff --git a/tests/integration/view/one_to_one/simple_test_test.go b/tests/integration/view/one_to_one/simple_test_test.go new file mode 100644 index 0000000000..96967acf07 --- /dev/null +++ b/tests/integration/view/one_to_one/simple_test_test.go @@ -0,0 +1,94 @@ +// 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 one_to_one + +import ( + "testing" + + testUtils "github.com/sourcenetwork/defradb/tests/integration" +) + +func TestView_OneToOneDuplicateEmbeddedSchema_Errors(t *testing.T) { + test := testUtils.TestCase{ + Description: "One to one view and duplicate embedded schema", + Actions: []any{ + testUtils.SchemaUpdate{ + Schema: ` + type Author { + name: String + books: [Book] + } + type Book { + name: String + author: Author + } + `, + }, + testUtils.CreateView{ + Query: ` + Author { + name + books { + name + } + } + `, + SDL: ` + type AuthorView { + name: String + books: [BookView] + } + interface BookView { + name: String + } + `, + }, + // Try and create a second view that creates a new `BookView`, this + // should error as `BookView` has already been created by the first view. + testUtils.CreateView{ + Query: ` + Author { + authorName: name + books { + bookName: name + } + } + `, + SDL: ` + type AuthorAliasView { + authorName: String + books: [BookView] + } + interface BookView { + bookName: String + } + `, + ExpectedError: "schema type already exists. Name: BookView", + }, + testUtils.IntrospectionRequest{ + Request: ` + query { + __type (name: "BookView") { + name + } + } + `, + ExpectedData: map[string]any{ + "__type": map[string]any{ + "name": "BookView", + }, + }, + }, + }, + } + + testUtils.ExecuteTestCase(t, test) +} diff --git a/tests/integration/view/one_to_one/with_restart_test.go b/tests/integration/view/one_to_one/with_restart_test.go new file mode 100644 index 0000000000..a17886867f --- /dev/null +++ b/tests/integration/view/one_to_one/with_restart_test.go @@ -0,0 +1,88 @@ +// 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 one_to_one + +import ( + "testing" + + testUtils "github.com/sourcenetwork/defradb/tests/integration" +) + +func TestView_OneToOneEmbeddedSchemaIsNotLostORestart(t *testing.T) { + test := testUtils.TestCase{ + Description: "One to one view and restart", + Actions: []any{ + testUtils.SchemaUpdate{ + Schema: ` + type Author { + name: String + books: [Book] + } + type Book { + name: String + author: Author + } + `, + }, + testUtils.CreateView{ + Query: ` + Author { + name + books { + name + } + } + `, + SDL: ` + type AuthorView { + name: String + books: [BookView] + } + interface BookView { + name: String + } + `, + }, + // After creating the view, restart and ensure that `BookView` is not forgotten. + testUtils.Restart{}, + testUtils.IntrospectionRequest{ + Request: ` + query { + __type (name: "AuthorView") { + name + } + } + `, + ExpectedData: map[string]any{ + "__type": map[string]any{ + "name": "AuthorView", + }, + }, + }, + testUtils.IntrospectionRequest{ + Request: ` + query { + __type (name: "BookView") { + name + } + } + `, + ExpectedData: map[string]any{ + "__type": map[string]any{ + "name": "BookView", + }, + }, + }, + }, + } + + testUtils.ExecuteTestCase(t, test) +}