Skip to content

Commit d6aeacf

Browse files
fix: resolve YAML merge keys during unmarshalling (#162)
* fix: resolve YAML merge keys during unmarshalling * refactor: address PR review feedback on merge key resolution
1 parent 6f8ee17 commit d6aeacf

File tree

5 files changed

+678
-13
lines changed

5 files changed

+678
-13
lines changed

jsonschema/oas3/schema_unmarshal_test.go

Lines changed: 232 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,7 @@ import (
66

77
"github.com/speakeasy-api/openapi/jsonschema/oas3"
88
"github.com/speakeasy-api/openapi/marshaller"
9+
"github.com/stretchr/testify/assert"
910
"github.com/stretchr/testify/require"
1011
)
1112

@@ -343,3 +344,234 @@ x-metadata:
343344
require.True(t, ok)
344345
require.NotNil(t, ext.Value)
345346
}
347+
348+
func TestSchema_Unmarshal_MergeKeys_Success(t *testing.T) {
349+
t.Parallel()
350+
351+
tests := []struct {
352+
name string
353+
yml string
354+
expectedProps []string
355+
checkOverride bool
356+
overrideKey string
357+
overrideDesc string
358+
}{
359+
{
360+
name: "merge key in properties inherits from anchor",
361+
yml: `
362+
type: object
363+
properties:
364+
id: &base_id
365+
type: integer
366+
description: Base ID
367+
name:
368+
type: string
369+
description: User name
370+
`,
371+
expectedProps: []string{"id", "name"},
372+
},
373+
{
374+
name: "merge key expands aliased mapping into properties",
375+
yml: `
376+
type: object
377+
properties:
378+
<<:
379+
id:
380+
type: integer
381+
description: Base ID
382+
name:
383+
type: string
384+
description: Base name
385+
email:
386+
type: string
387+
description: Email address
388+
`,
389+
expectedProps: []string{"id", "name", "email"},
390+
},
391+
{
392+
name: "merge key with alias expands into properties",
393+
// NOTE: YAML anchors/aliases must be defined in the same document.
394+
// We define the anchor at a top-level key and reference it in properties.
395+
yml: `
396+
type: object
397+
x-base-props: &base_props
398+
id:
399+
type: integer
400+
description: Base ID
401+
name:
402+
type: string
403+
description: Base name
404+
properties:
405+
<<: *base_props
406+
email:
407+
type: string
408+
description: Email address
409+
`,
410+
expectedProps: []string{"id", "name", "email"},
411+
},
412+
{
413+
name: "merge key with override in properties",
414+
yml: `
415+
type: object
416+
x-base-props: &base_props
417+
id:
418+
type: integer
419+
description: Base ID
420+
name:
421+
type: string
422+
description: Base name
423+
properties:
424+
<<: *base_props
425+
name:
426+
type: string
427+
description: Overridden name
428+
email:
429+
type: string
430+
description: Email address
431+
`,
432+
expectedProps: []string{"id", "name", "email"},
433+
checkOverride: true,
434+
overrideKey: "name",
435+
overrideDesc: "Overridden name",
436+
},
437+
}
438+
439+
for _, tt := range tests {
440+
t.Run(tt.name, func(t *testing.T) {
441+
t.Parallel()
442+
443+
var schema oas3.Schema
444+
445+
validationErrs, err := marshaller.Unmarshal(t.Context(), bytes.NewBufferString(tt.yml), &schema)
446+
require.NoError(t, err, "unmarshal should succeed")
447+
require.Empty(t, validationErrs, "should have no validation errors")
448+
449+
require.NotNil(t, schema.Properties, "properties should not be nil")
450+
451+
_, hasMergeKey := schema.Properties.Get("<<")
452+
assert.False(t, hasMergeKey, "merge key '<<' should not appear as a property name")
453+
454+
for _, prop := range tt.expectedProps {
455+
propSchema, ok := schema.Properties.Get(prop)
456+
assert.True(t, ok, "property %q should exist", prop)
457+
assert.NotNil(t, propSchema, "property %q schema should not be nil", prop)
458+
}
459+
460+
assert.Equal(t, len(tt.expectedProps), schema.Properties.Len(), "should have expected number of properties")
461+
462+
if tt.checkOverride {
463+
propSchema, ok := schema.Properties.Get(tt.overrideKey)
464+
require.True(t, ok, "override property %q should exist", tt.overrideKey)
465+
require.True(t, propSchema.IsSchema(), "override property %q should be a schema", tt.overrideKey)
466+
assert.Equal(t, tt.overrideDesc, propSchema.GetSchema().GetDescription(), "overridden property should have the overriding description")
467+
}
468+
})
469+
}
470+
}
471+
472+
func TestSchema_Unmarshal_MergeKeysAtModelLevel_Success(t *testing.T) {
473+
t.Parallel()
474+
475+
yml := `
476+
x-base: &base
477+
type: object
478+
description: Base schema description
479+
<<: *base
480+
title: Extended Schema
481+
properties:
482+
id:
483+
type: integer
484+
`
485+
486+
var schema oas3.Schema
487+
488+
validationErrs, err := marshaller.Unmarshal(t.Context(), bytes.NewBufferString(yml), &schema)
489+
require.NoError(t, err, "unmarshal should succeed")
490+
require.Empty(t, validationErrs, "should have no validation errors")
491+
492+
types := schema.GetType()
493+
require.Len(t, types, 1, "should have type from merge")
494+
assert.Equal(t, oas3.SchemaTypeObject, types[0], "merged type should be object")
495+
assert.Equal(t, "Base schema description", schema.GetDescription(), "should have description from merge")
496+
assert.Equal(t, "Extended Schema", schema.GetTitle(), "explicit title should be present")
497+
498+
require.NotNil(t, schema.Properties, "properties should not be nil")
499+
_, ok := schema.Properties.Get("id")
500+
assert.True(t, ok, "property 'id' should exist")
501+
}
502+
503+
func TestSchema_Unmarshal_NestedMergeChain_Success(t *testing.T) {
504+
t.Parallel()
505+
506+
// base1 defines {id, name}, base2 merges base1 and adds {email},
507+
// final schema merges base2 and adds {age}.
508+
// All four properties should be present via recursive merge resolution.
509+
yml := `
510+
x-base1: &base1
511+
id:
512+
type: integer
513+
name:
514+
type: string
515+
x-base2: &base2
516+
<<: *base1
517+
email:
518+
type: string
519+
type: object
520+
properties:
521+
<<: *base2
522+
age:
523+
type: integer
524+
`
525+
526+
var schema oas3.Schema
527+
528+
validationErrs, err := marshaller.Unmarshal(t.Context(), bytes.NewBufferString(yml), &schema)
529+
require.NoError(t, err, "unmarshal should succeed")
530+
require.Empty(t, validationErrs, "should have no validation errors")
531+
532+
require.NotNil(t, schema.Properties, "properties should not be nil")
533+
534+
_, hasMergeKey := schema.Properties.Get("<<")
535+
assert.False(t, hasMergeKey, "merge key '<<' should not appear as a property name")
536+
537+
expectedProps := []string{"id", "name", "email", "age"}
538+
for _, prop := range expectedProps {
539+
propSchema, ok := schema.Properties.Get(prop)
540+
assert.True(t, ok, "property %q should exist", prop)
541+
assert.NotNil(t, propSchema, "property %q schema should not be nil", prop)
542+
}
543+
544+
assert.Equal(t, len(expectedProps), schema.Properties.Len(), "should have all 4 properties from nested merge chain")
545+
}
546+
547+
func TestSchema_Unmarshal_QuotedLiteralMergeKey_Success(t *testing.T) {
548+
t.Parallel()
549+
550+
// A quoted '<<' is a regular string key, not a merge key.
551+
// It should be treated as a normal property name.
552+
yml := `
553+
type: object
554+
properties:
555+
'<<':
556+
type: string
557+
description: This is a literal property named <<
558+
name:
559+
type: string
560+
`
561+
562+
var schema oas3.Schema
563+
564+
validationErrs, err := marshaller.Unmarshal(t.Context(), bytes.NewBufferString(yml), &schema)
565+
require.NoError(t, err, "unmarshal should succeed")
566+
require.Empty(t, validationErrs, "should have no validation errors")
567+
568+
require.NotNil(t, schema.Properties, "properties should not be nil")
569+
assert.Equal(t, 2, schema.Properties.Len(), "should have 2 properties")
570+
571+
propSchema, ok := schema.Properties.Get("<<")
572+
assert.True(t, ok, "quoted '<<' should be a normal property")
573+
assert.NotNil(t, propSchema, "quoted '<<' property schema should not be nil")
574+
575+
_, ok = schema.Properties.Get("name")
576+
assert.True(t, ok, "property 'name' should exist")
577+
}

marshaller/sequencedmap.go

Lines changed: 9 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -36,6 +36,9 @@ func unmarshalSequencedMap(ctx context.Context, parentName string, node *yaml.No
3636

3737
target.Init()
3838

39+
// Resolve YAML merge keys (<<) before processing
40+
content := yml.ResolveMergeKeys(resolvedNode.Content)
41+
3942
// Pre-scan for duplicate keys to detect them before concurrent processing
4043
type keyInfo struct {
4144
firstLine int
@@ -45,8 +48,8 @@ func unmarshalSequencedMap(ctx context.Context, parentName string, node *yaml.No
4548
indicesToSkip := make(map[int]bool)
4649
var duplicateKeyErrs []error
4750

48-
for i := 0; i < len(resolvedNode.Content); i += 2 {
49-
keyNode := resolvedNode.Content[i]
51+
for i := 0; i < len(content); i += 2 {
52+
keyNode := content[i]
5053
resolvedKeyNode := yml.ResolveAlias(keyNode)
5154
if resolvedKeyNode == nil {
5255
continue
@@ -75,7 +78,7 @@ func unmarshalSequencedMap(ctx context.Context, parentName string, node *yaml.No
7578

7679
g, ctx := errgroup.WithContext(ctx)
7780

78-
numJobs := len(resolvedNode.Content) / 2
81+
numJobs := len(content) / 2
7982
jobsValidationErrs := make([][]error, numJobs)
8083

8184
type keyPair struct {
@@ -85,15 +88,15 @@ func unmarshalSequencedMap(ctx context.Context, parentName string, node *yaml.No
8588

8689
valuesToSet := make([]keyPair, numJobs)
8790

88-
for i := 0; i < len(resolvedNode.Content); i += 2 {
91+
for i := 0; i < len(content); i += 2 {
8992
g.Go(func() error {
9093
// Skip duplicate keys (all but the last occurrence)
9194
if indicesToSkip[i/2] {
9295
return nil
9396
}
9497

95-
keyNode := resolvedNode.Content[i]
96-
valueNode := resolvedNode.Content[i+1]
98+
keyNode := content[i]
99+
valueNode := content[i+1]
97100

98101
// Resolve alias for key node to handle alias keys like *keyAlias :
99102
resolvedKeyNode := yml.ResolveAlias(keyNode)

marshaller/unmarshaller.go

Lines changed: 17 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -360,6 +360,9 @@ func unmarshalModel(ctx context.Context, parentName string, node *yaml.Node, str
360360
}
361361
}
362362

363+
// Resolve YAML merge keys (<<) before processing
364+
content := yml.ResolveMergeKeys(resolvedNode.Content)
365+
363366
// Pre-scan for duplicate keys and determine which indices to skip
364367
// For duplicate keys, we only process the last occurrence (YAML spec behavior)
365368
// and report earlier occurrences as validation errors
@@ -371,8 +374,11 @@ func unmarshalModel(ctx context.Context, parentName string, node *yaml.Node, str
371374
indicesToSkip := make(map[int]bool) // indices that are duplicates (not the last occurrence)
372375
var duplicateKeyErrs []error
373376

374-
for i := 0; i < len(resolvedNode.Content); i += 2 {
375-
keyNode := resolvedNode.Content[i]
377+
for i := 0; i < len(content); i += 2 {
378+
keyNode := yml.ResolveAlias(content[i])
379+
if keyNode == nil {
380+
continue
381+
}
376382
key := keyNode.Value
377383
if info, exists := seenKeys[key]; exists {
378384
// This is a duplicate - mark the previous last occurrence for skipping
@@ -394,7 +400,7 @@ func unmarshalModel(ctx context.Context, parentName string, node *yaml.Node, str
394400
// Process YAML nodes and validate required fields in one pass
395401
foundRequiredFields := sync.Map{}
396402

397-
numJobs := len(resolvedNode.Content) / 2
403+
numJobs := len(content) / 2
398404

399405
var mapNode yaml.Node
400406
var jobMapContent [][]*yaml.Node
@@ -416,17 +422,21 @@ func unmarshalModel(ctx context.Context, parentName string, node *yaml.Node, str
416422
// TODO allow concurrency to be configurable
417423
g, ctx := errgroup.WithContext(ctx)
418424

419-
for i := 0; i < len(resolvedNode.Content); i += 2 {
425+
for i := 0; i < len(content); i += 2 {
420426
g.Go(func() error {
421427
// Skip duplicate keys (all but the last occurrence)
422428
if indicesToSkip[i/2] {
423429
return nil
424430
}
425431

426-
keyNode := resolvedNode.Content[i]
427-
valueNode := resolvedNode.Content[i+1]
432+
keyNode := content[i]
433+
valueNode := content[i+1]
428434

429-
key := keyNode.Value
435+
resolvedKeyNode := yml.ResolveAlias(keyNode)
436+
if resolvedKeyNode == nil {
437+
return nil
438+
}
439+
key := resolvedKeyNode.Value
430440

431441
// Direct field index lookup (eliminates map[string]Field allocation)
432442
fieldIndex, ok := fieldMap.FieldIndexes[key]

0 commit comments

Comments
 (0)