jasminb / jsonapi-converter

JSONAPI-Converter is a Java/Android library that provides support for working with JSONAPI spec
Apache License 2.0
273 stars 81 forks source link

Relationships that can return multiple types #136

Open dulleh opened 7 years ago

dulleh commented 7 years ago

So we have an issue with the kitsu.io api where a relationship we need can return one of two different types (in our case, mediaRelationship.source can be either an anime or a manga). I've written a test case which fails for unknown reasons, and was thinking perhaps you could help.

The test consists of a parent object that contains two child objects with the same @Relationship annotation as so:

@Type("polymorph-parent")
@JsonIdentityInfo(generator = ObjectIdGenerators.StringIdGenerator.class, property = "id")
@JsonIgnoreProperties(ignoreUnknown = true)
public class PolymorphRelationship {
    @Id
    private String id;

    @Relationship("arbitraryRelationship")
    public User users;

    @Relationship("arbitraryRelationship")
    public Author author;

    public String getId() {
        return id;
    }

    public void setId(String id) {
        this.id = id;
    }
}

The test:

    @Before
    public void setup() {
        converter = new ResourceConverter("https://api.example.com", Status.class, User.class, Author.class,
                Article.class, Comment.class, Engineer.class, EngineeringField.class, City.class,
                NoDefaultConstructorClass.class, PolymorphRelationship.class);
    }

    @Test
    public void testReadPolymorphicRelationship() throws IOException {
        InputStream parentStream = IOUtils.getResource("polymorph-relationship.json");

        JSONAPIDocument<PolymorphRelationship> parentDoc = converter.readDocument(parentStream, PolymorphRelationship.class);
        PolymorphRelationship parent = parentDoc.get();

        Assert.assertNotNull(parentDoc);
        Assert.assertNotNull(parent);
        if (parent.users != null) {
            System.out.println("parsed as user");
            Assert.assertEquals("1", parent.users.id);
            Assert.assertEquals("sam_i_am", parent.users.getName());
        } else if (parent.author != null) {
            System.out.println("parsed as author");
            Assert.assertEquals("1", parent.author.getId());
            Assert.assertEquals("sam_i_am", parent.author.getFirstName());
        } else {
            Assert.fail();
        }
    }

The json to be parsed (polymorph-relationship.json):

{
  "data":{
    "id":"someId",
    "type":"polymorph-parent",
    "relationships":{
      "arbitraryRelationship":{
        "links":{
          "self":"https://api.com/w/e",
          "related":"https://api.com/w/e"
        },
        "data":{
          "type":"people",
          "id":"1"
        }
      }
    }
  },
  "included":[
    {
      "type": "people",
      "id": "1",
      "attributes": {
        "firstName": "sam_i_am"
      }
    }
  ]
}

This gives the output: parsed as author

This passes so the assumption is that changing the object returned by the api to a User object will also pass:

{
  "data":{
    "id":"someId",
    "type":"polymorph-parent",
    "relationships":{
      "arbitraryRelationship":{
        "links":{
          "self":"https://api.com/w/e",
          "related":"https://api.com/w/e"
        },
        "data":{
          "type":"users",
          "id":"1"
        }
      }
    }
  },
  "included":[
    {
      "type": "users",
      "id": "1",
      "attributes": {
        "name": "sam_i_am"
      }
    }
  ]
}

However, this fails with the following error:

java.lang.IllegalArgumentException: Can not set com.github.jasminb.jsonapi.models.Author field com.github.jasminb.jsonapi.models.PolymorphRelationship.author to com.github.jasminb.jsonapi.models.User

    at sun.reflect.UnsafeFieldAccessorImpl.throwSetIllegalArgumentException(UnsafeFieldAccessorImpl.java:167)
    at sun.reflect.UnsafeFieldAccessorImpl.throwSetIllegalArgumentException(UnsafeFieldAccessorImpl.java:171)
    at sun.reflect.UnsafeObjectFieldAccessorImpl.set(UnsafeObjectFieldAccessorImpl.java:81)
    at java.lang.reflect.Field.set(Field.java:764)
    at com.github.jasminb.jsonapi.ResourceConverter.handleRelationships(ResourceConverter.java:481)
    at com.github.jasminb.jsonapi.ResourceConverter.readObject(ResourceConverter.java:330)
    at com.github.jasminb.jsonapi.ResourceConverter.readDocument(ResourceConverter.java:190)
    at com.github.jasminb.jsonapi.ResourceConverterTest.testReadPolymorphicRelationship(ResourceConverterTest.java:45)
    at sun.reflect.NativeMethodAccessorImpl.invoke0(Native Method)
    at sun.reflect.NativeMethodAccessorImpl.invoke(NativeMethodAccessorImpl.java:62)
    at sun.reflect.DelegatingMethodAccessorImpl.invoke(DelegatingMethodAccessorImpl.java:43)
    at java.lang.reflect.Method.invoke(Method.java:498)
    at org.junit.runners.model.FrameworkMethod$1.runReflectiveCall(FrameworkMethod.java:50)
    at org.junit.internal.runners.model.ReflectiveCallable.run(ReflectiveCallable.java:12)
    at org.junit.runners.model.FrameworkMethod.invokeExplosively(FrameworkMethod.java:47)
    at org.junit.internal.runners.statements.InvokeMethod.evaluate(InvokeMethod.java:17)
    at org.junit.internal.runners.statements.RunBefores.evaluate(RunBefores.java:26)
    at org.junit.runners.ParentRunner.runLeaf(ParentRunner.java:325)
    at org.junit.runners.BlockJUnit4ClassRunner.runChild(BlockJUnit4ClassRunner.java:78)
    at org.junit.runners.BlockJUnit4ClassRunner.runChild(BlockJUnit4ClassRunner.java:57)
    at org.junit.runners.ParentRunner$3.run(ParentRunner.java:290)
    at org.junit.runners.ParentRunner$1.schedule(ParentRunner.java:71)
    at org.junit.runners.ParentRunner.runChildren(ParentRunner.java:288)
    at org.junit.runners.ParentRunner.access$000(ParentRunner.java:58)
    at org.junit.runners.ParentRunner$2.evaluate(ParentRunner.java:268)
    at org.junit.runners.ParentRunner.run(ParentRunner.java:363)
    at org.junit.runner.JUnitCore.run(JUnitCore.java:137)
    at com.intellij.junit4.JUnit4IdeaTestRunner.startRunnerWithArgs(JUnit4IdeaTestRunner.java:68)
    at com.intellij.rt.execution.junit.IdeaTestRunner$Repeater.startRunnerWithArgs(IdeaTestRunner.java:51)
    at com.intellij.rt.execution.junit.JUnitStarter.prepareStreamsAndStart(JUnitStarter.java:237)
    at com.intellij.rt.execution.junit.JUnitStarter.main(JUnitStarter.java:70)
    at sun.reflect.NativeMethodAccessorImpl.invoke0(Native Method)
    at sun.reflect.NativeMethodAccessorImpl.invoke(NativeMethodAccessorImpl.java:62)
    at sun.reflect.DelegatingMethodAccessorImpl.invoke(DelegatingMethodAccessorImpl.java:43)
    at java.lang.reflect.Method.invoke(Method.java:498)
    at com.intellij.rt.execution.application.AppMain.main(AppMain.java:147)

Is it possible that the library can be made to leave relationships as null if the returned type is different? Perhaps to prevent errors for regular relationships being ignore a @PolymorphRelationship could be created?

Kevinrob commented 7 years ago

I do something similar with #108 😃

dulleh commented 7 years ago

@Kevinrob your PR does not resolve the issue as the returned object will be parsed just for the fields in the interface rather than as it's own object. In case that's not clear, here's what I mean: if we have a Parent model with a child field that can receive a Son or Daughter (implements Child) from the API, the json will be parsed as a Child rather than a Son or Daughter. We want to be parsing a Son or Daughter depending on what's received from the API.

Kevinrob commented 7 years ago

@dulleh I don't think that what you said it's right. I use it in production and relationships declared with an interface are deserialized to the correct type.

dulleh commented 7 years ago

It seems @Kevinrob is right, sorry about the confusion. I was under the impression that when you do something like Parent item = new Child() it casts the item as a Parent and therefore loses any additional information that a Child has (it doesn't). I've learned something fundamental about inheritance, thanks for helping me get there.

This means that #137 is no longer necessary as #108 already accomplishes what the PR attempts. However, I do think having polymorphic relationships explicitly defined is beneficial for models compared to having the responsibility fall on developers to comment their models with the return types. I don't know which @jasminb would prefer for this library of course, and there still remains the bug with RelationshipLinks in #137.