[WIP] First working version (need reviews)

[bakura10]

ping @Ocramius @borisguery @nimajneb

Hi everyone,

It's been a long time but I've been able to work on ZfrRest this whole week-end and I've been able to have a first "working" version. For a rough idea about how everything works, please check the Readme here: https://github.com/bakura10/ZfrRest/tree/develop

This PR merged several pending pull-requests (including @Ocramius initial work on route). Please note that I had to fix a bug on ZF 2 (zendframework/zendframework#4104) to make this working.

Please focus especially on the route algorithm (ZfrRest\Mvc\Route\Http\ResourceGraphRoute). The idea is to recursively parse the route and alternatively handle identifier, then association, then identifier... and so on. For instance, a route like "/users/5/tweets" will start at "users", find the users n°5, then check if the association to tweets has been exposed by mapping, and if so, get the collections of tweets.

The route also support query filtering for last part. So route like this "/users/5/tweets?title=something" also works.

As a result and if the mapping has been properly defined, it will be automatically dispatched to the controller, in the following form:

public function getList(Collection $tweets, ResourceMetadataInterface $metadata)
{
}

public function get(Tweet $tweet, ResourceMetadataInterface $metadata)
{
}

This means that you already receive concrete instances in your controller. You do not need to manually get a service, fetch the entity, filter it manually... all of this is handled by the route.

The code needs a lot of review, of course.

Here are a few questions:

1. The "entry points" need to be specified in config file like any other route. For instance, you will have this in your config:

return array(
    'router' => array(
        'routes' => array(
            'users' => array(
                'type'    => 'ResourceGraphRoute',
                'options' => array(
                    'route'    => '/users',
                    'resource' => 'Application\Entity\User'
                )
            ),

This will automatically open the routes /users, /users/5 and all associations that have been exposed (so /users/5/tweets, or even /users/5/tweets/6/retweets , although not recommended as it leads to non optimal queries).

What I do not like is that it's not intuitive here, because of the "resource" => 'Application\Entity\User'. I would have prefer to be able to specify something like a Repository or something Selectable. But this is not possible because I didn't have found any way to get dependencies from Doctrine. Which leads to the following hack: https://github.com/bakura10/ZfrRest/blob/develop/src/ZfrRest/Mvc/Router/Http/ResourceGraphRoute.php#L76-L78

This in turns means that the REST module can only work with entities, not things more "abstract" like a meteo REST service, that kind of things...

1. I see clearly how does it work for GET/DELETE: the router recursively parse the path, and make the queries, and give the result to the controller. However this obviously does not work for POST, as the router will make query for something that do not exists. For instance, POST /users will lead to "post" method in controller with a collection of users, while what we want is a single user, already validated and hydrated. How can we tackle this problem ? Should we create two different route like ResourceFetcherRoute and ResourceCreatorRoute that does different logic ?

The route is the biggest part of the module. Once we have something really flexible here I suppose it'll be easier to write the other parts like automatic serialization according to Content-Type, and other cool things.

[Ocramius]

As discussed on IRC, the list is a resource on its own, and should have its own controller

[bakura10]

How can we differentiate that in mapping. For instance, here is the mapping for User:

/**
 * @ORM\Entity
 * @REST\Controller(name="Application\Controller\UserController")
 */
class User
{
}

What kind of syntax are you thinking about to handle that ?

[bakura10]

@Ocramius , I've found another problem when trying to remove the ObjectManager. The problem is here: https://github.com/bakura10/ZfrRest/blob/c022ebcfc5dc06fc4e9b1647b5f91a819d038d98/src/ZfrRest/Mvc/Router/Http/ResourceGraphRoute.php#L76

As you know, the mapping for REST is mostly defined at entity level. However, if you define your config this way:

return array(
    'router' => array(
        'routes' => array(
            'users' => array(
                'type'    => 'ResourceGraphRoute',
                'options' => array(
                    'route'    => '/users',
                    'resource' => 'Application\Repository\UserRepository' // <<====== THIS
                )
            ),
   )
);

How can the metadata library find that when we ask mapping 'Application\Repository\UserRepository' it will fetch the metadata of ... 'Application\Entity\User'.

I think that if we are going this way, it's gonna need TONS of config... This obviously won't work.

EDIt : ObjectManager defines a getClassName. Ok we can use that. But how for Collection ?

[bakura10]

Hi @Ocramius ,

I've done some changes based on your feedback. First, I've been able to completely remove the ObjectManager from the ResourceGraphRoute. Please focus on this: https://github.com/bakura10/ZfrRest/blob/bb5e24272b8307e72e04afea1819faf89864b31f/src/ZfrRest/Mvc/Router/Http/ResourceGraphRoute.php#L248-L259

The problem here is that it will work correctly with ObjectRepository instances or entity, but I cannot see how to do it with Collections.

If it looks okey to you, please could you review the route in-depth ? For me it looks nearly ok.

I've done more work on the controller part too: https://github.com/bakura10/ZfrRest/blob/develop/src/ZfrRest/Mvc/Controller/AbstractRestfulController.php

What I've done is that each method is handled by a method called "handle*Method". I've found this nice as it allows to easily define new methods or overriding the current behaviour, simply by extending the base controller. I've also removed the BodyParser class, as I now need a "PostParser" too. I've found it a bit overkill to create a class for each of them without any true benefits. So I just moved this logic to the controller.

[bakura10]

Can I merge this @Ocramius ? May be simpler for everyone to work with something that at least "works" a little. No version or whatever yet, just for working convenience.