jsonapi-converter icon indicating copy to clipboard operation
jsonapi-converter copied to clipboard

Relationships that can return multiple types

Open dulleh opened this issue 8 years ago • 4 comments

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?

dulleh avatar Jul 07 '17 11:07 dulleh

I do something similar with #108 😃

Kevinrob avatar Jul 25 '17 09:07 Kevinrob

@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.

dulleh avatar Jul 27 '17 14:07 dulleh

@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.

Kevinrob avatar Aug 02 '17 15:08 Kevinrob

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.

dulleh avatar Aug 02 '17 18:08 dulleh