Skip to content

Commit

Permalink
fix: Use field name in json type schema if json tag is missing (#2011)
Browse files Browse the repository at this point in the history

I'm working on validating some data (internal issue cloudquery/cloudquery-issues#2971) using the json type schema and noticed a bug (noticeable on AWS where structs don't have json tags).
We should not use the name transformer to get the name as it defaults to `ToSnake` when a json tag is missing.
Instead we should mimic the JSON marshaling default behavior to use the field name as is.

A better approach can be to create an instance of the type, marshal it to JSON, then use that for the names. Open to ideas how to do that using reflection, with a caveat that we might need to initialize it using non zero values otherwise those can be omitted

---
  • Loading branch information
erezrokah authored Dec 19, 2024
1 parent da2da87 commit 7ca8009
Show file tree
Hide file tree
Showing 4 changed files with 47 additions and 6 deletions.
12 changes: 12 additions & 0 deletions transformers/name.go
Original file line number Diff line number Diff line change
Expand Up @@ -26,4 +26,16 @@ func DefaultNameTransformer(field reflect.StructField) (string, error) {
return defaultCaser.ToSnake(name), nil
}

func DefaultJSONColumnSchemaNameTransformer(field reflect.StructField) (string, error) {
name := field.Name
if jsonTag := strings.Split(field.Tag.Get("json"), ",")[0]; len(jsonTag) > 0 {
// return empty string if the field is not related api response
if jsonTag == "-" {
return "", nil
}
return jsonTag, nil
}
return name, nil
}

var _ NameTransformer = DefaultNameTransformer
8 changes: 8 additions & 0 deletions transformers/options.go
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,14 @@ func WithNameTransformer(transformer NameTransformer) StructTransformerOption {
}
}

// WithJSONSchemaNameTransformer overrides how column name will be determined.
// DefaultJSONColumnSchemaNameTransformer is used as the default.
func WithJSONSchemaNameTransformer(transformer NameTransformer) StructTransformerOption {
return func(t *structTransformer) {
t.jsonSchemaNameTransformer = transformer
}
}

// WithTypeTransformer overrides how column type will be determined.
// DefaultTypeTransformer is used as the default.
func WithTypeTransformer(transformer TypeTransformer) StructTransformerOption {
Expand Down
14 changes: 8 additions & 6 deletions transformers/struct.go
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,7 @@ type structTransformer struct {
pkFieldsFound []string
pkComponentFields []string
pkComponentFieldsFound []string
jsonSchemaNameTransformer NameTransformer

maxJSONTypeSchemaDepth int
}
Expand Down Expand Up @@ -194,11 +195,12 @@ func (t *structTransformer) addColumnFromField(field reflect.StructField, parent

func TransformWithStruct(st any, opts ...StructTransformerOption) schema.Transform {
t := &structTransformer{
nameTransformer: DefaultNameTransformer,
typeTransformer: DefaultTypeTransformer,
resolverTransformer: DefaultResolverTransformer,
ignoreInTestsTransformer: DefaultIgnoreInTestsTransformer,
maxJSONTypeSchemaDepth: DefaultMaxJSONTypeSchemaDepth,
nameTransformer: DefaultNameTransformer,
typeTransformer: DefaultTypeTransformer,
resolverTransformer: DefaultResolverTransformer,
ignoreInTestsTransformer: DefaultIgnoreInTestsTransformer,
jsonSchemaNameTransformer: DefaultJSONColumnSchemaNameTransformer,
maxJSONTypeSchemaDepth: DefaultMaxJSONTypeSchemaDepth,
}
for _, opt := range opts {
opt(t)
Expand Down Expand Up @@ -284,7 +286,7 @@ func (t *structTransformer) fieldToJSONSchema(field reflect.StructField, depth i
if !structField.IsExported() || isTypeIgnored(structField.Type) {
continue
}
name, err := t.nameTransformer(structField)
name, err := t.jsonSchemaNameTransformer(structField)
if err != nil {
continue
}
Expand Down
19 changes: 19 additions & 0 deletions transformers/struct_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@ import (
"github.com/cloudquery/plugin-sdk/v4/schema"
"github.com/cloudquery/plugin-sdk/v4/types"
"github.com/google/go-cmp/cmp"
"github.com/stretchr/testify/require"
)

type (
Expand Down Expand Up @@ -665,6 +666,23 @@ func TestJSONTypeSchema(t *testing.T) {
"item": `{"exported":"utf8"}`,
},
},
{
name: "no json tags",
testStruct: struct {
Tags map[string]string
Item struct {
Name string
Tags map[string]string
FlatItems []string
ComplexItems []struct {
Name string
}
}
}{},
want: map[string]string{
"item": `{"ComplexItems":[{"Name":"utf8"}],"FlatItems":["utf8"],"Name":"utf8","Tags":{"utf8":"utf8"}}`,
},
},
}

for _, tt := range tests {
Expand All @@ -684,6 +702,7 @@ func TestJSONTypeSchema(t *testing.T) {
}
for col, schema := range tt.want {
column := table.Column(col)
require.NotNil(t, column, "column %q not found", col)
if diff := cmp.Diff(column.TypeSchema, schema); diff != "" {
t.Fatalf("table does not match expected. diff (-got, +want): %v", diff)
}
Expand Down

0 comments on commit 7ca8009

Please sign in to comment.