Swagger-codegen: [Java] How can we set List and Map to null by default in models?

Created on 28 Mar 2017  Â·  34Comments  Â·  Source: swagger-api/swagger-codegen

Description

For example we have model in crud-service:

ModelDto {
 private List<String> list = null;
 private SomeOtherModelDto object = null;
 //get set
}

in swagger spec it will appear as:

"list": {
  "type": "array",
  "items": {
    "type": "string"
  }
}

and then swagger-codegen will generate such dto:

ModelDto {
 private List<String> list = new ArrayList<>();
 private SomeOtherModelDto object = null;

 //get set
}

How can we prevent this?

It is important for us because there is a difference between null list and empty list.

Thank you.

Swagger-codegen version

io.swagger:swagger-codegen-cli:2.2.2

Command line used for generation

generate -i http://SOME_URL -l java -o bin/sdk --artifact-id sdk-name

Java Question Java

Most helpful comment

@wing328 @fehguy @cbornet @ePaul
Hello, I’ll try to explain why this patch is so important for us.

We have RESTfull API service, which supports partial update. For us it means that user can update only 1 field, if he send us only 1 field in JSON.

For example we have model in API

BookDto {
 List<String> autors;
 String name;
}

And if user send JSON:

{
    "name" : "new name"
}

To our API, we will update only name.

But, if we perform this operation via SDK, what will happen:

//User creates dto using sdk library:
BookDto dto = new BookDto();
//(In this dto autors will be set to new ArrayList<String>() by default)

//Then user set name to "new name"
dto.setName("new name")

It means that our API will get following JSON:

{
    
    "autors" : [],
    "name": "new name"
}

And in this case our API will override real values to empty list, and we will lose data.

We cannot implement rules to check null or empty, because in this case it means that we cannot set autors to empty array (if we want so).

In conclusion: we cannot use swagger codgen to generate sdk for our users without this patch.

All 34 comments

The OpenAPI Specification 2.0 (= Swagger Specification 2.0) doesn't allow defining anything as JSON null.
What it allows is to not make it mandatory to include a property in the containing object. This would be done by not listing it in the required list of the containing object.

When mapping this to Java, I would expect a receiver (client for response, server for request) to have the property as null when the value is not passed. A sender would simply not include the null value in the sent object.

I guess the initialization of the list was done to prevent null pointer exceptions when trying to access the object. I would argue that allowing a nullable list property (for non-required properties) would be the better way to go.

@ePaul Yes, it is main point, we don't need nulls in json, but this list(in my example) is not required, and json for deserialization just do not contain this field (with list) at all, but after deserialization it is (because of default empty array value) and for us it would be perfect to have nullable list (and Map) property.

Any suggestions? @wing328 maybe you can help me?

@wing328 I created solution for this: https://github.com/swagger-api/swagger-codegen/pull/5341

@fehguy Can you please review my pull?

I think it's not a good idea to add yet another option. I would set null by default.
An empty list could be generated when the field is required though.

@wing328 @fehguy @cbornet @ePaul
Hello, I’ll try to explain why this patch is so important for us.

We have RESTfull API service, which supports partial update. For us it means that user can update only 1 field, if he send us only 1 field in JSON.

For example we have model in API

BookDto {
 List<String> autors;
 String name;
}

And if user send JSON:

{
    "name" : "new name"
}

To our API, we will update only name.

But, if we perform this operation via SDK, what will happen:

//User creates dto using sdk library:
BookDto dto = new BookDto();
//(In this dto autors will be set to new ArrayList<String>() by default)

//Then user set name to "new name"
dto.setName("new name")

It means that our API will get following JSON:

{
    
    "autors" : [],
    "name": "new name"
}

And in this case our API will override real values to empty list, and we will lose data.

We cannot implement rules to check null or empty, because in this case it means that we cannot set autors to empty array (if we want so).

In conclusion: we cannot use swagger codgen to generate sdk for our users without this patch.

@egormasalitin thanks for the patch.

I think it's not a good idea to add yet another option. I would set null by default.
An empty list could be generated when the field is required though.

Would that meet your requirement? I think we can easily update the template to use null for optional property and an empty list for required property.

@wing328 Hello, thank you a lot for your answer. This solution is better than nothing, but still not exactly what we want, because in case of partial update the problems remains the same.

For example we have same dto, but autors is required field:

BookDto {
 //required
 List<String> autors;
 //optional
 String name;
}

It means that when we will decide to change name only, we will override autors

Can you please tell me why required List or Map cannot be null?

It means that when we will decide to change name only, we will override authors

Let's step back and try to review required and optional. If a field is required, it must be present in the JSON/XML payload (and the RESTful backend is expecting that field in the payload). For your use case, you only want to update name (optional). Can you mark autors as optional instead?

I just want to avoid omitting a required field in the payload, which does not conform to the spec.

@wing328 yes, I get it, thank you, I thought about required in another way. Sorry about that. In this case solution:

I think we can easily update the template to use null for optional property and an empty list for required property.

Fits perfectly

I will try to file a PR tomorrow by updating the templates.

We'll also need to do the same for C# and other langauges as well.

@egormasalitin please test https://github.com/swagger-api/swagger-codegen/pull/5363 to see if it fixes the issue for you.

UPDATE: reviewed C# client, server generators and confirm there's no similar issue

@wing328 Hello, thank you, I just tested it, works great, but maybe better set it to null as in other cases (when it is object)?

Because now it looks like this:

SomeDto {
 private String string = null;
 private Integer integer = null;
 private Object object = null;
 private List<String> list;
 private Map<String, String> map;
}

maybe it should be like this?

SomeDto {
 private String string = null;
 private Integer integer = null;
 private Object object = null;
 private List<String> list = null;
 private Map<String, String> map = null;
}

I know that meaning is the same, but it can make them the same in details.

@egormasalitin I was actually thinking about the opposite: to omit null for other variable type (String, Integer, etc). Given that both are the same, it's just a matter of developer's preference (I don't recall style guide mentioning anything about this in particular). Meanwhile I'll take your suggestion to add back = null for consistency.

@wing328 Thank you a lot. I saw that in the rest of the generated files it was like that, so suggested such change. I agree that it can be {TYPE} {NAME}; or {TYPE} {NAME} = null; but it should be same for all fields. Thank you again.

@wing328 Thank you

@wing328 Sorry, I forgot about 2 more questions (not related to this issue)

  1. When approximately 2.2.3 will be released?
  2. Do you have some live artifactory? I mean repository with development project versions (like 2.2.3-master or so)

Usually it takes 3 to 4 months for a patch release. Upcoming release
(minor) is 2.3.0, which is due April/May.

We don't have a nightly build. You can either build the local JAR from the
latest master or use the docker image (which is updated whenever there's a
change to master as part of the Travis CI)

On Tue, Apr 11, 2017 at 7:40 PM, Egor Masalitin notifications@github.com
wrote:

@wing328 https://github.com/wing328 Sorry, I forgot about 2 more
questions (not related to this issue)

  1. When approximately 2.2.3 will be released?
  2. Do you have some live artifactory? I mean repository with
    development project versions (like 2.2.3-master or so)

—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
https://github.com/swagger-api/swagger-codegen/issues/5240#issuecomment-293231196,
or mute the thread
https://github.com/notifications/unsubscribe-auth/AA5BdDjLJjZ_TqBvA7z9Ss5nTy_dIsLXks5ru2algaJpZM4Mrs-N
.

@wing328 thank you for your answer.

Fix is not provided for Jaxrs/cxf
Can you provide the fix for Jaxrs/cxf as well?

@Sridhar-1987 sorry I've been very busy. https://github.com/swagger-api/swagger-codegen/pull/5363 is a good starting point. Would you have time to contribute a fix?

Is there a config option to always use empty sets (arrays, maps) like the old swagger versions had? I updated my swagger and my code is throwing a lot of null pointer exceptions!

@dylan-colaco you can use customized templates with the -t option by reverting the change in https://github.com/swagger-api/swagger-codegen/pull/7045/files#diff-88dc34e41d8b27bfe76b7e0958a1675c

Hi @wing328 we are hitting the same NullPointerExceptions issue as @dylan-colaco, due to moving from a previous 2.2.2 codegen version. To resolve the issue it looks like you need to include the previous version of the pojo template as well as the two dependent enum templates.

However taking the latest enum templates causes compilation failures as the get enum method now returns a string when previously it returned the enum.

public StatusEnum getStatus() {
    return status;
}

is now : -

  public String getStatus() {
    if (status == null) {
      return null;
    }
    return status.value();
   }

This seems to be a bug.

@adholland have you tried reverting the change in https://github.com/swagger-api/swagger-codegen/pull/7045/files#diff-88dc34e41d8b27bfe76b7e0958a1675c and use the -t option?

@wing328 Yes I have reverted the changes in https://github.com/swagger-api/swagger-codegen/pull/7045/files#diff-88dc34e41d8b27bfe76b7e0958a1675c and used the -t option. This now resolves the NullPointerExceptions, however it now introduces new compilation failures.

These failures are as a result of recent changes to the enumClass.mustache and enumOuterClass.mustache. These enum templates causes compilation failures as the get enum method now returns a string when previously it returned the enum.

Example:

public StatusEnum getStatus() {
    return status;
}

is now : -

  public String getStatus() {
    if (status == null) {
      return null;
    }
    return status.value();
   }

Have you tried reverting the change in the enum template to return the enum instead of a string?

@wing328 Yeah I had tried reverting the enum template to return an enum rather than a String and that was successful.

The issue is that when moving from one release to another we shouldn't have to add these workarounds. Moving from 2.2.2 to a more recent version should generate the same results. For instance the change to set empty Lists/Map to be null should have been an option. The change to return a String rather than an enum should also be an option.

We are unwilling to move to the latest version due to the fact we are able to work with what we have and are unwilling to patch a newer version to give similar results. Also we would not be sure what sort of impact this would have if we then more to an even more recent version in future - it could start becoming a mess.

Instead of us having to patch these changes, could you not provide a param option to handle these issues in codegen? This will at least give other users in a similar position the chance of migrating to a more recent version with as little or no impact - which is what we'd expect.

Technically we can add another option to cover your use case but we prefer developers customizing the templates to meet different requirements as we may end up adding too many options while the same can be achieved by modifying the templates.

I agree with @adholland that it should have been an option to default lists/maps to null. The changes break existing code _on runtime_. I did not expect this for a _patch_ release.

@wing328 Does the parser not pick up the default values for arrays? I was trying to write my own templates, but when I used the -DdebugSupportingFiles option it doesnt show defaultValue.
Is there any workaround for this?

Was this page helpful?
0 / 5 - 0 ratings