openapi3: process discriminator mapping values as refs (#1022)

* openapi3: make StringMap generic over value This allows us to support more use cases than just a string, allowing us to attach more metadata onto these StringMap types that we cannot encode in just a string. * openapi3: process discriminator mapping values as refs While the type of the discriminator mapping values is a string in the upstream specs, it contains a jsonschema reference to a schema object. It is surprising behaviour that these refs are not handled when calling functions such as InternalizeRefs. This patch adds the data structures to store the ref internally, and updates the Loader and InternalizeRefs to handle this case. There may be several more functions that need to be updated that I am not aware of. Since it is not a full Ref object we have to do some fudging to make it work with all the existing ref handling code.
getkin · Oct 24, 2024 · 4d76e26 · 4d76e26
1 parent 7145b2c
commit 4d76e26
Show file tree

Hide file tree

Showing 11 changed files with 179 additions and 19 deletions.
diff --git a/.github/docs/openapi3.txt b/.github/docs/openapi3.txt
@@ -411,8 +411,8 @@ type Discriminator struct {
 	Extensions map[string]any `json:"-" yaml:"-"`
 	Origin     *Origin        `json:"origin,omitempty" yaml:"origin,omitempty"`
 
-	PropertyName string    `json:"propertyName" yaml:"propertyName"` // required
-	Mapping      StringMap `json:"mapping,omitempty" yaml:"mapping,omitempty"`
+	PropertyName string                `json:"propertyName" yaml:"propertyName"` // required
+	Mapping      StringMap[MappingRef] `json:"mapping,omitempty" yaml:"mapping,omitempty"`
 }
     Discriminator is specified by OpenAPI/Swagger standard version 3. See
     https://github.com/OAI/OpenAPI-Specification/blob/main/versions/3.0.3.md#discriminator-object
@@ -831,6 +831,15 @@ type Location struct {
 }
     Location is a struct that contains the location of a field.
 
+type MappingRef SchemaRef
+    MappingRef is a ref to a Schema objects. Unlike SchemaRefs it is serialised
+    as a plain string instead of an object with a $ref key, as such it also does
+    not support extensions.
+
+func (mr MappingRef) MarshalText() ([]byte, error)
+
+func (mr *MappingRef) UnmarshalText(data []byte) error
+
 type MediaType struct {
 	Extensions map[string]any `json:"-" yaml:"-"`
 	Origin     *Origin        `json:"origin,omitempty" yaml:"origin,omitempty"`
@@ -913,10 +922,10 @@ type OAuthFlow struct {
 	Extensions map[string]any `json:"-" yaml:"-"`
 	Origin     *Origin        `json:"origin,omitempty" yaml:"origin,omitempty"`
 
-	AuthorizationURL string    `json:"authorizationUrl,omitempty" yaml:"authorizationUrl,omitempty"`
-	TokenURL         string    `json:"tokenUrl,omitempty" yaml:"tokenUrl,omitempty"`
-	RefreshURL       string    `json:"refreshUrl,omitempty" yaml:"refreshUrl,omitempty"`
-	Scopes           StringMap `json:"scopes" yaml:"scopes"` // required
+	AuthorizationURL string            `json:"authorizationUrl,omitempty" yaml:"authorizationUrl,omitempty"`
+	TokenURL         string            `json:"tokenUrl,omitempty" yaml:"tokenUrl,omitempty"`
+	RefreshURL       string            `json:"refreshUrl,omitempty" yaml:"refreshUrl,omitempty"`
+	Scopes           StringMap[string] `json:"scopes" yaml:"scopes"` // required
 }
     OAuthFlow is specified by OpenAPI/Swagger standard version 3. See
     https://github.com/OAI/OpenAPI-Specification/blob/main/versions/3.0.3.md#oauth-flow-object
@@ -2067,11 +2076,11 @@ func NewRegexpFormatValidator(pattern string) StringFormatValidator
     NewRegexpFormatValidator creates a new FormatValidator that uses a regular
     expression to validate the value.
 
-type StringMap map[string]string
+type StringMap[V any] map[string]V
     StringMap is a map[string]string that ignores the origin in the underlying
     json representation.
 
-func (stringMap *StringMap) UnmarshalJSON(data []byte) (err error)
+func (stringMap *StringMap[V]) UnmarshalJSON(data []byte) (err error)
     UnmarshalJSON sets StringMap to a copy of data.
 
 type T struct {

diff --git a/openapi3/discriminator.go b/openapi3/discriminator.go
@@ -11,8 +11,22 @@ type Discriminator struct {
 	Extensions map[string]any `json:"-" yaml:"-"`
 	Origin     *Origin        `json:"origin,omitempty" yaml:"origin,omitempty"`
 
-	PropertyName string    `json:"propertyName" yaml:"propertyName"` // required
-	Mapping      StringMap `json:"mapping,omitempty" yaml:"mapping,omitempty"`
+	PropertyName string                `json:"propertyName" yaml:"propertyName"` // required
+	Mapping      StringMap[MappingRef] `json:"mapping,omitempty" yaml:"mapping,omitempty"`
+}
+
+// MappingRef is a ref to a Schema objects. Unlike SchemaRefs it is serialised
+// as a plain string instead of an object with a $ref key, as such it also does
+// not support extensions.
+type MappingRef SchemaRef
+
+func (mr *MappingRef) UnmarshalText(data []byte) error {
+	mr.Ref = string(data)
+	return nil
+}
+
+func (mr MappingRef) MarshalText() ([]byte, error) {
+	return []byte(mr.Ref), nil
 }
 
 // MarshalJSON returns the JSON encoding of Discriminator.

diff --git a/openapi3/internalize_refs.go b/openapi3/internalize_refs.go
@@ -351,6 +351,16 @@ func (doc *T) derefSchema(s *Schema, refNameResolver RefNameResolver, parentIsEx
 			}
 		}
 	}
+	// Discriminator mapping values are special cases since they are not full
+	// ref objects but are string references to schema objects.
+	if s.Discriminator != nil && s.Discriminator.Mapping != nil {
+		for k, mapRef := range s.Discriminator.Mapping {
+			s2 := (*SchemaRef)(&mapRef)
+			isExternal := doc.addSchemaToSpec(s2, refNameResolver, parentIsExternal)
+			doc.derefSchema(s2.Value, refNameResolver, isExternal || parentIsExternal)
+			s.Discriminator.Mapping[k] = mapRef
+		}
+	}
 
 	for _, name := range componentNames(s.Properties) {
 		s2 := s.Properties[name]

diff --git a/openapi3/internalize_refs_test.go b/openapi3/internalize_refs_test.go
@@ -25,6 +25,7 @@ func TestInternalizeRefs(t *testing.T) {
 		{"testdata/issue831/testref.internalizepath.openapi.yml"},
 		{"testdata/issue959/openapi.yml"},
 		{"testdata/interalizationNameCollision/api.yml"},
+		{"testdata/discriminator.yml"},
 	}
 
 	for _, test := range tests {

diff --git a/openapi3/loader.go b/openapi3/loader.go
@@ -953,6 +953,16 @@ func (loader *Loader) resolveSchemaRef(doc *T, component *SchemaRef, documentPat
 			return err
 		}
 	}
+	// Discriminator mapping refs are a special case since they are not full
+	// ref objects but are plain strings that reference schema objects.
+	if value.Discriminator != nil && value.Discriminator.Mapping != nil {
+		for k, v := range value.Discriminator.Mapping {
+			if err := loader.resolveSchemaRef(doc, (*SchemaRef)(&v), documentPath, visited); err != nil {
+				return err
+			}
+			value.Discriminator.Mapping[k] = v
+		}
+	}
 	return nil
 }
 

diff --git a/openapi3/schema.go b/openapi3/schema.go
@@ -1298,7 +1298,7 @@ func (schema *Schema) visitNotOperation(settings *schemaValidationSettings, valu
 func (schema *Schema) visitXOFOperations(settings *schemaValidationSettings, value any) (err error, run bool) {
 	var visitedOneOf, visitedAnyOf, visitedAllOf bool
 	if v := schema.OneOf; len(v) > 0 {
-		var discriminatorRef string
+		var discriminatorRef MappingRef
 		if schema.Discriminator != nil {
 			pn := schema.Discriminator.PropertyName
 			if valuemap, okcheck := value.(map[string]any); okcheck {
@@ -1344,7 +1344,7 @@ func (schema *Schema) visitXOFOperations(settings *schemaValidationSettings, val
 				return foundUnresolvedRef(item.Ref), false
 			}
 
-			if discriminatorRef != "" && discriminatorRef != item.Ref {
+			if discriminatorRef.Ref != "" && discriminatorRef.Ref != item.Ref {
 				continue
 			}
 

diff --git a/openapi3/security_scheme.go b/openapi3/security_scheme.go
@@ -322,10 +322,10 @@ type OAuthFlow struct {
 	Extensions map[string]any `json:"-" yaml:"-"`
 	Origin     *Origin        `json:"origin,omitempty" yaml:"origin,omitempty"`
 
-	AuthorizationURL string    `json:"authorizationUrl,omitempty" yaml:"authorizationUrl,omitempty"`
-	TokenURL         string    `json:"tokenUrl,omitempty" yaml:"tokenUrl,omitempty"`
-	RefreshURL       string    `json:"refreshUrl,omitempty" yaml:"refreshUrl,omitempty"`
-	Scopes           StringMap `json:"scopes" yaml:"scopes"` // required
+	AuthorizationURL string            `json:"authorizationUrl,omitempty" yaml:"authorizationUrl,omitempty"`
+	TokenURL         string            `json:"tokenUrl,omitempty" yaml:"tokenUrl,omitempty"`
+	RefreshURL       string            `json:"refreshUrl,omitempty" yaml:"refreshUrl,omitempty"`
+	Scopes           StringMap[string] `json:"scopes" yaml:"scopes"` // required
 }
 
 // MarshalJSON returns the JSON encoding of OAuthFlow.

diff --git a/openapi3/stringmap.go b/openapi3/stringmap.go
@@ -3,11 +3,11 @@ package openapi3
 import "encoding/json"
 
 // StringMap is a map[string]string that ignores the origin in the underlying json representation.
-type StringMap map[string]string
+type StringMap[V any] map[string]V
 
 // UnmarshalJSON sets StringMap to a copy of data.
-func (stringMap *StringMap) UnmarshalJSON(data []byte) (err error) {
-	*stringMap, _, err = unmarshalStringMap[string](data)
+func (stringMap *StringMap[V]) UnmarshalJSON(data []byte) (err error) {
+	*stringMap, _, err = unmarshalStringMap[V](data)
 	return
 }
 

diff --git a/openapi3/testdata/discriminator.yml b/openapi3/testdata/discriminator.yml
@@ -0,0 +1,24 @@
+openapi: 3.1.0
+info:
+  title: foo
+  version: 1.0.0
+paths:
+  /:
+    get:
+      operationId: list
+      responses:
+        "200":
+          description: list
+          content:
+            application/json:
+              schema:
+                type: array
+                items:
+                  oneOf:
+                    - $ref: "./ext.yml#/schemas/Foo"
+                    - $ref: "./ext.yml#/schemas/Bar"
+                  discriminator:
+                    propertyName: cat
+                    mapping:
+                      foo: "./ext.yml#/schemas/Foo"
+                      bar: "./ext.yml#/schemas/Bar"
diff --git a/openapi3/testdata/discriminator.yml.internalized.yml b/openapi3/testdata/discriminator.yml.internalized.yml
@@ -0,0 +1,75 @@
+{
+    "openapi": "3.1.0",
+    "info": {
+        "title": "foo",
+        "version": "1.0.0"
+    },
+    "paths": {
+        "/": {
+            "get": {
+                "operationId": "list",
+                "responses": {
+                    "200": {
+                        "description": "list",
+                        "content": {
+                            "application/json": {
+                                "schema": {
+                                    "type": "array",
+                                    "items": {
+                                        "oneOf": [
+                                            {
+                                                "$ref": "#/components/schemas/ext_schemas_Foo"
+                                            },
+                                            {
+                                                "$ref": "#/components/schemas/ext_schemas_Bar"
+                                            }
+                                        ],
+                                        "discriminator": {
+                                            "propertyName": "cat",
+                                            "mapping": {
+                                                "foo": "#/components/schemas/ext_schemas_Foo",
+                                                "bar": "#/components/schemas/ext_schemas_Bar"
+                                            }
+                                        }
+                                    }
+                                }
+                            }
+                        }
+                    }
+                }
+            }
+        }
+    },
+    "components": {
+        "schemas": {
+            "ext_schemas_Foo": {
+                "type": "object",
+                "properties": {
+                    "cat": {
+                        "type": "string",
+                        "enum": [
+                            "foo"
+                        ]
+                    },
+                    "name": {
+                        "type": "string"
+                    }
+                }
+            },
+            "ext_schemas_Bar": {
+                "type": "object",
+                "properties": {
+                    "cat": {
+                        "type": "string",
+                        "enum": [
+                            "bar"
+                        ]
+                    },
+                    "other": {
+                        "type": "string"
+                    }
+                }
+            }
+        }
+    }
+}
diff --git a/openapi3/testdata/ext.yml b/openapi3/testdata/ext.yml
@@ -0,0 +1,17 @@
+schemas:
+  Foo:
+    type: object
+    properties:
+      cat:
+        type: string
+        enum: [ "foo" ]
+      name:
+        type: string
+  Bar:
+    type: object
+    properties:
+      cat:
+        type: string
+        enum: [ "bar" ]
+      other:
+        type: string