Skip to content

Commit

Permalink
Fixed panic when body type changed from array to object (#357)
Browse files Browse the repository at this point in the history
  • Loading branch information
derbylock authored Aug 8, 2023
1 parent 30738ef commit 0098f73
Show file tree
Hide file tree
Showing 8 changed files with 198 additions and 47 deletions.
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

0 comments on commit 0098f73

Please sign in to comment.