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

Mark Request parameters with default values as required #2149

Closed
ZachGagnon1 opened this issue Mar 20, 2023 · 7 comments
Closed

Mark Request parameters with default values as required #2149

ZachGagnon1 opened this issue Mar 20, 2023 · 7 comments
Labels
enhancement New feature or request

Comments

@ZachGagnon1
Copy link

ZachGagnon1 commented Mar 20, 2023

Describe the bug
Using @RequestParam(required = false) does not work since v2.0.3. It will always show the field as required in the specifications.

To Reproduce
Steps to reproduce the behavior:

  • What version of spring-boot you are using?
    3.0.4
  • What modules and versions of springdoc-openapi are you using?
    2.0.4
  • Provide with a sample code (HelloController) or Test that reproduces the problem
    java
import io.swagger.v3.oas.annotations.tags.Tag;
import lombok.AccessLevel;
import lombok.RequiredArgsConstructor;
import lombok.experimental.FieldDefaults;
import org.springframework.http.MediaType;
import org.springframework.http.ResponseEntity;
import org.springframework.web.bind.annotation.GetMapping;
import org.springframework.web.bind.annotation.RequestMapping;
import org.springframework.web.bind.annotation.RequestParam;
import org.springframework.web.bind.annotation.RestController;

@Tag(name = "Test")
@RestController
@RequestMapping(value = "/api/v2/test", produces = MediaType.APPLICATION_JSON_VALUE)
@RequiredArgsConstructor
@FieldDefaults(level = AccessLevel.PRIVATE, makeFinal = true)
public class TestController {
  @GetMapping
  public ResponseEntity<Void> getTest(
      @RequestParam(defaultValue = "false", required = false) boolean includeClosed) {
    return ResponseEntity.noContent().build();
  }
}

Expected behavior
Expect the swagger docs to say the field is not required.

Screenshots

V2.0.4

image

V2.0.2

image

No code has been changed between both pictures. Only the version.

Additional context
Using Java 17

@ZachGagnon1 ZachGagnon1 changed the title @RequestParam(required = false) Broken Since 2.0.3 @RequestParam(required = false) Broken Since v2.0.3 Mar 20, 2023
@bnasslahsen
Copy link
Contributor

Not reproducible
image

If you provide to a GITHUB repo with a Minimal, Reproducible Example - with HelloController that reproduces the problem, then this ticket can be reopened.

@bnasslahsen bnasslahsen added the invalid This doesn't seem right label Mar 20, 2023
@ZachGagnon1
Copy link
Author

ZachGagnon1 commented Mar 20, 2023

Not reproducible image

If you provide to a GITHUB repo with a Minimal, Reproducible Example - with HelloController that reproduces the problem, then this ticket can be reopened.

Done I updated and posted a controller where the issue is still there. The code ran was the same as the one above. Only change is still the version of the openapi dependancy

@JoeHegarty
Copy link

JoeHegarty commented Mar 21, 2023

We're also seeing this, setting a param to required=false does not take effect (it's true anyway) in 2.0.3 and later, but works fine in 2.0.2.

The fix for #2021 seems like the most likely cause at a quick glance. We have a mixed Kotlin/Java project, so I am suspecting that may be a prerequisite to repro it if 2021 is the cause. I will try and find some time to come up with a definitive case.

@bnasslahsen
Copy link
Contributor

@ZachGagnon1,

Again not reproducible:
image
Try providing a GITHUB repo with a Minimal, Reproducible Example - with HelloController that reproduces the problem, then this ticket can be reopened.

@JoeHegarty,

You are describing a kotlin issue

For this one, there is an enhancement for Kotlin projects you read the changelog.
You should try to get the previous behavior:

springdoc.nullable-request-parameter-enabled=false

@tobiberger
Copy link

Hi @bnasslahsen, I just stumbled upon the same issue and also created a repository to reproduce it:
https://github.com/tobiberger/demo-default-value-required
I'd say, the issue here is not really the required parameter of @RequestParameter, but it's about the defaultValue. If there is a default value defined, the parameter shouldn't be marked as required, no matter what the Kotlin nullability says.
As you suggested, disabling the new feature you built fixes the breaking change, but I'd still argue that this should still work as before in that case.
Looking at your code, I'd suggest checking for the existence of a default value in the @RequestParam annotation after checking the @Parameter annotation and before checking the Kotlin nullability.

@bnasslahsen
Copy link
Contributor

@tobiberger,

This could be an enhancement. Are you willing to propose a PR for it ?

@tobiberger
Copy link

Sure, why not. This should do the trick.

@bnasslahsen bnasslahsen changed the title @RequestParam(required = false) Broken Since v2.0.3 Mark Request parameters with default values as required Mar 26, 2023
@bnasslahsen bnasslahsen added enhancement New feature or request and removed invalid This doesn't seem right labels Mar 26, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

No branches or pull requests

4 participants