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

Encoding Relationships #55

Closed
lindyhopchris opened this issue Aug 4, 2015 · 20 comments
Closed

Encoding Relationships #55

lindyhopchris opened this issue Aug 4, 2015 · 20 comments
Assignees
Milestone

Comments

@lindyhopchris
Copy link
Collaborator

Hi,

Just wondering how relationship endpoints are meant to be encoded? The encoder interface doesn't seem to have a method for encoding relationships, and I can't find any mention in your wiki about it.

The example instance from the JSON-API spec would be:

GET /articles/1/relationships/author HTTP/1.1
Accept: application/vnd.api+json
HTTP/1.1 200 OK
Content-Type: application/vnd.api+json

{
  "links": {
    "self": "/articles/1/relationships/author",
    "related": "/articles/1/author"
  },
  "data": {
    "type": "people",
    "id": "12"
  }
}
@neomerx neomerx self-assigned this Aug 4, 2015
@neomerx
Copy link
Owner

neomerx commented Aug 4, 2015

Your links belongs not to the resource but to the document (side question for you what do you think it would belong to if data contains array of resources?).

Anyways you can add such links with a second parameter of encode method. For your case it will be

    $links = [
        'self' => new Link('/articles/1/relationships/author'),
        'related' => new Link('/articles/1/author'),
    ];

However most likely you want

  • to have those links in resource relationships
  • have them automatically generated

In this case you need to add them in Schema like here.

If you have any further questions please don't hesitate to ask.

@lindyhopchris
Copy link
Collaborator Author

The problem is the data member, not the links member. There's no way of passing in an object that has a type and id member, because the encoding process tries to look up a SchemaProvider for the object.

Effectively this endpoint is returning a Relationship Object that is in the root document. I've tried providing a Neomerx\JsonApi\Schema\RelationshipObject as the first parameter for encode, but it doesn't work because the encoding process tries to look up a SchemaProvider for it.

As there's an error and errors encoding method on the EncoderInterface, does there not also need to be a relationship method for encoding Relationship Objects directly to the document?

Also, would it not be possible for this to be done using the SchemaProvider? In the example above, effectively the endpoint is returning the author relationship that can be obtained by using the getRelationships method on the ArticleSchema object.

So effectively what you want to be able to do is something like this on the encoder interface:

public function relationship($resource, $relationship, $meta = null, EncodingParametersInterface $parameters = null)

which in the example would be called as:

$encoder->relationship($article, 'author');

@neomerx
Copy link
Owner

neomerx commented Aug 4, 2015

I admit I don't understand the question. After reading your original question a few more times I can't understand where it might come from.

My questions in particular

  • what is a 'relationship endpoint'?
  • why it should be encoded separately from object?
  • what is a real life example of it (maybe this help)?

I think it will be easier to discuss in more interactive way with Gitter

@neomerx
Copy link
Owner

neomerx commented Aug 4, 2015

This post might be completely irrelevant to your question but what I think while reading you second comment

There's no way of passing in an object that has a type and id member, because the encoding process tries to look up a SchemaProvider for the object.

Encoder don't work with input data directly it uses Schema for that. You can program Schema for reading any properties from objects (named id and type as well). You can register Schema for any type including object (obviously no more than 1 Schema per PHP data type) but I don't understand why not using classes for that as they have stronger typing.

Effectively this endpoint is returning a Relationship Object that is in the root document.

I've got same set of questions what is this endpoint, what PHP type it has, why it should be encoded separately and how it fits JSON-API spec.

does there not also need to be a relationship method for encoding Relationship Objects directly to the document?

That's likely the same question applied to similar object Error. I don't understand the original idea about endpoint-relationship and how it fits the spec.

Also, would it not be possible for this to be done using the SchemaProvider?

A real example might help to understand what you want to achieve.

@lindyhopchris
Copy link
Collaborator Author

@neomerx It's been evening here (York, UK) and I've been out tonight!

Will be able to write more tomorrow if needed, but the endpoint I'm talking about is fully documented in the JSON API spec. See http://jsonapi.org/format/#fetching-relationships

A server MUST support fetching relationship data for every relationship URL provided as a self link as part of a relationship's links object.

A server MUST respond to a successful request to fetch a relationship with a 200 OK response.

The primary data in the response document MUST match the appropriate value for resource linkage, as described above for relationship objects.

That section of the spec has examples in it.

@neomerx
Copy link
Owner

neomerx commented Aug 4, 2015

I got it.

You have to add handler to your controller that reads author/comments from data storage and returns encoded result.

class ArticlesController
{
    public function getRelationship($articleId, $relationshipName)
    {
        ...
        $author = ...;

        $links = ???

        $this->getResponse($author, 200, $links);
    }
}

The question is what is the best way to create $links?

$author doesn't contain information about response links and that it actually come from a relation to Article. So Encoder can't add such links to output without additional data.

You know you're in Article controller handler for relationship requests so one option is to create such data manually

$links = [
    'self' => new Link("/articles/$articleId/relationships/$relationshipName"),
];

Another option is to automate Link process generation fully or partly. For such link generation we need

  • Articles JSON-API type (from ArticleSchema)
  • $articleId and $relationshipName (Controller parameters)

Thus such generation function might look like

createRelationshipLink(Article::class, $articleId, $relationshipName);

What are your thoughts on that?

@lindyhopchris
Copy link
Collaborator Author

I really feel this totally has to be automated, because otherwise I'm having to program a lot of logic into a controller that is actually contained within the relevant schema providers. It's a duplication of code, which means I'll have two places to update stuff.

For this request:

GET /articles/1/relationships/author HTTP/1.1
Accept: application/vnd.api+json

The minimum response is:

HTTP/1.1 200 OK
Content-Type: application/vnd.api+json

{
  "data": {
    "type": "people",
    "id": "12"
  }
}

(I've intentionally left out the links, as they are not compulsory whereas the data is.)

In the controller, I get the article from the data store, and then get the author from that. I now have to construct a resource identifier for the response, for which I need to know in the controller that the type of $author is people. To work that out without hard coding it into my controller, I need to get the schema provider out of your container.

Also, if I'm using a single instance of an Encoder across my whole application, the other thing I'll need to do in the controller is return a response that is encoded consistently with every other endpoint in the API. That information is held in the EncoderOptions that the Encoder is created with. So therefore it definitely makes sense to me to use the encoder to generate the JSON response.

So the result of this, is I need the encoder to generate the entire JSON response, both data and links.

Another thing is the spec says that the response at this endpoint needs to match the relationship object contained within the resource object to which it relates.

The primary data in the response document MUST match the appropriate value for resource linkage, as described above for relationship objects.

So if my article resource object looks like this:

{
  "type": "articles",
  "id": "1",
  "attributes": {
    "title": "Rails is Omakase"
  },
  "relationships": {
    "author": {
      "links": {
        "self": "/articles/1/relationships/author",
        "related": "/articles/1/author"
      },
      "data": { "type": "people", "id": "9" }
    }
  }
}

Then the response to the /articles/1/relationships/author endpoint must match the value in the above JSON at /data/relationships/author. That means that the ArticleSchema class already knows how to generate that, because if you call getRelationships on that schema it's the author key in the returned array.

So all I need to do in the controller is get the article based on the id, then asked the encoder to encode the author relationship for the supplied article. The schema provider for article already knows how to get the author from the article because that's written into the getRelationships method, and it knows what the links are because they are also written into that method.

@neomerx neomerx added this to the 1.0 milestone Aug 5, 2015
@neomerx
Copy link
Owner

neomerx commented Aug 5, 2015

I've got an idea to split this task into 2 parts

  • first one is adding a helper for adding links to document
  • the second is adding support for encoding relationship of a given object (which will be using the first part)

I've added a non-working prototype (feature/issue55 branch) for the first part (needs small changes in Encoder). Please have a look if we are on the same page. Possible usage might look like

$encoder = ...;
$encoder->withRelationshipLink(
    DocumentInterface::KEYWORD_SELF,
    Article::class, 
    $articleId,
    $relationshipName
)->encode(...);

the code itself

Yes, I would appreciate your help with it. I'm working on CORS support at the moment (which might be useful for you as well).

@neomerx neomerx added ready and removed question labels Aug 5, 2015
@neomerx
Copy link
Owner

neomerx commented Aug 5, 2015

@lindyhopchris I can do first part quickly however the second part looks more complicated and I can start working on it in a few days

@neomerx
Copy link
Owner

neomerx commented Aug 5, 2015

@lindyhopchris I've added prototype for second part. It might be not as difficult as I thought in the first place.

the code

@lindyhopchris
Copy link
Collaborator Author

@neomerx

Yeah, the encodeRelationship looks fine - it makes sense to have a different method on the EncoderInterface that is specifically for this case, as the encoded data is different from encoding resource objects. It would make sense for the public interface to expose this single method rather than a 2-step process.

The only thing I'd question is the $linkName argument. For me it makes more sense for the first two arguments to be $resourceObject and $relationshipName, as they always have to be provided. The link is optional.

In the draft encodeRelationship you've written, will the internal call to encode know to encode $data as a resource identifier, rather than a resource object?

So for the author relationship on an articles resource object, this would be the correct result:

{
    "data": {
        "type": "people",
        "id": 1
    }
}

but this would not be correct:

{
    "data": {
        "type": "people",
        "id": 1,
        "attributes": {
            "firstName": "Foo",
            "surname": "Bar"
        }
}

@lindyhopchris
Copy link
Collaborator Author

PS: Working on it in the next few days is not a problem! Not expecting this to be solved straight away. We're currently prototyping a JSON-API in an application that's not due for immediate release, so I'm not working against an immediate deadline.

@neomerx
Copy link
Owner

neomerx commented Aug 5, 2015

Yeah, the encodeRelationship looks fine - it makes sense to have a different method on the EncoderInterface that is specifically for this case, as the encoded data is different from encoding resource objects. It would make sense for the public interface to expose this single method rather than a 2-step process.

I think of replacing encode list of parameters with methods. It will allow combining functionality with methods like encodeRelationship and adding new parameters via chain methods

$encoder = ...;
$encoder->withLinks(...)->withMeta(..)->withSomeBeer(...)->encode($data, $options);

The only thing I'd question is the $linkName argument. For me it makes more sense for the first two arguments to be $resourceObject and $relationshipName, as they always have to be provided. The link is optional.

You are right for this particular method it's not needed. The method can send the right value to one of the chaining methods.

In the draft encodeRelationship you've written, will the internal call to encode know to encode $data as a resource identifier, rather than a resource object?

It will be using the Schema used on $encoder creation. So the result depends on the Schema implementation. At the moment I have no definite opinion on whether it should force encoding as resource identifier and why adding attributes will be an error.

Anyways feel free to post a PR to feature/issue55 branch.

@lindyhopchris
Copy link
Collaborator Author

@neomerx

At the moment I have no definite opinion on whether it should force encoding as resource identifier and why adding attributes will be an error.

The JSON API spec says that a relationship endpoint returns the resource linkage, not the resource object.

For our article with an author relationship example, there are two endpoints that can be exposed for the relationship. These are the self and related links that are on the relationship object.

This request (the related link in the relationship):

GET /api/article/1/author

Returns the resource object:

{
    "data": {
        "type": "people",
        "id": 1,
        "attributes": {
            "firstName": "Foo",
            "surname": "Bar"
        }
   }
}

From the spec:

Consider, for example, a request to fetch a to-one related resource link. This request would respond with null when the relationship is empty (such that the link is corresponding to no resources) but with the single related resource's resource object otherwise.
http://jsonapi.org/format/#fetching-resources

Whereas this request (the self link in the relationship):

GET /api/article/1/relationships/author

Returns the resource linkage:

{
    "data": {
        "type": "people",
        "id": 1
    }
}

The primary data in the response document MUST match the appropriate value for resource linkage, as described above for relationship objects.
http://jsonapi.org/format/#fetching-relationships

Not meaning to be a pain about this, but there is definitely a distinction between the two! Your library is a great implementation, adding in this ability to encode the relationship resource linkage will increase its coverage of the spec.

@neomerx
Copy link
Owner

neomerx commented Aug 5, 2015

@lindyhopchris Ok thanks for your help with the spec.

  • encodeRelationship that returns resource identifier should be added
  • encodeRelationship might add 2 links for self and related

@neomerx
Copy link
Owner

neomerx commented Aug 8, 2015

@lindyhopchris I've partly solved the issue. How relationship links could be added you can see in this example.

As you can see you can encode relationships as this

$encoder->withRelationshipRelatedLink($post, $relationshipName)->encodeData($post->{$relationshipName});

Are you OK with this approach or like encodeRelationship($post, relationshipName) more? Here is prototype

The only problem encodeData encodes with attributes and relationships. I can see 2 solutions

  • Use Schemes that return empty attributes and relationships. This approach is very easy and could be used right now.
  • Add a special method for encoding resources as data identifiers e.g. encodeAsResourceIdentifiers (I would be happy if you suggested better name 😉 ). It's a good and clean solution from design perspective however it wouldn't be easy to add. It will require adding new parser manager that will instruct parser to skip following resources' relationships, new parser result interpreter which will ignore resources' attributes and possibly a couple more helper object. As everything is connected with interfaces It looks possible it's just won't be trivial.

Would be nice to have some feedback from you.

@neomerx
Copy link
Owner

neomerx commented Aug 10, 2015

@lindyhopchris I've finished this enhancement (some samples from tests here).

The code is in develop branch

That's the code snippet for encoding relationships

$encoder = ...;
$resource = ...;
$relationshipName = '...';

$encoder
    ->withRelationshipSelfLink($resource, $relationshipName)
    ->withRelationshipRelatedLink($resource, $relationshipName)
    ->encodeIdentifiers($resource->{$relationshipName});

@lindyhopchris
Copy link
Collaborator Author

@neomerx

Looks good. I'll be able to try it out tomorrow at work (sorry, didn't get a chance to do it today). From the sample tests, it looks like it'll work nicely.

@neomerx
Copy link
Owner

neomerx commented Aug 10, 2015

@lindyhopchris I would be happy to hear your opinion on paging links generation enhancement #61

@lindyhopchris
Copy link
Collaborator Author

@neomerx this all works a treat. Thanks for adding it in. :-)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants