From 1746d811dbe0e014a94192015983c55b17f4c80a Mon Sep 17 00:00:00 2001 From: quobix Date: Fri, 25 Sep 2026 19:24:49 -0400 Subject: [PATCH 1/3] perf: cut render, JSON and build costs across the pipeline Profile-driven fixes for time and allocation, with no change to output. A golden harness hashing every public output (Render, RenderInline, RenderJSON, per-schema renders, bundles, composed bundles, what-changed reports) across all 72 spec fixtures is byte-identical before and after. - NodeBuilder caches per-type field metadata instead of reflecting over every field of every model on every render. - orderedmap ToYamlNode finds keys and values through an index built once per map, replacing a linear search per entry. - datamodel/high/encode_cache.go memoizes yaml Node.Encode by content, bounded to 4096 entries of at most 64 nodes / 2 KB, and is cleared by ClearAllCaches. - Inline schema rendering hoists the circular reference invariants (filepath.Abs and os.Getwd were 49% of validation-render CPU). - internal/jsonnode parses JSON straight into the exact yaml.Node tree yaml v4 builds, and declines anything it cannot reproduce, so the YAML parser is skipped for JSON specs, rolodex files and overlays. - json.YAMLNodeToJSON writes JSON directly and indents with json.Indent; the old converter remains the fallback. - SetField compares against field types computed once; its case expressions were allocating an orderedmap per call. - The low SchemaProxy reads MergeReferencedProperties from the rolodex config instead of building a DocumentConfiguration per Schema(). Rendering used to mutate model enum nodes (Tag "!!str" became "") because Encode desolves []*yaml.Node values in place; encodeSafeValue now clones slices too. pipeline_bench_test.go adds end-to-end benchmarks with a retained-B/op metric. Interleaved A/B, geomean: -57% time, -56% bytes, -60% allocs. Co-Authored-By: Claude Opus 5.5 --- AGENTS.md | 2 + cache.go | 2 + datamodel/high/base/schema_proxy.go | 203 ++++++----- datamodel/high/base/schema_proxy_test.go | 47 +++ datamodel/high/encode_cache.go | 175 +++++++++ datamodel/high/encode_cache_test.go | 183 ++++++++++ datamodel/high/node_builder.go | 354 ++++++++++++------ datamodel/high/node_builder_test.go | 59 +++ datamodel/low/base/schema_proxy.go | 16 +- datamodel/low/base/schema_proxy_test.go | 2 +- datamodel/low/model_builder.go | 70 ++-- datamodel/spec_info.go | 3 +- index/rolodex_file_loader.go | 3 +- index/rolodex_remote_loader.go | 3 +- internal/jsonnode/jsonnode.go | 441 +++++++++++++++++++++++ internal/jsonnode/jsonnode_bench_test.go | 44 +++ internal/jsonnode/jsonnode_test.go | 224 ++++++++++++ json/json.go | 355 +++++++++++++++++- json/json_writer_test.go | 168 +++++++++ orderedmap/builder.go | 135 ++++++- orderedmap/builder_index_test.go | 191 ++++++++++ overlay/engine.go | 3 +- pipeline_bench_test.go | 208 +++++++++++ 23 files changed, 2646 insertions(+), 245 deletions(-) create mode 100644 datamodel/high/encode_cache.go create mode 100644 datamodel/high/encode_cache_test.go create mode 100644 internal/jsonnode/jsonnode.go create mode 100644 internal/jsonnode/jsonnode_bench_test.go create mode 100644 internal/jsonnode/jsonnode_test.go create mode 100644 json/json_writer_test.go create mode 100644 orderedmap/builder_index_test.go create mode 100644 pipeline_bench_test.go diff --git a/AGENTS.md b/AGENTS.md index c354ac0ea..b98119ab6 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -22,6 +22,7 @@ This repo is a library, not an app. The root package exposes the public entry po | `renderer/` | Schema/mock sample generation | | `orderedmap/` | Stable insertion-ordered map wrapper used throughout models/rendering | | `json/` | YAML-node to ordered JSON conversion | +| `internal/jsonnode/` | Direct JSON parser building the exact `yaml.Node` tree yaml v4 builds; used for JSON specs, declines to `yaml.Unmarshal` otherwise | | `tests/` | Cross-package integration and benchmark coverage, especially sibling-ref behavior | | `test_specs/` | Realistic fixtures and regression specs used across packages | @@ -61,6 +62,7 @@ This repo is a library, not an app. The root package exposes the public entry po | `go test ./bundler -run TestBundle` | Target bundler regressions | | `go test ./what-changed/... -run Test` | Target diff/breaking-rule regressions | | `go test -bench . ./index ./datamodel/low/... ./what-changed/...` | Run benchmarks in hot paths | +| `go test -run xxx -bench BenchmarkPipeline -count 6 .` | End-to-end build/render/compare/bundle benchmarks on large specs (compare runs with `benchstat`) | | `GOCACHE=/tmp/go-build go test ./...` | Useful in restricted sandboxes where default Go build cache is not writable | ## Testing Caveats diff --git a/cache.go b/cache.go index 432096e83..55afe9821 100644 --- a/cache.go +++ b/cache.go @@ -4,6 +4,7 @@ package libopenapi import ( + "github.com/pb33f/libopenapi/datamodel/high" highbase "github.com/pb33f/libopenapi/datamodel/high/base" "github.com/pb33f/libopenapi/datamodel/low" lowbase "github.com/pb33f/libopenapi/datamodel/low/base" @@ -24,5 +25,6 @@ func ClearAllCaches() { index.ClearHashCache() // nodeHashCache index.ClearContentDetectionCache() highbase.ClearInlineRenderingTracker() + high.ClearEncodeCache() utils.ClearJSONPathCache() } diff --git a/datamodel/high/base/schema_proxy.go b/datamodel/high/base/schema_proxy.go index 3401c677c..e44109e53 100644 --- a/datamodel/high/base/schema_proxy.go +++ b/datamodel/high/base/schema_proxy.go @@ -8,7 +8,6 @@ import ( "fmt" "net/url" "path/filepath" - "slices" "strconv" "strings" "sync" @@ -104,6 +103,34 @@ type InlineRenderContext struct { referenceNodeTargets sync.Map // authored reference node -> canonical target preservedReferenceNodes sync.Map // authored reference node -> struct{} referenceNodeRewrites sync.Map // authored reference node -> root component ref + absoluteSpecPaths sync.Map // spec path -> its absolute form, resolved once per render + canonicalIdentities sync.Map // reference -> index.CanonicalReferenceIdentity(reference) +} + +// absoluteSpecPath returns a spec path in absolute form, as filepath.Abs does for a relative local path. +// filepath.Abs queries the working directory, so each path is resolved once per render rather than for +// every reference rendered. +func (ctx *InlineRenderContext) absoluteSpecPath(specPath string) string { + if filepath.IsAbs(specPath) || strings.HasPrefix(specPath, "http") { + return specPath + } + if cached, ok := ctx.absoluteSpecPaths.Load(specPath); ok { + return cached.(string) + } + abs, _ := filepath.Abs(specPath) + ctx.absoluteSpecPaths.Store(specPath, abs) + return abs +} + +// canonicalReferenceIdentity memoizes index.CanonicalReferenceIdentity, which parses and cleans the +// reference, for the circular references compared against every rendered reference. +func (ctx *InlineRenderContext) canonicalReferenceIdentity(ref string) string { + if cached, ok := ctx.canonicalIdentities.Load(ref); ok { + return cached.(string) + } + identity := index.CanonicalReferenceIdentity(ref) + ctx.canonicalIdentities.Store(ref, identity) + return identity } // NewInlineRenderContext creates a new isolated rendering context with default bundle mode. @@ -1090,90 +1117,11 @@ func (sp *SchemaProxy) marshalYAMLInlineInternal(ctx *InlineRenderContext) (inte var err error s, err = sp.BuildSchema() - if s != nil && s.GoLow() != nil && s.GoLow().Index != nil { - idx := s.GoLow().Index - - // GetCircularReferences hands back the index's own slice, which the resolver grows with - // append and therefore leaves spare capacity on. clone before extending, or the appends - // below write into memory shared with every other render using this index. - circ := slices.Clone(idx.GetCircularReferences()) - - // extract ignored and safe circular references from rolodex if available, along with the - // root index circulars. circular references are registered on the rolodex root, but this - // schema's index is the one owning its resolved content, which for an external $ref is not - // the root. without that the guard below silently stops firing for referenced schemas. - if rolodex := idx.GetRolodex(); rolodex != nil { - if root := rolodex.GetRootIndex(); root != nil && root != idx { - circ = append(circ, root.GetCircularReferences()...) - } - circ = append(circ, rolodex.GetIgnoredCircularReferences()...) - circ = append(circ, rolodex.GetSafeCircularReferences()...) - } - - cirError := func(str string) error { - return fmt.Errorf("schema render failure, circular reference: `%s`", str) - } - - for _, c := range circ { - if sp.IsReference() { - if c == nil || c.LoopPoint == nil { - continue - } - if ctx.StrictCircularReferenceIdentity { - target := sp.referenceTargetIdentity(ctx) - if target == "" || target != index.CanonicalReferenceIdentity(c.LoopPoint.FullDefinition) { - continue - } - node, rewritten, refErr := rewrittenRefNode() - return sp.circularReferenceResult(ctx, node, rewritten, refErr, cirError(c.LoopPoint.Definition)) - } - if sp.GetReference() == c.LoopPoint.Definition { - node, rewritten, refErr := rewrittenRefNode() - return sp.circularReferenceResult(ctx, node, rewritten, refErr, cirError(c.LoopPoint.Definition)) - } - basePath := idx.GetSpecAbsolutePath() - - if !filepath.IsAbs(basePath) && !strings.HasPrefix(basePath, "http") { - basePath, _ = filepath.Abs(basePath) - } - - if basePath == c.LoopPoint.FullDefinition { - node, rewritten, refErr := rewrittenRefNode() - return sp.circularReferenceResult(ctx, node, rewritten, refErr, cirError(c.LoopPoint.Definition)) - } - a := utils.ReplaceWindowsDriveWithLinuxPath(strings.Replace(c.LoopPoint.FullDefinition, basePath, "", 1)) - b := sp.GetReference() - if strings.HasPrefix(b, "./") { - b = strings.Replace(b, "./", "/", 1) // strip any leading ./ from the reference - } - // if loading things in remotely and references are relative. - if strings.HasPrefix(a, "http") { - purl, _ := url.Parse(a) - if purl != nil { - specPath := filepath.Dir(purl.Path) - host := fmt.Sprintf("%s://%s", purl.Scheme, purl.Host) - a = strings.Replace(a, host, "", 1) - a = strings.Replace(a, specPath, "", 1) - } - } - - aBase, aFragment := index.SplitRefFragment(a) - bBase, bFragment := index.SplitRefFragment(b) - - if aFragment != "" && bFragment != "" && aFragment == bFragment { - node, rewritten, refErr := rewrittenRefNode() - return sp.circularReferenceResult(ctx, node, rewritten, refErr, cirError(c.LoopPoint.Definition)) - } - - if aFragment == "" && bFragment == "" { - aNorm := strings.TrimPrefix(strings.TrimPrefix(aBase, "./"), "/") - bNorm := strings.TrimPrefix(strings.TrimPrefix(bBase, "./"), "/") - if aNorm != "" && bNorm != "" && aNorm == bNorm { - node, rewritten, refErr := rewrittenRefNode() - return sp.circularReferenceResult(ctx, node, rewritten, refErr, cirError(c.LoopPoint.Definition)) - } - } - } + if s != nil && s.GoLow() != nil && s.GoLow().Index != nil && sp.IsReference() { + if loopDefinition, found := sp.matchCircularReference(ctx, s.GoLow().Index); found { + node, rewritten, refErr := rewrittenRefNode() + return sp.circularReferenceResult(ctx, node, rewritten, refErr, + fmt.Errorf("schema render failure, circular reference: `%s`", loopDefinition)) } } @@ -1198,6 +1146,91 @@ func (sp *SchemaProxy) marshalYAMLInlineInternal(ctx *InlineRenderContext) (inte return nil, errors.New("unable to render schema") } +// matchCircularReference reports whether this reference proxy closes a known circular reference, and the +// loop point definition of the first one it matches. The circular references are the schema index's own, +// then the rolodex root's (registered on the root, while an external $ref's resolved content is owned by +// its own index), then the rolodex's ignored and safe circular references. +func (sp *SchemaProxy) matchCircularReference(ctx *InlineRenderContext, idx *index.SpecIndex) (string, bool) { + sources := [4][]*index.CircularReferenceResult{idx.GetCircularReferences()} + if rolodex := idx.GetRolodex(); rolodex != nil { + if root := rolodex.GetRootIndex(); root != nil && root != idx { + sources[1] = root.GetCircularReferences() + } + sources[2] = rolodex.GetIgnoredCircularReferences() + sources[3] = rolodex.GetSafeCircularReferences() + } + + if ctx.StrictCircularReferenceIdentity { + target := sp.referenceTargetIdentity(ctx) + if target == "" { + return "", false + } + for _, circular := range sources { + for _, c := range circular { + if c != nil && c.LoopPoint != nil && + target == ctx.canonicalReferenceIdentity(c.LoopPoint.FullDefinition) { + return c.LoopPoint.Definition, true + } + } + } + return "", false + } + + // everything derived from this proxy's own reference is the same for every candidate. + ref := sp.GetReference() + b := ref + if strings.HasPrefix(b, "./") { + b = strings.Replace(b, "./", "/", 1) // strip any leading ./ from the reference + } + bBase, bFragment := index.SplitRefFragment(b) + bNorm := strings.TrimPrefix(strings.TrimPrefix(bBase, "./"), "/") + basePath := "" + basePathResolved := false + + for _, circular := range sources { + for _, c := range circular { + if c == nil || c.LoopPoint == nil { + continue + } + if ref == c.LoopPoint.Definition { + return c.LoopPoint.Definition, true + } + if !basePathResolved { + basePathResolved = true + basePath = ctx.absoluteSpecPath(idx.GetSpecAbsolutePath()) + } + if basePath == c.LoopPoint.FullDefinition { + return c.LoopPoint.Definition, true + } + a := utils.ReplaceWindowsDriveWithLinuxPath(strings.Replace(c.LoopPoint.FullDefinition, basePath, "", 1)) + // if loading things in remotely and references are relative. + if strings.HasPrefix(a, "http") { + purl, _ := url.Parse(a) + if purl != nil { + specPath := filepath.Dir(purl.Path) + host := fmt.Sprintf("%s://%s", purl.Scheme, purl.Host) + a = strings.Replace(a, host, "", 1) + a = strings.Replace(a, specPath, "", 1) + } + } + + aBase, aFragment := index.SplitRefFragment(a) + + if aFragment != "" && bFragment != "" && aFragment == bFragment { + return c.LoopPoint.Definition, true + } + + if aFragment == "" && bFragment == "" { + aNorm := strings.TrimPrefix(strings.TrimPrefix(aBase, "./"), "/") + if aNorm != "" && bNorm != "" && aNorm == bNorm { + return c.LoopPoint.Definition, true + } + } + } + } + return "", false +} + func (sp *SchemaProxy) marshalParsedRefWithSiblingsInline(ctx *InlineRenderContext, currentSibling *Schema) (interface{}, error) { s, err := sp.buildSemanticAllOfSchemaView(currentSibling) if err != nil { diff --git a/datamodel/high/base/schema_proxy_test.go b/datamodel/high/base/schema_proxy_test.go index 271cc77e2..09f5414dc 100644 --- a/datamodel/high/base/schema_proxy_test.go +++ b/datamodel/high/base/schema_proxy_test.go @@ -2572,3 +2572,50 @@ func TestSchemaProxy_MarshalYAMLInline_CircularReference_OtherWholeFile(t *testi require.NoError(t, err) assert.Equal(t, "type: object\n", string(out)) } + +// matchCircularReference walks every circular reference source in order and skips malformed entries, +// in both identity modes, and reports no match when nothing closes a loop. +func TestSchemaProxy_MatchCircularReference_NoMatch(t *testing.T) { + var idxNode yaml.Node + require.NoError(t, yaml.Unmarshal([]byte("components:\n schemas:\n Ten:\n type: object"), &idxNode)) + idx := index.NewSpecIndexWithConfig(&idxNode, index.CreateOpenAPIIndexConfig()) + idx.SetAbsolutePath(filepath.Join(t.TempDir(), "spec.yaml")) + other := &index.Reference{Definition: "#/components/schemas/Other", + FullDefinition: idx.GetSpecAbsolutePath() + "#/components/schemas/Other"} + idx.SetCircularReferences([]*index.CircularReferenceResult{nil, {}, {LoopPoint: other}}) + + refNode := utils.CreateRefNode("#/components/schemas/Ten") + lowProxy := new(lowbase.SchemaProxy) + require.NoError(t, lowProxy.Build(context.Background(), nil, refNode, idx)) + sp := NewSchemaProxy(&low.NodeReference[*lowbase.SchemaProxy]{Value: lowProxy, ValueNode: refNode}) + + loose := NewInlineRenderContext() + _, found := sp.matchCircularReference(loose, idx) + assert.False(t, found) + + strict := NewInlineRenderContext() + strict.StrictCircularReferenceIdentity = true + for range 2 { // the second pass answers the loop point identity from the context's cache + _, found = sp.matchCircularReference(strict, idx) + assert.False(t, found) + } + + // a reference with no resolvable target identity cannot close a loop in strict mode. + programmatic := CreateSchemaProxyRef("external.yaml#/Thing") + _, found = programmatic.matchCircularReference(strict, idx) + assert.False(t, found) +} + +// absoluteSpecPath resolves a relative spec path the way filepath.Abs does, once per render context, +// and leaves absolute and remote paths alone. +func TestInlineRenderContext_AbsoluteSpecPath(t *testing.T) { + ctx := NewInlineRenderContext() + want, err := filepath.Abs("spec.yaml") + require.NoError(t, err) + assert.Equal(t, want, ctx.absoluteSpecPath("spec.yaml")) + assert.Equal(t, want, ctx.absoluteSpecPath("spec.yaml")) + + abs := filepath.Join(t.TempDir(), "spec.yaml") + assert.Equal(t, abs, ctx.absoluteSpecPath(abs)) + assert.Equal(t, "https://example.com/spec.yaml", ctx.absoluteSpecPath("https://example.com/spec.yaml")) +} diff --git a/datamodel/high/encode_cache.go b/datamodel/high/encode_cache.go new file mode 100644 index 000000000..87e229c56 --- /dev/null +++ b/datamodel/high/encode_cache.go @@ -0,0 +1,175 @@ +// Copyright 2026 Princess B33f Heavy Industries / Dave Shanley +// SPDX-License-Identifier: MIT + +package high + +import ( + "encoding/binary" + "sync" + + "go.yaml.in/yaml/v4" +) + +// The node builder encodes raw values (enum lists, required lists, examples and extension values) with +// (*yaml.Node).Encode, which serializes the value to YAML text and parses it back. The same small values +// repeat throughout a specification, and again every time a schema is rendered inline. Encode's result is +// a pure function of the value's content (yaml ignores node positions when encoding), so a value with the +// same content always encodes to the same tree: the tree is kept, and each render gets its own copy. + +const ( + // encodeCacheMaxNodes bounds the values that are fingerprinted; larger ones are encoded directly. + encodeCacheMaxNodes = 64 + + // encodeCacheMaxKeyBytes bounds a fingerprint, so a large scalar is never held as a key. + encodeCacheMaxKeyBytes = 2048 + + // encodeCacheLimit bounds the entries kept; the cache is emptied when it fills. + encodeCacheLimit = 4096 +) + +var encodeCache = struct { + sync.RWMutex + entries map[string]*encodedValue +}{entries: make(map[string]*encodedValue)} + +// ClearEncodeCache empties the cache of encoded values kept by the node builder. It holds no references +// to model nodes, only its own copies of small encoded values, and is bounded in size. +func ClearEncodeCache() { + encodeCache.Lock() + clear(encodeCache.entries) + encodeCache.Unlock() +} + +// encodedValue is an encoded tree, with the node and Content slot counts of everything below its root. +type encodedValue struct { + root yaml.Node + nodes int + contents int +} + +// encodeValue stores in rawNode what rawNode.Encode(encodeSafeValue(value)) stores, reusing the result +// of an earlier encoding of a value with the same content. +func encodeValue(rawNode *yaml.Node, value any) error { + var scratch [256]byte + key, cacheable := appendEncodeFingerprint(scratch[:0], value) + if !cacheable || len(key) > encodeCacheMaxKeyBytes { + return rawNode.Encode(encodeSafeValue(value)) + } + encodeCache.RLock() + cached := encodeCache.entries[string(key)] + encodeCache.RUnlock() + if cached != nil { + cached.copyInto(rawNode) + return nil + } + if err := rawNode.Encode(encodeSafeValue(value)); err != nil { + return err + } + entry := &encodedValue{} + entry.root = *rawNode + entry.copyChildren(&entry.root, rawNode, nil, nil) + encodeCache.Lock() + if len(encodeCache.entries) >= encodeCacheLimit { + clear(encodeCache.entries) + } + encodeCache.entries[string(key)] = entry + encodeCache.Unlock() + return nil +} + +// copyInto stores a copy of the encoded tree in dst, allocating its nodes and Content slices in one go. +func (e *encodedValue) copyInto(dst *yaml.Node) { + nodes := make([]yaml.Node, e.nodes) + contents := make([]*yaml.Node, e.contents) + *dst = e.root + e.copyChildren(dst, &e.root, &nodes, &contents) +} + +// copyChildren deep copies src's children into dst, which already holds src's fields. With no slabs it +// allocates each node and counts the tree into e; otherwise it carves the copies from the slabs. +func (e *encodedValue) copyChildren(dst, src *yaml.Node, nodes *[]yaml.Node, contents *[]*yaml.Node) { + if src.Content == nil { + return + } + n := len(src.Content) + if contents == nil { + dst.Content = make([]*yaml.Node, n) + e.contents += n + } else { + dst.Content = (*contents)[:n:n] + *contents = (*contents)[n:] + } + for i, child := range src.Content { + var c *yaml.Node + if nodes == nil { + c = new(yaml.Node) + e.nodes++ + } else { + c = &(*nodes)[0] + *nodes = (*nodes)[1:] + } + *c = *child + dst.Content[i] = c + e.copyChildren(c, child, nodes, contents) + } +} + +// appendEncodeFingerprint appends an unambiguous encoding of everything Encode reads from value, and +// reports whether the value can be cached: a node, a slice of nodes or a slice of strings, of bounded +// size, with no anchors or aliases (whose encoding depends on nodes outside the value). +func appendEncodeFingerprint(b []byte, value any) ([]byte, bool) { + budget := encodeCacheMaxNodes + switch v := value.(type) { + case *yaml.Node: + return appendNodeFingerprint(append(b, 'n'), v, &budget) + case []*yaml.Node: + b = binary.AppendUvarint(append(b, 'l'), uint64(len(v))) + for _, n := range v { + var ok bool + if b, ok = appendNodeFingerprint(b, n, &budget); !ok { + return b, false + } + } + return b, true + case []string: + if len(v) > encodeCacheMaxNodes { + return b, false + } + b = binary.AppendUvarint(append(b, 's'), uint64(len(v))) + for _, s := range v { + b = appendFingerprintString(b, s) + } + return b, true + } + return b, false +} + +func appendNodeFingerprint(b []byte, n *yaml.Node, budget *int) ([]byte, bool) { + *budget-- + if n == nil || *budget < 0 || n.Anchor != "" || n.Alias != nil || n.Stream != nil || + (n.Kind != yaml.ScalarNode && n.Kind != yaml.SequenceNode && n.Kind != yaml.MappingNode) { + return b, false + } + b = binary.AppendUvarint(b, uint64(n.Kind)) + b = binary.AppendUvarint(b, uint64(n.Style)) + b = appendFingerprintString(b, n.Tag) + b = appendFingerprintString(b, n.Value) + b = appendFingerprintString(b, n.HeadComment) + b = appendFingerprintString(b, n.LineComment) + b = appendFingerprintString(b, n.FootComment) + if n.Content == nil { + return append(b, 0), true + } + b = binary.AppendUvarint(append(b, 1), uint64(len(n.Content))) + for _, c := range n.Content { + var ok bool + if b, ok = appendNodeFingerprint(b, c, budget); !ok { + return b, false + } + } + return b, true +} + +func appendFingerprintString(b []byte, s string) []byte { + return append(binary.AppendUvarint(b, uint64(len(s))), s...) +} diff --git a/datamodel/high/encode_cache_test.go b/datamodel/high/encode_cache_test.go new file mode 100644 index 000000000..62a4ae7e3 --- /dev/null +++ b/datamodel/high/encode_cache_test.go @@ -0,0 +1,183 @@ +// Copyright 2026 Princess B33f Heavy Industries / Dave Shanley +// SPDX-License-Identifier: MIT + +package high + +import ( + "fmt" + "os" + "path/filepath" + "strings" + "testing" + + "github.com/pb33f/libopenapi/utils" + "github.com/pb33f/testify/assert" + "github.com/pb33f/testify/require" + "go.yaml.in/yaml/v4" +) + +// sameEncodedTree reports the first difference between two node trees, comparing every field. +func sameEncodedTree(want, got *yaml.Node, path string) error { + switch { + case want.Kind != got.Kind || want.Style != got.Style || want.Tag != got.Tag || want.Value != got.Value: + return fmt.Errorf("%s: %v/%v/%q/%q != %v/%v/%q/%q", path, got.Kind, got.Style, got.Tag, got.Value, + want.Kind, want.Style, want.Tag, want.Value) + case want.Anchor != got.Anchor || want.Alias != got.Alias || want.Line != got.Line || want.Column != got.Column: + return fmt.Errorf("%s: anchor, alias or position differ", path) + case want.HeadComment != got.HeadComment || want.LineComment != got.LineComment || + want.FootComment != got.FootComment: + return fmt.Errorf("%s: comments differ", path) + case (want.Content == nil) != (got.Content == nil) || len(want.Content) != len(got.Content): + return fmt.Errorf("%s: content differs", path) + } + for i := range want.Content { + if err := sameEncodedTree(want.Content[i], got.Content[i], fmt.Sprintf("%s/%d", path, i)); err != nil { + return err + } + } + return nil +} + +// requireEncodesLikeYAML asserts encodeValue matches Encode on a first (uncached) and second (cached) call. +func requireEncodesLikeYAML(t *testing.T, value any) { + t.Helper() + var want yaml.Node + wantErr := want.Encode(encodeSafeValue(value)) + for pass := 0; pass < 2; pass++ { + var got yaml.Node + err := encodeValue(&got, value) + require.Equal(t, wantErr, err) + require.NoError(t, sameEncodedTree(&want, &got, "value"), "pass %d", pass) + } +} + +func walkNodes(n *yaml.Node, fn func(*yaml.Node)) { + fn(n) + for _, c := range n.Content { + walkNodes(c, fn) + } +} + +// Every value encodes exactly as Encode encodes it, whether or not an earlier encoding is reused. +func TestEncodeValue_MatchesEncode(t *testing.T) { + ClearEncodeCache() + fixtures := []string{"burgershop.openapi.yaml", "petstorev3.json", "all-the-components.yaml", + "vendor-extensions-components.yaml", "circular-tests.yaml", "nullable-examples.openapi.yaml"} + for _, fixture := range fixtures { + data, err := os.ReadFile(filepath.Join("..", "..", "test_specs", fixture)) + require.NoError(t, err) + var doc yaml.Node + require.NoError(t, yaml.Unmarshal(data, &doc)) + walkNodes(doc.Content[0], func(n *yaml.Node) { + if n.Kind == yaml.ScalarNode && n.Value == "" { + return // Encode panics on an empty scalar, cached or not + } + requireEncodesLikeYAML(t, n) + if n.Kind == yaml.SequenceNode { + requireEncodesLikeYAML(t, n.Content) + strs := make([]string, len(n.Content)) + for i, c := range n.Content { + strs[i] = c.Value + } + requireEncodesLikeYAML(t, strs) + } + }) + } + for _, value := range []any{ + []string{}, []string{"", "true", "123", "a: b", "- x", "multi\nline", " padded ", "#hash", "yes"}, + []*yaml.Node{}, []*yaml.Node{utils.CreateStringNode("one"), utils.CreateIntNode("2")}, + &yaml.Node{Kind: yaml.SequenceNode, Tag: "!!seq", Content: []*yaml.Node{}}, + &yaml.Node{Kind: yaml.ScalarNode, Tag: "!!str", Value: "commented", HeadComment: "# head", + LineComment: "# line", FootComment: "# foot"}, + } { + requireEncodesLikeYAML(t, value) + } +} + +// Values whose encoding cannot be reused are encoded directly, with the same result. +func TestEncodeValue_Uncacheable(t *testing.T) { + ClearEncodeCache() + anchored := &yaml.Node{Kind: yaml.ScalarNode, Tag: "!!str", Value: "anchored", Anchor: "a"} + big := &yaml.Node{Kind: yaml.SequenceNode, Tag: "!!seq"} + for i := 0; i <= encodeCacheMaxNodes; i++ { + big.Content = append(big.Content, utils.CreateIntNode(fmt.Sprint(i))) + } + manyStrings := make([]string, encodeCacheMaxNodes+1) + for i := range manyStrings { + manyStrings[i] = fmt.Sprint(i) + } + for _, value := range []any{ + anchored, + &yaml.Node{Kind: yaml.SequenceNode, Tag: "!!seq", Content: []*yaml.Node{anchored}}, + []*yaml.Node{utils.CreateStringNode("one"), anchored}, + big, + manyStrings, + []string{strings.Repeat("long ", encodeCacheMaxKeyBytes)}, + &yaml.Node{Kind: yaml.DocumentNode, Content: []*yaml.Node{utils.CreateStringNode("doc")}}, + []int{1, 2}, + map[string]string{"a": "b"}, + } { + key, cacheable := appendEncodeFingerprint(nil, value) + assert.False(t, cacheable && len(key) <= encodeCacheMaxKeyBytes, "%T should not be cached", value) + requireEncodesLikeYAML(t, value) + } + encodeCache.RLock() + assert.Empty(t, encodeCache.entries) + encodeCache.RUnlock() + + // an alias node and a nil slice element are never fingerprinted. + _, cacheable := appendEncodeFingerprint(nil, &yaml.Node{Kind: yaml.AliasNode, Alias: anchored}) + assert.False(t, cacheable) + _, cacheable = appendEncodeFingerprint(nil, []*yaml.Node{nil}) + assert.False(t, cacheable) +} + +// An encoding error is returned as Encode returns it, and nothing is cached. +func TestEncodeValue_Error(t *testing.T) { + ClearEncodeCache() + invalid := &yaml.Node{Kind: yaml.ScalarNode, Tag: "!!", Value: "no tag suffix"} + var want, got yaml.Node + wantErr := want.Encode(encodeSafeValue(invalid)) + require.Error(t, wantErr) + require.Equal(t, wantErr, encodeValue(&got, invalid)) + encodeCache.RLock() + assert.Empty(t, encodeCache.entries) + encodeCache.RUnlock() +} + +// The cache is emptied when it fills, so it stays bounded. +func TestEncodeValue_Bounded(t *testing.T) { + ClearEncodeCache() + for i := 0; i <= encodeCacheLimit; i++ { + var n yaml.Node + require.NoError(t, encodeValue(&n, []string{fmt.Sprint(i)})) + } + encodeCache.RLock() + assert.Len(t, encodeCache.entries, 1) + encodeCache.RUnlock() + + ClearEncodeCache() + encodeCache.RLock() + assert.Empty(t, encodeCache.entries) + encodeCache.RUnlock() +} + +// Rendering encodes copies of model nodes, so the model is never mutated by a render. +func TestNodeBuilder_RenderLeavesNodesUnchanged(t *testing.T) { + type test struct { + Enum []*yaml.Node `yaml:"enum,omitempty"` + Example *yaml.Node `yaml:"example,omitempty"` + } + enum := []*yaml.Node{utils.CreateStringNode("a"), utils.CreateStringNode("b")} + example := &yaml.Node{Kind: yaml.MappingNode, Tag: "!!map", + Content: []*yaml.Node{utils.CreateStringNode("k"), utils.CreateStringNode("v")}} + for pass := 0; pass < 2; pass++ { + _, err := yaml.Marshal(NewNodeBuilder(&test{Enum: enum, Example: example}, nil).Render()) + require.NoError(t, err) + for _, n := range append(enum, example.Content...) { + assert.Equal(t, "!!str", n.Tag) + assert.Equal(t, yaml.Style(0), n.Style) + } + assert.Equal(t, "!!map", example.Tag) + } +} diff --git a/datamodel/high/node_builder.go b/datamodel/high/node_builder.go index 4d22e0a60..90ec9b6fa 100644 --- a/datamodel/high/node_builder.go +++ b/datamodel/high/node_builder.go @@ -11,6 +11,7 @@ import ( "sort" "strconv" "strings" + "sync" "unicode" "github.com/pb33f/libopenapi/datamodel/high/nodes" @@ -67,6 +68,103 @@ func originalFloatLexeme(value float64, lowValue any) (string, bool) { return valueNode.Value, true } +// nodeBuilderField holds the reflection metadata NewNodeBuilder needs for one field of a high-level struct. +// It depends only on the high and low struct types, so it is derived once per type pair and cached rather +// than re-derived (field lookups by name, yaml tag parsing) for every object rendered. +type nodeBuilderField struct { + index int // index of the field in the high-level struct + name string // field name, used as the NodeEntry key + extensions bool // the Extensions field, which renders its map entries rather than itself + tagName string // yaml tag name + renderZero bool + omitEmpty bool + + // lowIndex is the index path of the same-named field in the low-level struct, nil when it has none. + lowIndex []int + + // lowEmptier and lowValueNoder report whether the low field's value type (the element type for a + // pointer field) implements IsEmpty and GetValueNode. When it does, those methods are called through + // a pointer to the field instead of copying the field into an interface: the pointer's method set + // carries the value-receiver methods, so the result is identical. lowDynamic marks a value type that + // is itself an interface or pointer, whose methods depend on the value held at runtime. + lowEmptier bool + lowValueNoder bool + lowDynamic bool +} + +type nodeBuilderTypes struct { + high reflect.Type + low reflect.Type +} + +type lowEmptier interface{ IsEmpty() bool } + +type lowValueNoder interface{ GetValueNode() *yaml.Node } + +var ( + nodeBuilderFieldCache sync.Map // nodeBuilderTypes -> []nodeBuilderField + + lowEmptierType = reflect.TypeFor[lowEmptier]() + lowValueNoderType = reflect.TypeFor[lowValueNoder]() + hasKeyNodeType = reflect.TypeFor[low.HasKeyNode]() + stringType = reflect.TypeFor[string]() +) + +// nodeBuilderFields returns the cached field metadata for a high-level struct type, paired with the +// low-level struct type (nil when there is no low-level model). +func nodeBuilderFields(highType, lowType reflect.Type) []nodeBuilderField { + key := nodeBuilderTypes{high: highType, low: lowType} + if cached, ok := nodeBuilderFieldCache.Load(key); ok { + return cached.([]nodeBuilderField) + } + fields := make([]nodeBuilderField, 0, highType.NumField()) + for i := 0; i < highType.NumField(); i++ { + sf := highType.Field(i) + // only operate on exported fields. + if unicode.IsLower(rune(sf.Name[0])) { + continue + } + field := nodeBuilderField{index: i, name: sf.Name} + if lowType != nil { + if lsf, ok := lowType.FieldByName(sf.Name); ok { + field.lowIndex = lsf.Index + valueType := lsf.Type + if valueType.Kind() == reflect.Ptr { + valueType = valueType.Elem() + } + if valueType.Kind() == reflect.Interface || valueType.Kind() == reflect.Ptr { + field.lowDynamic = true + } else { + field.lowEmptier = valueType.Implements(lowEmptierType) + field.lowValueNoder = valueType.Implements(lowValueNoderType) + } + } + } + if sf.Name == "Extensions" { + field.extensions = true + fields = append(fields, field) + continue + } + tag := sf.Tag.Get("yaml") + if tag == "-" { + continue + } + tagParts := strings.Split(tag, ",") + field.tagName = tagParts[0] + for _, part := range tagParts { + if part == renderZero { + field.renderZero = true + } + if part == "omitempty" { + field.omitEmpty = true + } + } + fields = append(fields, field) + } + actual, _ := nodeBuilderFieldCache.LoadOrStore(key, fields) + return actual.([]nodeBuilderField) +} + // NewNodeBuilder will create a new NodeBuilder instance, this is the only way to create a NodeBuilder. // The function accepts a high level object and a low level object (need to be siblings/same type). // @@ -79,58 +177,122 @@ func NewNodeBuilder(high any, low any) *NodeBuilder { nb.Low = low } + // resolve the low-level struct once; its fields supply line numbers and original styles. + var lowStruct reflect.Value + var lowType reflect.Type + if low != nil { + if lv := reflect.ValueOf(low); !lv.IsZero() { + if lv.Kind() == reflect.Ptr { + lowStruct = lv.Elem() + } else { + lowStruct = lv + } + lowType = lowStruct.Type() + } + } + // extract fields from the high level object and add them into our node builder. // this will allow us to extract the line numbers from the low level object as well. - v := reflect.ValueOf(high).Elem() - num := v.NumField() - for i := 0; i < num; i++ { - nb.add(v.Type().Field(i).Name, i) + highStruct := reflect.ValueOf(high).Elem() + fields := nodeBuilderFields(highStruct.Type(), lowType) + for i := range fields { + nb.add(&fields[i], highStruct, lowStruct) } return nb } -func (n *NodeBuilder) add(key string, i int) { - // only operate on exported fields. - if unicode.IsLower(rune(key[0])) { - return +// lowFieldHasContent reports whether a low-level field holds content, which keeps a zero high-level value +// in the rendered output. +func lowFieldHasContent(field *nodeBuilderField, lowFieldValue reflect.Value) bool { + if field.lowDynamic { + return dynamicLowFieldHasContent(lowFieldValue) + } + if !field.lowEmptier && !field.lowValueNoder { + return false + } + holder := lowFieldValue + if holder.Kind() == reflect.Ptr { + if holder.IsNil() { + return false + } + } else if holder.CanAddr() { + holder = holder.Addr() + } + h := holder.Interface() + if field.lowEmptier && !h.(lowEmptier).IsEmpty() { + return true + } + return field.lowValueNoder && h.(lowValueNoder).GetValueNode() != nil +} + +// dynamicLowFieldHasContent is lowFieldHasContent for a field whose methods depend on the value it holds. +func dynamicLowFieldHasContent(lowFieldValue reflect.Value) bool { + var lowInterface any + if lowFieldValue.Kind() == reflect.Ptr { + if lowFieldValue.IsNil() { + return false + } + lowInterface = lowFieldValue.Elem().Interface() + } else { + lowInterface = lowFieldValue.Interface() + } + if emptier, ok := lowInterface.(lowEmptier); ok && !emptier.IsEmpty() { + return true } + if nodeGetter, ok := lowInterface.(lowValueNoder); ok { + return nodeGetter.GetValueNode() != nil + } + return false +} +// lowestKeyLine returns the lowest key line of the items in a low-level slice, where items without a key +// node count as line zero. +func lowestKeyLine(value reflect.Value) int { + elemType := value.Type().Elem() + dynamic := elemType.Kind() == reflect.Interface + if !dynamic && !elemType.Implements(hasKeyNodeType) { + // no item can have a key node, so every item counts as line zero. + return 0 + } + lowest := 0 + for g := 0; g < value.Len(); g++ { + item := value.Index(g) + if !dynamic && item.Kind() != reflect.Ptr { + item = item.Addr() // call through a pointer rather than copying the item + } + line := 0 + if we, ok := item.Interface().(low.HasKeyNode); ok { + line = we.GetKeyNode().Line + } + if g == 0 || line < lowest { + lowest = line + } + } + return lowest +} + +func (n *NodeBuilder) add(field *nodeBuilderField, highStruct, lowStruct reflect.Value) { var ( lowFieldValue reflect.Value lowFieldValid bool ) - if n.Low != nil && !reflect.ValueOf(n.Low).IsZero() { - low := reflect.ValueOf(n.Low) - if low.Kind() == reflect.Ptr && !low.IsNil() { - elem := low.Elem() - if elem.IsValid() { - field := elem.FieldByName(key) - if field.IsValid() { - lowFieldValue = field - lowFieldValid = true - } - } - } else if low.IsValid() { - field := low.FieldByName(key) - if field.IsValid() { - lowFieldValue = field - lowFieldValid = true - } - } + if lowStruct.IsValid() && field.lowIndex != nil { + lowFieldValue = lowStruct.FieldByIndex(field.lowIndex) + lowFieldValid = true } // if the key is 'Extensions' then we need to extract the keys from the map // and add them to the node builder. - if key == "Extensions" { - ev := reflect.ValueOf(n.High).Elem().FieldByName(key).Interface() + if field.extensions { + ev := highStruct.Field(field.index).Interface() var extensions *orderedmap.Map[string, *yaml.Node] if ev != nil { extensions = ev.(*orderedmap.Map[string, *yaml.Node]) } var lowExtensions *orderedmap.Map[low.KeyReference[string], low.ValueReference[*yaml.Node]] - if n.Low != nil && !reflect.ValueOf(n.Low).IsZero() { + if lowStruct.IsValid() { if j, ok := n.Low.(low.HasExtensionsUntyped); ok { lowExtensions = j.GetExtensions() } @@ -160,28 +322,11 @@ func (n *NodeBuilder) add(key string, i int) { return } - // find the field with the tag supplied. - field, _ := reflect.TypeOf(n.High).Elem().FieldByName(key) - tag := string(field.Tag.Get("yaml")) - tagName := strings.Split(tag, ",")[0] - - if tag == "-" { - return - } - - var renderZeroFlag, omitEmptyFlag bool - tagParts := strings.Split(tag, ",") - for _, part := range tagParts { - if part == renderZero { - renderZeroFlag = true - } - if part == "omitempty" { - omitEmptyFlag = true - } - } + tagName := field.tagName + renderZeroFlag, omitEmptyFlag := field.renderZero, field.omitEmpty // extract the value of the field - fieldValue := reflect.ValueOf(n.High).Elem().FieldByName(key) + fieldValue := highStruct.Field(field.index) f := fieldValue.Interface() value := reflect.ValueOf(f) var isZero bool @@ -199,24 +344,8 @@ func (n *NodeBuilder) add(key string, i int) { } } - if isZero && lowFieldValid { - var lowInterface any - if lowFieldValue.Kind() == reflect.Ptr { - if !lowFieldValue.IsNil() { - lowInterface = lowFieldValue.Elem().Interface() - } - } else { - lowInterface = lowFieldValue.Interface() - } - if lowInterface != nil { - if emptier, ok := lowInterface.(interface{ IsEmpty() bool }); ok && !emptier.IsEmpty() { - isZero = false - } else if nodeGetter, ok := lowInterface.(interface{ GetValueNode() *yaml.Node }); ok { - if node := nodeGetter.GetValueNode(); node != nil { - isZero = false - } - } - } + if isZero && lowFieldValid && lowFieldHasContent(field, lowFieldValue) { + isZero = false } if !renderZeroFlag && isZero || omitEmptyFlag && isZero { @@ -224,7 +353,7 @@ func (n *NodeBuilder) add(key string, i int) { } // create a new node entry - nodeEntry := &nodes.NodeEntry{Tag: tagName, Key: key} + nodeEntry := &nodes.NodeEntry{Tag: tagName, Key: field.name} nodeEntry.RenderZero = renderZeroFlag switch value.Kind() { case reflect.Float64, reflect.Float32: @@ -235,7 +364,11 @@ func (n *NodeBuilder) add(key string, i int) { nodeEntry.Value = value.Int() nodeEntry.StringValue = value.String() case reflect.String: - nodeEntry.Value = value.String() + if value.Type() == stringType { + nodeEntry.Value = f // already boxed as a plain string + } else { + nodeEntry.Value = value.String() + } case reflect.Bool: nodeEntry.Value = value.Bool() case reflect.Slice: @@ -261,48 +394,32 @@ func (n *NodeBuilder) add(key string, i int) { // if there is no low-level object, then we cannot extract line numbers, // so skip and default to 0, which means a new entry to the spec. // this will place new content and the top of the rendered object. - if n.Low != nil && !reflect.ValueOf(n.Low).IsZero() { - if lowFieldValid { - fLow := lowFieldValue.Interface() - value = reflect.ValueOf(fLow) - - nodeEntry.LowValue = fLow - switch value.Kind() { - - case reflect.Slice: - l := value.Len() - lines := make([]int, l) - for g := 0; g < l; g++ { - qw := value.Index(g).Interface() - if we, wok := qw.(low.HasKeyNode); wok { - lines[g] = we.GetKeyNode().Line + if lowFieldValid { + fLow := lowFieldValue.Interface() + value = reflect.ValueOf(fLow) + + nodeEntry.LowValue = fLow + switch value.Kind() { + + case reflect.Slice: + nodeEntry.Line = lowestKeyLine(value) + case reflect.Struct: + nodeEntry.Line = 9999 + field.index + if nb, ok := fLow.(low.HasValueNodeUntyped); ok { + if nb.IsReference() { + if jk, kj := fLow.(low.HasKeyNode); kj { + nodeEntry.Line = jk.GetKeyNode().Line + break } } - sort.Slice(lines, func(i, j int) bool { - return lines[i] < lines[j] - }) - if len(lines) > 0 { - nodeEntry.Line = lines[0] + if nb.GetValueNode() != nil { + nodeEntry.Line = nb.GetValueNode().Line } - case reflect.Struct: - y := value.Interface() - nodeEntry.Line = 9999 + i - if nb, ok := y.(low.HasValueNodeUntyped); ok { - if nb.IsReference() { - if jk, kj := y.(low.HasKeyNode); kj { - nodeEntry.Line = jk.GetKeyNode().Line - break - } - } - if nb.GetValueNode() != nil { - nodeEntry.Line = nb.GetValueNode().Line - } - } - default: - // everything else, weight it to the bottom of the rendered object. - // this is things that we have no way of knowing where they should be placed. - nodeEntry.Line = 9999 + i } + default: + // everything else, weight it to the bottom of the rendered object. + // this is things that we have no way of knowing where they should be placed. + nodeEntry.Line = 9999 + field.index } } if nodeEntry.Value != nil { @@ -350,14 +467,21 @@ func (n *NodeBuilder) Render() *yaml.Node { } // encodeSafeValue returns a value safe to pass to (*yaml.Node).Encode. When the -// value is a *yaml.Node it returns a deep copy: Encode desolves the represented -// graph in place (Desolve rewrites Tag/Style), and the representer aliases input -// nodes, so encoding a model-owned node would mutate it. With concurrent renders -// (e.g. linters running rules in parallel) that mutation races with readers of +// value is a *yaml.Node, or a slice of them, it returns a deep copy: Encode desolves +// the represented graph in place (Desolve rewrites Tag/Style), and the representer +// aliases input nodes, so encoding a model-owned node would mutate it. With concurrent +// renders (e.g. linters running rules in parallel) that mutation races with readers of // the same node. Encoding a copy keeps shared nodes immutable. func encodeSafeValue(value any) any { - if vn, ok := value.(*yaml.Node); ok { - return utils.CloneYAMLNode(vn) + switch v := value.(type) { + case *yaml.Node: + return utils.CloneYAMLNode(v) + case []*yaml.Node: + cloned := make([]*yaml.Node, len(v)) + for i, n := range v { + cloned[i] = utils.CloneYAMLNode(n) + } + return cloned } return value } @@ -504,7 +628,7 @@ func (n *NodeBuilder) AddYAMLNode(parent *yaml.Node, entry *nodes.NodeEntry) *ya break } - if err := rawNode.Encode(encodeSafeValue(value)); err != nil { + if err := encodeValue(&rawNode, value); err != nil { // an item that failed to render has already reported why, and the encoder only echoes it. if errors.Join(nodeErrors...) == nil { nodeErrors = append(nodeErrors, err) @@ -651,7 +775,7 @@ func (n *NodeBuilder) AddYAMLNode(parent *yaml.Node, entry *nodes.NodeEntry) *ya } } - if err := rawNode.Encode(encodeSafeValue(value)); err != nil { + if err := encodeValue(&rawNode, value); err != nil { nodeErrors = append(nodeErrors, err) } else { valueNode = &rawNode diff --git a/datamodel/high/node_builder_test.go b/datamodel/high/node_builder_test.go index f55c91043..a1f4a0f8d 100644 --- a/datamodel/high/node_builder_test.go +++ b/datamodel/high/node_builder_test.go @@ -1837,3 +1837,62 @@ func TestNewNodeBuilder_FalseBoolPointerOmitted(t *testing.T) { assert.Empty(t, nb.Errors) assert.Empty(t, parent.Content) } + +type emptyWithValueNode struct{} + +func (emptyWithValueNode) IsEmpty() bool { return true } +func (emptyWithValueNode) GetValueNode() *yaml.Node { return utils.CreateStringNode("held") } + +type emptyWithoutValueNode struct{} + +func (emptyWithoutValueNode) IsEmpty() bool { return true } +func (emptyWithoutValueNode) GetValueNode() *yaml.Node { return nil } + +type dynamicLowHigh struct { + Held string `yaml:"held,omitempty"` +} + +type dynamicLowAny struct { + Held any +} + +type dynamicLowPointer struct { + Held *any +} + +// An interface-typed low field (or a pointer to one) can only be checked for content through the value +// it holds at runtime, so a zero high value renders exactly when that value reports content. +func TestNewNodeBuilder_DynamicLowFieldContent(t *testing.T) { + render := func(low any) string { + data, err := yaml.Marshal(NewNodeBuilder(&dynamicLowHigh{}, low).Render()) + require.NoError(t, err) + return strings.TrimSpace(string(data)) + } + held := func(v any) *any { return &v } + + assert.Equal(t, "{}", render(&dynamicLowAny{})) + assert.Equal(t, `held: ""`, render(&dynamicLowAny{Held: nonEmptyExample{}})) + assert.Equal(t, `held: ""`, render(&dynamicLowAny{Held: emptyWithValueNode{}})) + assert.Equal(t, "{}", render(&dynamicLowAny{Held: emptyWithoutValueNode{}})) + assert.Equal(t, "{}", render(&dynamicLowAny{Held: struct{}{}})) + + assert.Equal(t, "{}", render(&dynamicLowPointer{})) + assert.Equal(t, `held: ""`, render(&dynamicLowPointer{Held: held(nonEmptyExample{})})) + assert.Equal(t, "{}", render(&dynamicLowPointer{Held: held(struct{}{})})) +} + +type namedString string + +// A field with a named string type renders its value as a plain string. +func TestNewNodeBuilder_NamedStringType(t *testing.T) { + type test struct { + Name namedString `yaml:"name"` + } + nb := NewNodeBuilder(&test{Name: "chicken"}, nil) + require.Len(t, nb.Nodes, 1) + assert.Equal(t, "chicken", nb.Nodes[0].Value) + + data, err := yaml.Marshal(nb.Render()) + require.NoError(t, err) + assert.Equal(t, "name: chicken", strings.TrimSpace(string(data))) +} diff --git a/datamodel/low/base/schema_proxy.go b/datamodel/low/base/schema_proxy.go index be246c3a7..3b8a54548 100644 --- a/datamodel/low/base/schema_proxy.go +++ b/datamodel/low/base/schema_proxy.go @@ -230,8 +230,10 @@ func (sp *SchemaProxy) Schema() *Schema { // handle property merging for references with sibling properties buildNode := sp.vn if cfg != nil { - if docConfig := sp.getDocumentConfig(); docConfig != nil && docConfig.MergeReferencedProperties { - if mergedNode := sp.attemptPropertyMerging(buildNode, docConfig); mergedNode != nil { + // read the flag straight from the rolodex config: the document configuration is only built + // when merging is actually enabled. + if rolodexConfig := sp.rolodexConfig(); rolodexConfig != nil && rolodexConfig.MergeReferencedProperties { + if mergedNode := sp.attemptPropertyMerging(buildNode, rolodexConfig.ToDocumentConfiguration()); mergedNode != nil { buildNode = mergedNode } } @@ -418,16 +420,12 @@ type HasIndex interface { GetIndex() *index.SpecIndex } -// getDocumentConfig retrieves the document configuration from the index -func (sp *SchemaProxy) getDocumentConfig() *datamodel.DocumentConfiguration { +// rolodexConfig returns the index configuration of the rolodex this proxy's index belongs to, or nil. +func (sp *SchemaProxy) rolodexConfig() *index.SpecIndexConfig { if sp.idx == nil || sp.idx.GetRolodex() == nil { return nil } - rolodex := sp.idx.GetRolodex() - if config := rolodex.GetConfig(); config != nil { - return config.ToDocumentConfiguration() - } - return nil + return sp.idx.GetRolodex().GetConfig() } // attemptPropertyMerging attempts to merge properties for references with siblings diff --git a/datamodel/low/base/schema_proxy_test.go b/datamodel/low/base/schema_proxy_test.go index 0156f04a2..9091fcec3 100644 --- a/datamodel/low/base/schema_proxy_test.go +++ b/datamodel/low/base/schema_proxy_test.go @@ -648,7 +648,7 @@ components: assert.NotNil(t, schema) sp.idx.SetRolodex(&index.Rolodex{}) // set a rolodex to avoid nil deref - assert.Nil(t, sp.getDocumentConfig()) + assert.Nil(t, sp.rolodexConfig()) } func TestSchemaProxy_attemptPropertyMerging_MergeError(t *testing.T) { diff --git a/datamodel/low/model_builder.go b/datamodel/low/model_builder.go index 7b721b8f7..46b4ea32e 100644 --- a/datamodel/low/model_builder.go +++ b/datamodel/low/model_builder.go @@ -113,6 +113,32 @@ func BuildModel(node *yaml.Node, model interface{}) error { return nil } +// setField*Type are the field types SetField switches on, computed once rather than on every call. +var ( + setFieldMapStringNodeReferenceYAMLNodeType = reflect.TypeFor[*orderedmap.Map[string, NodeReference[*yaml.Node]]]() + setFieldMapStringNodeReferenceStringType = reflect.TypeFor[*orderedmap.Map[string, NodeReference[string]]]() + setFieldNodeReferenceYAMLNodeType = reflect.TypeFor[NodeReference[*yaml.Node]]() + setFieldSliceOfNodeReferenceYAMLNodeType = reflect.TypeFor[[]NodeReference[*yaml.Node]]() + setFieldNodeReferenceStringType = reflect.TypeFor[NodeReference[string]]() + setFieldValueReferenceStringType = reflect.TypeFor[ValueReference[string]]() + setFieldNodeReferenceBoolType = reflect.TypeFor[NodeReference[bool]]() + setFieldNodeReferenceIntType = reflect.TypeFor[NodeReference[int]]() + setFieldNodeReferenceInt64Type = reflect.TypeFor[NodeReference[int64]]() + setFieldNodeReferenceFloat32Type = reflect.TypeFor[NodeReference[float32]]() + setFieldNodeReferenceFloat64Type = reflect.TypeFor[NodeReference[float64]]() + setFieldSliceOfNodeReferenceStringType = reflect.TypeFor[[]NodeReference[string]]() + setFieldSliceOfNodeReferenceFloat32Type = reflect.TypeFor[[]NodeReference[float32]]() + setFieldSliceOfNodeReferenceFloat64Type = reflect.TypeFor[[]NodeReference[float64]]() + setFieldSliceOfNodeReferenceIntType = reflect.TypeFor[[]NodeReference[int]]() + setFieldSliceOfNodeReferenceInt64Type = reflect.TypeFor[[]NodeReference[int64]]() + setFieldSliceOfNodeReferenceBoolType = reflect.TypeFor[[]NodeReference[bool]]() + setFieldMapKeyReferenceStringValueReferenceStringType = reflect.TypeFor[*orderedmap.Map[KeyReference[string], ValueReference[string]]]() + setFieldKeyReferenceMapKeyReferenceStringValueReferenceStringType = reflect.TypeFor[KeyReference[*orderedmap.Map[KeyReference[string], ValueReference[string]]]]() + setFieldNodeReferenceMapKeyReferenceStringValueReferenceStringType = reflect.TypeFor[NodeReference[*orderedmap.Map[KeyReference[string], ValueReference[string]]]]() + setFieldNodeReferenceSliceValueReferenceStringType = reflect.TypeFor[NodeReference[[]ValueReference[string]]]() + setFieldNodeReferenceSliceValueReferenceYAMLNodeType = reflect.TypeFor[NodeReference[[]ValueReference[*yaml.Node]]]() +) + // SetField accepts a field reflection value, a yaml.Node valueNode and a yaml.Node keyNode. Using reflection, the // function will attempt to set the value of the field based on the key and value nodes. This method is only useful // for low-level models, it has no value to high-level ones. @@ -123,7 +149,7 @@ func SetField(field *reflect.Value, valueNode *yaml.Node, keyNode *yaml.Node) { switch field.Type() { - case reflect.TypeOf(orderedmap.New[string, NodeReference[*yaml.Node]]()): + case setFieldMapStringNodeReferenceYAMLNodeType: if utils.IsNodeMap(valueNode) { if field.CanSet() { @@ -144,7 +170,7 @@ func SetField(field *reflect.Value, valueNode *yaml.Node, keyNode *yaml.Node) { } } - case reflect.TypeOf(orderedmap.New[string, NodeReference[string]]()): + case setFieldMapStringNodeReferenceStringType: if utils.IsNodeMap(valueNode) { if field.CanSet() { @@ -165,14 +191,14 @@ func SetField(field *reflect.Value, valueNode *yaml.Node, keyNode *yaml.Node) { } } - case reflect.TypeOf(NodeReference[*yaml.Node]{}): + case setFieldNodeReferenceYAMLNodeType: if field.CanSet() { or := NodeReference[*yaml.Node]{Value: valueNode, ValueNode: valueNode, KeyNode: keyNode} field.Set(reflect.ValueOf(or)) } - case reflect.TypeOf([]NodeReference[*yaml.Node]{}): + case setFieldSliceOfNodeReferenceYAMLNodeType: if utils.IsNodeArray(valueNode) { if field.CanSet() { @@ -188,7 +214,7 @@ func SetField(field *reflect.Value, valueNode *yaml.Node, keyNode *yaml.Node) { } } - case reflect.TypeOf(NodeReference[string]{}): + case setFieldNodeReferenceStringType: if field.CanSet() { nr := NodeReference[string]{ @@ -199,7 +225,7 @@ func SetField(field *reflect.Value, valueNode *yaml.Node, keyNode *yaml.Node) { field.Set(reflect.ValueOf(nr)) } - case reflect.TypeOf(ValueReference[string]{}): + case setFieldValueReferenceStringType: if field.CanSet() { nr := ValueReference[string]{ @@ -209,7 +235,7 @@ func SetField(field *reflect.Value, valueNode *yaml.Node, keyNode *yaml.Node) { field.Set(reflect.ValueOf(nr)) } - case reflect.TypeOf(NodeReference[bool]{}): + case setFieldNodeReferenceBoolType: if utils.IsNodeBoolValue(valueNode) { if field.CanSet() { @@ -223,7 +249,7 @@ func SetField(field *reflect.Value, valueNode *yaml.Node, keyNode *yaml.Node) { } } - case reflect.TypeOf(NodeReference[int]{}): + case setFieldNodeReferenceIntType: if utils.IsNodeIntValue(valueNode) { if field.CanSet() { @@ -237,7 +263,7 @@ func SetField(field *reflect.Value, valueNode *yaml.Node, keyNode *yaml.Node) { } } - case reflect.TypeOf(NodeReference[int64]{}): + case setFieldNodeReferenceInt64Type: if utils.IsNodeIntValue(valueNode) || utils.IsNodeFloatValue(valueNode) { if field.CanSet() { @@ -251,7 +277,7 @@ func SetField(field *reflect.Value, valueNode *yaml.Node, keyNode *yaml.Node) { } } - case reflect.TypeOf(NodeReference[float32]{}): + case setFieldNodeReferenceFloat32Type: if utils.IsNodeNumberValue(valueNode) { if field.CanSet() { @@ -265,7 +291,7 @@ func SetField(field *reflect.Value, valueNode *yaml.Node, keyNode *yaml.Node) { } } - case reflect.TypeOf(NodeReference[float64]{}): + case setFieldNodeReferenceFloat64Type: if utils.IsNodeNumberValue(valueNode) { if field.CanSet() { @@ -279,7 +305,7 @@ func SetField(field *reflect.Value, valueNode *yaml.Node, keyNode *yaml.Node) { } } - case reflect.TypeOf([]NodeReference[string]{}): + case setFieldSliceOfNodeReferenceStringType: if utils.IsNodeArray(valueNode) { if field.CanSet() { @@ -295,7 +321,7 @@ func SetField(field *reflect.Value, valueNode *yaml.Node, keyNode *yaml.Node) { } } - case reflect.TypeOf([]NodeReference[float32]{}): + case setFieldSliceOfNodeReferenceFloat32Type: if utils.IsNodeArray(valueNode) { if field.CanSet() { @@ -312,7 +338,7 @@ func SetField(field *reflect.Value, valueNode *yaml.Node, keyNode *yaml.Node) { } } - case reflect.TypeOf([]NodeReference[float64]{}): + case setFieldSliceOfNodeReferenceFloat64Type: if utils.IsNodeArray(valueNode) { if field.CanSet() { @@ -325,7 +351,7 @@ func SetField(field *reflect.Value, valueNode *yaml.Node, keyNode *yaml.Node) { } } - case reflect.TypeOf([]NodeReference[int]{}): + case setFieldSliceOfNodeReferenceIntType: if utils.IsNodeArray(valueNode) { if field.CanSet() { @@ -342,7 +368,7 @@ func SetField(field *reflect.Value, valueNode *yaml.Node, keyNode *yaml.Node) { } } - case reflect.TypeOf([]NodeReference[int64]{}): + case setFieldSliceOfNodeReferenceInt64Type: if utils.IsNodeArray(valueNode) { if field.CanSet() { @@ -359,7 +385,7 @@ func SetField(field *reflect.Value, valueNode *yaml.Node, keyNode *yaml.Node) { } } - case reflect.TypeOf([]NodeReference[bool]{}): + case setFieldSliceOfNodeReferenceBoolType: if utils.IsNodeArray(valueNode) { if field.CanSet() { @@ -377,7 +403,7 @@ func SetField(field *reflect.Value, valueNode *yaml.Node, keyNode *yaml.Node) { } // helper for unpacking string maps. - case reflect.TypeOf(orderedmap.New[KeyReference[string], ValueReference[string]]()): + case setFieldMapKeyReferenceStringValueReferenceStringType: if utils.IsNodeMap(valueNode) { if field.CanSet() { @@ -397,7 +423,7 @@ func SetField(field *reflect.Value, valueNode *yaml.Node, keyNode *yaml.Node) { } } - case reflect.TypeOf(KeyReference[*orderedmap.Map[KeyReference[string], ValueReference[string]]]{}): + case setFieldKeyReferenceMapKeyReferenceStringValueReferenceStringType: if utils.IsNodeMap(valueNode) { if field.CanSet() { @@ -420,7 +446,7 @@ func SetField(field *reflect.Value, valueNode *yaml.Node, keyNode *yaml.Node) { field.Set(reflect.ValueOf(ref)) } } - case reflect.TypeOf(NodeReference[*orderedmap.Map[KeyReference[string], ValueReference[string]]]{}): + case setFieldNodeReferenceMapKeyReferenceStringValueReferenceStringType: if utils.IsNodeMap(valueNode) { if field.CanSet() { items := orderedmap.New[KeyReference[string], ValueReference[string]]() @@ -443,7 +469,7 @@ func SetField(field *reflect.Value, valueNode *yaml.Node, keyNode *yaml.Node) { field.Set(reflect.ValueOf(ref)) } } - case reflect.TypeOf(NodeReference[[]ValueReference[string]]{}): + case setFieldNodeReferenceSliceValueReferenceStringType: if utils.IsNodeArray(valueNode) { if field.CanSet() { @@ -463,7 +489,7 @@ func SetField(field *reflect.Value, valueNode *yaml.Node, keyNode *yaml.Node) { } } - case reflect.TypeOf(NodeReference[[]ValueReference[*yaml.Node]]{}): + case setFieldNodeReferenceSliceValueReferenceYAMLNodeType: if utils.IsNodeArray(valueNode) { if field.CanSet() { diff --git a/datamodel/spec_info.go b/datamodel/spec_info.go index ab947fd04..a68667731 100644 --- a/datamodel/spec_info.go +++ b/datamodel/spec_info.go @@ -14,6 +14,7 @@ import ( "unicode/utf16" "unicode/utf8" + "github.com/pb33f/libopenapi/internal/jsonnode" "github.com/pb33f/libopenapi/utils" "go.yaml.in/yaml/v4" ) @@ -188,7 +189,7 @@ func extractSpecInfoInternal(spec []byte, bypass bool, skipJSON bool) (*SpecInfo parseBytes = normalizeJSONForYAMLParser(spec) } - err := yaml.Unmarshal(parseBytes, &parsedSpec) + err := jsonnode.Unmarshal(parseBytes, &parsedSpec) if err != nil { if !bypass { return nil, fmt.Errorf("unable to parse specification: %s", err.Error()) diff --git a/index/rolodex_file_loader.go b/index/rolodex_file_loader.go index 3a2547ec7..1fea52bdf 100644 --- a/index/rolodex_file_loader.go +++ b/index/rolodex_file_loader.go @@ -19,6 +19,7 @@ import ( "context" "github.com/pb33f/libopenapi/datamodel" + "github.com/pb33f/libopenapi/internal/jsonnode" "github.com/pb33f/libopenapi/utils" "go.yaml.in/yaml/v4" ) @@ -293,7 +294,7 @@ func (l *LocalFile) GetContentAsYAMLNode() (*yaml.Node, error) { return nil, fmt.Errorf("no data to parse for file: %s", l.fullPath) } var root yaml.Node - err := yaml.Unmarshal(l.data, &root) + err := jsonnode.Unmarshal(l.data, &root) if err != nil { // we can't parse it, so create a fake document node with a single string content root = yaml.Node{ diff --git a/index/rolodex_remote_loader.go b/index/rolodex_remote_loader.go index 7f9b01447..818997a0b 100644 --- a/index/rolodex_remote_loader.go +++ b/index/rolodex_remote_loader.go @@ -21,6 +21,7 @@ import ( "time" "github.com/pb33f/libopenapi/datamodel" + "github.com/pb33f/libopenapi/internal/jsonnode" "github.com/pb33f/libopenapi/utils" "go.yaml.in/yaml/v4" @@ -307,7 +308,7 @@ func (f *RemoteFile) GetContentAsYAMLNode() (*yaml.Node, error) { return nil, fmt.Errorf("no data to parse for file: %s", f.fullPath) } var root yaml.Node - err := yaml.Unmarshal(f.data, &root) + err := jsonnode.Unmarshal(f.data, &root) if err != nil { return nil, err diff --git a/internal/jsonnode/jsonnode.go b/internal/jsonnode/jsonnode.go new file mode 100644 index 000000000..0fc56024c --- /dev/null +++ b/internal/jsonnode/jsonnode.go @@ -0,0 +1,441 @@ +// Copyright 2026 Princess B33f Heavy Industries / Dave Shanley +// SPDX-License-Identifier: MIT + +// Package jsonnode builds the yaml.Node tree for a JSON document directly, without running yaml's scanner. +// +// yaml parses JSON as a flow collection, and its scanner treats the opening '{' as a potential simple key, +// so it buffers every token of the document until the closing '}'. For a large JSON specification that is +// hundreds of megabytes of token queue. JSON's grammar is small enough to parse directly into the exact +// tree yaml would produce: the same kinds, tags, styles, values, lines and columns. +// +// Parse only accepts input whose yaml parse it reproduces exactly. Anything else (invalid JSON, or valid +// JSON that yaml reads differently, such as a key separated from its colon by a line break) is declined, +// and the caller parses it with yaml instead, getting yaml's result or yaml's error. +package jsonnode + +import ( + "regexp" + "strconv" + "unicode/utf8" + + "go.yaml.in/yaml/v4" +) + +const ( + // maxDepth bounds nesting well inside yaml's own depth limit. + maxDepth = 1000 + + // internLimit is the longest string interned. Keys and short values repeat heavily across a + // specification; long text rarely does and is not worth hashing. + internLimit = 64 + + nodeSlabSize = 512 + contentSlabSize = 2048 +) + +// yamlStyleFloat is the pattern yaml's resolver requires of a plain scalar before resolving it as a float. +var yamlStyleFloat = regexp.MustCompile(`^[-+]?(?:\.[0-9]+|[0-9]+(?:\.[0-9]*)?)(?:[eE][-+]?[0-9]+)?$`) + +type parser struct { + data []byte + pos int + line int // 1-based line of data[pos] + col int // 1-based column of data[pos], counted in characters as yaml counts them + + depth int + nodes []yaml.Node // slab the tree's nodes are allocated from + content []*yaml.Node // slab the collections' Content slices are carved from + stack []*yaml.Node // children of the collections still open + strs map[string]string + buf []byte // scratch space for decoding escaped strings +} + +// Unmarshal decodes data into out, producing exactly what yaml.Unmarshal produces: a JSON object is built +// by Parse, and anything Parse declines is decoded by yaml.Unmarshal. +func Unmarshal(data []byte, out *yaml.Node) error { + if doc, ok := Parse(data); ok { + *out = *doc + return nil + } + return yaml.Unmarshal(data, out) +} + +// Parse returns the yaml document node yaml.Unmarshal produces for the JSON document in data, and true. +// It returns false when data is not a JSON object whose yaml parse it reproduces exactly. +func Parse(data []byte) (*yaml.Node, bool) { + p := parser{data: data, line: 1, col: 1} + if !p.skipSpace(false) || p.pos >= len(data) || data[p.pos] != '{' { + return nil, false + } + p.strs = make(map[string]string, 256) + doc := &yaml.Node{Kind: yaml.DocumentNode, Line: p.line, Column: p.col} + root, ok := p.parseValue() + if !ok || !p.skipSpace(false) || p.pos != len(data) { + return nil, false + } + doc.Content = []*yaml.Node{root} + return doc, true +} + +func (p *parser) newNode(kind yaml.Kind, style yaml.Style, tag string) *yaml.Node { + if len(p.nodes) == cap(p.nodes) { + p.nodes = make([]yaml.Node, 0, nodeSlabSize) + } + p.nodes = append(p.nodes, yaml.Node{Kind: kind, Style: style, Tag: tag, Line: p.line, Column: p.col}) + return &p.nodes[len(p.nodes)-1] +} + +// carve returns a copy of children in a slice from the content slab. Its capacity equals its length, so +// appending to a node's Content never writes into a neighbour's. +func (p *parser) carve(children []*yaml.Node) []*yaml.Node { + n := len(children) + if cap(p.content)-len(p.content) < n { + p.content = make([]*yaml.Node, 0, max(contentSlabSize, n)) + } + start := len(p.content) + p.content = append(p.content, children...) + return p.content[start : start+n : start+n] +} + +func (p *parser) intern(b []byte) string { + if len(b) > internLimit { + return string(b) + } + if s, ok := p.strs[string(b)]; ok { + return s + } + s := string(b) + p.strs[s] = s + return s +} + +// skipSpace consumes JSON whitespace. Outside any collection yaml does not accept a tab, so one declines. +func (p *parser) skipSpace(inCollection bool) bool { + for p.pos < len(p.data) { + switch p.data[p.pos] { + case ' ': + p.pos++ + p.col++ + case '\t': + if !inCollection { + return false + } + p.pos++ + p.col++ + case '\n': + p.pos++ + p.line++ + p.col = 1 + case '\r': + // yaml reads CR LF as one line break; a lone CR is declined. + if p.pos+1 >= len(p.data) || p.data[p.pos+1] != '\n' { + return false + } + p.pos += 2 + p.line++ + p.col = 1 + default: + return true + } + } + return true +} + +func (p *parser) parseValue() (*yaml.Node, bool) { + if p.pos >= len(p.data) { + return nil, false + } + switch c := p.data[p.pos]; { + case c == '{': + return p.parseCollection(yaml.MappingNode, "!!map", '}') + case c == '[': + return p.parseCollection(yaml.SequenceNode, "!!seq", ']') + case c == '"': + return p.parseString() + case c == 't': + return p.parseLiteral("true", "!!bool") + case c == 'f': + return p.parseLiteral("false", "!!bool") + case c == 'n': + return p.parseLiteral("null", "!!null") + case c == '-' || (c >= '0' && c <= '9'): + return p.parseNumber() + } + return nil, false +} + +func (p *parser) parseCollection(kind yaml.Kind, tag string, closer byte) (*yaml.Node, bool) { + p.depth++ + if p.depth > maxDepth { + return nil, false + } + node := p.newNode(kind, yaml.FlowStyle, tag) + p.pos++ + p.col++ + base := len(p.stack) + if !p.skipSpace(true) { + return nil, false + } + if p.pos < len(p.data) && p.data[p.pos] == closer { + p.pos++ + p.col++ + p.depth-- + return node, true + } + for { + if kind == yaml.MappingNode { + if p.pos >= len(p.data) || p.data[p.pos] != '"' { + return nil, false + } + keyLine := p.line + key, ok := p.parseString() + if !ok || !p.skipSpace(true) { + return nil, false + } + // yaml only pairs a key with a colon on the same line. + if p.pos >= len(p.data) || p.data[p.pos] != ':' || p.line != keyLine { + return nil, false + } + p.pos++ + p.col++ + if !p.skipSpace(true) { + return nil, false + } + p.stack = append(p.stack, key) + } + value, ok := p.parseValue() + if !ok || !p.skipSpace(true) || p.pos >= len(p.data) { + return nil, false + } + p.stack = append(p.stack, value) + switch p.data[p.pos] { + case ',': + p.pos++ + p.col++ + if !p.skipSpace(true) { + return nil, false + } + continue + case closer: + p.pos++ + p.col++ + node.Content = p.carve(p.stack[base:]) + clear(p.stack[base:]) + p.stack = p.stack[:base] + p.depth-- + return node, true + } + return nil, false + } +} + +func (p *parser) parseLiteral(word, tag string) (*yaml.Node, bool) { + if len(p.data)-p.pos < len(word) || string(p.data[p.pos:p.pos+len(word)]) != word { + return nil, false + } + node := p.newNode(yaml.ScalarNode, 0, tag) + node.Value = word + p.pos += len(word) + p.col += len(word) + return node, true +} + +func (p *parser) parseNumber() (*yaml.Node, bool) { + start := p.pos + i := p.pos + if p.data[i] == '-' { + i++ + } + switch { + case i < len(p.data) && p.data[i] == '0': + i++ + case i < len(p.data) && p.data[i] >= '1' && p.data[i] <= '9': + i = skipDigits(p.data, i) + default: + return nil, false + } + if i < len(p.data) && p.data[i] == '.' { + i++ + if i >= len(p.data) || p.data[i] < '0' || p.data[i] > '9' { + return nil, false + } + i = skipDigits(p.data, i) + } + if i < len(p.data) && (p.data[i] == 'e' || p.data[i] == 'E') { + i++ + if i < len(p.data) && (p.data[i] == '+' || p.data[i] == '-') { + i++ + } + if i >= len(p.data) || p.data[i] < '0' || p.data[i] > '9' { + return nil, false + } + i = skipDigits(p.data, i) + } + value := p.intern(p.data[start:i]) + node := p.newNode(yaml.ScalarNode, 0, numberTag(value)) + node.Value = value + p.col += i - start + p.pos = i + return node, true +} + +func skipDigits(data []byte, i int) int { + for i < len(data) && data[i] >= '0' && data[i] <= '9' { + i++ + } + return i +} + +// numberTag resolves a JSON number the way yaml's resolver resolves a plain scalar starting with a digit +// or sign: negative zero is a float, then an integer when it parses as a signed or unsigned 64-bit +// integer, a float when it parses as one, and a string otherwise (a float literal out of range). +func numberTag(value string) string { + if value == "-0" || value == "-0.0" { + return "!!float" + } + if _, err := strconv.ParseInt(value, 0, 64); err == nil { + return "!!int" + } + if _, err := strconv.ParseUint(value, 0, 64); err == nil { + return "!!int" + } + if yamlStyleFloat.MatchString(value) { + if _, err := strconv.ParseFloat(value, 64); err == nil { + return "!!float" + } + } + return "!!str" +} + +func (p *parser) parseString() (*yaml.Node, bool) { + node := p.newNode(yaml.ScalarNode, yaml.DoubleQuotedStyle, "!!str") + p.pos++ + p.col++ + start := p.pos + escaped := false + p.buf = p.buf[:0] + for { + if p.pos >= len(p.data) { + return nil, false + } + c := p.data[p.pos] + switch { + case c == '"': + if escaped { + node.Value = p.intern(p.buf) + } else { + node.Value = p.intern(p.data[start:p.pos]) + } + p.pos++ + p.col++ + return node, true + case c == '\\': + if !escaped { + escaped = true + p.buf = append(p.buf, p.data[start:p.pos]...) + } + if !p.decodeEscape() { + return nil, false + } + case c >= 0x20 && c < 0x7F: + if escaped { + p.buf = append(p.buf, c) + } + p.pos++ + p.col++ + case c < 0x80: + // control characters and DEL: JSON forbids the former and yaml reads neither like JSON does. + return nil, false + default: + r, size := utf8.DecodeRune(p.data[p.pos:]) + if !plainRune(r, size) { + return nil, false + } + if escaped { + p.buf = append(p.buf, p.data[p.pos:p.pos+size]...) + } + p.pos += size + p.col++ + } + } +} + +// plainRune reports whether yaml reads a non-ASCII character inside a quoted scalar as itself: valid +// UTF-8 in the character set yaml accepts, and not one of the characters it treats as a line break or +// byte order mark, or as a C1 control. +func plainRune(r rune, size int) bool { + if r == utf8.RuneError && size == 1 { + return false + } + switch { + case r <= 0x9F: // C1 controls, including NEL + return false + case r == 0x2028 || r == 0x2029 || r == 0xFEFF: + return false + case r >= 0xFFFE && r <= 0xFFFF: + return false + } + return true +} + +// decodeEscape decodes the JSON escape at p.pos into p.buf. yaml has no "\\/" escape, so that one is declined. +func (p *parser) decodeEscape() bool { + if p.pos+1 >= len(p.data) { + return false + } + var b byte + switch p.data[p.pos+1] { + case '"': + b = '"' + case '\\': + b = '\\' + case 'b': + b = '\b' + case 'f': + b = '\f' + case 'n': + b = '\n' + case 'r': + b = '\r' + case 't': + b = '\t' + case 'u': + if p.pos+6 > len(p.data) { + return false + } + code := rune(0) + for _, h := range p.data[p.pos+2 : p.pos+6] { + d, ok := hexValue(h) + if !ok { + return false + } + code = code<<4 | d + } + // yaml rejects an escaped surrogate; the caller's yaml parse reports it. + if code >= 0xD800 && code <= 0xDFFF { + return false + } + p.buf = utf8.AppendRune(p.buf, code) + p.pos += 6 + p.col += 6 + return true + default: + return false + } + p.buf = append(p.buf, b) + p.pos += 2 + p.col += 2 + return true +} + +func hexValue(h byte) (rune, bool) { + switch { + case h >= '0' && h <= '9': + return rune(h - '0'), true + case h >= 'a' && h <= 'f': + return rune(h-'a') + 10, true + case h >= 'A' && h <= 'F': + return rune(h-'A') + 10, true + } + return 0, false +} diff --git a/internal/jsonnode/jsonnode_bench_test.go b/internal/jsonnode/jsonnode_bench_test.go new file mode 100644 index 000000000..8bf7d6b5e --- /dev/null +++ b/internal/jsonnode/jsonnode_bench_test.go @@ -0,0 +1,44 @@ +// Copyright 2026 Princess B33f Heavy Industries / Dave Shanley +// SPDX-License-Identifier: MIT + +package jsonnode + +import ( + "os" + "path/filepath" + "testing" + + "go.yaml.in/yaml/v4" +) + +func benchmarkSpec(b *testing.B) []byte { + b.Helper() + data, err := os.ReadFile(filepath.Join("..", "..", "test_specs", "docusignv3.1.json")) + if err != nil { + b.Fatal(err) + } + return data +} + +func BenchmarkParse_DocuSign(b *testing.B) { + data := benchmarkSpec(b) + b.SetBytes(int64(len(data))) + b.ReportAllocs() + for b.Loop() { + if _, ok := Parse(data); !ok { + b.Fatal("declined") + } + } +} + +func BenchmarkYAMLUnmarshal_DocuSign(b *testing.B) { + data := benchmarkSpec(b) + b.SetBytes(int64(len(data))) + b.ReportAllocs() + for b.Loop() { + var node yaml.Node + if err := yaml.Unmarshal(data, &node); err != nil { + b.Fatal(err) + } + } +} diff --git a/internal/jsonnode/jsonnode_test.go b/internal/jsonnode/jsonnode_test.go new file mode 100644 index 000000000..71ee4ddb6 --- /dev/null +++ b/internal/jsonnode/jsonnode_test.go @@ -0,0 +1,224 @@ +// Copyright 2026 Princess B33f Heavy Industries / Dave Shanley +// SPDX-License-Identifier: MIT + +package jsonnode + +import ( + "bytes" + "encoding/json" + "fmt" + "io/fs" + "os" + "path/filepath" + "strings" + "testing" + + "github.com/pb33f/testify/require" + "go.yaml.in/yaml/v4" +) + +// sameTree reports the first difference between two node trees, comparing every field yaml sets. +func sameTree(want, got *yaml.Node, path string) error { + switch { + case want.Kind != got.Kind: + return fmt.Errorf("%s: kind %v != %v", path, got.Kind, want.Kind) + case want.Style != got.Style: + return fmt.Errorf("%s: style %v != %v", path, got.Style, want.Style) + case want.Tag != got.Tag: + return fmt.Errorf("%s: tag %q != %q", path, got.Tag, want.Tag) + case want.Value != got.Value: + return fmt.Errorf("%s: value %q != %q", path, got.Value, want.Value) + case want.Line != got.Line || want.Column != got.Column: + return fmt.Errorf("%s: position %d:%d != %d:%d", path, got.Line, got.Column, want.Line, want.Column) + case want.Anchor != got.Anchor || want.Alias != got.Alias || want.Stream != got.Stream: + return fmt.Errorf("%s: anchor, alias or stream differ", path) + case want.HeadComment != got.HeadComment || want.LineComment != got.LineComment || + want.FootComment != got.FootComment: + return fmt.Errorf("%s: comments differ", path) + case (want.Content == nil) != (got.Content == nil) || len(want.Content) != len(got.Content): + return fmt.Errorf("%s: content %d (nil %v) != %d (nil %v)", path, + len(got.Content), got.Content == nil, len(want.Content), want.Content == nil) + } + for i := range want.Content { + if err := sameTree(want.Content[i], got.Content[i], fmt.Sprintf("%s/%d", path, i)); err != nil { + return err + } + } + return nil +} + +// checkParse asserts the parser either declines the input or builds exactly the tree yaml builds. +func checkParse(t testing.TB, data []byte) bool { + t.Helper() + got, ok := Parse(data) + if !ok { + return false + } + var want yaml.Node + require.NoError(t, yaml.Unmarshal(data, &want), "accepted input that yaml rejects: %q", data) + require.NoError(t, sameTree(&want, got, "doc"), "input: %q", data) + return true +} + +func TestParse_MatchesYAML(t *testing.T) { + accepted := []string{ + `{}`, + `{"a":1}`, + "\n\n {\"a\": {}, \"b\": [], \"c\": [1, -2.5e3, true, null, \"x\\u00e9\\n\"]}\n\n", + "{\r\n \"a\" : 1,\r\n \"b\": [\r\n 2\r\n ]\r\n}\r\n", + "{\n\t\"a\":\t1\n}", + `{"é": "ü", "日本": "語", "emoji": "😀", "x": "a\u00e9b"}`, + `{"esc": "\"\\\b\f\n\r\t\u0041\u00e9\u20AC\u0000"}`, + `{"n": [0, -0, -0.0, -0.00, 1, -1, 0.5, -0.5, 1e5, 1E5, 1e+5, 1e-5, 1.5E-10, 123456789012345678901234567890]}`, + `{"range": [9223372036854775807, 9223372036854775808, 18446744073709551615, 18446744073709551616, -9223372036854775808, -9223372036854775809]}`, + `{"inf": 1e400, "tiny": 1e-400}`, + `{"nested": {"deeper": {"deepest": [[[{"a": [null]}]]]}}}`, + `{"":""}`, + `{"a":"b" , "c" :"d"}`, + "{\"key\":\n \"value on the next line\"}", + `{"dup": 1, "dup": 2}`, + `{"#": "# not a comment", "&a": "*a", "!tag": "- item", "k": "a: b", "q": "'"}`, + `{"long": "` + strings.Repeat("long text ", 200) + `"}`, + `{"` + strings.Repeat("k", 2000) + `": 1}`, + `{"c1ok": "\u0085\u2028\u2029\ufeff\uffff"}`, + `{"del": "\u007f"}`, + } + for _, src := range accepted { + require.True(t, checkParse(t, []byte(src)), "expected the parser to accept %q", src) + } + + declined := []string{ + ``, + ` `, + `[]`, + `"string"`, + `{`, + `{"a"`, + `{"a":`, + `{"a":1`, + `{"a":1,`, + `{"a":1,}`, + `{"a" 1}`, + `{a: 1}`, + `{"a": 1} x`, + `{"a": 1} {}`, + "\t{}", + "{}\t", + "{\r}", + "{\"a\"\r: 1}", + "{\"a\":\r1}", + "{\"a\":1,\r\"b\":2}", + `{"a": "\u12`, + "{\"a\"\n: 1}", + `{"a": [1,]}`, + `{"a": [1 2]}`, + `{"a": tru}`, + `{"a": truex}`, + `{"a": nul}`, + `{"a": fals}`, + `{"a": -}`, + `{"a": 01}`, + `{"a": 1.}`, + `{"a": 1.e5}`, + `{"a": 1e}`, + `{"a": 1e+}`, + `{"a": .5}`, + `{"a": +1}`, + `{"a": 0x1F}`, + `{"a": x}`, + `{"a": "unterminated}`, + `{"a": "bad \x escape"}`, + `{"a": "solidus \/ escape"}`, + `{"a": "bad \u12 escape"}`, + `{"a": "bad \u12G4 escape"}`, + `{"a": "\ud83d\ude00"}`, + `{"a": "\udc00"}`, + "{\"a\": \"raw\ttab\"}", + "{\"a\": \"raw\nbreak\"}", + "{\"a\": \"del\x7f\"}", + "{\"a\": \"c1\u0085\"}", + "{\"a\": \"ls\xe2\x80\xa8\"}", + "{\"a\": \"ps\xe2\x80\xa9\"}", + "{\"a\": \"bom\xef\xbb\xbf\"}", + "{\"a\": \"nonchar\xef\xbf\xbf\"}", + "{\"a\": \"bad \xff utf8\"}", + "{\"a\": \"truncated \xe2\x82\"}", + `{"a": "trailing backslash\`, + strings.Repeat(`{"a":`, maxDepth+1) + "1" + strings.Repeat("}", maxDepth+1), + strings.Repeat(`{"a":[`, maxDepth/2) + "1" + strings.Repeat("]}", maxDepth/2) + "x", + } + for _, src := range declined { + _, ok := Parse([]byte(src)) + require.False(t, ok, "expected the parser to decline %q", src) + } +} + +// Every JSON fixture in the repository, and every YAML fixture re-encoded as JSON in several layouts, +// parses to exactly the tree yaml builds. +func TestParse_RepositoryFixtures(t *testing.T) { + root := filepath.Join("..", "..") + var jsonFiles, converted int + err := filepath.WalkDir(root, func(path string, d fs.DirEntry, err error) error { + require.NoError(t, err) + if d.IsDir() && (d.Name() == ".git" || d.Name() == ".claude") { + return filepath.SkipDir + } + if d.IsDir() { + return nil + } + data, err := os.ReadFile(path) + require.NoError(t, err) + switch filepath.Ext(path) { + case ".json": + trimmed := bytes.TrimSpace(data) + if len(trimmed) > 0 && trimmed[0] == '{' && trimmed[len(trimmed)-1] == '}' { + require.True(t, checkParse(t, data), "declined fixture %s", path) + jsonFiles++ + } + case ".yaml", ".yml": + var decoded map[string]any + if yaml.Unmarshal(data, &decoded) != nil || decoded == nil { + return nil + } + compact, err := json.Marshal(decoded) + if err != nil { + return nil // yaml values json cannot represent, such as maps with non-string keys + } + indented, err := json.MarshalIndent(decoded, "", " ") + require.NoError(t, err) + tabbed, err := json.MarshalIndent(decoded, "", "\t") + require.NoError(t, err) + for _, doc := range [][]byte{compact, indented, tabbed} { + require.True(t, checkParse(t, doc), "declined %s re-encoded as JSON", path) + } + converted++ + } + return nil + }) + require.NoError(t, err) + require.GreaterOrEqual(t, jsonFiles, 10) + require.GreaterOrEqual(t, converted, 100) +} + +// Unmarshal decodes JSON through Parse and everything else through yaml, matching yaml.Unmarshal either way. +func TestUnmarshal(t *testing.T) { + for _, src := range []string{`{"a": [1, "b"]}`, "a: 1\nb: [2]\n", `{"a": 1} x`, `{a: 1}`, `{"a": "\/"}`} { + var want, got yaml.Node + wantErr := yaml.Unmarshal([]byte(src), &want) + gotErr := Unmarshal([]byte(src), &got) + require.Equal(t, wantErr, gotErr, "input %q", src) + require.NoError(t, sameTree(&want, &got, "doc"), "input %q", src) + } +} + +func FuzzParse(f *testing.F) { + for _, seed := range []string{ + `{}`, `{"a":1}`, `{"a":[1,2.5,-3e4,true,false,null,"s"]}`, `{"a":{"b":{"c":"\u00e9\n"}}}`, + "{\r\n\t\"a\" : \"b\"\r\n}", `{"a":"😀"}`, `{"":0}`, + } { + f.Add([]byte(seed)) + } + f.Fuzz(func(t *testing.T, data []byte) { + checkParse(t, data) + }) +} diff --git a/json/json.go b/json/json.go index b223f7412..7dc7c5af8 100644 --- a/json/json.go +++ b/json/json.go @@ -1,9 +1,13 @@ package json import ( + "bytes" "encoding/json" "errors" "fmt" + "math" + "strconv" + "unicode/utf8" "github.com/pb33f/libopenapi/orderedmap" "go.yaml.in/yaml/v4" @@ -11,17 +15,362 @@ import ( // YAMLNodeToJSON converts yaml/json stored in a yaml.Node to json ordered matching the original yaml/json func YAMLNodeToJSON(node *yaml.Node, indentation string) ([]byte, error) { + w := writer{} + if err := w.writeNode(node); err != nil { + return nil, err + } + if w.marshalFailed { + // a value encoding/json cannot represent (NaN or an infinity) is reported by marshaling the whole + // converted value, which is where and how that error has always surfaced. + return convertAndMarshal(node, indentation) + } + var out bytes.Buffer + _ = json.Indent(&out, w.buf, "", indentation) // w.buf is valid JSON by construction + return out.Bytes(), nil +} + +// convertAndMarshal converts the node tree to ordered values and marshals them with encoding/json. +func convertAndMarshal(node *yaml.Node, indentation string) ([]byte, error) { c := converter{aliasesInFlight: make(map[*yaml.Node]struct{})} v, err := c.handleYAMLNode(node) if err != nil { return nil, err } - return json.MarshalIndent(v, "", indentation) } -// converter tracks alias targets currently being expanded, so a self-referencing anchor -// (e.g. `a: &x [1, *x]`) is reported as an error instead of recursing forever. +// dupScanLimit is the mapping size up to which repeated keys are found by scanning the keys written so +// far; larger mappings track their keys in a set. +const dupScanLimit = 16 + +// writer writes the compact JSON that encoding/json produces for the ordered values the converter +// builds, directly from the node tree. Values are written as json.Marshal writes them, so indenting +// the result gives exactly what json.MarshalIndent gives for the converted value. Anything outside the +// common shapes (scalars that need yaml's full decoding, complex keys, mappings with repeated keys) is +// converted by the converter and marshaled by encoding/json, as before. +type writer struct { + buf []byte + keys []string // keys of the mappings being written, for spotting repeated keys + aliasesInFlight map[*yaml.Node]struct{} + marshalFailed bool // a value could not be marshaled +} + +func (w *writer) converter() converter { + if w.aliasesInFlight == nil { + w.aliasesInFlight = make(map[*yaml.Node]struct{}) + } + return converter{aliasesInFlight: w.aliasesInFlight} +} + +func (w *writer) writeNode(node *yaml.Node) error { + if node == nil { + return errors.New("nil yaml node") + } + switch node.Kind { + case yaml.DocumentNode: + if len(node.Content) == 0 { + return errors.New("empty yaml document") + } + return w.writeNode(node.Content[0]) + case yaml.SequenceNode: + w.buf = append(w.buf, '[') + for i, n := range node.Content { + if i > 0 { + w.buf = append(w.buf, ',') + } + if err := w.writeNode(n); err != nil { + return err + } + } + w.buf = append(w.buf, ']') + return nil + case yaml.MappingNode: + return w.writeMapping(node) + case yaml.ScalarNode: + if b, ok := appendScalar(w.buf, node); ok { + w.buf = b + return nil + } + v, err := handleScalarNode(node) + if err != nil { + return err + } + w.writeMarshaled(v) + return nil + case yaml.AliasNode: + c := w.converter() + if _, inFlight := c.aliasesInFlight[node.Alias]; inFlight { + return fmt.Errorf("recursive alias '%s' at line %d, column %d", node.Value, node.Line, node.Column) + } + c.aliasesInFlight[node.Alias] = struct{}{} + defer delete(c.aliasesInFlight, node.Alias) + return w.writeNode(node.Alias) + default: + return fmt.Errorf("unknown node kind: %v", node.Kind) + } +} + +// writeMarshaled writes a value converted from the nodes. A value that cannot be marshaled is noted +// rather than returned: conversion errors anywhere in the tree take precedence over it. +func (w *writer) writeMarshaled(v any) { + data, err := json.Marshal(v) + if err != nil { + w.marshalFailed = true + data = []byte("null") + } + w.buf = append(w.buf, data...) +} + +func (w *writer) writeMapping(node *yaml.Node) error { + start, keysBase := len(w.buf), len(w.keys) + var seen map[string]struct{} + if len(node.Content)/2 > dupScanLimit { + seen = make(map[string]struct{}, len(node.Content)/2) + } + w.buf = append(w.buf, '{') + for i := 1; i < len(node.Content); i += 2 { + key, err := w.keyString(node.Content[i-1]) + if err != nil { + return err + } + if w.repeatedKey(key, keysBase, seen) { + // a repeated key keeps its first position and takes the last value: the ordered map does that. + w.buf, w.keys = w.buf[:start], w.keys[:keysBase] + v, err := w.converter().handleMappingNode(node) + if err != nil { + return err + } + w.writeMarshaled(v) + return nil + } + if i > 1 { + w.buf = append(w.buf, ',') + } + w.buf = append(appendJSONString(w.buf, key), ':') + if err := w.writeNode(node.Content[i]); err != nil { + return err + } + } + w.keys = w.keys[:keysBase] + w.buf = append(w.buf, '}') + return nil +} + +// repeatedKey reports whether key was already written in the current mapping, and records it. +func (w *writer) repeatedKey(key string, keysBase int, seen map[string]struct{}) bool { + if seen != nil { + if _, ok := seen[key]; ok { + return true + } + seen[key] = struct{}{} + return false + } + for _, k := range w.keys[keysBase:] { + if k == key { + return true + } + } + w.keys = append(w.keys, key) + return false +} + +// keyString returns the ordered map key the converter makes of a key node: a string key as it is, and +// any other key as its JSON encoding. +func (w *writer) keyString(keyNode *yaml.Node) (string, error) { + if keyNode != nil && keyNode.Kind == yaml.ScalarNode { + if keyNode.Tag == "!!str" { + return keyNode.Value, nil + } + var scratch [32]byte + if b, ok := appendScalar(scratch[:0], keyNode); ok { + return string(b), nil + } + } + kv, err := w.converter().handleYAMLNode(keyNode) + if err != nil { + return "", err + } + if key, isString := kv.(string); isString { + return key, nil + } + keyData, err := json.Marshal(kv) + if err != nil { + return "", err + } + return string(keyData), nil +} + +// appendScalar appends the JSON encoding of the value yaml decodes a scalar node into, for the tags and +// values whose decoding is known exactly, and reports false for anything else. +func appendScalar(b []byte, n *yaml.Node) ([]byte, bool) { + switch n.Tag { + case "!!str": + return appendJSONString(b, n.Value), true + case "!!int": + // a canonical decimal in range decodes to an integer that encodes back to the same digits. + if isJSONInteger(n.Value) && n.Value != "-0" { + if _, err := strconv.ParseInt(n.Value, 10, 64); err == nil { + return append(b, n.Value...), true + } + } + case "!!bool": + switch n.Value { + case "true", "True", "TRUE": + return append(b, "true"...), true + case "false", "False", "FALSE": + return append(b, "false"...), true + } + case "!!null": + switch n.Value { + case "", "~", "null", "Null", "NULL": + return append(b, "null"...), true + } + case "!!float": + // yaml resolves -0 to negative zero, and an integer beyond int64 to an unsigned value it will + // not accept as a float; everything else in JSON number form decodes like ParseFloat. + if n.Value == "-0" || n.Value == "-0.0" || !isJSONNumber(n.Value) { + return b, false + } + if isJSONInteger(n.Value) { + if _, err := strconv.ParseInt(n.Value, 10, 64); err != nil { + return b, false + } + } + if f, err := strconv.ParseFloat(n.Value, 64); err == nil { + return appendJSONFloat(b, f), true + } + } + return b, false +} + +// isJSONInteger reports whether s is a JSON integer: an optional minus and digits without a leading zero. +func isJSONInteger(s string) bool { + return len(s) > 0 && jsonNumberEnd(s, false) == len(s) +} + +// isJSONNumber reports whether s is a JSON number. +func isJSONNumber(s string) bool { + return len(s) > 0 && jsonNumberEnd(s, true) == len(s) +} + +// jsonNumberEnd returns the length of the JSON number (or integer, without fraction or exponent) at the +// start of s, or -1 when s does not start with one. +func jsonNumberEnd(s string, fractional bool) int { + i := 0 + if s[i] == '-' { + i++ + } + switch { + case i < len(s) && s[i] == '0': + i++ + case i < len(s) && s[i] >= '1' && s[i] <= '9': + for i < len(s) && s[i] >= '0' && s[i] <= '9' { + i++ + } + default: + return -1 + } + if !fractional { + return i + } + if i < len(s) && s[i] == '.' { + i++ + if i >= len(s) || s[i] < '0' || s[i] > '9' { + return -1 + } + for i < len(s) && s[i] >= '0' && s[i] <= '9' { + i++ + } + } + if i < len(s) && (s[i] == 'e' || s[i] == 'E') { + i++ + if i < len(s) && (s[i] == '+' || s[i] == '-') { + i++ + } + if i >= len(s) || s[i] < '0' || s[i] > '9' { + return -1 + } + for i < len(s) && s[i] >= '0' && s[i] <= '9' { + i++ + } + } + return i +} + +// appendJSONFloat appends a finite float64 formatted as encoding/json formats one. +func appendJSONFloat(b []byte, f float64) []byte { + format := byte('f') + if abs := math.Abs(f); abs != 0 && (abs < 1e-6 || abs >= 1e21) { + format = 'e' + } + b = strconv.AppendFloat(b, f, format, -1, 64) + if format == 'e' { + // clean up e-09 to e-9 + if n := len(b); n >= 4 && b[n-4] == 'e' && b[n-3] == '-' && b[n-2] == '0' { + b[n-2] = b[n-1] + b = b[:n-1] + } + } + return b +} + +const hexDigits = "0123456789abcdef" + +// appendJSONString appends s as a JSON string, escaped as encoding/json's Marshal escapes it: quotes, +// backslashes and control characters, the HTML characters <, > and &, U+2028 and U+2029, with invalid +// UTF-8 replaced by U+FFFD. +func appendJSONString(b []byte, s string) []byte { + b = append(b, '"') + start := 0 + for i := 0; i < len(s); { + if c := s[i]; c < utf8.RuneSelf { + if c >= 0x20 && c != '"' && c != '\\' && c != '<' && c != '>' && c != '&' { + i++ + continue + } + b = append(b, s[start:i]...) + switch c { + case '"', '\\': + b = append(b, '\\', c) + case '\b': + b = append(b, '\\', 'b') + case '\f': + b = append(b, '\\', 'f') + case '\n': + b = append(b, '\\', 'n') + case '\r': + b = append(b, '\\', 'r') + case '\t': + b = append(b, '\\', 't') + default: + b = append(b, '\\', 'u', '0', '0', hexDigits[c>>4], hexDigits[c&0xF]) + } + i++ + start = i + continue + } + r, size := utf8.DecodeRuneInString(s[i:]) + switch { + case r == utf8.RuneError && size == 1: + b = append(b, s[start:i]...) + b = append(b, `\ufffd`...) + case r == '\u2028' || r == '\u2029': + b = append(b, s[start:i]...) + b = append(b, '\\', 'u', '2', '0', '2', hexDigits[r&0xF]) + default: + i += size + continue + } + i += size + start = i + } + b = append(b, s[start:]...) + return append(b, '"') +} + +// converter builds ordered values from a node tree for encoding/json to marshal. It tracks alias targets +// currently being expanded, so a self-referencing anchor (e.g. `a: &x [1, *x]`) is reported as an error +// instead of recursing forever. type converter struct { aliasesInFlight map[*yaml.Node]struct{} } diff --git a/json/json_writer_test.go b/json/json_writer_test.go new file mode 100644 index 000000000..d25893908 --- /dev/null +++ b/json/json_writer_test.go @@ -0,0 +1,168 @@ +package json + +import ( + "encoding/json" + "io/fs" + "math" + "os" + "path/filepath" + "strings" + "testing" + + "github.com/pb33f/testify/require" + "go.yaml.in/yaml/v4" +) + +// requireMatchesMarshal asserts YAMLNodeToJSON gives exactly what converting the tree and marshaling it +// with encoding/json gives: the same bytes, or the same error. +func requireMatchesMarshal(t testing.TB, node *yaml.Node, indentation string) { + t.Helper() + want, wantErr := convertAndMarshal(node, indentation) + got, gotErr := YAMLNodeToJSON(node, indentation) + if wantErr != nil { + require.EqualError(t, gotErr, wantErr.Error()) + return + } + require.NoError(t, gotErr) + require.Equal(t, string(want), string(got)) +} + +func requireYAMLMatchesMarshal(t testing.TB, src string) { + t.Helper() + var node yaml.Node + if yaml.Unmarshal([]byte(src), &node) != nil { + return + } + for _, indentation := range []string{"", " ", "\t"} { + requireMatchesMarshal(t, &node, indentation) + } +} + +// Every fixture in the repository converts exactly as encoding/json marshals it. +func TestYAMLNodeToJSON_RepositoryFixtures(t *testing.T) { + var converted int + err := filepath.WalkDir(filepath.Join(".."), func(path string, d fs.DirEntry, err error) error { + require.NoError(t, err) + if d.IsDir() && (d.Name() == ".git" || d.Name() == ".claude") { + return filepath.SkipDir + } + switch filepath.Ext(path) { + case ".yaml", ".yml", ".json": + data, err := os.ReadFile(path) + require.NoError(t, err) + requireYAMLMatchesMarshal(t, string(data)) + converted++ + } + return nil + }) + require.NoError(t, err) + require.Greater(t, converted, 100) +} + +func TestYAMLNodeToJSON_MatchesMarshal(t *testing.T) { + var many strings.Builder + for i := 0; i < 3*dupScanLimit; i++ { + many.WriteString("k" + strings.Repeat("x", i) + ": 1\n") + } + manyWithRepeat := many.String() + "kxxx: 2\n" + for _, src := range []string{ + "a: 1\nb: [1, 2.5, -3e4, true, false, null, s]\nc: {d: e}\n", + "html: '&'\ncontrol: \"\\b\\f\\n\\r\\t\\x01\\x7f\"\nsep: \"\\u2028\\u2029\"\n", + "unicode: \"é 日本 😀\"\n", + "ints: [0, -1, 123, 007, 0x1F, 0o17, 1_000, +5, -0, 9223372036854775807, 9223372036854775808, 18446744073709551616]\n", + "floats: [0.5, -0.5, 1e5, 1E-7, 1.0e21, 1e20, 123456789012345678901234567890, .5, 5., -0.0, 1e400, .inf, -.inf, .nan]\n", + "tagged: [!!int 5, !!int -0, !!int 1.5, !!float 5, !!float 9223372036854775808, !!float 1e400, !!float -0, !!float abc]\n", + "bools: [true, True, TRUE, false, False, FALSE, !!bool yes]\nnulls: [~, null, Null, NULL, !!null '', !!null x]\n", + "stamp: 2001-12-14\nstampt: 2001-12-14t21:59:43.10-05:00\nbin: !!binary aGVsbG8=\nbadbin: !!binary '@@'\n", + "strs: [!!str 5, '5', \"true\", !custom thing, !!str]\n", + "200: ok\n1.5: float\ntrue: bool\n~: null\n2001-12-14: date\n.nan: nan\n[a, b]: seq\n{x: y}: map\n", + "dup: 1\ndup: 2\nother: 3\n", + manyWithRepeat, + many.String(), + "base: &b {x: 1}\nuse: *b\nlist: &l [1, 2]\nagain: *l\n", + "<<: {merged: true}\nplain: 1\n", + "empty: {}\nemptyList: []\nnested: [[], {}]\n", + "just a scalar\n", + "- a\n- b\n", + "", + } { + requireYAMLMatchesMarshal(t, src) + } +} + +// Node trees yaml.Unmarshal never builds: untagged scalars, broken structure and recursion. +func TestYAMLNodeToJSON_ConstructedTrees(t *testing.T) { + scalar := func(tag, value string, style yaml.Style) *yaml.Node { + return &yaml.Node{Kind: yaml.ScalarNode, Tag: tag, Value: value, Style: style} + } + mapping := func(content ...*yaml.Node) *yaml.Node { + return &yaml.Node{Kind: yaml.MappingNode, Tag: "!!map", Content: content} + } + recursive := &yaml.Node{Kind: yaml.SequenceNode, Tag: "!!seq", Anchor: "x"} + recursive.Content = []*yaml.Node{{Kind: yaml.AliasNode, Value: "x", Alias: recursive}} + recursiveKey := &yaml.Node{Kind: yaml.MappingNode, Tag: "!!map"} + recursiveKey.Content = []*yaml.Node{{Kind: yaml.AliasNode, Value: "k", Alias: recursiveKey}, scalar("!!str", "v", 0)} + nanNode := scalar("!!float", ".nan", 0) + for _, node := range []*yaml.Node{ + nil, + {Kind: yaml.DocumentNode}, + {Kind: yaml.Kind(99)}, + {Kind: yaml.DocumentNode, Content: []*yaml.Node{mapping(scalar("", "5", 0), scalar("", "true", 0))}}, + mapping(scalar("", "quoted", yaml.DoubleQuotedStyle), scalar("tag:yaml.org,2002:str", "long", 0)), + mapping(nil, scalar("!!str", "v", 0)), + mapping(scalar("!!str", "k", 0), nil), + mapping(scalar("!!str", "odd", 0)), + mapping(scalar("!!str", "a", 0), nanNode, scalar("!!str", "b", 0), &yaml.Node{Kind: yaml.Kind(99)}), + mapping(nanNode, scalar("!!str", "nan key", 0)), + mapping(scalar("!!str", "a", 0), nanNode, scalar("!!str", "a", 0), scalar("!!str", "dup", 0)), + mapping(scalar("!!str", "a", 0), scalar("!!str", "1", 0), scalar("!!str", "a", 0), &yaml.Node{Kind: yaml.Kind(99)}), + {Kind: yaml.SequenceNode, Tag: "!!seq", Content: []*yaml.Node{scalar("!!int", "1", 0), nil}}, + recursive, + recursiveKey, + nanNode, + {Kind: yaml.SequenceNode, Tag: "!!seq", Content: []*yaml.Node{ + scalar("!!float", "1e", 0), scalar("!!float", "1.5e+", 0), scalar("!!float", "1.", 0)}}, + } { + for _, indentation := range []string{"", " "} { + requireMatchesMarshal(t, node, indentation) + } + } +} + +func FuzzYAMLNodeToJSON(f *testing.F) { + for _, seed := range []string{ + "a: 1\n", "a: [1, 2.5, true, null, 'x']\n", "{\"a\": {\"b\": \"<&>\"}}", "200: x\n1.5: y\n", + "a: &x [1]\nb: *x\n", "a: 1\na: 2\n", "x: !!float 1e5\ny: !!int 7\n", "- 2001-12-14\n- !!binary aGk=\n", + } { + f.Add(seed) + } + f.Fuzz(func(t *testing.T, src string) { + requireYAMLMatchesMarshal(t, src) + }) +} + +// appendJSONString and appendJSONFloat write exactly what encoding/json writes. +func FuzzAppendJSONString(f *testing.F) { + for _, seed := range []string{"", "plain", "", "\"\\\b\f\n\r\t\x00\x1f\x7f", "é😀

", "\xff\xfe bad"} { + f.Add(seed) + } + f.Fuzz(func(t *testing.T, s string) { + want, err := json.Marshal(s) + require.NoError(t, err) + require.Equal(t, string(want), string(appendJSONString(nil, s))) + }) +} + +func FuzzAppendJSONFloat(f *testing.F) { + for _, seed := range []float64{0, math.Copysign(0, -1), 1, -1.5, 1e-7, 1e-6, 1e20, 1e21, 123456789.125, math.MaxFloat64, math.SmallestNonzeroFloat64} { + f.Add(seed) + } + f.Fuzz(func(t *testing.T, v float64) { + if math.IsNaN(v) || math.IsInf(v, 0) { + return + } + want, err := json.Marshal(v) + require.NoError(t, err) + require.Equal(t, string(want), string(appendJSONFloat(nil, v))) + }) +} diff --git a/orderedmap/builder.go b/orderedmap/builder.go index 057a965a8..100da2ef9 100644 --- a/orderedmap/builder.go +++ b/orderedmap/builder.go @@ -97,7 +97,7 @@ func encodeMarshalYAMLValue(value any) (any, error) { func (o *Map[K, V]) ToYamlNode(n NodeBuilder, l any) *yaml.Node { p := utils.CreateEmptyMapNode() if o != nil { - p.Content = make([]*yaml.Node, 0) + p.Content = make([]*yaml.Node, 0, 2*o.Len()) } var vn *yaml.Node @@ -112,6 +112,11 @@ func (o *Map[K, V]) ToYamlNode(n NodeBuilder, l any) *yaml.Node { } } + keyNodes := keyNodeIndex{mapNode: vn} + var lowValues *untypedValueIndex + var lowFinder findValueUntyped + lowResolved := false + for pair := First(o); pair != nil; pair = pair.Next() { var k any = pair.Key() if m, ok := k.(marshaler); ok { // TODO marshal inline? @@ -123,21 +128,31 @@ func (o *Map[K, V]) ToYamlNode(n NodeBuilder, l any) *yaml.Node { ks := k.(string) var keyStyle yaml.Style - keyNode := findKeyNode(ks, vn) + keyNode := keyNodes.find(ks) if keyNode != nil { keyStyle = keyNode.Style } - var lv any - if l != nil { + // resolve the low-level map once, indexing it so each key is found without a scan. + if !lowResolved { + lowResolved = true if hvut, ok := l.(hasValueUntyped); ok { vut := hvut.GetValueUntyped() - if m, ok := vut.(findValueUntyped); ok { - lv = m.FindValueUntyped(ks) + if indexer, ok := vut.(untypedValueIndexer); ok { + lowValues = indexer.untypedValueIndex() + } else if m, ok := vut.(findValueUntyped); ok { + lowFinder = m } } } + var lv any + if lowValues != nil { + lv = lowValues.find(ks) + } else if lowFinder != nil { + lv = lowFinder.FindValueUntyped(ks) + } + n.AddYAMLNode(p, &nodes.NodeEntry{ Tag: ks, Key: ks, @@ -152,6 +167,114 @@ func (o *Map[K, V]) ToYamlNode(n NodeBuilder, l any) *yaml.Node { return p } +// indexScanLimit is the entry count up to which an index answers lookups by scanning; above it, a hash +// map is built on first use. +const indexScanLimit = 16 + +// keyNodeIndex finds key nodes of a mapping node with the same first-match semantics as findKeyNode. +type keyNodeIndex struct { + mapNode *yaml.Node + byKey map[string]*yaml.Node +} + +func (x *keyNodeIndex) find(key string) *yaml.Node { + if x.mapNode == nil || len(x.mapNode.Content) <= 2*indexScanLimit { + return findKeyNode(key, x.mapNode) + } + if x.byKey == nil { + x.byKey = make(map[string]*yaml.Node, len(x.mapNode.Content)/2) + for i := 0; i < len(x.mapNode.Content); i += 2 { + if _, seen := x.byKey[x.mapNode.Content[i].Value]; !seen { + x.byKey[x.mapNode.Content[i].Value] = x.mapNode.Content[i] + } + } + } + return x.byKey[key] +} + +// untypedValueIndexer is implemented by Map. It lets ToYamlNode resolve the low-level value of every key +// in one pass over the low-level map, rather than one FindValueUntyped scan per key. +type untypedValueIndexer interface { + untypedValueIndex() *untypedValueIndex +} + +// untypedValueIndex answers FindValueUntyped lookups with identical results: a pair matches a key when +// the string form of its untyped key value, or of the key itself, equals the key, and the oldest +// matching pair wins. +type untypedValueIndex struct { + names []string // match strings, oldest pair first + values []any // values[i] is the value of the pair names[i] came from + byName map[string]int + + // braceForms is set when the keys are structs printed as "{...}". Those forms are not indexed, so a + // key starting with a brace is answered by the exact scan instead. + braceForms bool + finder findValueUntyped +} + +func (o *Map[K, V]) untypedValueIndex() *untypedValueIndex { + x := &untypedValueIndex{finder: o, braceForms: formatsAsStructLiteral(reflect.TypeFor[K]())} + if o == nil { + return x + } + for pair := o.Oldest(); pair != nil; pair = pair.Next() { + var k any = pair.Key + value := any(pair.Value) + if hvut, ok := k.(hasValueUntyped); ok { + x.names = append(x.names, formatUntyped(hvut.GetValueUntyped())) + x.values = append(x.values, value) + } + if !x.braceForms { + x.names = append(x.names, formatUntyped(k)) + x.values = append(x.values, value) + } + } + return x +} + +func (x *untypedValueIndex) find(key string) any { + if x.braceForms && strings.HasPrefix(key, "{") { + return x.finder.FindValueUntyped(key) + } + if len(x.names) <= indexScanLimit { + for i, name := range x.names { + if name == key { + return x.values[i] + } + } + return nil + } + if x.byName == nil { + x.byName = make(map[string]int, len(x.names)) + for i, name := range x.names { + if _, seen := x.byName[name]; !seen { + x.byName[name] = i + } + } + } + if i, ok := x.byName[key]; ok { + return x.values[i] + } + return nil +} + +// formatUntyped returns fmt.Sprintf("%v", v), skipping the formatter for plain strings. +func formatUntyped(v any) string { + if s, ok := v.(string); ok { + return s + } + return fmt.Sprintf("%v", v) +} + +// formatsAsStructLiteral reports whether %v prints values of t as "{...}": a struct with no method that +// would take over its formatting. +func formatsAsStructLiteral(t reflect.Type) bool { + return t.Kind() == reflect.Struct && + !t.Implements(reflect.TypeFor[fmt.Formatter]()) && + !t.Implements(reflect.TypeFor[fmt.Stringer]()) && + !t.Implements(reflect.TypeFor[error]()) +} + func findKeyNode(key string, m *yaml.Node) *yaml.Node { if m == nil { return nil diff --git a/orderedmap/builder_index_test.go b/orderedmap/builder_index_test.go new file mode 100644 index 000000000..1b232f2cc --- /dev/null +++ b/orderedmap/builder_index_test.go @@ -0,0 +1,191 @@ +package orderedmap + +import ( + "fmt" + "reflect" + "strconv" + "testing" + + "github.com/pb33f/libopenapi/datamodel/high/nodes" + "github.com/pb33f/testify/require" + "go.yaml.in/yaml/v4" +) + +// untypedKey mirrors low.KeyReference: a struct key exposing its string through GetValueUntyped. +type untypedKey struct { + Value string + Node *yaml.Node +} + +func (k untypedKey) GetValueUntyped() any { return k.Value } + +// stringerKey is a struct whose String method takes over %v formatting. +type stringerKey struct{ Value string } + +func (k stringerKey) String() string { return "s:" + k.Value } + +// plainKey is a struct with no methods at all, printed as "{...}" by %v. +type plainKey struct{ Value string } + +// errorKey and formatterKey take over %v formatting through error and fmt.Formatter. +type errorKey struct{ Value string } + +func (k errorKey) Error() string { return k.Value } + +type formatterKey struct{ Value string } + +func (k formatterKey) Format(f fmt.State, _ rune) { _, _ = f.Write([]byte(k.Value)) } + +// requireIndexMatchesScan asserts the index answers every probe exactly as the FindValueUntyped scan does. +func requireIndexMatchesScan[K comparable, V any](t *testing.T, m *Map[K, V], probes []string) { + t.Helper() + x := m.untypedValueIndex() + for _, probe := range probes { + require.Equal(t, m.FindValueUntyped(probe), x.find(probe), "probe %q", probe) + } +} + +func indexProbes(n int) []string { + probes := []string{"", "missing", "{", "{key-0", "s:key-1"} + for i := 0; i < n; i++ { + probes = append(probes, "key-"+strconv.Itoa(i)) + } + return probes +} + +func TestUntypedValueIndex_MatchesScan(t *testing.T) { + for _, n := range []int{0, 1, 5, indexScanLimit, indexScanLimit + 1, 64} { + t.Run(strconv.Itoa(n), func(t *testing.T) { + probes := indexProbes(n) + + strings := New[string, int]() + structs := New[untypedKey, int]() + pointers := New[*untypedKey, int]() + stringers := New[stringerKey, int]() + plains := New[plainKey, int]() + for i := 0; i < n; i++ { + name := "key-" + strconv.Itoa(i) + strings.Set(name, i) + structs.Set(untypedKey{Value: name, Node: &yaml.Node{}}, i) + pointers.Set(&untypedKey{Value: name}, i) + stringers.Set(stringerKey{Value: name}, i) + plains.Set(plainKey{Value: name}, i) + } + // the %v forms of these keys are only knowable at runtime, so probe for them too. + for k := range structs.KeysFromOldest() { + probes = append(probes, fmt.Sprintf("%v", k)) + } + for k := range pointers.KeysFromOldest() { + probes = append(probes, fmt.Sprintf("%v", k)) + } + for k := range plains.KeysFromOldest() { + probes = append(probes, fmt.Sprintf("%v", k)) + } + + requireIndexMatchesScan(t, strings, probes) + requireIndexMatchesScan(t, structs, probes) + requireIndexMatchesScan(t, pointers, probes) + requireIndexMatchesScan(t, stringers, probes) + requireIndexMatchesScan(t, plains, probes) + }) + } +} + +// When two pairs produce the same match string, the oldest pair wins, as it does for the scan. +func TestUntypedValueIndex_OldestMatchWins(t *testing.T) { + for _, n := range []int{2, indexScanLimit + 4} { + m := New[untypedKey, int]() + for i := 0; i < n; i++ { + m.Set(untypedKey{Value: "same", Node: &yaml.Node{Line: i}}, i) + } + require.Equal(t, 0, m.untypedValueIndex().find("same")) + requireIndexMatchesScan(t, m, []string{"same", "other"}) + } +} + +// A nil map finds nothing, matching FindValueUntyped on a nil map. +func TestUntypedValueIndex_NilMap(t *testing.T) { + var m *Map[untypedKey, int] + x := m.untypedValueIndex() + require.Nil(t, x.find("anything")) + require.Nil(t, m.FindValueUntyped("anything")) +} + +func TestFormatsAsStructLiteral(t *testing.T) { + require.True(t, formatsAsStructLiteral(reflect.TypeFor[untypedKey]())) + require.True(t, formatsAsStructLiteral(reflect.TypeFor[plainKey]())) + require.False(t, formatsAsStructLiteral(reflect.TypeFor[*untypedKey]())) + require.False(t, formatsAsStructLiteral(reflect.TypeFor[string]())) + require.False(t, formatsAsStructLiteral(reflect.TypeFor[stringerKey]())) + require.False(t, formatsAsStructLiteral(reflect.TypeFor[errorKey]())) + require.False(t, formatsAsStructLiteral(reflect.TypeFor[formatterKey]())) +} + +func TestKeyNodeIndex_MatchesFindKeyNode(t *testing.T) { + require.Nil(t, (&keyNodeIndex{}).find("anything")) + + for _, n := range []int{1, indexScanLimit, indexScanLimit + 1, 64} { + mapNode := &yaml.Node{Kind: yaml.MappingNode} + for i := 0; i < n; i++ { + // every key appears twice, so first-match semantics are exercised. + for _, dup := range []int{0, 1} { + mapNode.Content = append(mapNode.Content, + &yaml.Node{Kind: yaml.ScalarNode, Value: "key-" + strconv.Itoa(i), Line: dup}, + &yaml.Node{Kind: yaml.ScalarNode, Value: "value"}) + } + } + x := &keyNodeIndex{mapNode: mapNode} + for _, probe := range append(indexProbes(n), "value") { + require.Same(t, findKeyNode(probe, mapNode), x.find(probe), "probe %q", probe) + } + } +} + +// scanOnlyLookup implements FindValueUntyped without the index hook, as a type outside this package would. +type scanOnlyLookup struct{ values map[string]any } + +func (s scanOnlyLookup) FindValueUntyped(key string) any { return s.values[key] } + +type untypedHolder struct { + value any + node *yaml.Node +} + +func (h untypedHolder) GetValueUntyped() any { return h.value } +func (h untypedHolder) GetValueNode() *yaml.Node { return h.node } + +type recordingBuilder struct{ lowValues map[string]any } + +func (r *recordingBuilder) AddYAMLNode(parent *yaml.Node, entry *nodes.NodeEntry) *yaml.Node { + r.lowValues[entry.Key] = entry.LowValue + return parent +} + +// ToYamlNode resolves low values through the index when the low map provides one, and through +// FindValueUntyped when it does not. +func TestToYamlNode_LowValueResolution(t *testing.T) { + high := New[string, string]() + for i := 0; i < indexScanLimit+2; i++ { + high.Set("key-"+strconv.Itoa(i), "v") + } + + lowMap := New[untypedKey, int]() + for i := 0; i < indexScanLimit+2; i++ { + lowMap.Set(untypedKey{Value: "key-" + strconv.Itoa(i)}, i) + } + + indexed := &recordingBuilder{lowValues: map[string]any{}} + high.ToYamlNode(indexed, untypedHolder{value: lowMap}) + scanned := &recordingBuilder{lowValues: map[string]any{}} + high.ToYamlNode(scanned, untypedHolder{value: scanOnlyLookup{values: map[string]any{"key-3": 3}}}) + nilLow := &recordingBuilder{lowValues: map[string]any{}} + high.ToYamlNode(nilLow, untypedHolder{value: (*Map[untypedKey, int])(nil)}) + + for i := 0; i < indexScanLimit+2; i++ { + key := "key-" + strconv.Itoa(i) + require.Equal(t, i, indexed.lowValues[key]) + require.Nil(t, nilLow.lowValues[key]) + } + require.Equal(t, 3, scanned.lowValues["key-3"]) + require.Nil(t, scanned.lowValues["key-4"]) +} diff --git a/overlay/engine.go b/overlay/engine.go index c58ac5b40..f793f2e7e 100644 --- a/overlay/engine.go +++ b/overlay/engine.go @@ -7,6 +7,7 @@ import ( "github.com/pb33f/jsonpath/pkg/jsonpath" "github.com/pb33f/jsonpath/pkg/jsonpath/config" highoverlay "github.com/pb33f/libopenapi/datamodel/high/overlay" + "github.com/pb33f/libopenapi/internal/jsonnode" "github.com/pb33f/libopenapi/utils" "go.yaml.in/yaml/v4" ) @@ -23,7 +24,7 @@ func Apply(targetBytes []byte, overlay *highoverlay.Overlay) (*Result, error) { } var rootNode yaml.Node - if err := yaml.Unmarshal(targetBytes, &rootNode); err != nil { + if err := jsonnode.Unmarshal(targetBytes, &rootNode); err != nil { return nil, err } diff --git a/pipeline_bench_test.go b/pipeline_bench_test.go new file mode 100644 index 000000000..883c88a46 --- /dev/null +++ b/pipeline_bench_test.go @@ -0,0 +1,208 @@ +// Copyright 2026 Princess B33f Heavy Industries / Dave Shanley +// SPDX-License-Identifier: MIT + +package libopenapi_test + +import ( + "bytes" + "io" + "log/slog" + "os" + "runtime" + "testing" + + "github.com/pb33f/libopenapi" + "github.com/pb33f/libopenapi/bundler" + "github.com/pb33f/libopenapi/datamodel" + "github.com/pb33f/libopenapi/datamodel/high/base" +) + +// Pipeline benchmarks exercise the full public entry points against large, real-world specifications. +// Each benchmark also reports "retained-B/op": the live heap left behind by one iteration's result after a +// forced GC, which is the memory a long-lived consumer pays to hold the model. + +func pipelineSpec(b *testing.B, path string) []byte { + b.Helper() + spec, err := os.ReadFile(path) + if err != nil { + b.Fatalf("read %s: %v", path, err) + } + return spec +} + +func pipelineConfig() *datamodel.DocumentConfiguration { + return &datamodel.DocumentConfiguration{ + IgnorePolymorphicCircularReferences: true, + IgnoreArrayCircularReferences: true, + // keep resolution errors out of the benchmark output, where they break benchstat parsing. + Logger: slog.New(slog.NewTextHandler(io.Discard, nil)), + } +} + +func heapInUse() uint64 { + runtime.GC() + runtime.GC() + var ms runtime.MemStats + runtime.ReadMemStats(&ms) + return ms.HeapAlloc +} + +func benchmarkPipelineV3(b *testing.B, path string) { + spec := pipelineSpec(b, path) + b.ReportAllocs() + b.ResetTimer() + var keep any + for i := 0; i < b.N; i++ { + doc, err := libopenapi.NewDocumentWithConfiguration(spec, pipelineConfig()) + if err != nil { + b.Fatal(err) + } + m, _ := doc.BuildV3Model() + if m == nil { + b.Fatal("nil model") + } + keep = m + } + b.StopTimer() + keep = nil + before := heapInUse() + doc, _ := libopenapi.NewDocumentWithConfiguration(spec, pipelineConfig()) + m, _ := doc.BuildV3Model() + keep = m + after := heapInUse() + runtime.KeepAlive(keep) + runtime.KeepAlive(doc) + b.ReportMetric(float64(after-before), "retained-B/op") +} + +func benchmarkPipelineV2(b *testing.B, path string) { + spec := pipelineSpec(b, path) + b.ReportAllocs() + b.ResetTimer() + var keep any + for i := 0; i < b.N; i++ { + doc, err := libopenapi.NewDocumentWithConfiguration(spec, pipelineConfig()) + if err != nil { + b.Fatal(err) + } + m, _ := doc.BuildV2Model() + if m == nil { + b.Fatal("nil model") + } + keep = m + } + b.StopTimer() + keep = nil + before := heapInUse() + doc, _ := libopenapi.NewDocumentWithConfiguration(spec, pipelineConfig()) + m, _ := doc.BuildV2Model() + keep = m + after := heapInUse() + runtime.KeepAlive(keep) + runtime.KeepAlive(doc) + b.ReportMetric(float64(after-before), "retained-B/op") +} + +func BenchmarkPipeline_BuildV3_Stripe(b *testing.B) { benchmarkPipelineV3(b, "test_specs/stripe.yaml") } +func BenchmarkPipeline_BuildV3_DocuSign(b *testing.B) { + benchmarkPipelineV3(b, "test_specs/docusignv3.1.json") +} +func BenchmarkPipeline_BuildV3_Asana(b *testing.B) { benchmarkPipelineV3(b, "test_specs/asana.yaml") } +func BenchmarkPipeline_BuildV2_K8s(b *testing.B) { benchmarkPipelineV2(b, "test_specs/k8s.json") } +func BenchmarkPipeline_BuildV2_Xsoar(b *testing.B) { benchmarkPipelineV2(b, "test_specs/xsoar.json") } + +func BenchmarkPipeline_Render_Stripe(b *testing.B) { + spec := pipelineSpec(b, "test_specs/stripe.yaml") + doc, err := libopenapi.NewDocumentWithConfiguration(spec, pipelineConfig()) + if err != nil { + b.Fatal(err) + } + if m, _ := doc.BuildV3Model(); m == nil { + b.Fatal("nil model") + } + b.ReportAllocs() + b.ResetTimer() + for i := 0; i < b.N; i++ { + out, rErr := doc.Render() + if rErr != nil || len(out) == 0 { + b.Fatal(rErr) + } + } +} + +func BenchmarkPipeline_Render_DocuSignJSON(b *testing.B) { + spec := pipelineSpec(b, "test_specs/docusignv3.1.json") + doc, err := libopenapi.NewDocumentWithConfiguration(spec, pipelineConfig()) + if err != nil { + b.Fatal(err) + } + if m, _ := doc.BuildV3Model(); m == nil { + b.Fatal("nil model") + } + b.ReportAllocs() + b.ResetTimer() + for i := 0; i < b.N; i++ { + out, rErr := doc.Render() + if rErr != nil || len(out) == 0 { + b.Fatal(rErr) + } + } +} + +func BenchmarkPipeline_Compare_Stripe(b *testing.B) { + left := pipelineSpec(b, "test_specs/stripe.yaml") + // mutate a slice of the spec so the comparison has real changes to report, not just an identity walk. + right := bytes.Replace(left, []byte("type: string"), []byte("type: integer"), 200) + right = bytes.Replace(right, []byte("nullable: true"), []byte("nullable: false"), 300) + b.ReportAllocs() + b.ResetTimer() + for i := 0; i < b.N; i++ { + l, err := libopenapi.NewDocumentWithConfiguration(left, pipelineConfig()) + if err != nil { + b.Fatal(err) + } + r, err := libopenapi.NewDocumentWithConfiguration(right, pipelineConfig()) + if err != nil { + b.Fatal(err) + } + changes, _ := libopenapi.CompareDocuments(l, r) + if changes == nil { + b.Fatal("no changes") + } + } +} + +func BenchmarkPipeline_Bundle_Stripe(b *testing.B) { + spec := pipelineSpec(b, "test_specs/stripe.yaml") + b.ReportAllocs() + b.ResetTimer() + for i := 0; i < b.N; i++ { + out, _ := bundler.BundleBytes(spec, pipelineConfig()) + if len(out) == 0 { + b.Fatal("empty bundle") + } + } +} + +// BenchmarkPipeline_RenderSchemasInline_Stripe renders every component schema inline in validation mode, +// the way request/response validators compile schemas. +func BenchmarkPipeline_RenderSchemasInline_Stripe(b *testing.B) { + spec := pipelineSpec(b, "test_specs/stripe.yaml") + doc, err := libopenapi.NewDocumentWithConfiguration(spec, pipelineConfig()) + if err != nil { + b.Fatal(err) + } + m, _ := doc.BuildV3Model() + if m == nil { + b.Fatal("nil model") + } + b.ReportAllocs() + b.ResetTimer() + for i := 0; i < b.N; i++ { + for _, proxy := range m.Model.Components.Schemas.FromOldest() { + if _, rErr := proxy.Schema().RenderInlineWithContext(base.NewInlineRenderContextForValidation()); rErr != nil { + b.Fatal(rErr) + } + } + } +} From 6de9f95315efccd28d73edbb7190e7ba9b28e082 Mon Sep 17 00:00:00 2001 From: quobix Date: Fri, 25 Sep 2026 19:29:17 -0400 Subject: [PATCH 2/3] perf(datamodel/low)!: index node lines lazily, drop NodeReference.Context Every low model kept its line -> node map in a sync.Map, so each node it recorded paid for a Load and a Store, a sync.Map entry and a boxed int key. Most models are built and never asked for their nodes. NodeLines records writes in a plain log under a mutex and builds the map[int]any the first time it is read. NodeReference.Context held a context.Context on every NodeReference built anywhere. Its only reader was PathItem.Build, which now keeps each operation's context in a map keyed by the operation for the one build that needs it. what-changed on the Stripe spec: -16% bytes, -22% allocs; document builds -2% to -7% bytes. Time is within noise. Outputs across the 72 golden fixtures are unchanged. BREAKING CHANGE: NodeMap.Nodes is a *low.NodeLines instead of a *sync.Map. Its Store, Load and Range take int line numbers (Range(func(line int, value any) bool) visits lines in ascending order). ExtractNodes and ExtractNodesRecursive return *low.NodeLines; ExtractExtensionNodes and MergeRecursiveNodesIfLineAbsent take one. NodeReference.Context is removed. Co-Authored-By: Claude Opus 5.5 --- datamodel/low/base/contact.go | 5 +- datamodel/low/base/example.go | 5 +- datamodel/low/base/external_doc.go | 5 +- datamodel/low/base/info.go | 5 +- datamodel/low/base/license.go | 5 +- datamodel/low/base/schema.go | 3 +- datamodel/low/base/schema_build.go | 3 +- .../low/base/schema_build_coverage_test.go | 6 +- datamodel/low/base/schema_proxy.go | 10 +- datamodel/low/base/security_requirement.go | 5 +- datamodel/low/base/xml.go | 5 +- datamodel/low/node_lines_test.go | 97 ++++++++++++ datamodel/low/node_map.go | 144 ++++++++++++++---- datamodel/low/node_map_merge.go | 4 +- datamodel/low/node_map_merge_test.go | 7 +- datamodel/low/node_map_test.go | 29 ++-- datamodel/low/reference.go | 2 - datamodel/low/v3/callback.go | 5 +- datamodel/low/v3/components.go | 4 +- datamodel/low/v3/components_test.go | 3 +- datamodel/low/v3/encoding.go | 5 +- datamodel/low/v3/header.go | 5 +- datamodel/low/v3/link.go | 5 +- datamodel/low/v3/media_type.go | 5 +- datamodel/low/v3/oauth_flows.go | 9 +- datamodel/low/v3/operation.go | 5 +- datamodel/low/v3/parameter.go | 5 +- datamodel/low/v3/path_item.go | 14 +- datamodel/low/v3/paths.go | 5 +- datamodel/low/v3/request_body.go | 5 +- datamodel/low/v3/response.go | 5 +- datamodel/low/v3/responses.go | 5 +- datamodel/low/v3/security_scheme.go | 5 +- 33 files changed, 295 insertions(+), 135 deletions(-) create mode 100644 datamodel/low/node_lines_test.go diff --git a/datamodel/low/base/contact.go b/datamodel/low/base/contact.go index 8b33c1f5f..ce952cc3a 100644 --- a/datamodel/low/base/contact.go +++ b/datamodel/low/base/contact.go @@ -6,7 +6,6 @@ package base import ( "context" "hash/maphash" - "sync" "github.com/pb33f/libopenapi/datamodel/low" "github.com/pb33f/libopenapi/index" @@ -27,7 +26,7 @@ type Contact struct { RootNode *yaml.Node index *index.SpecIndex context context.Context - nodeStore sync.Map + nodeStore low.NodeLines reference low.Reference *low.Reference low.NodeMap @@ -38,7 +37,7 @@ func (c *Contact) Build(ctx context.Context, keyNode, root *yaml.Node, idx *inde c.RootNode = root c.reference = low.Reference{} c.Reference = &c.reference - c.nodeStore = sync.Map{} + c.nodeStore = low.NodeLines{} c.Nodes = &c.nodeStore if root == nil { c.Extensions = nil diff --git a/datamodel/low/base/example.go b/datamodel/low/base/example.go index f6b81075c..ca2b608b1 100644 --- a/datamodel/low/base/example.go +++ b/datamodel/low/base/example.go @@ -6,7 +6,6 @@ package base import ( "context" "hash/maphash" - "sync" "github.com/pb33f/libopenapi/datamodel/low" "github.com/pb33f/libopenapi/index" @@ -30,7 +29,7 @@ type Example struct { RootNode *yaml.Node index *index.SpecIndex context context.Context - nodeStore sync.Map + nodeStore low.NodeLines reference low.Reference *low.Reference low.NodeMap @@ -97,7 +96,7 @@ func (ex *Example) Build(ctx context.Context, keyNode, root *yaml.Node, idx *ind root = utils.NodeAlias(root) ex.RootNode = root utils.CheckForMergeNodes(root) - ex.nodeStore = sync.Map{} + ex.nodeStore = low.NodeLines{} ex.Nodes = &ex.nodeStore if len(root.Content) > 0 { ex.NodeMap.ExtractNodes(root, false) diff --git a/datamodel/low/base/external_doc.go b/datamodel/low/base/external_doc.go index b9054a8c6..13c477e46 100644 --- a/datamodel/low/base/external_doc.go +++ b/datamodel/low/base/external_doc.go @@ -6,7 +6,6 @@ package base import ( "context" "hash/maphash" - "sync" "github.com/pb33f/libopenapi/datamodel/low" "github.com/pb33f/libopenapi/index" @@ -29,7 +28,7 @@ type ExternalDoc struct { RootNode *yaml.Node index *index.SpecIndex context context.Context - nodeStore sync.Map + nodeStore low.NodeLines reference low.Reference *low.Reference low.NodeMap @@ -55,7 +54,7 @@ func (ex *ExternalDoc) Build(ctx context.Context, keyNode, root *yaml.Node, idx ex.KeyNode = keyNode ex.reference = low.Reference{} ex.Reference = &ex.reference - ex.nodeStore = sync.Map{} + ex.nodeStore = low.NodeLines{} ex.Nodes = &ex.nodeStore ex.context = ctx ex.index = idx diff --git a/datamodel/low/base/info.go b/datamodel/low/base/info.go index 8cb2dc58f..90df059a7 100644 --- a/datamodel/low/base/info.go +++ b/datamodel/low/base/info.go @@ -6,7 +6,6 @@ package base import ( "context" "hash/maphash" - "sync" "github.com/pb33f/libopenapi/orderedmap" "github.com/pb33f/libopenapi/utils" @@ -36,7 +35,7 @@ type Info struct { RootNode *yaml.Node index *index.SpecIndex context context.Context - nodeStore sync.Map + nodeStore low.NodeLines reference low.Reference *low.Reference low.NodeMap @@ -67,7 +66,7 @@ func (i *Info) Build(ctx context.Context, keyNode, root *yaml.Node, idx *index.S i.KeyNode = keyNode i.reference = low.Reference{} i.Reference = &i.reference - i.nodeStore = sync.Map{} + i.nodeStore = low.NodeLines{} i.Nodes = &i.nodeStore i.index = idx i.context = ctx diff --git a/datamodel/low/base/license.go b/datamodel/low/base/license.go index c6ec47a19..147b8653b 100644 --- a/datamodel/low/base/license.go +++ b/datamodel/low/base/license.go @@ -6,7 +6,6 @@ package base import ( "context" "hash/maphash" - "sync" "github.com/pb33f/libopenapi/datamodel/low" "github.com/pb33f/libopenapi/index" @@ -28,7 +27,7 @@ type License struct { RootNode *yaml.Node index *index.SpecIndex context context.Context - nodeStore sync.Map + nodeStore low.NodeLines reference low.Reference *low.Reference low.NodeMap @@ -39,7 +38,7 @@ func (l *License) Build(ctx context.Context, keyNode, root *yaml.Node, idx *inde l.KeyNode = keyNode l.reference = low.Reference{} l.Reference = &l.reference - l.nodeStore = sync.Map{} + l.nodeStore = low.NodeLines{} l.Nodes = &l.nodeStore l.context = ctx l.index = idx diff --git a/datamodel/low/base/schema.go b/datamodel/low/base/schema.go index 36074ac30..463e25ee4 100644 --- a/datamodel/low/base/schema.go +++ b/datamodel/low/base/schema.go @@ -2,7 +2,6 @@ package base import ( "context" - "sync" "github.com/pb33f/libopenapi/datamodel/low" "github.com/pb33f/libopenapi/index" @@ -161,7 +160,7 @@ type Schema struct { refIndex *index.SpecIndex context context.Context - nodeStore sync.Map + nodeStore low.NodeLines reference low.Reference *low.Reference low.NodeMap diff --git a/datamodel/low/base/schema_build.go b/datamodel/low/base/schema_build.go index 86526a601..bda3b422a 100644 --- a/datamodel/low/base/schema_build.go +++ b/datamodel/low/base/schema_build.go @@ -7,7 +7,6 @@ import ( "context" "fmt" "strconv" - "sync" "github.com/pb33f/libopenapi/datamodel/low" "github.com/pb33f/libopenapi/index" @@ -49,7 +48,7 @@ func (s *Schema) Build(ctx context.Context, root *yaml.Node, idx *index.SpecInde s.reference = low.Reference{} s.Reference = &s.reference - s.nodeStore = sync.Map{} + s.nodeStore = low.NodeLines{} s.Nodes = &s.nodeStore if root != nil && len(root.Content) > 0 { s.NodeMap.ExtractNodes(root, false) diff --git a/datamodel/low/base/schema_build_coverage_test.go b/datamodel/low/base/schema_build_coverage_test.go index f6f06b2bb..899f82047 100644 --- a/datamodel/low/base/schema_build_coverage_test.go +++ b/datamodel/low/base/schema_build_coverage_test.go @@ -217,7 +217,7 @@ func TestRecursiveSchemaNodeHelpers(t *testing.T) { require.NoError(t, yaml.Unmarshal([]byte("example:\n nested:\n value: ok\n"), &root)) node := root.Content[0] - var dst sync.Map + var dst low.NodeLines blockedLine := node.Content[0].Line dst.Store(blockedLine, []*yaml.Node{{Value: "existing"}}) @@ -227,8 +227,8 @@ func TestRecursiveSchemaNodeHelpers(t *testing.T) { assert.True(t, blocked) var foundNested bool - dst.Range(func(key, value any) bool { - if key.(int) == node.Content[1].Content[0].Line { + dst.Range(func(key int, value any) bool { + if key == node.Content[1].Content[0].Line { foundNested = true } assert.NotNil(t, value) diff --git a/datamodel/low/base/schema_proxy.go b/datamodel/low/base/schema_proxy.go index 3b8a54548..375ad8c60 100644 --- a/datamodel/low/base/schema_proxy.go +++ b/datamodel/low/base/schema_proxy.go @@ -67,7 +67,7 @@ type SchemaProxy struct { hashMu sync.Mutex // protects cachedHash + hashGen cachedHash *uint64 // protected by hashMu hashGen uint64 // generation counter for invalidation - nodeStore sync.Map + nodeStore low.NodeLines nodeMap low.NodeMap TransformedRef *yaml.Node // Original node that contained the ref before transformation transformedRef *transformedSiblingRef @@ -103,7 +103,7 @@ func (sp *SchemaProxy) Build(ctx context.Context, key, value *yaml.Node, idx *in } // for transformed schemas, don't set reference since it's now an allOf structure // the reference is embedded within the allOf, but the schema itself is not a pure reference - sp.nodeStore = sync.Map{} + sp.nodeStore = low.NodeLines{} sp.nodeMap = low.NodeMap{Nodes: &sp.nodeStore} sp.NodeMap = &sp.nodeMap return nil @@ -133,7 +133,7 @@ func (sp *SchemaProxy) prepareForResolvedBuild(ctx context.Context, key, value, if refLocation != "" { sp.SetReference(refLocation, refNode) } - sp.nodeStore = sync.Map{} + sp.nodeStore = low.NodeLines{} sp.nodeMap = low.NodeMap{Nodes: &sp.nodeStore} sp.NodeMap = &sp.nodeMap } @@ -255,8 +255,8 @@ func (sp *SchemaProxy) Schema() *Schema { // Copy accumulated nodes to the built schema if sp.NodeMap != nil { - sp.NodeMap.Nodes.Range(func(key, value any) bool { - schema.AddNode(key.(int), value.(*yaml.Node)) + sp.NodeMap.Nodes.Range(func(line int, value any) bool { + schema.AddNode(line, value.(*yaml.Node)) return true }) } diff --git a/datamodel/low/base/security_requirement.go b/datamodel/low/base/security_requirement.go index ba2fa5cbe..2a7adbb8f 100644 --- a/datamodel/low/base/security_requirement.go +++ b/datamodel/low/base/security_requirement.go @@ -7,7 +7,6 @@ import ( "context" "hash/maphash" "sort" - "sync" "github.com/pb33f/libopenapi/datamodel/low" "github.com/pb33f/libopenapi/index" @@ -31,7 +30,7 @@ type SecurityRequirement struct { ContainsEmptyRequirement bool // if a requirement is empty (this means it's optional) index *index.SpecIndex context context.Context - nodeStore sync.Map + nodeStore low.NodeLines reference low.Reference *low.Reference low.NodeMap @@ -52,7 +51,7 @@ func (s *SecurityRequirement) Build(ctx context.Context, keyNode, root *yaml.Nod s.KeyNode = keyNode s.reference = low.Reference{} s.Reference = &s.reference - s.nodeStore = sync.Map{} + s.nodeStore = low.NodeLines{} s.Nodes = &s.nodeStore s.context = ctx s.index = idx diff --git a/datamodel/low/base/xml.go b/datamodel/low/base/xml.go index 29d383afa..a73249472 100644 --- a/datamodel/low/base/xml.go +++ b/datamodel/low/base/xml.go @@ -6,7 +6,6 @@ package base import ( "context" "hash/maphash" - "sync" "github.com/pb33f/libopenapi/datamodel/low" "github.com/pb33f/libopenapi/index" @@ -35,7 +34,7 @@ type XML struct { RootNode *yaml.Node index *index.SpecIndex context context.Context - nodeStore sync.Map + nodeStore low.NodeLines reference low.Reference *low.Reference low.NodeMap @@ -45,7 +44,7 @@ type XML struct { func (x *XML) Build(root *yaml.Node, idx *index.SpecIndex) error { x.reference = low.Reference{} x.Reference = &x.reference - x.nodeStore = sync.Map{} + x.nodeStore = low.NodeLines{} x.Nodes = &x.nodeStore x.index = idx if root == nil { diff --git a/datamodel/low/node_lines_test.go b/datamodel/low/node_lines_test.go new file mode 100644 index 000000000..cfea56b09 --- /dev/null +++ b/datamodel/low/node_lines_test.go @@ -0,0 +1,97 @@ +// Copyright 2026 Princess B33f Heavy Industries / Dave Shanley +// SPDX-License-Identifier: MIT + +package low + +import ( + "sync" + "testing" + + "github.com/pb33f/testify/assert" + "go.yaml.in/yaml/v4" +) + +// writeLines applies the same writes to a NodeLines: a stored single node that a later add turns into a +// pair, a line built up by adds, a stored value that a later add leaves alone, and a stored slice that a +// later Store replaces. +func writeLines(n *NodeLines, a, b, c *yaml.Node) { + n.Store(1, a) + n.add(1, b) + n.add(2, a) + n.add(2, b) + n.add(2, c) + n.Store(3, "not a node") + n.add(3, a) + n.Store(4, []*yaml.Node{a}) + n.Store(4, []*yaml.Node{c}) +} + +// Writes give the same result whether they are recorded before the line index is built or applied to it. +func TestNodeLines_WritesBeforeAndAfterIndexing(t *testing.T) { + a, b, c := &yaml.Node{Value: "a"}, &yaml.Node{Value: "b"}, &yaml.Node{Value: "c"} + + var recorded NodeLines + writeLines(&recorded, a, b, c) + + var applied NodeLines + _, ok := applied.Load(1) // builds the empty index, so every write below is applied directly + assert.False(t, ok) + writeLines(&applied, a, b, c) + + for _, n := range []*NodeLines{&recorded, &applied} { + one, _ := n.Load(1) + assert.Equal(t, []*yaml.Node{a, b}, one) + two, _ := n.Load(2) + assert.Equal(t, []*yaml.Node{a, b, c}, two) + three, _ := n.Load(3) + assert.Equal(t, "not a node", three) + four, _ := n.Load(4) + assert.Equal(t, []*yaml.Node{c}, four) + } +} + +// Range visits lines in order, stops when asked, and lets the callback write to the nodes it ranges over. +func TestNodeLines_Range(t *testing.T) { + var n NodeLines + for _, line := range []int{30, 10, 20} { + n.Store(line, &yaml.Node{Line: line}) + } + + var visited []int + n.Range(func(line int, _ any) bool { + visited = append(visited, line) + n.Store(line+1, &yaml.Node{}) // writing mid-range must not deadlock + return line < 20 + }) + assert.Equal(t, []int{10, 20}, visited) + + _, written := n.Load(11) + assert.True(t, written) +} + +// NodeLines is safe for concurrent writes and reads. +func TestNodeLines_Concurrent(t *testing.T) { + var n NodeLines + var wg sync.WaitGroup + for g := 0; g < 8; g++ { + wg.Add(1) + go func(g int) { + defer wg.Done() + for i := 0; i < 100; i++ { + n.add(i, &yaml.Node{Line: i}) + if i%10 == g { + n.Load(i) + } + } + }(g) + } + wg.Wait() + + lines := 0 + n.Range(func(_ int, value any) bool { + assert.Len(t, value, 8) + lines++ + return true + }) + assert.Equal(t, 100, lines) +} diff --git a/datamodel/low/node_map.go b/datamodel/low/node_map.go index b2b1d8bb0..99dc13bdc 100644 --- a/datamodel/low/node_map.go +++ b/datamodel/low/node_map.go @@ -6,6 +6,7 @@ package low import ( "context" + "slices" "sync" "github.com/pb33f/libopenapi/orderedmap" @@ -22,38 +23,130 @@ type AddNodes interface { AddNode(key int, node *yaml.Node) } +// NodeLines holds the yaml nodes of a model object by line number. A line holds a *yaml.Node, or a +// []*yaml.Node when several nodes share it (as they do in JSON). +// +// Node maps are filled for every object built but read only by tools that map lines back to objects, so +// writes are recorded as they happen and the line index is built when the nodes are first read. +// NodeLines is safe for concurrent use, and its zero value is ready to use. +type NodeLines struct { + mu sync.Mutex + pending []nodeLineWrite + lines map[int]any +} + +type nodeLineWrite struct { + line int + value any + add bool // add the node to the line rather than replace the line's value +} + +// Store sets the value held for a line. +func (n *NodeLines) Store(line int, value any) { + n.write(nodeLineWrite{line: line, value: value}) +} + +// Load returns the value held for a line, and whether the line holds one. +func (n *NodeLines) Load(line int) (any, bool) { + n.mu.Lock() + defer n.mu.Unlock() + value, ok := n.index()[line] + return value, ok +} + +// Range calls f for each line and its value, in line order, until f returns false. The lines are read +// before f is first called, so f may write to the nodes. +func (n *NodeLines) Range(f func(line int, value any) bool) { + n.mu.Lock() + index := n.index() + lines := make([]int, 0, len(index)) + for line := range index { + lines = append(lines, line) + } + slices.Sort(lines) + values := make([]any, len(lines)) + for i, line := range lines { + values[i] = index[line] + } + n.mu.Unlock() + for i, line := range lines { + if !f(line, values[i]) { + return + } + } +} + +// add appends a node to a line. +func (n *NodeLines) add(line int, node *yaml.Node) { + n.write(nodeLineWrite{line: line, value: node, add: true}) +} + +func (n *NodeLines) write(w nodeLineWrite) { + n.mu.Lock() + defer n.mu.Unlock() + if n.lines != nil { + n.apply(w) + return + } + if n.pending == nil { + n.pending = make([]nodeLineWrite, 0, 8) + } + n.pending = append(n.pending, w) +} + +// index builds the line index from the recorded writes, once. The caller holds the lock. +func (n *NodeLines) index() map[int]any { + if n.lines == nil { + n.lines = make(map[int]any, len(n.pending)) + for _, w := range n.pending { + n.apply(w) + } + n.pending = nil + } + return n.lines +} + +func (n *NodeLines) apply(w nodeLineWrite) { + if !w.add { + n.lines[w.line] = w.value + return + } + node := w.value.(*yaml.Node) + existing, ok := n.lines[w.line] + if !ok { + n.lines[w.line] = []*yaml.Node{node} + return + } + switch ext := existing.(type) { + case *yaml.Node: + n.lines[w.line] = []*yaml.Node{ext, node} + case []*yaml.Node: + n.lines[w.line] = append(ext, node) + } +} + // NodeMap represents a map of yaml nodes type NodeMap struct { - // Nodes is a sync map of nodes for this object, and the key is the line number of the node - // a line can contain many nodes (in JSON), so the value is a slice of *yaml.Node - Nodes *sync.Map `yaml:"-" json:"-"` + // Nodes holds the nodes of this object by line number. A line can contain many nodes (in JSON), so + // a line's value is a *yaml.Node or a []*yaml.Node. + Nodes *NodeLines `yaml:"-" json:"-"` } // AddNode will add a node to the NodeMap func (nm *NodeMap) AddNode(key int, node *yaml.Node) { - if existing, ok := nm.Nodes.Load(key); ok { - if ext, ko := existing.(*yaml.Node); ko { - nm.Nodes.Store(key, []*yaml.Node{ext, node}) - } - if ext, ko := existing.([]*yaml.Node); ko { - ext = append(ext, node) - nm.Nodes.Store(key, ext) - } - } else { - nm.Nodes.Store(key, []*yaml.Node{node}) - } + nm.Nodes.add(key, node) } // GetNodes will return the map of nodes func (nm *NodeMap) GetNodes() map[int][]*yaml.Node { composed := make(map[int][]*yaml.Node) if nm.Nodes != nil { - nm.Nodes.Range(func(key, value interface{}) bool { + nm.Nodes.Range(func(line int, value any) bool { if v, ok := value.([]*yaml.Node); ok { - composed[key.(int)] = v + composed[line] = v } if v, ok := value.(*yaml.Node); ok { - composed[key.(int)] = []*yaml.Node{v} + composed[line] = []*yaml.Node{v} } return true @@ -95,9 +188,8 @@ func (nm *NodeMap) ContainsLine(line int) bool { } // ExtractNodes will extract all nodes from a yaml.Node and return them in a map -func ExtractNodes(_ context.Context, root *yaml.Node) *sync.Map { - var syncMap sync.Map - nm := &NodeMap{Nodes: &syncMap} +func ExtractNodes(_ context.Context, root *yaml.Node) *NodeLines { + nm := &NodeMap{Nodes: &NodeLines{}} if root != nil && len(root.Content) > 0 { nm.ExtractNodes(root, false) } else { @@ -111,9 +203,8 @@ func ExtractNodes(_ context.Context, root *yaml.Node) *sync.Map { // ExtractNodesRecursive will extract all nodes from a yaml.Node and return them in a map, just like ExtractNodes // however, this version will dive-down the tree and extract all nodes from all child nodes as well until the tree // is done. -func ExtractNodesRecursive(_ context.Context, root *yaml.Node) *sync.Map { - var syncMap sync.Map - nm := &NodeMap{Nodes: &syncMap} +func ExtractNodesRecursive(_ context.Context, root *yaml.Node) *NodeLines { + nm := &NodeMap{Nodes: &NodeLines{}} nm.ExtractNodes(root, true) return nm.Nodes } @@ -121,16 +212,15 @@ func ExtractNodesRecursive(_ context.Context, root *yaml.Node) *sync.Map { // ExtractExtensionNodes will extract all extension nodes from a map of extensions, recursively. func ExtractExtensionNodes(_ context.Context, extensionMap *orderedmap.Map[KeyReference[string], - ValueReference[*yaml.Node]], nodeMap *sync.Map, + ValueReference[*yaml.Node]], nodeMap *NodeLines, ) { // range over the extension map and extract all nodes for k, v := range extensionMap.FromOldest() { results := []*yaml.Node{k.KeyNode} - var newNodeMap sync.Map - nm := &NodeMap{Nodes: &newNodeMap} + nm := &NodeMap{Nodes: &NodeLines{}} if len(v.ValueNode.Content) > 0 { nm.ExtractNodes(v.ValueNode, true) - nm.Nodes.Range(func(key, value interface{}) bool { + nm.Nodes.Range(func(_ int, value any) bool { for _, n := range value.([]*yaml.Node) { results = append(results, n) } diff --git a/datamodel/low/node_map_merge.go b/datamodel/low/node_map_merge.go index ec6794d58..73bc739b7 100644 --- a/datamodel/low/node_map_merge.go +++ b/datamodel/low/node_map_merge.go @@ -4,14 +4,12 @@ package low import ( - "sync" - "go.yaml.in/yaml/v4" ) // MergeRecursiveNodesIfLineAbsent walks a node tree and adds each discovered node to dst // unless that line already exists in the destination map. -func MergeRecursiveNodesIfLineAbsent(dst *sync.Map, node *yaml.Node) { +func MergeRecursiveNodesIfLineAbsent(dst *NodeLines, node *yaml.Node) { if dst == nil || node == nil { return } diff --git a/datamodel/low/node_map_merge_test.go b/datamodel/low/node_map_merge_test.go index 9635fcc04..f9bf26103 100644 --- a/datamodel/low/node_map_merge_test.go +++ b/datamodel/low/node_map_merge_test.go @@ -4,7 +4,6 @@ package low import ( - "sync" "testing" "github.com/pb33f/testify/assert" @@ -29,7 +28,7 @@ func TestNodeMapMergeHelpers(t *testing.T) { require.NoError(t, yaml.Unmarshal([]byte("example:\n nested:\n value: ok\n"), &root)) node := root.Content[0] - var dst sync.Map + var dst NodeLines blockedLine := node.Content[0].Line dst.Store(blockedLine, []*yaml.Node{{Value: "existing"}}) @@ -39,8 +38,8 @@ func TestNodeMapMergeHelpers(t *testing.T) { assert.True(t, blocked) var foundNested bool - dst.Range(func(key, value any) bool { - if key.(int) == node.Content[1].Content[0].Line { + dst.Range(func(key int, value any) bool { + if key == node.Content[1].Content[0].Line { foundNested = true } assert.NotNil(t, value) diff --git a/datamodel/low/node_map_test.go b/datamodel/low/node_map_test.go index a16b925d0..f60f0b961 100644 --- a/datamodel/low/node_map_test.go +++ b/datamodel/low/node_map_test.go @@ -4,7 +4,6 @@ package low import ( - "sync" "testing" "github.com/pb33f/testify/assert" @@ -22,7 +21,7 @@ four: var root yaml.Node _ = yaml.Unmarshal([]byte(yml), &root) - var syncMap sync.Map + var syncMap NodeLines nm := &NodeMap{Nodes: &syncMap} nm.ExtractNodes(root.Content[0], false) testTheThing(t, nm) @@ -30,7 +29,7 @@ four: func testTheThing(t *testing.T, nm *NodeMap) { count := 0 - nm.Nodes.Range(func(key, value interface{}) bool { + nm.Nodes.Range(func(key int, value any) bool { count++ return true }) @@ -52,7 +51,7 @@ func testTheThing(t *testing.T, nm *NodeMap) { assert.Equal(t, "four", nodes[4][0].Value) } -func testTheThingUnmarshalled(t *testing.T, nm *sync.Map) { +func testTheThingUnmarshalled(t *testing.T, nm *NodeLines) { n := &NodeMap{Nodes: nm} nodes := n.GetNodes() @@ -84,7 +83,7 @@ four: nm := ExtractNodes(nil, root.Content[0]) count := 0 - nm.Range(func(key, value interface{}) bool { + nm.Range(func(key int, value any) bool { count++ return true }) @@ -108,7 +107,7 @@ four: nm := ExtractNodesRecursive(nil, root.Content[0]) count := 0 - nm.Range(func(key, value interface{}) bool { + nm.Range(func(key int, value any) bool { count++ return true }) @@ -118,12 +117,12 @@ four: } func TestExtractNodes_Nil(t *testing.T) { - var syncMap sync.Map + var syncMap NodeLines nm := &NodeMap{Nodes: &syncMap} nm.ExtractNodes(nil, false) count := 0 - nm.Nodes.Range(func(key, value interface{}) bool { + nm.Nodes.Range(func(key int, value any) bool { count++ return true }) @@ -142,7 +141,7 @@ four: var root yaml.Node _ = yaml.Unmarshal([]byte(yml), &root) - var syncMap sync.Map + var syncMap NodeLines nm := &NodeMap{Nodes: &syncMap} syncMap.Store(1, root.Content[0]) @@ -151,7 +150,7 @@ four: } func Test_NodeMapGetNodes_SingleNode(t *testing.T) { - var syncMap sync.Map + var syncMap NodeLines nm := &NodeMap{Nodes: &syncMap} syncMap.Store(1, &yaml.Node{}) @@ -171,7 +170,7 @@ four: var root yaml.Node _ = yaml.Unmarshal([]byte(yml), &root) - var syncMap sync.Map + var syncMap NodeLines nm := &NodeMap{Nodes: &syncMap} nm.ExtractNodes(root.Content[0], true) @@ -185,7 +184,7 @@ four: } func Test_NodeMapGetNodes_EmptyNode(t *testing.T) { - var syncMap sync.Map + var syncMap NodeLines nm := &NodeMap{Nodes: &syncMap} ex := nm.GetNodes() @@ -206,11 +205,11 @@ x-rice: _ = yaml.Unmarshal([]byte(yml), &root) extensions := ExtractExtensions(root.Content[0]) - var syncMap sync.Map + var syncMap NodeLines ExtractExtensionNodes(nil, extensions, &syncMap) count := 0 - syncMap.Range(func(key, value interface{}) bool { + syncMap.Range(func(key int, value any) bool { count++ return true }) @@ -227,7 +226,7 @@ func TestExtractNodes_NoContent(t *testing.T) { nm := ExtractNodes(nil, root.Content[0]) count := 0 - nm.Range(func(key, value interface{}) bool { + nm.Range(func(key int, value any) bool { count++ return true }) diff --git a/datamodel/low/reference.go b/datamodel/low/reference.go index c7d1cc19d..47a1d808b 100644 --- a/datamodel/low/reference.go +++ b/datamodel/low/reference.go @@ -122,8 +122,6 @@ type NodeReference[T any] struct { // The yaml.Node that is the key, that contains the value. KeyNode *yaml.Node - - Context context.Context } var _ HasValueNodeUntyped = &NodeReference[any]{} diff --git a/datamodel/low/v3/callback.go b/datamodel/low/v3/callback.go index 1642b3764..2a29e5e24 100644 --- a/datamodel/low/v3/callback.go +++ b/datamodel/low/v3/callback.go @@ -6,7 +6,6 @@ package v3 import ( "context" "hash/maphash" - "sync" "github.com/pb33f/libopenapi/orderedmap" "github.com/pb33f/libopenapi/utils" @@ -30,7 +29,7 @@ type Callback struct { RootNode *yaml.Node index *index.SpecIndex context context.Context - nodeStore sync.Map + nodeStore low.NodeLines reference low.Reference *low.Reference low.NodeMap @@ -77,7 +76,7 @@ func (cb *Callback) Build(ctx context.Context, keyNode, root *yaml.Node, idx *in root = utils.NodeAlias(root) cb.RootNode = root utils.CheckForMergeNodes(root) - cb.nodeStore = sync.Map{} + cb.nodeStore = low.NodeLines{} cb.Nodes = &cb.nodeStore if len(root.Content) > 0 { cb.NodeMap.ExtractNodes(root, false) diff --git a/datamodel/low/v3/components.go b/datamodel/low/v3/components.go index ce6dbdb06..65974ba6b 100644 --- a/datamodel/low/v3/components.go +++ b/datamodel/low/v3/components.go @@ -42,7 +42,7 @@ type Components struct { RootNode *yaml.Node index *index.SpecIndex context context.Context - nodeStore sync.Map + nodeStore low.NodeLines reference low.Reference *low.Reference low.NodeMap @@ -175,7 +175,7 @@ func (co *Components) Build(ctx context.Context, root *yaml.Node, idx *index.Spe utils.CheckForMergeNodes(root) co.reference = low.Reference{} co.Reference = &co.reference - co.nodeStore = sync.Map{} + co.nodeStore = low.NodeLines{} co.Nodes = &co.nodeStore if len(root.Content) > 0 { co.NodeMap.ExtractNodes(root, false) diff --git a/datamodel/low/v3/components_test.go b/datamodel/low/v3/components_test.go index 2c6e48feb..a6d2c1e72 100644 --- a/datamodel/low/v3/components_test.go +++ b/datamodel/low/v3/components_test.go @@ -6,7 +6,6 @@ package v3 import ( "context" "fmt" - "sync" "testing" "github.com/pb33f/libopenapi/datamodel/low" @@ -273,7 +272,7 @@ components: idx := index.NewSpecIndex(&idxNode) _, _, compNode := utils.FindKeyNodeFullTop(ComponentsLabel, idxNode.Content[0].Content) - var nodeStore sync.Map + var nodeStore low.NodeLines components := &Components{} components.Nodes = &nodeStore diff --git a/datamodel/low/v3/encoding.go b/datamodel/low/v3/encoding.go index 8f1d1dd73..e5f15399b 100644 --- a/datamodel/low/v3/encoding.go +++ b/datamodel/low/v3/encoding.go @@ -7,7 +7,6 @@ import ( "context" "fmt" "hash/maphash" - "sync" "github.com/pb33f/libopenapi/datamodel/low" "github.com/pb33f/libopenapi/index" @@ -28,7 +27,7 @@ type Encoding struct { RootNode *yaml.Node index *index.SpecIndex context context.Context - nodeStore sync.Map + nodeStore low.NodeLines reference low.Reference *low.Reference low.NodeMap @@ -88,7 +87,7 @@ func (en *Encoding) Build(ctx context.Context, keyNode, root *yaml.Node, idx *in root = utils.NodeAlias(root) en.RootNode = root utils.CheckForMergeNodes(root) - en.nodeStore = sync.Map{} + en.nodeStore = low.NodeLines{} en.Nodes = &en.nodeStore if len(root.Content) > 0 { en.NodeMap.ExtractNodes(root, false) diff --git a/datamodel/low/v3/header.go b/datamodel/low/v3/header.go index 02929e25b..76149443a 100644 --- a/datamodel/low/v3/header.go +++ b/datamodel/low/v3/header.go @@ -7,7 +7,6 @@ import ( "context" "fmt" "hash/maphash" - "sync" "github.com/pb33f/libopenapi/datamodel/low" "github.com/pb33f/libopenapi/datamodel/low/base" @@ -36,7 +35,7 @@ type Header struct { RootNode *yaml.Node index *index.SpecIndex context context.Context - nodeStore sync.Map + nodeStore low.NodeLines reference low.Reference *low.Reference low.NodeMap @@ -138,7 +137,7 @@ func (h *Header) Build(ctx context.Context, keyNode, root *yaml.Node, idx *index root = utils.NodeAlias(root) h.RootNode = root utils.CheckForMergeNodes(root) - h.nodeStore = sync.Map{} + h.nodeStore = low.NodeLines{} h.Nodes = &h.nodeStore if len(root.Content) > 0 { h.NodeMap.ExtractNodes(root, false) diff --git a/datamodel/low/v3/link.go b/datamodel/low/v3/link.go index 5e262c989..290f760b6 100644 --- a/datamodel/low/v3/link.go +++ b/datamodel/low/v3/link.go @@ -6,7 +6,6 @@ package v3 import ( "context" "hash/maphash" - "sync" "github.com/pb33f/libopenapi/datamodel/low" "github.com/pb33f/libopenapi/index" @@ -39,7 +38,7 @@ type Link struct { RootNode *yaml.Node index *index.SpecIndex context context.Context - nodeStore sync.Map + nodeStore low.NodeLines reference low.Reference *low.Reference low.NodeMap @@ -91,7 +90,7 @@ func (l *Link) Build(ctx context.Context, keyNode, root *yaml.Node, idx *index.S root = utils.NodeAlias(root) l.RootNode = root utils.CheckForMergeNodes(root) - l.nodeStore = sync.Map{} + l.nodeStore = low.NodeLines{} l.Nodes = &l.nodeStore if len(root.Content) > 0 { l.NodeMap.ExtractNodes(root, false) diff --git a/datamodel/low/v3/media_type.go b/datamodel/low/v3/media_type.go index 122e027d5..ef1d37b3b 100644 --- a/datamodel/low/v3/media_type.go +++ b/datamodel/low/v3/media_type.go @@ -7,7 +7,6 @@ import ( "context" "hash/maphash" "slices" - "sync" "github.com/pb33f/libopenapi/datamodel/low" "github.com/pb33f/libopenapi/datamodel/low/base" @@ -33,7 +32,7 @@ type MediaType struct { RootNode *yaml.Node index *index.SpecIndex context context.Context - nodeStore sync.Map + nodeStore low.NodeLines reference low.Reference *low.Reference low.NodeMap @@ -92,7 +91,7 @@ func (mt *MediaType) Build(ctx context.Context, keyNode, root *yaml.Node, idx *i utils.CheckForMergeNodes(root) mt.reference = low.Reference{} mt.Reference = &mt.reference - mt.nodeStore = sync.Map{} + mt.nodeStore = low.NodeLines{} mt.Nodes = &mt.nodeStore if len(root.Content) > 0 { mt.NodeMap.ExtractNodes(root, false) diff --git a/datamodel/low/v3/oauth_flows.go b/datamodel/low/v3/oauth_flows.go index 90dd3456c..67ada5642 100644 --- a/datamodel/low/v3/oauth_flows.go +++ b/datamodel/low/v3/oauth_flows.go @@ -7,7 +7,6 @@ import ( "context" "fmt" "hash/maphash" - "sync" "github.com/pb33f/libopenapi/datamodel/low" "github.com/pb33f/libopenapi/index" @@ -29,7 +28,7 @@ type OAuthFlows struct { RootNode *yaml.Node index *index.SpecIndex context context.Context - nodeStore sync.Map + nodeStore low.NodeLines reference low.Reference *low.Reference low.NodeMap @@ -73,7 +72,7 @@ func (o *OAuthFlows) Build(ctx context.Context, keyNode, root *yaml.Node, idx *i utils.CheckForMergeNodes(root) o.reference = low.Reference{} o.Reference = &o.reference - o.nodeStore = sync.Map{} + o.nodeStore = low.NodeLines{} o.Nodes = &o.nodeStore if len(root.Content) > 0 { o.NodeMap.ExtractNodes(root, false) @@ -159,7 +158,7 @@ type OAuthFlow struct { RootNode *yaml.Node index *index.SpecIndex context context.Context - nodeStore sync.Map + nodeStore low.NodeLines reference low.Reference *low.Reference low.NodeMap @@ -199,7 +198,7 @@ func (o *OAuthFlow) GetRootNode() *yaml.Node { func (o *OAuthFlow) Build(ctx context.Context, _, root *yaml.Node, idx *index.SpecIndex) error { o.reference = low.Reference{} o.Reference = &o.reference - o.nodeStore = sync.Map{} + o.nodeStore = low.NodeLines{} o.Nodes = &o.nodeStore if len(root.Content) > 0 { o.NodeMap.ExtractNodes(root, false) diff --git a/datamodel/low/v3/operation.go b/datamodel/low/v3/operation.go index e54657d6d..e9d54affb 100644 --- a/datamodel/low/v3/operation.go +++ b/datamodel/low/v3/operation.go @@ -7,7 +7,6 @@ import ( "context" "hash/maphash" "sort" - "sync" "github.com/pb33f/libopenapi/datamodel/low" "github.com/pb33f/libopenapi/datamodel/low/base" @@ -40,7 +39,7 @@ type Operation struct { RootNode *yaml.Node index *index.SpecIndex context context.Context - nodeStore sync.Map + nodeStore low.NodeLines reference low.Reference *low.Reference low.NodeMap @@ -92,7 +91,7 @@ func (o *Operation) Build(ctx context.Context, keyNode, root *yaml.Node, idx *in utils.CheckForMergeNodes(root) o.reference = low.Reference{} o.Reference = &o.reference - o.nodeStore = sync.Map{} + o.nodeStore = low.NodeLines{} o.Nodes = &o.nodeStore if len(root.Content) > 0 { o.NodeMap.ExtractNodes(root, false) diff --git a/datamodel/low/v3/parameter.go b/datamodel/low/v3/parameter.go index 8cf942e28..8d542ce76 100644 --- a/datamodel/low/v3/parameter.go +++ b/datamodel/low/v3/parameter.go @@ -8,7 +8,6 @@ import ( "fmt" "hash/maphash" "slices" - "sync" "github.com/pb33f/libopenapi/datamodel/low" "github.com/pb33f/libopenapi/datamodel/low/base" @@ -41,7 +40,7 @@ type Parameter struct { Extensions *orderedmap.Map[low.KeyReference[string], low.ValueReference[*yaml.Node]] index *index.SpecIndex context context.Context - nodeStore sync.Map + nodeStore low.NodeLines reference low.Reference *low.Reference low.NodeMap @@ -98,7 +97,7 @@ func (p *Parameter) Build(ctx context.Context, keyNode, root *yaml.Node, idx *in p.KeyNode = keyNode p.RootNode = root utils.CheckForMergeNodes(root) - p.nodeStore = sync.Map{} + p.nodeStore = low.NodeLines{} p.Nodes = &p.nodeStore if len(root.Content) > 0 { p.NodeMap.ExtractNodes(root, false) diff --git a/datamodel/low/v3/path_item.go b/datamodel/low/v3/path_item.go index b80b1b6bd..1038a09d5 100644 --- a/datamodel/low/v3/path_item.go +++ b/datamodel/low/v3/path_item.go @@ -9,7 +9,6 @@ import ( "hash/maphash" "sort" "strings" - "sync" "github.com/pb33f/libopenapi/datamodel" "github.com/pb33f/libopenapi/datamodel/low" @@ -45,7 +44,7 @@ type PathItem struct { RootNode *yaml.Node index *index.SpecIndex context context.Context - nodeStore sync.Map + nodeStore low.NodeLines reference low.Reference *low.Reference low.NodeMap @@ -206,7 +205,7 @@ func (p *PathItem) Build(ctx context.Context, keyNode, root *yaml.Node, idx *ind p.KeyNode = keyNode p.RootNode = root utils.CheckForMergeNodes(root) - p.nodeStore = sync.Map{} + p.nodeStore = low.NodeLines{} p.Nodes = &p.nodeStore if len(root.Content) > 0 { p.NodeMap.ExtractNodes(root, false) @@ -222,6 +221,8 @@ func (p *PathItem) Build(ctx context.Context, keyNode, root *yaml.Node, idx *ind var currentNode *yaml.Node ops := make([]low.NodeReference[*Operation], 0, len(root.Content)/2) + // the context each operation was resolved in, which its build runs with. + opContexts := make(map[*Operation]context.Context, len(root.Content)/2) var additionalOps *orderedmap.Map[low.KeyReference[string], low.NodeReference[*Operation]] var additionalOpsKeyNode, additionalOpsValueNode *yaml.Node @@ -329,13 +330,13 @@ func (p *PathItem) Build(ctx context.Context, keyNode, root *yaml.Node, idx *ind Value: &op, KeyNode: currentNode, ValueNode: pathNode, - Context: foundContext, } if opIsRef { opRef.SetReference(opRefVal, opRefNode) } ops = append(ops, opRef) + opContexts[opRef.Value] = foundContext if isStandardOp { switch currentNode.Value { @@ -388,11 +389,11 @@ func (p *PathItem) Build(ctx context.Context, keyNode, root *yaml.Node, idx *ind Value: &addOp, KeyNode: opKeyNode, ValueNode: opValueNode, - Context: foundContext, } if opIsRef { addOpRef.SetReference(opRefVal, opRefNode) } + opContexts[addOpRef.Value] = foundContext additionalOps.Set(low.KeyReference[string]{ KeyNode: opKeyNode, @@ -425,7 +426,8 @@ func (p *PathItem) Build(ctx context.Context, keyNode, root *yaml.Node, idx *ind refNode = op.GetReferenceNode() } - err := op.Value.Build(op.Context, op.KeyNode, op.ValueNode, op.Context.Value(index.FoundIndexKey).(*index.SpecIndex)) + opCtx := opContexts[op.Value] + err := op.Value.Build(opCtx, op.KeyNode, op.ValueNode, opCtx.Value(index.FoundIndexKey).(*index.SpecIndex)) if ref != "" { op.Value.Reference.SetReference(ref, refNode) } diff --git a/datamodel/low/v3/paths.go b/datamodel/low/v3/paths.go index 148242838..6868d798d 100644 --- a/datamodel/low/v3/paths.go +++ b/datamodel/low/v3/paths.go @@ -7,7 +7,6 @@ import ( "context" "fmt" "hash/maphash" - "sync" "github.com/pb33f/libopenapi/datamodel" "github.com/pb33f/libopenapi/datamodel/low" @@ -30,7 +29,7 @@ type Paths struct { RootNode *yaml.Node index *index.SpecIndex context context.Context - nodeStore sync.Map + nodeStore low.NodeLines reference low.Reference *low.Reference low.NodeMap @@ -97,7 +96,7 @@ func (p *Paths) Build(ctx context.Context, keyNode, root *yaml.Node, idx *index. utils.CheckForMergeNodes(root) p.reference = low.Reference{} p.Reference = &p.reference - p.nodeStore = sync.Map{} + p.nodeStore = low.NodeLines{} p.Nodes = &p.nodeStore if keyNode != nil { p.AddNode(keyNode.Line, keyNode) diff --git a/datamodel/low/v3/request_body.go b/datamodel/low/v3/request_body.go index 344822e2d..12b0e3aa6 100644 --- a/datamodel/low/v3/request_body.go +++ b/datamodel/low/v3/request_body.go @@ -6,7 +6,6 @@ package v3 import ( "context" "hash/maphash" - "sync" "github.com/pb33f/libopenapi/datamodel/low" "github.com/pb33f/libopenapi/index" @@ -26,7 +25,7 @@ type RequestBody struct { RootNode *yaml.Node index *index.SpecIndex context context.Context - nodeStore sync.Map + nodeStore low.NodeLines reference low.Reference *low.Reference low.NodeMap @@ -78,7 +77,7 @@ func (rb *RequestBody) Build(ctx context.Context, keyNode, root *yaml.Node, idx root = utils.NodeAlias(root) rb.RootNode = root utils.CheckForMergeNodes(root) - rb.nodeStore = sync.Map{} + rb.nodeStore = low.NodeLines{} rb.Nodes = &rb.nodeStore if len(root.Content) > 0 { rb.NodeMap.ExtractNodes(root, false) diff --git a/datamodel/low/v3/response.go b/datamodel/low/v3/response.go index 15a94d46c..61cc45d0d 100644 --- a/datamodel/low/v3/response.go +++ b/datamodel/low/v3/response.go @@ -6,7 +6,6 @@ package v3 import ( "context" "hash/maphash" - "sync" "github.com/pb33f/libopenapi/datamodel/low" "github.com/pb33f/libopenapi/index" @@ -31,7 +30,7 @@ type Response struct { RootNode *yaml.Node index *index.SpecIndex context context.Context - nodeStore sync.Map + nodeStore low.NodeLines reference low.Reference *low.Reference low.NodeMap @@ -93,7 +92,7 @@ func (r *Response) Build(ctx context.Context, keyNode, root *yaml.Node, idx *ind root = utils.NodeAlias(root) r.RootNode = root utils.CheckForMergeNodes(root) - r.nodeStore = sync.Map{} + r.nodeStore = low.NodeLines{} r.Nodes = &r.nodeStore if len(root.Content) > 0 { r.NodeMap.ExtractNodes(root, false) diff --git a/datamodel/low/v3/responses.go b/datamodel/low/v3/responses.go index b7891e9b8..08df233f2 100644 --- a/datamodel/low/v3/responses.go +++ b/datamodel/low/v3/responses.go @@ -8,7 +8,6 @@ import ( "fmt" "hash/maphash" "strings" - "sync" "github.com/pb33f/libopenapi/datamodel/low" "github.com/pb33f/libopenapi/index" @@ -43,7 +42,7 @@ type Responses struct { RootNode *yaml.Node index *index.SpecIndex context context.Context - nodeStore sync.Map + nodeStore low.NodeLines reference low.Reference *low.Reference low.NodeMap @@ -81,7 +80,7 @@ func (r *Responses) Build(ctx context.Context, keyNode, root *yaml.Node, idx *in r.RootNode = root r.reference = low.Reference{} r.Reference = &r.reference - r.nodeStore = sync.Map{} + r.nodeStore = low.NodeLines{} r.Nodes = &r.nodeStore if len(root.Content) > 0 { r.NodeMap.ExtractNodes(root, false) diff --git a/datamodel/low/v3/security_scheme.go b/datamodel/low/v3/security_scheme.go index 66185deb5..fa07fc330 100644 --- a/datamodel/low/v3/security_scheme.go +++ b/datamodel/low/v3/security_scheme.go @@ -6,7 +6,6 @@ package v3 import ( "context" "hash/maphash" - "sync" "github.com/pb33f/libopenapi/datamodel/low" "github.com/pb33f/libopenapi/index" @@ -41,7 +40,7 @@ type SecurityScheme struct { RootNode *yaml.Node index *index.SpecIndex context context.Context - nodeStore sync.Map + nodeStore low.NodeLines reference low.Reference *low.Reference low.NodeMap @@ -88,7 +87,7 @@ func (ss *SecurityScheme) Build(ctx context.Context, keyNode, root *yaml.Node, i root = utils.NodeAlias(root) ss.RootNode = root utils.CheckForMergeNodes(root) - ss.nodeStore = sync.Map{} + ss.nodeStore = low.NodeLines{} ss.Nodes = &ss.nodeStore if len(root.Content) > 0 { ss.NodeMap.ExtractNodes(root, false) From 8ef6d09da8bf71c42d3a26478409a22afccc92aa Mon Sep 17 00:00:00 2001 From: quobix Date: Mon, 28 Sep 2026 08:07:48 -0400 Subject: [PATCH 3/3] docs: add a migration guide for the low-level API changes MIGRATING.md covers the two breaking changes in this branch, each with a table of old and new signatures and a complete program whose output is shown verbatim: - NodeMap.Nodes is a *low.NodeLines: Range, Load and Store take int lines, the extraction helpers take and return *low.NodeLines, and sync.Map's other methods are not provided. - NodeReference.Context is removed: libopenapi only ever set it on the operations of a PathItem, where Operation.GetContext() returns the same context. README links to it. ClearAllCaches' comment now lists the encode cache among the content-keyed caches it empties. Co-Authored-By: Claude Opus 5.5 --- MIGRATING.md | 177 +++++++++++++++++++++++++++++++++++++++++++++++++++ README.md | 1 + cache.go | 6 +- 3 files changed, 181 insertions(+), 3 deletions(-) create mode 100644 MIGRATING.md diff --git a/MIGRATING.md b/MIGRATING.md new file mode 100644 index 000000000..b543a67a8 --- /dev/null +++ b/MIGRATING.md @@ -0,0 +1,177 @@ +# Migrating + +This file lists breaking changes to the libopenapi Go API and how to update code for each one. + +## Upgrading past v0.40 + +This release makes two changes to the low-level model API. They don't change parsing, rendering, bundling or +what-changed output. + +You don't need to change anything if your code only uses: +- the high-level model +- `GetNodes()` +- `AddNode()` + +### `NodeMap.Nodes` is now a `*low.NodeLines` + +Every low-level model embeds `low.NodeMap`, which records the YAML nodes the model was built from, keyed by line +number. `Nodes` was a `*sync.Map` keyed by `int`. It is now a `*low.NodeLines`. A `NodeLines` records writes as they +happen and builds its line index the first time it is read. Most models are built and never read, so building a +document and comparing documents with what-changed allocate less. + +| v0.40 | now | +|---|---| +| `Nodes *sync.Map` | `Nodes *low.NodeLines` | +| `Nodes.Range(func(key, value any) bool)` | `Nodes.Range(func(line int, value any) bool)`, visiting lines in ascending order | +| `Nodes.Load(key any) (any, bool)` | `Nodes.Load(line int) (any, bool)` | +| `Nodes.Store(key, value any)` | `Nodes.Store(line int, value any)` | +| `low.NodeMap{Nodes: &sync.Map{}}` | `low.NodeMap{Nodes: &low.NodeLines{}}` (the zero value is ready to use) | +| `low.ExtractNodes(ctx, root) *sync.Map` | returns `*low.NodeLines` | +| `low.ExtractNodesRecursive(ctx, root) *sync.Map` | returns `*low.NodeLines` | +| `low.ExtractExtensionNodes(ctx, extensions, nodes *sync.Map)` | takes `*low.NodeLines` | +| `low.MergeRecursiveNodesIfLineAbsent(dst *sync.Map, node)` | takes `*low.NodeLines` | + +`NodeLines` does not have `sync.Map`'s other methods: +- `Delete` and `Clear` +- `LoadOrStore` and `LoadAndDelete` +- `Swap`, `CompareAndSwap` and `CompareAndDelete` + +A line's value is a `*yaml.Node`, or a `[]*yaml.Node` when several nodes share the line, as before. + +This program reads the nodes of an `info` object: + +```go +package main + +import ( + "fmt" + + "github.com/pb33f/libopenapi" + "go.yaml.in/yaml/v4" +) + +const spec = `openapi: 3.1.0 +info: + title: Burger Shop + version: 1.0.0 +paths: {} +` + +func main() { + doc, err := libopenapi.NewDocument([]byte(spec)) + if err != nil { + panic(err) + } + model, err := doc.BuildV3Model() + if err != nil { + panic(err) + } + info := model.Model.GoLow().Info.Value + + // Range passes each line as an int, in ascending order. A line holds a *yaml.Node, or a + // []*yaml.Node when several nodes share it. + info.Nodes.Range(func(line int, value any) bool { + switch v := value.(type) { + case *yaml.Node: + fmt.Printf("line %d: %s\n", line, v.Value) + case []*yaml.Node: + for _, n := range v { + fmt.Printf("line %d: %s\n", line, n.Value) + } + } + return true + }) + + // Load takes the line as an int. + if value, ok := info.Nodes.Load(4); ok { + fmt.Printf("line 4 holds %d nodes\n", len(value.([]*yaml.Node))) + } + + // GetNodes is unchanged. + fmt.Printf("GetNodes: %d lines\n", len(info.GetNodes())) +} +``` + +Output: + +``` +line 3: title +line 3: Burger Shop +line 4: version +line 4: 1.0.0 +line 4 holds 2 nodes +GetNodes: 2 lines +``` + +### `NodeReference.Context` is removed + +`low.NodeReference[T]` had a `Context` field. libopenapi only ever set it on the operations of a `PathItem`: +- `Get`, `Put`, `Post`, `Delete`, `Options`, `Head`, `Patch`, `Trace` and `Query` +- the operations in `AdditionalOperations` + +On those references it held the context the operation was resolved and built with. On every other reference it +was nil. + +Read that context from the operation instead: + +| v0.40 | now | +|---|---| +| `pathItem.Get.Context` | `pathItem.Get.Value.GetContext()` | + +`GetContext()` returns the context passed to the operation's `Build`, which is the context `Context` held. If an +operation is a `$ref` to another file, that context carries the other file's index. + +If your code sets `Context` on references it creates, keep the context next to the reference in a type of your +own, for example `struct { Ref low.NodeReference[T]; Ctx context.Context }`. + +This program reads the context the `get` operation of `/burgers` was built with: + +```go +package main + +import ( + "fmt" + + "github.com/pb33f/libopenapi" + "github.com/pb33f/libopenapi/index" +) + +const spec = `openapi: 3.1.0 +info: + title: Burger Shop + version: 1.0.0 +paths: + /burgers: + get: + operationId: listBurgers + responses: + '200': + description: OK +` + +func main() { + doc, err := libopenapi.NewDocument([]byte(spec)) + if err != nil { + panic(err) + } + model, err := doc.BuildV3Model() + if err != nil { + panic(err) + } + pathItem := model.Model.GoLow().Paths.Value.FindPath("/burgers").Value + + // was: ctx := pathItem.Get.Context + ctx := pathItem.Get.Value.GetContext() + + idx := ctx.Value(index.FoundIndexKey).(*index.SpecIndex) + fmt.Println("operation:", pathItem.Get.Value.OperationId.Value) + fmt.Println("built with the operation's index:", idx == pathItem.Get.Value.GetIndex()) +} +``` + +Output: + +``` +operation: listBurgers +built with the operation's index: true +``` diff --git a/README.md b/README.md index 0bf7d358a..07f595084 100644 --- a/README.md +++ b/README.md @@ -76,6 +76,7 @@ See all the documentation at https://pb33f.io/libopenapi/ - [Parsing Code](https://pb33f.io/libopenapi/parsing-code/) - [FAQ](https://pb33f.io/libopenapi/faq/) - [About libopenapi](https://pb33f.io/libopenapi/about/) +- [Migrating: breaking API changes by version](MIGRATING.md) ### Generating TypeScript models diff --git a/cache.go b/cache.go index 55afe9821..32efeb3f0 100644 --- a/cache.go +++ b/cache.go @@ -16,9 +16,9 @@ import ( // // Calling it is not required to release memory: caches keyed by YAML nodes or model objects hold them weakly, // so a document is reclaimed as soon as the caller drops it. Use it to force hashes to be recalculated after -// YAML nodes or low-level models were modified in place, or to empty the string-keyed caches (compiled JSONPath -// expressions, schema quick hashes and remote content types). It is safe to call while other goroutines parse, -// build or compare documents. +// YAML nodes or low-level models were modified in place, or to empty the content-keyed caches (compiled +// JSONPath expressions, schema quick hashes, encoded values and remote content types). It is safe to call +// while other goroutines parse, build or compare documents. func ClearAllCaches() { low.ClearHashCache() // model and YAML node hashes lowbase.ClearSchemaQuickHashMap() // SchemaQuickHashMap