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

[BUG][JAVA] IllegalArgumentException while parsing responses with unknown fields ❗

Open jaaufauvre opened this issue 3 years ago • 13 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

API client libraries generated with OpenAPI Generator should ignore response fields that were not existing at the time the code was generated, instead of raising the following error:

java.lang.IllegalArgumentException: The field `updated` in the JSON string is not defined in the `Dog` properties. JSON: {"name":"Luna","created":"2021-04-23T17:32:28Z","updated":"2021-04-23T17:32:28Z"}
openapi-generator version
Version Status
OpenAPI Generator 5.4.0 ✔️
OpenAPI Generator 6.0.0
OpenAPI Generator 6.0.1-SNAPSHOT
OpenAPI declaration file content or url
  • sample-v1.yaml
  • sample-v2.yaml

Download ⤓

Generation Details
  • java -jar openapi-generator-cli-5.4.0.jar generate -g java -i sample-v1.yaml -c config.json -o api_client_5.4
  • java -jar openapi-generator-cli-6.0.0.jar generate -g java -i sample-v1.yaml -c config.json -o api_client_6.0
{ 
    "groupId" : "com.acme.app",
    "artifactId" : "sample-api-client",
    "artifactVersion" : "1.0.0",
    "invokerPackage" : "com.acme.app",
    "apiPackage" : "com.acme.app.api",
    "modelPackage" : "com.acme.app.model"
}
Steps to reproduce
  1. Generate a client from sample-v1.yaml:
java -jar openapi-generator-cli-6.0.0.jar generate -g java -i sample-v1.yaml -c config.json -o api_client_6.0
  1. Update DogsApiTest:
public class DogsApiTest {

    @Test
    public void addDogTest() throws ApiException {
        // GIVEN
        ApiClient client = new ApiClient();
        client.setDebugging(true);
        client.setBasePath("http://localhost:4010");
        NewDog newDog = new NewDog()
                .name("Bella");

        // WHEN
        Dog dog = new DogsApi(client).addDog(newDog);

        // THEN
        assertNotNull(dog);
    }

}
  1. Run prism.exe mock sample-v1.yaml
  2. Run addDogTest => ✔️
  3. Run prism.exe mock sample-v2.yaml
  4. Run addDogTest => ❌
java.lang.IllegalArgumentException: The field `updated` in the JSON string is not defined in the `Dog` properties. JSON: {"name":"Luna","created":"2021-04-23T17:32:28Z","updated":"2021-04-23T17:32:28Z"}

	at com.acme.app.model.Dog.validateJsonObject(Dog.java:217)
	at com.acme.app.model.Dog$CustomTypeAdapterFactory$1.read(Dog.java:253)
	at com.acme.app.model.Dog$CustomTypeAdapterFactory$1.read(Dog.java:243)
	at com.google.gson.TypeAdapter$1.read(TypeAdapter.java:199)
	at com.google.gson.Gson.fromJson(Gson.java:991)
	at com.google.gson.Gson.fromJson(Gson.java:956)
	at com.google.gson.Gson.fromJson(Gson.java:905)
	at com.acme.app.JSON.deserialize(JSON.java:151)
	at com.acme.app.ApiClient.deserialize(ApiClient.java:845)
	at com.acme.app.ApiClient.handleResponse(ApiClient.java:1053)
...

jaaufauvre avatar Jun 07 '22 11:06 jaaufauvre

@jaaufauvre Configuring disallowAdditionalPropertiesIfNotPresent to false fixes this issue for me. Still testing to see what other fallouts come from setting this, and not sure why the change surfaced in the latest version.

https://openapi-generator.tech/docs/generators/java

carmenquan avatar Jun 10 '22 17:06 carmenquan

This is a concerning one, because a client generated with OpenAPI Generator 6.0.x is likely to break as soon as new fields are added to responses. In addition to that, it makes it impossible to remove deprecated fields from an API specification.

jaaufauvre avatar Jul 27 '22 15:07 jaaufauvre

Any news on this, folks?

sergiofigueras avatar Aug 02 '22 17:08 sergiofigueras

Following this issue, generating kotlin client with gradle is having the same issue. The code is enforcing all properties to be define in contract, that we do not need in this client implementation at all.
Need a method to ignore properties not defined in contract while actually send back to client.

Rugal avatar Aug 03 '22 07:08 Rugal

Very related to this (I ran into the exact same issues as the OP). Including the additional config file of {"disallowAdditionalPropertiesIfNotPresent":false} worked to not include the block of validation code throwing IllegalArgumentExceptions. So that's good! Thanks for documenting this issue and workaround..! 🙏🏼

My problem came right after that, because my unknown/additional field is an array... and the code at the bottom of the generated class (the TypeAdapter<T> read() method, that tries to store additional fields expects it to be either a String, Number, Boolean, or Object. Not arrays:

// store additional fields in the deserialized instance
Consumer instance = thisAdapter.fromJsonTree(jsonObj);
for (Map.Entry<String, JsonElement> entry : jsonObj.entrySet()) {
  if (!openapiFields.contains(entry.getKey())) {
    if (entry.getValue().isJsonPrimitive()) { // primitive type
      if (entry.getValue().getAsJsonPrimitive().isString())
 	    instance.putAdditionalProperty(entry.getKey(), entry.getValue().getAsString());
      else if (entry.getValue().getAsJsonPrimitive().isNumber())
	    instance.putAdditionalProperty(entry.getKey(), entry.getValue().getAsNumber());
      else if (entry.getValue().getAsJsonPrimitive().isBoolean())
	    instance.putAdditionalProperty(entry.getKey(), entry.getValue().getAsBoolean());
      else
	    throw new IllegalArgumentException(String.format("The field `%s` has unknown primitive type. Value: %s", entry.getKey(), entry.getValue().toString()));
    } else { // non-primitive type
      instance.putAdditionalProperty(entry.getKey(), gson.fromJson(entry.getValue(), HashMap.class));  <-- Assumes JSON Object?
    }
  }
}

Exception:

java.lang.IllegalStateException: Expected BEGIN_ARRAY but was BEGIN_OBJECT at path $[0]
com.google.gson.JsonSyntaxException: java.lang.IllegalStateException: Expected BEGIN_ARRAY but was BEGIN_OBJECT at path $[0]
	at com.google.gson.Gson.fromJson(Gson.java:944)
	at com.google.gson.Gson.fromJson(Gson.java:1003)
	at com.google.gson.Gson.fromJson(Gson.java:975)
	at community.solace.ep.client.model.Consumer$CustomTypeAdapterFactory$1.read(Consumer.java:518)    <-- that's the line above with the HashMap.class

Dang! I guess I'll be raising an Issue for this!

EDIT: if I manually add these two lines into that method, everything works. So I guess I know what needs to be updated in the code generation template:

} else if (entry.getValue().isJsonArray()) {
    instance.putAdditionalProperty(entry.getKey(), gson.fromJson(entry.getValue(), ArrayList.class));

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

Looks like it's solved in latest release.

janisz avatar Mar 17 '23 17:03 janisz

I also encountered this nasty bug and looks like it is fixed via this commit: https://github.com/OpenAPITools/openapi-generator/pull/13630

The 6.4.0 upgrade and configOption mentioned above are what fixed it for me.

IMO, the configOption needs to be set to false as default in the next major release. Thoughts?

mas-chen avatar Mar 17 '23 23:03 mas-chen

With 6.6.0 we still have to set configOptions disallowAdditionalPropertiesIfNotPresent to false. Will this issue ever get fixed for compatibility with the old Version 5.4.0?

FloCoop avatar Jun 05 '23 13:06 FloCoop

Running into this issue on 6.4.0 without setting the disallowAdditionalPropertiesIfNotPresent config option to false - wanted to chime in and say that this config option should be false as the default, its perfectly acceptable for a REST API to add properties to a response body, this is non breaking and clients should ignore anything they don't recognize. AFAIK the other languages don't behave in this way.

ajrice6713 avatar Jun 22 '23 13:06 ajrice6713

With 6.6.0 disallowAdditionalPropertiesIfNotPresent doesn't change anything at all for me with the default library. (setting in gradle)

anm-cb avatar Jun 30 '23 13:06 anm-cb

We were able to get this to work on 6.6.0 with disallowAdditionalPropertiesIfNotPresent set. However I do agree with @ajrice6713 that this should be false by default, the claim in the documentation that this should be true by default and this is the new OAS compliant standard seems mis-guided.

A client shouldn't break if there is additional data in the response that it doesn't understand.

thejeff77 avatar Aug 24 '23 14:08 thejeff77