perf(compilers/openapi): index the source tree once per compile

compilers/openapi/cycles_test.go

@@ -8,11 +8,13 @@ import (
 
 	"github.com/stretchr/testify/assert"
 	"github.com/stretchr/testify/require"
+	yaml "gopkg.in/yaml.v3"
 
 	"github.com/dexpace/morphic/compilers"
 	"github.com/dexpace/morphic/compilers/openapi/internal/diag"
 	"github.com/dexpace/morphic/compilers/openapi/internal/openapitest"
 	"github.com/dexpace/morphic/compilers/openapi/internal/scan"
+	"github.com/dexpace/morphic/compilers/openapi/internal/sourceindex"
 	"github.com/dexpace/morphic/ir"
 )
 
@@ -96,7 +98,7 @@ func TestDetectCycles_ComponentOnlyCyclesLeftToResolver(t *testing.T) {
 	for _, tc := range componentOnlyCycles {
 		t.Run(tc.name, func(t *testing.T) {
 			t.Parallel()
-			assert.Empty(t, scan.Cycles(0, []byte(tc.data)),
+			assert.Empty(t, scan.Cycles(0, scanIndex(t, tc.data)),
 				"a components-only cycle is the resolver's to report")
 
 			_, diags, err := New().Compile(t.Context(),
@@ -209,6 +211,15 @@ func TestCompile_RefShapedDataNotRefused(t *testing.T) {
 	}
 }
 
+// scanIndex decodes a spec and indexes it, which is what load hands the scan
+// once the compile's one decode has run.
+func scanIndex(t *testing.T, src string) sourceindex.Index {
+	t.Helper()
+	var root yaml.Node
+	require.NoError(t, yaml.Unmarshal([]byte(src), &root))
+	return sourceindex.Build(&root, sourceindex.MaxIndexedNodes)
+}
+
 // TestCompile_SchemaEmptyPointerSegmentIsUnresolved pins where reading the
 // empty token changes a verdict rather than a hang. Reading '/A/' as stopping
 // at A made this shape a cycle; reading it as descending through A makes it

compilers/openapi/internal/diag/diag.go

@@ -38,6 +38,12 @@ const (
 	// for the source. It is a warning, never a refusal: the compile still
 	// proceeds, and every cycle the scan did classify is still caught.
 	CycleScanFailed = "openapi/cycle-scan-failed"
+	// SourceTooLarge reports a document with more YAML nodes than the pre-parse
+	// scan indexes (sourceindex.MaxIndexedNodes). Every answer the index gives
+	// about such a document is a partial one, including the node count the
+	// alias-expansion allowance is derived from, so the document is refused rather
+	// than scanned against a bound computed from a count that stopped early.
+	SourceTooLarge = "openapi/source-too-large"
 	// OverlayInvalid reports an overlay document that could not be parsed, or that
 	// parsed but is not a valid Overlay — a missing version, no actions, an action
 	// naming no target. Nothing is applied, so the compile refuses rather than

compilers/openapi/internal/diag/diag_test.go

@@ -128,7 +128,8 @@ func TestHasError_Cases(t *testing.T) {
 func codes() []string {
 	return []string{
 		diag.Validation, diag.UnsupportedVersion, diag.UnresolvedRef, diag.CyclicRef,
-		diag.CycleScanFailed, diag.OverlayInvalid, diag.OverlayFailed,
+		diag.CycleScanFailed, diag.SourceTooLarge,
+		diag.OverlayInvalid, diag.OverlayFailed,
 		diag.OverlayAction, diag.OverlayOriginIncomplete,
 		diag.ValidationOnlyKeyword, diag.FalseSchema,
 		diag.NumericPrecision, diag.ExclusiveBoundForm, diag.InvalidStatusKey,

compilers/openapi/internal/load/entry_internal_test.go

@@ -8,11 +8,14 @@ import (
 	"github.com/speakeasy-api/openapi/validation"
 	"github.com/stretchr/testify/assert"
 	"github.com/stretchr/testify/require"
+	yaml "gopkg.in/yaml.v3"
 
 	"github.com/dexpace/morphic/compilers"
 	"github.com/dexpace/morphic/compilers/openapi/internal/diag"
 	"github.com/dexpace/morphic/compilers/openapi/internal/openapitest"
 	"github.com/dexpace/morphic/compilers/openapi/internal/overlay"
+	"github.com/dexpace/morphic/compilers/openapi/internal/sourceindex"
+	"github.com/dexpace/morphic/ir"
 )
 
 // TestLoad_DegenerateCycleIsRefusedBeforeParsing pins the first gate in the
@@ -298,3 +301,84 @@ func TestLoad_RejectsAnOverlaySharingTheSourceIndex(t *testing.T) {
 	require.NoError(t, err, "an index of its own is fine: %+v", diags)
 	assert.True(t, got.Overlay.Applied())
 }
+
+// TestLoad_ADocumentTooLargeToIndexIsRefused drives the refusal a document draws
+// before the cycle scan reads a single reference: one with more YAML nodes than
+// the source index walks.
+//
+// The bound is reached by shrinking it rather than by building a document of
+// sourceindex.MaxIndexedNodes nodes — that would be several gigabytes of
+// fixture, and the part worth testing is what the loader does with a truncated
+// index, not that the walk stops at a number.
+func TestLoad_ADocumentTooLargeToIndexIsRefused(t *testing.T) {
+	t.Parallel()
+	opts := Options{buildIndex: func(root *yaml.Node) sourceindex.Index {
+		return sourceindex.Build(root, 1)
+	}}
+
+	doc, diags, err := Load(t.Context(), 4, openapitest.SourceOf(minimal31), opts)
+
+	require.NoError(t, err, "an oversized document is a spec problem, not a Go error")
+	assert.Nil(t, doc, "nothing is lowered from a document the pre-parse scan cannot cover")
+	require.Len(t, diags, 1)
+	assert.Equal(t, diag.SourceTooLarge, diags[0].Code)
+	assert.Equal(t, ir.SeverityError, diags[0].Severity)
+	assert.Equal(t, 4, diags[0].Provenance.Source, "the refusal names the source it read")
+}
+
+// TestLoad_TheSameDocumentLoadsOnceItFitsTheIndex is the control for the test
+// above: with the real bound in force the identical source loads, so the refusal
+// is the bound's doing and not the document's.
+func TestLoad_TheSameDocumentLoadsOnceItFitsTheIndex(t *testing.T) {
+	t.Parallel()
+	got, diags, err := Load(t.Context(), 4, openapitest.SourceOf(minimal31), Options{})
+
+	require.NoError(t, err)
+	require.NotNil(t, got)
+	assert.False(t, diag.HasError(diags), "unexpected refusal: %+v", diags)
+}
+
+// countingIndexBuilder returns base with an index builder that counts its calls,
+// and the counter it writes. The count is one load's own, so tests using it stay
+// independent of each other and of anything running beside them.
+func countingIndexBuilder(base Options) (Options, *int) {
+	built := 0
+	base.buildIndex = func(root *yaml.Node) sourceindex.Index {
+		built++
+		return defaultIndex(root)
+	}
+	return base, &built
+}
+
+// TestLoad_IndexesTheSourceOnce guards the shape of this path rather than its
+// output: one decode, one index, and every pre-parse refusal reading that index
+// instead of walking the tree again. A change that re-added a walk of its own
+// would break no assertion about what the compiler reports — the refusals would
+// still be right — so the count is the only thing that can hold it.
+func TestLoad_IndexesTheSourceOnce(t *testing.T) {
+	t.Parallel()
+	opts, built := countingIndexBuilder(Options{})
+
+	got, diags, err := Load(t.Context(), 0, openapitest.SourceOf(minimal31), opts)
+
+	require.NoError(t, err)
+	require.NotNil(t, got)
+	assert.False(t, diag.HasError(diags), "unexpected refusal: %+v", diags)
+	assert.Equal(t, 1, *built, "a compile with no overlay indexes its source exactly once")
+}
+
+// TestLoad_IndexesAPatchedTreeAgain is the one second index that is correct: an
+// overlay leaves behind a tree the first one no longer describes, and what the
+// refusals answer for is the tree the parser is handed.
+func TestLoad_IndexesAPatchedTreeAgain(t *testing.T) {
+	t.Parallel()
+	opts, built := countingIndexBuilder(
+		overlayOptions("  - target: $.info\n    update: {description: d}\n"))
+
+	got, diags, err := Load(t.Context(), 0, openapitest.SourceOf(minimal31), opts)
+
+	require.NoError(t, err)
+	require.NotNil(t, got)
+	assert.False(t, diag.HasError(diags), "unexpected refusal: %+v", diags)
+	assert.Equal(t, 2, *built, "the source, then the tree the overlay left behind")
+}

compilers/openapi/internal/load/load.go

@@ -29,6 +29,7 @@ import (
 	"github.com/dexpace/morphic/compilers/openapi/internal/diag"
 	"github.com/dexpace/morphic/compilers/openapi/internal/overlay"
 	"github.com/dexpace/morphic/compilers/openapi/internal/scan"
+	"github.com/dexpace/morphic/compilers/openapi/internal/sourceindex"
 	"github.com/dexpace/morphic/compilers/openapi/internal/value"
 	"github.com/dexpace/morphic/ir"
 )
@@ -49,6 +50,14 @@ type Options struct {
 	// OverlaySrcIndex is the index the overlay document takes in Document.Sources.
 	// It is read only when Overlay is set.
 	OverlaySrcIndex int
+	// buildIndex builds the pre-parse index over a decoded tree, or nil for the
+	// compiler's own node bound. It is unexported because it is this package's
+	// test seam: it drives the truncated-index refusal without materializing a
+	// document of sourceindex.MaxIndexedNodes nodes, and counts the indexes one
+	// load builds. Carrying it here rather than in a package-level variable keeps
+	// the bound an input to the stage, so nothing a test does to it is visible to
+	// a concurrent load.
+	buildIndex func(root *yaml.Node) sourceindex.Index
 }
 
 // errParse marks a hard failure to parse a source document — an I/O- or
@@ -90,17 +99,17 @@ func Load(ctx context.Context, srcIndex int, src compilers.Source, opts Options)
 		return nil, nil, fmt.Errorf("openapi: overlay source index %d is source %d's own", opts.OverlaySrcIndex, srcIndex)
 	}
 
-	cyc := scan.Cycles(srcIndex, src.Data)
-	if diag.HasError(cyc) {
-		return nil, cyc, nil // degenerate cycle: refuse to lower, do not crash the parser
-	}
-	// cyc may still hold a non-fatal scan-incomplete warning; carry it forward.
-
 	root, err := decode(src.Data)
 	if err != nil {
 		return nil, nil, fmt.Errorf("openapi: decode source %d: %w", srcIndex, err)
 	}
 
+	cyc := refusals(srcIndex, root, opts)
+	if diag.HasError(cyc) {
+		return nil, cyc, nil // degenerate cycle: refuse to lower, do not crash the parser
+	}
+	// cyc may still hold a non-fatal scan-incomplete warning; carry it forward.
+
 	origin, patchDiags := patch(srcIndex, root, opts)
 	cyc = append(cyc, patchDiags...)
 	if diag.HasError(cyc) {
@@ -149,12 +158,44 @@ func Load(ctx context.Context, srcIndex int, src compilers.Source, opts Options)
 	}, diags, nil
 }
 
+// defaultIndex indexes a decoded tree under the compiler's node bound. It is
+// what Options.buildIndex stands in for when a caller leaves it nil, which
+// everything outside this package's tests does.
+func defaultIndex(root *yaml.Node) sourceindex.Index {
+	return sourceindex.Build(root, sourceindex.MaxIndexedNodes)
+}
+
+// refusals indexes a decoded tree once and reports the pre-parse refusals over
+// it: the degenerate reference and alias structures scan finds, or — when the
+// document is too large to index in full — a refusal of its own.
+//
+// The size refusal is here rather than in scan because every answer in a
+// truncated index is a partial one, and the alias-expansion allowance derived
+// from a partial node count would refuse documents on a bound they never
+// crossed. A document that large is beyond what the pre-parse guarantees cover,
+// so it is refused rather than lowered on incomplete information.
+func refusals(srcIndex int, root *yaml.Node, opts Options) []ir.Diagnostic {
+	build := opts.buildIndex
+	if build == nil {
+		build = defaultIndex
+	}
+
+	idx := build(root)
+	if idx.Truncated() {
+		return []ir.Diagnostic{diag.Newf(ir.SeverityError, diag.SourceTooLarge,
+			ir.Provenance{Source: srcIndex},
+			"source document exceeds the %d-node bound the pre-parse scan indexes",
+			sourceindex.MaxIndexedNodes)}
+	}
+	return scan.Cycles(srcIndex, idx)
+}
+
 // patch applies the caller's overlay to the decoded tree, or does nothing when
 // there is none.
 //
 // It re-runs the pre-parse refusals over the result, because the tree that
-// reaches the parser is no longer the bytes scan.Cycles saw: an overlay action
-// can graft a $ref cycle onto a document that had none, and the guarantee those
+// reaches the parser is no longer the one they first saw: an overlay action can
+// graft a $ref cycle onto a document that had none, and the guarantee those
 // refusals exist for is about what the parser is handed.
 func patch(srcIndex int, root *yaml.Node, opts Options) (overlay.Origin, []ir.Diagnostic) {
 	if opts.Overlay == nil {
@@ -164,7 +205,7 @@ func patch(srcIndex int, root *yaml.Node, opts Options) (overlay.Origin, []ir.Di
 	if diag.HasError(diags) {
 		return overlay.Origin{}, diags
 	}
-	return origin, append(diags, scan.CyclesInNode(srcIndex, root)...)
+	return origin, append(diags, refusals(srcIndex, root, opts)...)
 }
 
 // metaSchemaReconciledMinor is the OpenAPI minor whose schema findings are
@@ -364,11 +405,15 @@ func walkNumericScalars(node *yaml.Node, depth int, visit func(*yaml.Node)) {
 // renumbers every line in the document, and every diagnostic about the source
 // would then name a position in a file that exists nowhere.
 //
+// It is the compile's only parse of the source: the pre-parse refusals used to
+// decode the same bytes a second time to scan them, and now read the tree this
+// produces. That also makes it the one place the yaml.v3 alias budget is spent,
+// which is what bounds a billion-laughs expansion before anything walks it.
+//
 // It carries no recover of its own, unlike the model build and the resolve below
-// it. yaml.v3 converts its own faults into errors before they leave Unmarshal,
-// and the pre-parse scan has already decoded these same bytes under a barrier by
-// the time this runs — the third-party code that has been seen to fault here is
-// the layer above the decode, which is where the barrier is.
+// it. yaml.v3 converts its own faults into errors before they leave Unmarshal;
+// the third-party code that has been seen to fault is the layer above the
+// decode, which is where the barriers are.
 func decode(data []byte) (*yaml.Node, error) {
 	var root yaml.Node
 	if err := yaml.Unmarshal(data, &root); err != nil {