Merge pull request #877 from tdakkota/refactor/move-default-typecheck

refactor(jsonschema): move default value type check to generator
ogen-go · Apr 25, 2023 · c1ded8d · c1ded8d
2 parents 982b8fc + 78a8ebf
commit c1ded8d
Show file tree

Hide file tree

Showing 9 changed files with 246 additions and 51 deletions.
diff --git a/_testdata/negative/wrong_default_type/array_value.json 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
@@ -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
@@ -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/...testdata/negative/null/default_value.json → ...gative/wrong_default_type/null_value.json b/...testdata/negative/null/default_value.json → ...gative/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
@@ -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
@@ -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"
+                    }
+                  }
+                }
+              }
+            }
+          }
+        }
+      }
+    }
+  }
+}
diff --git 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
@@ -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
@@ -41,65 +41,54 @@ 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
 			}); err != nil {
 				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)
 		}
 	}