Open bakura10 opened 9 years ago
You'll have to ping me later on for this, as I currently don't have time to get to it :-(
I will, I definitely need feedback on this :).
I will likely implement this much sooner than expected considering how ZfrRest restricts me currently, so if someone has some time reading this, especially the part related to rendering, it would be very helpful.
Ping @danizord @ocramius
Hi everyone,
I have given more thoughts to how ZfrRest should evolve. I have open a refactor branch, where I have kept what I'm happy with (errors handling), and throw out everything else (basically).
The controller part is now much more simpler and dumb than before. There is no longer automatic routing, transparent validation... but some helpers are here to make your life easier. The only thing I've made is automatically injecting into methods the matched route params (as those are nearly always needed for fetching resources using services). Here is a simple example using the new controller:
class UserController extends AbstractRestfulController
{
private $userService;
public function __construct(UserService $userService)
{
$this->userService = $userService;
}
public function get($params)
{
$user = $this->userService->getById($params['user_id']);
return new ResourceViewModel(['user' => $user]);
}
public function delete($params)
{
$user = $this->userServcie->getById($params['user_id']);
$this->userService->delete($user);
}
}
class UserListController extends AbstractRestfulController
{
private $userService;
public function __construct(UserService $userService)
{
$this->userService = $userService;
}
public function post()
{
$data = $this->validateIncomingData(UserInputFilter::class);
$user = $this->hydrateData(UserHydrator::class, new User());
$this->userService->create($user);
return new ResourceViewModel(['user' => $user]);
}
public function put($params)
{
$data = $this->validateIncomingData(UserInputFilter::class);
$user = $this->userService->getById($params['user_id']);
$user = $this->hydrateData(UserHydrator::class, $user);
$this->userService->create($user);
return new ResourceViewModel(['user' => $user]);
}
}
Plain and simple. Contrary to ZF2 Abstract Restful controller, you are not restricted to one-level of URI, as you receive all the route params. You can now performs optimized queries, using your own service layer, fetching related entities...
Rendering was the part I was not sure about. So I listen to what some people needed, what I actually always need when building an API, and came to a list of features I absolutely want:
The resource view model is a sub-class of ViewModel and works very similar. The first parameter is a list of variables. This is what allows to render different resources into the same payload.
The second parameter is a list of options, and currently accepts the following ones:
Some other may be introduced in the future, and while it may make some people unhappy, I want to favour "pragmatic" usage over a strict usage.
The resource renderer will map a ResourceViewModel to a PHP template file. Template files will be map according to their route name. For instance, considering the following routes:
'users' => ['/users', UserListController::class, 'children' => [
'user' => ['/:user_id', UserController::class, 'children' => [
'tweets' => ['/tweets', UserTweetListController::class, 'children' => [
'tweet' => ['/:tweet_id', UserTweetController::class]
]]
]],
'tweets' => ['/tweets', TweetListController::class]
If you want to render a tweet that belongs to a user, the renderer will look for the template called "default.users.user.tweet". The first part is actually the version (default to "default").
You may mix multiple versions. For instance, you may want to render the "user" differently (but only user). In this case, you could introduce a new template called "v2.users.user". Based on the extracted version, ZfrRest will select this one.
If it does not find a specific template for a given version, it will either choose the default one if strict_version is false (default), or throw an exception if strict_version is true.
The resource strategy will be enabled whenever it encounters a "ResourceViewModel" instance. Otherwise, it is not executed and still allows you to render other types like JsonModel.
Now, the interesting part: the template. I actually tried several syntaxes, until I find one that is flexible enough and fast to write. This has been made possible with the usage of new view helpers (or renderer methods, I don't know yet how implementation will work):
Let's take an example: I want to render a user, with their associated tweets, and for each tweets, their retweets, and also render the user billing info (that is not part of the user class but was loaded using a JOIN within an optimized repository, for instance). Here is how it looks like:
// Render the user itself
$this->renderResource(UserHydrator::class, $this->user);
// If there is a specific endpoint for getting billing info:
$this->renderTemplate('users.user.billing', $this->billing);
// Or we can use using a hydrator too:
$this->renderResource(UserBillingHydrator::class, $this->billing);
// Render a template using an include
if ($this->hasInclude('tweets')) {
$this->includeTemplate($this->getInclude('tweets'), 'tweets', ['tweets' => $user->getTweets()]);
}
As you can see, this approach and the use of recursive template has a nice side-effect: you no longer need to write complex hydrators that implement recursive extraction, because this is handled through the template (which is much more logical IMO).
The include template is like renderTemplate, but instead, it understand a "dot" notation. For instance, you could pass the include "tweets.retweets". When calling "hasInclude('tweets')" it will return true because the "tweets.retweets" include "tweets".
This will render the "tweets" template, using the new context, but the include will be "changed" to "retweets" only, which allow you to have this in your tweets
template:
// We could also use $this->renderCollection, but as we want to handle include, we need
// to render a separate template for more control
foreach ($this->tweets as $tweet) {
$this->renderTemplate('tweets.tweet', ['tweet' => $tweet])
}
And finally the tweets.tweet
template:
$this->renderResource(TweetHydrator::class, $this->tweet);
if ($this->hasInclude('retweets')) {
$this->renderTemplate('retweets.retweet', ['retweets' => $this->tweet->getRetweets())];
// If retweets had also a possibility of include, we could use an includeTemplate instead
$this->includeTemplate($this->getInclude('retweets'), 'retweets', ['retweets' => $this->tweet->getRetweets()])
}
Let me know what you think, if you have a better syntax idea....
ping @danizord @Orkin
Hi everyone,
I've had a talk with @Orkin, and he found the view layer confusing. Instead, he suggests skipping the hydrator for rendering (which kinda makes sense as the view is already a "rendering purpose"). This means that in ZfrRest context, it makes "extract" method useless.
He therefore suggests something completely explicit. Here is an example of a view:
$data = [];
foreach ($this->users as $user) {
$data[] = $this->renderTemplate('users.user', ['user' => $user]);
}
return $data;
Or a single user:
$data = [
'first_name' => $this->user->getFirstName(),
'last_name' => $this->user->getLastName()
];
// A related resource
if (isset($this->billing)) {
$data['billing'] = [
'card' => $this->billing->getCard()
]
}
// Tweets of the user
foreach ($this->user->getTweets() as $tweet) {
$data['tweets'][] = $this->renderTemplate('tweets.tweet', ['tweet' => $tweet]);
}
To me it is essential to keep a "renderTemplate" that allows to render another named template, to avoid code duplication. What do you think?
I think it's better doing like this ;)
Le 19 janv. 2015 à 19:06, Michaël Gallego notifications@github.com a écrit :
Hi everyone,
I've had a talk with @Orkin, and he found the view layer confusing. Instead, he suggests skipping the hydrator for rendering (which kinda makes sense as the view is already a "rendering purpose"). This means that in ZfrRest context, it makes "extract" method useless.
He therefore suggests something completely explicit. Here is an example of a view:
$data = [];
foreach ($this->users as $user) { $data[] = $this->renderTemplate('users.user', ['user' => $user]); }
return $data; Or a single user:
$data = [ 'first_name' => $this->user->getFirstName(), 'last_name' => $this->user->getLastName() ];
// A related resource if (isset($this->billing)) { $data['billing'] = [ 'card' => $this->billing->getCard() ] }
// Tweets of the user
foreach ($this->user->getTweets() as $tweet) { $data['tweets'][] = $this->renderTemplate('tweets.tweet', ['tweet' => $tweet]); }
To me it is essential to keep a "renderTemplate" that allows to render another named template, to avoid code duplication. What do you think? — Reply to this email directly or view it on GitHub.
@bakura10 Sorry, I'm very busy these days :sleepy:. But I'll have a deep look this weekend ;)
Thanks!
I really like your origin thoughts about ZfrRest as it reflects many problems I got while working with ZF2 on apis. Did not have the chance to tryout ZfrRests - but this is on my todo list now!
I'm def a fan of less magic - having simple route definition sounds much better then having magic inside that part. I would prefer using the same style, that zf3 is going to use (would wonder, if its not the solution DasPRID was working on? Is dash style there used, too?) - but :+1: for that!
ResourceViewModel sounds like a great way to change the format of output. I had that problem several times with client projects and ended up with some dirty hacks, to get the schema, that frontend/managers/documentation wants.
Post-processors for field filter: perfect.
This all reads like a whishlist for Doctrine/ZF2 based API development. Tried apigility, but Doctrine integration somehoew feels a little bit strange there. Really looking forward to test out ZfrRest.
Good and very transparent work in writing down your thoughts.
Thanks @Mischosch . Unfortunately, automatism can't work when you need efficient queries, flexible rendering... Anyway, the code is already ready in this branch #184
This post replaces the previous one and tries to formalise a bit more what I have in mind for next ZfrRest (that I will start developing once we have ZF3 components, especially refactored router). Those changes are based on my experience in using ZfrRest for a moderately complex API in production.
Error handling
ZfrRest uses its own set of exceptions based on HTTP error. A listener is used to automatically catch the error, and generate a coherent error message that includes status_code, error message and additional information (in case of validation).
It also has the ability to configure an exception_map that automatically maps a user-land exception to a ZfrRest exception.
In overall, I especially like this part of ZfrRest. It works as expected, and I have no intention to modify it.
Routing/controller
Current ZfrRest provides an automatic routing through the
ResourceGraphRoute
. This route reads the Doctrine mapping and ZfrRest mapping, and uses the Criteria API to recursively filter the resources.Advantages
Simplicity
For simple use cases (few resources with very simple associations), this works well and allows to quickly build an API with a few ZfrRest annotations.
RAD
Because resources are fetched transparently in the route, you receive a ready-to-consume entity in your controler. In case of POST or PUT, incoming data has already been validated, filtered, and hydrated. This means that your actions are very lightweight (basically, just a permission call and a service update).
Drawbacks
In real-life applications though, current ZfrRest architecture proved to be highly frustrating.
Force to introduce bi-directional associations
Because accessing resources is made through associations traversal, ZfrRest forces you to introduce associations that may not make sense or may complicate your domain model. For instance, I had in my application a "Project" entity. Each projet could have multiple "members". However, I originally did not want to include the bi-directional associations (and instead keep it one-sided, with the "Member" entity having an association to a Project).
However, for the "/projects/:id/members" to be accessible, I needed to introduce this non-necessary association.
Unecessary database calls
Assuming the following path "/users/4/tweets", the router would have first matched the users 4 (one database call), and then returned a PersistentCollection to tweets. The persistent collection does not generate API calls until it is traversed (allowing you to filter it without loading all the tweets). However, this still does 2 database calls, while it could have been done in 1.
The problem is more visible for URI that are even more nested: /users/4/tweets/5/retweets. While not necesarily the best practice, URIs could be nested like that, and this one would generate 3 database calls, while an optimized service could do that in 1.
The reason is that the route is a recursive process, and cannot know how to perform optimized DB calls as it uses the simple Criteria API.
Can lead to highly inefficient code
In my application, I have a "Project" entity, and each team member has their own set of settings for the project. I want my API to return something like that:
Depending on the user, the "settings" is filled with the specific data of the user. Indeed, it is not possible to introduce a "settings" association to the Project entity because it depends on the user.
In ZfrRest, this resource can be acceded through the "/users/4/projects" endpoint. The controller receives a Collection of projects, and you then need to iterate through them, and fetching the settings based on the logged user. If you have 10 projects, this would mean 10 DB calls, while this could be done in 1 JOIN.
Once again, the problem comes from the fact that ZfrRest automatically gives you resources, and prevents you to use an optimized service layer.
Proposal
I suggest completely removing the routing layer of ZfrRest, and instead replace it by an explicit routing. This document uses a Dash-like syntax.
I'd like if ZF3/next completely removes the AbstractRestfulController, and just comes with an AbstractActionController.
In overall, the ZfrRest controller/routing layer will be a better ZF2 AbstractRestfulController.
Routing
For simple routes, it works like any action controller, except that the action is ommited:
This route would be used to dispatch a collection URI (like POST /users, GET /users). The controller would automatically dispatch to the method based on the HTTP verb. If no matching method could be used, the controller would trigger a
MethodNotAllowedException
to return a 405 error.For OPTIONS method, a built-in method will be present in the base controller.
Details route
Now, if we want to access a single user:
If hitting the /users/3 URI, the request will be dispatched to the UserController instead, and would follow the same rules.
Nested routes
Nested routes would work as you would expect them to work:
Contrary to ZF2 AbstractRestfulController, ZfrRest AbstractRestfulController will allows you to pass different named parameters (instead of forcing you to name all identifiers "id").
Dash compact syntax (assuming that some changes are made on the default order - ping @Dasprid) allows to make this very usable and explicit. Contrary to ZfrRest, you can very easily map a different controller depending on wether you access a resource directly, or through a nested URI (here, you can see that tweets is mapped either to UserTweetListController or TweetListController).
Injection
For simplicity, ZfrRest will not try to automatically inject repository or service based on a resource name. As all controllers are fetched from service manager, you are responsible to inject the right service.
Controller verbs
In ZfrRest, you used to have code like this:
In ZfrRest NEXT, it will mostly behave like a normal action controller:
RPC like action
Sometimes, it is actually useful to be able to define actions, even in a REST context, as not everything map to REST properly. For instance, a "start import" cannot be expressed as a HTTP verb that easily.
In ZfrRest this was frustrating, because you were forced to create a different controller, with a different factory. This leaded to a lot of duplication code.
I suggest that ZfrRest controller actually supports both things by extending the default AbstractActionController. For instance:
Data validation/filtering
In ZfrRest, the validation and filtering was done automagically:
Once again, this works quite well for simple cases, but falls up completely for more complexe use cases. I'd suggest that we'd go back to explicitness again, with some shortcuts for simplicity.
Validation/filtering
Before refactoring ZfrRest, I'll probably refactor input filters. What I'd like to add is a better validation groups, and more importantly "named" validation group. Each named validation group will actually contain a list of fields to validate, plus additional context for dynamic based validation/filtering. ZfrRest controller would offer a shortcut to avoid manually fetching the input filter manager, creating it, setting it...
Internally, the
validateIncomingData
will be responsible to:Data hydration
Once again, new ZfrRest will go explicit. Similarily, controllers would provide a shortcut for hydrating data:
Once again, this is just a shortcut for:
Both
validateIncomingData
andhydrateForClass
would no nothing magic, those are just shortcut to reduce boilerplate code.Typical GET request
Typical DELETE request
Typical POST request
Typical PUT request
All of this is more verbose than current ZfrRest, but at least we keep control about what happen (I suspect that as we remove all the magic, performance will be drastically better too). This also solves cleanly different validation rules based on method.
Resource rendering
In current ZfrRest, the rendering is automatically done through the the bound hydrator, and a specific rendering strategy.
Advantages
Drawbacks
Proposal
I actually suggest a new concept called a ResourceModel. This is actually a hybrid thing between JsonModel and ViewModel, and will look like a .phtml template, instead that it will output your model. At its simplest, you will return a ResourceModel like you would return a ViewModel:
Similarily as a view model, it will actually look for a template (I suppose in the "views" folder, but we may decide something else). Its name will be infered from the route hierarchy and method.
Assuming the following route config:
It will look for a template called "get.php":
This file will be very simple, and you will need to return a simple PHP array (yeah, 100% control!). Here is an example of such a file (assuming a "tweet" context and "user" that is the logged user):
As you can see, templates do not contain any complex logic. It's basically controlling how the data is rendered. Because of the context, it allows you to include or refused some fields depending on the context.
Post-processors
Often, responses include a lot of common patterns. For instance, some API always wrap the response under a "data" key. When paginating, you always want to include a "total_count", "offset" and "limit" properties... and so-on.
Obviously, manually writing those would be cumbersome. You can therefore register post-processors (and create your own), inside the ZfrRest config:
Because order is important, you will be able to set "priority". Internally, I suspect that those post_processors will be implemented as listeners inside the Rendering Strategy.
At its simplest, the PostProcessor interface looks like this:
Each post-processor can be enabled/disabled directly in the view, and configured.
For instance, if we want to always wrap data around a "data" key, we could write our post-processor:
For instance, let's assume that we are using a "GET" and that we want to wrap the payload around a common "data" key:
Because this is always registered, it will automatically wrap the result.
Let's take another example with a Paginator post-processor, that we need to enable whenever we have a collection resource:
Boom, the post-processor will execute.
Versioning
ZfrRest will be able to support simple versioning by dynamically changing the template. There will be two ways to configure it: either using a special parameter in the ResourceModel, or using headers. For instance, using headers, you will configure a specific Accept-Content to a version:
Now, instead of fetching the default view, it will check according to a based folder called "v2" (or whatever you prefer). If I take my previous example, it will look for this file:
If ZfrRest does not find the "v2", it will fallback to the default template. Alternatively, you can force a specific version in the controller by using a second parameter:
For instance, let's take we have a "cards" resource that will output the credit card resource. The default template will be:
Now, let's say that we release a v2, that output the "type" as "brand" instead. We therefore create our new template inside the v2 folder:
While this work, there is a lot of duplicate. Therefore, the renderer will support two helpers methods:
renderDefaultVersion' and
renderVersion`. Previous example could be re-written as:If we release a v3 that depends on v2:
Conversion to JSON
At the very end of the process (once all the post-processor have been run), ZfrRest will convert the output to JSON, and return the response to the client.
End rendering
ZfrRest takes care of properly setting the response status code for GET, POST, PUT and DELETE methods. It is currently doing it in the MethodHandler.
However, I intend to do this in a listener, that will likely hook to the
finish
event. Based on the method, and the result, it will properly set the headers.Thoughts?
ping @ocramius @danizord @mac_nibblet @thinkscape @blanchonvincent