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

Add simple renderer #116

Merged
merged 2 commits into from
Jan 20, 2014
Merged

Add simple renderer #116

merged 2 commits into from
Jan 20, 2014

Conversation

bakura10
Copy link
Member

Following our discussion, this PR includes:

  • It removes the CreateModelResourceListener: now you must explicitly return a ResourceModel from your controller if you want automatic serialization to happen.
  • Add a new ResourceRendererInterface.
  • Add a new SimpleResourceRenderer that does a very simple output, which is the default renderer strategy.
  • Updated the doc and added a recipe in the cookbook to explain how to add custom data through custom hydrator

The task is to create a HalRenderer. Then, any third party module could create its own renderer (for instance a EmberResourceRenderer) to comply with JavaScript clients.

What do you think?

@bakura10
Copy link
Member Author

ping @Ocramius @danizord

@coveralls
Copy link

Coverage Status

Coverage decreased (-0.39%) when pulling 0cfbdc8 on remove-listener into b4aecf8 on master.

@bakura10
Copy link
Member Author

I'm going to merge as I need this to do other tests. If somebody want to review afterward, feel free!

bakura10 added a commit that referenced this pull request Jan 20, 2014
@bakura10 bakura10 merged commit 88c5fb2 into master Jan 20, 2014
@bakura10 bakura10 deleted the remove-listener branch January 20, 2014 09:47
* {
* "id": 1,
* "title": "ZfrRest is awesome",
* "author": {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@bakura10 This probably will not work with Doctrine Hydrator since it does not extract recursively. Right?

@bakura10
Copy link
Member Author

bakura10 commented Feb 7, 2014

Yep. Recursive hydrator is a bit hard...

@bakura10
Copy link
Member Author

Since this PR, it is now a bit more ugly to deal with controllers, because you need to manually create a ResourceModel:

public function get(User $user, ResourceMetadataInterface $metadata)
    {
        return new ResourceModel(new Resource($user, $metadata));
    }

As you can see, the user needs to explicitly write the second parameter of each controller's action (ResourceMetadataInterface $metadata), and create the resource model (as well as creating the new resource).

Of course, the idea of forcing the user to return a ResourceModel to trigger the ResourceRenderer was pretty good, but it is now a bit annoying to write this boilerplate code.

What I'd like to preserve is being able to directly return an object:

public function get(User $user)
    {
        return $user;
    }

While still preserving the fact that if we return an array or a ViewModel/JsonModel, ZfrRest still honors this. My idea is writing the listener like this:

if ($result instanceof ModelInterface || is_array($result)) {
   return; // if explicitly returned a model or an array... do nothing
}

// If we explicitly return an object, we let the listener create a ResourceModel
if (is_object($result)) {
    $event->setResult(new ResourceModel(new Resource($result, $metadata));
}

What do you think? ping @danizord @Ocramius

@bakura10
Copy link
Member Author

OR we allow to have a new controller plugin:

public function get(User $user)
{
    return $this->createResourceModel($user);
}

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

Successfully merging this pull request may close these issues.

3 participants