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

Fix #2855, fix incorrect api parameter type #2856

Closed
wants to merge 0 commits into from

Conversation

dulong
Copy link

@dulong dulong commented Jan 10, 2019

No description provided.

@codecov
Copy link

codecov bot commented Jan 10, 2019

Codecov Report

Merging #2856 into master will decrease coverage by 0.01%.
The diff coverage is 85.71%.

Impacted file tree graph

@@             Coverage Diff              @@
##             master    #2856      +/-   ##
============================================
- Coverage     94.43%   94.42%   -0.02%     
+ Complexity     3226     3225       -1     
============================================
  Files           364      364              
  Lines          8302     8304       +2     
  Branches        619      619              
============================================
+ Hits           7840     7841       +1     
  Misses          304      304              
- Partials        158      159       +1
Impacted Files Coverage Δ Complexity Δ
...ing/web/readers/parameter/ParameterTypeReader.java 100% <100%> (ø) 17 <0> (-1) ⬇️
...wagger1/readers/parameter/ParameterNameReader.java 93.33% <66.66%> (-6.67%) 6 <0> (ø)

@codecov
Copy link

codecov bot commented Jan 10, 2019

Codecov Report

Merging #2856 into master will increase coverage by 1.49%.
The diff coverage is 100%.

Impacted file tree graph

@@             Coverage Diff              @@
##             master    #2856      +/-   ##
============================================
+ Coverage     92.93%   94.43%   +1.49%     
+ Complexity     3513     3226     -287     
============================================
  Files           382      364      -18     
  Lines          9330     8304    -1026     
  Branches        768      619     -149     
============================================
- Hits           8671     7842     -829     
+ Misses          471      304     -167     
+ Partials        188      158      -30
Impacted Files Coverage Δ Complexity Δ
...wagger1/readers/parameter/ParameterNameReader.java 100% <100%> (ø) 7 <0> (+1) ⬆️
...ing/web/readers/parameter/ParameterTypeReader.java 100% <100%> (ø) 17 <0> (-1) ⬇️
...tation/schema/JaxbPresentInClassPathCondition.java 50% <0%> (-50%) 3% <0%> (+1%)
...s/PropertyDiscriminatorBasedInheritancePlugin.java 88% <0%> (-4.31%) 9% <0%> (-1%)
...ntation/swagger2/web/Swagger2ControllerWebMvc.java 88.46% <0%> (-3.85%) 7% <0%> (-1%)
...documentation/swagger/common/HostNameProvider.java 88.23% <0%> (-1.77%) 4% <0%> (-1%)
...x/documentation/builders/ModelPropertyBuilder.java 94.87% <0%> (-1.62%) 21% <0%> (ø)
...ema/property/OptimizedModelPropertiesProvider.java 97.91% <0%> (-1.51%) 35% <0%> (-8%)
.../documentation/swagger/schema/ApiModelBuilder.java 82.35% <0%> (-0.99%) 5% <0%> (ø)
...ntation/spring/data/rest/SpecificationBuilder.java 92.56% <0%> (-0.92%) 9% <0%> (-6%)
... and 118 more

@springfox springfox deleted a comment Jan 21, 2019
@springfox springfox deleted a comment Jan 21, 2019
Copy link
Member

@dilipkrish dilipkrish left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Mostly looks ok, I feel its introducing a breaking change that will force people to have to annotate @RequestBody. While it fixes your original bug, it breaks existing assumptions.

return "body";
return determineScalarParameterType(
parameterContext.getOperationContext().consumes(),
parameterContext.getOperationContext().httpMethod());
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

When will it return a body in this case?

@@ -65,7 +65,7 @@
@ResponseBody
@ApiOperation(value = "Creates list of users with given input array")
public Mono<ResponseEntity<User>> createUsersWithArrayInput(@ApiParam(value = "List of user object", required = true)
User[] users) {
@RequestBody User[] users) {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Isnt this incorrect? You're fixing the existing tests by adding this request body. In essence you're not preserving the existing behavior

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants