From fc4c55ba35be8063bd5ce805342a1619bdc15e61 Mon Sep 17 00:00:00 2001 From: monalisa Date: Tue, 15 Aug 2023 23:22:33 +0200 Subject: [PATCH 01/13] Add enum support for bundle templates --- libs/cmdio/logger.go | 54 +++++++++++++++++++++++++++++++++++++++ libs/jsonschema/schema.go | 2 ++ libs/template/config.go | 24 ++++++++++++++--- 3 files changed, 77 insertions(+), 3 deletions(-) diff --git a/libs/cmdio/logger.go b/libs/cmdio/logger.go index 0663306e134..5082be35ef3 100644 --- a/libs/cmdio/logger.go +++ b/libs/cmdio/logger.go @@ -10,6 +10,8 @@ import ( "strings" "github.com/databricks/cli/libs/flags" + "golang.org/x/exp/maps" + "golang.org/x/exp/slices" ) // This is the interface for all io interactions with a user @@ -104,6 +106,58 @@ func AskYesOrNo(ctx context.Context, question string) (bool, error) { return false, nil } +func AskChoice(ctx context.Context, question string, defaultVal string, choices []string) (string, error) { + logger, ok := FromContext(ctx) + if !ok { + logger = Default() + } + + // Find index of the default value + defaultIndex := "" + for i, v := range choices { + if v == defaultVal { + defaultIndex = fmt.Sprint(i + 1) + break + } + } + + // If default value is not present, return error + if defaultIndex == "" { + return "", fmt.Errorf("failed to find default value %s among choices %#v", defaultVal, choices) + } + + indexToChoice := make(map[string]string, 0) + question += ":\n" + for index, choice := range choices { + // Map choice values against an string representation of their indices. + // This helps read choice value corresponding to the index user enters + choiceIndex := fmt.Sprint(index + 1) + indexToChoice[choiceIndex] = choice + + // Add this choice as a option in the prompt + question += fmt.Sprintf("%s. %s\n", choiceIndex, choice) + + } + + // Add text informing user of valid options to choose from + question += fmt.Sprintf("Choose from %s", strings.Join(maps.Keys(indexToChoice), ", ")) + + // prompt the user. + ans, err := logger.Ask(question, defaultIndex) + if err != nil { + return "", err + } + + choice, ok := indexToChoice[ans] + if !ok { + expectedOptions := maps.Keys(indexToChoice) + slices.Sort(expectedOptions) + return "", fmt.Errorf("expected one of %s. Got: %s", strings.Join(expectedOptions, ", "), ans) + } + + return choice, nil +} + func (l *Logger) Ask(question string, defaultVal string) (string, error) { if l.Mode == flags.ModeJson { return "", fmt.Errorf("question prompts are not supported in json mode") diff --git a/libs/jsonschema/schema.go b/libs/jsonschema/schema.go index c0d1736c1dc..b96784bf6ee 100644 --- a/libs/jsonschema/schema.go +++ b/libs/jsonschema/schema.go @@ -40,6 +40,8 @@ type Schema struct { // Default value for the property / object Default any `json:"default,omitempty"` + + Enum []any `json:"enum,omitempty"` } type Type string diff --git a/libs/template/config.go b/libs/template/config.go index 302a1361933..682a37ac8c2 100644 --- a/libs/template/config.go +++ b/libs/template/config.go @@ -134,9 +134,27 @@ func (c *config) promptForValues() error { } // Get user input by running the prompt - userInput, err := cmdio.Ask(c.ctx, property.Description, defaultVal) - if err != nil { - return err + var userInput string + if property.Enum != nil { + // convert list of enums to string + enums := []string{} + for _, enum := range property.Enum { + v, err := toString(enum, property.Type) + if err != nil { + return err + } + enums = append(enums, v) + } + userInput, err = cmdio.AskChoice(c.ctx, property.Description, defaultVal, enums) + if err != nil { + return err + } + } else { + userInput, err = cmdio.Ask(c.ctx, property.Description, defaultVal) + if err != nil { + return err + } + } // Convert user input string back to a value From 45c48e439db27192b17c136163f551d86c1a42f7 Mon Sep 17 00:00:00 2001 From: monalisa Date: Tue, 15 Aug 2023 23:34:08 +0200 Subject: [PATCH 02/13] some tests and lint --- libs/cmdio/logger.go | 2 +- libs/cmdio/logger_test.go | 18 ++++++++++++++++++ libs/jsonschema/schema.go | 2 ++ 3 files changed, 21 insertions(+), 1 deletion(-) diff --git a/libs/cmdio/logger.go b/libs/cmdio/logger.go index 5082be35ef3..393a3d465fc 100644 --- a/libs/cmdio/logger.go +++ b/libs/cmdio/logger.go @@ -123,7 +123,7 @@ func AskChoice(ctx context.Context, question string, defaultVal string, choices // If default value is not present, return error if defaultIndex == "" { - return "", fmt.Errorf("failed to find default value %s among choices %#v", defaultVal, choices) + return "", fmt.Errorf("failed to find default value %q among choices: %#v", defaultVal, choices) } indexToChoice := make(map[string]string, 0) diff --git a/libs/cmdio/logger_test.go b/libs/cmdio/logger_test.go index da6190462e0..dd6ca6e94df 100644 --- a/libs/cmdio/logger_test.go +++ b/libs/cmdio/logger_test.go @@ -1,6 +1,7 @@ package cmdio import ( + "context" "testing" "github.com/databricks/cli/libs/flags" @@ -12,3 +13,20 @@ func TestAskFailedInJsonMode(t *testing.T) { _, err := l.Ask("What is your spirit animal?", "") assert.ErrorContains(t, err, "question prompts are not supported in json mode") } + +func TestAskChoiceFailsInJsonMode(t *testing.T) { + l := NewLogger(flags.ModeJson) + ctx := NewContext(context.Background(), l) + + _, err := AskChoice(ctx, "what is a question?", "a", []string{"b", "c", "a"}) + assert.EqualError(t, err, "question prompts are not supported in json mode") +} + +func TestAskChoiceDefaultValueAbsent(t *testing.T) { + l := NewLogger(flags.ModeJson) + ctx := NewContext(context.Background(), l) + + // Expect error that default value is missing from choices. + _, err := AskChoice(ctx, "what is a question?", "a", []string{"b", "c", "d"}) + assert.EqualError(t, err, "failed to find default value \"a\" among choices: []string{\"b\", \"c\", \"d\"}") +} diff --git a/libs/jsonschema/schema.go b/libs/jsonschema/schema.go index b96784bf6ee..4cb1bb0f4d7 100644 --- a/libs/jsonschema/schema.go +++ b/libs/jsonschema/schema.go @@ -41,6 +41,8 @@ type Schema struct { // Default value for the property / object Default any `json:"default,omitempty"` + // If specified this is the list of valid values for the object. Values + // here should be consistent with the type defined in the schema. Enum []any `json:"enum,omitempty"` } From e5e6aa0d676ec75e613134e81a8a33558020cccb Mon Sep 17 00:00:00 2001 From: monalisa Date: Wed, 16 Aug 2023 11:23:48 +0200 Subject: [PATCH 03/13] comments refine --- libs/cmdio/logger.go | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/libs/cmdio/logger.go b/libs/cmdio/logger.go index 393a3d465fc..0a824ef592f 100644 --- a/libs/cmdio/logger.go +++ b/libs/cmdio/logger.go @@ -129,17 +129,17 @@ func AskChoice(ctx context.Context, question string, defaultVal string, choices indexToChoice := make(map[string]string, 0) question += ":\n" for index, choice := range choices { - // Map choice values against an string representation of their indices. - // This helps read choice value corresponding to the index user enters + // Map choices against a string representation of their indices. + // This helps resolve the choice corresponding to the index the user enters. choiceIndex := fmt.Sprint(index + 1) indexToChoice[choiceIndex] = choice - // Add this choice as a option in the prompt + // Add this choice as a option in the prompt text. question += fmt.Sprintf("%s. %s\n", choiceIndex, choice) } - // Add text informing user of valid options to choose from + // Add text informing user of the list of valid options to choose from question += fmt.Sprintf("Choose from %s", strings.Join(maps.Keys(indexToChoice), ", ")) // prompt the user. From cb1ebaee22a64ab4c018eef3ece7ab67dd5a2ab0 Mon Sep 17 00:00:00 2001 From: monalisa Date: Wed, 16 Aug 2023 15:20:27 +0200 Subject: [PATCH 04/13] use promptui for select --- libs/cmdio/logger.go | 61 ++++++++++++++------------------------- libs/cmdio/logger_test.go | 11 +------ libs/template/config.go | 2 +- 3 files changed, 23 insertions(+), 51 deletions(-) diff --git a/libs/cmdio/logger.go b/libs/cmdio/logger.go index 0a824ef592f..f0a267f7104 100644 --- a/libs/cmdio/logger.go +++ b/libs/cmdio/logger.go @@ -10,8 +10,7 @@ import ( "strings" "github.com/databricks/cli/libs/flags" - "golang.org/x/exp/maps" - "golang.org/x/exp/slices" + "github.com/manifoldco/promptui" ) // This is the interface for all io interactions with a user @@ -106,56 +105,38 @@ func AskYesOrNo(ctx context.Context, question string) (bool, error) { return false, nil } -func AskChoice(ctx context.Context, question string, defaultVal string, choices []string) (string, error) { +// TODO: allow enums without a default value specified. +// TODO: we require enum validation for both config-file and any --parameters introduced. +// TODO: add validation for default being in enum list. + +func AskSelect(ctx context.Context, question string, choices []string) (string, error) { logger, ok := FromContext(ctx) if !ok { logger = Default() } + return logger.AskSelect(question, choices) +} - // Find index of the default value - defaultIndex := "" - for i, v := range choices { - if v == defaultVal { - defaultIndex = fmt.Sprint(i + 1) - break - } +func (l *Logger) AskSelect(question string, choices []string) (string, error) { + if l.Mode == flags.ModeJson { + return "", fmt.Errorf("question prompts are not supported in json mode") } - // If default value is not present, return error - if defaultIndex == "" { - return "", fmt.Errorf("failed to find default value %q among choices: %#v", defaultVal, choices) + prompt := promptui.Select{ + Label: question, + Items: choices, + HideHelp: true, + Templates: &promptui.SelectTemplates{ + Label: "{{.}}: ", + Selected: fmt.Sprintf("%s: {{.}}", question), + }, } - indexToChoice := make(map[string]string, 0) - question += ":\n" - for index, choice := range choices { - // Map choices against a string representation of their indices. - // This helps resolve the choice corresponding to the index the user enters. - choiceIndex := fmt.Sprint(index + 1) - indexToChoice[choiceIndex] = choice - - // Add this choice as a option in the prompt text. - question += fmt.Sprintf("%s. %s\n", choiceIndex, choice) - - } - - // Add text informing user of the list of valid options to choose from - question += fmt.Sprintf("Choose from %s", strings.Join(maps.Keys(indexToChoice), ", ")) - - // prompt the user. - ans, err := logger.Ask(question, defaultIndex) + _, ans, err := prompt.Run() if err != nil { return "", err } - - choice, ok := indexToChoice[ans] - if !ok { - expectedOptions := maps.Keys(indexToChoice) - slices.Sort(expectedOptions) - return "", fmt.Errorf("expected one of %s. Got: %s", strings.Join(expectedOptions, ", "), ans) - } - - return choice, nil + return ans, nil } func (l *Logger) Ask(question string, defaultVal string) (string, error) { diff --git a/libs/cmdio/logger_test.go b/libs/cmdio/logger_test.go index dd6ca6e94df..c5c00d022ff 100644 --- a/libs/cmdio/logger_test.go +++ b/libs/cmdio/logger_test.go @@ -18,15 +18,6 @@ func TestAskChoiceFailsInJsonMode(t *testing.T) { l := NewLogger(flags.ModeJson) ctx := NewContext(context.Background(), l) - _, err := AskChoice(ctx, "what is a question?", "a", []string{"b", "c", "a"}) + _, err := AskSelect(ctx, "what is a question?", []string{"b", "c", "a"}) assert.EqualError(t, err, "question prompts are not supported in json mode") } - -func TestAskChoiceDefaultValueAbsent(t *testing.T) { - l := NewLogger(flags.ModeJson) - ctx := NewContext(context.Background(), l) - - // Expect error that default value is missing from choices. - _, err := AskChoice(ctx, "what is a question?", "a", []string{"b", "c", "d"}) - assert.EqualError(t, err, "failed to find default value \"a\" among choices: []string{\"b\", \"c\", \"d\"}") -} diff --git a/libs/template/config.go b/libs/template/config.go index 682a37ac8c2..89038023b7d 100644 --- a/libs/template/config.go +++ b/libs/template/config.go @@ -145,7 +145,7 @@ func (c *config) promptForValues() error { } enums = append(enums, v) } - userInput, err = cmdio.AskChoice(c.ctx, property.Description, defaultVal, enums) + userInput, err = cmdio.AskSelect(c.ctx, property.Description, enums) if err != nil { return err } From 4bd5cf2bc96c2f432195e140a2c32dcbd1876f99 Mon Sep 17 00:00:00 2001 From: monalisa Date: Wed, 16 Aug 2023 15:57:29 +0200 Subject: [PATCH 05/13] add validation for enum values --- libs/template/config.go | 35 ++++++++++++++++++++++++++++------- libs/template/config_test.go | 29 +++++++++++++++++++++++++++++ libs/template/utils.go | 12 ++++++++++++ 3 files changed, 69 insertions(+), 7 deletions(-) diff --git a/libs/template/config.go b/libs/template/config.go index 89038023b7d..317e5418fd1 100644 --- a/libs/template/config.go +++ b/libs/template/config.go @@ -5,6 +5,9 @@ import ( "encoding/json" "fmt" "os" + "strings" + + "slices" "github.com/databricks/cli/libs/cmdio" "github.com/databricks/cli/libs/jsonschema" @@ -137,13 +140,9 @@ func (c *config) promptForValues() error { var userInput string if property.Enum != nil { // convert list of enums to string - enums := []string{} - for _, enum := range property.Enum { - v, err := toString(enum, property.Type) - if err != nil { - return err - } - enums = append(enums, v) + enums, err := toStringSlice(property.Enum, property.Type) + if err != nil { + return err } userInput, err = cmdio.AskSelect(c.ctx, property.Description, enums) if err != nil { @@ -181,6 +180,7 @@ func (c *config) validate() error { validateFns := []func() error{ c.validateValuesDefined, c.validateValuesType, + c.validateEnumValues, } for _, fn := range validateFns { @@ -217,3 +217,24 @@ func (c *config) validateValuesType() error { } return nil } + +func (c *config) validateEnumValues() error { + for k, schema := range c.schema.Properties { + if schema.Enum == nil { + // skip if property is not enum + continue + } + if !slices.Contains(schema.Enum, c.values[k]) { + enums, err := toStringSlice(schema.Enum, schema.Type) + if err != nil { + return err + } + valueString, err := toString(c.values[k], schema.Type) + if err != nil { + return err + } + return fmt.Errorf("expect value of property %q to be one of %s. Found: %s", k, strings.Join(enums, ", "), valueString) + } + } + return nil +} diff --git a/libs/template/config_test.go b/libs/template/config_test.go index 335242467f8..c0c41aab8d5 100644 --- a/libs/template/config_test.go +++ b/libs/template/config_test.go @@ -192,3 +192,32 @@ func TestTemplateValidateSchema(t *testing.T) { err = validateSchema(toSchema("array")) assert.EqualError(t, err, "property type array is not supported by bundle templates") } + +func TestTemplateEnumValidation(t *testing.T) { + schema := jsonschema.Schema{ + Properties: map[string]*jsonschema.Schema{ + "abc": { + Type: "integer", + Enum: []any{1, 2, 3, 4}, + }, + }, + } + + c := &config{ + schema: &schema, + values: map[string]any{ + "abc": 5, + }, + } + assert.EqualError(t, c.validateEnumValues(), "expect value of property \"abc\" to be one of 1, 2, 3, 4. Found: 5") + assert.EqualError(t, c.validate(), "expect value of property \"abc\" to be one of 1, 2, 3, 4. Found: 5") + + c = &config{ + schema: &schema, + values: map[string]any{ + "abc": 4, + }, + } + assert.NoError(t, c.validateEnumValues()) + assert.NoError(t, c.validate()) +} diff --git a/libs/template/utils.go b/libs/template/utils.go index ade6a573058..a3ba6267582 100644 --- a/libs/template/utils.go +++ b/libs/template/utils.go @@ -73,6 +73,18 @@ func toString(v any, T jsonschema.Type) (string, error) { } } +func toStringSlice(arr []any, T jsonschema.Type) ([]string, error) { + res := []string{} + for _, v := range arr { + s, err := toString(v, T) + if err != nil { + return nil, err + } + res = append(res, s) + } + return res, nil +} + func fromString(s string, T jsonschema.Type) (any, error) { if T == jsonschema.StringType { return s, nil From b7e18a40fdc94c7b41c917832b6930b7d3c2f67e Mon Sep 17 00:00:00 2001 From: monalisa Date: Wed, 16 Aug 2023 16:08:13 +0200 Subject: [PATCH 06/13] remove todods --- libs/cmdio/logger.go | 4 ---- 1 file changed, 4 deletions(-) diff --git a/libs/cmdio/logger.go b/libs/cmdio/logger.go index f0a267f7104..7d760b998e0 100644 --- a/libs/cmdio/logger.go +++ b/libs/cmdio/logger.go @@ -105,10 +105,6 @@ func AskYesOrNo(ctx context.Context, question string) (bool, error) { return false, nil } -// TODO: allow enums without a default value specified. -// TODO: we require enum validation for both config-file and any --parameters introduced. -// TODO: add validation for default being in enum list. - func AskSelect(ctx context.Context, question string, choices []string) (string, error) { logger, ok := FromContext(ctx) if !ok { From b93821991b80844d03bfefd842caba1a4626cf46 Mon Sep 17 00:00:00 2001 From: monalisa Date: Wed, 16 Aug 2023 17:53:39 +0200 Subject: [PATCH 07/13] nit --- libs/template/config.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/libs/template/config.go b/libs/template/config.go index 317e5418fd1..078832ba6cf 100644 --- a/libs/template/config.go +++ b/libs/template/config.go @@ -139,7 +139,7 @@ func (c *config) promptForValues() error { // Get user input by running the prompt var userInput string if property.Enum != nil { - // convert list of enums to string + // convert list of enums to string slice enums, err := toStringSlice(property.Enum, property.Type) if err != nil { return err From 2f7801232405bc84d7a69f9aa102742cf445aaed Mon Sep 17 00:00:00 2001 From: monalisa Date: Wed, 6 Sep 2023 16:07:41 +0200 Subject: [PATCH 08/13] add validator for schema default and enum values --- libs/jsonschema/schema.go | 61 +++++++++ libs/jsonschema/schema_test.go | 99 +++++++++++++- .../schema-invalid-default.json | 9 ++ .../schema-load-int/schema-invalid-enum.json | 10 ++ .../schema-load-int/schema-valid.json | 10 ++ libs/jsonschema/type_validator.go | 56 ++++++++ libs/jsonschema/type_validator_test.go | 128 ++++++++++++++++++ libs/jsonschema/utils.go | 35 +++++ libs/jsonschema/utils_test.go | 49 +++++++ 9 files changed, 456 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-invalid-enum.json create mode 100644 libs/jsonschema/testdata/schema-load-int/schema-valid.json create mode 100644 libs/jsonschema/type_validator.go create mode 100644 libs/jsonschema/type_validator_test.go create mode 100644 libs/jsonschema/utils.go create mode 100644 libs/jsonschema/utils_test.go diff --git a/libs/jsonschema/schema.go b/libs/jsonschema/schema.go index 48a61aa66ec..a0c40c547a9 100644 --- a/libs/jsonschema/schema.go +++ b/libs/jsonschema/schema.go @@ -4,6 +4,7 @@ import ( "encoding/json" "fmt" "os" + "slices" ) // defines schema for a json object @@ -61,7 +62,10 @@ const ( IntegerType Type = "integer" ) +// TODO: add additional properties validator and require validator. + 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: @@ -76,6 +80,41 @@ 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) + } + } + + // Validate enum field values for properties are consistent with types. + for name, property := range schema.Properties { + if property.Enum == nil { + continue + } + for i, enum := range property.Enum { + err := validateType(enum, property.Type) + if err != nil { + return fmt.Errorf("type validation for enum at index %v failed for property %s: %w", i, name, err) + } + } + } + + // Validate default value is contained in the list of enums if both are defined. + for name, property := range schema.Properties { + if property.Default == nil || property.Enum == nil { + continue + } + // We expect the default value to be consistent with the list of enum + // values. + if !slices.Contains(property.Enum, property.Default) { + return fmt.Errorf("list of enum values for property %s does not contain default value %v: %v", name, property.Default, property.Enum) + } + } return nil } @@ -89,5 +128,27 @@ 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) + } + } + for i, enum := range property.Enum { + property.Enum[i], err = toInteger(enum) + if err != nil { + return nil, fmt.Errorf("failed to parse enum value %v at index %v for property %s: %w", enum, i, name, err) + } + } + } + return schema, schema.validate() } diff --git a/libs/jsonschema/schema_test.go b/libs/jsonschema/schema_test.go index 76112492f57..db559ea8855 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,100 @@ 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) + assert.Equal(t, []any{int64(1), int64(2), int64(3)}, schema.Properties["abc"].Enum) +} + +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 TestSchemaLoadIntegersWithInvalidEnums(t *testing.T) { + _, err := Load("./testdata/schema-load-int/schema-invalid-enum.json") + assert.EqualError(t, err, "failed to parse enum value 2.4 at index 1 for property abc: expected integer value, got: 2.4") +} + +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) +} + +func TestSchemaValidateEnumType(t *testing.T) { + invalidSchema := &Schema{ + Properties: map[string]*Schema{ + "foo": { + Type: "boolean", + Enum: []any{true, "false"}, + }, + }, + } + + err := invalidSchema.validate() + assert.EqualError(t, err, "type validation for enum at index 1 failed for property foo: expected type boolean, but value is \"false\"") + + validSchema := &Schema{ + Properties: map[string]*Schema{ + "foo": { + Type: "boolean", + Enum: []any{true, false}, + }, + }, + } + + err = validSchema.validate() + assert.NoError(t, err) +} + +func TestSchemaValidateErrorWhenDefaultValueIsNotInEnums(t *testing.T) { + invalidSchema := &Schema{ + Properties: map[string]*Schema{ + "foo": { + Type: "string", + Default: "abc", + Enum: []any{"def", "ghi"}, + }, + }, + } + + err := invalidSchema.validate() + assert.EqualError(t, err, "list of enum values for property foo does not contain default value abc: [def ghi]") + + validSchema := &Schema{ + Properties: map[string]*Schema{ + "foo": { + Type: "string", + Default: "abc", + Enum: []any{"def", "ghi", "abc"}, + }, + }, + } + + 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-invalid-enum.json b/libs/jsonschema/testdata/schema-load-int/schema-invalid-enum.json new file mode 100644 index 00000000000..5bd2b3f2b7e --- /dev/null +++ b/libs/jsonschema/testdata/schema-load-int/schema-invalid-enum.json @@ -0,0 +1,10 @@ +{ + "type": "object", + "properties": { + "abc": { + "type": "integer", + "default": 1, + "enum": [1,2.4,3] + } + } +} 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..a1167a6c98f --- /dev/null +++ b/libs/jsonschema/testdata/schema-load-int/schema-valid.json @@ -0,0 +1,10 @@ +{ + "type": "object", + "properties": { + "abc": { + "type": "integer", + "default": 1, + "enum": [1,2,3] + } + } +} diff --git a/libs/jsonschema/type_validator.go b/libs/jsonschema/type_validator.go new file mode 100644 index 00000000000..125d6b20b26 --- /dev/null +++ b/libs/jsonschema/type_validator.go @@ -0,0 +1,56 @@ +package jsonschema + +import ( + "fmt" + "reflect" + "slices" +) + +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/type_validator_test.go b/libs/jsonschema/type_validator_test.go new file mode 100644 index 00000000000..36d9e5758df --- /dev/null +++ b/libs/jsonschema/type_validator_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") +} diff --git a/libs/jsonschema/utils.go b/libs/jsonschema/utils.go new file mode 100644 index 00000000000..34b51f4d94d --- /dev/null +++ b/libs/jsonschema/utils.go @@ -0,0 +1,35 @@ +package jsonschema + +import "fmt" + +// 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") +} From c64db464e402a22eb619a5c26056871e043afc92 Mon Sep 17 00:00:00 2001 From: monalisa Date: Wed, 6 Sep 2023 16:09:47 +0200 Subject: [PATCH 09/13] add todos --- libs/jsonschema/utils.go | 3 +++ libs/jsonschema/{type_validator.go => validate_type.go} | 3 +++ .../{type_validator_test.go => validate_type_test.go} | 0 3 files changed, 6 insertions(+) rename libs/jsonschema/{type_validator.go => validate_type.go} (89%) rename libs/jsonschema/{type_validator_test.go => validate_type_test.go} (100%) diff --git a/libs/jsonschema/utils.go b/libs/jsonschema/utils.go index 34b51f4d94d..90803e6241d 100644 --- a/libs/jsonschema/utils.go +++ b/libs/jsonschema/utils.go @@ -2,6 +2,9 @@ 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/type_validator.go b/libs/jsonschema/validate_type.go similarity index 89% rename from libs/jsonschema/type_validator.go rename to libs/jsonschema/validate_type.go index 125d6b20b26..a4e43aa583b 100644 --- a/libs/jsonschema/type_validator.go +++ b/libs/jsonschema/validate_type.go @@ -6,6 +6,9 @@ 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 { diff --git a/libs/jsonschema/type_validator_test.go b/libs/jsonschema/validate_type_test.go similarity index 100% rename from libs/jsonschema/type_validator_test.go rename to libs/jsonschema/validate_type_test.go From d273eae7f76164a2bca30086b5d766f336b12559 Mon Sep 17 00:00:00 2001 From: monalisa Date: Thu, 7 Sep 2023 18:51:41 +0200 Subject: [PATCH 10/13] reconcile merge differences --- libs/jsonschema/instance.go | 61 +++++++------------ libs/jsonschema/instance_test.go | 26 ++++++++ libs/jsonschema/schema.go | 2 - .../instance-validate/test-schema-enum.json | 12 ++++ libs/template/config.go | 24 -------- libs/template/config_test.go | 4 +- 6 files changed, 61 insertions(+), 68 deletions(-) create mode 100644 libs/jsonschema/testdata/instance-validate/test-schema-enum.json diff --git a/libs/jsonschema/instance.go b/libs/jsonschema/instance.go index 1a03a3cca69..cbb9f4a78bc 100644 --- a/libs/jsonschema/instance.go +++ b/libs/jsonschema/instance.go @@ -4,6 +4,7 @@ import ( "encoding/json" "fmt" "os" + "slices" ) // Load a JSON document and validate it against the JSON schema. Instance here @@ -39,13 +40,20 @@ func (s *Schema) LoadInstance(path string) (map[string]any, error) { } func (s *Schema) ValidateInstance(instance map[string]any) error { - if err := s.validateAdditionalProperties(instance); err != nil { - return err + validationFuncs := []func(map[string]any) error{ + s.validateAdditionalProperties, + s.validateEnum, + s.validateRequired, + s.validateTypes, } - if err := s.validateRequired(instance); err != nil { - return err + + for _, fn := range validationFuncs { + err := fn(instance) + if err != nil { + return err + } } - return s.validateTypes(instance) + return nil } // If additional properties is set to false, this function validates instance only @@ -90,43 +98,18 @@ func (s *Schema) validateTypes(instance map[string]any) error { return nil } -/* -OLD Enum validation, previously in lib/template - validateFns := []func() error{ - c.validateValuesDefined, - c.validateValuesType, - c.validateEnumValues, - } - - for _, fn := range validateFns { - err := fn() - if err != nil { - return err +func (s *Schema) validateEnum(instance map[string]any) error { + for k, v := range instance { + fieldInfo, ok := s.Properties[k] + if !ok { + continue } - } - 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 { + if fieldInfo.Enum == nil { continue } - return fmt.Errorf("no value has been assigned to input parameter %s", k) + if !slices.Contains(fieldInfo.Enum, v) { + return fmt.Errorf("expected value of property %s to be one of %v. Found: %v", k, fieldInfo.Enum, v) + } } 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) - } -*/ diff --git a/libs/jsonschema/instance_test.go b/libs/jsonschema/instance_test.go index d5e0766dd23..ffd10ca435c 100644 --- a/libs/jsonschema/instance_test.go +++ b/libs/jsonschema/instance_test.go @@ -127,3 +127,29 @@ func TestLoadInstance(t *testing.T) { _, 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") } + +func TestValidateInstanceEnum(t *testing.T) { + schema, err := Load("./testdata/instance-validate/test-schema-enum.json") + require.NoError(t, err) + + validInstance := map[string]any{ + "foo": "b", + "bar": int64(6), + } + assert.NoError(t, schema.validateEnum(validInstance)) + assert.NoError(t, schema.ValidateInstance(validInstance)) + + invalidStringInstance := map[string]any{ + "foo": "d", + "bar": int64(2), + } + assert.EqualError(t, schema.validateEnum(invalidStringInstance), "expected value of property foo to be one of [a b c]. Found: d") + assert.EqualError(t, schema.ValidateInstance(invalidStringInstance), "expected value of property foo to be one of [a b c]. Found: d") + + invalidIntInstance := map[string]any{ + "foo": "a", + "bar": int64(1), + } + assert.EqualError(t, schema.validateEnum(invalidIntInstance), "expected value of property bar to be one of [2 4 6]. Found: 1") + assert.EqualError(t, schema.ValidateInstance(invalidIntInstance), "expected value of property bar to be one of [2 4 6]. Found: 1") +} diff --git a/libs/jsonschema/schema.go b/libs/jsonschema/schema.go index 820f858afa8..592d16cce46 100644 --- a/libs/jsonschema/schema.go +++ b/libs/jsonschema/schema.go @@ -62,8 +62,6 @@ const ( IntegerType Type = "integer" ) -// TODO: add additional properties validator and require validator. - func (schema *Schema) validate() error { // Validate property types are all valid JSON schema types. for _, v := range schema.Properties { diff --git a/libs/jsonschema/testdata/instance-validate/test-schema-enum.json b/libs/jsonschema/testdata/instance-validate/test-schema-enum.json new file mode 100644 index 00000000000..75ffd6eb808 --- /dev/null +++ b/libs/jsonschema/testdata/instance-validate/test-schema-enum.json @@ -0,0 +1,12 @@ +{ + "properties": { + "foo": { + "type": "string", + "enum": ["a", "b", "c"] + }, + "bar": { + "type": "integer", + "enum": [2,4,6] + } + } +} diff --git a/libs/template/config.go b/libs/template/config.go index a2fe2a34390..21618ac9a19 100644 --- a/libs/template/config.go +++ b/libs/template/config.go @@ -3,8 +3,6 @@ package template import ( "context" "fmt" - "slices" - "strings" "github.com/databricks/cli/libs/cmdio" "github.com/databricks/cli/libs/jsonschema" @@ -151,25 +149,3 @@ func (c *config) validate() error { } return nil } - -// TODO: OLD: Needs to be moved over to the json schema package. -func (c *config) validateEnumValues() error { - for k, schema := range c.schema.Properties { - if schema.Enum == nil { - // skip if property is not enum - continue - } - if !slices.Contains(schema.Enum, c.values[k]) { - enums, err := jsonschema.ToStringSlice(schema.Enum, schema.Type) - if err != nil { - return err - } - valueString, err := jsonschema.ToString(c.values[k], schema.Type) - if err != nil { - return err - } - return fmt.Errorf("expect value of property %q to be one of %s. Found: %s", k, strings.Join(enums, ", "), valueString) - } - } - return nil -} diff --git a/libs/template/config_test.go b/libs/template/config_test.go index 703583b5a27..1b1fc338365 100644 --- a/libs/template/config_test.go +++ b/libs/template/config_test.go @@ -159,8 +159,7 @@ func TestTemplateEnumValidation(t *testing.T) { "abc": 5, }, } - assert.EqualError(t, c.validateEnumValues(), "expect value of property \"abc\" to be one of 1, 2, 3, 4. Found: 5") - assert.EqualError(t, c.validate(), "expect value of property \"abc\" to be one of 1, 2, 3, 4. Found: 5") + assert.EqualError(t, c.validate(), "validation for template input parameters failed. expected value of property abc to be one of [1 2 3 4]. Found: 5") c = &config{ schema: &schema, @@ -168,6 +167,5 @@ func TestTemplateEnumValidation(t *testing.T) { "abc": 4, }, } - assert.NoError(t, c.validateEnumValues()) assert.NoError(t, c.validate()) } From 7944f2f0dd4230a0c8ddcbf0c21aa8f1661107cb Mon Sep 17 00:00:00 2001 From: monalisa Date: Thu, 7 Sep 2023 19:16:09 +0200 Subject: [PATCH 11/13] added test for to string slice --- libs/jsonschema/utils_test.go | 10 ++++++++++ 1 file changed, 10 insertions(+) diff --git a/libs/jsonschema/utils_test.go b/libs/jsonschema/utils_test.go index 9686cf39bbd..29529aaa910 100644 --- a/libs/jsonschema/utils_test.go +++ b/libs/jsonschema/utils_test.go @@ -118,3 +118,13 @@ func TestTemplateFromString(t *testing.T) { _, err = FromString("1.0", "foobar") assert.EqualError(t, err, "unknown json schema type: \"foobar\"") } + +func TestTemplateToStringSlice(t *testing.T) { + s, err := ToStringSlice([]any{"a", "b", "c"}, StringType) + assert.NoError(t, err) + assert.Equal(t, []string{"a", "b", "c"}, s) + + s, err = ToStringSlice([]any{1.1, 2.2, 3.3}, NumberType) + assert.NoError(t, err) + assert.Equal(t, []string{"1.1", "2.2", "3.3"}, s) +} From 71c00799e97242ed8703638c5abdc3897b3216da Mon Sep 17 00:00:00 2001 From: monalisa Date: Thu, 7 Sep 2023 19:21:34 +0200 Subject: [PATCH 12/13] - --- libs/jsonschema/schema.go | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/libs/jsonschema/schema.go b/libs/jsonschema/schema.go index 592d16cce46..108102a648c 100644 --- a/libs/jsonschema/schema.go +++ b/libs/jsonschema/schema.go @@ -42,8 +42,7 @@ type Schema struct { // Default value for the property / object Default any `json:"default,omitempty"` - // If specified this is the list of valid values for the object. Values - // here should be consistent with the type defined in the schema. + // List of valid values for a JSON instance for this schema. Enum []any `json:"enum,omitempty"` // Extension embeds our custom JSON schema extensions. From d7f19570bea333779f58708add0eea4ccae2b3d0 Mon Sep 17 00:00:00 2001 From: monalisa Date: Fri, 8 Sep 2023 14:00:45 +0200 Subject: [PATCH 13/13] inline --- libs/jsonschema/instance.go | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-) diff --git a/libs/jsonschema/instance.go b/libs/jsonschema/instance.go index cbb9f4a78bc..229a45b53fb 100644 --- a/libs/jsonschema/instance.go +++ b/libs/jsonschema/instance.go @@ -40,14 +40,12 @@ func (s *Schema) LoadInstance(path string) (map[string]any, error) { } func (s *Schema) ValidateInstance(instance map[string]any) error { - validationFuncs := []func(map[string]any) error{ + for _, fn := range []func(map[string]any) error{ s.validateAdditionalProperties, s.validateEnum, s.validateRequired, s.validateTypes, - } - - for _, fn := range validationFuncs { + } { err := fn(instance) if err != nil { return err