fix(i): Retain embedded schema within gql types on reload (sourcenetw…

…ork#2185)

## Relevant issue(s)

Resolves sourcenetwork#2184 sourcenetwork#2182

## Description

Retain embedded schema within gql types on reload and further GQL
modification.

Also refactors the code a little to make it harder to re-introduce
(calling `loadSchema` instead of sometimes relying on function vars).

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

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
+}

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

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
 }

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",
 			},
 		},
 	}

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)
+}

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)
+}