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.

Sign up for GitHub

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

Jump to bottom

Fixed issue with optional parameters when diffing #237

Merged
merged 3 commits into from
Jan 20, 2024
Merged

Fixed issue with optional parameters when diffing #237

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
7d5b580
Fixed issue with optional parameters when diffing
daveshanley Jan 19, 2024
cd46a8b
Fixed tests
daveshanley Jan 19, 2024
c7bc75b
added missing test coverage
daveshanley Jan 20, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
18 changes: 16 additions & 2 deletions what-changed/model/operation.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -262,8 +262,15 @@ func CompareOperations(l, r any) *OperationChanges {
nil)
}
if lParamsUntyped.IsEmpty() && !rParamsUntyped.IsEmpty() {
rParams := rParamsUntyped.Value.([]low.ValueReference[*v2.Parameter])
breaking := false
for i := range rParams {
if rParams[i].Value.Required.Value {
breaking = true
}
}
CreateChange(&changes, PropertyAdded, v3.ParametersLabel,
nil, rParamsUntyped.ValueNode, true, nil,
nil, rParamsUntyped.ValueNode, breaking, nil,
rParamsUntyped.Value)
}

Expand Down Expand Up @@ -358,8 +365,15 @@ func CompareOperations(l, r any) *OperationChanges {
nil)
}
if lParamsUntyped.IsEmpty() && !rParamsUntyped.IsEmpty() {
rParams := rParamsUntyped.Value.([]low.ValueReference[*v3.Parameter])
breaking := false
for i := range rParams {
if rParams[i].Value.Required.Value {
breaking = true
}
}
CreateChange(&changes, PropertyAdded, v3.ParametersLabel,
nil, rParamsUntyped.ValueNode, true, nil,
nil, rParamsUntyped.ValueNode, breaking, nil,
rParamsUntyped.Value)
}

Expand Down
120 changes: 118 additions & 2 deletions what-changed/model/operation_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -155,7 +155,7 @@ parameters:
extChanges := CompareOperations(&lDoc, &rDoc)
assert.Equal(t, 1, extChanges.TotalChanges())
assert.Len(t, extChanges.GetAllChanges(), 1)
assert.Equal(t, 1, extChanges.TotalBreakingChanges())
assert.Equal(t, 0, extChanges.TotalBreakingChanges())
assert.Equal(t, PropertyAdded, extChanges.Changes[0].ChangeType)
assert.Equal(t, "parameters", extChanges.Changes[0].Property)

Expand Down Expand Up @@ -977,7 +977,7 @@ parameters:
extChanges := CompareOperations(&rDoc, &lDoc)
assert.Equal(t, 1, extChanges.TotalChanges())
assert.Len(t, extChanges.GetAllChanges(), 1)
assert.Equal(t, 1, extChanges.TotalBreakingChanges())
assert.Equal(t, 0, extChanges.TotalBreakingChanges())
assert.Equal(t, PropertyAdded, extChanges.Changes[0].ChangeType)
}
func TestCompareOperations_V3_ModifyTag(t *testing.T) {
Expand Down Expand Up @@ -1588,3 +1588,119 @@ requestBody:
assert.Equal(t, 1, extChanges.TotalBreakingChanges())
assert.Equal(t, PropertyRemoved, extChanges.Changes[0].ChangeType)
}

func TestComparePathItem_V3_AddOptionalParam(t *testing.T) {

left := `operationId: listBurgerDressings`

right := `operationId: listBurgerDressings
parameters:
- in: head
name: burgerId
required: false`

var lNode, rNode yaml.Node
_ = yaml.Unmarshal([]byte(left), &lNode)
_ = yaml.Unmarshal([]byte(right), &rNode)

// create low level objects
var lDoc v3.Operation
var rDoc v3.Operation
_ = low.BuildModel(lNode.Content[0], &lDoc)
_ = low.BuildModel(rNode.Content[0], &rDoc)
_ = lDoc.Build(context.Background(), nil, lNode.Content[0], nil)
_ = rDoc.Build(context.Background(), nil, rNode.Content[0], nil)

// compare.
extChanges := CompareOperations(&lDoc, &rDoc)
assert.Equal(t, 1, extChanges.TotalChanges())
assert.Len(t, extChanges.GetAllChanges(), 1)
assert.Equal(t, 0, extChanges.TotalBreakingChanges())
}

func TestComparePathItem_V2_AddOptionalParam(t *testing.T) {

left := `operationId: listBurgerDressings`

right := `operationId: listBurgerDressings
parameters:
- in: head
name: burgerId
required: false`

var lNode, rNode yaml.Node
_ = yaml.Unmarshal([]byte(left), &lNode)
_ = yaml.Unmarshal([]byte(right), &rNode)

// create low level objects
var lDoc v2.Operation
var rDoc v2.Operation
_ = low.BuildModel(lNode.Content[0], &lDoc)
_ = low.BuildModel(rNode.Content[0], &rDoc)
_ = lDoc.Build(context.Background(), nil, lNode.Content[0], nil)
_ = rDoc.Build(context.Background(), nil, rNode.Content[0], nil)

// compare.
extChanges := CompareOperations(&lDoc, &rDoc)
assert.Equal(t, 1, extChanges.TotalChanges())
assert.Len(t, extChanges.GetAllChanges(), 1)
assert.Equal(t, 0, extChanges.TotalBreakingChanges())
}

func TestComparePathItem_V3_AddRequiredParam(t *testing.T) {

left := `operationId: listBurgerDressings`

right := `operationId: listBurgerDressings
parameters:
- in: head
name: burgerId
required: true`

var lNode, rNode yaml.Node
_ = yaml.Unmarshal([]byte(left), &lNode)
_ = yaml.Unmarshal([]byte(right), &rNode)

// create low level objects
var lDoc v3.Operation
var rDoc v3.Operation
_ = low.BuildModel(lNode.Content[0], &lDoc)
_ = low.BuildModel(rNode.Content[0], &rDoc)
_ = lDoc.Build(context.Background(), nil, lNode.Content[0], nil)
_ = rDoc.Build(context.Background(), nil, rNode.Content[0], nil)

// compare.
extChanges := CompareOperations(&lDoc, &rDoc)
assert.Equal(t, 1, extChanges.TotalChanges())
assert.Len(t, extChanges.GetAllChanges(), 1)
assert.Equal(t, 1, extChanges.TotalBreakingChanges())
}

func TestComparePathItem_V2_AddRequiredParam(t *testing.T) {

left := `operationId: listBurgerDressings`

right := `operationId: listBurgerDressings
parameters:
- in: head
name: burgerId
required: true`

var lNode, rNode yaml.Node
_ = yaml.Unmarshal([]byte(left), &lNode)
_ = yaml.Unmarshal([]byte(right), &rNode)

// create low level objects
var lDoc v2.Operation
var rDoc v2.Operation
_ = low.BuildModel(lNode.Content[0], &lDoc)
_ = low.BuildModel(rNode.Content[0], &rDoc)
_ = lDoc.Build(context.Background(), nil, lNode.Content[0], nil)
_ = rDoc.Build(context.Background(), nil, rNode.Content[0], nil)

// compare.
extChanges := CompareOperations(&lDoc, &rDoc)
assert.Equal(t, 1, extChanges.TotalChanges())
assert.Len(t, extChanges.GetAllChanges(), 1)
assert.Equal(t, 1, extChanges.TotalBreakingChanges())
}
20 changes: 18 additions & 2 deletions what-changed/model/path_item.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -335,8 +335,16 @@ func compareSwaggerPathItem(lPath, rPath *v2.PathItem, changes *[]*Change, pc *P
nil)
}
if lPath.Parameters.IsEmpty() && !rPath.Parameters.IsEmpty() {
breaking := false
for i := range rPath.Parameters.Value {
param := rPath.Parameters.Value[i].Value
if param.Required.Value {
breaking = true
break
}
}
CreateChange(changes, PropertyAdded, v3.ParametersLabel,
nil, rPath.Parameters.ValueNode, true, nil,
nil, rPath.Parameters.ValueNode, breaking, nil,
rPath.Parameters.Value)
}

Expand Down Expand Up @@ -589,8 +597,16 @@ func compareOpenAPIPathItem(lPath, rPath *v3.PathItem, changes *[]*Change, pc *P
nil)
}
if lPath.Parameters.IsEmpty() && !rPath.Parameters.IsEmpty() {
breaking := false
for i := range rPath.Parameters.Value {
param := rPath.Parameters.Value[i].Value
if param.Required.Value {
breaking = true
break
}
}
CreateChange(changes, PropertyAdded, v3.ParametersLabel,
nil, rPath.Parameters.ValueNode, true, nil,
nil, rPath.Parameters.ValueNode, breaking, nil,
rPath.Parameters.Value)
}

Expand Down
107 changes: 94 additions & 13 deletions what-changed/model/path_item_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -259,7 +259,7 @@ parameters:
// compare.
extChanges := ComparePathItems(&lDoc, &rDoc)
assert.Equal(t, 1, extChanges.TotalChanges())
assert.Equal(t, 1, extChanges.TotalBreakingChanges())
assert.Equal(t, 0, extChanges.TotalBreakingChanges())
assert.Equal(t, PropertyAdded, extChanges.Changes[0].ChangeType)
assert.Equal(t, v3.ParametersLabel, extChanges.Changes[0].Property)

Expand Down Expand Up @@ -519,7 +519,7 @@ parameters:
extChanges := ComparePathItems(&lDoc, &rDoc)
assert.Equal(t, 1, extChanges.TotalChanges())
assert.Len(t, extChanges.GetAllChanges(), 1)
assert.Equal(t, 1, extChanges.TotalBreakingChanges())
assert.Equal(t, 0, extChanges.TotalBreakingChanges())
assert.Equal(t, PropertyAdded, extChanges.Changes[0].ChangeType)
assert.Equal(t, v3.ParametersLabel, extChanges.Changes[0].Property)
}
Expand Down Expand Up @@ -635,19 +635,71 @@ trace:
assert.Equal(t, 8, extChanges.TotalBreakingChanges())
}

func TestComparePathItem_V3_ChangeParam(t *testing.T) {
func TestComparePathItem_V3_AddParam(t *testing.T) {

left := `get:
operationId: listBurgerDressings
parameters:
- in: query
name: burgerId`
left := `summary: hello`

right := `get:
operationId: listBurgerDressings
parameters:
- in: head
name: burgerId`
right := `summary: hello
parameters:
- in: head
name: burgerId`

var lNode, rNode yaml.Node
_ = yaml.Unmarshal([]byte(left), &lNode)
_ = yaml.Unmarshal([]byte(right), &rNode)

// create low level objects
var lDoc v3.PathItem
var rDoc v3.PathItem
_ = low.BuildModel(lNode.Content[0], &lDoc)
_ = low.BuildModel(rNode.Content[0], &rDoc)
_ = lDoc.Build(context.Background(), nil, lNode.Content[0], nil)
_ = rDoc.Build(context.Background(), nil, rNode.Content[0], nil)

// compare.
extChanges := ComparePathItems(&lDoc, &rDoc)
assert.Equal(t, 1, extChanges.TotalChanges())
assert.Len(t, extChanges.GetAllChanges(), 1)
assert.Equal(t, 0, extChanges.TotalBreakingChanges())
}

func TestComparePathItem_V2_AddParamOptional(t *testing.T) {

left := `summary: hello`

right := `summary: hello
parameters:
- in: head
name: burgerId`

var lNode, rNode yaml.Node
_ = yaml.Unmarshal([]byte(left), &lNode)
_ = yaml.Unmarshal([]byte(right), &rNode)

// create low level objects
var lDoc v2.PathItem
var rDoc v2.PathItem
_ = low.BuildModel(lNode.Content[0], &lDoc)
_ = low.BuildModel(rNode.Content[0], &rDoc)
_ = lDoc.Build(context.Background(), nil, lNode.Content[0], nil)
_ = rDoc.Build(context.Background(), nil, rNode.Content[0], nil)

// compare.
extChanges := ComparePathItems(&lDoc, &rDoc)
assert.Equal(t, 1, extChanges.TotalChanges())
assert.Len(t, extChanges.GetAllChanges(), 1)
assert.Equal(t, 0, extChanges.TotalBreakingChanges())
}

func TestComparePathItem_V3_AddParamRequired(t *testing.T) {

left := `summary: hello`

right := `summary: hello
parameters:
- in: head
name: burgerId
required: true`

var lNode, rNode yaml.Node
_ = yaml.Unmarshal([]byte(left), &lNode)
Expand All @@ -667,3 +719,32 @@ func TestComparePathItem_V3_ChangeParam(t *testing.T) {
assert.Len(t, extChanges.GetAllChanges(), 1)
assert.Equal(t, 1, extChanges.TotalBreakingChanges())
}

func TestComparePathItem_V2_AddParamRequired(t *testing.T) {

left := `summary: hello`

right := `summary: hello
parameters:
- in: head
name: burgerId
required: true`

var lNode, rNode yaml.Node
_ = yaml.Unmarshal([]byte(left), &lNode)
_ = yaml.Unmarshal([]byte(right), &rNode)

// create low level objects
var lDoc v2.PathItem
var rDoc v2.PathItem
_ = low.BuildModel(lNode.Content[0], &lDoc)
_ = low.BuildModel(rNode.Content[0], &rDoc)
_ = lDoc.Build(context.Background(), nil, lNode.Content[0], nil)
_ = rDoc.Build(context.Background(), nil, rNode.Content[0], nil)

// compare.
extChanges := ComparePathItems(&lDoc, &rDoc)
assert.Equal(t, 1, extChanges.TotalChanges())
assert.Len(t, extChanges.GetAllChanges(), 1)
assert.Equal(t, 1, extChanges.TotalBreakingChanges())
}