From cb8d6a9f60ca4b360e0d24291fbfbc67fd924361 Mon Sep 17 00:00:00 2001 From: Shreyas Goenka Date: Tue, 27 Aug 2024 18:34:33 +0200 Subject: [PATCH] self pass 1 --- bundle/internal/schema/parser.go | 22 +++++++--------------- cmd/bundle/schema.go | 4 ++-- libs/dyn/dynvar/ref.go | 4 +--- libs/dyn/path.go | 4 ++-- libs/dyn/visit.go | 8 ++++---- libs/dyn/visit_set.go | 4 ++-- libs/jsonschema/from_type.go | 20 +++++++------------- libs/jsonschema/schema.go | 4 +--- 8 files changed, 26 insertions(+), 44 deletions(-) diff --git a/bundle/internal/schema/parser.go b/bundle/internal/schema/parser.go index dc85da389..ef3d6e719 100644 --- a/bundle/internal/schema/parser.go +++ b/bundle/internal/schema/parser.go @@ -40,14 +40,16 @@ func newParser(path string) (*openapiParser, error) { return p, nil } -// This function finds any JSON schemas that were defined in the OpenAPI spec -// that correspond to the given Go SDK type. It looks both at the type itself -// and any embedded types within it. +// This function checks if the input type: +// 1. Is a Databricks Go SDK type. +// 2. Has a Databricks Go SDK type embedded in it. +// +// If the above conditions are met, the function returns the JSON schema +// corresponding to the Databricks Go SDK type from the OpenAPI spec. func (p *openapiParser) findRef(typ reflect.Type) (jsonschema.Schema, bool) { typs := []reflect.Type{typ} - // If the type is a struct, the corresponding Go SDK struct might be embedded - // in it. We need to check for those as well. + // Check for embedded Databricks Go SDK types. if typ.Kind() == reflect.Struct { for i := 0; i < typ.NumField(); i++ { if !typ.Field(i).Anonymous { @@ -94,11 +96,6 @@ func (p *openapiParser) addDescriptions(typ reflect.Type, s jsonschema.Schema) j } s.Description = ref.Description - - // Iterate over properties to load descriptions. This is not needed for any - // OpenAPI spec generated from protobufs, which are guaranteed to be one level - // deep. - // Needed for any hand-written OpenAPI specs. for k, v := range s.Properties { if refProp, ok := ref.Properties[k]; ok { v.Description = refProp.Description @@ -116,11 +113,6 @@ func (p *openapiParser) addEnums(typ reflect.Type, s jsonschema.Schema) jsonsche } s.Enum = append(s.Enum, ref.Enum...) - - // Iterate over properties to load enums. This is not needed for any - // OpenAPI spec generated from protobufs, which are guaranteed to be one level - // deep. - // Needed for any hand-written OpenAPI specs. for k, v := range s.Properties { if refProp, ok := ref.Properties[k]; ok { v.Enum = append(v.Enum, refProp.Enum...) diff --git a/cmd/bundle/schema.go b/cmd/bundle/schema.go index 425be5e12..d33714c9c 100644 --- a/cmd/bundle/schema.go +++ b/cmd/bundle/schema.go @@ -8,7 +8,7 @@ import ( ) //go:embed _generated/jsonschema.json -var b []byte +var bundleSchemaBytes []byte func newSchemaCommand() *cobra.Command { cmd := &cobra.Command{ @@ -18,7 +18,7 @@ func newSchemaCommand() *cobra.Command { } cmd.RunE = func(cmd *cobra.Command, args []string) error { - _, err := cmd.OutOrStdout().Write(b) + _, err := cmd.OutOrStdout().Write(bundleSchemaBytes) return err } diff --git a/libs/dyn/dynvar/ref.go b/libs/dyn/dynvar/ref.go index f686d6779..338ac8ce6 100644 --- a/libs/dyn/dynvar/ref.go +++ b/libs/dyn/dynvar/ref.go @@ -6,9 +6,7 @@ import ( "github.com/databricks/cli/libs/dyn" ) -const ReferenceRegex = `\$\{([a-zA-Z]+([-_]?[a-zA-Z0-9]+)*(\.[a-zA-Z]+([-_]?[a-zA-Z0-9]+)*(\[[0-9]+\])*)*(\[[0-9]+\])*)\}` - -var re = regexp.MustCompile(ReferenceRegex) +var re = regexp.MustCompile(`\$\{([a-zA-Z]+([-_]?[a-zA-Z0-9]+)*(\.[a-zA-Z]+([-_]?[a-zA-Z0-9]+)*(\[[0-9]+\])*)*(\[[0-9]+\])*)\}`) // ref represents a variable reference. // It is a string [dyn.Value] contained in a larger [dyn.Value]. diff --git a/libs/dyn/path.go b/libs/dyn/path.go index 1d0d4afab..76377e2dc 100644 --- a/libs/dyn/path.go +++ b/libs/dyn/path.go @@ -18,11 +18,11 @@ func (c pathComponent) Index() int { return c.index } -func (c pathComponent) IsKey() bool { +func (c pathComponent) isKey() bool { return c.key != "" } -func (c pathComponent) IsIndex() bool { +func (c pathComponent) isIndex() bool { return c.key == "" } diff --git a/libs/dyn/visit.go b/libs/dyn/visit.go index ee30227d7..4d3cf5014 100644 --- a/libs/dyn/visit.go +++ b/libs/dyn/visit.go @@ -14,9 +14,9 @@ type cannotTraverseNilError struct { func (e cannotTraverseNilError) Error() string { component := e.p[len(e.p)-1] switch { - case component.IsKey(): + case component.isKey(): return fmt.Sprintf("expected a map to index %q, found nil", e.p) - case component.IsIndex(): + case component.isIndex(): return fmt.Sprintf("expected a sequence to index %q, found nil", e.p) default: panic("invalid component") @@ -90,7 +90,7 @@ func (component pathComponent) visit(v Value, prefix Path, suffix Pattern, opts path := append(prefix, component) switch { - case component.IsKey(): + case component.isKey(): // Expect a map to be set if this is a key. switch v.Kind() { case KindMap: @@ -129,7 +129,7 @@ func (component pathComponent) visit(v Value, prefix Path, suffix Pattern, opts l: v.l, }, nil - case component.IsIndex(): + case component.isIndex(): // Expect a sequence to be set if this is an index. switch v.Kind() { case KindSequence: diff --git a/libs/dyn/visit_set.go b/libs/dyn/visit_set.go index cf3b3b312..b086fb8a9 100644 --- a/libs/dyn/visit_set.go +++ b/libs/dyn/visit_set.go @@ -32,7 +32,7 @@ func SetByPath(v Value, p Path, nv Value) (Value, error) { path := append(prefix, component) switch { - case component.IsKey(): + case component.isKey(): // Expect a map to be set if this is a key. m, ok := v.AsMap() if !ok { @@ -48,7 +48,7 @@ func SetByPath(v Value, p Path, nv Value) (Value, error) { l: v.l, }, nil - case component.IsIndex(): + case component.isIndex(): // Expect a sequence to be set if this is an index. s, ok := v.AsSequence() if !ok { diff --git a/libs/jsonschema/from_type.go b/libs/jsonschema/from_type.go index d2bbb4550..f916caa0c 100644 --- a/libs/jsonschema/from_type.go +++ b/libs/jsonschema/from_type.go @@ -19,7 +19,7 @@ const readonlyTag = "readonly" const internalTag = "internal" // Annotation for bundle fields that have been deprecated. -// Fields tagged as "deprecated" are removed/omitted from the generated schema. +// Fields tagged as "deprecated" are omitted from the generated schema. const deprecatedTag = "deprecated" type constructor struct { @@ -36,8 +36,8 @@ type constructor struct { } // The $defs block in a JSON schema cannot contain "/", otherwise it will not be -// correctly parsed by a JSON schema validator. So we replace "/" with an additional -// level of nesting in the output map. +// correctly parsed by a JSON schema validator (like the Red Hat YAML extension for VSCode). +// So we replace "/" with an additional level of nesting in the output map. // // For example: // {"a/b/c": "value"} is converted to {"a": {"b": {"c": "value"}}} @@ -57,11 +57,14 @@ func (c *constructor) Definitions() any { parts := strings.Split(k, "/") cur := res for i, p := range parts { + // Set the value for the last part. if i == len(parts)-1 { cur[p] = v break } + // For all but the last part, create a new map value to add a level + // of nesting. if _, ok := cur[p]; !ok { cur[p] = make(map[string]any) } @@ -77,7 +80,7 @@ func (c *constructor) Definitions() any { // for every Go type and referring them using $ref in the corresponding node in // the JSON schema. // -// fns is a list of transformation functions that will be applied to all $defs +// fns is a list of transformation functions that will be applied in order to all $defs // in the schema. func FromType(typ reflect.Type, fns []func(typ reflect.Type, s Schema) Schema) (Schema, error) { c := constructor{ @@ -156,11 +159,6 @@ func (c *constructor) walk(typ reflect.Type) error { } c.seen[typPath] = typ - // Return early directly if it's already been processed. - if _, ok := c.definitions[typPath]; ok { - return nil - } - var s Schema var err error @@ -315,10 +313,6 @@ func (c *constructor) fromTypeMap(typ reflect.Type) (Schema, error) { return Schema{}, fmt.Errorf("expected map, got %s", typ.Kind()) } - if typ.Key().Kind() != reflect.String { - return Schema{}, fmt.Errorf("found map with non-string key: %v", typ.Key()) - } - res := Schema{ Type: ObjectType, } diff --git a/libs/jsonschema/schema.go b/libs/jsonschema/schema.go index 5b58d81d5..10e646165 100644 --- a/libs/jsonschema/schema.go +++ b/libs/jsonschema/schema.go @@ -41,7 +41,7 @@ type Schema struct { // A boolean type with value false. Setting false here validates that all // properties in the config have been defined in the json schema as properties // - // Its type during runtime will either be Schema or bool + // Its type during runtime will either be *Schema or bool AdditionalProperties any `json:"additionalProperties,omitempty"` // Required properties for the object. Any fields missing the "omitempty" @@ -88,8 +88,6 @@ func (s *Schema) ParseString(v string) (any, error) { type Type string const ( - // Default zero value of a schema. This does not correspond to a type in the - // JSON schema spec and is an internal type defined for convenience. InvalidType Type = "invalid" BooleanType Type = "boolean" StringType Type = "string"