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

Serialization Error #71

Closed
1 of 2 tasks
ChappIO opened this issue Jan 26, 2018 · 3 comments
Closed
1 of 2 tasks

Serialization Error #71

ChappIO opened this issue Jan 26, 2018 · 3 comments

Comments

@ChappIO
Copy link

ChappIO commented Jan 26, 2018

Steps to reproduce:

  1. Make sure you have a geolocated payment on your account
  2. Request a list of Payments

What should happen:

  1. The list is returned

What happens:

  1. A serialization error occurs

Traceback

SDK version and environment

  • Tested on 0.12.4
  • Sandbox
  • Production

Response id

  • Response id: e68bcf3d-e12d-41ea-b119-d0ee4a8b2af6

Extra info:

Currently com.bunq.sdk.model.generated.object.Geolocation has three BigDecimal fields. However, there seems to be a mismatch. If you take a look at com.bunq.sdk.json.BigDecimalTypeAdapter it reads a BigDecimal from type String. Yet the api returns a value of type number.

I did not make the pull request myself because I don't know which fix you'd like me to make. I see a few options:

  1. Make sure the API returns a String. However, this is probably extremely impactful as it is a breaking api change.
  2. Revert the if statement in com.bunq.sdk.json.BigDecimalTypeAdapter so it becomes
         if (input.peek() == JsonToken.NULL) {
            input.nextNull();
            return null;
        } else {
            return new BigDecimal(input.nextString());
        }
  3. Have Geolocation use Doubles instead of BigIntegers

My personal preference would go to option 2 since in my opinion it is a more resilient solution. I am willing to make the pull request myself. Just give me your decision.

@ChappIO
Copy link
Author

ChappIO commented Jan 26, 2018

My apologies, I just noticed this was already fixed in a way quite similar to option 2.
I will build application using the develop branch for now and wait for the official release.

@ChappIO ChappIO closed this as completed Jan 26, 2018
@OGKevin
Copy link
Contributor

OGKevin commented Jan 27, 2018

Hey ChappIO,

This is indeed a duplicate of #46 and will be fixed in the 0.12.5 release.

Im hoping to release it beginning next week after fixing #25.

@ChappIO
Copy link
Author

ChappIO commented Jan 27, 2018

@OGKevin thank you very much. I won't finish my application by then anyway, so I will upgrade after the release :)

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

2 participants