kamikat / moshi-jsonapi

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

Priority should make more sense #57

Closed reisub closed 7 years ago

reisub commented 7 years ago

Hi @kamikat,

@JsonApi has a priority value and this is what the docs say:

    /**
     * Priority to determine the class to de-serializing a generic type of
     * resource across classes of same resource type name.
     * Class with smaller priority value should be chosen by the conflict
     * resolving function.
     */
    int priority() default 100;

I find it very counter-intuitive that a lower value means higher priority. May I suggest changing the logic so that higher number means higher priority?

kamikat commented 7 years ago

Umm... The design concern for priority is to manually set the deserialization class for specific type name since all types must be registered in ResourceAdapterFactory to se-/dese-rialize. But as of 3.2.0 Resource classes can be "JSONify" without being added to ResourceAdapterFactory. So I'm thinking about deprecating the priority annotation. Is there any use case about priority?

reisub commented 7 years ago

So you're saying the models no longer need to be registered with ResourceAdapterFactory, even if they are used in relationships?

Here's what I'm using priority for right now (on 3.0.6).

I have two models which both have @JsonApi(type = "therapies")

One of them is the "real resource" which contains all of the fields the API returns, and the other one is used as a request body to update a single property of the resource without sending the rest of the fields in the request and it contains only that single field.

So what I want is to always have the real resource have higher priority in deserializing, otherwise the relationships including this resource break with a ClassCastException, and currently I achieve this by using priority.

Is there another way to do this?

kamikat commented 7 years ago

So you're saying the models no longer need to be registered with ResourceAdapterFactory, even if they are used in relationships?

No, the class must be registered when doing deserialization. But a class can be un-registered when it is only used in serialization. And classes used in relationships must be registered since they are relevant with deserialization.

In your case, you can register the class with all of the fields (which is relevant with deserialization) to ResourceAdapterFactory and leave the class for request body (which is only used in serialization) un-registered. (and priority is no longer useful in this case)

reisub commented 7 years ago

Great, sounds good!

kamikat commented 7 years ago

Commit https://github.com/kamikat/moshi-jsonapi/commit/f1664f88bdcf886f626a8240618fc510381b51fd deprecates priority and introduces policy to @JsonApi as a part of class documentation.

kamikat commented 7 years ago

The patch is released as 3.2.2 🍻