kamikat / moshi-jsonapi

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

fix #11 support primitive error fields #16

Closed mreichelt closed 8 years ago

coveralls commented 8 years ago

Coverage Status

Coverage increased (+0.6%) to 93.078% when pulling 9cb89eec7b8e784bd4c133923251cfb3fe286512 on mreichelt:fix-11-support-primitive-error-fields into fcc44e098de87b484525723f3307816db95ef137 on kamikat:master.

mreichelt commented 8 years ago

@kamikat here is a proposal how we can easily support error parsing by throwing an exception from the parsing process. Big kudos to @mrnugget for pair programming with me! :) Next step is trying it out with Retrofit.

kamikat commented 8 years ago

Thank you @mreichelt and @mrnugget

Throwing an IOException for error in business domain from parser's scope is thought an anti-pattern to single responsibility principle. And it can mess things up saying we got a parser exception when there's something wrong in the business domain.

coveralls commented 8 years ago

Coverage Status

Coverage decreased (-0.2%) to 90.827% when pulling b4b295cdfe3b59a0e0c372581ece87acc1004159 on mreichelt:fix-11-support-primitive-error-fields into 8f5e466cdedccb20164fa8cb50a7cd41121b54d6 on kamikat:master.

coveralls commented 8 years ago

Coverage Status

Coverage decreased (-0.2%) to 90.827% when pulling b4b295cdfe3b59a0e0c372581ece87acc1004159 on mreichelt:fix-11-support-primitive-error-fields into 8f5e466cdedccb20164fa8cb50a7cd41121b54d6 on kamikat:master.

mreichelt commented 8 years ago

@kamikat I refactored the code so that no exception is thrown during the parsing process. In fact, the parsing process now is unaware of errors. Developers can parse the error object directly, like this:

JsonApiError apiError = moshi().adapter(JsonApiError.class).fromJson(json);

So now, with Retrofit, users can handle the response or parse the error when unsuccessful:

Response<User> response = userApi.create(user).execute();
if (response.isSuccessful()) {
    User newUser = response.body();
    // do something with freshly created user
} else {
    JsonApiError apiError = moshi.adapter(JsonApiError.class).fromJson(response.errorBody().source());
    // do something with error
}

To avoid boilerplate code, I introduced an OkHttp interceptor in our project to automatically throw an error when a JSON API error occurs (so not every call has to parse the exception manually):

@Override
public okhttp3.Response intercept(Chain chain) throws IOException {
    okhttp3.Response response = chain.proceed(chain.request());
    if (response.isSuccessful()) {
        return response;
    } else {
        // JsonApiException is our own and extends IOException
        throw new JsonApiException(moshi.adapter(JsonApiError.class).fromJson(response.body().source()));
    }
}

I hope you like the updated PR. The new code is already up and running in our project. Thanks again to @mrnugget who helped me on this, and who suggested to use a mockwebserver to test everything.

mreichelt commented 8 years ago

@kamikat any updates on this? I really need error support for our project… Right now I have to use my own (source-copied) variant.

kamikat commented 8 years ago

I'm now working on adding getter/setter support to Resource object. I'll merge the commit as soon as a getter/setter pattern is supported by the project.