Skip to content

Fix getter methods for Java model with additionalProperties#301

Closed
stianlik wants to merge 3 commits intoOpenAPITools:masterfrom
stianlik:pr-8245
Closed

Fix getter methods for Java model with additionalProperties#301
stianlik wants to merge 3 commits intoOpenAPITools:masterfrom
stianlik:pr-8245

Conversation

@stianlik
Copy link
Contributor

@stianlik stianlik commented Jun 13, 2018

stianlik added 2 commits June 13, 2018 07:45
@stianlik
Fix getter methods for Java model with additionalProperties
bd016f3 
When a model class is configured with additionalProperties, getters for
non-dynamic properties always return null because values have been set as properties
of the map instead of the class. As a workaround, this commit ensures that we these
values are available in getters as well.

This fixes #4970, fixes #5259, and fixes #5187
@stianlik
Update Petstore sample
dc854f0
@wing328 wing328 added this to the 3.0.2 milestone Jun 13, 2018
@wing328
Copy link
Member

wing328 commented Jun 13, 2018

cc @bbdouglas (2017/07) @JFCote (2017/08) @sreeshas (2017/08) @jfiala (2017/08) @lukoyanov (2017/09) @cbornet (2017/09) @jeff9finger (2018/01) for review

JFCote
JFCote requested changes Jun 13, 2018
View reviewed changes
samples/client/petstore/java/okhttp-gson/src/main/java/org/openapitools/client/JSON.java Outdated
})
;
return fireBuilder.createGsonBuilder();
GsonBuilder builder = fireBuilder.createGsonBuilder();
Copy link
Member

@JFCote JFCote Jun 13, 2018

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

why this change?

Copy link
Contributor Author

@stianlik stianlik Jun 13, 2018

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is a change related to one of my other PRs, not sure why it was included here, removed after regenerating samples. Sorry about that.

samples/client/petstore/java/rest-assured/src/main/java/org/openapitools/client/JSON.java Outdated
})
;
return fireBuilder.createGsonBuilder();
GsonBuilder builder = fireBuilder.createGsonBuilder();
Copy link
Member

@JFCote JFCote Jun 13, 2018

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

why?

@jmini
Copy link
Member

jmini commented Jun 13, 2018

Shippable — Run 770 status is FAILED.

It seems that other java clients under samples/ are affected by this change.

  • run mvn clean verify locally
  • run bin/java-petstore-all.sh
  • commit changes to this PR.

(instead of bin/java-petstore-all.sh you can also run bin/ensure-up-to-date -- the same script than the one executed by Shippable CI. But this will take longer to run).

@stianlik
Copy link
Contributor Author

stianlik commented Jun 13, 2018

It looks like this issue has been resolved differently in openapi-generator. When additionalProperties is defined, the java client does not implement a domain class, but returns a generic Map instead. If that is the case, this PR is useless.

@jmini @JFCote Can you confirm that the expected behavior is as described above?

@JFCote
Copy link
Member

JFCote commented Jun 13, 2018

I have create a bug for this here:
#191

@stianlik
Copy link
Contributor Author

stianlik commented Jun 15, 2018

I'm closing this PR as it's a bugfix for an obsolete solution to #191. Assuming #191 is solved properly, it is no need for an additional bugfix.

@stianlik stianlik closed this Jun 15, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@JFCote JFCote JFCote requested changes

Assignees

No one assigned

Labels

Client: Java Issue: Bug

Projects

None yet

Milestone

3.0.2

Development

Successfully merging this pull request may close these issues.

4 participants

@stianlik @wing328 @jmini @JFCote