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

Getting current object in relation provider. #290

Closed
Invis1ble opened this issue Apr 14, 2019 · 15 comments
Closed

Getting current object in relation provider. #290

Invis1ble opened this issue Apr 14, 2019 · 15 comments
Labels

Comments

@Invis1ble
Copy link

Invis1ble commented Apr 14, 2019

In previous version relation providers receives current object as the first argument.
After updating to the latest version I can't find a way to get current object. When I try to pass it with object variable like this:

My\DTO:
    # ... some config ...
    relation_providers: [ "expr(service('my_dto_relation_provider').addRelations(object))" ]

then the following error appears:

Variable "object" is not valid around position 72 for expression service('my_dto_relation_provider').addRelations(object).

How can I get current object inside service? I need it because I dynamically add some relations based on current object and authenticated user.
Thanks.

@goetas
Copy link
Collaborator

goetas commented Apr 15, 2019

Relations in v3.x can not be "dynamically" added or removed, but they can be "dynamically excluded" .

This means that your addRelations can not accept parameters, but inside the method you can define relations that use an exclusion strategy to exclude that relation in some cases (and exclusion strategies have access to object).

Here an example on how should be done.
The Hateoas\Configuration\Relation constructor has a Hateoas\Configuration\Exclusion parameter where you can specify the exclusion rules.

@goetas goetas closed this as completed Apr 15, 2019
@egonolieux
Copy link

egonolieux commented May 10, 2019

What about building a collection of relations from an array property of the object (e.g. customer->orders). I'm not sure how exclusion strategies would work here?:

    public function getExtraRelations(Customer $customer) : array
    {
        foreach ($customer->getOrders() as $order)
        {
            $relations[] = new Relation(
                'orders',
                new Route(
                    'card',
                    ['orderId' => $order->getId()],
                    true
                )
            );
        }

        return $relations;
    }

@goetas
Copy link
Collaborator

goetas commented May 11, 2019

In that case exclusion strategies will not work in the best way.

Here the relation is still one: orders. just with multiple "values". Probably should be possible to alter the current behavior somehow allowing "array" values on a relation... but that still needs to be implemented.

I know that by HAL specs your case is perfectly valid, but wouldn't in that case be better to use _embedded? Having just an array of links seems not that useful.

Created #293 as feature request to keep track of it.

@egonolieux
Copy link

My intention was to create a single relation by following the example given here. To my knowledge it seems the creation of a single relation with multiple links is done by returning an array containing multiple relation objects using the same name.

I'm not sure what you are suggesting in this new issue. Are you suggesting to simplify the above process by allowing an array of routes on the relation object? How does this solve the problem of not being able to dynamically build the array of links/routes based on a property of the object?

Regarding _embedded, I can give you a more specific example. What about self referencing many to many relations? Let's say you have a card game, and different cards may have the same artwork. I would want to reference those cards without fully embedding them:

"_links": {
    "cardsWithSameArtwork": [
        {"href": "/cards/1"},
        {"href": "/cards/2"},
        {"href": "/cards/3"}
    ]
}

@goetas
Copy link
Collaborator

goetas commented May 11, 2019

The problem with the previous way of handling links was that:

    public function getExtraRelations(Customer $customer) : array
    {
        foreach ($customer->getOrders() as $order)
        {
            $relations[] = new Relation(
                'orders',
                new Route(
                    'card',
                    ['orderId' => $order->getId()],
                    true
                )
            );
        }

        return $relations;
    }

When there are multiple order, will return:

{
"_links": {
    "orders": [
        {"href": "/order/1"},
        {"href": "/order/2"},
        {"href": "/order/3"}
    ]
}
}

But if your object has only one order, will return:

{
"_links": {
    "orders": 
        {"href": "/order/1"}
}
}

That is wrong as the consumer will expect an array of links, but here gets an object

@egonolieux
Copy link

egonolieux commented May 11, 2019

Ah, I understand now. So even if I was able to dynamically generate the links, the single object would be quite unexpected. How would you suggest implementing the cards example I gave in the context of the current cababilities of the library while still being valid HAL (without embedding the full objects)?

@goetas
Copy link
Collaborator

goetas commented May 11, 2019

IMO the only way (without making changes to the willdurand/Hateoas library) is to return some value-object for your Order object, and define hateoas metadata for that value object.

To have:

{
"_embedded": {
    "orders": [
     {"_links":{"href": "/order/1"}},
     {"_links":{"href": "/order/3"}},
     {"_links":{"href": "/order/3"}}
  ]
}

@goetas
Copy link
Collaborator

goetas commented May 11, 2019

Your value object could be:

/** @relation(PUT LINK INFO HERE) */
class ValueOrder
{
  function __construct(Order $realOrder){
  //...
  }
}

@egonolieux
Copy link

That would work, thanks!

@egonolieux
Copy link

Sorry to bother you again, but I just ran into the same issue again for another project. I would like to ask why you think passing object to a relation provider is a bad idea? And why do you think having an array of links would not be useful? This seems like an extremely common use case for to-many relations. _embedded should only be used to embed full resources, not to provide a list of links in my opinion (thats what _links is for).

Previously, using _embedded turned out to be the best way to go in my case, because we later decided to include more data than just links. However, the project I'm working on now literally just needs a list of links. Since we can't pass object to the relationprovider anymore, it seems this can't be done and that we're forced to use _embedded.

Having to do this is pretty ugly:

{
    "_links": {
        "self": { "href": "/authors/1" }
    },
    "_embedded": {
        "books": [
            { "href": "/books/1" },
            { "href": "/books/2" },
            { "href": "/books/3" },
        ]
    }
}

compared to:

{
    "_links": {
        "self": { "href": "/authors/1" },
        "books": [
            { "href": "/books/1" },
            { "href": "/books/2" },
            { "href": "/books/3" },
        ]
    }
}

@goetas
Copy link
Collaborator

goetas commented Jun 25, 2019

I would like to ask why you think passing object to a relation provider is a bad idea?

Not passing the object to the relation provider means that relations do not depend on the underlying data. This means that they can be cached and as example can be generated automatic documentation about relations.
If your relations depend on your objects, caches will be per-object instead (means potentially infinite..)

And why do you think having an array of links would not be useful?

It do not think that is not useful, that is why i have created #293 to keep track of the feature request.

I think this is something that can be useful, will be happy to se an implementation proposal.

@egonolieux
Copy link

It do not think that is not useful, that is why i have created #293 to keep track of the feature request.

My bad, thought that issue was referring to this problem specifically:
#290 (comment)

Not passing the object to the relation provider means that relations do not depend on the underlying data. This means that they can be cached and as example can be generated automatic documentation about relations.
If your relations depend on your objects, caches will be per-object instead (means potentially infinite..)

But the annotations in my objects also refer to the underlying data. If I want to embed a books relation in author, I want the books specifically for that author. So how is this different from the relation provider?

@goetas
Copy link
Collaborator

goetas commented Jun 26, 2019

@egonolieux
Lets suppose you have this relation provider, as it was in 2.x:

use Hateoas\Configuration\Relation;
use Hateoas\Configuration\Route;

class UserRelPrvider
{
    /**
     * @return Relation[]
     */
    public function getExtraRelations($object, ClassMetadataInterface $classMetadata)
    {
        // You need to return the relations
        $relations = [];
       if (!$object->someMethod()) {
           $relations[] =  new Relation(
                'self',
                new Route(
                    'foo_get',
                    ['id' => 'object.getId()']
                )
            );
        }
        return $relations;
    }
}

Without having the actual object that will be used for se, we can not tell anything about the relations that are going to be serialized.
Each time we encounter a new object to serialize, we need to call getExtraRelations to discover the relations is going to have.

Lets consider this second example as it is in 3.x:

use Hateoas\Configuration\Relation;
use Hateoas\Configuration\Route;
use Hateoas\Configuration\Exclusion;

class UserRelPrvider
{
    private $evaluator;
    public function __construct(CompilableExpressionEvaluatorInterface $evaluator)
    {
        $this->evaluator = $evaluator;
    }
    /**
     * @return Relation[]
     */
    public function getExtraRelations(): array
    {
        // You need to return the relations
        return array(
            new Relation(
                'self',
                new Route(
                    'foo_get',
                    ['id' => $this->evaluator->parse('object.getId()', ['object'])]
                ),
                null, 
                [],
                new Exclusion(
                     null,
                     null,
                     null,
                     null,
                    $this->evaluator->parse('object.someMethod()', ['object'])
                )
            )
        );
    }
}

In this case we do not need the object and we can already call getExtraRelations and discover that we will have an optional relation named self.

This information as example is used by libraries as NelmioApiDocBundle to automatically generate documentation from the code.

In the same way, since the expression-language expressions to generate the links (object.someMethod() or object.getId() as example) are not dependent from the data, they can be parsed only once for the whole lifetime of the application.
This library compiles all the expression only the first time a class is loaded. This can be also pre-compiled using the cache warm-up feature, removing any expression parsing at run-time (means that getExtraRelations will never be called at run-time).

Hope this gives you an overview of the difference between evaluating expressions at run-time depending on the underlying data, versus being able to discover the relations at deploy/cache-warm-up time.

@egonolieux
Copy link

I understand what you mean now, thanks for taking the time to explain!

I don't know to what extent you plan on including a replacement for this functionality regarding #293, but what do you think of something like this?

 * @Hateoas\Relation(
 *     "books",
 *     @Hateoas\RelationCollection(
 *         expr(object.getBooks()),
 *         href=@Hateoas\Route(
 *             "book",
 *             parameters={"bookId"="expr(collectionItem.getId())"},
 *             absolute=true
 *         )
 *     )
 * )

The RelationCollection would accept an iterable value (expr(object.getBooks())), of which its items would be exposed as collectionItem. I don't know much about the internal workings though, so I have no idea if this is possible.

Maybe we should continue this in the relevant issue #293 ?

@goetas
Copy link
Collaborator

goetas commented Jun 27, 2019

I like your proposal. having a RelationCollection annotation could be a good idea.

I don't know to what extent you plan on including a replacement for this functionality..

Actually I do not have plans to work on this feature in the near future, but would be happy to see a PR implementing it, and of course I'm available to review it and/or to provide help.

Probably we can continue the conversation on #293

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