Skip to content

Commit

Permalink
SQUASHED - sourcenetwork#1957
Browse files Browse the repository at this point in the history
  • Loading branch information
AndrewSisley committed Oct 17, 2023
1 parent 25d4767 commit ba4bbda
Show file tree
Hide file tree
Showing 56 changed files with 343 additions and 620 deletions.
2 changes: 1 addition & 1 deletion client/db.go
Original file line number Diff line number Diff line change
Expand Up @@ -92,7 +92,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.
//
Expand Down
78 changes: 5 additions & 73 deletions db/collection.go
Original file line number Diff line number Diff line change
Expand Up @@ -190,18 +190,17 @@ func (db *db) createCollection(
return db.getCollectionByName(ctx, txn, desc.Name)
}

// updateCollection updates the persisted collection description matching the name of the given
// updateSchema updates the persisted schema description matching the name of the given
// description, to the values in the given description.
//
// It will validate the given description using [ValidateUpdateCollectionTxn] before updating it.
// It will validate the given description using [validateUpdateSchema] before updating it.
//
// The collection (including the schema version ID) will only be updated if any changes have actually
// The schema (including the schema version ID) will only be updated if any changes have actually
// been made, if the given description matches the current persisted description then no changes will be
// applied.
func (db *db) updateCollection(
func (db *db) updateSchema(
ctx context.Context,
txn datastore.Txn,
existingDescriptionsByName map[string]client.CollectionDescription,
existingSchemaByName map[string]client.SchemaDescription,
proposedDescriptionsByName map[string]client.SchemaDescription,
def client.CollectionDefinition,
Expand All @@ -210,12 +209,7 @@ func (db *db) updateCollection(
schema := def.Schema
desc := def.Description

hasChanged, err := db.validateUpdateCollection(ctx, existingDescriptionsByName, desc)
if err != nil {
return nil, err
}

hasSchemaChanged, err := db.validateUpdateSchema(
hasChanged, err := db.validateUpdateSchema(
ctx,
txn,
existingSchemaByName,
Expand All @@ -226,7 +220,6 @@ func (db *db) updateCollection(
return nil, err
}

hasChanged = hasChanged || hasSchemaChanged
if !hasChanged {
return db.getCollectionByName(ctx, txn, desc.Name)
}
Expand Down Expand Up @@ -304,32 +297,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
Expand All @@ -341,10 +308,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)
Expand Down Expand Up @@ -564,37 +527,6 @@ func validateUpdateSchemaFields(
return hasChanged, nil
}

func validateUpdateCollectionIndexes(
existingIndexes []client.IndexDescription,
proposedIndexes []client.IndexDescription,
) (bool, error) {
existingNameToIndex := map[string]client.IndexDescription{}
for _, index := range existingIndexes {
existingNameToIndex[index.Name] = index
}
for _, proposedIndex := range proposedIndexes {
if existingIndex, exists := existingNameToIndex[proposedIndex.Name]; exists {
if len(existingIndex.Fields) != len(proposedIndex.Fields) {
return false, ErrCanNotChangeIndexWithPatch
}
for i := range existingIndex.Fields {
if existingIndex.Fields[i] != proposedIndex.Fields[i] {
return false, ErrCanNotChangeIndexWithPatch
}
}
delete(existingNameToIndex, proposedIndex.Name)
} else {
return false, NewErrCannotAddIndexWithPatch(proposedIndex.Name)
}
}
if len(existingNameToIndex) > 0 {
for _, index := range existingNameToIndex {
return false, NewErrCannotDropIndexWithPatch(index.Name)
}
}
return false, nil
}

func (db *db) setDefaultSchemaVersion(
ctx context.Context,
txn datastore.Txn,
Expand Down
73 changes: 38 additions & 35 deletions db/schema.go
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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
}
Expand All @@ -128,28 +127,33 @@ 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 {
def := client.CollectionDefinition{Description: desc, Schema: desc.Schema}
for _, schema := range newSchemaByName {
if schema.Name == "" {
return ErrSchemaNameEmpty
}

collectionDescription, ok := collectionsByName[schema.Name]
if !ok {
return NewErrAddCollectionWithPatch(schema.Name)
}

def := client.CollectionDefinition{Description: collectionDescription, Schema: schema}
newCollections = append(newCollections, def)
newSchemaByName[def.Schema.Name] = def.Schema
}

for i, col := range newCollections {
col, err := db.updateCollection(
col, err := db.updateSchema(
ctx,
txn,
collectionsByName,
existingSchemaByName,
newSchemaByName,
col,
Expand Down Expand Up @@ -189,13 +193,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 {
fieldIndexesBySchema := make(map[string]map[string]int, len(schemaByName))
for schemaName, schema := range schemaByName {
fieldIndexesByName := make(map[string]int, len(schema.Fields))
fieldIndexesBySchema[schemaName] = fieldIndexesByName
for i, field := range schema.Fields {
fieldIndexesByName[field.Name] = i
}
}
Expand Down Expand Up @@ -238,9 +242,9 @@ 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 fieldIndexesByName, ok := fieldIndexesBySchema[desc.Name]; ok {
if i, ok := fieldIndexesByName[fieldIndexer]; ok {
index = fmt.Sprint(i)
}
Expand All @@ -249,7 +253,7 @@ func substituteSchemaPatch(
index = "-"
// If this is a new field we need to track its location so that subsequent operations
// within the patch may access it by field name.
fieldIndexesByCollection[desc.Name][fieldIndexer] = len(fieldIndexesByCollection[desc.Name])
fieldIndexesBySchema[desc.Name][fieldIndexer] = len(fieldIndexesBySchema[desc.Name])
}

splitPath[fieldIndexPathIndex] = index
Expand All @@ -261,17 +265,17 @@ func substituteSchemaPatch(

if isField {
if kind, isString := field["Kind"].(string); isString {
substitute, collectionName, err := getSubstituteFieldKind(kind, collectionsByName)
substitute, schemaName, err := getSubstituteFieldKind(kind, schemaByName)
if err != nil {
return nil, err
}

field["Kind"] = substitute
if collectionName != "" {
if field["Schema"] != nil && field["Schema"] != collectionName {
if schemaName != "" {
if field["Schema"] != nil && field["Schema"] != schemaName {
return nil, NewErrFieldKindDoesNotMatchFieldSchema(kind, field["Schema"].(string))
}
field["Schema"] = collectionName
field["Schema"] = schemaName
}

newPatchValue = immutable.Some[any](field)
Expand All @@ -284,7 +288,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
}
Expand Down Expand Up @@ -314,7 +318,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 {
Expand All @@ -330,7 +334,7 @@ func getSubstituteFieldKind(
substitute = client.FieldKind_FOREIGN_OBJECT
}

if _, substituteFound := collectionsByName[collectionName]; substituteFound {
if _, substituteFound := schemaByName[collectionName]; substituteFound {
return substitute, collectionName, nil
}

Expand All @@ -341,20 +345,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.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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} }
]
`,
},
Expand Down Expand Up @@ -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} }
]
`,
},
Expand Down Expand Up @@ -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} }
]
`,
},
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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} }
]
`,
},
Expand Down Expand Up @@ -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} }
]
`,
},
Expand Down
Loading

0 comments on commit ba4bbda

Please sign in to comment.