Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Links-only relations not handled correctly #77

Closed
lindyhopchris opened this issue Aug 4, 2020 · 4 comments · Fixed by #78
Closed

Links-only relations not handled correctly #77

lindyhopchris opened this issue Aug 4, 2020 · 4 comments · Fixed by #78

Comments

@lindyhopchris
Copy link

Detailed description

If you retrieve an item that has links-only relations, modify it and then save it - the JSON sent to the remote server includes the relations with an empty data member - even though the client doesn't know what the data content of that relationship is.

Here's an example to illustrate this. Given this code:

$repo = app(\App\Repositories\PostRepository::class);
$post = $repo->find(1)->getData();
$json = $post->toJsonApiArray();

Say the server replied to the find request with this:

{
  "data": {
    "type": "posts",
    "id": "1",
    "attributes": {
      "content": "This is my first blog.",
      "title": "Hello World!"
    },
    "relationships": {
      "comments": {
        "links": {
          "self": "http://localhost/api/v1/posts/1/relationships/comments",
          "related": "http://localhost/api/v1/posts/1/comments"
        }
      }
    },
    "links": {
      "self": "http://localhost/api/v1/posts/1"
    }
  }
}

I.e. the comments relationship has no data member.

When toJsonApiArray() is called, it will serialize this:

[
    'type' => 'posts',
    'id' => '1',
    'attributes' => [
        'content' => 'This is my first blog.',
        'title' => 'Hello World!'
    ],
    'relationships' => [
        'comments' => [
            'data' => [],
        ],
    ],
    'links' => [
        'self' => 'http://localhost/api/v1/posts/1'
    ],
]

I.e. the comments relationship now has an empty data member. This is incorrect because the data member was never retrieved from the remote server, so the client has no way of knowing what the value for the relationship is.

Presumably the same bug will occur if the server replied with a relationship that only had a meta member.

Context

The JSON API spec says that a relationship must have at least one of data, links, meta - so the client should handle relationships that do not have the data member. It should not assume the relationship has an empty data member for a relationship for which it has not been provided a data member.

In my scenario, changing an attribute on the post and saving it back to the server causes the request to be rejected, as the server does not accept a data member for the comments relationship.

Possible implementation

When serializing relationships, particularly when saving the resource, any relationships that do not have a data member should be skipped, rather than the relationship being assumed to be empty.

Your environment

Laravel 7.22, PHP 7.2, latest version of this package.

@JaZo
Copy link
Member

JaZo commented Aug 5, 2020

@lindyhopchris, can you verify this is fixed on master?

@lindyhopchris
Copy link
Author

@JaZo Hi! thanks for looking at this so quickly.

This does now work - it doesn't assume that a links-only relation is empty.

However, it throws up a new issue when saving the resource. Relations with no data member should not be present in the payload that's sent to the remote server. This is specified in the spec for creating:

If a relationship is provided in the relationships member of the resource object, its value MUST be a relationship object with a data member. The value of this key represents the linkage the new resource is to have. https://jsonapi.org/format/#crud-creating

And for updating...

If a relationship is provided in the relationships member of a resource object in a PATCH request, its value MUST be a relationship object with a data member. The relationship’s value will be replaced with the value specified in this member. https://jsonapi.org/format/#crud-updating

In other words, the serialization for an outbound request from the client to the remote server is a bit different - it needs to strip out relations that do not have a data member.

@JaZo JaZo reopened this Aug 5, 2020
@JaZo
Copy link
Member

JaZo commented Aug 5, 2020

You are absolutely right, missed that! I've changed the serialization to strip out relations without a data member. Thanks for testing!

@JaZo JaZo closed this as completed Aug 6, 2020
@JaZo
Copy link
Member

JaZo commented Aug 6, 2020

Fix released in 1.1.1.

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 a pull request may close this issue.

2 participants