kamikat / moshi-jsonapi

JSON API v1.0 Specification in Moshi.
MIT License
156 stars 34 forks source link

Add example project for Retrofit usage #19

Closed mreichelt closed 7 years ago

mreichelt commented 8 years ago

During the development of PR https://github.com/kamikat/moshi-jsonapi/pull/16 I found it incredibly helpful to test the integration of moshi-jsonapi and Retrofit. Because this is quite a common use-case and involves some additional code that currently has no documentation, I would like to add a retrofit-example Java module that shows how to use moshi-jsonapi with Retrofit. Bonus: we can add nice integration tests with MockWebServer!

@kamikat please tell me if you would like to have this. If you do I'll start working on it! :) 🎉

p-fischer commented 7 years ago

Even with the documentation about interface declaration, I'm still having trouble to define the RequestBody and ResponseBody classes properly. @kamikat A sample project using moshi-json API in combination with Retrofit would still be very helpful.

kamikat commented 7 years ago

@p-fischer I'm sorry but I've no much time working on the example module. However, we may discuss about questions you have in your project and help improve the documentation.

p-fischer commented 7 years ago

Hi @kamikat thanks for your answer! One question I'm having is: I prefer List<> types over arrays. Are those supported? I'm new to Moshi. All the examples I've found are not in conjunction with Retrofit but create an explicit adapter.

When using List<> in my Retrofit interface specification as return type I get the following error:

java.lang.IllegalArgumentException: Could not locate ResponseBody converter for java.util.List<com.takeda.models.DiaryEntry>.
                                                                      Tried:
                                                                       * retrofit2.BuiltInConverters
                                                                       * com.takeda.serialization.JsonApiConverterFactory
                                                                        at retrofit2.Retrofit.nextResponseBodyConverter(Retrofit.java:349)
                                                                        at retrofit2.Retrofit.responseBodyConverter(Retrofit.java:311)
                                                                        at retrofit2.ServiceMethod$Builder.createResponseConverter(ServiceMethod.java:735)

If I use an array type instead it works. Should lists work out of the box?

PS: These are my dependencies:

compile "com.squareup.retrofit2:retrofit:2.2.0" // retrofit 2
compile 'moe.banana:moshi-jsonapi:3.0.6'
kamikat commented 7 years ago

@p-fischer You can try the updated JsonApiConverterFactory.

Maybe there should be a module for this factory instead of a gist.

p-fischer commented 7 years ago

@kamikat Thanks for your super quick reply and your adjustments! I agree an extra module for the converter would be a good solution. (It could use the original jsonapi module as a dependency, so that one only needs to include one dependency - either the plain or the retrofit version.) I'm using the new version now. However, I run into this exception:

java.lang.RuntimeException: Cannot find default constructor of [java.util.List].
    at com.takeda.serialization.JsonApiConverterFactory$MoshiResponseBodyConverter.convert(JsonApiConverterFactory.java:170)
    at com.takeda.serialization.JsonApiConverterFactory$MoshiResponseBodyConverter.convert(JsonApiConverterFactory.java:125)
    at retrofit2.ServiceMethod.toResponse(ServiceMethod.java:118)
...

I was looking into the moshi converter and did not find an equivalent piece of code. Is that because moshi does not rely on empty constructors and moshi-jsonapi does? Can interfaces be used at all with moshi-jsonapi?

kamikat commented 7 years ago

Ah... I forget to place a default class for List. The exception is thrown by https://gist.github.com/kamikat/baa7d086f932b0dc4fc3f9f02e37a485/revisions#diff-6cc28be0a3606a8ff3ad7ce3e432507fR115

I've updated the gist to fix that... 😅

p-fischer commented 7 years ago

@kamikat it works! thanks a lot! I'm starting to work with the lib now :)