Bugfix: links and pagination handling

[Art4]

This PR covers #21 and #22. The following changes were made:

• DocumentLink, ErrorLink and RelationshipLink allows every custom links as URL or object
• An object inside links will be parsed as Link and could contain URLs, other link objects or a meta object.
• Pagination links (which can only be string or null) will parse as string or Link object, if the parent Relationship object doesn't have a "data" attribute.

@fcastillo-es Can you confirm this changes will fix the two issues?

Note: I'm not finished yet. I'm now working on tests and refactoring for reaching 💯% code coverage again.

[fcastilloes]

Sorry for the delay, it's been a couple busy days.

I'm testing now and I find ok the custom links. I just noticed that if a link has a "meta" key, the client parses it like a Meta object, but from the spec in the Links section:

The value of each links member MUST be an object (a "links object")

So from that I think that a links member can have no meta object, but if a meta key is found, it should be treated like a link.

As for #21 I can't get the validation. If I have time I'll look it deeper, but I think that what the code is doing right now is that if data is present at the same level that the links member, it tries to set the pagination links, but it does'n check if data is object or collection, so if there is data and its not a collection I get pagination links anyway.

And while we're on it, it seems your intent is to silently remove pagination links in case they're not valid. Shouldn't be this also a validation error?

[Art4]

I just noticed that if a link has a "meta" key, the client parses it like a Meta object, but from the spec in the Links section:

The value of each links member MUST be an object (a "links object")

Your quote from the spec is out of context. See the sentence before:

Where specified, a links member can be used to represent links. The value of each links member MUST be an object (a "links object").

This means, if you have an object (e.g. Document) with a links member, this member must be an object (a "links object").

{
  "data":{...},
  "links": { // <- This links member must be an links object
    ...
  }
}

The next sentence is the significant part. Every part in a links object must be a "link".

Each member of a links object is a "link". A link MUST be represented as either:

• a string containing the link's URL.
• an object ("link object") which can contain the following members:
  • href: a string containing the link's URL.
  • meta: a meta object containing non-standard meta-information about the link.

So this example is valid:

"links": {
  "related": {
    "href": "http://example.com/articles/1/comments",
    "meta": { // <- Meta object
      "count": 10
    },
  },
  "meta": { // <- Link object
    "href": "http://example.com/meta-url",
    "meta": { // <- Meta object
      "foo": "bar"
    }
  }
}

So, you are right. JsonApiClient parses the links incorrect, but links can contain a meta object. I'll fix that.

but it does'n check if data is object or collection, so if there is data and its not a collection I get pagination links anyway.

Thanks, you're right. Will be fixed.

it seems your intent is to silently remove pagination links in case they're not valid. Shouldn't be this also a validation error?

No, if there is no data attribute, the pagination links will be parsed a normal link. They would still be there and would not removed.

[fcastilloes]

Sorry for the quote, I was trying to be concise, but obviously did it wrong.

About the last point

it seems your intent is to silently remove pagination links in case they're not valid. Shouldn't be this also a validation error?

No, if there is no data attribute, the pagination links will be parsed a normal link. They would still be there and would not removed.

I mean if there is a data attribute, but data is not a collection, but a resource object, resource identifier object or null. In that case pagination links should be invalid.

I agree that with no data attribute the pagination links are valid.

[Art4]

I mean if there is a data attribute, but data is not a collection, but a resource object, resource identifier object or null.

In this case the pagination links should be parsed as a "normal" link too.

[fcastilloes]

In that case, I'm ok with just the fixes mentioned before.

[Art4]

Pagination links will now parsed, if the relationship "data" attribute contains a identifier collection.

I'm now working on the "meta" objects in links which won't be so trivial. 😃 I need the parent object inside Link::__construct() to check if "meta" should be parsed as Link or Meta object.

[Art4]

I've made the changes but spotted another problem: A link object can't be empty by spec, but JsonApiClient allows this atm. I could add more if statements in Link object, but it would imho to complex.

I'm thinking about introducing a new Resource\ItemLink for the "links" attribute in Resource\Item. This would be more consistent, because we already have DocumentLink, ErrorLink and RelationshipLink.

[fcastilloes]

👍