From 7f05e5c96dbf1eec5dca4307a39902d1fbef527c Mon Sep 17 00:00:00 2001 From: monalisa Date: Wed, 6 Sep 2023 16:45:49 +0200 Subject: [PATCH 01/12] Add validation for schema default value during Load. Also cast default value to float if needed. --- libs/jsonschema/schema.go | 28 ++++ libs/jsonschema/schema_test.go | 39 +++++- .../schema-invalid-default.json | 9 ++ .../schema-load-int/schema-valid.json | 9 ++ libs/jsonschema/utils.go | 38 ++++++ libs/jsonschema/utils_test.go | 49 +++++++ libs/jsonschema/validate_type.go | 59 ++++++++ libs/jsonschema/validate_type_test.go | 128 ++++++++++++++++++ 8 files changed, 358 insertions(+), 1 deletion(-) create mode 100644 libs/jsonschema/testdata/schema-load-int/schema-invalid-default.json create mode 100644 libs/jsonschema/testdata/schema-load-int/schema-valid.json create mode 100644 libs/jsonschema/utils.go create mode 100644 libs/jsonschema/utils_test.go create mode 100644 libs/jsonschema/validate_type.go create mode 100644 libs/jsonschema/validate_type_test.go diff --git a/libs/jsonschema/schema.go b/libs/jsonschema/schema.go index 87e9acd566b..a575f181c7a 100644 --- a/libs/jsonschema/schema.go +++ b/libs/jsonschema/schema.go @@ -58,6 +58,7 @@ const ( ) func (schema *Schema) validate() error { + // Validate property types are all valid JSON schema types. for _, v := range schema.Properties { switch v.Type { case NumberType, BooleanType, StringType, IntegerType: @@ -72,6 +73,17 @@ func (schema *Schema) validate() error { return fmt.Errorf("type %s is not a recognized json schema type", v.Type) } } + + // Validate default property values are consistent with types. + for name, property := range schema.Properties { + if property.Default == nil { + continue + } + if err := validateType(property.Default, property.Type); err != nil { + return fmt.Errorf("type validation for default value of property %s failed: %w", name, err) + } + } + return nil } @@ -85,5 +97,21 @@ func Load(path string) (*Schema, error) { if err != nil { return nil, err } + + // Convert the top properties default values and enum values to integers. + // This is require because the default JSON unmarshaler parses untyped numbers + // as floats. + for name, property := range schema.Properties { + if property.Type != IntegerType { + continue + } + if property.Default != nil { + property.Default, err = toInteger(property.Default) + if err != nil { + return nil, fmt.Errorf("failed to parse default value for property %s: %w", name, err) + } + } + } + return schema, schema.validate() } diff --git a/libs/jsonschema/schema_test.go b/libs/jsonschema/schema_test.go index 76112492f57..5b92d84669e 100644 --- a/libs/jsonschema/schema_test.go +++ b/libs/jsonschema/schema_test.go @@ -6,7 +6,7 @@ import ( "github.com/stretchr/testify/assert" ) -func TestJsonSchemaValidate(t *testing.T) { +func TestSchemaValidateTypeNames(t *testing.T) { var err error toSchema := func(s string) *Schema { return &Schema{ @@ -42,3 +42,40 @@ func TestJsonSchemaValidate(t *testing.T) { err = toSchema("foobar").validate() assert.EqualError(t, err, "type foobar is not a recognized json schema type") } + +func TestSchemaLoadIntegers(t *testing.T) { + schema, err := Load("./testdata/schema-load-int/schema-valid.json") + assert.NoError(t, err) + assert.Equal(t, int64(1), schema.Properties["abc"].Default) +} + +func TestSchemaLoadIntegersWithInvalidDefault(t *testing.T) { + _, err := Load("./testdata/schema-load-int/schema-invalid-default.json") + assert.EqualError(t, err, "failed to parse default value for property abc: expected integer value, got: 1.1") +} + +func TestSchemaValidateDefaultType(t *testing.T) { + invalidSchema := &Schema{ + Properties: map[string]*Schema{ + "foo": { + Type: "number", + Default: "abc", + }, + }, + } + + err := invalidSchema.validate() + assert.EqualError(t, err, "type validation for default value of property foo failed: expected type float, but value is \"abc\"") + + validSchema := &Schema{ + Properties: map[string]*Schema{ + "foo": { + Type: "boolean", + Default: true, + }, + }, + } + + err = validSchema.validate() + assert.NoError(t, err) +} diff --git a/libs/jsonschema/testdata/schema-load-int/schema-invalid-default.json b/libs/jsonschema/testdata/schema-load-int/schema-invalid-default.json new file mode 100644 index 00000000000..1e709f622c5 --- /dev/null +++ b/libs/jsonschema/testdata/schema-load-int/schema-invalid-default.json @@ -0,0 +1,9 @@ +{ + "type": "object", + "properties": { + "abc": { + "type": "integer", + "default": 1.1 + } + } +} diff --git a/libs/jsonschema/testdata/schema-load-int/schema-valid.json b/libs/jsonschema/testdata/schema-load-int/schema-valid.json new file mode 100644 index 00000000000..599ac04d0d3 --- /dev/null +++ b/libs/jsonschema/testdata/schema-load-int/schema-valid.json @@ -0,0 +1,9 @@ +{ + "type": "object", + "properties": { + "abc": { + "type": "integer", + "default": 1 + } + } +} diff --git a/libs/jsonschema/utils.go b/libs/jsonschema/utils.go new file mode 100644 index 00000000000..90803e6241d --- /dev/null +++ b/libs/jsonschema/utils.go @@ -0,0 +1,38 @@ +package jsonschema + +import "fmt" + +// TODO: this code is copied from libs/template/utils.go. Remove it from +// the template package once validation is moved to the json schema package + +// function to check whether a float value represents an integer +func isIntegerValue(v float64) bool { + return v == float64(int64(v)) +} + +func toInteger(v any) (int64, error) { + switch typedVal := v.(type) { + // cast float to int + case float32: + if !isIntegerValue(float64(typedVal)) { + return 0, fmt.Errorf("expected integer value, got: %v", v) + } + return int64(typedVal), nil + case float64: + if !isIntegerValue(typedVal) { + return 0, fmt.Errorf("expected integer value, got: %v", v) + } + return int64(typedVal), nil + + // pass through common integer cases + case int: + return int64(typedVal), nil + case int32: + return int64(typedVal), nil + case int64: + return typedVal, nil + + default: + return 0, fmt.Errorf("cannot convert %#v to an integer", v) + } +} diff --git a/libs/jsonschema/utils_test.go b/libs/jsonschema/utils_test.go new file mode 100644 index 00000000000..799b26ecaf9 --- /dev/null +++ b/libs/jsonschema/utils_test.go @@ -0,0 +1,49 @@ +package jsonschema + +import ( + "math" + "testing" + + "github.com/stretchr/testify/assert" +) + +func TestTemplateIsInteger(t *testing.T) { + assert.False(t, isIntegerValue(1.1)) + assert.False(t, isIntegerValue(0.1)) + assert.False(t, isIntegerValue(-0.1)) + + assert.True(t, isIntegerValue(-1.0)) + assert.True(t, isIntegerValue(0.0)) + assert.True(t, isIntegerValue(2.0)) +} + +func TestTemplateToInteger(t *testing.T) { + v, err := toInteger(float32(2)) + assert.NoError(t, err) + assert.Equal(t, int64(2), v) + + v, err = toInteger(float64(4)) + assert.NoError(t, err) + assert.Equal(t, int64(4), v) + + v, err = toInteger(float64(4)) + assert.NoError(t, err) + assert.Equal(t, int64(4), v) + + v, err = toInteger(float64(math.MaxInt32 + 10)) + assert.NoError(t, err) + assert.Equal(t, int64(2147483657), v) + + v, err = toInteger(2) + assert.NoError(t, err) + assert.Equal(t, int64(2), v) + + _, err = toInteger(float32(2.2)) + assert.EqualError(t, err, "expected integer value, got: 2.2") + + _, err = toInteger(float64(math.MaxInt32 + 100.1)) + assert.ErrorContains(t, err, "expected integer value, got: 2.1474837471e+09") + + _, err = toInteger("abcd") + assert.EqualError(t, err, "cannot convert \"abcd\" to an integer") +} diff --git a/libs/jsonschema/validate_type.go b/libs/jsonschema/validate_type.go new file mode 100644 index 00000000000..a4e43aa583b --- /dev/null +++ b/libs/jsonschema/validate_type.go @@ -0,0 +1,59 @@ +package jsonschema + +import ( + "fmt" + "reflect" + "slices" +) + +// TODO: this code is copied from libs/template/validators.go. Remove it from +// the template package once validation is moved to the json schema package + +type validateTypeFunc func(v any) error + +func validateType(v any, fieldType Type) error { + validateFunc, ok := validateTypeFuncs[fieldType] + if !ok { + return nil + } + return validateFunc(v) +} + +func validateString(v any) error { + if _, ok := v.(string); !ok { + return fmt.Errorf("expected type string, but value is %#v", v) + } + return nil +} + +func validateBoolean(v any) error { + if _, ok := v.(bool); !ok { + return fmt.Errorf("expected type boolean, but value is %#v", v) + } + return nil +} + +func validateNumber(v any) error { + if !slices.Contains([]reflect.Kind{reflect.Float32, reflect.Float64}, + reflect.TypeOf(v).Kind()) { + return fmt.Errorf("expected type float, but value is %#v", v) + } + return nil +} + +func validateInteger(v any) error { + if !slices.Contains([]reflect.Kind{reflect.Int, reflect.Int8, reflect.Int16, + reflect.Int32, reflect.Int64, reflect.Uint, reflect.Uint8, reflect.Uint16, + reflect.Uint32, reflect.Uint64}, + reflect.TypeOf(v).Kind()) { + return fmt.Errorf("expected type integer, but value is %#v", v) + } + return nil +} + +var validateTypeFuncs map[Type]validateTypeFunc = map[Type]validateTypeFunc{ + StringType: validateString, + BooleanType: validateBoolean, + IntegerType: validateInteger, + NumberType: validateNumber, +} diff --git a/libs/jsonschema/validate_type_test.go b/libs/jsonschema/validate_type_test.go new file mode 100644 index 00000000000..36d9e5758df --- /dev/null +++ b/libs/jsonschema/validate_type_test.go @@ -0,0 +1,128 @@ +package jsonschema + +import ( + "testing" + + "github.com/stretchr/testify/assert" +) + +func TestValidatorString(t *testing.T) { + err := validateString("abc") + assert.NoError(t, err) + + err = validateString(1) + assert.ErrorContains(t, err, "expected type string, but value is 1") + + err = validateString(true) + assert.ErrorContains(t, err, "expected type string, but value is true") + + err = validateString("false") + assert.NoError(t, err) +} + +func TestValidatorBoolean(t *testing.T) { + err := validateBoolean(true) + assert.NoError(t, err) + + err = validateBoolean(1) + assert.ErrorContains(t, err, "expected type boolean, but value is 1") + + err = validateBoolean("abc") + assert.ErrorContains(t, err, "expected type boolean, but value is \"abc\"") + + err = validateBoolean("false") + assert.ErrorContains(t, err, "expected type boolean, but value is \"false\"") +} + +func TestValidatorNumber(t *testing.T) { + err := validateNumber(true) + assert.ErrorContains(t, err, "expected type float, but value is true") + + err = validateNumber(int32(1)) + assert.ErrorContains(t, err, "expected type float, but value is 1") + + err = validateNumber(int64(2)) + assert.ErrorContains(t, err, "expected type float, but value is 2") + + err = validateNumber(float32(1)) + assert.NoError(t, err) + + err = validateNumber(float64(1)) + assert.NoError(t, err) + + err = validateNumber("abc") + assert.ErrorContains(t, err, "expected type float, but value is \"abc\"") +} + +func TestValidatorInt(t *testing.T) { + err := validateInteger(true) + assert.ErrorContains(t, err, "expected type integer, but value is true") + + err = validateInteger(int32(1)) + assert.NoError(t, err) + + err = validateInteger(int64(1)) + assert.NoError(t, err) + + err = validateInteger(float32(1)) + assert.ErrorContains(t, err, "expected type integer, but value is 1") + + err = validateInteger(float64(1)) + assert.ErrorContains(t, err, "expected type integer, but value is 1") + + err = validateInteger("abc") + assert.ErrorContains(t, err, "expected type integer, but value is \"abc\"") +} + +func TestTemplateValidateType(t *testing.T) { + // assert validation passing + err := validateType(int(0), IntegerType) + assert.NoError(t, err) + err = validateType(int32(1), IntegerType) + assert.NoError(t, err) + err = validateType(int64(1), IntegerType) + assert.NoError(t, err) + + err = validateType(float32(1.1), NumberType) + assert.NoError(t, err) + err = validateType(float64(1.2), NumberType) + assert.NoError(t, err) + + err = validateType(false, BooleanType) + assert.NoError(t, err) + + err = validateType("abc", StringType) + assert.NoError(t, err) + + // assert validation failing for integers + err = validateType(float64(1.2), IntegerType) + assert.ErrorContains(t, err, "expected type integer, but value is 1.2") + err = validateType(true, IntegerType) + assert.ErrorContains(t, err, "expected type integer, but value is true") + err = validateType("abc", IntegerType) + assert.ErrorContains(t, err, "expected type integer, but value is \"abc\"") + + // assert validation failing for floats + err = validateType(true, NumberType) + assert.ErrorContains(t, err, "expected type float, but value is true") + err = validateType("abc", NumberType) + assert.ErrorContains(t, err, "expected type float, but value is \"abc\"") + err = validateType(int(1), NumberType) + assert.ErrorContains(t, err, "expected type float, but value is 1") + + // assert validation failing for boolean + err = validateType(int(1), BooleanType) + assert.ErrorContains(t, err, "expected type boolean, but value is 1") + err = validateType(float64(1), BooleanType) + assert.ErrorContains(t, err, "expected type boolean, but value is 1") + err = validateType("abc", BooleanType) + assert.ErrorContains(t, err, "expected type boolean, but value is \"abc\"") + + // assert validation failing for string + err = validateType(int(1), StringType) + assert.ErrorContains(t, err, "expected type string, but value is 1") + err = validateType(float64(1), StringType) + assert.ErrorContains(t, err, "expected type string, but value is 1") + err = validateType(false, StringType) + assert.ErrorContains(t, err, "expected type string, but value is false") +} From ff045963fa4830083ac1997af9ae2e19b5a144c6 Mon Sep 17 00:00:00 2001 From: monalisa Date: Wed, 6 Sep 2023 16:51:17 +0200 Subject: [PATCH 02/12] removing todos --- libs/jsonschema/utils.go | 3 --- libs/jsonschema/validate_type.go | 3 --- 2 files changed, 6 deletions(-) diff --git a/libs/jsonschema/utils.go b/libs/jsonschema/utils.go index 90803e6241d..34b51f4d94d 100644 --- a/libs/jsonschema/utils.go +++ b/libs/jsonschema/utils.go @@ -2,9 +2,6 @@ package jsonschema import "fmt" -// TODO: this code is copied from libs/template/utils.go. Remove it from -// the template package once validation is moved to the json schema package - // function to check whether a float value represents an integer func isIntegerValue(v float64) bool { return v == float64(int64(v)) diff --git a/libs/jsonschema/validate_type.go b/libs/jsonschema/validate_type.go index a4e43aa583b..125d6b20b26 100644 --- a/libs/jsonschema/validate_type.go +++ b/libs/jsonschema/validate_type.go @@ -6,9 +6,6 @@ import ( "slices" ) -// TODO: this code is copied from libs/template/validators.go. Remove it from -// the template package once validation is moved to the json schema package - type validateTypeFunc func(v any) error func validateType(v any, fieldType Type) error { From b865190de3ad38e3147aa87ec6603bef4c0d3c3f Mon Sep 17 00:00:00 2001 From: monalisa Date: Wed, 6 Sep 2023 19:29:35 +0200 Subject: [PATCH 03/12] This commit introduces the LoadInstance and ValidateInstance methods. Replaces load and validation in assignValuesFromFile to use these instead. --- libs/jsonschema/instance.go | 76 +++++++++++++++++ libs/jsonschema/instance_test.go | 85 +++++++++++++++++++ .../test-schema-no-additional-properties.json | 19 +++++ .../instance-validate/test-schema.json | 18 ++++ libs/template/config.go | 33 ++----- libs/template/config_test.go | 12 +-- 6 files changed, 205 insertions(+), 38 deletions(-) create mode 100644 libs/jsonschema/instance.go create mode 100644 libs/jsonschema/instance_test.go create mode 100644 libs/jsonschema/testdata/instance-validate/test-schema-no-additional-properties.json create mode 100644 libs/jsonschema/testdata/instance-validate/test-schema.json diff --git a/libs/jsonschema/instance.go b/libs/jsonschema/instance.go new file mode 100644 index 00000000000..a2bdc01c61e --- /dev/null +++ b/libs/jsonschema/instance.go @@ -0,0 +1,76 @@ +package jsonschema + +import ( + "encoding/json" + "fmt" + "os" +) + +// Load a JSON document, also validating it against the JSON schema. Instance here +// refers to a JSON document. see: https://json-schema.org/draft/2020-12/json-schema-core.html#name-instance +func (s *Schema) LoadInstance(path string) (map[string]any, error) { + instance := make(map[string]any, 0) + b, err := os.ReadFile(path) + if err != nil { + return nil, err + } + err = json.Unmarshal(b, &instance) + if err != nil { + return nil, err + } + + // The default JSON unmarshaler parses untyped number values as float64. + // We convert integer properties from float64 to int64 here. + for name, v := range instance { + propertySchema, ok := s.Properties[name] + if !ok { + continue + } + if propertySchema.Type != IntegerType { + continue + } + integerValue, err := toInteger(v) + if err != nil { + return nil, fmt.Errorf("failed to parse property %s: %w", name, err) + } + instance[name] = integerValue + } + return instance, s.ValidateInstance(instance) +} + +func (s *Schema) ValidateInstance(instance map[string]any) error { + if err := s.validateAdditionalProperties(instance); err != nil { + return err + } + return s.validateTypes(instance) +} + +// If additional properties is set to false, this function validates instance only +// contains properties defined in the schema. +func (s *Schema) validateAdditionalProperties(instance map[string]any) error { + if s.AdditionalProperties != false { + return nil + } + for k := range instance { + _, ok := s.Properties[k] + if !ok { + return fmt.Errorf("property %s is not defined in the schema", k) + } + } + return nil +} + +// Validates the types of all input properties values match their types defined in the schema +func (s *Schema) validateTypes(instance map[string]any) error { + for k, v := range instance { + fieldInfo, ok := s.Properties[k] + if !ok { + continue + } + err := validateType(v, fieldInfo.Type) + if err != nil { + return fmt.Errorf("incorrect type for property %s: %w", k, err) + } + } + return nil +} diff --git a/libs/jsonschema/instance_test.go b/libs/jsonschema/instance_test.go new file mode 100644 index 00000000000..1e0404f3ac5 --- /dev/null +++ b/libs/jsonschema/instance_test.go @@ -0,0 +1,85 @@ +package jsonschema + +import ( + "testing" + + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" +) + +func TestValidateInstanceAdditionalPropertiesPermitted(t *testing.T) { + instance := map[string]any{ + "int_val": 1, + "float_val": 1.0, + "bool_val": false, + "an_additional_property": "abc", + } + + schema, err := Load("./testdata/instance-validate/test-schema.json") + require.NoError(t, err) + + err = schema.validateAdditionalProperties(instance) + assert.NoError(t, err) + + err = schema.ValidateInstance(instance) + assert.NoError(t, err) +} + +func TestValidateInstanceAdditionalPropertiesForbidden(t *testing.T) { + instance := map[string]any{ + "int_val": 1, + "float_val": 1.0, + "bool_val": false, + "an_additional_property": "abc", + } + + schema, err := Load("./testdata/instance-validate/test-schema-no-additional-properties.json") + require.NoError(t, err) + + err = schema.validateAdditionalProperties(instance) + assert.EqualError(t, err, "property an_additional_property is not defined in the schema") + + err = schema.ValidateInstance(instance) + assert.EqualError(t, err, "property an_additional_property is not defined in the schema") + + instanceWOAdditionalProperties := map[string]any{ + "int_val": 1, + "float_val": 1.0, + "bool_val": false, + } + + err = schema.validateAdditionalProperties(instanceWOAdditionalProperties) + assert.NoError(t, err) + + err = schema.ValidateInstance(instanceWOAdditionalProperties) + assert.NoError(t, err) +} + +func TestValidateInstanceTypes(t *testing.T) { + schema, err := Load("./testdata/instance-validate/test-schema.json") + require.NoError(t, err) + + validInstance := map[string]any{ + "int_val": 1, + "float_val": 1.0, + "bool_val": false, + } + + err = schema.validateTypes(validInstance) + assert.NoError(t, err) + + err = schema.ValidateInstance(validInstance) + assert.NoError(t, err) + + invalidInstance := map[string]any{ + "int_val": "abc", + "float_val": 1.0, + "bool_val": false, + } + + err = schema.validateTypes(invalidInstance) + assert.EqualError(t, err, "incorrect type for property int_val: expected type integer, but value is \"abc\"") + + err = schema.ValidateInstance(invalidInstance) + assert.EqualError(t, err, "incorrect type for property int_val: expected type integer, but value is \"abc\"") +} diff --git a/libs/jsonschema/testdata/instance-validate/test-schema-no-additional-properties.json b/libs/jsonschema/testdata/instance-validate/test-schema-no-additional-properties.json new file mode 100644 index 00000000000..98b19d5a489 --- /dev/null +++ b/libs/jsonschema/testdata/instance-validate/test-schema-no-additional-properties.json @@ -0,0 +1,19 @@ +{ + "properties": { + "int_val": { + "type": "integer", + "default": 123 + }, + "float_val": { + "type": "number" + }, + "bool_val": { + "type": "boolean" + }, + "string_val": { + "type": "string", + "default": "abc" + } + }, + "additionalProperties": false +} diff --git a/libs/jsonschema/testdata/instance-validate/test-schema.json b/libs/jsonschema/testdata/instance-validate/test-schema.json new file mode 100644 index 00000000000..41eb825190f --- /dev/null +++ b/libs/jsonschema/testdata/instance-validate/test-schema.json @@ -0,0 +1,18 @@ +{ + "properties": { + "int_val": { + "type": "integer", + "default": 123 + }, + "float_val": { + "type": "number" + }, + "bool_val": { + "type": "boolean" + }, + "string_val": { + "type": "string", + "default": "abc" + } + } +} diff --git a/libs/template/config.go b/libs/template/config.go index 8a1ed6c82a8..b7ba3c7da61 100644 --- a/libs/template/config.go +++ b/libs/template/config.go @@ -2,9 +2,7 @@ package template import ( "context" - "encoding/json" "fmt" - "os" "github.com/databricks/cli/libs/cmdio" "github.com/databricks/cli/libs/jsonschema" @@ -26,6 +24,9 @@ func newConfig(ctx context.Context, schemaPath string) (*config, error) { return nil, err } + // Do not allow template input variables that are not defined in the schema. + schema.AdditionalProperties = false + // Return config return &config{ ctx: ctx, @@ -45,32 +46,10 @@ func validateSchema(schema *jsonschema.Schema) error { // Reads json file at path and assigns values from the file func (c *config) assignValuesFromFile(path string) error { - // Read the config file - configFromFile := make(map[string]any, 0) - b, err := os.ReadFile(path) - if err != nil { - return err - } - err = json.Unmarshal(b, &configFromFile) + // Load the config file. + configFromFile, err := c.schema.LoadInstance(path) if err != nil { - return err - } - - // Cast any integer properties, from float to integer. Required because - // the json unmarshaller treats all json numbers as floating point - for name, floatVal := range configFromFile { - property, ok := c.schema.Properties[name] - if !ok { - return fmt.Errorf("%s is not defined as an input parameter for the template", name) - } - if property.Type != jsonschema.IntegerType { - continue - } - v, err := toInteger(floatVal) - if err != nil { - return fmt.Errorf("failed to cast value %v of property %s from file %s to an integer: %w", floatVal, name, path, err) - } - configFromFile[name] = v + return fmt.Errorf("failed to load config from file %s. %w", path, err) } // Write configs from the file to the input map, not overwriting any existing diff --git a/libs/template/config_test.go b/libs/template/config_test.go index 335242467f8..f51b95f64aa 100644 --- a/libs/template/config_test.go +++ b/libs/template/config_test.go @@ -49,16 +49,6 @@ func TestTemplateConfigAssignValuesFromFile(t *testing.T) { assert.Equal(t, "hello", c.values["string_val"]) } -func TestTemplateConfigAssignValuesFromFileForUnknownField(t *testing.T) { - c := config{ - schema: testSchema(t), - values: make(map[string]any), - } - - err := c.assignValuesFromFile("./testdata/config-assign-from-file-unknown-property/config.json") - assert.EqualError(t, err, "unknown_prop is not defined as an input parameter for the template") -} - func TestTemplateConfigAssignValuesFromFileForInvalidIntegerValue(t *testing.T) { c := config{ schema: testSchema(t), @@ -66,7 +56,7 @@ func TestTemplateConfigAssignValuesFromFileForInvalidIntegerValue(t *testing.T) } err := c.assignValuesFromFile("./testdata/config-assign-from-file-invalid-int/config.json") - assert.EqualError(t, err, "failed to cast value abc of property int_val from file ./testdata/config-assign-from-file-invalid-int/config.json to an integer: cannot convert \"abc\" to an integer") + assert.EqualError(t, err, "failed to load config from file ./testdata/config-assign-from-file-invalid-int/config.json. failed to parse property int_val: cannot convert \"abc\" to an integer") } func TestTemplateConfigAssignValuesFromFileDoesNotOverwriteExistingConfigs(t *testing.T) { From 8bad4e9eb9a8d9134a18b2dc4aa0a487d34c6aab Mon Sep 17 00:00:00 2001 From: monalisa Date: Wed, 6 Sep 2023 19:37:57 +0200 Subject: [PATCH 04/12] Remove integer conversion logic during config default assignment --- libs/template/config.go | 17 +------------ libs/template/config_test.go | 24 ++----------------- .../config-test-schema/test-schema.json | 18 ++++++++++++++ 3 files changed, 21 insertions(+), 38 deletions(-) create mode 100644 libs/template/testdata/config-test-schema/test-schema.json diff --git a/libs/template/config.go b/libs/template/config.go index b7ba3c7da61..3fb1cbcf2f3 100644 --- a/libs/template/config.go +++ b/libs/template/config.go @@ -70,26 +70,11 @@ func (c *config) assignDefaultValues() error { if _, ok := c.values[name]; ok { continue } - // No default value defined for the property if property.Default == nil { continue } - - // Assign default value if property is not an integer - if property.Type != jsonschema.IntegerType { - c.values[name] = property.Default - continue - } - - // Cast default value to int before assigning to an integer configuration. - // Required because untyped field Default will read all numbers as floats - // during unmarshalling - v, err := toInteger(property.Default) - if err != nil { - return fmt.Errorf("failed to cast default value %v of property %s to an integer: %w", property.Default, name, err) - } - c.values[name] = v + c.values[name] = property.Default } return nil } diff --git a/libs/template/config_test.go b/libs/template/config_test.go index f51b95f64aa..d84833b78aa 100644 --- a/libs/template/config_test.go +++ b/libs/template/config_test.go @@ -1,7 +1,6 @@ package template import ( - "encoding/json" "testing" "github.com/databricks/cli/libs/jsonschema" @@ -10,28 +9,9 @@ import ( ) func testSchema(t *testing.T) *jsonschema.Schema { - schemaJson := `{ - "properties": { - "int_val": { - "type": "integer", - "default": 123 - }, - "float_val": { - "type": "number" - }, - "bool_val": { - "type": "boolean" - }, - "string_val": { - "type": "string", - "default": "abc" - } - } - }` - var jsonSchema jsonschema.Schema - err := json.Unmarshal([]byte(schemaJson), &jsonSchema) + s, err := jsonschema.Load("./testdata/config-test-schema/test-schema.json") require.NoError(t, err) - return &jsonSchema + return s } func TestTemplateConfigAssignValuesFromFile(t *testing.T) { diff --git a/libs/template/testdata/config-test-schema/test-schema.json b/libs/template/testdata/config-test-schema/test-schema.json new file mode 100644 index 00000000000..41eb825190f --- /dev/null +++ b/libs/template/testdata/config-test-schema/test-schema.json @@ -0,0 +1,18 @@ +{ + "properties": { + "int_val": { + "type": "integer", + "default": 123 + }, + "float_val": { + "type": "number" + }, + "bool_val": { + "type": "boolean" + }, + "string_val": { + "type": "string", + "default": "abc" + } + } +} From 634a2603591840484956761bc1846abe5005c734 Mon Sep 17 00:00:00 2001 From: monalisa Date: Wed, 6 Sep 2023 19:58:09 +0200 Subject: [PATCH 05/12] Add validation for required properties --- libs/jsonschema/instance.go | 14 +++++++++++ libs/jsonschema/instance_test.go | 25 +++++++++++++++++++ .../test-schema-some-fields-required.json | 19 ++++++++++++++ 3 files changed, 58 insertions(+) create mode 100644 libs/jsonschema/testdata/instance-validate/test-schema-some-fields-required.json diff --git a/libs/jsonschema/instance.go b/libs/jsonschema/instance.go index a2bdc01c61e..bd7d866c6f3 100644 --- a/libs/jsonschema/instance.go +++ b/libs/jsonschema/instance.go @@ -42,6 +42,9 @@ func (s *Schema) ValidateInstance(instance map[string]any) error { if err := s.validateAdditionalProperties(instance); err != nil { return err } + if err := s.validateRequired(instance); err != nil { + return err + } return s.validateTypes(instance) } @@ -60,6 +63,17 @@ func (s *Schema) validateAdditionalProperties(instance map[string]any) error { return nil } +// This function validates that all require properties in the schema have values +// in the instance. +func (s *Schema) validateRequired(instance map[string]any) error { + for _, name := range s.Required { + if _, ok := instance[name]; !ok { + return fmt.Errorf("no value provided for required property %s", name) + } + } + return nil +} + // Validates the types of all input properties values match their types defined in the schema func (s *Schema) validateTypes(instance map[string]any) error { for k, v := range instance { diff --git a/libs/jsonschema/instance_test.go b/libs/jsonschema/instance_test.go index 1e0404f3ac5..a338700a958 100644 --- a/libs/jsonschema/instance_test.go +++ b/libs/jsonschema/instance_test.go @@ -83,3 +83,28 @@ func TestValidateInstanceTypes(t *testing.T) { err = schema.ValidateInstance(invalidInstance) assert.EqualError(t, err, "incorrect type for property int_val: expected type integer, but value is \"abc\"") } + +func TestValidateInstanceRequired(t *testing.T) { + schema, err := Load("./testdata/instance-validate/test-schema-some-fields-required.json") + require.NoError(t, err) + + validInstance := map[string]any{ + "int_val": 1, + "float_val": 1.0, + "bool_val": false, + } + err = schema.validateRequired(validInstance) + assert.NoError(t, err) + err = schema.ValidateInstance(validInstance) + assert.NoError(t, err) + + invalidInstance := map[string]any{ + "string_val": "abc", + "float_val": 1.0, + "bool_val": false, + } + err = schema.validateRequired(invalidInstance) + assert.EqualError(t, err, "no value provided for required property int_val") + err = schema.ValidateInstance(invalidInstance) + assert.EqualError(t, err, "no value provided for required property int_val") +} diff --git a/libs/jsonschema/testdata/instance-validate/test-schema-some-fields-required.json b/libs/jsonschema/testdata/instance-validate/test-schema-some-fields-required.json new file mode 100644 index 00000000000..46581103454 --- /dev/null +++ b/libs/jsonschema/testdata/instance-validate/test-schema-some-fields-required.json @@ -0,0 +1,19 @@ +{ + "properties": { + "int_val": { + "type": "integer", + "default": 123 + }, + "float_val": { + "type": "number" + }, + "bool_val": { + "type": "boolean" + }, + "string_val": { + "type": "string", + "default": "abc" + } + }, + "required": ["int_val", "float_val", "bool_val"] +} From 189e7ad101dbdea3914eaf2259317c69f8d2ed09 Mon Sep 17 00:00:00 2001 From: monalisa Date: Thu, 7 Sep 2023 13:27:01 +0200 Subject: [PATCH 06/12] Replace template config validation with validation defined in the jsonschema package --- libs/template/config.go | 41 ++------------ libs/template/config_test.go | 102 ++++++++++++++--------------------- 2 files changed, 46 insertions(+), 97 deletions(-) diff --git a/libs/template/config.go b/libs/template/config.go index 3fb1cbcf2f3..94aff5ee6e3 100644 --- a/libs/template/config.go +++ b/libs/template/config.go @@ -6,6 +6,7 @@ import ( "github.com/databricks/cli/libs/cmdio" "github.com/databricks/cli/libs/jsonschema" + "golang.org/x/exp/maps" ) type config struct { @@ -127,42 +128,10 @@ func (c *config) promptOrAssignDefaultValues() error { // Validates the configuration. If passes, the configuration is ready to be used // to initialize the template. func (c *config) validate() error { - validateFns := []func() error{ - c.validateValuesDefined, - c.validateValuesType, - } - - for _, fn := range validateFns { - err := fn() - if err != nil { - return err - } - } - return nil -} - -// Validates all input properties have a user defined value assigned to them -func (c *config) validateValuesDefined() error { - for k := range c.schema.Properties { - if _, ok := c.values[k]; ok { - continue - } - return fmt.Errorf("no value has been assigned to input parameter %s", k) - } - return nil -} - -// Validates the types of all input properties values match their types defined in the schema -func (c *config) validateValuesType() error { - for k, v := range c.values { - fieldInfo, ok := c.schema.Properties[k] - if !ok { - return fmt.Errorf("%s is not defined as an input parameter for the template", k) - } - err := validateType(v, fieldInfo.Type) - if err != nil { - return fmt.Errorf("incorrect type for %s. %w", k, err) - } + // All properties in the JSON schema should have a value defined. + c.schema.Required = maps.Keys(c.schema.Properties) + if err := c.schema.ValidateInstance(c.values); err != nil { + return fmt.Errorf("validation for template input parameters failed. %w", err) } return nil } diff --git a/libs/template/config_test.go b/libs/template/config_test.go index d84833b78aa..b5d53eb7541 100644 --- a/libs/template/config_test.go +++ b/libs/template/config_test.go @@ -1,6 +1,7 @@ package template import ( + "context" "testing" "github.com/databricks/cli/libs/jsonschema" @@ -8,17 +9,14 @@ import ( "github.com/stretchr/testify/require" ) -func testSchema(t *testing.T) *jsonschema.Schema { - s, err := jsonschema.Load("./testdata/config-test-schema/test-schema.json") +func testConfig(t *testing.T) *config { + c, err := newConfig(context.Background(), "./testdata/config-test-schema/test-schema.json") require.NoError(t, err) - return s + return c } func TestTemplateConfigAssignValuesFromFile(t *testing.T) { - c := config{ - schema: testSchema(t), - values: make(map[string]any), - } + c := testConfig(t) err := c.assignValuesFromFile("./testdata/config-assign-from-file/config.json") assert.NoError(t, err) @@ -30,21 +28,16 @@ func TestTemplateConfigAssignValuesFromFile(t *testing.T) { } func TestTemplateConfigAssignValuesFromFileForInvalidIntegerValue(t *testing.T) { - c := config{ - schema: testSchema(t), - values: make(map[string]any), - } + c := testConfig(t) err := c.assignValuesFromFile("./testdata/config-assign-from-file-invalid-int/config.json") assert.EqualError(t, err, "failed to load config from file ./testdata/config-assign-from-file-invalid-int/config.json. failed to parse property int_val: cannot convert \"abc\" to an integer") } func TestTemplateConfigAssignValuesFromFileDoesNotOverwriteExistingConfigs(t *testing.T) { - c := config{ - schema: testSchema(t), - values: map[string]any{ - "string_val": "this-is-not-overwritten", - }, + c := testConfig(t) + c.values = map[string]any{ + "string_val": "this-is-not-overwritten", } err := c.assignValuesFromFile("./testdata/config-assign-from-file/config.json") @@ -57,10 +50,7 @@ func TestTemplateConfigAssignValuesFromFileDoesNotOverwriteExistingConfigs(t *te } func TestTemplateConfigAssignDefaultValues(t *testing.T) { - c := config{ - schema: testSchema(t), - values: make(map[string]any), - } + c := testConfig(t) err := c.assignDefaultValues() assert.NoError(t, err) @@ -71,65 +61,55 @@ func TestTemplateConfigAssignDefaultValues(t *testing.T) { } func TestTemplateConfigValidateValuesDefined(t *testing.T) { - c := config{ - schema: testSchema(t), - values: map[string]any{ - "int_val": 1, - "float_val": 1.0, - "bool_val": false, - }, + c := testConfig(t) + c.values = map[string]any{ + "int_val": 1, + "float_val": 1.0, + "bool_val": false, } - err := c.validateValuesDefined() - assert.EqualError(t, err, "no value has been assigned to input parameter string_val") + err := c.validate() + assert.EqualError(t, err, "validation for template input parameters failed. no value provided for required property string_val") } func TestTemplateConfigValidateTypeForValidConfig(t *testing.T) { - c := &config{ - schema: testSchema(t), - values: map[string]any{ - "int_val": 1, - "float_val": 1.1, - "bool_val": true, - "string_val": "abcd", - }, + c := testConfig(t) + c.values = map[string]any{ + "int_val": 1, + "float_val": 1.1, + "bool_val": true, + "string_val": "abcd", } - err := c.validateValuesType() - assert.NoError(t, err) - - err = c.validate() + err := c.validate() assert.NoError(t, err) } func TestTemplateConfigValidateTypeForUnknownField(t *testing.T) { - c := &config{ - schema: testSchema(t), - values: map[string]any{ - "unknown_prop": 1, - }, + c := testConfig(t) + c.values = map[string]any{ + "unknown_prop": 1, + "int_val": 1, + "float_val": 1.1, + "bool_val": true, + "string_val": "abcd", } - err := c.validateValuesType() - assert.EqualError(t, err, "unknown_prop is not defined as an input parameter for the template") + err := c.validate() + assert.EqualError(t, err, "validation for template input parameters failed. property unknown_prop is not defined in the schema") } func TestTemplateConfigValidateTypeForInvalidType(t *testing.T) { - c := &config{ - schema: testSchema(t), - values: map[string]any{ - "int_val": "this-should-be-an-int", - "float_val": 1.1, - "bool_val": true, - "string_val": "abcd", - }, + c := testConfig(t) + c.values = map[string]any{ + "int_val": "this-should-be-an-int", + "float_val": 1.1, + "bool_val": true, + "string_val": "abcd", } - err := c.validateValuesType() - assert.EqualError(t, err, `incorrect type for int_val. expected type integer, but value is "this-should-be-an-int"`) - - err = c.validate() - assert.EqualError(t, err, `incorrect type for int_val. expected type integer, but value is "this-should-be-an-int"`) + err := c.validate() + assert.EqualError(t, err, "validation for template input parameters failed. incorrect type for property int_val: expected type integer, but value is \"this-should-be-an-int\"") } func TestTemplateValidateSchema(t *testing.T) { From 7413bed8b13d004f1355f22d95d71b98066a97f8 Mon Sep 17 00:00:00 2001 From: monalisa Date: Thu, 7 Sep 2023 13:31:28 +0200 Subject: [PATCH 07/12] Remove validation type function from template library --- libs/template/validators.go | 58 -------------- libs/template/validators_test.go | 129 ------------------------------- 2 files changed, 187 deletions(-) delete mode 100644 libs/template/validators.go delete mode 100644 libs/template/validators_test.go diff --git a/libs/template/validators.go b/libs/template/validators.go deleted file mode 100644 index 209700b63c7..00000000000 --- a/libs/template/validators.go +++ /dev/null @@ -1,58 +0,0 @@ -package template - -import ( - "fmt" - "reflect" - "slices" - - "github.com/databricks/cli/libs/jsonschema" -) - -type validator func(v any) error - -func validateType(v any, fieldType jsonschema.Type) error { - validateFunc, ok := validators[fieldType] - if !ok { - return nil - } - return validateFunc(v) -} - -func validateString(v any) error { - if _, ok := v.(string); !ok { - return fmt.Errorf("expected type string, but value is %#v", v) - } - return nil -} - -func validateBoolean(v any) error { - if _, ok := v.(bool); !ok { - return fmt.Errorf("expected type boolean, but value is %#v", v) - } - return nil -} - -func validateNumber(v any) error { - if !slices.Contains([]reflect.Kind{reflect.Float32, reflect.Float64}, - reflect.TypeOf(v).Kind()) { - return fmt.Errorf("expected type float, but value is %#v", v) - } - return nil -} - -func validateInteger(v any) error { - if !slices.Contains([]reflect.Kind{reflect.Int, reflect.Int8, reflect.Int16, - reflect.Int32, reflect.Int64, reflect.Uint, reflect.Uint8, reflect.Uint16, - reflect.Uint32, reflect.Uint64}, - reflect.TypeOf(v).Kind()) { - return fmt.Errorf("expected type integer, but value is %#v", v) - } - return nil -} - -var validators map[jsonschema.Type]validator = map[jsonschema.Type]validator{ - jsonschema.StringType: validateString, - jsonschema.BooleanType: validateBoolean, - jsonschema.IntegerType: validateInteger, - jsonschema.NumberType: validateNumber, -} diff --git a/libs/template/validators_test.go b/libs/template/validators_test.go deleted file mode 100644 index f34f037a1ef..00000000000 --- a/libs/template/validators_test.go +++ /dev/null @@ -1,129 +0,0 @@ -package template - -import ( - "testing" - - "github.com/databricks/cli/libs/jsonschema" - "github.com/stretchr/testify/assert" -) - -func TestValidatorString(t *testing.T) { - err := validateString("abc") - assert.NoError(t, err) - - err = validateString(1) - assert.ErrorContains(t, err, "expected type string, but value is 1") - - err = validateString(true) - assert.ErrorContains(t, err, "expected type string, but value is true") - - err = validateString("false") - assert.NoError(t, err) -} - -func TestValidatorBoolean(t *testing.T) { - err := validateBoolean(true) - assert.NoError(t, err) - - err = validateBoolean(1) - assert.ErrorContains(t, err, "expected type boolean, but value is 1") - - err = validateBoolean("abc") - assert.ErrorContains(t, err, "expected type boolean, but value is \"abc\"") - - err = validateBoolean("false") - assert.ErrorContains(t, err, "expected type boolean, but value is \"false\"") -} - -func TestValidatorNumber(t *testing.T) { - err := validateNumber(true) - assert.ErrorContains(t, err, "expected type float, but value is true") - - err = validateNumber(int32(1)) - assert.ErrorContains(t, err, "expected type float, but value is 1") - - err = validateNumber(int64(2)) - assert.ErrorContains(t, err, "expected type float, but value is 2") - - err = validateNumber(float32(1)) - assert.NoError(t, err) - - err = validateNumber(float64(1)) - assert.NoError(t, err) - - err = validateNumber("abc") - assert.ErrorContains(t, err, "expected type float, but value is \"abc\"") -} - -func TestValidatorInt(t *testing.T) { - err := validateInteger(true) - assert.ErrorContains(t, err, "expected type integer, but value is true") - - err = validateInteger(int32(1)) - assert.NoError(t, err) - - err = validateInteger(int64(1)) - assert.NoError(t, err) - - err = validateInteger(float32(1)) - assert.ErrorContains(t, err, "expected type integer, but value is 1") - - err = validateInteger(float64(1)) - assert.ErrorContains(t, err, "expected type integer, but value is 1") - - err = validateInteger("abc") - assert.ErrorContains(t, err, "expected type integer, but value is \"abc\"") -} - -func TestTemplateValidateType(t *testing.T) { - // assert validation passing - err := validateType(int(0), jsonschema.IntegerType) - assert.NoError(t, err) - err = validateType(int32(1), jsonschema.IntegerType) - assert.NoError(t, err) - err = validateType(int64(1), jsonschema.IntegerType) - assert.NoError(t, err) - - err = validateType(float32(1.1), jsonschema.NumberType) - assert.NoError(t, err) - err = validateType(float64(1.2), jsonschema.NumberType) - assert.NoError(t, err) - - err = validateType(false, jsonschema.BooleanType) - assert.NoError(t, err) - - err = validateType("abc", jsonschema.StringType) - assert.NoError(t, err) - - // assert validation failing for integers - err = validateType(float64(1.2), jsonschema.IntegerType) - assert.ErrorContains(t, err, "expected type integer, but value is 1.2") - err = validateType(true, jsonschema.IntegerType) - assert.ErrorContains(t, err, "expected type integer, but value is true") - err = validateType("abc", jsonschema.IntegerType) - assert.ErrorContains(t, err, "expected type integer, but value is \"abc\"") - - // assert validation failing for floats - err = validateType(true, jsonschema.NumberType) - assert.ErrorContains(t, err, "expected type float, but value is true") - err = validateType("abc", jsonschema.NumberType) - assert.ErrorContains(t, err, "expected type float, but value is \"abc\"") - err = validateType(int(1), jsonschema.NumberType) - assert.ErrorContains(t, err, "expected type float, but value is 1") - - // assert validation failing for boolean - err = validateType(int(1), jsonschema.BooleanType) - assert.ErrorContains(t, err, "expected type boolean, but value is 1") - err = validateType(float64(1), jsonschema.BooleanType) - assert.ErrorContains(t, err, "expected type boolean, but value is 1") - err = validateType("abc", jsonschema.BooleanType) - assert.ErrorContains(t, err, "expected type boolean, but value is \"abc\"") - - // assert validation failing for string - err = validateType(int(1), jsonschema.StringType) - assert.ErrorContains(t, err, "expected type string, but value is 1") - err = validateType(float64(1), jsonschema.StringType) - assert.ErrorContains(t, err, "expected type string, but value is 1") - err = validateType(false, jsonschema.StringType) - assert.ErrorContains(t, err, "expected type string, but value is false") -} From d8ef933f8beb0d1bcfa6ab47b054e072080fd501 Mon Sep 17 00:00:00 2001 From: monalisa Date: Thu, 7 Sep 2023 13:47:31 +0200 Subject: [PATCH 08/12] Move to/from string methods to the json schema package. Delete utils.go from template library --- libs/jsonschema/utils.go | 68 ++++++++++++++++++- libs/jsonschema/utils_test.go | 71 ++++++++++++++++++++ libs/template/config.go | 4 +- libs/template/utils.go | 103 ----------------------------- libs/template/utils_test.go | 121 ---------------------------------- 5 files changed, 140 insertions(+), 227 deletions(-) delete mode 100644 libs/template/utils.go delete mode 100644 libs/template/utils_test.go diff --git a/libs/jsonschema/utils.go b/libs/jsonschema/utils.go index 34b51f4d94d..21866965e2a 100644 --- a/libs/jsonschema/utils.go +++ b/libs/jsonschema/utils.go @@ -1,6 +1,10 @@ package jsonschema -import "fmt" +import ( + "errors" + "fmt" + "strconv" +) // function to check whether a float value represents an integer func isIntegerValue(v float64) bool { @@ -33,3 +37,65 @@ func toInteger(v any) (int64, error) { return 0, fmt.Errorf("cannot convert %#v to an integer", v) } } + +func ToString(v any, T Type) (string, error) { + switch T { + case BooleanType: + boolVal, ok := v.(bool) + if !ok { + return "", fmt.Errorf("expected bool, got: %#v", v) + } + return strconv.FormatBool(boolVal), nil + case StringType: + strVal, ok := v.(string) + if !ok { + return "", fmt.Errorf("expected string, got: %#v", v) + } + return strVal, nil + case NumberType: + floatVal, ok := v.(float64) + if !ok { + return "", fmt.Errorf("expected float, got: %#v", v) + } + return strconv.FormatFloat(floatVal, 'f', -1, 64), nil + case IntegerType: + intVal, err := toInteger(v) + if err != nil { + return "", err + } + return strconv.FormatInt(intVal, 10), nil + case ArrayType, ObjectType: + return "", fmt.Errorf("cannot format object of type %s as a string. Value of object: %#v", T, v) + default: + return "", fmt.Errorf("unknown json schema type: %q", T) + } +} + +func FromString(s string, T Type) (any, error) { + if T == StringType { + return s, nil + } + + // Variables to store value and error from parsing + var v any + var err error + + switch T { + case BooleanType: + v, err = strconv.ParseBool(s) + case NumberType: + v, err = strconv.ParseFloat(s, 32) + case IntegerType: + v, err = strconv.ParseInt(s, 10, 64) + case ArrayType, ObjectType: + return "", fmt.Errorf("cannot parse string as object of type %s. Value of string: %q", T, s) + default: + return "", fmt.Errorf("unknown json schema type: %q", T) + } + + // Return more readable error incase of a syntax error + if errors.Is(err, strconv.ErrSyntax) { + return nil, fmt.Errorf("could not parse %q as a %s: %w", s, T, err) + } + return v, err +} diff --git a/libs/jsonschema/utils_test.go b/libs/jsonschema/utils_test.go index 799b26ecaf9..9686cf39bbd 100644 --- a/libs/jsonschema/utils_test.go +++ b/libs/jsonschema/utils_test.go @@ -47,3 +47,74 @@ func TestTemplateToInteger(t *testing.T) { _, err = toInteger("abcd") assert.EqualError(t, err, "cannot convert \"abcd\" to an integer") } + +func TestTemplateToString(t *testing.T) { + s, err := ToString(true, BooleanType) + assert.NoError(t, err) + assert.Equal(t, "true", s) + + s, err = ToString("abc", StringType) + assert.NoError(t, err) + assert.Equal(t, "abc", s) + + s, err = ToString(1.1, NumberType) + assert.NoError(t, err) + assert.Equal(t, "1.1", s) + + s, err = ToString(2, IntegerType) + assert.NoError(t, err) + assert.Equal(t, "2", s) + + _, err = ToString([]string{}, ArrayType) + assert.EqualError(t, err, "cannot format object of type array as a string. Value of object: []string{}") + + _, err = ToString("true", BooleanType) + assert.EqualError(t, err, "expected bool, got: \"true\"") + + _, err = ToString(123, StringType) + assert.EqualError(t, err, "expected string, got: 123") + + _, err = ToString(false, NumberType) + assert.EqualError(t, err, "expected float, got: false") + + _, err = ToString("abc", IntegerType) + assert.EqualError(t, err, "cannot convert \"abc\" to an integer") + + _, err = ToString("abc", "foobar") + assert.EqualError(t, err, "unknown json schema type: \"foobar\"") +} + +func TestTemplateFromString(t *testing.T) { + v, err := FromString("true", BooleanType) + assert.NoError(t, err) + assert.Equal(t, true, v) + + v, err = FromString("abc", StringType) + assert.NoError(t, err) + assert.Equal(t, "abc", v) + + v, err = FromString("1.1", NumberType) + assert.NoError(t, err) + // Floating point conversions are not perfect + assert.True(t, (v.(float64)-1.1) < 0.000001) + + v, err = FromString("12345", IntegerType) + assert.NoError(t, err) + assert.Equal(t, int64(12345), v) + + v, err = FromString("123", NumberType) + assert.NoError(t, err) + assert.Equal(t, float64(123), v) + + _, err = FromString("qrt", ArrayType) + assert.EqualError(t, err, "cannot parse string as object of type array. Value of string: \"qrt\"") + + _, err = FromString("abc", IntegerType) + assert.EqualError(t, err, "could not parse \"abc\" as a integer: strconv.ParseInt: parsing \"abc\": invalid syntax") + + _, err = FromString("1.0", IntegerType) + assert.EqualError(t, err, "could not parse \"1.0\" as a integer: strconv.ParseInt: parsing \"1.0\": invalid syntax") + + _, err = FromString("1.0", "foobar") + assert.EqualError(t, err, "unknown json schema type: \"foobar\"") +} diff --git a/libs/template/config.go b/libs/template/config.go index 94aff5ee6e3..b88ee7e3350 100644 --- a/libs/template/config.go +++ b/libs/template/config.go @@ -95,7 +95,7 @@ func (c *config) promptForValues() error { var defaultVal string var err error if property.Default != nil { - defaultVal, err = toString(property.Default, property.Type) + defaultVal, err = jsonschema.ToString(property.Default, property.Type) if err != nil { return err } @@ -108,7 +108,7 @@ func (c *config) promptForValues() error { } // Convert user input string back to a value - c.values[name], err = fromString(userInput, property.Type) + c.values[name], err = jsonschema.FromString(userInput, property.Type) if err != nil { return err } diff --git a/libs/template/utils.go b/libs/template/utils.go deleted file mode 100644 index ade6a573058..00000000000 --- a/libs/template/utils.go +++ /dev/null @@ -1,103 +0,0 @@ -package template - -import ( - "errors" - "fmt" - "strconv" - - "github.com/databricks/cli/libs/jsonschema" -) - -// function to check whether a float value represents an integer -func isIntegerValue(v float64) bool { - return v == float64(int64(v)) -} - -func toInteger(v any) (int64, error) { - switch typedVal := v.(type) { - // cast float to int - case float32: - if !isIntegerValue(float64(typedVal)) { - return 0, fmt.Errorf("expected integer value, got: %v", v) - } - return int64(typedVal), nil - case float64: - if !isIntegerValue(typedVal) { - return 0, fmt.Errorf("expected integer value, got: %v", v) - } - return int64(typedVal), nil - - // pass through common integer cases - case int: - return int64(typedVal), nil - case int32: - return int64(typedVal), nil - case int64: - return typedVal, nil - - default: - return 0, fmt.Errorf("cannot convert %#v to an integer", v) - } -} - -func toString(v any, T jsonschema.Type) (string, error) { - switch T { - case jsonschema.BooleanType: - boolVal, ok := v.(bool) - if !ok { - return "", fmt.Errorf("expected bool, got: %#v", v) - } - return strconv.FormatBool(boolVal), nil - case jsonschema.StringType: - strVal, ok := v.(string) - if !ok { - return "", fmt.Errorf("expected string, got: %#v", v) - } - return strVal, nil - case jsonschema.NumberType: - floatVal, ok := v.(float64) - if !ok { - return "", fmt.Errorf("expected float, got: %#v", v) - } - return strconv.FormatFloat(floatVal, 'f', -1, 64), nil - case jsonschema.IntegerType: - intVal, err := toInteger(v) - if err != nil { - return "", err - } - return strconv.FormatInt(intVal, 10), nil - case jsonschema.ArrayType, jsonschema.ObjectType: - return "", fmt.Errorf("cannot format object of type %s as a string. Value of object: %#v", T, v) - default: - return "", fmt.Errorf("unknown json schema type: %q", T) - } -} - -func fromString(s string, T jsonschema.Type) (any, error) { - if T == jsonschema.StringType { - return s, nil - } - - // Variables to store value and error from parsing - var v any - var err error - - switch T { - case jsonschema.BooleanType: - v, err = strconv.ParseBool(s) - case jsonschema.NumberType: - v, err = strconv.ParseFloat(s, 32) - case jsonschema.IntegerType: - v, err = strconv.ParseInt(s, 10, 64) - case jsonschema.ArrayType, jsonschema.ObjectType: - return "", fmt.Errorf("cannot parse string as object of type %s. Value of string: %q", T, s) - default: - return "", fmt.Errorf("unknown json schema type: %q", T) - } - - // Return more readable error incase of a syntax error - if errors.Is(err, strconv.ErrSyntax) { - return nil, fmt.Errorf("could not parse %q as a %s: %w", s, T, err) - } - return v, err -} diff --git a/libs/template/utils_test.go b/libs/template/utils_test.go deleted file mode 100644 index 1e038aac6f8..00000000000 --- a/libs/template/utils_test.go +++ /dev/null @@ -1,121 +0,0 @@ -package template - -import ( - "math" - "testing" - - "github.com/databricks/cli/libs/jsonschema" - "github.com/stretchr/testify/assert" -) - -func TestTemplateIsInteger(t *testing.T) { - assert.False(t, isIntegerValue(1.1)) - assert.False(t, isIntegerValue(0.1)) - assert.False(t, isIntegerValue(-0.1)) - - assert.True(t, isIntegerValue(-1.0)) - assert.True(t, isIntegerValue(0.0)) - assert.True(t, isIntegerValue(2.0)) -} - -func TestTemplateToInteger(t *testing.T) { - v, err := toInteger(float32(2)) - assert.NoError(t, err) - assert.Equal(t, int64(2), v) - - v, err = toInteger(float64(4)) - assert.NoError(t, err) - assert.Equal(t, int64(4), v) - - v, err = toInteger(float64(4)) - assert.NoError(t, err) - assert.Equal(t, int64(4), v) - - v, err = toInteger(float64(math.MaxInt32 + 10)) - assert.NoError(t, err) - assert.Equal(t, int64(2147483657), v) - - v, err = toInteger(2) - assert.NoError(t, err) - assert.Equal(t, int64(2), v) - - _, err = toInteger(float32(2.2)) - assert.EqualError(t, err, "expected integer value, got: 2.2") - - _, err = toInteger(float64(math.MaxInt32 + 100.1)) - assert.ErrorContains(t, err, "expected integer value, got: 2.1474837471e+09") - - _, err = toInteger("abcd") - assert.EqualError(t, err, "cannot convert \"abcd\" to an integer") -} - -func TestTemplateToString(t *testing.T) { - s, err := toString(true, jsonschema.BooleanType) - assert.NoError(t, err) - assert.Equal(t, "true", s) - - s, err = toString("abc", jsonschema.StringType) - assert.NoError(t, err) - assert.Equal(t, "abc", s) - - s, err = toString(1.1, jsonschema.NumberType) - assert.NoError(t, err) - assert.Equal(t, "1.1", s) - - s, err = toString(2, jsonschema.IntegerType) - assert.NoError(t, err) - assert.Equal(t, "2", s) - - _, err = toString([]string{}, jsonschema.ArrayType) - assert.EqualError(t, err, "cannot format object of type array as a string. Value of object: []string{}") - - _, err = toString("true", jsonschema.BooleanType) - assert.EqualError(t, err, "expected bool, got: \"true\"") - - _, err = toString(123, jsonschema.StringType) - assert.EqualError(t, err, "expected string, got: 123") - - _, err = toString(false, jsonschema.NumberType) - assert.EqualError(t, err, "expected float, got: false") - - _, err = toString("abc", jsonschema.IntegerType) - assert.EqualError(t, err, "cannot convert \"abc\" to an integer") - - _, err = toString("abc", "foobar") - assert.EqualError(t, err, "unknown json schema type: \"foobar\"") -} - -func TestTemplateFromString(t *testing.T) { - v, err := fromString("true", jsonschema.BooleanType) - assert.NoError(t, err) - assert.Equal(t, true, v) - - v, err = fromString("abc", jsonschema.StringType) - assert.NoError(t, err) - assert.Equal(t, "abc", v) - - v, err = fromString("1.1", jsonschema.NumberType) - assert.NoError(t, err) - // Floating point conversions are not perfect - assert.True(t, (v.(float64)-1.1) < 0.000001) - - v, err = fromString("12345", jsonschema.IntegerType) - assert.NoError(t, err) - assert.Equal(t, int64(12345), v) - - v, err = fromString("123", jsonschema.NumberType) - assert.NoError(t, err) - assert.Equal(t, float64(123), v) - - _, err = fromString("qrt", jsonschema.ArrayType) - assert.EqualError(t, err, "cannot parse string as object of type array. Value of string: \"qrt\"") - - _, err = fromString("abc", jsonschema.IntegerType) - assert.EqualError(t, err, "could not parse \"abc\" as a integer: strconv.ParseInt: parsing \"abc\": invalid syntax") - - _, err = fromString("1.0", jsonschema.IntegerType) - assert.EqualError(t, err, "could not parse \"1.0\" as a integer: strconv.ParseInt: parsing \"1.0\": invalid syntax") - - _, err = fromString("1.0", "foobar") - assert.EqualError(t, err, "unknown json schema type: \"foobar\"") -} From a741d2b29d274c1c6ea6ee7193245164a6558614 Mon Sep 17 00:00:00 2001 From: monalisa Date: Thu, 7 Sep 2023 15:06:21 +0200 Subject: [PATCH 09/12] - --- libs/jsonschema/schema.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/libs/jsonschema/schema.go b/libs/jsonschema/schema.go index a575f181c7a..6bd0c02cedf 100644 --- a/libs/jsonschema/schema.go +++ b/libs/jsonschema/schema.go @@ -98,7 +98,7 @@ func Load(path string) (*Schema, error) { return nil, err } - // Convert the top properties default values and enum values to integers. + // Convert the top properties default values to integers. // This is require because the default JSON unmarshaler parses untyped numbers // as floats. for name, property := range schema.Properties { From f2acc0924a76affb82a10377fcb238d9a53d946f Mon Sep 17 00:00:00 2001 From: monalisa Date: Thu, 7 Sep 2023 15:55:30 +0200 Subject: [PATCH 10/12] Add unit test for LoadInstance --- libs/jsonschema/instance_test.go | 19 +++++++++++++++++++ .../instance-load/invalid-type-instance.json | 6 ++++++ .../instance-load/valid-instance.json | 6 ++++++ 3 files changed, 31 insertions(+) create mode 100644 libs/jsonschema/testdata/instance-load/invalid-type-instance.json create mode 100644 libs/jsonschema/testdata/instance-load/valid-instance.json diff --git a/libs/jsonschema/instance_test.go b/libs/jsonschema/instance_test.go index a338700a958..d5e0766dd23 100644 --- a/libs/jsonschema/instance_test.go +++ b/libs/jsonschema/instance_test.go @@ -108,3 +108,22 @@ func TestValidateInstanceRequired(t *testing.T) { err = schema.ValidateInstance(invalidInstance) assert.EqualError(t, err, "no value provided for required property int_val") } + +func TestLoadInstance(t *testing.T) { + schema, err := Load("./testdata/instance-validate/test-schema.json") + require.NoError(t, err) + + // Expect the instance to be loaded successfully. + instance, err := schema.LoadInstance("./testdata/instance-load/valid-instance.json") + assert.NoError(t, err) + assert.Equal(t, map[string]any{ + "bool_val": false, + "int_val": int64(1), + "string_val": "abc", + "float_val": 2.0, + }, instance) + + // Expect instance validation against the schema to fail. + _, err = schema.LoadInstance("./testdata/instance-load/invalid-type-instance.json") + assert.EqualError(t, err, "incorrect type for property string_val: expected type string, but value is 123") +} diff --git a/libs/jsonschema/testdata/instance-load/invalid-type-instance.json b/libs/jsonschema/testdata/instance-load/invalid-type-instance.json new file mode 100644 index 00000000000..c55b6fccb72 --- /dev/null +++ b/libs/jsonschema/testdata/instance-load/invalid-type-instance.json @@ -0,0 +1,6 @@ +{ + "int_val": 1, + "bool_val": false, + "string_val": 123, + "float_val": 3.0 +} diff --git a/libs/jsonschema/testdata/instance-load/valid-instance.json b/libs/jsonschema/testdata/instance-load/valid-instance.json new file mode 100644 index 00000000000..7d4dc818ad4 --- /dev/null +++ b/libs/jsonschema/testdata/instance-load/valid-instance.json @@ -0,0 +1,6 @@ +{ + "int_val": 1, + "bool_val": false, + "string_val": "abc", + "float_val": 2.0 +} From 369e8b0f3bdb96af788b6d51d663ee66e8f50348 Mon Sep 17 00:00:00 2001 From: monalisa Date: Thu, 7 Sep 2023 16:26:30 +0200 Subject: [PATCH 11/12] address commments --- libs/jsonschema/instance.go | 4 ++-- libs/jsonschema/schema.go | 10 +++++++--- libs/template/config.go | 2 +- libs/template/config_test.go | 2 +- 4 files changed, 11 insertions(+), 7 deletions(-) diff --git a/libs/jsonschema/instance.go b/libs/jsonschema/instance.go index bd7d866c6f3..2ff669aa78f 100644 --- a/libs/jsonschema/instance.go +++ b/libs/jsonschema/instance.go @@ -6,10 +6,10 @@ import ( "os" ) -// Load a JSON document, also validating it against the JSON schema. Instance here +// Load a JSON document and validate it against the JSON schema. Instance here // refers to a JSON document. see: https://json-schema.org/draft/2020-12/json-schema-core.html#name-instance func (s *Schema) LoadInstance(path string) (map[string]any, error) { - instance := make(map[string]any, 0) + instance := make(map[string]any) b, err := os.ReadFile(path) if err != nil { return nil, err diff --git a/libs/jsonschema/schema.go b/libs/jsonschema/schema.go index 6bd0c02cedf..44c65ecc6a5 100644 --- a/libs/jsonschema/schema.go +++ b/libs/jsonschema/schema.go @@ -98,9 +98,13 @@ func Load(path string) (*Schema, error) { return nil, err } - // Convert the top properties default values to integers. - // This is require because the default JSON unmarshaler parses untyped numbers - // as floats. + // Convert the default values of top-level properties to integers. + // This is required because the default JSON unmarshaler parses numbers + // as floats when the Golang field it's being loaded to is untyped. + // + // NOTE: properties can be recursively defined in a schema, but the current + // use-cases only uses the first layer of properties so we skip converting + // any recursive properties. for name, property := range schema.Properties { if property.Type != IntegerType { continue diff --git a/libs/template/config.go b/libs/template/config.go index b88ee7e3350..6f980f61319 100644 --- a/libs/template/config.go +++ b/libs/template/config.go @@ -50,7 +50,7 @@ func (c *config) assignValuesFromFile(path string) error { // Load the config file. configFromFile, err := c.schema.LoadInstance(path) if err != nil { - return fmt.Errorf("failed to load config from file %s. %w", path, err) + return fmt.Errorf("failed to load config from file %s: %w", path, err) } // Write configs from the file to the input map, not overwriting any existing diff --git a/libs/template/config_test.go b/libs/template/config_test.go index b5d53eb7541..bba22c75875 100644 --- a/libs/template/config_test.go +++ b/libs/template/config_test.go @@ -31,7 +31,7 @@ func TestTemplateConfigAssignValuesFromFileForInvalidIntegerValue(t *testing.T) c := testConfig(t) err := c.assignValuesFromFile("./testdata/config-assign-from-file-invalid-int/config.json") - assert.EqualError(t, err, "failed to load config from file ./testdata/config-assign-from-file-invalid-int/config.json. failed to parse property int_val: cannot convert \"abc\" to an integer") + assert.EqualError(t, err, "failed to load config from file ./testdata/config-assign-from-file-invalid-int/config.json: failed to parse property int_val: cannot convert \"abc\" to an integer") } func TestTemplateConfigAssignValuesFromFileDoesNotOverwriteExistingConfigs(t *testing.T) { From ec0da3de8e46e91773d6e1a79e11830c4fb8a4ef Mon Sep 17 00:00:00 2001 From: monalisa Date: Thu, 7 Sep 2023 16:29:45 +0200 Subject: [PATCH 12/12] - --- libs/jsonschema/instance.go | 1 + 1 file changed, 1 insertion(+) diff --git a/libs/jsonschema/instance.go b/libs/jsonschema/instance.go index 2ff669aa78f..02ab9f281f9 100644 --- a/libs/jsonschema/instance.go +++ b/libs/jsonschema/instance.go @@ -51,6 +51,7 @@ func (s *Schema) ValidateInstance(instance map[string]any) error { // If additional properties is set to false, this function validates instance only // contains properties defined in the schema. func (s *Schema) validateAdditionalProperties(instance map[string]any) error { + // Note: AdditionalProperties has the type any. if s.AdditionalProperties != false { return nil }