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.

Author: RaduPetreTarean

.github/docs/openapi3.txt

@@ -430,8 +430,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
@@ -847,6 +847,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"`
@@ -929,10 +938,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
@@ -2083,11 +2092,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 {

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.

openapi3/discriminator_mapping_refs_test.go

@@ -0,0 +1,58 @@
+package openapi3
+
+import (
+	"context"
+	"strings"
+	"testing"
+
+	"github.com/stretchr/testify/require"
+)
+
+// TestDiscriminatorMappingRefsInternalize tests that discriminator mapping refs
+// are properly internalized when calling InternalizeRefs.
+// This test demonstrates the issue that discriminator mapping values, which are
+// JSON schema references serialized as plain strings, are not handled by InternalizeRefs.
+func TestDiscriminatorMappingRefsInternalize(t *testing.T) {
+	ctx := context.Background()
+
+	// Load the spec with external discriminator mapping refs
+	sl := NewLoader()
+	sl.IsExternalRefsAllowed = true
+	doc, err := sl.LoadFromFile("testdata/discriminator.yml")
+	require.NoError(t, err, "loading test file")
+	err = doc.Validate(ctx)
+	require.NoError(t, err, "validating spec")
+
+	// Verify the discriminator mapping refs are external before internalization
+	schema := doc.Paths.Value("/").Get.Responses.Status(200).Value.Content.Get("application/json").Schema.Value.Items.Value
+	require.NotNil(t, schema.Discriminator, "discriminator should exist")
+	require.NotNil(t, schema.Discriminator.Mapping, "discriminator mapping should exist")
+
+	// Before internalization, the mapping refs should be external
+	fooMapping := schema.Discriminator.Mapping["foo"].Ref
+	barMapping := schema.Discriminator.Mapping["bar"].Ref
+	t.Logf("Before internalization - foo mapping: %s", fooMapping)
+	t.Logf("Before internalization - bar mapping: %s", barMapping)
+
+	require.True(t, strings.Contains(fooMapping, "ext.yml"), "foo mapping should reference external file before internalization")
+	require.True(t, strings.Contains(barMapping, "ext.yml"), "bar mapping should reference external file before internalization")
+
+	// Internalize the references
+	doc.InternalizeRefs(ctx, nil)
+
+	// Validate the internalized spec
+	err = doc.Validate(ctx)
+	require.NoError(t, err, "validating internalized spec")
+
+	// After internalization, the mapping refs should be internal (#/components/...)
+	schema = doc.Paths.Value("/").Get.Responses.Status(200).Value.Content.Get("application/json").Schema.Value.Items.Value
+	fooMapping = schema.Discriminator.Mapping["foo"].Ref
+	barMapping = schema.Discriminator.Mapping["bar"].Ref
+	t.Logf("After internalization - foo mapping: %s", fooMapping)
+	t.Logf("After internalization - bar mapping: %s", barMapping)
+
+	// This is where the test fails currently - the mapping refs are NOT being internalized
+	require.True(t, strings.HasPrefix(fooMapping, "#/components/"), "foo mapping should be internalized to #/components/...")
+	require.True(t, strings.HasPrefix(barMapping, "#/components/"), "bar mapping should be internalized to #/components/...")
+}
+

openapi3/internalize_refs.go

@@ -345,6 +345,17 @@ 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] = MappingRef(*s2)
+		}
+	}
+
 	for _, name := range componentNames(s.Properties) {
 		s2 := s.Properties[name]
 		isExternal := doc.addSchemaToSpec(s2, refNameResolver, parentIsExternal)

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 {

openapi3/loader.go

@@ -970,6 +970,22 @@ 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.
+	// Only resolve refs that look like external references (contain a path).
+	// Plain schema names like "Dog" or internal refs like "#/components/schemas/Dog"
+	// don't need to be resolved by the loader.
+	if value.Discriminator != nil && value.Discriminator.Mapping != nil {
+		for k, v := range value.Discriminator.Mapping {
+			// Only resolve if it looks like an external ref (contains path separator)
+			if strings.Contains(v.Ref, "/") && !strings.HasPrefix(v.Ref, "#") {
+				if err := loader.resolveSchemaRef(doc, (*SchemaRef)(&v), documentPath, visited); err != nil {
+					return err
+				}
+				value.Discriminator.Mapping[k] = v
+			}
+		}
+	}
 	return nil
 }
 

openapi3/schema.go

@@ -1299,7 +1299,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 {
@@ -1345,7 +1345,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
 			}
 

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.

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
 }
 

openapi3/testdata/discriminator.yml

@@ -0,0 +1,25 @@
+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"
+