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 17, 2024 · acf2dba · acf2dba
1 parent 56505dc
commit acf2dba
Show file tree

Hide file tree

Showing 9 changed files with 166 additions and 6 deletions.
diff --git a/.github/docs/openapi3.txt b/.github/docs/openapi3.txt
@@ -400,8 +400,8 @@ func (content Content) Validate(ctx context.Context, opts ...ValidationOption) e
 type Discriminator struct {
 	Extensions map[string]any `json:"-" yaml:"-"`
 
-	PropertyName string    `json:"propertyName" yaml:"propertyName"` // required
-	Mapping      StringMap `json:"mapping,omitempty" yaml:"mapping,omitempty"`
+	PropertyName string                `json:"propertyName" yaml:"propertyName"` // required
+	Mapping      map[string]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
@@ -793,6 +793,15 @@ func (loader *Loader) LoadFromURI(location *url.URL) (*T, error)
 func (loader *Loader) ResolveRefsIn(doc *T, location *url.URL) (err error)
     ResolveRefsIn expands references if for instance spec was just unmarshaled
 
+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:"-"`
 

diff --git a/openapi3/discriminator.go b/openapi3/discriminator.go
@@ -10,8 +10,22 @@ import (
 type Discriminator struct {
 	Extensions map[string]any `json:"-" yaml:"-"`
 
-	PropertyName string    `json:"propertyName" yaml:"propertyName"` // required
-	Mapping      StringMap `json:"mapping,omitempty" yaml:"mapping,omitempty"`
+	PropertyName string                `json:"propertyName" yaml:"propertyName"` // required
+	Mapping      map[string]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
@@ -950,6 +950,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
@@ -1296,7 +1296,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 {
@@ -1342,7 +1342,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/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