Improve coverage. Simplify error handling.

pb33f · Aug 2, 2023 · 8e118ff · 8e118ff
1 parent 8f710b2
commit 8e118ff
Show file tree

Hide file tree

Showing 7 changed files with 193 additions and 109 deletions.
diff --git 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,39 +102,33 @@ func (p *Paths) Build(root *yaml.Node, idx *index.SpecIndex) error {
 				currentNode: currentNode,
 				pathNode:    pathNode,
 			}:
-			case <-ctx.Done():
+			case <-done:
 				return
 			}
 		}
 	}()
 
 	// 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(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
@@ -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
@@ -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,19 +262,18 @@ 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.
 	go func() {
 		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
@@ -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
@@ -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,29 +109,23 @@ func (p *Paths) Build(root *yaml.Node, idx *index.SpecIndex) error {
 				currentNode: currentNode,
 				pathNode:    pathNode,
 			}:
-			case <-ctx.Done():
+			case <-done:
 				return
 			}
 		}
 	}()
 
 	// 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,