diff --git a/.gitignore b/.gitignore index 069611e7..d890c2ef 100644 --- a/.gitignore +++ b/.gitignore @@ -1,3 +1,4 @@ test-operation.yaml .idea/ -*.iml \ No newline at end of file +*.iml +.zed/ diff --git a/bundler/bundler_composer_test.go b/bundler/bundler_composer_test.go index dd437737..c6db5f59 100644 --- a/bundler/bundler_composer_test.go +++ b/bundler/bundler_composer_test.go @@ -2728,3 +2728,74 @@ func TestBundleComposed_DoubleBuildNoErrors(t *testing.T) { assert.Contains(t, schemas, "File") assert.Contains(t, schemas, "Error") } + +// TestBundleBytesComposed_JSONSchemaTarget covers https://github.com/pb33f/libopenapi/issues/562: +// external $ref targets written as JSON (rather than YAML) must still be hoisted into +// components.schemas rather than inlined at the ref site. JSON is a subset of YAML so every +// key arrives as yaml.DoubleQuotedStyle; component-type detection must not treat that as an +// opt-out from OpenAPI keyword recognition. +func TestBundleBytesComposed_JSONSchemaTarget(t *testing.T) { + rootSpec := `openapi: 3.0.3 +info: + title: t + version: '1' +paths: + /x: + get: + responses: + '200': + description: ok + content: + application/json: + schema: + $ref: "User.json" +` + + userJSON := `{ + "type": "object", + "properties": { + "id": { "type": "string" } + } +} +` + + tmp := t.TempDir() + write := func(name, src string) { + require.NoError(t, os.WriteFile(filepath.Join(tmp, name), []byte(src), 0644)) + } + write("openapi.yaml", rootSpec) + write("User.json", userJSON) + + specBytes, err := os.ReadFile(filepath.Join(tmp, "openapi.yaml")) + require.NoError(t, err) + + var logBuf strings.Builder + logger := slog.New(slog.NewTextHandler(&logBuf, &slog.HandlerOptions{Level: slog.LevelDebug})) + + bundled, err := BundleBytesComposed(specBytes, &datamodel.DocumentConfiguration{ + BasePath: tmp, + SpecFilePath: filepath.Join(tmp, "openapi.yaml"), + AllowFileReferences: true, + Logger: logger, + }, nil) + require.NoError(t, err) + + // the composer should never give up on a recognisable schema just because it arrived + // from a JSON file — that warning is the bug's calling card. + assert.NotContains(t, logBuf.String(), "unable to compose reference", + "JSON schema refs must be classifiable; log:\n%s", logBuf.String()) + + var doc map[string]any + require.NoError(t, yaml.Unmarshal(bundled, &doc)) + + components, ok := doc["components"].(map[string]any) + require.True(t, ok, "bundled doc must have components") + schemas, ok := components["schemas"].(map[string]any) + require.True(t, ok, "bundled doc must have components.schemas; got:\n%s", string(bundled)) + assert.Contains(t, schemas, "User", + "JSON-sourced schema should be hoisted under components.schemas; got:\n%s", string(bundled)) + + // and the original $ref site should now point at the hoisted component, not contain the raw schema. + assert.Contains(t, string(bundled), "#/components/schemas/User") + assert.NotContains(t, string(bundled), "User.json") +} diff --git a/bundler/detect_type.go b/bundler/detect_type.go index 70ab6db8..ee997b36 100644 --- a/bundler/detect_type.go +++ b/bundler/detect_type.go @@ -148,26 +148,38 @@ func hasPathItemProperties(node *yaml.Node) bool { return containsKey(keys, v3.ParametersLabel) } -// Helper function to get all keys from a mapping node -// Excludes quoted keys since they should be treated as literal strings, not OpenAPI keywords +// getNodeKeys returns the keys of a mapping node for component-type detection. +// +// When the source document mixes plain and quoted keys, a quoted key is interpreted as a +// deliberate escape — the user is signalling "treat this as a literal string, not an OpenAPI +// keyword" — and is excluded from the result. When every key in the mapping shares the same +// quote style, the style carries no such signal: most commonly this is a JSON-sourced +// mapping where quoting is syntactic (JSON requires `"key":`). In that case every key is +// returned. See https://github.com/pb33f/libopenapi/issues/562. func getNodeKeys(node *yaml.Node) []string { if node.Kind == yaml.DocumentNode && len(node.Content) > 0 { node = node.Content[0] } - if node.Kind != yaml.MappingNode { + if node.Kind != yaml.MappingNode || len(node.Content) == 0 { return nil } + mixedQuoteStyle := false + firstStyle := node.Content[0].Style + for i := 2; i < len(node.Content); i += 2 { + if node.Content[i].Style != firstStyle { + mixedQuoteStyle = true + break + } + } + var keys []string for i := 0; i < len(node.Content); i += 2 { - if i < len(node.Content) { - keyNode := node.Content[i] - // Skip quoted keys - they should not be treated as OpenAPI keywords - if keyNode.Style == yaml.SingleQuotedStyle || keyNode.Style == yaml.DoubleQuotedStyle { - continue - } - keys = append(keys, keyNode.Value) + keyNode := node.Content[i] + if mixedQuoteStyle && (keyNode.Style == yaml.SingleQuotedStyle || keyNode.Style == yaml.DoubleQuotedStyle) { + continue } + keys = append(keys, keyNode.Value) } return keys } diff --git a/bundler/detect_type_test.go b/bundler/detect_type_test.go index e4a4865e..5b8dc828 100644 --- a/bundler/detect_type_test.go +++ b/bundler/detect_type_test.go @@ -601,19 +601,35 @@ func parseYaml(t *testing.T, yamlStr string) *yaml.Node { } func TestDetectOpenAPIComponentType_QuotedKeys(t *testing.T) { - // Test that quoted "items" key is NOT treated as schema - quotedItemsYaml := ` + // mixed-style: a user deliberately escaping a single OpenAPI keyword in otherwise + // plain YAML. The escape signals "treat this key as a literal string", so the + // quoted key must not participate in component-type detection. The unquoted key + // (`type`) still classifies this mapping as a schema. + mixedQuotedItemsYaml := ` +type: object "items": - product_id: 1000012 fulfillment_node_id: US01 count: 10 ` - node := parseYaml(t, quotedItemsYaml) + node := parseYaml(t, mixedQuotedItemsYaml) componentType, detected := DetectOpenAPIComponentType(node) + assert.Equal(t, v3.SchemasLabel, componentType) + assert.True(t, detected) + + // fully escaped: every key is quoted with no other schema indicators. With nothing + // left to classify on, detection correctly fails. + fullyEscapedYaml := ` +description: not a schema +"items": + - product_id: 1000012 +` + node = parseYaml(t, fullyEscapedYaml) + componentType, detected = DetectOpenAPIComponentType(node) assert.Equal(t, "", componentType) assert.False(t, detected) - // Test that unquoted items key IS treated as schema + // unquoted `items` key on its own still detects as a schema. unquotedItemsYaml := ` items: type: string @@ -623,17 +639,7 @@ items: assert.Equal(t, v3.SchemasLabel, componentType) assert.True(t, detected) - // Test that quoted "type" key is NOT treated as schema - quotedTypeYaml := ` -"type": "user" -"name": "John" -` - node = parseYaml(t, quotedTypeYaml) - componentType, detected = DetectOpenAPIComponentType(node) - assert.Equal(t, "", componentType) - assert.False(t, detected) - - // Test that unquoted type key IS treated as schema + // plain-style schema is unaffected. unquotedTypeYaml := ` type: object properties: @@ -645,7 +651,7 @@ properties: assert.Equal(t, v3.SchemasLabel, componentType) assert.True(t, detected) - // Test mixed quoted and unquoted keys - should detect schema from unquoted key + // mixed-style: the quoted `items` escape is ignored, classification driven by `type`. mixedYaml := ` type: object "items": @@ -656,3 +662,89 @@ type: object assert.Equal(t, v3.SchemasLabel, componentType) assert.True(t, detected) } + +// TestDetectOpenAPIComponentType_JSONSourced covers https://github.com/pb33f/libopenapi/issues/562: +// SON-parsed mappings arrive with every key marked yaml.DoubleQuotedStyle. +// Uniform quoting on a JSON-sourced mapping is syntactic — not a +// user escape — and must not disable component-type detection. +func TestDetectOpenAPIComponentType_JSONSourced(t *testing.T) { + cases := []struct { + name string + json string + want string + }{ + { + name: "schema with type and properties", + json: `{"type": "object", "properties": {"id": {"type": "string"}}}`, + want: v3.SchemasLabel, + }, + { + name: "schema with items", + json: `{"type": "array", "items": {"type": "string"}}`, + want: v3.SchemasLabel, + }, + { + name: "schema with allOf", + json: `{"allOf": [{"type": "object"}]}`, + want: v3.SchemasLabel, + }, + { + name: "parameter", + json: `{"name": "id", "in": "query"}`, + want: v3.ParametersLabel, + }, + { + name: "response with content", + json: `{"description": "ok", "content": {"application/json": {"schema": {"type": "object"}}}}`, + want: v3.ResponsesLabel, + }, + } + + for _, tc := range cases { + t.Run(tc.name, func(t *testing.T) { + node := parseYaml(t, tc.json) + componentType, detected := DetectOpenAPIComponentType(node) + assert.True(t, detected, "expected detection to succeed for JSON input") + assert.Equal(t, tc.want, componentType) + }) + } +} + +// TestGetNodeKeys_UniformQuoting guards the detector's key-filter against JSON. When every +// key in a mapping shares the same quote style, the style carries no intent to escape +// keywords, the filter only kicks in when styles are mixed within one mapping. +func TestGetNodeKeys_UniformQuoting(t *testing.T) { + cases := []struct { + name string + yaml string + want []string + }{ + { + name: "JSON (all double-quoted)", + yaml: `{"type": "object", "properties": {"x": {"type": "string"}}}`, + want: []string{"type", "properties"}, + }, + { + name: "YAML all single-quoted", + yaml: "'type': object\n'properties':\n name:\n type: string\n", + want: []string{"type", "properties"}, + }, + { + name: "mixed quoting drops the quoted keys", + yaml: "type: object\n\"properties\": literal\n", + want: []string{"type"}, + }, + { + name: "plain YAML untouched", + yaml: "type: object\nproperties:\n x:\n type: string\n", + want: []string{"type", "properties"}, + }, + } + + for _, tc := range cases { + t.Run(tc.name, func(t *testing.T) { + node := parseYaml(t, tc.yaml) + assert.ElementsMatch(t, tc.want, getNodeKeys(node)) + }) + } +}