Skip to content

[JAVA] Codegen - swagger type=string format=byte not serialised correctly #4824

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

Closed
jritmeijer-muhimbi opened this issue Feb 18, 2017 · 8 comments · Fixed by #7473
Closed

Comments

@jritmeijer-muhimbi
Copy link

jritmeijer-muhimbi commented Feb 18, 2017

Description

I am currently creating frameworks for the various supported languages using Codegen. Generated clients for Python, PHP, Ruby and C# work as expected, as do other 3rd party systems that support Swagger files (e.g. Microsoft Flow). However, the JSON generated by the Codegen Java client is not correct, or at least not in line with the JSON generated by the other languages.

Related snippet from the Swagger file

        "source_file_content": {
          "description": "Content of the file to convert",
          "x-ms-summary": "Source file content",
          "type": "string",
          "format": "byte",
          "uniqueItems": false
        }

The Java code that is generated (snippets from the same model file):

  @SerializedName("source_file_content")
  private byte[] sourceFileContent = null;

  public ConvertData sourceFileContent(byte[] sourceFileContent) {
    this.sourceFileContent = sourceFileContent;
    return this;
  }

  @ApiModelProperty(example = "null", required = true, value = "Content of the file to convert")
  public byte[] getSourceFileContent() {
    return sourceFileContent;
  }

  public void setSourceFileContent(byte[] sourceFileContent) {
    this.sourceFileContent = sourceFileContent;
  }

When I make the REST call this is serialised to the following JSON

    {
        "use_async_pattern":false,
        "source_file_name":"test.txt",
        "source_file_content":[83,71,86,108,98,71,56,61],
        "output_format":"PDF",
        "copy_metadata":false,
        "fail_on_error":true
    }

As you can see the byte array is serialised as the individual bytes in the array,

However, the JSON generated by the clients for the other languages serialise to a string, which is what we need. (Please ignore that the string is base64 encoded, that is not done by the client framework, but rather by the application that calls the framework)

    {
         "use_async_pattern":false,
         "source_file_name":"test.txt",
         "source_file_content":" UEsDBBQAB[string truncated for brevity]==",
         "output_format":"PDF",
         "copy_metadata":false,
         "fail_on_error":true
    }

When I change the byte[] types in the generated model to String then all works well.

  @SerializedName("source_file_content")
  private String sourceFileContent = null;

  public ConvertData sourceFileContent(String sourceFileContent) {
    this.sourceFileContent = sourceFileContent;
    return this;
  }

  @ApiModelProperty(example = "null", required = true, value = "Content of the file to convert")
  public String getSourceFileContent() {
    return sourceFileContent;
  }

  public void setSourceFileContent(String sourceFileContent) {
    this.sourceFileContent = sourceFileContent;
  }
Swagger-codegen version

I have tried both the current stable (2.2.1) as well as the Master. Both exhibit the same behaviour.

Swagger declaration file content or url

The Swagger file can be found at https://api.muhimbi.com/api-docs/v1/swagger.json

Command line used for generation
java -jar modules/swagger-codegen-cli/target/swagger-codegen-cli.jar generate \
   -i https://api.muhimbi.com/api-docs/v1/swagger.json \
   -l java \
   -o java-client-new
Steps to reproduce
  1. Generate client framework as per the above command.
  2. Open the model/ConvertData.java file and inspect the sourceFileContent related properties.
Related issues

Issue #669 (Support binary data in body). It appears that this may actually be the cause of this bug, though not 100% sure.

Suggest a Fix

I am afraid that my knowledge of Java is limited, so I am in no position to fix this myself. However, based on the behaviour described above, as well as the way other codegen clients behave, when type=string and format=byte the data type of the generated Java property should be String, not byte[].

When I manually make this change in the generated model, the correct JSON is generated and our API works well when used from Java. There is no need to change the serialisation logic, just the data types.

Interestingly, looking at the C# code generated by Codegen, that also uses the byte[] type for these properties. As that client generates the correct JSON, the serialiser must behave differently.

If this issue results in a change I am able (and happy) to test it.

@wing328
Copy link
Contributor

wing328 commented Mar 20, 2017

@jritmeijer-muhimbi thanks for reporting the issue. I think it has to do with the serializer in the Java API client.

We'll mark this as "Need community contribution"

For the time being, please use string as a workaround.

@wing328 wing328 modified the milestone: v2.2.3 Jul 16, 2017
@RoeeShlomo
Copy link

In my case, I tracked down the issue to the generated ApiClient#parameterToString.
The generated code converts byte[] to String with String.valueOf. If I change it to one of the String constructors that accepts byte[] then it works as expected.

If there are no other side effects to this change then the fix would be a simple change in ApiClient.mustache.

@wing328
Copy link
Contributor

wing328 commented Jul 27, 2017

@RoeeShlomo can you please file a PR with the suggested fix?

@wing328 wing328 added this to the v2.3.0 milestone Jul 27, 2017
@benrobot
Copy link
Contributor

benrobot commented Dec 6, 2017

Here's the work around I am using until this issue can be resolved:

Create Gson type adapter for the Byte Array:

public class ByteArrayAdapter implements JsonSerializer<byte[]>, JsonDeserializer<byte[]> {
    @Override
    public JsonElement serialize(byte[] src, Type typeOfSrc, JsonSerializationContext context) {
        if (src == null) {
            return JsonNull.INSTANCE;
        } else {
        	String base64 = Base64.getEncoder().encodeToString(src); // JDK 8 specific, if on older JDK use a library
            return new JsonPrimitive(base64);
        }
    }

    @Override
    public byte[] deserialize(JsonElement json, Type byteArray, JsonDeserializationContext context) throws JsonParseException {
        String str = json.getAsJsonPrimitive().getAsString();
        try {
        	return Base64.getDecoder().decode(str); // JDK 8 specific, if on older JDK use a library
        } catch (RuntimeException e) {
            throw new JsonParseException(e);
        }
    }
}

Register the new type adapter:

ApiClient client = new ApiClient();
JSON swaggersJson = new JSON(client);
Gson gson = swaggersJson.getGson();
GsonBuilder gsonBuilder = new GsonBuilder();

//Re-register whatever types already registered by the JSON() constructor (inspect it to verify)
gsonBuilder.registerTypeAdapter(Date.class, gson.getAdapter(Date.class));
gsonBuilder.registerTypeAdapter(OffsetDateTime.class, gson.getAdapter(OffsetDateTime.class));
gsonBuilder.registerTypeAdapter(LocalDate.class, gson.getAdapter(LocalDate.class));

// Register our new ByterArray type adapter
gsonBuilder.registerTypeAdapter(byte[].class, new ByteArrayAdapter(client));

swaggersJson.setGson(gsonBuilder.create());
client.setJSON(swaggersJson);

@wing328
Copy link
Contributor

wing328 commented Dec 11, 2017

@benrobot thanks for sharing the workaround.

@cbornet
Copy link
Contributor

cbornet commented Dec 13, 2017

@benrobot this is more the fix than a workaround. Would you do a PR ?

@brendandburns
Copy link
Contributor

I'm happy to do a PR for this if @benrobot doesn't have time, this is affecting the Kubernetes client...

@benrobot
Copy link
Contributor

@cbornet, PR done. Hope it is acceptable otherwise I'll be glad to update it.

@brendandburns, thanks for waiting to give me a chance to do the PR. I would not have minded if you jumped on it to speed things up.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

6 participants