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

The library does not support JSON Specification "8.3 Inclusion of Related Resources" #258

Closed puneethere closed 1 year ago

puneethere commented 1 year ago

As per the JSON Specification "8.3 Inclusion of Related Resources"

If an endpoint supports the include parameter and a client supplies it:

Now in my case, the client is supplying the include parameter and the corresponding relationship is empty then the JSON Response does not have the "included" key, following is the response:

{ "data": { "type": "XXX", "id": "XXX", "attributes": { "AAA": "XXX", "BBB": "XXX" }, "relationships": { "RRRR": { "data": [] } } } }

For data security, I have annotated the real data with arbitrary values.

jasminb commented 1 year ago

Hello @puneethere, thank you for submitting the ticket. Will take a look when I get some time and update you here.

jasminb commented 1 year ago

@puneethere taken a quick look, If I am understanding you correctly, you are expecting the included attribute to be present regardless of the fact that there is no data to be included as the relationship that you are including has no elements?

puneethere commented 1 year ago

@jasminb your understanding is correct, as per json specification, the included element must be present as an empty array if the relationship is empty

puneethere commented 1 year ago

@jasminb have a look at the section 8.3 of Json specification https://jsonapi.org/format/.

Also, please do let me know if this can be fixed? Thanks

jasminb commented 1 year ago

@puneethere will try to create a new release this week.

jasminb commented 1 year ago

@puneethere please checkout the latest version of the tool (0.12) and if ok close the issue.

puneethere commented 1 year ago

Thanks for the quick turnaround on this one @jasminb. I will update the library and will get back to you. Thanks

puneethere commented 1 year ago

@jasminb I have tested the latest version (0.12) and the API response now do have the included key, but it is present irrespective of whether the client supplies the include parameter or not.

puneethere commented 1 year ago

For instance, all endpoints now produce the following output, even when client does not supply the 'include' parameter: {"data": [], "included": []} rather when client doesn't supply the 'include' parameter then the response should not have the 'included' key.

jasminb commented 1 year ago

Please provide a unit test that reproduces the issue, here is a unit test that is passing in case relationship inclusion is disabled:


  @Test
  public void testRelationshipInclusionDisabled() throws DocumentSerializationException {
    ResourceConverter customConverter = new ResourceConverter(Article.class, Author.class);
    customConverter.disableSerializationOption(SerializationFeature.INCLUDE_RELATIONSHIP_ATTRIBUTES);

    Author author = new Author();
    author.setId("author_id");
    author.setLastName("Bar");
    author.setFirstName("Foo");
    author.setArticles(new ArrayList<Article>());

    byte[] serialized = customConverter.writeDocument(new JSONAPIDocument<>(author));
    // included element is not there as the relationship inclusion is disabled on the converter level
    Assert.assertFalse(new String(serialized).contains("\"included\":[]"));

    SerializationSettings serializationSettings =
      new SerializationSettings.Builder().includeRelationship("articles").build();

    // included element is present as the settings are enabling the serialization of relationship
    serialized = customConverter.writeDocument(new JSONAPIDocument<>(author), serializationSettings);
    Assert.assertTrue(new String(serialized).contains("\"included\":[]"));
  }

Note that inclusion of the empty included[] element depends on serialization options on ResourceConverter level or on the level of writeDocument invocation using SerializationSettings.

puneethere commented 1 year ago

I understand that the library would only respond with included key in the response when the SerializationFeature.INCLUDE_RELATIONSHIP_ATTRIBUTES is enabled on ResourceConverter. And I need this feature as I have endpoints which based on client request needs to respond with 'included' array. The presence of included should be govern by whether the client is requesting for it or not by supplying the 'include' parameter in the endpoint request. Currently what is happening is that even when client is not supplying the 'include' parameter in the endpoint request, the library is producing the response with blank 'included' array. Now if we disable the INCLUDE_RELATIONSHIP_ATTRIBUTES then it will stop sending the included array even when client has requested with 'include' parameter.

In summary, what I am alluding to is that once we have INCLUDE_RELATIONSHIP_ATTRIBUTES enabled on the resource converter, then the library should also check for the 'include' parameter in the request before adding the 'included' array in the response. This is as per the JSON specification.

jasminb commented 1 year ago

Ok, so if I understand you correctly, you are using the lib on the server side as a framework to serialise responses to user requests? If this is the case, I suggest configuring different flavours of the resource converter and using them depending on the user preference, e.g. if the user sent the include param, use the instance that has the INCLUDE_RELATIOINSHIP_ATTRIBUTES enabled, if no param is sent than use an instance with disabled rel attributes.

Another way would be to have a single instance and use the SerializationSettings that are built for each requests using the user-provided params.

jasminb commented 1 year ago

Closing as fixed and the requested feature is out of scope for the library (understanding user requests in context of an HTTP API).