diff --git a/openapi3/loader.go b/openapi3/loader.go index 2f2480c07..cf1c75382 100644 --- a/openapi3/loader.go +++ b/openapi3/loader.go @@ -149,8 +149,26 @@ func (loader *Loader) loadSingleElementFromURI(ref string, rootPath *url.URL, el // rememberOriginTree retains doc's origin tree for attachOriginToResolved. // tree is nil when IncludeOrigin is off or the data took the json path. +// +// The tree is kept only for a document a $ref can reach into untyped, which is +// what attachOriginToResolved exists to re-origin. In practice that means a +// file of shared fragments, whose top level is the fragment name itself rather +// than the fields of an OpenAPI Object: +// +// User: # a $ref to "./schemas.yaml#/User" lands here, untyped +// type: object +// +// Anything OpenAPI defines a field for resolves through typed structures and +// keeps its origins on the way, so its tree could never be read. That includes +// a referenced document that is itself an OpenAPI Object: a $ref to +// "#/components/schemas/User" needs no tree. (A top-level x- extension is +// undefined by the same rule, so a document carrying one keeps its tree too, +// whether or not anything ever points at it.) +// +// Worth the condition: on a 22 MB spec the retained tree was a third of +// everything the loader held. func (loader *Loader) rememberOriginTree(doc *T, tree *originTree) { - if tree == nil { + if tree == nil || len(doc.Extensions) == 0 { return } if loader.originTrees == nil { diff --git a/openapi3/origin_retention_test.go b/openapi3/origin_retention_test.go new file mode 100644 index 000000000..0b188026f --- /dev/null +++ b/openapi3/origin_retention_test.go @@ -0,0 +1,71 @@ +package openapi3 + +import ( + "testing" + + "github.com/stretchr/testify/require" +) + +// An ordinary spec never consults its origin tree, so the loader does not keep +// one. Origins are still attached to the document itself; what goes away is the +// second copy the loader used to hold for the lifetime of the loader, which on +// a large spec was a third of everything it retained. +func TestOriginTree_NotRetainedForAnOrdinarySpec(t *testing.T) { + loader := NewLoader() + loader.IncludeOrigin = true + loader.Context = t.Context() + + doc, err := loader.LoadFromFile("testdata/origin/simple.yaml") + require.NoError(t, err) + + require.NotNil(t, doc.Origin, "origins are still attached to the document") + require.Empty(t, loader.originTrees, "no tree is retained: nothing could read it") +} + +// The tree is kept for exactly the documents that can consult it, and this OAD +// contains one of each: the entry document has only fields OpenAPI defines, +// while the file it $refs is a shared fragment whose top level is the schema +// name "User", which is what arrives untyped and what attachOriginToResolved +// walks the tree for. +// +// The retained tree still does its job here, so this pins the saving and the +// capability together: dropping the tree for the referenced file would leave +// the resolved schema without an origin. +func TestOriginTree_RetainedOnlyWhereItCanBeRead(t *testing.T) { + loader := NewLoader() + loader.IsExternalRefsAllowed = true + loader.IncludeOrigin = true + loader.Context = t.Context() + + doc, err := loader.LoadFromFile("testdata/origin/arbitrary_key.yaml") + require.NoError(t, err) + + require.NotContains(t, loader.originTrees, doc, + "the entry document has only fields OpenAPI defines, so no $ref reaches into it untyped") + require.Len(t, loader.originTrees, 1, "only the shared-fragment file keeps its tree") + for retained := range loader.originTrees { + require.NotEmpty(t, retained.Extensions, + "a tree is kept only for a document with top-level fields OpenAPI does not define") + } + + // The capability the retained tree exists for: the resolved schema carries + // the origin of its own file, not of the $ref site. + schema := doc.Paths.Find("/users").Get.Responses.Value("200").Value. + Content["application/json"].Schema.Value + require.NotNil(t, schema.Origin, "the resolved schema keeps its origin") + require.Contains(t, schema.Origin.Key.File, "arbitrary_key_schemas.yaml") +} + +// Without IncludeOrigin there is no tree to begin with, so the new condition +// cannot change anything here. +func TestOriginTree_NotRetainedWhenOriginsAreOff(t *testing.T) { + loader := NewLoader() + loader.IsExternalRefsAllowed = true + loader.Context = t.Context() + + doc, err := loader.LoadFromFile("testdata/origin/arbitrary_key.yaml") + require.NoError(t, err) + + require.Nil(t, doc.Origin) + require.Empty(t, loader.originTrees) +}