diff --git a/datamodel/low/v2/paths.go b/datamodel/low/v2/paths.go index 5397449..97647e6 100644 --- a/datamodel/low/v2/paths.go +++ b/datamodel/low/v2/paths.go @@ -4,7 +4,6 @@ package v2 import ( - "context" "crypto/sha256" "fmt" "sort" @@ -72,8 +71,7 @@ func (p *Paths) Build(_, root *yaml.Node, idx *index.SpecIndex) error { pathsMap := make(map[low.KeyReference[string]]low.ValueReference[*PathItem]) in := make(chan buildInput) out := make(chan pathBuildResult) - ctx, cancel := context.WithCancel(context.Background()) - defer cancel() + done := make(chan struct{}) var wg sync.WaitGroup wg.Add(2) // input and output goroutines. @@ -104,7 +102,7 @@ func (p *Paths) Build(_, root *yaml.Node, idx *index.SpecIndex) error { currentNode: currentNode, pathNode: pathNode, }: - case <-ctx.Done(): + case <-done: return } } @@ -112,31 +110,25 @@ func (p *Paths) Build(_, root *yaml.Node, idx *index.SpecIndex) error { // TranslatePipeline output. go func() { - defer func() { - cancel() - wg.Done() - }() for { - select { - case result, ok := <-out: - if !ok { - return - } - pathsMap[result.key] = result.value - case <-ctx.Done(): - return + result, ok := <-out + if !ok { + break } + pathsMap[result.key] = result.value } + close(done) + wg.Done() }() - translateFunc := func(value buildInput) (retval pathBuildResult, _ error) { + translateFunc := func(value buildInput) (pathBuildResult, error) { pNode := value.pathNode cNode := value.currentNode path := new(PathItem) _ = low.BuildModel(pNode, path) err := path.Build(cNode, pNode, idx) if err != nil { - return retval, err + return pathBuildResult{}, err } return pathBuildResult{ key: low.KeyReference[string]{ diff --git a/datamodel/low/v2/paths_test.go b/datamodel/low/v2/paths_test.go index 127b861..593fc7b 100644 --- a/datamodel/low/v2/paths_test.go +++ b/datamodel/low/v2/paths_test.go @@ -4,11 +4,13 @@ package v2 import ( + "fmt" + "testing" + "github.com/pb33f/libopenapi/datamodel/low" "github.com/pb33f/libopenapi/index" "github.com/stretchr/testify/assert" "gopkg.in/yaml.v3" - "testing" ) func TestPaths_Build(t *testing.T) { @@ -33,10 +35,10 @@ func TestPaths_Build(t *testing.T) { func TestPaths_FindPathAndKey(t *testing.T) { yml := `/no/sleep: - get: + get: description: til brooklyn /no/pizza: - post: + post: description: because i'm fat` var idxNode yaml.Node @@ -57,7 +59,7 @@ func TestPaths_Hash(t *testing.T) { yml := `/data/dog: get: - description: does data kinda, ish. + description: does data kinda, ish. /snow/flake: get: description: does data @@ -80,7 +82,7 @@ x-milk: creamy` description: does data the best /data/dog: get: - description: does data kinda, ish. + description: does data kinda, ish. /snow/flake: get: description: does data @@ -99,3 +101,28 @@ x-milk: creamy` assert.Len(t, n.GetExtensions(), 1) } + +// Test parse failure among many paths. +// This stresses `TranslatePipeline`'s error handling. +func TestPaths_Build_Fail_Many(t *testing.T) { + var yml string + for i := 0; i < 1000; i++ { + format := `"/fresh/code%d": + parameters: + $ref: break +` + yml += fmt.Sprintf(format, i) + } + + var idxNode yaml.Node + mErr := yaml.Unmarshal([]byte(yml), &idxNode) + assert.NoError(t, mErr) + idx := index.NewSpecIndex(&idxNode) + + var n Paths + err := low.BuildModel(&idxNode, &n) + assert.NoError(t, err) + + err = n.Build(idxNode.Content[0], idx) + assert.Error(t, err) +} diff --git a/datamodel/low/v3/components.go b/datamodel/low/v3/components.go index 3f76856..d8b6f8d 100644 --- a/datamodel/low/v3/components.go +++ b/datamodel/low/v3/components.go @@ -4,7 +4,6 @@ package v3 import ( - "context" "crypto/sha256" "fmt" "sort" @@ -234,16 +233,18 @@ func extractComponentValues[T low.Buildable[N], N any](label string, root *yaml. return emptyResult, fmt.Errorf("node is array, cannot be used in components: line %d, column %d", nodeValue.Line, nodeValue.Column) } - ctx, cancel := context.WithCancel(context.Background()) - defer cancel() in := make(chan componentInput) out := make(chan componentBuildResult[T]) + done := make(chan struct{}) var wg sync.WaitGroup wg.Add(2) // input and output goroutines. // Send input. go func() { - defer wg.Done() + defer func() { + close(in) + wg.Done() + }() var currentLabel *yaml.Node for i, node := range nodeValue.Content { // always ignore extensions @@ -261,11 +262,10 @@ func extractComponentValues[T low.Buildable[N], N any](label string, root *yaml. node: node, currentLabel: currentLabel, }: - case <-ctx.Done(): + case <-done: return } } - close(in) }() // Collect output. @@ -273,7 +273,7 @@ func extractComponentValues[T low.Buildable[N], N any](label string, root *yaml. for result := range out { componentValues[result.key] = result.value } - cancel() + close(done) wg.Done() }() diff --git a/datamodel/low/v3/components_test.go b/datamodel/low/v3/components_test.go index a18a7da..18951d7 100644 --- a/datamodel/low/v3/components_test.go +++ b/datamodel/low/v3/components_test.go @@ -4,11 +4,13 @@ package v3 import ( + "fmt" + "testing" + "github.com/pb33f/libopenapi/datamodel/low" "github.com/pb33f/libopenapi/index" "github.com/stretchr/testify/assert" "gopkg.in/yaml.v3" - "testing" ) var testComponentsYaml = ` @@ -38,7 +40,7 @@ var testComponentsYaml = ` description: nine of many ten: description: ten of many - headers: + headers: eleven: description: eleven of many twelve: @@ -53,7 +55,7 @@ var testComponentsYaml = ` description: fifteen of many sixteen: description: sixteen of many - callbacks: + callbacks: seventeen: '{reference}': post: @@ -124,7 +126,7 @@ func TestComponents_Build_Success_Skip(t *testing.T) { func TestComponents_Build_Fail(t *testing.T) { yml := ` - parameters: + parameters: schema: $ref: '#/this is a problem.'` @@ -164,10 +166,39 @@ func TestComponents_Build_ParameterFail(t *testing.T) { } +// Test parse failure among many parameters. +// This stresses `TranslatePipeline`'s error handling. +func TestComponents_Build_ParameterFail_Many(t *testing.T) { + yml := ` + parameters: +` + + for i := 0; i < 1000; i++ { + format := ` + pizza%d: + schema: + $ref: '#/this is a problem.' +` + yml += fmt.Sprintf(format, i) + } + + var idxNode yaml.Node + mErr := yaml.Unmarshal([]byte(yml), &idxNode) + assert.NoError(t, mErr) + idx := index.NewSpecIndex(&idxNode) + + var n Components + err := low.BuildModel(&idxNode, &n) + assert.NoError(t, err) + + err = n.Build(idxNode.Content[0], idx) + assert.Error(t, err) +} + func TestComponents_Build_Fail_TypeFail(t *testing.T) { yml := ` - parameters: + parameters: - schema: $ref: #/this is a problem.` diff --git a/datamodel/low/v3/paths.go b/datamodel/low/v3/paths.go index 937288f..1e5501e 100644 --- a/datamodel/low/v3/paths.go +++ b/datamodel/low/v3/paths.go @@ -4,7 +4,6 @@ package v3 import ( - "context" "crypto/sha256" "fmt" "sort" @@ -69,7 +68,7 @@ func (p *Paths) Build(_, root *yaml.Node, idx *index.SpecIndex) error { // Translate YAML nodes to pathsMap using `TranslatePipeline`. type buildResult struct { - key low.KeyReference[string] + key low.KeyReference[string] value low.ValueReference[*PathItem] } type buildInput struct { @@ -79,8 +78,7 @@ func (p *Paths) Build(_, root *yaml.Node, idx *index.SpecIndex) error { pathsMap := make(map[low.KeyReference[string]]low.ValueReference[*PathItem]) in := make(chan buildInput) out := make(chan buildResult) - ctx, cancel := context.WithCancel(context.Background()) - defer cancel() + done := make(chan struct{}) var wg sync.WaitGroup wg.Add(2) // input and output goroutines. @@ -111,7 +109,7 @@ func (p *Paths) Build(_, root *yaml.Node, idx *index.SpecIndex) error { currentNode: currentNode, pathNode: pathNode, }: - case <-ctx.Done(): + case <-done: return } } @@ -119,21 +117,15 @@ func (p *Paths) Build(_, root *yaml.Node, idx *index.SpecIndex) error { // TranslatePipeline output. go func() { - defer func() { - cancel() - wg.Done() - }() for { - select { - case result, ok := <-out: - if !ok { - return - } - pathsMap[result.key] = result.value - case <-ctx.Done(): - return + result, ok := <-out + if !ok { + break } + pathsMap[result.key] = result.value } + close(done) + wg.Done() }() err := datamodel.TranslatePipeline[buildInput, buildResult](in, out, diff --git a/datamodel/low/v3/paths_test.go b/datamodel/low/v3/paths_test.go index 18a95cc..1a937df 100644 --- a/datamodel/low/v3/paths_test.go +++ b/datamodel/low/v3/paths_test.go @@ -4,6 +4,7 @@ package v3 import ( + "fmt" "testing" "github.com/pb33f/libopenapi/datamodel/low" @@ -21,15 +22,15 @@ func TestPaths_Build(t *testing.T) { post: description: post method put: - description: put method + description: put method delete: description: delete method options: description: options method patch: - description: patch method + description: patch method head: - description: head method + description: head method trace: description: trace method servers: @@ -128,7 +129,7 @@ func TestPaths_Build_FailRefDeadEnd(t *testing.T) { $ref: '#/nowhere' "/some/path": get: - $ref: '#/no/path' + $ref: '#/no/path' "/another/path": $ref: '#/~1some~1path'` @@ -239,7 +240,7 @@ func TestPathItem_Build_GoodRef(t *testing.T) { get: $ref: '#/~1cakes/get' "/cakes": - description: cakes are awesome + description: cakes are awesome get: description: get method from /cakes` @@ -269,7 +270,7 @@ func TestPathItem_Build_BadRef(t *testing.T) { get: $ref: '#/~1cakes/NotFound' "/cakes": - description: cakes are awesome + description: cakes are awesome get: description: get method from /cakes` @@ -478,3 +479,27 @@ x-france: french` assert.Nil(t, b) } + +// Test parse failure among many paths. +// This stresses `TranslatePipeline`'s error handling. +func TestPaths_Build_Fail_Many(t *testing.T) { + var yml string + for i := 0; i < 1000; i++ { + format := `"/fresh/code%d": + parameters: + $ref: break +` + yml += fmt.Sprintf(format, i) + } + + var idxNode yaml.Node + _ = yaml.Unmarshal([]byte(yml), &idxNode) + idx := index.NewSpecIndex(&idxNode) + + var n Paths + err := low.BuildModel(&idxNode, &n) + assert.NoError(t, err) + + err = n.Build(idxNode.Content[0], idx) + assert.Error(t, err) +} diff --git a/datamodel/translate_test.go b/datamodel/translate_test.go index bda86f4..ef7704b 100644 --- a/datamodel/translate_test.go +++ b/datamodel/translate_test.go @@ -1,7 +1,6 @@ package datamodel_test import ( - "context" "errors" "fmt" "io" @@ -10,7 +9,6 @@ import ( "sync" "sync/atomic" "testing" - "time" "github.com/pb33f/libopenapi/datamodel" "github.com/stretchr/testify/assert" @@ -49,12 +47,29 @@ func TestTranslateSliceParallel(t *testing.T) { return nil } err := datamodel.TranslateSliceParallel[int, string](sl, translateFunc, resultFunc) - time.Sleep(10 * time.Millisecond) // DEBUG require.NoError(t, err) assert.Equal(t, int64(mapSize), translateCounter) assert.Equal(t, mapSize, resultCounter) }) + t.Run("nil", func(t *testing.T) { + var sl []int + var translateCounter int64 + translateFunc := func(_, value int) (string, error) { + atomic.AddInt64(&translateCounter, 1) + return "", nil + } + var resultCounter int + resultFunc := func(value string) error { + resultCounter++ + return nil + } + err := datamodel.TranslateSliceParallel[int, string](sl, translateFunc, resultFunc) + require.NoError(t, err) + assert.Zero(t, translateCounter) + assert.Zero(t, resultCounter) + }) + t.Run("Error in translate", func(t *testing.T) { var sl []int for i := 0; i < mapSize; i++ { @@ -188,6 +203,24 @@ func TestTranslateMapParallel(t *testing.T) { assert.Equal(t, expectedResults, results) }) + t.Run("nil", func(t *testing.T) { + var m map[string]int + var translateCounter int64 + translateFunc := func(_ string, value int) (string, error) { + atomic.AddInt64(&translateCounter, 1) + return "", nil + } + var resultCounter int + resultFunc := func(value string) error { + resultCounter++ + return nil + } + err := datamodel.TranslateMapParallel[string, int, string](m, translateFunc, resultFunc) + require.NoError(t, err) + assert.Zero(t, translateCounter) + assert.Zero(t, resultCounter) + }) + t.Run("Error in translate", func(t *testing.T) { m := make(map[string]int) for i := 0; i < mapSize; i++ { @@ -303,43 +336,39 @@ func TestTranslatePipeline(t *testing.T) { var inputErr error in := make(chan int) out := make(chan string) - ctx, cancel := context.WithCancel(context.Background()) - defer cancel() + done := make(chan struct{}) var wg sync.WaitGroup wg.Add(2) // input and output goroutines. // Send input. go func() { - defer wg.Done() + defer func() { + close(in) + wg.Done() + }() for i := 0; i < itemCount; i++ { select { case in <- i: - case <-ctx.Done(): - inputErr = errors.New("Context canceled unexpectedly") + case <-done: + inputErr = errors.New("Exited unexpectedly") + return } } - close(in) }() // Collect output. var resultCounter int go func() { - defer func() { - cancel() - wg.Done() - }() for { - select { - case result, ok := <-out: - if !ok { - return - } - assert.Equal(t, strconv.Itoa(resultCounter), result) - resultCounter++ - case <-ctx.Done(): - return + result, ok := <-out + if !ok { + break } + assert.Equal(t, strconv.Itoa(resultCounter), result) + resultCounter++ } + close(done) + wg.Done() }() err := datamodel.TranslatePipeline[int, string](in, out, @@ -356,41 +385,36 @@ func TestTranslatePipeline(t *testing.T) { t.Run("Error in translate", func(t *testing.T) { in := make(chan int) out := make(chan string) - ctx, cancel := context.WithCancel(context.Background()) - defer cancel() + done := make(chan struct{}) var wg sync.WaitGroup wg.Add(2) // input and output goroutines. // Send input. go func() { - defer wg.Done() for i := 0; i < itemCount; i++ { select { case in <- i: - case <-ctx.Done(): - // Context expected to cancel after the first translate. + case <-done: + // Expected to exit after the first translate. } } close(in) + wg.Done() }() // Collect output. var resultCounter int go func() { defer func() { - cancel() + close(done) wg.Done() }() for { - select { - case _, ok := <-out: - if !ok { - return - } - resultCounter++ - case <-ctx.Done(): + _, ok := <-out + if !ok { return } + resultCounter++ } }() @@ -408,8 +432,7 @@ func TestTranslatePipeline(t *testing.T) { var inputErr error in := make(chan int) out := make(chan string) - ctx, cancel := context.WithCancel(context.Background()) - defer cancel() + done := make(chan struct{}) var wg sync.WaitGroup wg.Add(2) // input and output goroutines. @@ -419,8 +442,8 @@ func TestTranslatePipeline(t *testing.T) { for i := 0; i < itemCount; i++ { select { case in <- i: - case <-ctx.Done(): - inputErr = errors.New("Context canceled unexpectedly") + case <-done: + inputErr = errors.New("Exited unexpectedly") } } close(in) @@ -429,21 +452,15 @@ func TestTranslatePipeline(t *testing.T) { // Collect output. var resultCounter int go func() { - defer func() { - cancel() - wg.Done() - }() for { - select { - case _, ok := <-out: - if !ok { - return - } - resultCounter++ - case <-ctx.Done(): - return + _, ok := <-out + if !ok { + break } + resultCounter++ } + close(done) + wg.Done() }() err := datamodel.TranslatePipeline[int, string](in, out,