Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
43 changes: 37 additions & 6 deletions bundler/bundler.go
Original file line number Diff line number Diff line change
Expand Up @@ -8,17 +8,19 @@ import (
"context"
"errors"
"fmt"
"gopkg.in/yaml.v3"
"path/filepath"
"slices"
"strings"
"sync"

"go.yaml.in/yaml/v4"

"github.com/pb33f/libopenapi"
"github.com/pb33f/libopenapi/datamodel"
v3 "github.com/pb33f/libopenapi/datamodel/high/v3"
"github.com/pb33f/libopenapi/index"
"github.com/pb33f/libopenapi/orderedmap"
"github.com/pb33f/libopenapi/utils"
)

// ErrInvalidModel is returned when the model is not usable.
Expand Down Expand Up @@ -158,6 +160,7 @@ func compose(model *v3.Document, compositionConfig *BundleCompositionConfig) ([]
updateDiscriminatorMappingsComposed(discriminatorMappings, processedNodes, rolodex)

// anything that could not be recomposed and needs inlining
inlinedPaths := make(map[string]*yaml.Node)
for _, pr := range cf.inlineRequired {
if pr.refPointer != "" {

Expand All @@ -171,11 +174,40 @@ func compose(model *v3.Document, compositionConfig *BundleCompositionConfig) ([]
}
pointerRef := pr.idx.FindComponent(context.Background(), strings.Join(uri, "#/"))
pr.seqRef.Node.Content = pointerRef.Node.Content
// Track this inlined content for reuse
if pr.ref != nil {
inlinedPaths[pr.ref.FullDefinition] = pointerRef.Node
}
continue
}
}
}
pr.seqRef.Node.Content = pr.ref.Node.Content
// Track this inlined content for reuse
if pr.ref != nil {
inlinedPaths[pr.ref.FullDefinition] = pr.ref.Node
}
}

// Fix any remaining absolute path references that match inlined content
// Also check the root index
allIndexes := append(indexes, rolodex.GetRootIndex())
for _, idx := range allIndexes {
for _, seqRef := range idx.GetRawReferencesSequenced() {
if isRef, _, refVal := utils.IsNodeRefValue(seqRef.Node); isRef {
// Check if this is an absolute path that should have been inlined
if filepath.IsAbs(refVal) {
// Try to find matching inlined content
for inlinedPath, inlinedNode := range inlinedPaths {
// Match if paths are the same or if they refer to the same file
if refVal == inlinedPath {
seqRef.Node.Content = inlinedNode.Content
break
}
}
}
}
}
}

b, err := model.Render()
Expand Down Expand Up @@ -390,18 +422,17 @@ func collectDiscriminatorMappingNodesFromIndex(idx *index.SpecIndex, n *yaml.Nod
}
}


// updateDiscriminatorMappingsComposed updates discriminator mapping references to point to composed component locations.
func updateDiscriminatorMappingsComposed(mappingNodes []*yaml.Node, processedNodes *orderedmap.Map[string, *processRef], rolodex *index.Rolodex) {
for _, mappingNode := range mappingNodes {
originalValue := mappingNode.Value

if !strings.Contains(originalValue, "#/") {
continue
}

var matchingIdx *index.SpecIndex

// Search root index first
if ref, refIdx := rolodex.GetRootIndex().SearchIndexForReference(originalValue); ref != nil {
matchingIdx = refIdx
Expand All @@ -414,7 +445,7 @@ func updateDiscriminatorMappingsComposed(mappingNodes []*yaml.Node, processedNod
}
}
}

if matchingIdx != nil {
newRef := renameRef(matchingIdx, originalValue, processedNodes)
if newRef != originalValue {
Expand Down
2 changes: 1 addition & 1 deletion bundler/bundler_composer.go
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@ import (
v3low "github.com/pb33f/libopenapi/datamodel/low/v3"
"github.com/pb33f/libopenapi/index"
"github.com/pb33f/libopenapi/orderedmap"
"gopkg.in/yaml.v3"
"go.yaml.in/yaml/v4"
)

type processRef struct {
Expand Down
36 changes: 18 additions & 18 deletions bundler/bundler_composer_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,7 @@ import (
"github.com/pb33f/libopenapi/index"
"github.com/stretchr/testify/assert"
"github.com/stretchr/testify/require"
"gopkg.in/yaml.v3"
"go.yaml.in/yaml/v4"
)

func TestBundlerComposed(t *testing.T) {
Expand Down Expand Up @@ -206,12 +206,12 @@ components:
// discriminator mapping should be updated to point to the new component reference
mapping := animal["discriminator"].(map[string]any)["mapping"].(map[string]any)
catMapping := mapping["cat"].(string)
assert.True(t, strings.HasPrefix(catMapping, "#/components/schemas/"),
assert.True(t, strings.HasPrefix(catMapping, "#/components/schemas/"),
"discriminator mapping should point to component reference, got: %s", catMapping)
assert.False(t, strings.Contains(catMapping, "./external-cat.yaml"),
assert.False(t, strings.Contains(catMapping, "./external-cat.yaml"),
"discriminator mapping should not contain external file path, got: %s", catMapping)

// oneOf should be updated to point to the new component reference
// oneOf should be updated to point to the new component reference
oneOf := animal["oneOf"].([]any)[0].(map[string]any)
oneOfRef := oneOf["$ref"].(string)
assert.True(t, strings.HasPrefix(oneOfRef, "#/components/schemas/"),
Expand All @@ -232,7 +232,7 @@ components:
runtime.GC()
}

// TestBundleBytesComposed_DiscriminatorMappingMultiple tests that composed bundling
// TestBundleBytesComposed_DiscriminatorMappingMultiple tests that composed bundling
// correctly updates discriminator mappings for multiple external schemas.
func TestBundleBytesComposed_DiscriminatorMappingMultiple(t *testing.T) {
spec := `openapi: 3.0.0
Expand Down Expand Up @@ -303,7 +303,7 @@ components:
assert.False(t, strings.Contains(bikeMapping, "./vehicles/bike.yaml"),
"bike mapping should not contain external file path, got: %s", bikeMapping)

// oneOf should be updated
// oneOf should be updated
oneOf := vehicle["oneOf"].([]any)
carRef := oneOf[0].(map[string]any)["$ref"].(string)
bikeRef := oneOf[1].(map[string]any)["$ref"].(string)
Expand Down Expand Up @@ -393,7 +393,7 @@ components:

mp := vehicle["discriminator"].(map[string]any)["mapping"].(map[string]any)
assert.Equal(t, 1, len(mp), "no new mapping rows should have been synthesised")

carMapping := mp["car"].(string)
assert.True(t, strings.HasPrefix(carMapping, "#/components/schemas/"),
"car mapping should point to component reference, got: %s", carMapping)
Expand All @@ -402,7 +402,7 @@ components:

// Both oneOf entries should be updated to component references
oneOf := vehicle["oneOf"].([]any)
carRef := oneOf[0].(map[string]any)["$ref"].(string)
carRef := oneOf[0].(map[string]any)["$ref"].(string)
bikeRef := oneOf[1].(map[string]any)["$ref"].(string)
assert.True(t, strings.HasPrefix(carRef, "#/components/schemas/"),
"car oneOf reference should point to component reference, got: %s", carRef)
Expand Down Expand Up @@ -565,15 +565,15 @@ components:
animal := schemas["Animal"].(map[string]any)

mapping := animal["discriminator"].(map[string]any)["mapping"].(map[string]any)

catMapping := mapping["cat"].(string)
assert.True(t, strings.HasPrefix(catMapping, "#/components/schemas/"),
"external cat mapping should point to component reference, got: %s", catMapping)

dogMapping := mapping["dog"].(string)
assert.Equal(t, "#/components/schemas/Dog", dogMapping,
"internal dog mapping should remain unchanged, got: %s", dogMapping)

birdMapping := mapping["bird"].(string)
assert.Equal(t, "Bird", birdMapping,
"non-reference bird mapping should remain unchanged, got: %s", birdMapping)
Expand All @@ -588,7 +588,7 @@ components:

_, dogExists := schemas["Dog"]
assert.True(t, dogExists, "Dog schema should exist in components")

foundCat := false
for schemaName := range schemas {
if schemaName == "Cat" || (schemaName != "Animal" && schemaName != "Dog" && strings.Contains(schemaName, "Cat")) {
Expand Down Expand Up @@ -650,11 +650,11 @@ components:
animal := schemas["Animal"].(map[string]any)

mapping := animal["discriminator"].(map[string]any)["mapping"].(map[string]any)

catMapping := mapping["cat"].(string)
assert.Equal(t, "./nonexistent.yaml#/components/schemas/Cat", catMapping,
"invalid cat mapping should remain unchanged, got: %s", catMapping)

dogMapping := mapping["dog"].(string)
assert.True(t, strings.HasPrefix(dogMapping, "#/components/schemas/"),
"valid dog mapping should be updated, got: %s", dogMapping)
Expand Down Expand Up @@ -713,7 +713,7 @@ oneOf:
require.NoError(t, yaml.Unmarshal(out, &doc))

schemas := doc["components"].(map[string]any)["schemas"].(map[string]any)

// Find the composed Animal schema (might be renamed)
var animalSchema map[string]any
for _, schema := range schemas {
Expand All @@ -729,12 +729,12 @@ oneOf:
// discriminator mapping should be updated to point to the new component reference
mapping := animalSchema["discriminator"].(map[string]any)["mapping"].(map[string]any)
catMapping := mapping["cat"].(string)
assert.True(t, strings.HasPrefix(catMapping, "#/components/schemas/"),
assert.True(t, strings.HasPrefix(catMapping, "#/components/schemas/"),
"discriminator mapping should point to component reference, got: %s", catMapping)
assert.False(t, strings.Contains(catMapping, "./cat.yaml"),
assert.False(t, strings.Contains(catMapping, "./cat.yaml"),
"discriminator mapping should not contain external file path, got: %s", catMapping)

// oneOf should be updated to point to the new component reference
// oneOf should be updated to point to the new component reference
oneOf := animalSchema["oneOf"].([]any)[0].(map[string]any)
oneOfRef := oneOf["$ref"].(string)
assert.True(t, strings.HasPrefix(oneOfRef, "#/components/schemas/"),
Expand Down
Loading
Loading