From 20f96db224702706fbfe8d10eeebe8c2955c4aae Mon Sep 17 00:00:00 2001 From: Aaron Craelius Date: Thu, 4 Jul 2024 21:46:42 +0200 Subject: [PATCH] feat(schema)!: updates based on postgres testing (#20858) --- schema/enum.go | 31 +++++++++++ schema/field.go | 3 +- schema/fields.go | 8 +-- schema/fields_test.go | 4 +- schema/kind.go | 38 +++++++++---- schema/kind_test.go | 104 ++++++++++++++++++----------------- schema/module_schema.go | 47 +--------------- schema/module_schema_test.go | 33 +++++++++++ schema/object_type.go | 25 ++++++++- schema/object_type_test.go | 41 ++++++++++++++ 10 files changed, 217 insertions(+), 117 deletions(-) diff --git a/schema/enum.go b/schema/enum.go index 0ed710af37..6e0be7c615 100644 --- a/schema/enum.go +++ b/schema/enum.go @@ -5,6 +5,9 @@ import "fmt" // EnumDefinition represents the definition of an enum type. type EnumDefinition struct { // Name is the name of the enum type. It must conform to the NameFormat regular expression. + // Its name must be unique between all enum types and object types in the module. + // The same enum, however, can be used in multiple object types and fields as long as the + // definition is identical each time Name string // Values is a list of distinct, non-empty values that are part of the enum type. @@ -44,3 +47,31 @@ func (e EnumDefinition) ValidateValue(value string) error { } return fmt.Errorf("value %q is not a valid enum value for %s", value, e.Name) } + +// checkEnumCompatibility checks that the enum values are consistent across object types and fields. +func checkEnumCompatibility(enumValueMap map[string]map[string]bool, field Field) error { + if field.Kind != EnumKind { + return nil + } + + enum := field.EnumDefinition + + if existing, ok := enumValueMap[enum.Name]; ok { + if len(existing) != len(enum.Values) { + return fmt.Errorf("enum %q has different number of values in different object types", enum.Name) + } + + for _, value := range enum.Values { + if !existing[value] { + return fmt.Errorf("enum %q has different values in different object types", enum.Name) + } + } + } else { + valueMap := map[string]bool{} + for _, value := range enum.Values { + valueMap[value] = true + } + enumValueMap[enum.Name] = valueMap + } + return nil +} diff --git a/schema/field.go b/schema/field.go index a3b6ea2784..57f34b9cfa 100644 --- a/schema/field.go +++ b/schema/field.go @@ -10,10 +10,11 @@ type Field struct { // Kind is the basic type of the field. Kind Kind - // Nullable indicates whether null values are accepted for the field. + // Nullable indicates whether null values are accepted for the field. Key fields CANNOT be nullable. Nullable bool // AddressPrefix is the address prefix of the field's kind, currently only used for Bech32AddressKind. + // TODO: in a future update, stricter criteria and validation for address prefixes should be added AddressPrefix string // EnumDefinition is the definition of the enum type and is only valid when Kind is EnumKind. diff --git a/schema/fields.go b/schema/fields.go index 1500d40bfa..be08ca66ef 100644 --- a/schema/fields.go +++ b/schema/fields.go @@ -2,15 +2,15 @@ package schema import "fmt" -// ValidateForKeyFields validates that the value conforms to the set of fields as a Key in an ObjectUpdate. +// ValidateObjectKey validates that the value conforms to the set of fields as a Key in an ObjectUpdate. // See ObjectUpdate.Key for documentation on the requirements of such keys. -func ValidateForKeyFields(keyFields []Field, value interface{}) error { +func ValidateObjectKey(keyFields []Field, value interface{}) error { return validateFieldsValue(keyFields, value) } -// ValidateForValueFields validates that the value conforms to the set of fields as a Value in an ObjectUpdate. +// ValidateObjectValue validates that the value conforms to the set of fields as a Value in an ObjectUpdate. // See ObjectUpdate.Value for documentation on the requirements of such values. -func ValidateForValueFields(valueFields []Field, value interface{}) error { +func ValidateObjectValue(valueFields []Field, value interface{}) error { valueUpdates, ok := value.(ValueUpdates) if !ok { return validateFieldsValue(valueFields, value) diff --git a/schema/fields_test.go b/schema/fields_test.go index aa22e02a14..befa968657 100644 --- a/schema/fields_test.go +++ b/schema/fields_test.go @@ -56,7 +56,7 @@ func TestValidateForKeyFields(t *testing.T) { for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { - err := ValidateForKeyFields(tt.keyFields, tt.key) + err := ValidateObjectKey(tt.keyFields, tt.key) if tt.errContains == "" { if err != nil { t.Fatalf("unexpected error: %v", err) @@ -128,7 +128,7 @@ func TestValidateForValueFields(t *testing.T) { for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { - err := ValidateForValueFields(tt.valueFields, tt.value) + err := ValidateObjectValue(tt.valueFields, tt.value) if tt.errContains == "" { if err != nil { t.Fatalf("unexpected error: %v", err) diff --git a/schema/kind.go b/schema/kind.go index 91602b0bd2..c3fefad52c 100644 --- a/schema/kind.go +++ b/schema/kind.go @@ -5,6 +5,7 @@ import ( "fmt" "regexp" "time" + "unicode/utf8" ) // Kind represents the basic type of a field in an object. @@ -16,7 +17,8 @@ const ( // InvalidKind indicates that an invalid type. InvalidKind Kind = iota - // StringKind is a string type and values of this type must be of the go type string. + // StringKind is a string type and values of this type must be of the go type string + // containing valid UTF-8 and cannot contain null characters. StringKind // BytesKind is a bytes type and values of this type must be of the go type []byte. @@ -46,14 +48,14 @@ const ( // Uint64Kind is a uint64 type and values of this type must be of the go type uint64. Uint64Kind - // IntegerKind represents an arbitrary precision integer number. Values of this type must + // IntegerStringKind represents an arbitrary precision integer number. Values of this type must // be of the go type string and formatted as base10 integers, specifically matching to // the IntegerFormat regex. - IntegerKind + IntegerStringKind - // DecimalKind represents an arbitrary precision decimal or integer number. Values of this type + // DecimalStringKind represents an arbitrary precision decimal or integer number. Values of this type // must be of the go type string and match the DecimalFormat regex. - DecimalKind + DecimalStringKind // BoolKind is a boolean type and values of this type must be of the go type bool. BoolKind @@ -134,9 +136,9 @@ func (t Kind) String() string { return "int64" case Uint64Kind: return "uint64" - case DecimalKind: + case DecimalStringKind: return "decimal" - case IntegerKind: + case IntegerStringKind: return "integer" case BoolKind: return "bool" @@ -216,13 +218,13 @@ func (t Kind) ValidateValueType(value interface{}) error { if !ok { return fmt.Errorf("expected uint64, got %T", value) } - case IntegerKind: + case IntegerStringKind: _, ok := value.(string) if !ok { return fmt.Errorf("expected string, got %T", value) } - case DecimalKind: + case DecimalStringKind: _, ok := value.(string) if !ok { return fmt.Errorf("expected string, got %T", value) @@ -283,11 +285,23 @@ func (t Kind) ValidateValue(value interface{}) error { } switch t { - case IntegerKind: + case StringKind: + str := value.(string) + if !utf8.ValidString(str) { + return fmt.Errorf("expected valid utf-8 string, got %s", value) + } + + // check for null characters + for _, r := range str { + if r == 0 { + return fmt.Errorf("expected string without null characters, got %s", value) + } + } + case IntegerStringKind: if !integerRegex.Match([]byte(value.(string))) { return fmt.Errorf("expected base10 integer, got %s", value) } - case DecimalKind: + case DecimalStringKind: if !decimalRegex.Match([]byte(value.(string))) { return fmt.Errorf("expected decimal number, got %s", value) } @@ -307,7 +321,7 @@ var ( ) // KindForGoValue finds the simplest kind that can represent the given go value. It will not, however, -// return kinds such as IntegerKind, DecimalKind, Bech32AddressKind, or EnumKind which all can be +// return kinds such as IntegerStringKind, DecimalStringKind, Bech32AddressKind, or EnumKind which all can be // represented as strings. func KindForGoValue(value interface{}) Kind { switch value.(type) { diff --git a/schema/kind_test.go b/schema/kind_test.go index c287bb61aa..49799fe495 100644 --- a/schema/kind_test.go +++ b/schema/kind_test.go @@ -53,12 +53,12 @@ func TestKind_ValidateValueType(t *testing.T) { {kind: Int64Kind, value: int32(1), valid: false}, {kind: Uint64Kind, value: uint64(1), valid: true}, {kind: Uint64Kind, value: uint32(1), valid: false}, - {kind: IntegerKind, value: "1", valid: true}, - {kind: IntegerKind, value: int32(1), valid: false}, - {kind: DecimalKind, value: "1.0", valid: true}, - {kind: DecimalKind, value: "1", valid: true}, - {kind: DecimalKind, value: "1.1e4", valid: true}, - {kind: DecimalKind, value: int32(1), valid: false}, + {kind: IntegerStringKind, value: "1", valid: true}, + {kind: IntegerStringKind, value: int32(1), valid: false}, + {kind: DecimalStringKind, value: "1.0", valid: true}, + {kind: DecimalStringKind, value: "1", valid: true}, + {kind: DecimalStringKind, value: "1.1e4", valid: true}, + {kind: DecimalStringKind, value: int32(1), valid: false}, {kind: Bech32AddressKind, value: []byte("hello"), valid: true}, {kind: Bech32AddressKind, value: 1, valid: false}, {kind: BoolKind, value: true, valid: true}, @@ -110,55 +110,59 @@ func TestKind_ValidateValue(t *testing.T) { {Int64Kind, int64(1), true}, {Int32Kind, "abc", false}, {BytesKind, nil, false}, + // string must be valid UTF-8 + {StringKind, string([]byte{0xff, 0xfe, 0xfd}), false}, + // strings with null characters are invalid + {StringKind, string([]byte{1, 2, 0, 3}), false}, // check integer, decimal and json more thoroughly - {IntegerKind, "1", true}, - {IntegerKind, "0", true}, - {IntegerKind, "10", true}, - {IntegerKind, "-100", true}, - {IntegerKind, "1.0", false}, - {IntegerKind, "00", true}, // leading zeros are allowed - {IntegerKind, "001", true}, - {IntegerKind, "-01", true}, + {IntegerStringKind, "1", true}, + {IntegerStringKind, "0", true}, + {IntegerStringKind, "10", true}, + {IntegerStringKind, "-100", true}, + {IntegerStringKind, "1.0", false}, + {IntegerStringKind, "00", true}, // leading zeros are allowed + {IntegerStringKind, "001", true}, + {IntegerStringKind, "-01", true}, // 100 digits - {IntegerKind, "1000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000", true}, + {IntegerStringKind, "1000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000", true}, // more than 100 digits - {IntegerKind, "10000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000", false}, - {IntegerKind, "", false}, - {IntegerKind, "abc", false}, - {IntegerKind, "abc100", false}, - {DecimalKind, "1.0", true}, - {DecimalKind, "0.0", true}, - {DecimalKind, "-100.075", true}, - {DecimalKind, "1002346.000", true}, - {DecimalKind, "0", true}, - {DecimalKind, "10", true}, - {DecimalKind, "-100", true}, - {DecimalKind, "1", true}, - {DecimalKind, "1.0e4", true}, - {DecimalKind, "1.0e-4", true}, - {DecimalKind, "1.0e+4", true}, - {DecimalKind, "1.0e", false}, - {DecimalKind, "1.0e4.0", false}, - {DecimalKind, "1.0e-4.0", false}, - {DecimalKind, "1.0e+4.0", false}, - {DecimalKind, "-1.0e-4", true}, - {DecimalKind, "-1.0e+4", true}, - {DecimalKind, "-1.0E4", true}, - {DecimalKind, "1E-9", true}, - {DecimalKind, "1E-99", true}, - {DecimalKind, "1E+9", true}, - {DecimalKind, "1E+99", true}, + {IntegerStringKind, "10000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000", false}, + {IntegerStringKind, "", false}, + {IntegerStringKind, "abc", false}, + {IntegerStringKind, "abc100", false}, + {DecimalStringKind, "1.0", true}, + {DecimalStringKind, "0.0", true}, + {DecimalStringKind, "-100.075", true}, + {DecimalStringKind, "1002346.000", true}, + {DecimalStringKind, "0", true}, + {DecimalStringKind, "10", true}, + {DecimalStringKind, "-100", true}, + {DecimalStringKind, "1", true}, + {DecimalStringKind, "1.0e4", true}, + {DecimalStringKind, "1.0e-4", true}, + {DecimalStringKind, "1.0e+4", true}, + {DecimalStringKind, "1.0e", false}, + {DecimalStringKind, "1.0e4.0", false}, + {DecimalStringKind, "1.0e-4.0", false}, + {DecimalStringKind, "1.0e+4.0", false}, + {DecimalStringKind, "-1.0e-4", true}, + {DecimalStringKind, "-1.0e+4", true}, + {DecimalStringKind, "-1.0E4", true}, + {DecimalStringKind, "1E-9", true}, + {DecimalStringKind, "1E-99", true}, + {DecimalStringKind, "1E+9", true}, + {DecimalStringKind, "1E+99", true}, // 50 digits before and after the decimal point - {DecimalKind, "10000000000000000000000000000000000000000000000000.10000000000000000000000000000000000000000000000001", true}, + {DecimalStringKind, "10000000000000000000000000000000000000000000000000.10000000000000000000000000000000000000000000000001", true}, // too many digits before the decimal point - {DecimalKind, "10000000000000000000000000000000000000000000000000000000000000000000000000", false}, + {DecimalStringKind, "10000000000000000000000000000000000000000000000000000000000000000000000000", false}, // too many digits after the decimal point - {DecimalKind, "1.0000000000000000000000000000000000000000000000000000000000000000000000001", false}, + {DecimalStringKind, "1.0000000000000000000000000000000000000000000000000000000000000000000000001", false}, // exponent too big - {DecimalKind, "1E-999", false}, - {DecimalKind, "", false}, - {DecimalKind, "abc", false}, - {DecimalKind, "abc", false}, + {DecimalStringKind, "1E-999", false}, + {DecimalStringKind, "", false}, + {DecimalStringKind, "abc", false}, + {DecimalStringKind, "abc", false}, {JSONKind, json.RawMessage(`{"a":10}`), true}, {JSONKind, json.RawMessage("10"), true}, {JSONKind, json.RawMessage("10.0"), true}, @@ -200,8 +204,8 @@ func TestKind_String(t *testing.T) { {Uint32Kind, "uint32"}, {Int64Kind, "int64"}, {Uint64Kind, "uint64"}, - {IntegerKind, "integer"}, - {DecimalKind, "decimal"}, + {IntegerStringKind, "integer"}, + {DecimalStringKind, "decimal"}, {BoolKind, "bool"}, {TimeKind, "time"}, {DurationKind, "duration"}, diff --git a/schema/module_schema.go b/schema/module_schema.go index 170288b650..9412c4456c 100644 --- a/schema/module_schema.go +++ b/schema/module_schema.go @@ -10,56 +10,13 @@ type ModuleSchema struct { // Validate validates the module schema. func (s ModuleSchema) Validate() error { + enumValueMap := map[string]map[string]bool{} for _, objType := range s.ObjectTypes { - if err := objType.Validate(); err != nil { + if err := objType.validate(enumValueMap); err != nil { return err } } - // validate that shared enum types are consistent across object types - enumValueMap := map[string]map[string]bool{} - for _, objType := range s.ObjectTypes { - for _, field := range objType.KeyFields { - err := checkEnum(enumValueMap, field) - if err != nil { - return err - } - } - for _, field := range objType.ValueFields { - err := checkEnum(enumValueMap, field) - if err != nil { - return err - } - } - } - - return nil -} - -func checkEnum(enumValueMap map[string]map[string]bool, field Field) error { - if field.Kind != EnumKind { - return nil - } - - enum := field.EnumDefinition - - if existing, ok := enumValueMap[enum.Name]; ok { - if len(existing) != len(enum.Values) { - return fmt.Errorf("enum %q has different number of values in different object types", enum.Name) - } - - for _, value := range enum.Values { - if !existing[value] { - return fmt.Errorf("enum %q has different values in different object types", enum.Name) - } - } - } else { - valueMap := map[string]bool{} - for _, value := range enum.Values { - valueMap[value] = true - } - enumValueMap[enum.Name] = valueMap - } return nil } diff --git a/schema/module_schema_test.go b/schema/module_schema_test.go index d2b6220a4f..d04327811b 100644 --- a/schema/module_schema_test.go +++ b/schema/module_schema_test.go @@ -110,6 +110,39 @@ func TestModuleSchema_Validate(t *testing.T) { }, errContains: "different values", }, + { + name: "same enum", + moduleSchema: ModuleSchema{ + ObjectTypes: []ObjectType{ + { + Name: "object1", + KeyFields: []Field{ + { + Name: "k", + Kind: EnumKind, + EnumDefinition: EnumDefinition{ + Name: "enum1", + Values: []string{"a", "b"}, + }, + }, + }, + }, + { + Name: "object2", + KeyFields: []Field{ + { + Name: "k", + Kind: EnumKind, + EnumDefinition: EnumDefinition{ + Name: "enum1", + Values: []string{"a", "b"}, + }, + }, + }, + }, + }, + }, + }, } for _, tt := range tests { diff --git a/schema/object_type.go b/schema/object_type.go index f991329eff..9560c5d4e3 100644 --- a/schema/object_type.go +++ b/schema/object_type.go @@ -11,7 +11,7 @@ type ObjectType struct { // KeyFields is a list of fields that make up the primary key of the object. // It can be empty in which case indexers should assume that this object is // a singleton and only has one value. Field names must be unique within the - // object between both key and value fields. + // object between both key and value fields. Key fields CANNOT be nullable. KeyFields []Field // ValueFields is a list of fields that are not part of the primary key of the object. @@ -29,20 +29,35 @@ type ObjectType struct { // Validate validates the object type. func (o ObjectType) Validate() error { + return o.validate(map[string]map[string]bool{}) +} + +// validate validates the object type with an enumValueMap that can be +// shared across a whole module schema. +func (o ObjectType) validate(enumValueMap map[string]map[string]bool) error { if !ValidateName(o.Name) { return fmt.Errorf("invalid object type name %q", o.Name) } fieldNames := map[string]bool{} + for _, field := range o.KeyFields { if err := field.Validate(); err != nil { return fmt.Errorf("invalid key field %q: %w", field.Name, err) } + if field.Nullable { + return fmt.Errorf("key field %q cannot be nullable", field.Name) + } + if fieldNames[field.Name] { return fmt.Errorf("duplicate field name %q", field.Name) } fieldNames[field.Name] = true + + if err := checkEnumCompatibility(enumValueMap, field); err != nil { + return err + } } for _, field := range o.ValueFields { @@ -54,6 +69,10 @@ func (o ObjectType) Validate() error { return fmt.Errorf("duplicate field name %q", field.Name) } fieldNames[field.Name] = true + + if err := checkEnumCompatibility(enumValueMap, field); err != nil { + return err + } } if len(o.KeyFields) == 0 && len(o.ValueFields) == 0 { @@ -69,7 +88,7 @@ func (o ObjectType) ValidateObjectUpdate(update ObjectUpdate) error { return fmt.Errorf("object type name %q does not match update type name %q", o.Name, update.TypeName) } - if err := ValidateForKeyFields(o.KeyFields, update.Key); err != nil { + if err := ValidateObjectKey(o.KeyFields, update.Key); err != nil { return fmt.Errorf("invalid key for object type %q: %w", update.TypeName, err) } @@ -77,5 +96,5 @@ func (o ObjectType) ValidateObjectUpdate(update ObjectUpdate) error { return nil } - return ValidateForValueFields(o.ValueFields, update.Value) + return ValidateObjectValue(o.ValueFields, update.Value) } diff --git a/schema/object_type_test.go b/schema/object_type_test.go index 32e8885bbc..0a78a371aa 100644 --- a/schema/object_type_test.go +++ b/schema/object_type_test.go @@ -148,6 +148,47 @@ func TestObjectType_Validate(t *testing.T) { }, errContains: "duplicate field name", }, + { + name: "nullable key field", + objectType: ObjectType{ + Name: "objectNullKey", + KeyFields: []Field{ + { + Name: "field1", + Kind: StringKind, + Nullable: true, + }, + }, + }, + errContains: "key field \"field1\" cannot be nullable", + }, + { + name: "duplicate incompatible enum", + objectType: ObjectType{ + Name: "objectWithEnums", + KeyFields: []Field{ + { + Name: "key", + Kind: EnumKind, + EnumDefinition: EnumDefinition{ + Name: "enum1", + Values: []string{"a", "b"}, + }, + }, + }, + ValueFields: []Field{ + { + Name: "value", + Kind: EnumKind, + EnumDefinition: EnumDefinition{ + Name: "enum1", + Values: []string{"c", "b"}, + }, + }, + }, + }, + errContains: "enum \"enum1\" has different values", + }, } for _, tt := range tests {