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

Adding Links to resources and included #54

Closed
kasperhelweg opened this issue Aug 2, 2015 · 39 comments
Closed

Adding Links to resources and included #54

kasperhelweg opened this issue Aug 2, 2015 · 39 comments
Assignees
Labels
Milestone

Comments

@kasperhelweg
Copy link

How can one add links to a resource object such as

{
    "data": {
        "type": "leads",
        "id": "29",
        "attributes": {
            "status": 0
        },
        "links": {
            "self": "localhost:4000\/leads\/29",
            "somelink": "link"
        }
    }
}

where "somelink" is the link to be added? Thanks!

@neomerx neomerx self-assigned this Aug 2, 2015
@neomerx
Copy link
Owner

neomerx commented Aug 2, 2015

Wiki
Code example

For your case it would look like

...
    'somelink' => new Link('link')
...

@neomerx
Copy link
Owner

neomerx commented Aug 2, 2015

Have I answered you question?

@kasperhelweg
Copy link
Author

Thanks for your reply. I get

Call to undefined method MySchema::linkAddTo()

@neomerx
Copy link
Owner

neomerx commented Aug 2, 2015

Oh yeah. Sorry for incorrect example. Tests have it's own special 'development' version of schema. I'll update 'normal' example in a few minutes.

@kasperhelweg
Copy link
Author

Thanks!

neomerx added a commit that referenced this issue Aug 2, 2015
@neomerx
Copy link
Owner

neomerx commented Aug 2, 2015

Example

which produces

{
    "data": {
        "type": "sites",
        "id": "1",
        "attributes": {
            "name": "JSON API Samples"
        },
        "relationships": {
            "posts": {
                "data": [
                    {
                        "type": "posts",
                        "id": "321"
                    }
                ],
                "links": {
                    "some-sublink": "http://example.com/sites/1/resource-sublink",
                    "external-link": "www.example.com"
                }
            }
        },
        "links": {
            "self": "http:\/\/example.com\/sites\/1"
        }
    },
    "included": [
        ...
    ]
}

@neomerx
Copy link
Owner

neomerx commented Aug 2, 2015

Is good now?

@kasperhelweg
Copy link
Author

Is it on purpose that links can only be added to the links of a relationship resource object? What I'm looking for is adding a link to the primary resource-objects links object. In your example that would be the links object containing

"self": "http:\/\/example.com\/sites\/1"

But maybe the link objects are not supposed to be used like that? Are they restricted to having only a self link and a related link?

@neomerx
Copy link
Owner

neomerx commented Aug 2, 2015

links could be added to the resource as well

@neomerx
Copy link
Owner

neomerx commented Aug 2, 2015

@kasperhelweg Does it work for you?

@kasperhelweg
Copy link
Author

Yes, it works. Thanks. One final question: how would I have the links for a resource appear in included if that resource is included? Is there a way to add links to included object? There is a

protected $isShowSelfInIncluded = true;

that will show self links. But can I add other links to those objects?

@neomerx
Copy link
Owner

neomerx commented Aug 2, 2015

Do you mean to show/hide some relationship links depending on whether object is a main resource or included resource?

@kasperhelweg
Copy link
Author

If for example I have a resource that that looks like this

"data": {
    "type": "leads",
    "id": "29",
    "attributes": {
        "status": 0
    },
    "relationships": {
         ....
    },
    "links": {
        "self": "localhost:4000\/leads\/29",
        "other-link": "other-link"
    }
}

and I include that with another resoruce

{
    ...,
    "included": [{
        "type": "leads",
        "id": "29",
        "attributes": {
            "status": 0
        },
        "relationships": {
            ....
        },
        "links": {
            "self": "localhost:4000\/leads\/29",
            "other-link": "other-link"
        }
    }]
}

@kasperhelweg
Copy link
Author

But I can only see that I can set

$isShowSelfInIncluded = true;

which will include the self link but not other-link

@neomerx
Copy link
Owner

neomerx commented Aug 2, 2015

I haven't thought about such a scenario 😄
You can do it with a schema that behaves differently depending on main resource encoded

Naive approach
        $encoder  = Encoder::instance([
            Author::class  => AuthorSchema::class,
            Comment::class => CommentSchemaWithoutLinks::class,
        ]);

        echo $encoder->encode($author) . PHP_EOL;

// and with links for comments when main resource

        $encoder  = Encoder::instance([
            Author::class  => AuthorSchema::class,
            Comment::class => CommentSchemaWithLinks::class,
        ]);

        echo $encoder->encode($comment) . PHP_EOL;
Better approach

Schema could act dynamically when created with Closure. And example of that could be found here

So for your question it might be something like

        $addLinksInComments = true;
        $schemaClosure = function ($factory, $container) use (&$addLinksInComments) {
            $schema = new CommentSchema($factory, $container, $addLinksInComments);
            return $schema;
        };
        $encoder  = Encoder::instance([
            Author::class  => AuthorSchema::class,
            Comment::class => $schemaClosure,
        ]);

        echo $encoder->encode($author) . PHP_EOL;

CommentSchema should act accordingly to value in $addLinksInComments

UPD Actually this feature a is used in tests very actively. In the code example I gave earlier you can see it in action.

@neomerx
Copy link
Owner

neomerx commented Aug 2, 2015

This feature might look as non-trivial. Are you OK with it or need some help?

@kasperhelweg
Copy link
Author

Well, I'm not sure exactly what the example does 😃 My problem is that I can't figure out how to define links in a schema in general. For example, when extending SchemaProvider I can overwrite getAttributes( ) and getRelationships( ) which works really well. I'm looking for something like a getLinks( ) method that I can overwrite. Does that make sense?

@kasperhelweg
Copy link
Author

I can add links to a top-level object by doing $encoder->encode($resource, $links, $meta) and I can add links to the relationships of an object by overwriting getRelationships( ). But I just can't figure out how to define links for a resource in general.

@neomerx
Copy link
Owner

neomerx commented Aug 2, 2015

You can't (maybe yet, I don't think it's difficult to add). Do you want Schema to have top level links that will be added to document on encoding?

@kasperhelweg
Copy link
Author

Yes, something like that... If it makes sense to have that 😄

@neomerx
Copy link
Owner

neomerx commented Aug 2, 2015

I will create a branch and we can work on your questions. As far as I understand you've got 2 issues

  • Ability to set some default links that will be added to top level links when resource is encoded as main resource (Enhancement)
  • A working example with a Schema that could be managed. E.g. $commentSchema->setIncludeLinks(bool) and depending on that value links will be added/hidden

@kasperhelweg
Copy link
Author

But if the main resource-representation contains a number of links, I guess it would make sense that those links could also be included if that resource is included with another resource? Just like all the relationships can currently be included with an included resource.

neomerx added a commit that referenced this issue Aug 2, 2015
@kasperhelweg
Copy link
Author

That would be cool! Yes, those are my issues 😄

@neomerx
Copy link
Owner

neomerx commented Aug 2, 2015

@kasperhelweg I've found even easier way to do that. Check out this sample which addresses your second issue. It corresponds with this code in Schema.

Full commit code

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

neomerx commented Aug 2, 2015

@kasperhelweg With default top level links taken from schema there is a problem. What if encoded data is array? Should links be added and if yes which ones, first resource's or from all resources?

@neomerx
Copy link
Owner

neomerx commented Aug 2, 2015

I'm afraid the only reasonable way to add these links is to pass them to Encoder on encoding. What are your thoughts on that?

@kasperhelweg
Copy link
Author

Cool. Just to be clear that we are on the same page 😄 , if I do

GET /comments/1
{
    "data": {
        "type": "comments",
        "id": "1",
        "attributes": {
            ....
        },
    "links": {
        "self": "/comments/1",
        "other-link": "other-link"
    }
}

would be a primary resource representation. I can easily do this by $encoder->encode($comment, $links) where $links contains the links. Now if I do

GET /posts/1

I would expect to get the exact same representation in the included section. Like this

{
    "data": {
        "type": "posts",
        "id": "1",
        "attributes": {
            ....
        },
        "relationships": {
            "comment": {
                "data": {
                    "type": "comments",
                    "id": "1"
                }
            }
        },
        "links": {
            "self": "/posts/1",
        }
    },
    "included": [
        {
            "type": "comments",
            "id": "1"
            "attributes": {
                ....
            },
            "links": {
                "self": "/comments/1",
                "other-link": "other-link"
            }
        }
    ]
}

The problem is that I can't easily do $encoder->encode($post, $links) because the links would be added to the posts object.

What I imagine is that there is simply a getLinks( ) method in SchemaProvider that I can overwrite just like getAttributes( ). Then, there might be a bool protected $isShowLinksInIncluded that can be set on the Schema and if true, would return links like above.

I would gladly help working on this at some point.

@kasperhelweg
Copy link
Author

Alternatively, maybe I could pass the links to the encoder but something like

$encoder->encode($post, $links)

where

$links  = [ 'post' => [ Link::SELF => new Link('/posts/1') ], 'comment' => [ .... ] ];

not sure if that makes sense at all...

@neomerx
Copy link
Owner

neomerx commented Aug 2, 2015

I think you mess a bit with links. In your first example link will be added to document level but not to comment (very easy to miss).

@neomerx
Copy link
Owner

neomerx commented Aug 2, 2015

it will look like this

{
    "links" : {
        "link-as-param-to-encode" : "/could/be/anything"
    },
    "data" : {
        "type"       : "people",
        "id"         : "9",
        "attributes" : {
            "first_name" : "Dan",
            "last_name"  : "Gebhardt"
        },
        "links" : {
            "self" : "http://example.com/people/9"
        }
    }
}

See? You have links at document level and in resource.

@kasperhelweg
Copy link
Author

Oh, you'r right.

@neomerx
Copy link
Owner

neomerx commented Aug 2, 2015

OK, wrap it up.

We have solved issue with dynamic Schema, haven't we?
We found out that document top level links looks as most reasonable solution. Having them in Schema appeared interesting from the first look but it causes issues when resources are arrayed. Agreed?

Anything left unanswered?

@kasperhelweg
Copy link
Author

Nope, that's all good! Thanks for the effort!!

@neomerx
Copy link
Owner

neomerx commented Aug 2, 2015

Super. Don't forget to support the project with ⭐ 😉

@kasperhelweg
Copy link
Author

I will!

neomerx added a commit that referenced this issue Aug 2, 2015
@neomerx neomerx added fixed and removed in progress labels Aug 2, 2015
@neomerx neomerx closed this as completed in 1bde935 Aug 2, 2015
@neomerx neomerx removed the fixed label Aug 2, 2015
@yoanisgil
Copy link

I have a somewhat similar situation:

{
  "data": [
    {
      "type": "domain",
      "id": "example.org",
      "attributes": {
        "name": "example.org",
        "ipV4Address": "1.1.1.1"
      },

    },
    {
      "type": "domain",
      "id": "as.com",
      "attributes": {
        "name": "as.com",
        "ipV4Address": "1.1.1.2"
      }
    }
  ]
}

and I'd like to include a links object for each resource of type domain. $encoder->withLinks won't work since it will added at the top level, and relationships seems a bit overkill. Below what I would like to achieve:

{
  "data": [
    {
      "type": "domain",
      "id": "example.org",
      "attributes": {
        "name": "example.org",
        "ipV4Address": "1.1.1.1"
      },
      "links": {
          "configuration": "/domains/example.org/configuration" 
      }
    },
    {
      "type": "domain",
      "id": "as.com",
      "attributes": {
        "name": "as.com",
        "ipV4Address": "1.1.1.2"
      },
      "links": {
          "configuration": "/domains/as.com/configuration" 
      }
    }
  ]
}

NOTE: I just want to know if something like this is easily doable, if not I think it will have to go for a relationships entry for each resource.

@yoanisgil
Copy link

Now that I think about it I don't really want to use relationships since I will be forced to resolve DNS records for the given domain and that's just to costly. I would really like to provide a link for an endpoint providing that information, should the client ever need to retrieve it.

@neomerx
Copy link
Owner

neomerx commented Aug 27, 2015

@yoanisgil it seems the lib restricts links to 'self' only on resource level. I've looked to the spec and there is no such restriction (likely it was in earlier revisions). It could be an enhancement/bug.

If you still want to add links despite your DNS issues you can use relationships. I don't see any real difference to have those links on resource level or relationship level.

Anyways thanks for spotting this issue. I'll create an enhancement for it.

@yoanisgil
Copy link

Not a problem. The issue with relationships is that I don't really want to return DNS records because I will be forced to resolve them which is a rather expensive operation. That's why I just want to provide a link to an endpoint providing such information.

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

No branches or pull requests

3 participants