From 887ee9b403ac5ba4169bb09779b009451150c925 Mon Sep 17 00:00:00 2001 From: tdakkota Date: Tue, 25 Apr 2023 06:43:11 +0300 Subject: [PATCH 1/2] refactor(jsonschema): move default value type check to generator --- gen/schema_gen.go | 64 ++++++++++++++++++++++++++++++++++-- jsonschema/parser.go | 6 +--- jsonschema/parser_enum.go | 69 ++++++++++++++++----------------------- 3 files changed, 91 insertions(+), 48 deletions(-) diff --git a/gen/schema_gen.go b/gen/schema_gen.go index 2f1894a92..3e7a5bd07 100644 --- a/gen/schema_gen.go +++ b/gen/schema_gen.go @@ -147,16 +147,25 @@ func (g *schemaGen) generate2(name string, schema *jsonschema.Schema) (ret *ir.T if schema.DefaultSet { var implErr error - switch schema.Type { - case jsonschema.Object: + switch { + case schema.Type == jsonschema.Object: implErr = &ErrNotImplemented{Name: "object defaults"} - case jsonschema.Array: + case schema.Type == jsonschema.Array: implErr = &ErrNotImplemented{Name: "array defaults"} + case schema.Type == jsonschema.Empty || + len(schema.AnyOf)+len(schema.OneOf) > 0: + implErr = &ErrNotImplemented{Name: "complex defaults"} } // Do not fail schema generation if we cannot handle defaults. if err := g.fail(implErr); err != nil { return nil, err } + + if implErr == nil { + if err := g.checkDefaultType(schema, schema.Default); err != nil { + return nil, errors.Wrap(err, "check default type") + } + } schema.DefaultSet = implErr == nil } @@ -494,3 +503,52 @@ func (g *schemaGen) regtype(name string, t *ir.Type) *ir.Type { return t } + +func (g *schemaGen) checkDefaultType(s *jsonschema.Schema, val any) error { + if s == nil { + // Schema has no validators. + return nil + } + if val == nil && s.Nullable { + return nil + } + + var ok bool + switch s.Type { + case jsonschema.Object: + _, ok = val.(map[string]any) + case jsonschema.Array: + _, ok = val.([]any) + case jsonschema.Integer: + _, ok = val.(int64) + case jsonschema.Number: + _, ok = val.(int64) + if !ok { + _, ok = val.(float64) + } + case jsonschema.String: + _, ok = val.(string) + case jsonschema.Boolean: + _, ok = val.(bool) + case jsonschema.Null: + ok = val == nil + } + + if !ok { + err := errors.Errorf("expected schema type is %q, default value is %T", s.Type, val) + p := s.Pointer.Field("default") + + pos, ok := p.Position() + if !ok { + return err + } + + return &location.Error{ + File: p.File(), + Pos: pos, + Err: err, + } + } + + return nil +} diff --git a/jsonschema/parser.go b/jsonschema/parser.go index 271f26cda..28489de8a 100644 --- a/jsonschema/parser.go +++ b/jsonschema/parser.go @@ -116,15 +116,11 @@ func (p *Parser) parse1(schema *RawSchema, ctx *jsonpointer.ResolveCtx, hook fun } if d := schema.Default; len(d) > 0 { if err := func() error { - v, err := parseJSONValue(s, json.RawMessage(d)) + v, err := parseJSONValue(nil, json.RawMessage(d)) if err != nil { return err } - if v == nil && !s.Nullable { - return errors.New("unexpected default \"null\" value") - } - s.Default = v s.DefaultSet = true return nil diff --git a/jsonschema/parser_enum.go b/jsonschema/parser_enum.go index 99cdebfe6..bfa9a8486 100644 --- a/jsonschema/parser_enum.go +++ b/jsonschema/parser_enum.go @@ -41,56 +41,32 @@ func parseEnumValues(s *Schema, rawValues []json.RawMessage) ([]any, error) { func parseJSONValue(root *Schema, v json.RawMessage) (any, error) { var parse func(s *Schema, d *jx.Decoder) (any, error) parse = func(s *Schema, d *jx.Decoder) (any, error) { - next := d.Next() - if next == jx.Null { - // We do not check nullability here because enum with null value is completely valid - // even if it is not nullable. - return nil, nil - } - switch typ := s.Type; typ { - case String: - if next != jx.String { - return nil, errors.Errorf("expected type %q, got %q", typ, next) - } + switch next := d.Next(); next { + case jx.Null: + return nil, d.Null() + case jx.String: return d.Str() - case Integer: - if next != jx.Number { - return nil, errors.Errorf("expected type %q, got %q", typ, next) - } + case jx.Number: n, err := d.Num() if err != nil { return nil, err } - if !n.IsInt() { - return nil, errors.New("expected integer, got float") - } - return n.Int64() - case Number: - if next != jx.Number { - return nil, errors.Errorf("expected type %q, got %q", typ, next) - } - n, err := d.Num() - if err != nil { - return nil, err + if n.IsInt() { + return n.Int64() } return n.Float64() - case Boolean: - if next != jx.Bool { - return nil, errors.Errorf("expected type %q, got %q", typ, next) - } + case jx.Bool: return d.Bool() - case Array: - if next != jx.Array { - return nil, errors.Errorf("expected type %q, got %q", typ, next) - } - if s.Item == nil { - return nil, errors.New("can't validate untyped array item") - } + case jx.Array: var arr []any if err := d.Arr(func(d *jx.Decoder) error { - v, err := parse(s.Item, d) + var item *Schema + if s != nil { + item = s.Item + } + v, err := parse(item, d) if err != nil { - return errors.Wrap(err, "validate item") + return errors.Wrap(err, "array item") } arr = append(arr, v) return nil @@ -98,8 +74,21 @@ func parseJSONValue(root *Schema, v json.RawMessage) (any, error) { return nil, err } return arr, nil + case jx.Object: + obj := map[string]any{} + if err := d.ObjBytes(func(d *jx.Decoder, key []byte) error { + v, err := parse(nil, d) + if err != nil { + return errors.Wrapf(err, "property %q", key) + } + obj[string(key)] = v + return nil + }); err != nil { + return nil, err + } + return obj, nil default: - return nil, errors.Errorf("unexpected type: %q", typ) + return nil, errors.Errorf("unexpected type: %q", next) } } From 78a8ebf72766898555115c950f189eccdc9f4f14 Mon Sep 17 00:00:00 2001 From: tdakkota Date: Tue, 25 Apr 2023 09:01:27 +0300 Subject: [PATCH 2/2] test(gen): add negative tests for default value type check --- .../wrong_default_type/array_value.json | 31 +++++++++++++++++++ .../wrong_default_type/bool_value.json | 31 +++++++++++++++++++ .../wrong_default_type/float_value.json | 31 +++++++++++++++++++ .../wrong_default_type/null_value.json | 3 -- .../wrong_default_type/object_value.json | 31 +++++++++++++++++++ .../wrong_default_type/string_value.json | 31 +++++++++++++++++++ 6 files changed, 155 insertions(+), 3 deletions(-) create mode 100644 _testdata/negative/wrong_default_type/array_value.json create mode 100644 _testdata/negative/wrong_default_type/bool_value.json create mode 100644 _testdata/negative/wrong_default_type/float_value.json rename openapi/parser/_testdata/negative/null/default_value.json => _testdata/negative/wrong_default_type/null_value.json (87%) create mode 100644 _testdata/negative/wrong_default_type/object_value.json create mode 100644 _testdata/negative/wrong_default_type/string_value.json diff --git a/_testdata/negative/wrong_default_type/array_value.json b/_testdata/negative/wrong_default_type/array_value.json new file mode 100644 index 000000000..ba6c6ed5a --- /dev/null +++ b/_testdata/negative/wrong_default_type/array_value.json @@ -0,0 +1,31 @@ +{ + "openapi": "3.0.3", + "info": { + "title": "title", + "version": "v0.1.0" + }, + "paths": { + "/foo": { + "get": { + "responses": { + "200": { + "description": "User info", + "content": { + "application/json": { + "schema": { + "type": "object", + "properties": { + "not_nullable": { + "type": "integer", + "default": [] + } + } + } + } + } + } + } + } + } + } +} diff --git a/_testdata/negative/wrong_default_type/bool_value.json b/_testdata/negative/wrong_default_type/bool_value.json new file mode 100644 index 000000000..a1259a95c --- /dev/null +++ b/_testdata/negative/wrong_default_type/bool_value.json @@ -0,0 +1,31 @@ +{ + "openapi": "3.0.3", + "info": { + "title": "title", + "version": "v0.1.0" + }, + "paths": { + "/foo": { + "get": { + "responses": { + "200": { + "description": "User info", + "content": { + "application/json": { + "schema": { + "type": "object", + "properties": { + "not_nullable": { + "type": "integer", + "default": false + } + } + } + } + } + } + } + } + } + } +} diff --git a/_testdata/negative/wrong_default_type/float_value.json b/_testdata/negative/wrong_default_type/float_value.json new file mode 100644 index 000000000..198e9a454 --- /dev/null +++ b/_testdata/negative/wrong_default_type/float_value.json @@ -0,0 +1,31 @@ +{ + "openapi": "3.0.3", + "info": { + "title": "title", + "version": "v0.1.0" + }, + "paths": { + "/foo": { + "get": { + "responses": { + "200": { + "description": "User info", + "content": { + "application/json": { + "schema": { + "type": "object", + "properties": { + "not_nullable": { + "type": "integer", + "default": 3.14 + } + } + } + } + } + } + } + } + } + } +} diff --git a/openapi/parser/_testdata/negative/null/default_value.json b/_testdata/negative/wrong_default_type/null_value.json similarity index 87% rename from openapi/parser/_testdata/negative/null/default_value.json rename to _testdata/negative/wrong_default_type/null_value.json index 1ef75ffa9..207d664f0 100644 --- a/openapi/parser/_testdata/negative/null/default_value.json +++ b/_testdata/negative/wrong_default_type/null_value.json @@ -14,9 +14,6 @@ "application/json": { "schema": { "type": "object", - "required": [ - "not_nullable" - ], "properties": { "not_nullable": { "type": "integer", diff --git a/_testdata/negative/wrong_default_type/object_value.json b/_testdata/negative/wrong_default_type/object_value.json new file mode 100644 index 000000000..4535e44d6 --- /dev/null +++ b/_testdata/negative/wrong_default_type/object_value.json @@ -0,0 +1,31 @@ +{ + "openapi": "3.0.3", + "info": { + "title": "title", + "version": "v0.1.0" + }, + "paths": { + "/foo": { + "get": { + "responses": { + "200": { + "description": "User info", + "content": { + "application/json": { + "schema": { + "type": "object", + "properties": { + "not_nullable": { + "type": "integer", + "default": {} + } + } + } + } + } + } + } + } + } + } +} diff --git a/_testdata/negative/wrong_default_type/string_value.json b/_testdata/negative/wrong_default_type/string_value.json new file mode 100644 index 000000000..479d2f326 --- /dev/null +++ b/_testdata/negative/wrong_default_type/string_value.json @@ -0,0 +1,31 @@ +{ + "openapi": "3.0.3", + "info": { + "title": "title", + "version": "v0.1.0" + }, + "paths": { + "/foo": { + "get": { + "responses": { + "200": { + "description": "User info", + "content": { + "application/json": { + "schema": { + "type": "object", + "properties": { + "not_nullable": { + "type": "integer", + "default": "amongus" + } + } + } + } + } + } + } + } + } + } +}