Skip to content
This repository has been archived by the owner on Jan 8, 2020. It is now read-only.

Basz feature/accepted model controller plugin #2615

Closed
wants to merge 11 commits into from
Closed

Basz feature/accepted model controller plugin #2615

wants to merge 11 commits into from

Conversation

Freeaqingme
Copy link
Member

@ghost ghost assigned weierophinney Oct 2, 2012
@Freeaqingme
Copy link
Member Author

@weierophinney I incorporated @basz 's feedback. Up for review.

@Freeaqingme
Copy link
Member Author

Rebased!

@Freeaqingme
Copy link
Member Author

This plugin allows one to easily and safely define what View Model type should be used based on the accept header string.

<?php
public function foobarAction()
{
   $arr = array(
        'Zend\View\Model\JsonModel' => array(
            'application/json',
            'application/javascript'
        ),
        'Zend\View\Model\FeedModel' => array(
            'application/rss+xml',
            'application/atom+xml'
        ),
        'Zend\View\Model\ViewModel' => '*/*'
    );


    $this->request->getHeaders()->addHeader($header);
    $viewModel = $this->acceptantViewModelSelector($arr);
    //    $viewModel now is an instance of Zend\View\Model\FeedModel
}
?>

Generally this mapping array will be defined on a controller level, or even application wide level. But it does allow one to deviate from the defaults.

The need for this functionality arises because of multiple difficulties currently experienced with accept header handling and view model selection:

  • The default render strategy strategy kicks in only after the controller has dispatched. However, based on the accept header it may be required to perform different actions.
  • The current render strategy setup is not capable of handling accept headers like /.
  • Because render strategies are defined on an application-wide level, security issues arise, eg. JsonStrategy security fix #2410

* @package Zend_Mvc
* @subpackage Controller
*/
class AcceptantViewModelSelector extends AbstractPlugin
Copy link
Contributor

Choose a reason for hiding this comment

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

I think this should be AcceptedViewModelSelector

@weierophinney
Copy link
Member

My only concern at this point is whether or not the ViewManager should allow registering strategies at initialization or not.

Removing that functionality is technically a BC break, but, as you and others have noted, it could also be a potential security issue.

I'll ask the security team their opinion. Otherwise, this is a great set of functionality, and I look forward to merging it and using it.

* Set a Field Value Part this Field Value Part matched against.
*
* @param AbstractFieldValuePart $matchedPart
* return AbstractFieldValuePart provides fluent interface
Copy link
Member

Choose a reason for hiding this comment

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

missed @ in front of return

@juriansluiman
Copy link
Contributor

Would it be possible to make some use cases a bit easier to work with? In my particular case, I have quite often support for a Json model when application/json or application/javascript is requested, all other types are simply handled the same way. In case JSON is requested, it will always be for q=1 (so at top-priority).

I could imagine a simple way like this:

class MyController
{
  public function fooAction()
  {
    $service = $this->getService();
    $result  = $service->doSomeWork();

    if ($this->accept()->match(array('application/json', 'application/javascript'))) {
      return new JsonModel($result);
    }

    return $this->redirect()->toRoute('foo/bar');
  }
}

I can create my own plugin using Request::match() myself, but it seems something like this could also be implemented in this AcceptableViewModelSelector?

@Maks3w
Copy link
Member

Maks3w commented Nov 7, 2012

@basz @Freeaqingme Don't forget send a PR to zf2-documentation documenting the plugn!

@basz
Copy link
Contributor

basz commented Nov 7, 2012

I can start with this... but not within a few days...

@Freeaqingme
Copy link
Member Author

@juriansluiman We could indeed implement such functionality. However, it should be 100% configurable, and may need a little bit more thought put into it. Lets get this baby done first, then look what else we could do.

@Maks3w No worries, you're getting your docs! ;)

@weierophinney
Copy link
Member

@Freeaqingme As noted yesterday on IRC, we'll need to merge this against the master branch as well. The only way I see to do this is to merge a range of commits or cherry-pick. My question is: do you want to do this, or are you okay with somebody else doing it? Both will rewrite history, so the question is if you want that rewrite to happen in your own branch, or in mine.

weierophinney added a commit to weierophinney/zendframework that referenced this pull request Nov 7, 2012
- Added security note about change to JsonStrategy and FeedStrategy, and
  also detail AcceptableViewModelSelector usage
weierophinney added a commit to weierophinney/zendframework that referenced this pull request Nov 7, 2012
weierophinney added a commit to weierophinney/zendframework that referenced this pull request Nov 7, 2012
- Split testInvokeWithoutDefaults() into two separate tests, for the
  separate behaviors it tested
  (testInvokeWithoutDefaultsReturnsNullWhenNoMatchesOccur() and
  testInvokeReturnsFieldValuePartOnMatchWhenReferenceProvided())
- Renamed testInvoke__2() to
  testHonorsAcceptPrecedenceAndPriorityWhenInvoked()
weierophinney added a commit that referenced this pull request Nov 7, 2012
Forward port #2615 and #2410

Conflicts:
	library/Zend/Mvc/Controller/PluginManager.php
weierophinney added a commit to zendframework/zend-http that referenced this pull request May 15, 2015
Introduce AcceptableViewModelSelector, and remove Accept matching from
JsonStrategy and FeedStrategy

Close zendframework/zendframework#2615
Close zendframework/zendframework#2410
weierophinney added a commit to zendframework/zend-http that referenced this pull request May 15, 2015
Forward port zendframework/zendframework#2615 and zendframework/zendframework#2410

Conflicts:
	library/Zend/Mvc/Controller/PluginManager.php
weierophinney added a commit to zendframework/zend-view that referenced this pull request May 15, 2015
Introduce AcceptableViewModelSelector, and remove Accept matching from
JsonStrategy and FeedStrategy

Close zendframework/zendframework#2615
Close zendframework/zendframework#2410
weierophinney added a commit to zendframework/zend-view that referenced this pull request May 15, 2015
Forward port zendframework/zendframework#2615 and zendframework/zendframework#2410

Conflicts:
	library/Zend/Mvc/Controller/PluginManager.php
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants