Skip to content

Add new rule: 1050 - ParameterLocationHasChanged; Change behavior of ChangedParameterOrder rule with regards to x-ms-parameter-location #334

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

Merged
merged 26 commits into from
Jun 21, 2024
Merged

Add new rule: 1050 - ParameterLocationHasChanged; Change behavior of ChangedParameterOrder rule with regards to x-ms-parameter-location #334

Show file tree
Hide file tree
Changes from 13 commits
Commits
Show all changes
26 commits
Select commit Hold shift + click to select a range
6621033
ongoing
Jun 12, 2024
6914784
add tests for desired param order change behavior
Jun 12, 2024
c9d65ed
refactor and document
Jun 20, 2024
f417632
bugfix
Jun 20, 2024
a1eebb4
ongoing
Jun 20, 2024
4d05e1d
ongoing
Jun 20, 2024
3975ebe
fix .csproj def
Jun 20, 2024
eb3ed79
add comma
Jun 20, 2024
09a0678
fix
Jun 20, 2024
9dde950
remove obsolete todo
Jun 20, 2024
872ec36
refactor
Jun 20, 2024
abeb2ec
comment
Jun 20, 2024
6f448f3
Merge branch 'main' into users/kojamroz/param_order
Jun 20, 2024
266db50
clarify missing behavior and unit tests
Jun 21, 2024
415e61c
bugfix
Jun 21, 2024
6bcbcc7
add doc for new rule 1050: ParameterLocationHasChanged
Jun 21, 2024
9248f5a
add test stub: ParameterLocationChanged
Jun 21, 2024
d6b0f3b
WIP: DetectChangedParameterLocation
Jun 21, 2024
8784f8d
ongoing
Jun 21, 2024
58ecd24
ongoing
Jun 21, 2024
34090c9
fixups
Jun 21, 2024
cc7fc61
doc update
Jun 21, 2024
0814e81
update doc
Jun 21, 2024
7aa66e2
fix doc
Jun 21, 2024
f671370
better doc
Jun 21, 2024
341f76a
add test for removing params
Jun 21, 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
8 changes: 5 additions & 3 deletions openapi-diff/src/modeler/AutoRest.Swagger.Tests/AutoRest.Swagger.Tests.csproj
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8,8 +8,6 @@
<LangVersion>7.1</LangVersion>
</PropertyGroup>



<ItemGroup>
<Content Include="Resource\Swagger\new\added_path.json" />
<Content Include="Resource\Swagger\new\added_readonly_required_property.json" />
Expand All @@ -24,6 +22,8 @@
<Content Include="Resource\Swagger\new\operation_check_03.json" />
<Content Include="Resource\Swagger\new\operation_check_04.json" />
<Content Include="Resource\Swagger\new\operation_check_05.json" />
<Content Include="Resource\Swagger\new\global_parameter_order_change.json" />
<Content Include="Resource\Swagger\new\global_parameter_no_order_change.json" />
<Content Include="Resource\Swagger\new\param_check_01.json" />
<Content Include="Resource\Swagger\new\removed_definition.json" />
<Content Include="Resource\Swagger\new\removed_operation.json" />
Expand All @@ -50,6 +50,8 @@
<Content Include="Resource\Swagger\new\version_check_04.json" />
<Content Include="Resource\Swagger\old\added_readonly_required_property.json" />
<Content Include="Resource\Swagger\old\added_required_property.json" />
<Content Include="Resource\Swagger\old\global_parameter_no_order_change.json" />
<Content Include="Resource\Swagger\old\global_parameter_order_change.json" />
<Content Include="Resource\Swagger\old\recursive_model.json" />
<Content Include="Resource\Swagger\old\changed_operation_id.json" />
<Content Include="Resource\Swagger\old\added_path.json" />
Expand Down Expand Up @@ -94,4 +96,4 @@
</ItemGroup>

<ProjectExtensions><VisualStudio><UserProperties /></VisualStudio></ProjectExtensions>
</Project>
</Project>
67 changes: 67 additions & 0 deletions ...modeler/AutoRest.Swagger.Tests/Resource/Swagger/new/global_parameter_no_order_change.json
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,67 @@
{
"swagger": 2.0,
"info": {
"title": "required_parameter",
"version": "1.0"
},
"host": "localhost:8000",
"schemes": [ "http", "https" ],
"paths": {
"/api/Parameters": {
"get": {
"tag": [ "Parameters" ],
"operationId": "Parameters_Get",
"produces": [
"text/plain"
],
"parameters": [
{
"$ref": "#/parameters/MethodLocationParam1"
},
{
"$ref": "#/parameters/ClientLocationParam"
},
{
"$ref": "#/parameters/MethodLocationParam2"
},
{
"$ref": "#/parameters/ImplicitLocationParam"
}
]
}
}
},
"parameters": {
"ImplicitLocationParam": {
"name": "implicit_location_param",
"in": "query",
"required": true,
"type": "string",
"description": "Implicit x-ms-parameter-location param"
},
"ClientLocationParam": {
"name": "current_location_param",
"in": "query",
"required": true,
"type": "string",
"description": "client x-ms-parameter-location param",
"x-ms-parameter-location": "client"
},
"MethodLocationParam1": {
"name": "method_location_param_1",
"in": "query",
"required": true,
"type": "string",
"description": "method x-ms-parameter-location param 1",
"x-ms-parameter-location": "method"
},
"MethodLocationParam2": {
"name": "method_location_param_2",
"in": "query",
"required": true,
"type": "string",
"description": "method x-ms-parameter-location param 2",
"x-ms-parameter-location": "method"
}
}
}
67 changes: 67 additions & 0 deletions ...rc/modeler/AutoRest.Swagger.Tests/Resource/Swagger/new/global_parameter_order_change.json
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,67 @@
{
"swagger": 2.0,
"info": {
"title": "required_parameter",
"version": "1.0"
},
"host": "localhost:8000",
"schemes": [ "http", "https" ],
"paths": {
"/api/Parameters": {
"get": {
"tag": [ "Parameters" ],
"operationId": "Parameters_Get",
"produces": [
"text/plain"
],
"parameters": [
{
"$ref": "#/parameters/MethodLocationParam2"
},
{
"$ref": "#/parameters/ImplicitLocationParam"
},
{
"$ref": "#/parameters/MethodLocationParam1"
},
{
"$ref": "#/parameters/ClientLocationParam"
}
]
}
}
},
"parameters": {
"ImplicitLocationParam": {
"name": "implicit_location_param",
"in": "query",
"required": true,
"type": "string",
"description": "Implicit x-ms-parameter-location param"
},
"ClientLocationParam": {
"name": "current_location_param",
"in": "query",
"required": true,
"type": "string",
"description": "client x-ms-parameter-location param",
"x-ms-parameter-location": "client"
},
"MethodLocationParam1": {
"name": "method_location_param_1",
"in": "query",
"required": true,
"type": "string",
"description": "method x-ms-parameter-location param 1",
"x-ms-parameter-location": "method"
},
"MethodLocationParam2": {
"name": "method_location_param_2",
"in": "query",
"required": true,
"type": "string",
"description": "method x-ms-parameter-location param 2",
"x-ms-parameter-location": "method"
}
}
}
67 changes: 67 additions & 0 deletions ...modeler/AutoRest.Swagger.Tests/Resource/Swagger/old/global_parameter_no_order_change.json
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,67 @@
{
"swagger": 2.0,
"info": {
"title": "required_parameter",
"version": "1.0"
},
"host": "localhost:8000",
"schemes": [ "http", "https" ],
"paths": {
"/api/Parameters": {
"get": {
"tag": [ "Parameters" ],
"operationId": "Parameters_Get",
"produces": [
"text/plain"
],
"parameters": [
{
"$ref": "#/parameters/ImplicitLocationParam"
},
{
"$ref": "#/parameters/MethodLocationParam1"
},
{
"$ref": "#/parameters/ClientLocationParam"
},
{
"$ref": "#/parameters/MethodLocationParam2"
}
]
}
}
},
"parameters": {
"ImplicitLocationParam": {
"name": "implicit_location_param",
"in": "query",
"required": true,
"type": "string",
"description": "Implicit x-ms-parameter-location param"
},
"ClientLocationParam": {
"name": "current_location_param",
"in": "query",
"required": true,
"type": "string",
"description": "client x-ms-parameter-location param",
"x-ms-parameter-location": "client"
},
"MethodLocationParam1": {
"name": "method_location_param_1",
"in": "query",
"required": true,
"type": "string",
"description": "method x-ms-parameter-location param 1",
"x-ms-parameter-location": "method"
},
"MethodLocationParam2": {
"name": "method_location_param_2",
"in": "query",
"required": true,
"type": "string",
"description": "method x-ms-parameter-location param 2",
"x-ms-parameter-location": "method"
}
}
}
67 changes: 67 additions & 0 deletions ...rc/modeler/AutoRest.Swagger.Tests/Resource/Swagger/old/global_parameter_order_change.json
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,67 @@
{
"swagger": 2.0,
"info": {
"title": "required_parameter",
"version": "1.0"
},
"host": "localhost:8000",
"schemes": [ "http", "https" ],
"paths": {
"/api/Parameters": {
"get": {
"tag": [ "Parameters" ],
"operationId": "Parameters_Get",
"produces": [
"text/plain"
],
"parameters": [
{
"$ref": "#/parameters/ImplicitLocationParam"
},
{
"$ref": "#/parameters/MethodLocationParam1"
},
{
"$ref": "#/parameters/ClientLocationParam"
},
{
"$ref": "#/parameters/MethodLocationParam2"
}
]
}
}
},
"parameters": {
"ImplicitLocationParam": {
"name": "implicit_location_param",
"in": "query",
"required": true,
"type": "string",
"description": "Implicit x-ms-parameter-location param"
},
"ClientLocationParam": {
"name": "current_location_param",
"in": "query",
"required": true,
"type": "string",
"description": "client x-ms-parameter-location param",
"x-ms-parameter-location": "client"
},
"MethodLocationParam1": {
"name": "method_location_param_1",
"in": "query",
"required": true,
"type": "string",
"description": "method x-ms-parameter-location param 1",
"x-ms-parameter-location": "method"
},
"MethodLocationParam2": {
"name": "method_location_param_2",
"in": "query",
"required": true,
"type": "string",
"description": "method x-ms-parameter-location param 2",
"x-ms-parameter-location": "method"
}
}
}
38 changes: 38 additions & 0 deletions openapi-diff/src/modeler/AutoRest.Swagger.Tests/SwaggerModelerCompareTests.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -903,10 +903,48 @@ public void XmsEnumModelAsString()
[Fact]
public void ChangedParameterOrder()
{
// Reference:
// https://github.com/Azure/autorest/blob/main/docs/generate/how-autorest-generates-code-from-openapi.md#specifying-required-parameters-and-properties
var messages = CompareSwagger("parameter_order_change.json").ToArray();
Assert.Equal(2, messages.Where(m => m.Id == ComparisonMessages.ChangedParameterOrder.Id).Count());
}

[Fact]
public void ChangedGlobalParameterOrder()
{
var messages = CompareSwagger("global_parameter_order_change.json").ToArray();

// Changed order from:
// ( Implicit , Method1 , Client , Method2 )
// to ( Method2 , Implicit , Method1 , Client )
//
// So there are two errors reported:
// - On Method1, as now it is before Method2
// - On Method2, as now it is after Method1
//
// There are no errors for Implicit and Client as their order doesn't matter.
//
// Reference: https://github.com/Azure/autorest/blob/main/docs/extensions/readme.md#x-ms-parameter-location
Assert.Equal(2, messages.Where(m => m.Id == ComparisonMessages.ChangedParameterOrder.Id).Count());
}

[Fact]
public void DidNotChangeGlobalParameterOrder()
{
var messages = CompareSwagger("global_parameter_no_order_change.json").ToArray();

// Changed order from:
// ( Implicit , Method1 , Client , Method2 )
// to ( Method1 , Client , Method2 , Implicit )
//
// So there is no issue with order change:
// The Method1 param still comes before Method2 param, and order
// of Implicit and Client doesn't matter.
//
// Reference: https://github.com/Azure/autorest/blob/main/docs/extensions/readme.md#x-ms-parameter-location
Assert.Equal(0, messages.Where(m => m.Id == ComparisonMessages.ChangedParameterOrder.Id).Count());
}

[Fact]
public void ChangedXmsLongRunningOperation()
{
Expand Down
2 changes: 1 addition & 1 deletion openapi-diff/src/modeler/AutoRest.Swagger/ComparisonMessages.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -110,7 +110,7 @@ public static class ComparisonMessages
{
Id = 1042,
Code = nameof(ComparisonMessages.ChangedParameterOrder),
Message = "The order of parameter '{0}' was changed. ",
Message = "The order of parameter with Name: '{0}', In: '{1}', was changed.",
Type = MessageType.Update
};

Expand Down
Loading