openapi-generator icon indicating copy to clipboard operation
openapi-generator copied to clipboard

[BUG] [Java] Nullable field failing when explicitly set to "null"

Open valmoz opened this issue 2 years ago • 11 comments

Bug Report Checklist

  • [x] Have you provided a full/minimal spec to reproduce the issue?
  • [x] Have you validated the input using an OpenAPI validator (example)?
  • [x] Have you tested with the latest master to confirm the issue still exists?
  • [x] Have you searched for related issues/PRs?
  • [x] What's the actual output vs expected output?
  • [ ] [Optional] Sponsorship to speed up the bug fix or feature request (example)
Description

In our OpenAPI spec, we have a model with a nullable field called "prev_page_url". If we test the generated code (with generator v6.0.0) omitting the parameter, it works correctly.

If instead we test setting the field explicitly to null we obtain the following error:

Expected the field `prev_page_url` to be a primitive type in the JSON string but got `null`
openapi-generator version

v6.0.0

OpenAPI declaration file content or URL

The parameter is defined here, and the main spec can be found here

title: Pagination
type: object
description: ''
properties:
  current_page:
    type: integer
    description: Current page number.
    nullable: true
  first_page_url:
    type: string
    format: uri
    description: First page url.
    nullable: true
  from:
    type: integer
    nullable: true
    description: First result of the page.
  last_page:
    type: integer
    description: Last page number.
    nullable: true
  last_page_url:
    type: string
    format: uri
    description: Last page url.
    nullable: true
  next_page_url:
    type: string
    format: uri
    nullable: true
    description: Next page url
  path:
    type: string
    format: uri
    description: Request path.
    nullable: true
  per_page:
    type: integer
    description: Number of result per page.
    nullable: true
  prev_page_url:
    type: string
    format: uri
    nullable: true
    description: Previous page url.
  to:
    type: integer
    nullable: true
    description: Last result of the page.
  total:
    type: integer
    description: Total number of results
    nullable: true
Generation Details

Our github action can be found here openapi-generator-cli generate -i ./openapi.yaml -g java -o ./generated/java/

Steps to reproduce

obtain a JSON response with the parameter set as follows:

{
  "prev_page_url":null
}
Related issues/PRs

I couldn't find any...

Suggest a fix

In this case, Gson doesn't set the param to null, but to an instance of JsonNull class. To check it should be able to change the pojo.mustache template in this way:

if (({{^isRequired}}jsonObj.get("{{{baseName}}}") != null && {{^isRequired}}jsonObj.get("{{{baseName}}}").isJsonNull()) && {{/isRequired}}!jsonObj.get("{{{baseName}}}").isJsonPrimitive()) {

If you want, I can try to prepare a PR for this issue.

valmoz avatar Jun 07 '22 10:06 valmoz

The template that should be modified: https://github.com/OpenAPITools/openapi-generator/blob/master/modules/openapi-generator/src/main/resources/Java/libraries/okhttp-gson/pojo.mustache line 493

valmoz avatar Jun 07 '22 10:06 valmoz

I'm affected by this issue as well and I am interested in fixing it. I will submit a PR for it if @valmoz is not already working on it.

petitcl avatar Jun 16 '22 15:06 petitcl

+1

vlauciani avatar Jun 17 '22 13:06 vlauciani

Hi, I prepared a PR for this https://github.com/OpenAPITools/openapi-generator/pull/12630 I'll try to launch the sample generation as soon as possible, to make it acceptable for the team.

valmoz avatar Jun 17 '22 15:06 valmoz

+1

melloware avatar Jun 23 '22 11:06 melloware

+1

vlauciani avatar Jul 12 '22 09:07 vlauciani

I just ran into this too in 6.0.1! I managed to make it work by modifying the generated code to do the isJsonNull() check, but obvs this should be addressed..! Thanks.

aaron-613 avatar Sep 08 '22 07:09 aaron-613

Hi @aaron-613 my PR was merged, but I think it will be released in the next milestone. https://github.com/OpenAPITools/openapi-generator/pull/12630

valmoz avatar Sep 08 '22 08:09 valmoz

@valmoz very cool..! Wow 131 files changed!? That sounds like a lot. I stumbled onto another bug that is similar to this one, auto-generated validation code not being complete/correct. See my comment here: https://github.com/OpenAPITools/openapi-generator/issues/12550#issuecomment-1240358272 I was thinking I could attempt to fix myself if I just needed to update that mustache file to add an extra check if I had an array, but your PR scares me. Wondering how many files you manually had to update vs. auto-generated checks/tests..? Thanks.

aaron-613 avatar Sep 08 '22 15:09 aaron-613

Waiting the release, I'm using the @valmoz fix running:

docker run --rm openapitools/openapi-generator-cli@sha256:0f5e6f741bda7397bf30dc96c23a5aea633e0b1a37e2c2178a08c571b6722b7a

vlauciani avatar Sep 08 '22 15:09 vlauciani

@aaron-613 I just modified one mustache file... and then launched the generator to update the example files

valmoz avatar Sep 08 '22 15:09 valmoz

Im using v6.2.1 and am generating a Java client with:

openapi-generator generate -i swagger.yaml -g java -o ./generated/ledger \
  --group-id com.clever \
  --api-package com.clever.sdk.ledgerdataservice.api \
  --artifact-id ledger-data-service-java-client \
  --model-package com.clever.sdk.ledgerdataservice.models \
  --additional-properties=dateLibrary=java8,library=okhttp-gson

When any property is null, even if including nullable: true in the definition, eg

Expected the field currencyCodeto be a primitive type in the JSON string but gotnull``

Is this fixed by the PR above? It doesnt seem to be for Java

Zander1983 avatar Dec 07 '22 09:12 Zander1983

This fix works for me (using v6.3.0), so should probably be closed

nikosmoum avatar Feb 22 '23 11:02 nikosmoum

I'm on the latest 6.4.0 and still seems to be an issue for arrays when explicitly set to null in the JSON response. The issue in the mustache seems to be this part, perhaps it should also do a check for isJsonNull()?

      // ensure the optional json data is an array if present
      if (jsonObj.get("{{{baseName}}}") != null && !jsonObj.get("{{{baseName}}}").isJsonArray()) {
        throw new IllegalArgumentException(String.format("Expected the field `{{{baseName}}}` to be an array in the JSON string but got `%s`", jsonObj.get("{{{baseName}}}").toString()));
      }

jaavaguy avatar Mar 06 '23 04:03 jaavaguy

I have the same problem as @jaavaguy with version 6.4.0 i get code which runs into an exception: java.lang.IllegalArgumentException: Expected the field foo to be an array in the JSON string but got null

// ensure the optional json data is an array if present if (jsonObj.get("foo") != null && !jsonObj.get("foo").isJsonArray()) { throw new IllegalArgumentException(String.format("Expected the field 'foo' to be an array in the JSON string but got '%s'", jsonObj.get("foo").toString())); }

KJSchwenze avatar Mar 23 '23 12:03 KJSchwenze

I'm also seeing this issue with a required, nullable, readOnly object string property. Particularly the issue seems to be when mixing both nullable and required.

samypr100 avatar May 09 '23 16:05 samypr100

I'm also seeing this issue with required: true in combination with nullable: true in Version 6.6.0. Whenever the value is "null", the validation is throwing an Exception for trying to cast gson.JsonNull to gson.JsonObject.

FloCoop avatar Jun 05 '23 11:06 FloCoop

I'm also seeing this issue with required: true in combination with nullable: true in Version 6.6.0. Whenever the value is "null", the validation is throwing an Exception for trying to cast gson.JsonNull to gson.JsonObject.

+1 on this error. Even if required or nullable is not set, the exception for casting gson.JsonNull to gson.JsonObject is happening.

Is there a known workaround to the issue on casting?

keeed avatar Jun 22 '23 22:06 keeed

@keeed Given https://github.com/OpenAPITools/openapi-generator/pull/15462 hopefully it will be fixed when 7.0.0 comes out.

samypr100 avatar Jun 22 '23 22:06 samypr100

@keeed Given https://github.com/OpenAPITools/openapi-generator/pull/15462 hopefully it will be fixed when 7.0.0 comes out.

Is there a known workaround? The client that I'm working on keeps failing because of the casting exception even though it's a valid case and my field has been marked as nullable.

I don't want to force the server to return default values just to satisfy the client 😅

keeed avatar Jun 22 '23 22:06 keeed

I'm on the latest 6.4.0 and still seems to be an issue for arrays when explicitly set to null in the JSON response. The issue in the mustache seems to be this part, perhaps it should also do a check for isJsonNull()?

      // ensure the optional json data is an array if present
      if (jsonObj.get("{{{baseName}}}") != null && !jsonObj.get("{{{baseName}}}").isJsonArray()) {
        throw new IllegalArgumentException(String.format("Expected the field `{{{baseName}}}` to be an array in the JSON string but got `%s`", jsonObj.get("{{{baseName}}}").toString()));
      }

+1, still same issue in v6.6.0

leanNiki avatar Jul 07 '23 12:07 leanNiki

the same occurs with java-spring generator with jackson-databind-nullable. When deserializing explicit null, the following code is executed JsonNullable.of(null)

which incorrectly assigns isPresent property:

        this.value = null;
        this.isPresent = true;

agrochmal avatar Oct 20 '23 18:10 agrochmal

@agrochmal this was only fixed in okhttp-gson generator, but feel free to submit a PR for java-spring

samypr100 avatar Oct 21 '23 00:10 samypr100