diff --git a/.github/docs/openapi3.txt b/.github/docs/openapi3.txt index 7458a1b1..28e1d994 100644 --- a/.github/docs/openapi3.txt +++ b/.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 { diff --git a/openapi3/discriminator.go b/openapi3/discriminator.go index 6934c170..faf53dcd 100644 --- 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/discriminator_mapping_refs_test.go b/openapi3/discriminator_mapping_refs_test.go new file mode 100644 index 00000000..19579e8c --- /dev/null +++ b/openapi3/discriminator_mapping_refs_test.go @@ -0,0 +1,57 @@ +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/...") +} diff --git a/openapi3/internalize_refs.go b/openapi3/internalize_refs.go index b725bafc..5799c3c3 100644 --- a/openapi3/internalize_refs.go +++ b/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) diff --git a/openapi3/internalize_refs_test.go b/openapi3/internalize_refs_test.go index 6e9853ac..967ebca5 100644 --- 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 index 67674fc6..be9591ba 100644 --- a/openapi3/loader.go +++ b/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 } diff --git a/openapi3/schema.go b/openapi3/schema.go index 6f6f39b3..4fce3d8f 100644 --- a/openapi3/schema.go +++ b/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 } diff --git a/openapi3/security_scheme.go b/openapi3/security_scheme.go index 676002aa..b874ccae 100644 --- 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 index d1b91ac9..10e7e6de 100644 --- 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 new file mode 100644 index 00000000..b83b34c8 --- /dev/null +++ b/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" + diff --git a/openapi3/testdata/discriminator.yml.internalized.yml b/openapi3/testdata/discriminator.yml.internalized.yml new file mode 100644 index 00000000..d18ba740 --- /dev/null +++ b/openapi3/testdata/discriminator.yml.internalized.yml @@ -0,0 +1,76 @@ +{ + "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 new file mode 100644 index 00000000..c0caaf3c --- /dev/null +++ b/openapi3/testdata/ext.yml @@ -0,0 +1,18 @@ +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 +