Remove collection from patchSchema

client/db.go

@@ -95,7 +95,7 @@ type Store interface {
 	// types previously defined.
 	AddSchema(context.Context, string) ([]CollectionDescription, error)
 
-	// PatchSchema takes the given JSON patch string and applies it to the set of CollectionDescriptions
+	// PatchSchema takes the given JSON patch string and applies it to the set of SchemaDescriptions
 	// present in the database. If true is provided, the new schema versions will be made default, otherwise
 	// [SetDefaultSchemaVersion] should be called to set them so.
 	//

db/collection.go

@@ -206,19 +206,13 @@ func (db *db) createCollection(
 func (db *db) updateCollection(
 	ctx context.Context,
 	txn datastore.Txn,
-	existingDescriptionsByName map[string]client.CollectionDescription,
 	existingSchemaByName map[string]client.SchemaDescription,
 	proposedDescriptionsByName map[string]client.SchemaDescription,
 	desc client.CollectionDescription,
 	schema client.SchemaDescription,
 	setAsDefaultVersion bool,
 ) (client.Collection, error) {
-	hasChanged, err := db.validateUpdateCollection(ctx, existingDescriptionsByName, desc)
-	if err != nil {
-		return nil, err
-	}
-
-	hasSchemaChanged, err := db.validateUpdateSchema(
+	hasChanged, err := db.validateUpdateSchema(
 		ctx,
 		txn,
 		existingSchemaByName,
@@ -229,7 +223,6 @@ func (db *db) updateCollection(
 		return nil, err
 	}
 
-	hasChanged = hasChanged || hasSchemaChanged
 	if !hasChanged {
 		return db.getCollectionByName(ctx, txn, desc.Name)
 	}
@@ -307,32 +300,6 @@ func (db *db) updateCollection(
 	return db.getCollectionByName(ctx, txn, desc.Name)
 }
 
-// validateUpdateCollection validates that the given collection description is a valid update.
-//
-// Will return true if the given description differs from the current persisted state of the
-// collection. Will return an error if it fails validation.
-func (db *db) validateUpdateCollection(
-	ctx context.Context,
-	existingDescriptionsByName map[string]client.CollectionDescription,
-	proposedDesc client.CollectionDescription,
-) (bool, error) {
-	if proposedDesc.Name == "" {
-		return false, ErrCollectionNameEmpty
-	}
-
-	existingDesc, collectionExists := existingDescriptionsByName[proposedDesc.Name]
-	if !collectionExists {
-		return false, NewErrAddCollectionWithPatch(proposedDesc.Name)
-	}
-
-	if proposedDesc.ID != existingDesc.ID {
-		return false, NewErrCollectionIDDoesntMatch(proposedDesc.Name, existingDesc.ID, proposedDesc.ID)
-	}
-
-	hasChangedIndexes, err := validateUpdateCollectionIndexes(existingDesc.Indexes, proposedDesc.Indexes)
-	return hasChangedIndexes, err
-}
-
 // validateUpdateSchema validates that the given schema description is a valid update.
 //
 // Will return true if the given description differs from the current persisted state of the
@@ -344,10 +311,6 @@ func (db *db) validateUpdateSchema(
 	proposedDescriptionsByName map[string]client.SchemaDescription,
 	proposedDesc client.SchemaDescription,
 ) (bool, error) {
-	if proposedDesc.Name == "" {
-		return false, ErrSchemaNameEmpty
-	}
-
 	existingDesc, collectionExists := existingDescriptionsByName[proposedDesc.Name]
 	if !collectionExists {
 		return false, NewErrAddCollectionWithPatch(proposedDesc.Name)

db/schema.go

@@ -27,9 +27,8 @@ import (
 
 const (
 	schemaNamePathIndex int = 0
-	schemaPathIndex     int = 1
-	fieldsPathIndex     int = 2
-	fieldIndexPathIndex int = 3
+	fieldsPathIndex     int = 1
+	fieldIndexPathIndex int = 2
 )
 
 // addSchema takes the provided schema in SDL format, and applies it to the database,
@@ -85,7 +84,7 @@ func (db *db) loadSchema(ctx context.Context, txn datastore.Txn) error {
 	return db.parser.SetSchema(ctx, txn, definitions)
 }
 
-// patchSchema takes the given JSON patch string and applies it to the set of CollectionDescriptions
+// patchSchema takes the given JSON patch string and applies it to the set of SchemaDescriptions
 // present in the database.
 //
 // It will also update the GQL types used by the query system. It will error and not apply any of the
@@ -113,12 +112,12 @@ func (db *db) patchSchema(ctx context.Context, txn datastore.Txn, patchString st
 	}
 
 	// Here we swap out any string representations of enums for their integer values
-	patch, err = substituteSchemaPatch(patch, collectionsByName)
+	patch, err = substituteSchemaPatch(patch, existingSchemaByName)
 	if err != nil {
 		return err
 	}
 
-	existingDescriptionJson, err := json.Marshal(collectionsByName)
+	existingDescriptionJson, err := json.Marshal(existingSchemaByName)
 	if err != nil {
 		return err
 	}
@@ -128,31 +127,37 @@ func (db *db) patchSchema(ctx context.Context, txn datastore.Txn, patchString st
 		return err
 	}
 
-	var newDescriptionsByName map[string]client.CollectionDescription
+	var newSchemaByName map[string]client.SchemaDescription
 	decoder := json.NewDecoder(strings.NewReader(string(newDescriptionJson)))
 	decoder.DisallowUnknownFields()
-	err = decoder.Decode(&newDescriptionsByName)
+	err = decoder.Decode(&newSchemaByName)
 	if err != nil {
 		return err
 	}
 
 	newCollections := []client.CollectionDefinition{}
-	newSchemaByName := map[string]client.SchemaDescription{}
-	for _, desc := range newDescriptionsByName {
-		col, err := db.newCollection(desc, desc.Schema)
+	for _, schema := range newSchemaByName {
+		if schema.Name == "" {
+			return ErrSchemaNameEmpty
+		}
+
+		collectionDescription, ok := collectionsByName[schema.Name]
+		if !ok {
+			return NewErrAddCollectionWithPatch(schema.Name)
+		}
+
+		col, err := db.newCollection(collectionDescription, schema)
 		if err != nil {
 			return err
 		}
 
 		newCollections = append(newCollections, col)
-		newSchemaByName[col.schema.Name] = col.schema
 	}
 
 	for i, col := range newCollections {
 		col, err := db.updateCollection(
 			ctx,
 			txn,
-			collectionsByName,
 			existingSchemaByName,
 			newSchemaByName,
 			col.Description(),
@@ -193,13 +198,13 @@ func (db *db) getCollectionsByName(
 // value.
 func substituteSchemaPatch(
 	patch jsonpatch.Patch,
-	collectionsByName map[string]client.CollectionDescription,
+	schemaByName map[string]client.SchemaDescription,
 ) (jsonpatch.Patch, error) {
-	fieldIndexesByCollection := make(map[string]map[string]int, len(collectionsByName))
-	for colName, col := range collectionsByName {
-		fieldIndexesByName := make(map[string]int, len(col.Schema.Fields))
-		fieldIndexesByCollection[colName] = fieldIndexesByName
-		for i, field := range col.Schema.Fields {
+	fieldIndexesByCollection := make(map[string]map[string]int, len(schemaByName))
+	for schemaName, schema := range schemaByName {
+		fieldIndexesByName := make(map[string]int, len(schema.Fields))
+		fieldIndexesByCollection[schemaName] = fieldIndexesByName
+		for i, field := range schema.Fields {
 			fieldIndexesByName[field.Name] = i
 		}
 	}
@@ -242,7 +247,7 @@ func substituteSchemaPatch(
 						newPatchValue = immutable.Some[any](field)
 					}
 
-					desc := collectionsByName[splitPath[schemaNamePathIndex]]
+					desc := schemaByName[splitPath[schemaNamePathIndex]]
 					var index string
 					if fieldIndexesByName, ok := fieldIndexesByCollection[desc.Name]; ok {
 						if i, ok := fieldIndexesByName[fieldIndexer]; ok {
@@ -265,7 +270,7 @@ func substituteSchemaPatch(
 
 			if isField {
 				if kind, isString := field["Kind"].(string); isString {
-					substitute, collectionName, err := getSubstituteFieldKind(kind, collectionsByName)
+					substitute, collectionName, err := getSubstituteFieldKind(kind, schemaByName)
 					if err != nil {
 						return nil, err
 					}
@@ -288,7 +293,7 @@ func substituteSchemaPatch(
 				}
 
 				if kind, isString := kind.(string); isString {
-					substitute, _, err := getSubstituteFieldKind(kind, collectionsByName)
+					substitute, _, err := getSubstituteFieldKind(kind, schemaByName)
 					if err != nil {
 						return nil, err
 					}
@@ -318,7 +323,7 @@ func substituteSchemaPatch(
 // If the value represents a foreign relation the collection name will also be returned.
 func getSubstituteFieldKind(
 	kind string,
-	collectionsByName map[string]client.CollectionDescription,
+	schemaByName map[string]client.SchemaDescription,
 ) (client.FieldKind, string, error) {
 	substitute, substituteFound := client.FieldKindStringToEnumMapping[kind]
 	if substituteFound {
@@ -334,7 +339,7 @@ func getSubstituteFieldKind(
 			substitute = client.FieldKind_FOREIGN_OBJECT
 		}
 
-		if _, substituteFound := collectionsByName[collectionName]; substituteFound {
+		if _, substituteFound := schemaByName[collectionName]; substituteFound {
 			return substitute, collectionName, nil
 		}
 
@@ -345,20 +350,19 @@ func getSubstituteFieldKind(
 // isFieldOrInner returns true if the given path points to a FieldDescription or a property within it.
 func isFieldOrInner(path []string) bool {
 	//nolint:goconst
-	return len(path) >= 4 && path[fieldsPathIndex] == "Fields" && path[schemaPathIndex] == "Schema"
+	return len(path) >= 3 && path[fieldsPathIndex] == "Fields"
 }
 
 // isField returns true if the given path points to a FieldDescription.
 func isField(path []string) bool {
-	return len(path) == 4 && path[fieldsPathIndex] == "Fields" && path[schemaPathIndex] == "Schema"
+	return len(path) == 3 && path[fieldsPathIndex] == "Fields"
 }
 
 // isField returns true if the given path points to a FieldDescription.Kind property.
 func isFieldKind(path []string) bool {
-	return len(path) == 5 &&
+	return len(path) == 4 &&
 		path[fieldIndexPathIndex+1] == "Kind" &&
-		path[fieldsPathIndex] == "Fields" &&
-		path[schemaPathIndex] == "Schema"
+		path[fieldsPathIndex] == "Fields"
 }
 
 // containsLetter returns true if the string contains a single unicode character.

tests/integration/net/state/simple/peer/with_create_add_field_test.go

@@ -35,7 +35,7 @@ func TestP2PPeerCreateWithNewFieldSyncsDocsToOlderSchemaVersion(t *testing.T) {
 				NodeID: immutable.Some(0),
 				Patch: `
 					[
-						{ "op": "add", "path": "/Users/Schema/Fields/-", "value": {"Name": "Email", "Kind": 11} }
+						{ "op": "add", "path": "/Users/Fields/-", "value": {"Name": "Email", "Kind": 11} }
 					]
 				`,
 			},
@@ -108,7 +108,7 @@ func TestP2PPeerCreateWithNewFieldSyncsDocsToNewerSchemaVersion(t *testing.T) {
 				NodeID: immutable.Some(1),
 				Patch: `
 					[
-						{ "op": "add", "path": "/Users/Schema/Fields/-", "value": {"Name": "Email", "Kind": 11} }
+						{ "op": "add", "path": "/Users/Fields/-", "value": {"Name": "Email", "Kind": 11} }
 					]
 				`,
 			},
@@ -164,7 +164,7 @@ func TestP2PPeerCreateWithNewFieldSyncsDocsToUpdatedSchemaVersion(t *testing.T)
 				// Patch the schema on all nodes
 				Patch: `
 					[
-						{ "op": "add", "path": "/Users/Schema/Fields/-", "value": {"Name": "Email", "Kind": 11} }
+						{ "op": "add", "path": "/Users/Fields/-", "value": {"Name": "Email", "Kind": 11} }
 					]
 				`,
 			},

tests/integration/net/state/simple/peer/with_update_add_field_test.go

@@ -48,7 +48,7 @@ func TestP2PPeerUpdateWithNewFieldSyncsDocsToOlderSchemaVersionMultistep(t *test
 				NodeID: immutable.Some(0),
 				Patch: `
 					[
-						{ "op": "add", "path": "/Users/Schema/Fields/-", "value": {"Name": "Email", "Kind": 11} }
+						{ "op": "add", "path": "/Users/Fields/-", "value": {"Name": "Email", "Kind": 11} }
 					]
 				`,
 			},
@@ -133,7 +133,7 @@ func TestP2PPeerUpdateWithNewFieldSyncsDocsToOlderSchemaVersion(t *testing.T) {
 				NodeID: immutable.Some(0),
 				Patch: `
 					[
-						{ "op": "add", "path": "/Users/Schema/Fields/-", "value": {"Name": "Email", "Kind": 11} }
+						{ "op": "add", "path": "/Users/Fields/-", "value": {"Name": "Email", "Kind": 11} }
 					]
 				`,
 			},

tests/integration/net/state/simple/replicator/with_create_add_field_test.go

@@ -35,7 +35,7 @@ func TestP2POneToOneReplicatorCreateWithNewFieldSyncsDocsToOlderSchemaVersion(t
 				NodeID: immutable.Some(0),
 				Patch: `
 					[
-						{ "op": "add", "path": "/Users/Schema/Fields/-", "value": {"Name": "Email", "Kind": 11} }
+						{ "op": "add", "path": "/Users/Fields/-", "value": {"Name": "Email", "Kind": 11} }
 					]
 				`,
 			},
@@ -87,7 +87,7 @@ func TestP2POneToOneReplicatorCreateWithNewFieldSyncsDocsToNewerSchemaVersion(t
 				NodeID: immutable.Some(1),
 				Patch: `
 					[
-						{ "op": "add", "path": "/Users/Schema/Fields/-", "value": {"Name": "Email", "Kind": 11} }
+						{ "op": "add", "path": "/Users/Fields/-", "value": {"Name": "Email", "Kind": 11} }
 					]
 				`,
 			},
@@ -137,7 +137,7 @@ func TestP2POneToOneReplicatorCreateWithNewFieldSyncsDocsToUpdatedSchemaVersion(
 				// Patch the schema on all nodes
 				Patch: `
 					[
-						{ "op": "add", "path": "/Users/Schema/Fields/-", "value": {"Name": "Email", "Kind": 11} }
+						{ "op": "add", "path": "/Users/Fields/-", "value": {"Name": "Email", "Kind": 11} }
 					]
 				`,
 			},

tests/integration/net/state/simple/replicator/with_update_add_field_test.go

@@ -44,7 +44,7 @@ func TestP2PReplicatorUpdateWithNewFieldSyncsDocsToOlderSchemaVersionMultistep(t
 				NodeID: immutable.Some(0),
 				Patch: `
 					[
-						{ "op": "add", "path": "/Users/Schema/Fields/-", "value": {"Name": "Email", "Kind": 11} }
+						{ "op": "add", "path": "/Users/Fields/-", "value": {"Name": "Email", "Kind": 11} }
 					]
 				`,
 			},
@@ -125,7 +125,7 @@ func TestP2PReplicatorUpdateWithNewFieldSyncsDocsToOlderSchemaVersion(t *testing
 				NodeID: immutable.Some(0),
 				Patch: `
 					[
-						{ "op": "add", "path": "/Users/Schema/Fields/-", "value": {"Name": "Email", "Kind": 11} }
+						{ "op": "add", "path": "/Users/Fields/-", "value": {"Name": "Email", "Kind": 11} }
 					]
 				`,
 			},