Remove automatic serialization

[bakura10]

Hi,

ping @Ocramius @danizord

As discussed with @danizord, there are two main problems with the current automatic serialization. For remind, the current way is either directly returns the entity (for instance a User) or a paginator. The ResourceRenderer then automatically use the hydrator to extract data, or add metadata if it detects a paginator was found.

This has two problems:

• It's hard to customize the way the output is serialized. For instance you may want to have underscore_separated keys, and wrapping all the items inside a "items" key in the payload.
• You may want to add other data that are computed and actually don't belong to the entity itself. For instance, the GitHub API (https://api.github.com/users/danizord) adds data like "public_repos" that don't really belong to the entity itself (I suppose those are fetched from a repository and added to the payload). Because the hydrator is used, it makes it nearly impossible.

Therefore, albeit convenient, all this automation is hard to use in a lot of cases. This PR completely removes any automation, so the serialization has to be done in the controller directly:

public function get(User $user)
{
    return ['username' => $user->getUsername()]; // ...
}

Note that this solution is far from optimal, because now everything has to be done manually. Furthermore, a lot of duplication is done (there is great chance that the "get" method of the UsersCollectionController will do the same thing).

[danizord]

Wow, you threw out a lot of code :o

Well, this change makes it a lot more flexible, while harder to use. Now the controller has more control over the proccess, which is great. But now the developer need to write more code, I'm not pretty sure what is the best way.

Probably we should provide a easy way to the user to get the hydrator, and then use it to extract data, instead of extract it by hand. Maybe a controller plugin?

Also, now we can replace CreateResourceModelListener by a content-negotiation listener to create the appropriate view model, e.g a JsonModel, so we can use JsonRenderer to render it :)
It'd be great if it works with html response :D

[Ocramius]

This removes information that may be valuable to the view layer, such as associated resources.

[bakura10]

So... what should I do ? Basically, without this, ZfrRest is more or less only traversal + validation.

[danizord]

@bakura10 The powerful ZfrRest routing reduces a lot of boilerplate code in controllers.

[bakura10]

Hi everyone,

So here (again!) for a new proposal regarding this issue. This solution is based on an idea that danizord throw yesterday on IRC.

First, here is the feedback: this PR completely removes any automatism. So it offers a lot of customization but needs to write a lot of boilerplate code, when it's often not needed.

The idea is to use controller plugins to generate an initial payload IN THE CONTROLLER. Then, the user can alter the payload by adding more properties (or remove properties) before returning it.

As a consequence, the controller is actually responsible to extract the data (this seems correct as it's actually what happens when we pass variable to the view).

The user now MUST return a ResourceModel (array is no longer supported). The ResourceModel will then be passed to a ResourceRenderer (like actual ZfrRest) but with one big difference: the resource renderer does not extract data from payload. This step has already been done in the controller.

However, the only task of the renderer is to format this data. The ResourceRendererInterface will be made of various methods like "normalizeKey", "normalizeLink", "normalizeAssociation"... Actually, I already tried to implement this feature before, but it was not in the view layer and therefore felt wrong. Now, as a view layer part, it feels good.

The question that quickly arises was: how the renderer can know what is actual entity data, paginator data like count page, links... It cannot know it. Actually, if your controller send data like this:

[
    'username' => 'foo',
    'age' => 23,
    'current_page' => 4
]

It cannot knows that "current_page" was actually created from paginator, while other data come from the entity.

I sat down, looked at how other langauges did it and it appears you have three big categories:

• data : this is actually the data itself. In context of a JavaScript app, it's what is actually map to the entity client side.
• links : links to other resources.
• meta : metadata is anything else. An example of metadata is data from paginator (current_page, total_count...).

The resource model therefore wrap those three kinds of data:

class ResourceModel
{
    public function __construct($data, $metadata, $links, ResourceMetadata $metadata)
}

In your controller, all this data is created through controller plugin:

Inside a item controller:

public function get(User $user)
{
    $data = $this->extractData($user);

    // Add custom data
    $followersCount = $this->userService->getFollowersCount($user);
    $data['followers_count'] = $followersCount;   

    return new ResourceModel($data);
}

Inside a collection controller:

public function get(Collection $users)
{
    // We may want to get a paginator...
    $paginator = $this->paginatorWrapper($users);
    $paginator->setCurrentPage(2);

   // For data, maybe $paginator->getCurrentItems())
    $data = $this->extractData($paginator);
    $meta = $this->extractMetadata($paginator);

    return new ResourceModel($data, $meta);
}

What's importnat here is that "extractData" will actually return only data for the users (using the hydrator). NOT the paginator data. This will allow the ResourceRenderer to dissociate those two!

Thoughts?

[Ocramius]

@bakura10 I like the idea of not auto-converting to ResourceModel - much more clean.

As for the metadata about pagination & co, we could really use HAL - it already uses special "reserved" keys to store all this information (current page, next page, page amount, blabla)

As for building the resource model, shouldn't we at least pass some sort of identifier or list of identifiers (or the entities) to the model itself? I don't see how it is going to build links otherwise...

[bakura10]

For the links it's currently a bit obscure for me now, as the ZfrRest router still does not support generating link, and to be honest I have no idea about how to do it (I tried a syntax some weeks ago but it was so obscure... I'm not sure anyone could understand it). We would need to have a syntax for "assemble" method to be able to create links.

Regarding HAL I'll see, but the whole point of the ResourceRenderer is to be able to have any syntax, hence the fact that the ResourceModel actually separate entity data, metadata... so that we can provide one or two renderers for the most popular formats (HAL, maybe JsonAPI)

[Ocramius]

@bakura10 links don't need to be relative - absolute links are also fine. As for where to put them, HAL manages also that.

I think it is fine to have a default renderer using HAL first, then other things. After all, renderers are pluggable :-)

[bakura10]

@Ocramius , the main problem is that: let's say you have a route "/users", and the URI is POST "/users/1/tweets". Actually, only the route for "users" exist. The tweets route is "auto-discovered" by the router. "/users/:id/tweets" has not been explicitly define in a config.

Starting from this, the following cannot work:

$this->assemble("users/tweets", ...)

Because the only route that exist is "users". That was my whole problem from the beginning. Not a question of absolute vs relative.

[Ocramius]

@bakura10 I'd actually say that $this->assemble($tweet) would work fine. I agree on subpaths, that's tricky, but we can leave it un-implemented until we have a solution (we will need a specialized helper that can return null)

[Ocramius]

Going to repeat what I already discussed with @bakura10 on IRC.

It looks to me that this change was introduced to allow custom data manipulation in the controller.
My idea is that this is wrong - this does not need to be handled by ZfrRest. If you need custom data, just use a JsonModel.

Here's how I would re-think the feature:

• serialization (to array and then to json) happens in the view layer
• ZfrRest handles serialization only when a ResourceModel is passed to the view
• the ResourceModel requires the intact resource (entity or collection) and its metadata
• the view layer uses the configured hydrator to extract data from the resource

So yeah, kinda like what we're doing already, but moved to a ResourceModelRenderer.

If you have the use case for custom data, we already implemented that: use a custom hydrator!
I have very complex hydrators in some of my projects, such as new UserHydrator(new UserScoreService()) - they allow this sort of data manipulation and retain their single responsibility.

An alternative may be to simply use a JsonModel:

public function get(...)
{
    return new JsonModel(array_merge(
        $this->getZfrRest()->getHydrator($entity)->extract($entity),
        $myCustomData
    ));
}

Where getZfrRest and getHydrator are just examples. This case would require the user to play around with ZfrRest's internal services (and that's fine!).

@danizord ping

[bakura10]

@Ocramius , maybe you have not follow all my changes but... isn't it exactly what I'm doing currently: https://github.com/zf-fr/zfr-rest/blob/master/src/ZfrRest/View/Renderer/ResourceRenderer.php#L84 ?

[bakura10]

What I wanted to achieve with the various controller plugins is to have a place to actual do transformation data (like transforming keys to underscore_separated for instance). Now, as the renderer already do the extracting, I don't want to give too much responsibility to it.

However currently ZfrRest create a ResourceModel if it receives an object. I'll make sure to enforce the ResourceRenderer is triggered only fro ResourceModel.

[Ocramius]

@bakura10 in the example, you pass an array to the ResourceModel. I think the array should be optional, while the resource should be mandatory...

[bakura10]

Well, I can simply remove this listener:

zfr-rest/src/ZfrRest/Mvc/CreateResourceModelListener.php

Line 64 in b4aecf8

if (($result instanceof ModelInterface) || $result === null || !$resource instanceof ResourceInterface) {

Then the ResourceStraetgy will delegate the work to ResourceRenderer only if it's a ResourceModel

[Ocramius]

@bakura10 and that's fine - I was more thinking about the example in your controller:

public function get(User $user)
{
    $data = $this->extractData($user);

    // Add custom data
    $followersCount = $this->userService->getFollowersCount($user);
    $data['followers_count'] = $followersCount;   

    return new ResourceModel($data);
}

[bakura10]

Addressed by #116