Skip to content

Conversation

@shipra-aeron
Copy link

@shipra-aeron shipra-aeron commented Jan 7, 2021

The purpose of this PR is to allow to have all the relationships of a resource object to be always present inside relationships node and have links if they exists for a relationship.

As per the json api spec ->

A “relationship object” MUST contain at least one of the following:

  • links: a links object containing at least one of the following:
    • self: a link for the relationship itself (a “relationship link”). This link allows the client to directly manipulate the relationship. For example, removing an author through an article’s relationship URL would disconnect the person from the article without deleting the people resource itself. When fetched successfully, this link returns the linkage for the related resources as its primary data. (See Fetching Relationships.)
    • related: a related resource link
  • data: resource linkage
  • meta: a meta object that contains non-standard meta-information about the relationship.

Since above we have allowed all the relationships node to be always present, hence one should also provide a way to have atleast one to be always present out of the (data, meta and links)

The sole need for the above change is to make the client be aware of all the relationships associated with the resource objects.

@jasminb
Copy link
Owner

jasminb commented Jan 7, 2021

@shipra-aeron will take a look and comment/merge.

Thanks for the PR.

Edit:
Currently bit ill, will probably take few days to do the above.

// Make sure that relationship object i.e. statuses is null
assertNull(users.get(0).getStatuses());

byte[] convertedData = converter.writeObjectCollection(users);
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks like JSON produced here holds an empty statuses relationship object and breaks the spec which states that it must have one of links/da/data/meta.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks. I will fix it asap.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Request change has been incorporated.

Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hello, first of all apologies for a such long time that it has taken for me to take a look again.

Lib still produces empty relationship object for resources that are serialised and are lacking relationship details in the annotation. For example:

	@Relationship(value = "statuses")
	private List<Status> statuses;

Produces:

{
  "data": [
    {
      "type": "users",
      "id": "1",
      "attributes": {
        "name": "liz"
      },
      "relationships": {
        "statuses": {}
      },
      "links": {
        "self": "https://api.example.com/users/1"
      }
    },
    {
      "type": "users",
      "id": "2",
      "attributes": {
        "name": "john"
      },
      "relationships": {
        "statuses": {}
      },
      "links": {
        "self": "https://api.example.com/users/2"
      }
    }
  ]
}

It should omit relationships altogether as there are no non-empty elements in it:

{
  "data": [
    {
      "type": "users",
      "id": "1",
      "attributes": {
        "name": "liz"
      },
      "links": {
        "self": "https://api.example.com/users/1"
      }
    },
    {
      "type": "users",
      "id": "2",
      "attributes": {
        "name": "john"
      },
      "links": {
        "self": "https://api.example.com/users/2"
      }
    }
  ]
}

Adding relationship annotation fixes the issue but thats besides the point as its not required.

	@Relationship(value = "statuses", path = "/relationship/statuses")
	private List<Status> statuses;

@shipra-aeron shipra-aeron requested a review from jasminb January 12, 2021 17:50
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants