Skip to content
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

Handle null values in Kotlin TypesAdapter #109

Open
mikeyshean opened this issue Feb 11, 2020 · 10 comments
Open

Handle null values in Kotlin TypesAdapter #109

mikeyshean opened this issue Feb 11, 2020 · 10 comments

Comments

@mikeyshean
Copy link

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

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

@macisamuele
Copy link
Collaborator

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
Copy link
Collaborator

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

A failing JUnit test would also suffice

@djensen47
Copy link

djensen47 commented Feb 13, 2020

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
Copy link
Collaborator

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
Copy link
Collaborator

ping @djensen47 @mikeyshean

@djensen47
Copy link

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
Copy link
Collaborator

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
Copy link

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

@cortinico
Copy link
Collaborator

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
Copy link

djensen47 commented Oct 27, 2020

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?

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

No branches or pull requests

5 participants