From 787fc8f062b7204fed75d14d4c10b01a9b0c5713 Mon Sep 17 00:00:00 2001 From: Jay Shah Date: Tue, 17 Sep 2024 11:19:57 -0400 Subject: [PATCH 1/4] fix: issue unmarshalling when discriminator field is set in openapi2.0 --- .github/docs/openapi3.txt | 2 +- openapi2/issues1010_test.go | 97 ++++++++++++++++++++++++++++++++++ openapi3/discriminator_test.go | 13 ++++- openapi3/schema.go | 32 +++++++++-- 4 files changed, 139 insertions(+), 5 deletions(-) create mode 100644 openapi2/issues1010_test.go diff --git a/.github/docs/openapi3.txt b/.github/docs/openapi3.txt index cfccb97aa..0b5b06e35 100644 --- a/.github/docs/openapi3.txt +++ b/.github/docs/openapi3.txt @@ -1524,7 +1524,7 @@ type Schema struct { MinProps uint64 `json:"minProperties,omitempty" yaml:"minProperties,omitempty"` MaxProps *uint64 `json:"maxProperties,omitempty" yaml:"maxProperties,omitempty"` AdditionalProperties AdditionalProperties `json:"additionalProperties,omitempty" yaml:"additionalProperties,omitempty"` - Discriminator *Discriminator `json:"discriminator,omitempty" yaml:"discriminator,omitempty"` + Discriminator any `json:"discriminator,omitempty" yaml:"discriminator,omitempty"` } Schema is specified by OpenAPI/Swagger 3.0 standard. See https://github.com/OAI/OpenAPI-Specification/blob/main/versions/3.0.3.md#schema-object diff --git a/openapi2/issues1010_test.go b/openapi2/issues1010_test.go new file mode 100644 index 000000000..dfb6fd2a6 --- /dev/null +++ b/openapi2/issues1010_test.go @@ -0,0 +1,97 @@ +package openapi2 + +import ( + "encoding/json" + "testing" + + "github.com/stretchr/testify/require" +) + +func TestIssue1010(t *testing.T) { + v2 := []byte(` +{ + "basePath": "/v2", + "host": "test.example.com", + "info": { + "title": "MyAPI", + "version": "0.1", + "x-info": "info extension" + }, + "paths": { + "/foo": { + "get": { + "operationId": "getFoo", + "responses": { + "200": { + "description": "returns all information", + "schema": { + "$ref": "#/definitions/Pet" + } + }, + "default": { + "description": "OK" + } + }, + "summary": "get foo" + } + } + }, + "schemes": [ + "http" + ], + "swagger": "2.0", + "definitions": { + "Pet": { + "type": "object", + "required": ["petType"], + "properties": { + "petType": { + "type": "string" + }, + "name": { + "type": "string" + }, + "age": { + "type": "integer" + } + }, + "discriminator": "petType" + }, + "Dog": { + "allOf": [ + { + "$ref": "#/definitions/Pet" + }, + { + "type": "object", + "properties": { + "breed": { + "type": "string" + } + } + } + ] + }, + "Cat": { + "allOf": [ + { + "$ref": "#/definitions/Pet" + }, + { + "type": "object", + "properties": { + "color": { + "type": "string" + } + } + } + ] + } + } +} +`) + + var doc2 T + err := json.Unmarshal(v2, &doc2) + require.NoError(t, err) +} diff --git a/openapi3/discriminator_test.go b/openapi3/discriminator_test.go index d56791439..7157fd2bf 100644 --- a/openapi3/discriminator_test.go +++ b/openapi3/discriminator_test.go @@ -1,6 +1,7 @@ package openapi3 import ( + "encoding/json" "testing" "github.com/stretchr/testify/require" @@ -52,5 +53,15 @@ func TestParsingDiscriminator(t *testing.T) { err = doc.Validate(loader.Context) require.NoError(t, err) - require.Len(t, doc.Components.Schemas["MyResponseType"].Value.Discriminator.Mapping, 2) + discriminatorMap, ok := doc.Components.Schemas["MyResponseType"].Value.Discriminator.(map[string]interface{}) + require.True(t, ok) + + marshaledDiscriminator, err := json.Marshal(discriminatorMap) + require.NoError(t, err) + + var discriminator *Discriminator + err = json.Unmarshal(marshaledDiscriminator, &discriminator) + require.NoError(t, err) + + require.Len(t, discriminator.Mapping, 2) } diff --git a/openapi3/schema.go b/openapi3/schema.go index f81196066..b9776ba37 100644 --- a/openapi3/schema.go +++ b/openapi3/schema.go @@ -129,7 +129,7 @@ type Schema struct { MinProps uint64 `json:"minProperties,omitempty" yaml:"minProperties,omitempty"` MaxProps *uint64 `json:"maxProperties,omitempty" yaml:"maxProperties,omitempty"` AdditionalProperties AdditionalProperties `json:"additionalProperties,omitempty" yaml:"additionalProperties,omitempty"` - Discriminator *Discriminator `json:"discriminator,omitempty" yaml:"discriminator,omitempty"` + Discriminator any `json:"discriminator,omitempty" yaml:"discriminator,omitempty"` } type Types []string @@ -1298,7 +1298,33 @@ func (schema *Schema) visitXOFOperations(settings *schemaValidationSettings, val if v := schema.OneOf; len(v) > 0 { var discriminatorRef string if schema.Discriminator != nil { - pn := schema.Discriminator.PropertyName + var discriminator *Discriminator + if descriminatorValuemap, okcheck := schema.Discriminator.(map[string]any); okcheck { + marshaledDiscriminator, err := json.Marshal(descriminatorValuemap) + if err != nil { + return &SchemaError{ + Schema: schema, + SchemaField: "discriminator", + Reason: fmt.Sprintf("unable to marshal the discriminator field in schema: %v", err), + }, false + } + + if err := json.Unmarshal(marshaledDiscriminator, &discriminator); err != nil { + return &SchemaError{ + Schema: schema, + SchemaField: "discriminator", + Reason: fmt.Sprintf("unable to unmarshall the discriminator field in schema: %v", err), + }, false + } + } else { + return &SchemaError{ + Schema: schema, + SchemaField: "discriminator", + Reason: fmt.Sprintf("discriminator is expected to be an object, but received an unknown type"), + }, false + } + + pn := discriminator.PropertyName if valuemap, okcheck := value.(map[string]any); okcheck { discriminatorVal, okcheck := valuemap[pn] if !okcheck { @@ -1319,7 +1345,7 @@ func (schema *Schema) visitXOFOperations(settings *schemaValidationSettings, val }, false } - if discriminatorRef, okcheck = schema.Discriminator.Mapping[discriminatorValString]; len(schema.Discriminator.Mapping) > 0 && !okcheck { + if discriminatorRef, okcheck = discriminator.Mapping[discriminatorValString]; len(discriminator.Mapping) > 0 && !okcheck { return &SchemaError{ Value: discriminatorVal, Schema: schema, From 2e539ed641bac8935388cd679a6cbdd78154669b Mon Sep 17 00:00:00 2001 From: Jay Shah Date: Sun, 22 Sep 2024 20:03:29 -0400 Subject: [PATCH 2/4] revert original approach --- .github/docs/openapi3.txt | 2 +- openapi3/discriminator_test.go | 13 +------------ openapi3/schema.go | 32 +++----------------------------- 3 files changed, 5 insertions(+), 42 deletions(-) diff --git a/.github/docs/openapi3.txt b/.github/docs/openapi3.txt index 0b5b06e35..cfccb97aa 100644 --- a/.github/docs/openapi3.txt +++ b/.github/docs/openapi3.txt @@ -1524,7 +1524,7 @@ type Schema struct { MinProps uint64 `json:"minProperties,omitempty" yaml:"minProperties,omitempty"` MaxProps *uint64 `json:"maxProperties,omitempty" yaml:"maxProperties,omitempty"` AdditionalProperties AdditionalProperties `json:"additionalProperties,omitempty" yaml:"additionalProperties,omitempty"` - Discriminator any `json:"discriminator,omitempty" yaml:"discriminator,omitempty"` + Discriminator *Discriminator `json:"discriminator,omitempty" yaml:"discriminator,omitempty"` } Schema is specified by OpenAPI/Swagger 3.0 standard. See https://github.com/OAI/OpenAPI-Specification/blob/main/versions/3.0.3.md#schema-object diff --git a/openapi3/discriminator_test.go b/openapi3/discriminator_test.go index 7157fd2bf..d56791439 100644 --- a/openapi3/discriminator_test.go +++ b/openapi3/discriminator_test.go @@ -1,7 +1,6 @@ package openapi3 import ( - "encoding/json" "testing" "github.com/stretchr/testify/require" @@ -53,15 +52,5 @@ func TestParsingDiscriminator(t *testing.T) { err = doc.Validate(loader.Context) require.NoError(t, err) - discriminatorMap, ok := doc.Components.Schemas["MyResponseType"].Value.Discriminator.(map[string]interface{}) - require.True(t, ok) - - marshaledDiscriminator, err := json.Marshal(discriminatorMap) - require.NoError(t, err) - - var discriminator *Discriminator - err = json.Unmarshal(marshaledDiscriminator, &discriminator) - require.NoError(t, err) - - require.Len(t, discriminator.Mapping, 2) + require.Len(t, doc.Components.Schemas["MyResponseType"].Value.Discriminator.Mapping, 2) } diff --git a/openapi3/schema.go b/openapi3/schema.go index b9776ba37..f81196066 100644 --- a/openapi3/schema.go +++ b/openapi3/schema.go @@ -129,7 +129,7 @@ type Schema struct { MinProps uint64 `json:"minProperties,omitempty" yaml:"minProperties,omitempty"` MaxProps *uint64 `json:"maxProperties,omitempty" yaml:"maxProperties,omitempty"` AdditionalProperties AdditionalProperties `json:"additionalProperties,omitempty" yaml:"additionalProperties,omitempty"` - Discriminator any `json:"discriminator,omitempty" yaml:"discriminator,omitempty"` + Discriminator *Discriminator `json:"discriminator,omitempty" yaml:"discriminator,omitempty"` } type Types []string @@ -1298,33 +1298,7 @@ func (schema *Schema) visitXOFOperations(settings *schemaValidationSettings, val if v := schema.OneOf; len(v) > 0 { var discriminatorRef string if schema.Discriminator != nil { - var discriminator *Discriminator - if descriminatorValuemap, okcheck := schema.Discriminator.(map[string]any); okcheck { - marshaledDiscriminator, err := json.Marshal(descriminatorValuemap) - if err != nil { - return &SchemaError{ - Schema: schema, - SchemaField: "discriminator", - Reason: fmt.Sprintf("unable to marshal the discriminator field in schema: %v", err), - }, false - } - - if err := json.Unmarshal(marshaledDiscriminator, &discriminator); err != nil { - return &SchemaError{ - Schema: schema, - SchemaField: "discriminator", - Reason: fmt.Sprintf("unable to unmarshall the discriminator field in schema: %v", err), - }, false - } - } else { - return &SchemaError{ - Schema: schema, - SchemaField: "discriminator", - Reason: fmt.Sprintf("discriminator is expected to be an object, but received an unknown type"), - }, false - } - - pn := discriminator.PropertyName + pn := schema.Discriminator.PropertyName if valuemap, okcheck := value.(map[string]any); okcheck { discriminatorVal, okcheck := valuemap[pn] if !okcheck { @@ -1345,7 +1319,7 @@ func (schema *Schema) visitXOFOperations(settings *schemaValidationSettings, val }, false } - if discriminatorRef, okcheck = discriminator.Mapping[discriminatorValString]; len(discriminator.Mapping) > 0 && !okcheck { + if discriminatorRef, okcheck = schema.Discriminator.Mapping[discriminatorValString]; len(schema.Discriminator.Mapping) > 0 && !okcheck { return &SchemaError{ Value: discriminatorVal, Schema: schema, From b0f93cc1f4585441e991498df073b05a5b4be04f Mon Sep 17 00:00:00 2001 From: Jay Shah Date: Sun, 22 Sep 2024 20:07:22 -0400 Subject: [PATCH 3/4] update with different approach --- openapi2/issues1010_test.go | 1 + openapi3/schema.go | 36 +++++++++++++++++++++++++++++++++--- 2 files changed, 34 insertions(+), 3 deletions(-) diff --git a/openapi2/issues1010_test.go b/openapi2/issues1010_test.go index dfb6fd2a6..9b7a93748 100644 --- a/openapi2/issues1010_test.go +++ b/openapi2/issues1010_test.go @@ -94,4 +94,5 @@ func TestIssue1010(t *testing.T) { var doc2 T err := json.Unmarshal(v2, &doc2) require.NoError(t, err) + require.Equal(t, "petType", doc2.Definitions["Pet"].Value.Discriminator.PropertyName) } diff --git a/openapi3/schema.go b/openapi3/schema.go index f81196066..60cf3428c 100644 --- a/openapi3/schema.go +++ b/openapi3/schema.go @@ -404,12 +404,42 @@ func (schema Schema) MarshalYAML() (any, error) { // UnmarshalJSON sets Schema to a copy of data. func (schema *Schema) UnmarshalJSON(data []byte) error { - type SchemaBis Schema + type Alias Schema + type SchemaBis struct { + Alias + Discriminator json.RawMessage `json:"discriminator,omitempty"` + } + var x SchemaBis if err := json.Unmarshal(data, &x); err != nil { return unmarshalError(err) } - _ = json.Unmarshal(data, &x.Extensions) + + type DiscriminatorObject struct { + Discriminator *Discriminator `json:"discriminator,omitempty" yaml:"discriminator,omitempty"` + } + + if len(x.Discriminator) > 0 { + var ds *string + if err := json.Unmarshal(x.Discriminator, &ds); err == nil { + // In OpenAPI 2, the Discriminator is a string field, while in OpenAPI 3, + // it corresponds to the Discriminator.PropertyName field. + // If the OpenAPI 2 Discriminator (ds) is not nil, we assign it to + // the OpenAPI 3 equivalent, which is Discriminator.PropertyName. + if ds != nil { + x.Alias.Discriminator = &Discriminator{ + PropertyName: *ds, + } + } + } else { + var do DiscriminatorObject + if err := json.Unmarshal(x.Discriminator, &do.Discriminator); err == nil && do.Discriminator != nil { + x.Alias.Discriminator = do.Discriminator + } + } + } + + _ = json.Unmarshal(data, &x.Alias.Extensions) delete(x.Extensions, "oneOf") delete(x.Extensions, "anyOf") @@ -464,7 +494,7 @@ func (schema *Schema) UnmarshalJSON(data []byte) error { x.Extensions = nil } - *schema = Schema(x) + *schema = Schema(x.Alias) if schema.Format == "date" { // This is a fix for: https://github.com/getkin/kin-openapi/issues/697 From 78fe7643826084aa28357e8d6293f6b71ff6df6e Mon Sep 17 00:00:00 2001 From: Jay Shah Date: Sun, 22 Sep 2024 20:20:56 -0400 Subject: [PATCH 4/4] fix lint error --- openapi3/schema.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/openapi3/schema.go b/openapi3/schema.go index 60cf3428c..fc201e500 100644 --- a/openapi3/schema.go +++ b/openapi3/schema.go @@ -407,7 +407,7 @@ func (schema *Schema) UnmarshalJSON(data []byte) error { type Alias Schema type SchemaBis struct { Alias - Discriminator json.RawMessage `json:"discriminator,omitempty"` + Discriminator json.RawMessage `json:"discriminator,omitempty" yaml:"discriminator,omitempty"` } var x SchemaBis