swagger-api / swagger-codegen-generators

Apache License 2.0
284 stars 424 forks source link

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

Open yeikel opened 5 months ago

yeikel commented 5 months ago

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 https://github.com/swagger-api/swagger-codegen-generators/pull/1291

lukw4l commented 4 months ago

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 commented 4 months ago

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?

frantuma commented 2 months ago

nullable and required related validation has been addressed thoroughly with additional options in https://github.com/swagger-api/swagger-codegen-generators/pull/1308. Please add additional comments there and/or open a new ticket

frantuma commented 2 months ago

Re-opening, see https://github.com/swagger-api/swagger-codegen-generators/pull/1308#issuecomment-2336758761