search

Yelp / swagger-gradle-codegen

💫 A Gradle Plugin to generate your networking code from Swagger
Apache License 2.0
412 stars 37 forks source link

Handle null values in Kotlin TypesAdapter #109

Open mikeyshean opened 4 years ago

mikeyshean commented 4 years ago

Example: Null values for BigDecimal in request body will throw "Nesting Problem" error. Description of root cause is similar to the issue described here: https://github.com/square/moshi/issues/806

The TypesAdapter will need explicit handling for null values to prevent this.

macisamuele commented 4 years ago

Asking here, as the issue looks a better place to talk about it.

Could you please publish a minimal spec example that triggers the issue?

cortinico commented 4 years ago

Could you please publish a minimal spec example that triggers the issue?

A failing JUnit test would also suffice

djensen47 commented 4 years ago

It might take us a while to construct a useful JUnit test because the YAML triggering the error is just too big to use as a test case: https://developer.sabre.com/sites/default/files/eWejJIu8.yaml

Aside: The attribute that triggered the issue on serialization was OnTimeRate and it was the first BigDecimal value that was attempted to be processed.

So we'll have to construct something artificial but similar.

macisamuele commented 4 years ago

So we'll have to construct something artificial but similar.

That's definitely OK. Having an additional endpoint into junit_tessts_specs.json would be great.

I might be supportive on identifying the minimal example. Something that would help is having a small example on how you were interacting with the API that lead to the identification of the issue. cc: @djensen47

macisamuele commented 4 years ago

ping @djensen47 @mikeyshean

djensen47 commented 4 years ago

Sorry, your ping got lost in a river of Github notifications.

We're not going to have cycles to create a proper reproduction of the bug until early or mid-April. Same goes for updating the proposed PR. We have a workaround in place but since it's a bit of a hack we definitely want to get this fixed properly in the library. I'm actually creating a ticket in our Jira system to revisit and help fix this.

cortinico commented 4 years ago

We're not going to have cycles to create a proper reproduction of the bug until early or mid-April. Same goes for updating the proposed PR. We have a workaround in place but since it's a bit of a hack we definitely want to get this fixed properly in the library. I'm actually creating a ticket in our Jira system to revisit and help fix this.

Awesome, keep us posted 👍

stew-wwt commented 4 years ago

Any update this issue? We ran into the same issue implementing the plugin in our project today

cortinico commented 4 years ago

Any update this issue? We ran into the same issue implementing the plugin in our project today

A reproducer would be helpful to investigate. Having a sample swagger file would also be useful. Ideally we would love to see #110 merged in some form soon.

djensen47 commented 4 years ago

Unfortunately, the scope and status of our team and project has changed since the pandemic hit and I'm not sure we have the bandwidth to work on this. If @stew-wwt could provide a file to reproduce, then maybe somebody else could take over the PR?