Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fixed panic when body type changed from array to object #357

Merged
4 changes: 3 additions & 1 deletion BREAKING-CHANGES-EXAMPLES.md
Original file line number Diff line number Diff line change
Expand Up @@ -200,6 +200,8 @@ These examples are automatically generated from unit tests.
[changing optional response property to write-only](checker/check-response-optional-property-write-only-read-only_test.go?plain=1#L11)
[changing optional response write-only property to required](checker/check-response-property-became-required_test.go?plain=1#L33)
[changing pattern of request parameters](checker/check-request-parameter-pattern-added-or-changed_test.go?plain=1#L11)
[changing request body and property types from array to object](checker/check-request-property-type-changed_test.go?plain=1#L84)
[changing request body and property types from object to array](checker/check-request-property-type-changed_test.go?plain=1#L116)
[changing request body default value](checker/check-request-property-default-value-changed_test.go?plain=1#L11)
[changing request body to not nullable](checker/check-request-property-became-not-nuallable_test.go?plain=1#L84)
[changing request body to nullable](checker/check-request-property-became-not-nuallable_test.go?plain=1#L58)
Expand All @@ -211,7 +213,7 @@ These examples are automatically generated from unit tests.
[changing request path parameter format](checker/check-request-parameters-type-changed_test.go?plain=1#L86)
[changing request path parameter type](checker/check-request-parameters-type-changed_test.go?plain=1#L11)
[changing request property default value](checker/check-request-property-default-value-changed_test.go?plain=1#L34)
[changing request property format](checker/check-request-property-type-changed_test.go?plain=1#L84)
[changing request property format](checker/check-request-property-type-changed_test.go?plain=1#L148)
[changing request property pattern](checker/check-request-property-pattern-added-or-changed_test.go?plain=1#L11)
[changing request property required value to false](checker/check-request-property-required-updated_test.go?plain=1#L34)
[changing request property required value to true](checker/check-request-property-required-updated_test.go?plain=1#L11)
Expand Down
3 changes: 3 additions & 0 deletions checker/check-request-property-type-changed.go
Original file line number Diff line number Diff line change
Expand Up @@ -49,6 +49,9 @@ func RequestPropertyTypeChangedCheck(diffReport *diff.Diff, operationsSources *d
CheckModifiedPropertiesDiff(
mediaTypeDiff.SchemaDiff,
func(propertyPath string, propertyName string, propertyDiff *diff.SchemaDiff, parent *diff.SchemaDiff) {
if propertyDiff.Revision == nil || propertyDiff.Revision.Value == nil {
return
}
if propertyDiff.Revision.Value.ReadOnly {
return
}
Expand Down
64 changes: 64 additions & 0 deletions checker/check-request-property-type-changed_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -81,6 +81,70 @@ func TestRequestPropertyTypeChangedCheck(t *testing.T) {
}, errs[0])
}

// CL: changing request body and property types from array to object
func TestRequestBodyAndPropertyTypesChangedCheckArrayToObject(t *testing.T) {
s1, err := open("../data/checker/request_property_type_changed_base_array_to_object.yaml")
require.NoError(t, err)
s2, err := open("../data/checker/request_property_type_changed_revision_array_to_object.yaml")
require.NoError(t, err)

d, osm, err := diff.GetWithOperationsSourcesMap(getConfig(), s1, s2)
require.NoError(t, err)

errs := checker.CheckBackwardCompatibilityUntilLevel(singleCheckConfig(checker.RequestPropertyTypeChangedCheck), d, osm, checker.ERR)
require.Len(t, errs, 2)
require.Equal(t, checker.ApiChange{
Id: "request-property-type-changed",
Level: checker.ERR,
Text: "the 'colors' request property type/format changed from 'array'/'none' to 'object'/'none'",
Operation: "POST",
Path: "/dogs",
Source: "../data/checker/request_property_type_changed_revision_array_to_object.yaml",
OperationId: "addDog",
}, errs[0])
require.Equal(t, checker.ApiChange{
Id: "request-body-type-changed",
Level: checker.ERR,
Text: "the request's body type/format changed from 'array'/'none' to 'object'/'none'",
Operation: "POST",
Path: "/pets",
Source: "../data/checker/request_property_type_changed_revision_array_to_object.yaml",
OperationId: "addPet",
}, errs[1])
}

// CL: changing request body and property types from object to array
func TestRequestBodyAndPropertyTypesChangedCheckObjectToArray(t *testing.T) {
s1, err := open("../data/checker/request_property_type_changed_revision_array_to_object.yaml")
require.NoError(t, err)
s2, err := open("../data/checker/request_property_type_changed_base_array_to_object.yaml")
require.NoError(t, err)

d, osm, err := diff.GetWithOperationsSourcesMap(getConfig(), s1, s2)
require.NoError(t, err)

errs := checker.CheckBackwardCompatibilityUntilLevel(singleCheckConfig(checker.RequestPropertyTypeChangedCheck), d, osm, checker.ERR)
require.Len(t, errs, 2)
require.Equal(t, checker.ApiChange{
Id: "request-property-type-changed",
Level: checker.ERR,
Text: "the 'colors' request property type/format changed from 'object'/'none' to 'array'/'none'",
Operation: "POST",
Path: "/dogs",
Source: "../data/checker/request_property_type_changed_base_array_to_object.yaml",
OperationId: "addDog",
}, errs[0])
require.Equal(t, checker.ApiChange{
Id: "request-body-type-changed",
Level: checker.ERR,
Text: "the request's body type/format changed from 'object'/'none' to 'array'/'none'",
Operation: "POST",
Path: "/pets",
Source: "../data/checker/request_property_type_changed_base_array_to_object.yaml",
OperationId: "addPet",
}, errs[1])
}

// CL: changing request property format
func TestRequestPropertyFormatChangedCheck(t *testing.T) {
s1, err := open("../data/checker/request_property_type_changed_base.yaml")
Expand Down
4 changes: 2 additions & 2 deletions checker/localizations/localizations.go

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

4 changes: 2 additions & 2 deletions checker/localizations_src/ru/messages.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -28,8 +28,8 @@ sunset-deleted: удалена дата sunset date у API, но сохранё
api-sunset-date-changed-too-small: дата sunset у API изменена на более раннюю с %s на %s, новая дата sunset должна быть либо не раньше %s, либо, как минимум, %d дней от текущего дня
new-required-request-parameter: добавлен новый обязательный %s параметр зароса %s
new-optional-request-parameter: добавлен новый необязательный %s параметр зароса %s
new-required-request-property: добавлено новле обязательное поле запроса %s
new-optional-request-property: добавлено новле необязательное поле запроса %s
new-required-request-property: добавлено новое обязательное поле запроса %s
new-optional-request-property: добавлено новое необязательное поле запроса %s
new-required-request-header-property: в заголовке запроса %s добавлено новое обязательное поле %s
request-body-became-required: тело запроса стало обязательным
request-body-became-optional: тело запроса стало необязательным
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,46 @@
openapi: 3.0.1
info:
title: Sample API
version: 1.0.0
paths:
/pets:
post:
summary: Add a new pet
operationId: addPet
requestBody:
required: true
content:
application/json:
schema:
type: array
items:
type: object
properties:
name:
type: string
format: string
age:
type: integer
format: int32
required:
- name
/dogs:
post:
summary: Add a new dog
operationId: addDog
requestBody:
required: true
content:
application/json:
schema:
type: object
properties:
name:
type: string
format: string
colors:
type: array
items:
type: integer
required:
- name
Original file line number Diff line number Diff line change
@@ -0,0 +1,61 @@
openapi: 3.0.1
info:
title: Sample API
version: 1.0.0
paths:
/pets:
post:
summary: Add a new pet
operationId: addPet
requestBody:
required: true
content:
application/json:
schema:
type: object
required:
- data
properties:
data:
type: array
items:
type: object
properties:
name:
type: string
format: string
age:
type: integer
format: int32
required:
- name
/dogs:
post:
summary: Add a new dog
operationId: addDog
requestBody:
required: true
content:
application/json:
schema:
type: object
properties:
name:
type: string
format: string
colors:
type: object
properties:
red:
type: integer
green:
type: integer
blue:
type: integer
required:
- red
- blue
- green
required:
- name

59 changes: 17 additions & 42 deletions diff/schema_diff.go
Original file line number Diff line number Diff line change
Expand Up @@ -78,23 +78,29 @@ func getSchemaDiff(config *Config, state *state, schema1, schema2 *openapi3.Sche
}

func getSchemaDiffInternal(config *Config, state *state, schema1, schema2 *openapi3.SchemaRef) (*SchemaDiff, error) {
if status := getSchemaStatus(schema1, schema2); status != schemaStatusOK {
switch status {
case schemaStatusNoSchemas:
return nil, nil
case schemaStatusSchemaAdded:
return &SchemaDiff{SchemaAdded: true}, nil
case schemaStatusSchemaDeleted:
return &SchemaDiff{SchemaDeleted: true}, nil
}

result := SchemaDiff{
Base: schema1,
Revision: schema2,
}

if schema1 == nil && schema2 == nil {
return &result, nil
} else if schema1 == nil {
result.SchemaAdded = true
return &result, nil
} else if schema2 == nil {
result.SchemaDeleted = true
return &result, nil
}

if status := getCircularRefsDiff(state.visitedSchemasBase, state.visitedSchemasRevision, schema1, schema2); status != circularRefStatusNone {
switch status {
case circularRefStatusDiff:
return &SchemaDiff{CircularRefDiff: true}, nil
result.CircularRefDiff = true
return &result, nil
case circularRefStatusNoDiff:
return nil, nil
return &result, nil
}
}

Expand All @@ -119,8 +125,6 @@ func getSchemaDiffInternal(config *Config, state *state, schema1, schema2 *opena
defer state.visitedSchemasRevision.Remove(schema2.Ref)
}

result := SchemaDiff{}

result.ExtensionsDiff = getExtensionsDiff(config, state, value1.Extensions, value2.Extensions)
result.OneOfDiff, err = getSchemaListsDiff(config, state, value1.OneOf, value2.OneOf)
if err != nil {
Expand Down Expand Up @@ -186,38 +190,9 @@ func getSchemaDiffInternal(config *Config, state *state, schema1, schema2 *opena

result.DiscriminatorDiff = getDiscriminatorDiff(config, state, value1.Discriminator, value2.Discriminator)

result.Base = schema1
result.Revision = schema2

return &result, nil
}

type schemaStatus int

const (
schemaStatusOK schemaStatus = iota
schemaStatusNoSchemas
schemaStatusSchemaAdded
schemaStatusSchemaDeleted
schemaStatusCircularRefDiff
)

func getSchemaStatus(schema1, schema2 *openapi3.SchemaRef) schemaStatus {
if schema1 == nil && schema2 == nil {
return schemaStatusNoSchemas
}

if schema1 == nil && schema2 != nil {
return schemaStatusSchemaAdded
}

if schema1 != nil && schema2 == nil {
return schemaStatusSchemaDeleted
}

return schemaStatusOK
}

func derefSchema(ref *openapi3.SchemaRef) (*openapi3.Schema, error) {
if ref == nil || ref.Value == nil {
return nil, errors.New("schema reference is nil")
Expand Down