From 61964822b70ab22ac3fa9f4734c091ab8141dc4f Mon Sep 17 00:00:00 2001 From: Andrew Sisley Date: Fri, 14 Jun 2024 17:25:51 -0400 Subject: [PATCH] PR FIXUP - Remove CollectionAlreadyExists check from col_define.go Is now handled by the (existing) validation rules. --- internal/db/collection_define.go | 29 +++++++++----------------- internal/db/definition_validation.go | 30 ++++++++++++++++++++------- internal/db/view.go | 31 ++-------------------------- 3 files changed, 34 insertions(+), 56 deletions(-) diff --git a/internal/db/collection_define.go b/internal/db/collection_define.go index 53160eb324..dcb7301e43 100644 --- a/internal/db/collection_define.go +++ b/internal/db/collection_define.go @@ -56,24 +56,10 @@ func (db *db) createCollections( } for _, def := range newDefinitions { - desc := def.Description - - if desc.Name.HasValue() { - exists, err := description.HasCollectionByName(ctx, txn, desc.Name.Value()) - if err != nil { - return nil, err - } - if exists { - return nil, ErrCollectionAlreadyExists - } - } - - definitionsByName := map[string]client.CollectionDefinition{} - for _, existingDefinition := range existingDefinitions { - definitionsByName[existingDefinition.GetName()] = existingDefinition - } - for _, newDefinition := range newDefinitions { - definitionsByName[newDefinition.GetName()] = newDefinition + if len(def.Description.Fields) == 0 { + // This is a schema-only definition, we should not create a collection for it + returnDescriptions = append(returnDescriptions, def) + continue } colSeq, err := db.getSequence(ctx, core.CollectionIDSequenceKey{}) @@ -90,6 +76,7 @@ func (db *db) createCollections( return nil, err } + desc := def.Description desc.ID = uint32(colID) desc.RootID = desc.ID @@ -115,7 +102,11 @@ func (db *db) createCollections( } } - err = db.validateNewCollection(ctx, definitionsByName) + err = db.validateNewCollection( + ctx, + append(newDefinitions, existingDefinitions...), + existingDefinitions, + ) if err != nil { return nil, err } diff --git a/internal/db/definition_validation.go b/internal/db/definition_validation.go index 158064997e..08e6e603a7 100644 --- a/internal/db/definition_validation.go +++ b/internal/db/definition_validation.go @@ -163,23 +163,37 @@ func (db *db) validateCollectionChanges( func (db *db) validateNewCollection( ctx context.Context, - newDefsByName map[string]client.CollectionDefinition, + newDefinitions []client.CollectionDefinition, + oldDefinitions []client.CollectionDefinition, ) error { - collections := []client.CollectionDescription{} - schemasByID := map[string]client.SchemaDescription{} + newCollections := []client.CollectionDescription{} + newSchemasByID := map[string]client.SchemaDescription{} - for _, def := range newDefsByName { + for _, def := range newDefinitions { if len(def.Description.Fields) != 0 { - collections = append(collections, def.Description) + newCollections = append(newCollections, def.Description) } - schemasByID[def.Schema.VersionID] = def.Schema + newSchemasByID[def.Schema.VersionID] = def.Schema } - newState := newDefinitionState(collections, schemasByID) + newState := newDefinitionState(newCollections, newSchemasByID) + + oldCollections := []client.CollectionDescription{} + oldSchemasByID := map[string]client.SchemaDescription{} + + for _, def := range oldDefinitions { + if len(def.Description.Fields) != 0 { + oldCollections = append(oldCollections, def.Description) + } + + oldSchemasByID[def.Schema.VersionID] = def.Schema + } + + oldState := newDefinitionState(oldCollections, oldSchemasByID) for _, validator := range createValidators { - err := validator(ctx, db, newState, &definitionState{}) + err := validator(ctx, db, newState, oldState) if err != nil { return err } diff --git a/internal/db/view.go b/internal/db/view.go index 190f365b6f..2664dd4a57 100644 --- a/internal/db/view.go +++ b/internal/db/view.go @@ -20,7 +20,6 @@ import ( "github.com/sourcenetwork/defradb/client" "github.com/sourcenetwork/defradb/client/request" - "github.com/sourcenetwork/defradb/internal/db/description" ) func (db *db) addView( @@ -29,8 +28,6 @@ func (db *db) addView( sdl string, transform immutable.Option[model.Lens], ) ([]client.CollectionDefinition, error) { - txn := mustGetContextTxn(ctx) - // Wrap the given query as part of the GQL query object - this simplifies the syntax for users // and ensures that we can't be given mutations. In the future this line should disappear along // with the all calls to the parser appart from `ParseSDL` when we implement the DQL stuff. @@ -68,36 +65,12 @@ func (db *db) addView( newDefinitions[i].Description.Sources = append(newDefinitions[i].Description.Sources, &source) } - returnDescriptions := make([]client.CollectionDefinition, 0, len(newDefinitions)) - collectionDefinitions := []client.CollectionDefinition{} - schemaOnlyDefinitions := []client.SchemaDescription{} - - for _, definition := range newDefinitions { - if definition.Description.Name.HasValue() { - collectionDefinitions = append(collectionDefinitions, definition) - } else { - schemaOnlyDefinitions = append(schemaOnlyDefinitions, definition.Schema) - } - } - - for _, schema := range schemaOnlyDefinitions { - schema, err := description.CreateSchemaVersion(ctx, txn, schema) - if err != nil { - return nil, err - } - returnDescriptions = append(returnDescriptions, client.CollectionDefinition{ - // `Collection` is left as default for embedded types - Schema: schema, - }) - } - - returnColDescriptions, err := db.createCollections(ctx, collectionDefinitions) + returnDescriptions, err := db.createCollections(ctx, newDefinitions) if err != nil { return nil, err } - returnDescriptions = append(returnDescriptions, returnColDescriptions...) - for _, definition := range returnColDescriptions { + for _, definition := range returnDescriptions { for _, source := range definition.Description.QuerySources() { if source.Transform.HasValue() { err = db.LensRegistry().SetMigration(ctx, definition.Description.ID, source.Transform.Value())