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

@NotNull is added by default for fields that are not required #1295

Open
yeikel opened this issue May 31, 2024 · 4 comments
Open

@NotNull is added by default for fields that are not required #1295

yeikel opened this issue May 31, 2024 · 4 comments

Comments

@yeikel
Copy link

yeikel commented May 31, 2024

Given the following step:

Swagger Definition:

address_start_date:
        type: string
        format: 'date-time'
        example: "2011-12-03 10:15:30"
        description: |
          Start Date for address

swagger-codegen-maven-plugin 3.0.56 generates the following model:

   /**
   * Start Date for address 
   * @return addressStartDate
  **/
  @Valid
  @Schema(description = "Start Date for address ")
  public LocalDateTime getAddressStartDate() {
    return addressStartDate;
  }

While swagger-codegen-maven-plugin 3.0.57 generates this:

   /**
   * Start Date for address 
   * @return addressStartDate
  **/
  @NotNull // <--- This is added by default
  @Valid
  @Schema(description = "Start Date for address ")
  public LocalDateTime getAddressStartDate() {
    return addressStartDate;
  }

It seems that this is what introduced with #1291

@yeikel yeikel changed the title @NotNull is added by default for fields that are required @NotNull is added by default for fields that are not required May 31, 2024
@lukw4l
Copy link

lukw4l commented Jul 2, 2024

I found out that you can restore the old behavior (no @NotNull annotation for not explicitly required fields) if you set the new configOption useNullableForNotNull to false.

The description of this option is "Add @NotNull depending on nullable property instead of required" and is set to true by default. It seems to completely ignore if a field is required or not.

@frantuma would it be enough to change the default to false so one can enable this feature if desired or should this new option be enhanced to work correctly (from my understanding) with not required as well as nullable fields?

@duke1995
Copy link

What is the proper way to set the cliOption useNullableForNotNull in the swagger-codegen-maven-plugin?

I found some examples online where similar cliOptions are added under <additionalProperties>, so for useNullableForNotNull I would have something like this:

        <plugin>
          <groupId>io.swagger.codegen.v3</groupId>
          <artifactId>swagger-codegen-maven-plugin</artifactId>
          <executions>
            <execution>
              <?m2e execute onConfiguration,onIncremental?>
              <id>gcloud-generation</id>
              <phase>process-resources</phase>
              <goals>
                <goal>generate</goal>
              </goals>
              <configuration>
                <inputSpec>${swagger.input.spec}</inputSpec>
                <language>spring</language>
                <apiPackage>${swagger.api.package}</apiPackage>
                <modelPackage>${swagger.model.package}</modelPackage>
                <invokerPackage>${swagger.invoker.package}</invokerPackage>
                <generateApiTests>false</generateApiTests>
                <configOptions>
                  <serializableModel>true</serializableModel>
                  <interfaceOnly>true</interfaceOnly>
                  <dateLibrary>java8-localdatetime</dateLibrary>
                  <sourceFolder>.</sourceFolder>
                  <useTags>true</useTags>
                </configOptions>
                <additionalProperties>
                  <!-- useNullableForNotNull -->
                  <!-- Add @NotNull depending on `nullable` property instead of `required` -->
                  <additionalProperty>useNullableForNotNull=false</additionalProperty>
                </additionalProperties>
              </configuration>
            </execution>
          </executions>
        </plugin>

However it also seems to work when i add the config option under <configOptions>:

        <plugin>
          <groupId>io.swagger.codegen.v3</groupId>
          <artifactId>swagger-codegen-maven-plugin</artifactId>
          <executions>
            <execution>
              <?m2e execute onConfiguration,onIncremental?>
              <id>gcloud-generation</id>
              <phase>process-resources</phase>
              <goals>
                <goal>generate</goal>
              </goals>
              <configuration>
                <inputSpec>${swagger.input.spec}</inputSpec>
                <language>spring</language>
                <apiPackage>${swagger.api.package}</apiPackage>
                <modelPackage>${swagger.model.package}</modelPackage>
                <invokerPackage>${swagger.invoker.package}</invokerPackage>
                <generateApiTests>false</generateApiTests>
                <configOptions>
                  <serializableModel>true</serializableModel>
                  <interfaceOnly>true</interfaceOnly>
                  <dateLibrary>java8-localdatetime</dateLibrary>
                  <sourceFolder>.</sourceFolder>
                  <useTags>true</useTags>
                  <!-- useNullableForNotNull -->
                  <!-- Add @NotNull depending on `nullable` property instead of `required` -->
                  <useNullableForNotNull>false</useNullableForNotNull>
                </configOptions>
              </configuration>
            </execution>
          </executions>
        </plugin>

In practice, is there a difference between the 2 ways to define the config option? If so, which is the recommended way?

okotsopoulos added a commit to DataBiosphere/jade-data-repo that referenced this issue Aug 7, 2024
3.0.57 introduced a change in behavior which breaks many of our MVC unit tests.
See: swagger-api/swagger-codegen-generators#1295

Runtime groovy dependency caused bootRun to fail on startup.  It was previously needed for logback, but we use XML now (logback hasn't supported groovy since 2021: https://logback.qos.ch/news.html#1.2.9)

Hadoop upgrades used in tests required the exclusion of slf4j-reload4j dependency for Spring Boot tests to initialize.
@frantuma
Copy link
Member

nullable and required related validation has been addressed thoroughly with additional options in #1308. Please add additional comments there and/or open a new ticket

@frantuma
Copy link
Member

frantuma commented Sep 8, 2024

Re-opening, see #1308 (comment)

@frantuma frantuma reopened this Sep 8, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

4 participants