From efc8260fded08a61bc3ea3ea67e9cddbea42bcda Mon Sep 17 00:00:00 2001 From: Amine Date: Fri, 27 Sep 2024 02:10:12 -0700 Subject: [PATCH] Enhance `simpleschema` package to support nested complex types (#31) This commit significantly improves the `simpleschema` package ability to handle complex, nested data structures. The `parseMapType` function has been refactored to correctly parse nested map types and a new `findMatchingBracket` function has been added to ensure robust bracket parsing. The Transformer.BuildOpenAPISchema method has been restructured allowing for more flexible and accurate schema generation for nested slices and maps. By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license. --- internal/typesystem/simpleschema/atomic.go | 45 ++- .../typesystem/simpleschema/atomic_test.go | 9 +- .../typesystem/simpleschema/field_test.go | 3 +- internal/typesystem/simpleschema/transform.go | 268 ++++++++++-------- .../typesystem/simpleschema/transform_test.go | 122 ++++++-- 5 files changed, 294 insertions(+), 153 deletions(-) diff --git a/internal/typesystem/simpleschema/atomic.go b/internal/typesystem/simpleschema/atomic.go index 22c1019..f5a0c59 100644 --- a/internal/typesystem/simpleschema/atomic.go +++ b/internal/typesystem/simpleschema/atomic.go @@ -80,14 +80,46 @@ func isSliceType(s string) bool { // parseMapType parses a map type string and returns the key and value types. func parseMapType(s string) (string, string, error) { - // Remove the "map[" prefix and "]" suffix. - s = strings.TrimPrefix(s, "map[") - s = strings.TrimSuffix(s, "]") - parts := strings.Split(s, "]") - if len(parts) != 2 { + if !strings.HasPrefix(s, "map[") { return "", "", fmt.Errorf("invalid map type: %s", s) } - return parts[0], parts[1], nil + + // remove the "map[" prefix + s = s[4:] + + keyEndIndex := findMatchingBracket(s) + if keyEndIndex == -1 { + return "", "", fmt.Errorf("invalid map key type: %s", s) + } + + keyType := s[:keyEndIndex] + valueType := s[keyEndIndex+1:] + + valueType = strings.TrimSuffix(valueType, "]") + if keyType == "" { + return "", "", fmt.Errorf("empty map key type") + } + if valueType == "" { + return "", "", fmt.Errorf("empty map value type") + } + + return keyType, valueType, nil +} +func findMatchingBracket(s string) int { + depth := 1 + for i, char := range s { + switch char { + case '[': + depth++ + case ']': + depth-- + if depth == 0 { + return i + } + } + } + // no matching bracket found + return -1 } // parseSliceType parses a slice type string and returns the element type. @@ -95,6 +127,7 @@ func parseSliceType(s string) (string, error) { if !strings.HasPrefix(s, "[]") { return "", fmt.Errorf("invalid slice type: %s", s) } + // Remove the "[]" prefix. s = strings.TrimPrefix(s, "[]") if s == "" { diff --git a/internal/typesystem/simpleschema/atomic_test.go b/internal/typesystem/simpleschema/atomic_test.go index 3c76caa..34df3e3 100644 --- a/internal/typesystem/simpleschema/atomic_test.go +++ b/internal/typesystem/simpleschema/atomic_test.go @@ -112,8 +112,9 @@ func TestParseMapType(t *testing.T) { wantErr bool }{ {"valid map", "map[string]integer", "string", "integer", false}, - // not supported yet... do we need to support this? - // {"Valid Complex Map", "map[string]map[int]bool", "string", "map[int]bool", false}, + {"Valid Complex Map", "map[string]map[int]bool", "string", "map[int]bool", false}, + {"Nested Map", "map[string]map[string]map[string]integer", "string", "map[string]map[string]integer", false}, + {"invalid map", "map[]", "", "", true}, {"invalid map", "map[string]", "", "", true}, {"not a map", "something", "", "", true}, } @@ -143,8 +144,8 @@ func TestParseSliceType(t *testing.T) { wantErr bool }{ {"valid slice", "[]string", "string", false}, - // not supported yet - // {"Valid Complex Slice", "[]map[string]int", "map[string]int", false}, + {"Valid Complex Slice", "[]map[string]int", "map[string]int", false}, + {"Nested Slice", "[][][]int", "[][]int", false}, {"invalid slice", "[]", "", true}, {"Not a slice", "string", "", true}, } diff --git a/internal/typesystem/simpleschema/field_test.go b/internal/typesystem/simpleschema/field_test.go index 405fbd9..ec74a91 100644 --- a/internal/typesystem/simpleschema/field_test.go +++ b/internal/typesystem/simpleschema/field_test.go @@ -32,7 +32,8 @@ func TestParseFieldSchema(t *testing.T) { wantType: "string", wantMarkers: []*Marker{ {MarkerType: MarkerTypeRequired, Key: "required", Value: "true"}, - {MarkerType: MarkerTypeDescription, Key: "description", Value: "A test field"}, + {MarkerType: MarkerTypeDescription, Key: "description", Value: "A-test-field"}, + {MarkerType: MarkerTypeDefault, Key: "default", Value: "kubernetes-is-very-nice!"}, }, wantErr: false, }, diff --git a/internal/typesystem/simpleschema/transform.go b/internal/typesystem/simpleschema/transform.go index 4d58065..514b590 100644 --- a/internal/typesystem/simpleschema/transform.go +++ b/internal/typesystem/simpleschema/transform.go @@ -24,12 +24,15 @@ type Transformer struct { preDefinedTypes map[string]extv1.JSONSchemaProps } +// NewTransformer creates a new transformer func NewTransformer() *Transformer { return &Transformer{ preDefinedTypes: make(map[string]extv1.JSONSchemaProps), } } +// LoadPreDefinedTypes loads pre-defined types into the transformer. +// The pre-defined types are used to resolve references in the schema. func (t *Transformer) LoadPreDefinedTypes(obj map[string]interface{}) error { t.preDefinedTypes = make(map[string]extv1.JSONSchemaProps) @@ -44,6 +47,8 @@ func (t *Transformer) LoadPreDefinedTypes(obj map[string]interface{}) error { return nil } +// BuildOpenAPISchema builds an OpenAPI schema from the given object +// of a SimpleSchema. func (tf *Transformer) BuildOpenAPISchema(obj map[string]interface{}) (*extv1.JSONSchemaProps, error) { schema := &extv1.JSONSchemaProps{ Type: "object", @@ -51,144 +56,155 @@ func (tf *Transformer) BuildOpenAPISchema(obj map[string]interface{}) (*extv1.JS } for key, value := range obj { - switch v := value.(type) { - case map[interface{}]interface{}: + fieldSchema, err := tf.transformField(key, value, schema) + if err != nil { + return nil, err + } + schema.Properties[key] = *fieldSchema + } - // we have a nested object - nMap := transformMap(v) + return schema, nil +} +func (tf *Transformer) transformField( + key string, value interface{}, + // parentSchema is used to add the key to the required list + parentSchema *extv1.JSONSchemaProps, +) (*extv1.JSONSchemaProps, error) { + switch v := value.(type) { + case map[interface{}]interface{}: + nMap := transformMap(v) + return tf.BuildOpenAPISchema(nMap) + case map[string]interface{}: + return tf.BuildOpenAPISchema(v) + case string: + return tf.parseFieldSchema(key, v, parentSchema) + default: + return nil, fmt.Errorf("unknown type in schema: key: %s, value: %v", key, value) + } +} - fieldSchemaProps, err := tf.BuildOpenAPISchema(nMap) - if err != nil { - return nil, err - } - schema.Properties[key] = *fieldSchemaProps - case map[string]interface{}: - // transform the map to a map[inteface{}]interface{} - newMap := make(map[interface{}]interface{}) - for k, v := range v { - newMap[k] = v - } +func (tf *Transformer) parseFieldSchema(key, fieldValue string, parentSchema *extv1.JSONSchemaProps) (*extv1.JSONSchemaProps, error) { + fieldType, markers, err := parseFieldSchema(fieldValue) + if err != nil { + return nil, fmt.Errorf("failed to parse field schema for %s: %v", key, err) + } - // we have a nested object - nMap := transformMap(newMap) + fieldJSONSchemaProps := &extv1.JSONSchemaProps{} + + if isAtomicType(fieldType) { + fieldJSONSchemaProps.Type = string(fieldType) + } else if isCollectionType(fieldType) { + if isMapType(fieldType) { + fieldJSONSchemaProps, err = tf.handleMapType(key, fieldType) + } else if isSliceType(fieldType) { + fieldJSONSchemaProps, err = tf.handleSliceType(key, fieldType) + } else { + return nil, fmt.Errorf("unknown collection type: %s", fieldType) + } + if err != nil { + return nil, err + } + } else { + preDefinedType, ok := tf.preDefinedTypes[fieldType] + if !ok { + return nil, fmt.Errorf("unknown type: %s", fieldType) + } + fieldJSONSchemaProps = &preDefinedType + } - fieldSchemaProps, err := tf.BuildOpenAPISchema(nMap) - if err != nil { - return nil, err - } - schema.Properties[key] = *fieldSchemaProps - case string: - // we have a string. Meaning it's an atomic type, a reference to another type, or a collection type. - // It could also contain markers like `required=true` or `description="some description"` - // We need to parse the string to determine the type and any markers. - fieldType, markers, err := parseFieldSchema(value.(string)) - if err != nil { - return nil, fmt.Errorf("failed to parse field schema for %s: %v", key, err) - } + tf.applyMarkers(fieldJSONSchemaProps, markers, key, parentSchema) - fieldJSONSchemaProps := extv1.JSONSchemaProps{} - - if isAtomicType(fieldType) { - // this is an atomic type - fieldJSONSchemaProps.Type = string(fieldType) - } else if isCollectionType(fieldType) { - // this is a collection type, either an array or a map - if isMapType(fieldType) { - keyType, valueType, err := parseMapType(fieldType) - if err != nil { - return nil, fmt.Errorf("failed to parse map type for %s: %w", key, err) - } - fieldJSONSchemaProps.Type = "object" - fieldJSONSchemaProps.AdditionalProperties = &extv1.JSONSchemaPropsOrBool{ - Schema: &extv1.JSONSchemaProps{ - Type: keyType, - }, - } - - if preDefinedType, ok := tf.preDefinedTypes[valueType]; ok { - fieldJSONSchemaProps.AdditionalProperties.Schema = &preDefinedType - } else if isAtomicType(valueType) { - fieldJSONSchemaProps.AdditionalProperties.Schema = &extv1.JSONSchemaProps{ - Type: valueType, - } - } else { - return nil, fmt.Errorf("unknown type: %s", fieldType) - } - } else if isSliceType(fieldType) { - elementType, err := parseSliceType(fieldType) - if err != nil { - return nil, fmt.Errorf("failed to parse slice type for %s: %w", key, err) - } - - fieldJSONSchemaProps.Type = "array" - fieldJSONSchemaProps.Items = &extv1.JSONSchemaPropsOrArray{ - Schema: &extv1.JSONSchemaProps{ - Type: elementType, - }, - } - - if preDefinedType, ok := tf.preDefinedTypes[elementType]; ok { - fieldJSONSchemaProps.Items.Schema = &preDefinedType - } else if isAtomicType(elementType) { - fieldJSONSchemaProps.Items.Schema = &extv1.JSONSchemaProps{ - Type: elementType, - } - } else { - return nil, fmt.Errorf("unknown type: %s", fieldType) - } - } else { - return nil, fmt.Errorf("unknown collection type: %s", fieldType) - } - } else { - // this is a reference to pre defined type.. we should look it up - preDefinedType, ok := tf.preDefinedTypes[fieldType] - if !ok { - return nil, fmt.Errorf("unknown type: %s", fieldType) - } - fieldJSONSchemaProps = preDefinedType - } + return fieldJSONSchemaProps, nil +} - // apply markers - for _, marker := range markers { - switch marker.MarkerType { - case MarkerTypeRequired: - schema.Required = append(fieldJSONSchemaProps.Required, key) - case MarkerTypeDefault: - // depending on the type, we need to set the default value accordingly - var defaultValue []byte - switch fieldJSONSchemaProps.Type { - case "string": - defaultValue = []byte(fmt.Sprintf("\"%s\"", marker.Value)) - case "integer", "number": - defaultValue = []byte(marker.Value) - case "boolean": - defaultValue = []byte(marker.Value) - default: - // probably an object, array, or a map type. We can just - // set the raw value as the default - defaultValue = []byte(marker.Value) - } - - fieldJSONSchemaProps.Default = &extv1.JSON{ - Raw: defaultValue, - } - case MarkerTypeDescription: - fieldJSONSchemaProps.Description = marker.Value - default: - return nil, fmt.Errorf("unknown marker: %s", marker.MarkerType) - } - } +func (tf *Transformer) handleMapType(key, fieldType string) (*extv1.JSONSchemaProps, error) { + keyType, valueType, err := parseMapType(fieldType) + if err != nil { + return nil, fmt.Errorf("failed to parse map type for %s: %w", key, err) + } + if keyType != "string" { + return nil, fmt.Errorf("unsupported key type for maps: %s", keyType) + } + + fieldJSONSchemaProps := &extv1.JSONSchemaProps{ + Type: "object", + AdditionalProperties: &extv1.JSONSchemaPropsOrBool{ + Schema: &extv1.JSONSchemaProps{}, + }, + } - schema.Properties[key] = fieldJSONSchemaProps - default: - // arrays and maps are only supported using the `[]` and `map[]` prefixes - return nil, fmt.Errorf("unknown type in schema: key: %s, value: %s", key, value) + if isCollectionType(valueType) { + valueSchema, err := tf.parseFieldSchema(key, valueType, fieldJSONSchemaProps) + if err != nil { + return nil, err } + fieldJSONSchemaProps.AdditionalProperties.Schema = valueSchema + } else if preDefinedType, ok := tf.preDefinedTypes[valueType]; ok { + fieldJSONSchemaProps.AdditionalProperties.Schema = &preDefinedType + } else if isAtomicType(valueType) { + fieldJSONSchemaProps.AdditionalProperties.Schema.Type = valueType + } else { + return nil, fmt.Errorf("unknown type: %s", valueType) + } + return fieldJSONSchemaProps, nil +} + +func (tf *Transformer) handleSliceType(key, fieldType string) (*extv1.JSONSchemaProps, error) { + elementType, err := parseSliceType(fieldType) + if err != nil { + return nil, fmt.Errorf("failed to parse slice type for %s: %w", key, err) + } + + fieldJSONSchemaProps := &extv1.JSONSchemaProps{ + Type: "array", + Items: &extv1.JSONSchemaPropsOrArray{ + Schema: &extv1.JSONSchemaProps{}, + }, + } + + if isCollectionType(elementType) { + elementSchema, err := tf.parseFieldSchema(key, elementType, fieldJSONSchemaProps) + if err != nil { + return nil, err + } + fieldJSONSchemaProps.Items.Schema = elementSchema + } else if isAtomicType(elementType) { + fieldJSONSchemaProps.Items.Schema.Type = elementType + } else if preDefinedType, ok := tf.preDefinedTypes[elementType]; ok { + fieldJSONSchemaProps.Items.Schema = &preDefinedType + } else { + return nil, fmt.Errorf("unknown type: %s", elementType) + } + + return fieldJSONSchemaProps, nil +} + +func (tf *Transformer) applyMarkers(schema *extv1.JSONSchemaProps, markers []*Marker, key string, parentSchema *extv1.JSONSchemaProps) { + for _, marker := range markers { + switch marker.MarkerType { + case MarkerTypeRequired: + if parentSchema != nil { + parentSchema.Required = append(parentSchema.Required, key) + } + case MarkerTypeDefault: + var defaultValue []byte + switch schema.Type { + case "string": + defaultValue = []byte(fmt.Sprintf("\"%s\"", marker.Value)) + case "integer", "number", "boolean": + defaultValue = []byte(marker.Value) + default: + defaultValue = []byte(marker.Value) + } + schema.Default = &extv1.JSON{Raw: defaultValue} + case MarkerTypeDescription: + schema.Description = marker.Value + } } - return schema, nil } +// Other functions (LoadPreDefinedTypes, transformMap) remain unchanged func transformMap(original map[interface{}]interface{}) map[string]interface{} { result := make(map[string]interface{}) for key, value := range original { diff --git a/internal/typesystem/simpleschema/transform_test.go b/internal/typesystem/simpleschema/transform_test.go index ce41cd0..44a9453 100644 --- a/internal/typesystem/simpleschema/transform_test.go +++ b/internal/typesystem/simpleschema/transform_test.go @@ -30,6 +30,10 @@ func TestBuildOpenAPISchema(t *testing.T) { "city": "string", "country": "string", }, + "Person": map[string]interface{}{ + "name": "string", + "age": "integer", + }, }) if err != nil { t.Fatalf("Failed to load pre-defined types: %v", err) @@ -58,7 +62,8 @@ func TestBuildOpenAPISchema(t *testing.T) { "friends": "[]Person", }, want: &extv1.JSONSchemaProps{ - Type: "object", + Type: "object", + Required: []string{"name"}, Properties: map[string]extv1.JSONSchemaProps{ "name": {Type: "string"}, "age": { @@ -120,7 +125,6 @@ func TestBuildOpenAPISchema(t *testing.T) { }, }, }, - Required: []string{"name"}, }, wantErr: false, }, @@ -178,6 +182,103 @@ func TestBuildOpenAPISchema(t *testing.T) { want: nil, wantErr: true, }, + { + name: "Nested slices", + obj: map[string]interface{}{ + "matrix": "[][][]string", + }, + want: &extv1.JSONSchemaProps{ + Type: "object", + Properties: map[string]extv1.JSONSchemaProps{ + "matrix": { + Type: "array", + Items: &extv1.JSONSchemaPropsOrArray{ + Schema: &extv1.JSONSchemaProps{ + Type: "array", + Items: &extv1.JSONSchemaPropsOrArray{ + Schema: &extv1.JSONSchemaProps{ + Type: "array", + Items: &extv1.JSONSchemaPropsOrArray{ + Schema: &extv1.JSONSchemaProps{Type: "string"}, + }, + }, + }, + }, + }, + }, + }, + }, + wantErr: false, + }, + { + name: "Nested slices with custom type", + obj: map[string]interface{}{ + "matrix": "[][][]Person", + }, + want: &extv1.JSONSchemaProps{ + Type: "object", + Properties: map[string]extv1.JSONSchemaProps{ + "matrix": { + Type: "array", + + Items: &extv1.JSONSchemaPropsOrArray{ + Schema: &extv1.JSONSchemaProps{ + Type: "array", + Items: &extv1.JSONSchemaPropsOrArray{ + Schema: &extv1.JSONSchemaProps{ + Type: "array", + Items: &extv1.JSONSchemaPropsOrArray{ + Schema: &extv1.JSONSchemaProps{ + Type: "object", + Properties: map[string]extv1.JSONSchemaProps{ + "name": {Type: "string"}, + "age": {Type: "integer"}, + }, + }, + }, + }, + }, + }, + }, + }, + }, + }, + wantErr: false, + }, + { + name: "Nested maps", + obj: map[string]interface{}{ + "matrix": "map[string]map[string]map[string]Person", + }, + want: &extv1.JSONSchemaProps{ + Type: "object", + Properties: map[string]extv1.JSONSchemaProps{ + "matrix": { + Type: "object", + AdditionalProperties: &extv1.JSONSchemaPropsOrBool{ + Schema: &extv1.JSONSchemaProps{ + Type: "object", + AdditionalProperties: &extv1.JSONSchemaPropsOrBool{ + Schema: &extv1.JSONSchemaProps{ + Type: "object", + AdditionalProperties: &extv1.JSONSchemaPropsOrBool{ + Schema: &extv1.JSONSchemaProps{ + Type: "object", + Properties: map[string]extv1.JSONSchemaProps{ + "name": {Type: "string"}, + "age": {Type: "integer"}, + }, + }, + }, + }, + }, + }, + }, + }, + }, + }, + wantErr: false, + }, } for _, tt := range tests { @@ -188,7 +289,7 @@ func TestBuildOpenAPISchema(t *testing.T) { return } if !reflect.DeepEqual(got, tt.want) { - t.Errorf("BuildOpenAPISchema() = %v, want %v", got, tt.want) + t.Errorf("BuildOpenAPISchema() = %+v, want %+v", got, tt.want) } }) } @@ -208,7 +309,7 @@ func TestLoadPreDefinedTypes(t *testing.T) { }, "Company": map[string]interface{}{ "name": "string", - "employees": "[]Person", + "employees": "[]string", }, } @@ -260,18 +361,7 @@ func TestLoadPreDefinedTypes(t *testing.T) { Type: "array", Items: &extv1.JSONSchemaPropsOrArray{ Schema: &extv1.JSONSchemaProps{ - Type: "object", - Properties: map[string]extv1.JSONSchemaProps{ - "name": {Type: "string"}, - "age": {Type: "integer"}, - "address": { - Type: "object", - Properties: map[string]extv1.JSONSchemaProps{ - "street": {Type: "string"}, - "city": {Type: "string"}, - }, - }, - }, + Type: "string", }, }, },