Skip to content

Commit ff053d3

Browse files
FollowTheProcessdaveshanley
authored andcommitted
Fix JSON component composed bundling
1 parent c3785df commit ff053d3

4 files changed

Lines changed: 203 additions & 27 deletions

File tree

.gitignore

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,3 +1,4 @@
11
test-operation.yaml
22
.idea/
3-
*.iml
3+
*.iml
4+
.zed/

bundler/bundler_composer_test.go

Lines changed: 71 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2728,3 +2728,74 @@ func TestBundleComposed_DoubleBuildNoErrors(t *testing.T) {
27282728
assert.Contains(t, schemas, "File")
27292729
assert.Contains(t, schemas, "Error")
27302730
}
2731+
2732+
// TestBundleBytesComposed_JSONSchemaTarget covers https://github.com/pb33f/libopenapi/issues/562:
2733+
// external $ref targets written as JSON (rather than YAML) must still be hoisted into
2734+
// components.schemas rather than inlined at the ref site. JSON is a subset of YAML so every
2735+
// key arrives as yaml.DoubleQuotedStyle; component-type detection must not treat that as an
2736+
// opt-out from OpenAPI keyword recognition.
2737+
func TestBundleBytesComposed_JSONSchemaTarget(t *testing.T) {
2738+
rootSpec := `openapi: 3.0.3
2739+
info:
2740+
title: t
2741+
version: '1'
2742+
paths:
2743+
/x:
2744+
get:
2745+
responses:
2746+
'200':
2747+
description: ok
2748+
content:
2749+
application/json:
2750+
schema:
2751+
$ref: "User.json"
2752+
`
2753+
2754+
userJSON := `{
2755+
"type": "object",
2756+
"properties": {
2757+
"id": { "type": "string" }
2758+
}
2759+
}
2760+
`
2761+
2762+
tmp := t.TempDir()
2763+
write := func(name, src string) {
2764+
require.NoError(t, os.WriteFile(filepath.Join(tmp, name), []byte(src), 0644))
2765+
}
2766+
write("openapi.yaml", rootSpec)
2767+
write("User.json", userJSON)
2768+
2769+
specBytes, err := os.ReadFile(filepath.Join(tmp, "openapi.yaml"))
2770+
require.NoError(t, err)
2771+
2772+
var logBuf strings.Builder
2773+
logger := slog.New(slog.NewTextHandler(&logBuf, &slog.HandlerOptions{Level: slog.LevelDebug}))
2774+
2775+
bundled, err := BundleBytesComposed(specBytes, &datamodel.DocumentConfiguration{
2776+
BasePath: tmp,
2777+
SpecFilePath: filepath.Join(tmp, "openapi.yaml"),
2778+
AllowFileReferences: true,
2779+
Logger: logger,
2780+
}, nil)
2781+
require.NoError(t, err)
2782+
2783+
// the composer should never give up on a recognisable schema just because it arrived
2784+
// from a JSON file — that warning is the bug's calling card.
2785+
assert.NotContains(t, logBuf.String(), "unable to compose reference",
2786+
"JSON schema refs must be classifiable; log:\n%s", logBuf.String())
2787+
2788+
var doc map[string]any
2789+
require.NoError(t, yaml.Unmarshal(bundled, &doc))
2790+
2791+
components, ok := doc["components"].(map[string]any)
2792+
require.True(t, ok, "bundled doc must have components")
2793+
schemas, ok := components["schemas"].(map[string]any)
2794+
require.True(t, ok, "bundled doc must have components.schemas; got:\n%s", string(bundled))
2795+
assert.Contains(t, schemas, "User",
2796+
"JSON-sourced schema should be hoisted under components.schemas; got:\n%s", string(bundled))
2797+
2798+
// and the original $ref site should now point at the hoisted component, not contain the raw schema.
2799+
assert.Contains(t, string(bundled), "#/components/schemas/User")
2800+
assert.NotContains(t, string(bundled), "User.json")
2801+
}

bundler/detect_type.go

Lines changed: 22 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -148,26 +148,38 @@ func hasPathItemProperties(node *yaml.Node) bool {
148148
return containsKey(keys, v3.ParametersLabel)
149149
}
150150

151-
// Helper function to get all keys from a mapping node
152-
// Excludes quoted keys since they should be treated as literal strings, not OpenAPI keywords
151+
// getNodeKeys returns the keys of a mapping node for component-type detection.
152+
//
153+
// When the source document mixes plain and quoted keys, a quoted key is interpreted as a
154+
// deliberate escape — the user is signalling "treat this as a literal string, not an OpenAPI
155+
// keyword" — and is excluded from the result. When every key in the mapping shares the same
156+
// quote style, the style carries no such signal: most commonly this is a JSON-sourced
157+
// mapping where quoting is syntactic (JSON requires `"key":`). In that case every key is
158+
// returned. See https://github.com/pb33f/libopenapi/issues/562.
153159
func getNodeKeys(node *yaml.Node) []string {
154160
if node.Kind == yaml.DocumentNode && len(node.Content) > 0 {
155161
node = node.Content[0]
156162
}
157-
if node.Kind != yaml.MappingNode {
163+
if node.Kind != yaml.MappingNode || len(node.Content) == 0 {
158164
return nil
159165
}
160166

167+
mixedQuoteStyle := false
168+
firstStyle := node.Content[0].Style
169+
for i := 2; i < len(node.Content); i += 2 {
170+
if node.Content[i].Style != firstStyle {
171+
mixedQuoteStyle = true
172+
break
173+
}
174+
}
175+
161176
var keys []string
162177
for i := 0; i < len(node.Content); i += 2 {
163-
if i < len(node.Content) {
164-
keyNode := node.Content[i]
165-
// Skip quoted keys - they should not be treated as OpenAPI keywords
166-
if keyNode.Style == yaml.SingleQuotedStyle || keyNode.Style == yaml.DoubleQuotedStyle {
167-
continue
168-
}
169-
keys = append(keys, keyNode.Value)
178+
keyNode := node.Content[i]
179+
if mixedQuoteStyle && (keyNode.Style == yaml.SingleQuotedStyle || keyNode.Style == yaml.DoubleQuotedStyle) {
180+
continue
170181
}
182+
keys = append(keys, keyNode.Value)
171183
}
172184
return keys
173185
}

bundler/detect_type_test.go

Lines changed: 108 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -601,19 +601,35 @@ func parseYaml(t *testing.T, yamlStr string) *yaml.Node {
601601
}
602602

603603
func TestDetectOpenAPIComponentType_QuotedKeys(t *testing.T) {
604-
// Test that quoted "items" key is NOT treated as schema
605-
quotedItemsYaml := `
604+
// mixed-style: a user deliberately escaping a single OpenAPI keyword in otherwise
605+
// plain YAML. The escape signals "treat this key as a literal string", so the
606+
// quoted key must not participate in component-type detection. The unquoted key
607+
// (`type`) still classifies this mapping as a schema.
608+
mixedQuotedItemsYaml := `
609+
type: object
606610
"items":
607611
- product_id: 1000012
608612
fulfillment_node_id: US01
609613
count: 10
610614
`
611-
node := parseYaml(t, quotedItemsYaml)
615+
node := parseYaml(t, mixedQuotedItemsYaml)
612616
componentType, detected := DetectOpenAPIComponentType(node)
617+
assert.Equal(t, v3.SchemasLabel, componentType)
618+
assert.True(t, detected)
619+
620+
// fully escaped: every key is quoted with no other schema indicators. With nothing
621+
// left to classify on, detection correctly fails.
622+
fullyEscapedYaml := `
623+
description: not a schema
624+
"items":
625+
- product_id: 1000012
626+
`
627+
node = parseYaml(t, fullyEscapedYaml)
628+
componentType, detected = DetectOpenAPIComponentType(node)
613629
assert.Equal(t, "", componentType)
614630
assert.False(t, detected)
615631

616-
// Test that unquoted items key IS treated as schema
632+
// unquoted `items` key on its own still detects as a schema.
617633
unquotedItemsYaml := `
618634
items:
619635
type: string
@@ -623,17 +639,7 @@ items:
623639
assert.Equal(t, v3.SchemasLabel, componentType)
624640
assert.True(t, detected)
625641

626-
// Test that quoted "type" key is NOT treated as schema
627-
quotedTypeYaml := `
628-
"type": "user"
629-
"name": "John"
630-
`
631-
node = parseYaml(t, quotedTypeYaml)
632-
componentType, detected = DetectOpenAPIComponentType(node)
633-
assert.Equal(t, "", componentType)
634-
assert.False(t, detected)
635-
636-
// Test that unquoted type key IS treated as schema
642+
// plain-style schema is unaffected.
637643
unquotedTypeYaml := `
638644
type: object
639645
properties:
@@ -645,7 +651,7 @@ properties:
645651
assert.Equal(t, v3.SchemasLabel, componentType)
646652
assert.True(t, detected)
647653

648-
// Test mixed quoted and unquoted keys - should detect schema from unquoted key
654+
// mixed-style: the quoted `items` escape is ignored, classification driven by `type`.
649655
mixedYaml := `
650656
type: object
651657
"items":
@@ -656,3 +662,89 @@ type: object
656662
assert.Equal(t, v3.SchemasLabel, componentType)
657663
assert.True(t, detected)
658664
}
665+
666+
// TestDetectOpenAPIComponentType_JSONSourced covers https://github.com/pb33f/libopenapi/issues/562:
667+
// SON-parsed mappings arrive with every key marked yaml.DoubleQuotedStyle.
668+
// Uniform quoting on a JSON-sourced mapping is syntactic — not a
669+
// user escape — and must not disable component-type detection.
670+
func TestDetectOpenAPIComponentType_JSONSourced(t *testing.T) {
671+
cases := []struct {
672+
name string
673+
json string
674+
want string
675+
}{
676+
{
677+
name: "schema with type and properties",
678+
json: `{"type": "object", "properties": {"id": {"type": "string"}}}`,
679+
want: v3.SchemasLabel,
680+
},
681+
{
682+
name: "schema with items",
683+
json: `{"type": "array", "items": {"type": "string"}}`,
684+
want: v3.SchemasLabel,
685+
},
686+
{
687+
name: "schema with allOf",
688+
json: `{"allOf": [{"type": "object"}]}`,
689+
want: v3.SchemasLabel,
690+
},
691+
{
692+
name: "parameter",
693+
json: `{"name": "id", "in": "query"}`,
694+
want: v3.ParametersLabel,
695+
},
696+
{
697+
name: "response with content",
698+
json: `{"description": "ok", "content": {"application/json": {"schema": {"type": "object"}}}}`,
699+
want: v3.ResponsesLabel,
700+
},
701+
}
702+
703+
for _, tc := range cases {
704+
t.Run(tc.name, func(t *testing.T) {
705+
node := parseYaml(t, tc.json)
706+
componentType, detected := DetectOpenAPIComponentType(node)
707+
assert.True(t, detected, "expected detection to succeed for JSON input")
708+
assert.Equal(t, tc.want, componentType)
709+
})
710+
}
711+
}
712+
713+
// TestGetNodeKeys_UniformQuoting guards the detector's key-filter against JSON. When every
714+
// key in a mapping shares the same quote style, the style carries no intent to escape
715+
// keywords, the filter only kicks in when styles are mixed within one mapping.
716+
func TestGetNodeKeys_UniformQuoting(t *testing.T) {
717+
cases := []struct {
718+
name string
719+
yaml string
720+
want []string
721+
}{
722+
{
723+
name: "JSON (all double-quoted)",
724+
yaml: `{"type": "object", "properties": {"x": {"type": "string"}}}`,
725+
want: []string{"type", "properties"},
726+
},
727+
{
728+
name: "YAML all single-quoted",
729+
yaml: "'type': object\n'properties':\n name:\n type: string\n",
730+
want: []string{"type", "properties"},
731+
},
732+
{
733+
name: "mixed quoting drops the quoted keys",
734+
yaml: "type: object\n\"properties\": literal\n",
735+
want: []string{"type"},
736+
},
737+
{
738+
name: "plain YAML untouched",
739+
yaml: "type: object\nproperties:\n x:\n type: string\n",
740+
want: []string{"type", "properties"},
741+
},
742+
}
743+
744+
for _, tc := range cases {
745+
t.Run(tc.name, func(t *testing.T) {
746+
node := parseYaml(t, tc.yaml)
747+
assert.ElementsMatch(t, tc.want, getNodeKeys(node))
748+
})
749+
}
750+
}

0 commit comments

Comments
 (0)