Repository: getkin/kin-openapi
Pull request #1055 (2025-03-07 09:13:09)

openapi3: delete origin keys only when IncludeOrigin=true
This PR fixes https://github.com/getkin/kin-openapi/issues/1051.
The problem was caused because openapi3 removed `origin` keys from `Schema.Properties` and other map-based types including all `Components` types.
To fix this I changed the behavior to only delete `origin` keys when `IncludeOrigin` is set to true.
Note that this required moving `Loader.IncludeOrigin` to the global scope so that map unmarshalling functions can access it.
For example: `Schemas.UnmarshalJSON` -> `unmarshalStringMapP` -> `popOrigin` which accesses `IncludeOrigin`

fenollp: LGTM but please rebase now that I merged https://github.com/getkin/kin-openapi/pull/1057
Also please rename the PR to something that makes standalone sense.
reuvenharrison: @fenollp the PR is ready to merge

Patch:
diff --git a/.github/docs/openapi3.txt b/.github/docs/openapi3.txt
index 2f9142d..61d952e 100644
--- a/.github/docs/openapi3.txt
+++ b/.github/docs/openapi3.txt
@@ -90,6 +90,11 @@ var ErrURINotSupported = errors.New("unsupported URI")
     ErrURINotSupported indicates the ReadFromURIFunc does not know how to handle
     a given URI.
 
+var IncludeOrigin = false
+    IncludeOrigin specifies whether to include the origin of the OpenAPI
+    elements Set this to true before loading a spec to include the origin of the
+    OpenAPI elements Note it is global and affects all loaders
+
 
 FUNCTIONS
 
@@ -790,9 +795,6 @@ type Loader struct {
 	// IsExternalRefsAllowed enables visiting other files
 	IsExternalRefsAllowed bool
 
-	// IncludeOrigin specifies whether to include the origin of the OpenAPI elements
-	IncludeOrigin bool
-
 	// ReadFromURIFunc allows overriding the any file/URL reading func
 	ReadFromURIFunc ReadFromURIFunc
 
diff --git a/openapi3/loader.go b/openapi3/loader.go
index 0f3b8cb..436a1b3 100644
--- a/openapi3/loader.go
+++ b/openapi3/loader.go
@@ -15,6 +15,11 @@ import (
 	"strings"
 )
 
+// IncludeOrigin specifies whether to include the origin of the OpenAPI elements
+// Set this to true before loading a spec to include the origin of the OpenAPI elements
+// Note it is global and affects all loaders
+var IncludeOrigin = false
+
 func foundUnresolvedRef(ref string) error {
 	return fmt.Errorf("found unresolved ref: %q", ref)
 }
@@ -28,9 +33,6 @@ type Loader struct {
 	// IsExternalRefsAllowed enables visiting other files
 	IsExternalRefsAllowed bool
 
-	// IncludeOrigin specifies whether to include the origin of the OpenAPI elements
-	IncludeOrigin bool
-
 	// ReadFromURIFunc allows overriding the any file/URL reading func
 	ReadFromURIFunc ReadFromURIFunc
 
@@ -106,7 +108,7 @@ func (loader *Loader) loadSingleElementFromURI(ref string, rootPath *url.URL, el
 	if err != nil {
 		return nil, err
 	}
-	if err := unmarshal(data, element, loader.IncludeOrigin); err != nil {
+	if err := unmarshal(data, element, IncludeOrigin); err != nil {
 		return nil, err
 	}
 
@@ -142,7 +144,7 @@ func (loader *Loader) LoadFromIoReader(reader io.Reader) (*T, error) {
 func (loader *Loader) LoadFromData(data []byte) (*T, error) {
 	loader.resetVisitedPathItemRefs()
 	doc := &T{}
-	if err := unmarshal(data, doc, loader.IncludeOrigin); err != nil {
+	if err := unmarshal(data, doc, IncludeOrigin); err != nil {
 		return nil, err
 	}
 	if err := loader.ResolveRefsIn(doc, nil); err != nil {
@@ -171,7 +173,7 @@ func (loader *Loader) loadFromDataWithPathInternal(data []byte, location *url.UR
 	doc := &T{}
 	loader.visitedDocuments[uri] = doc
 
-	if err := unmarshal(data, doc, loader.IncludeOrigin); err != nil {
+	if err := unmarshal(data, doc, IncludeOrigin); err != nil {
 		return nil, err
 	}
 
@@ -425,7 +427,7 @@ func (loader *Loader) resolveComponent(doc *T, ref string, path *url.URL, resolv
 		if err2 != nil {
 			return nil, nil, err
 		}
-		if err2 = unmarshal(data, &cursor, loader.IncludeOrigin); err2 != nil {
+		if err2 = unmarshal(data, &cursor, IncludeOrigin); err2 != nil {
 			return nil, nil, err
 		}
 		if cursor, err2 = drill(cursor); err2 != nil || cursor == nil {
diff --git a/openapi3/stringmap.go b/openapi3/stringmap.go
index 354a4c8..d1b91ac 100644
--- a/openapi3/stringmap.go
+++ b/openapi3/stringmap.go
@@ -18,11 +18,10 @@ func unmarshalStringMapP[V any](data []byte) (map[string]*V, *Origin, error) {
 		return nil, nil, err
 	}
 
-	origin, err := deepCast[Origin](m[originKey])
+	origin, err := popOrigin(m, originKey)
 	if err != nil {
 		return nil, nil, err
 	}
-	delete(m, originKey)
 
 	result := make(map[string]*V, len(m))
 	for k, v := range m {
@@ -43,11 +42,10 @@ func unmarshalStringMap[V any](data []byte) (map[string]V, *Origin, error) {
 		return nil, nil, err
 	}
 
-	origin, err := deepCast[Origin](m[originKey])
+	origin, err := popOrigin(m, originKey)
 	if err != nil {
 		return nil, nil, err
 	}
-	delete(m, originKey)
 
 	result := make(map[string]V, len(m))
 	for k, v := range m {
@@ -74,3 +72,17 @@ func deepCast[V any](value any) (*V, error) {
 	}
 	return &result, nil
 }
+
+// popOrigin removes the origin from the map and returns it.
+func popOrigin(m map[string]any, key string) (*Origin, error) {
+	if !IncludeOrigin {
+		return nil, nil
+	}
+
+	origin, err := deepCast[Origin](m[key])
+	if err != nil {
+		return nil, err
+	}
+	delete(m, key)
+	return origin, nil
+}


Test patch:
diff --git a/openapi3/origin_test.go b/openapi3/origin_test.go
index d955056..2c1d46d 100644
--- a/openapi3/origin_test.go
+++ b/openapi3/origin_test.go
@@ -7,10 +7,17 @@ import (
 	"github.com/stretchr/testify/require"
 )
 
+func unsetIncludeOrigin() {
+	IncludeOrigin = false
+}
+
 func TestOrigin_Info(t *testing.T) {
 	loader := NewLoader()
 	loader.IsExternalRefsAllowed = true
-	loader.IncludeOrigin = true
+
+	IncludeOrigin = true
+	defer unsetIncludeOrigin()
+
 	loader.Context = context.Background()
 
 	doc, err := loader.LoadFromFile("testdata/origin/simple.yaml")
@@ -42,7 +49,10 @@ func TestOrigin_Info(t *testing.T) {
 func TestOrigin_Paths(t *testing.T) {
 	loader := NewLoader()
 	loader.IsExternalRefsAllowed = true
-	loader.IncludeOrigin = true
+
+	IncludeOrigin = true
+	defer unsetIncludeOrigin()
+
 	loader.Context = context.Background()
 
 	doc, err := loader.LoadFromFile("testdata/origin/simple.yaml")
@@ -78,7 +88,10 @@ func TestOrigin_Paths(t *testing.T) {
 func TestOrigin_RequestBody(t *testing.T) {
 	loader := NewLoader()
 	loader.IsExternalRefsAllowed = true
-	loader.IncludeOrigin = true
+
+	IncludeOrigin = true
+	defer unsetIncludeOrigin()
+
 	loader.Context = context.Background()
 
 	doc, err := loader.LoadFromFile("testdata/origin/request_body.yaml")
@@ -105,7 +118,10 @@ func TestOrigin_RequestBody(t *testing.T) {
 func TestOrigin_Responses(t *testing.T) {
 	loader := NewLoader()
 	loader.IsExternalRefsAllowed = true
-	loader.IncludeOrigin = true
+
+	IncludeOrigin = true
+	defer unsetIncludeOrigin()
+
 	loader.Context = context.Background()
 
 	doc, err := loader.LoadFromFile("testdata/origin/simple.yaml")
@@ -140,7 +156,10 @@ func TestOrigin_Responses(t *testing.T) {
 func TestOrigin_Parameters(t *testing.T) {
 	loader := NewLoader()
 	loader.IsExternalRefsAllowed = true
-	loader.IncludeOrigin = true
+
+	IncludeOrigin = true
+	defer unsetIncludeOrigin()
+
 	loader.Context = context.Background()
 
 	doc, err := loader.LoadFromFile("testdata/origin/parameters.yaml")
@@ -173,7 +192,10 @@ func TestOrigin_Parameters(t *testing.T) {
 func TestOrigin_SchemaInAdditionalProperties(t *testing.T) {
 	loader := NewLoader()
 	loader.IsExternalRefsAllowed = true
-	loader.IncludeOrigin = true
+
+	IncludeOrigin = true
+	defer unsetIncludeOrigin()
+
 	loader.Context = context.Background()
 
 	doc, err := loader.LoadFromFile("testdata/origin/additional_properties.yaml")
@@ -201,7 +223,10 @@ func TestOrigin_SchemaInAdditionalProperties(t *testing.T) {
 func TestOrigin_ExternalDocs(t *testing.T) {
 	loader := NewLoader()
 	loader.IsExternalRefsAllowed = true
-	loader.IncludeOrigin = true
+
+	IncludeOrigin = true
+	defer unsetIncludeOrigin()
+
 	loader.Context = context.Background()
 
 	doc, err := loader.LoadFromFile("testdata/origin/external_docs.yaml")
@@ -235,7 +260,10 @@ func TestOrigin_ExternalDocs(t *testing.T) {
 func TestOrigin_Security(t *testing.T) {
 	loader := NewLoader()
 	loader.IsExternalRefsAllowed = true
-	loader.IncludeOrigin = true
+
+	IncludeOrigin = true
+	defer unsetIncludeOrigin()
+
 	loader.Context = context.Background()
 
 	doc, err := loader.LoadFromFile("testdata/origin/security.yaml")
@@ -283,7 +311,10 @@ func TestOrigin_Security(t *testing.T) {
 func TestOrigin_Example(t *testing.T) {
 	loader := NewLoader()
 	loader.IsExternalRefsAllowed = true
-	loader.IncludeOrigin = true
+
+	IncludeOrigin = true
+	defer unsetIncludeOrigin()
+
 	loader.Context = context.Background()
 
 	doc, err := loader.LoadFromFile("testdata/origin/example.yaml")
@@ -319,7 +350,10 @@ func TestOrigin_Example(t *testing.T) {
 func TestOrigin_XML(t *testing.T) {
 	loader := NewLoader()
 	loader.IsExternalRefsAllowed = true
-	loader.IncludeOrigin = true
+
+	IncludeOrigin = true
+	defer unsetIncludeOrigin()
+
 	loader.Context = context.Background()
 
 	doc, err := loader.LoadFromFile("testdata/origin/xml.yaml")
@@ -348,3 +382,37 @@ func TestOrigin_XML(t *testing.T) {
 		},
 		base.Origin.Fields["prefix"])
 }
+
+// TestOrigin_OriginExistsInProperties verifies that loading fails when a specification
+// contains a property named "__origin__", highlighting a limitation in the current implementation.
+func TestOrigin_OriginExistsInProperties(t *testing.T) {
+	var data = `
+paths:
+  /foo:
+    get:
+      responses:
+        "200":
+          description: OK
+          content:
+            application/json:
+              schema:
+                $ref: "#/components/schemas/Foo"
+components:
+  schemas:
+    Foo:
+      type: object
+      properties:
+        __origin__:
+          type: string
+`
+
+	loader := NewLoader()
+
+	IncludeOrigin = true
+	defer unsetIncludeOrigin()
+
+	_, err := loader.LoadFromData([]byte(data))
+	require.Error(t, err)
+	require.Equal(t, `failed to unmarshal data: json error: invalid character 'p' looking for beginning of value, yaml error: error converting YAML to JSON: yaml: unmarshal errors:
+  line 0: mapping key "__origin__" already defined at line 17`, err.Error())
+}