square / retrofit

A type-safe HTTP client for Android and the JVM
https://square.github.io/retrofit/
Apache License 2.0
43.11k stars 7.3k forks source link

Issue<Using Custom GSON Converters for multiple Base_API_URLs> #1914

Closed Sheshlok closed 8 years ago

Sheshlok commented 8 years ago

Hi - I have searched extensively on SO and issues list here but could not find an optimal solution to the issue. Can you please help ? Here is the gist: https://gist.github.com/Sheshlok/f8ee09a6760f6aa3b05ad9c03d6b247b

Issue The second API returns a malformed JSON response, specifically, one that is wrapped in "finance_charts_json_callback(.....)". I tried implementing a custom GSON converter as you can see in the gist above but keep getting the same error ([@Observable#getStockHistory -> onError() -> 'Use JsonReader.setLenient(true) to accept malformed JSON at line 1 column 1 path $'])

Note:

  1. Have checked JsonpParser with the actual API response and it works fine.
  2. I can solve this problem by using OkHttp class 'onResponse' callback, and trimming the response string appropriately before parsing JSON. However, I needed to use Observables.
JakeWharton commented 8 years ago

You could use an OkHttp interceptor to detect this prefix and then wrap the Source with one that skips the prefix finance_charts_json_callback( and always reads one byte-ahead to detect EOF and skip the trailing ). This is effectively the same thing that you said, except you are doing it in the normal request pipeline and it can thus be done automatically by the client instead of at the Retrofit level.

You can certainly continue to do this in a custom converter though. It's hard to see what's wrong with it since there's no tests to prove the behavior.

I will say that the readerToString method can be completely eliminated by just calling value.string() in the convert method, although that's unlikely to be part of the problem.

Sheshlok commented 8 years ago

@JakeWharton : Thanks for clarifying and simplifying the code !!! One point of clarification though on the custom converters - when you say "there's no tests to prove the behavior", do you mean I should write a test, similar to 'GsonConverterFactoryTest' in gson src, to make sure its working as desired OR is it something else you are referring to?

Sheshlok commented 8 years ago

@JakeWharton : Am including the 'LoggingInterceptor' output here. Please have a look - May be this can help to find out why the custom converter is not working.
https://gist.github.com/Sheshlok/935016b5dbc761f1d83739009ad07158

Sheshlok commented 8 years ago

@JakeWharton : Thanks, the 'interceptor' worked seamlessly and got the job done. As you said, It definitely is more elegant than writing custom converters for this particular use case.

Hence, I am still intrigued at why the custom converters did not work. Anything you can suggest to troubleshoot ?