Yii Framework Forum: Model Binders - Yii Framework Forum

Jump to content

Page 1 of 1
  • You cannot start a new topic
  • You cannot reply to this topic

Model Binders Another idea, perhaps for Yii 2.0? Rate Topic: ***** 1 Votes

#1 User is offline   mindplay 

  • Advanced Member
  • PipPipPip
  • Yii
  • Group: Members
  • Posts: 397
  • Joined: 03-September 09
  • Location:New York

Posted 27 July 2010 - 11:27 AM

Model binders is a wonderful concept I discovered recently while working with ASP.NET MVC 2.

If you're not familiar with the concept, you can read about it here.

I wrote a PHP example, just to demonstrate that this technique is now possible with PHP5...


<?php

/**
 * A simple model object
 */
class User
{
  public $id;
  public $username;
  public $password;
}

/**
 * A simple controller
 */
class TestController
{
  public function actionTest($id, User $user=null)
  {
    $user->id = $id;
    var_dump($id, $user);
  }
}

/**
 * A very simple value provider
 */
class ValueProvider
{
  public function provide(&$value, $name)
  {
    if (!isset($_POST[$name]))
      return false;
    
    $value = $_POST[$name];
    return true;
  }
}

/**
 * A very basic object provider
 */
class ObjectProvider
{
  public function provide(&$object, $name, $class)
  {
    if (!isset($_POST[$name]))
      return false;
    
    $object = new $class;
    foreach ($_POST[$name] as $key=>$value)
      $object->$key = $value;
    
    return true;
  }
}

/**
 * A sampler dispatcher, integrating providers
 */
function dispatch($controllerId, $actionId)
{
  $vp = new ValueProvider;
  $op = new ObjectProvider;
  
  $controllerName = ucfirst($controllerId).'Controller';
  $methodName = 'action'.ucfirst($actionId);
  
  $params = array();
  
  $method = new ReflectionMethod($controllerName,$methodName);
  foreach ($method->getParameters() as $p)
  {
    $paramName = $p->getName();
    
    if ($pc = $p->getClass())
      $className = $pc->getName();
    else
      $className = null;
    
    if ($optional = $p->isOptional())
      $default = $p->getDefaultValue();
    else
      $default = null;
    
    if ($className)
      $op->provide($params[$paramName], $paramName, $className);
    else
      $vp->provide($params[$paramName], $paramName);
  }
  
  $controller = new $controllerName;
  call_user_func_array(array($controller,$methodName), $params);
}

// mock POST data:

$_POST['id'] = 123;
$_POST['user'] = array(
  'username' => 'admin',
  'password' => 'super_secret',
);

// dispatch the controller/action:

header('Content-type: text/plain');

dispatch('test','test');


Just a flat PHP script with no dependency on Yii or otherwise.

Note the simplicity of TestController::actionTest() - in this case, the $id and $user parameters have been hydrated by the sample dispatcher, using two example providers, one for values and one for objects.

This is just a proof of concept, but should be sufficient to demonstrate the elegance of this technique - even for complex objects, you would write your recipe for transforming posted form data into an object, once, in the form of a provider, and then never need to deal with that in your action methods again.

No more if (isset($_POST['user'])) or $_GET['id'] in your action methods - much cleaner.

And also makes it possible to write clean unit tests for your controllers...
1

#2 User is offline   mindplay 

  • Advanced Member
  • PipPipPip
  • Yii
  • Group: Members
  • Posts: 397
  • Joined: 03-September 09
  • Location:New York

Posted 27 July 2010 - 11:29 AM

Come to think of it, this wouldn't even have to wait for 2.0 - since existing action methods have no parameters, the provider feature would not interfere with existing parameter-less action methods... :-)
0

#3 User is offline   jonah 

  • Advanced Member
  • Yii
  • Group: Yii Dev Team
  • Posts: 733
  • Joined: 27-November 08
  • Location:California (US)

Posted 14 August 2010 - 11:03 PM

Interesting idea. I think CakePHP does something like this, but only for GET params..
http://php-thoughts.cubedwater.com - my bloggings about Yii
0

#4 User is offline   mindplay 

  • Advanced Member
  • PipPipPip
  • Yii
  • Group: Members
  • Posts: 397
  • Joined: 03-September 09
  • Location:New York

Posted 15 August 2010 - 08:01 AM

The usefulness of model binders combined with attributes is incredible - using attributes, you can override which model binders to use for a particular method's parameters...
0

#5 User is offline   DarkNSF 

  • Standard Member
  • PipPip
  • Yii
  • Group: Members
  • Posts: 287
  • Joined: 12-November 08
  • Location:Palm Bay, Florida

Posted 18 August 2010 - 12:14 PM

View Postmindplay, on 15 August 2010 - 08:01 AM, said:

The usefulness of model binders combined with attributes is incredible - using attributes, you can override which model binders to use for a particular method's parameters...


I like this idea as long as the translation from a POST array to model can be done in CModel to prevent ugly translation methods in every model.
0

#6 User is offline   mindplay 

  • Advanced Member
  • PipPipPip
  • Yii
  • Group: Members
  • Posts: 397
  • Joined: 03-September 09
  • Location:New York

Posted 30 August 2010 - 07:51 PM

Looks like something has been implemented now for 1.1.4, but it's not quite binding.

basics.controller.txt

Looks like a convenient feature, but it appears to be a shortcut to query parameters - with binding, you could have a method signature like actionEdit(Page $page) and your Page model would be automatically loaded and populated before the method is invoked. For example:

function actionEdit(Page $page, $delete=false)
{
  if ($delete)
    $page->delete();
  else if ($page->save())
    return $this->redirect(array('list'), false);
  else
    $this->render('edit', array('page'=>$page));
}


It doesn't get much simpler than that.

In frameworks that use binders, they are usually separate classes, but it could be implemented as simply as adding an interface, e.g. IBindable - for example:

class Page extends CActiveRecord implements IBindable
{
  public static function bindModel($name, $value)
  {
    $model = Page::model()->findByPk($value);
    if (isset($_POST[$name]))
      $model->attributes = $_POST[$name];
    return $model;
  }
}


Now finding and updating the model is neatly tucked away in a method, so that this code only needs to be written once.

Another advantage is for unit testing, where you can pass mock objects to your controllers for testing...
0

#7 User is offline   mindplay 

  • Advanced Member
  • PipPipPip
  • Yii
  • Group: Members
  • Posts: 397
  • Joined: 03-September 09
  • Location:New York

Posted 30 August 2010 - 07:55 PM

Oh, and most frameworks would offer a default/automatic binder - so a model only needs to implement IBindable if it does something special. For simple cases like the example above, the binder would be smart enough to simply try $_POST['Page'] if an object of type Page was required. Reflect on the primary key, and look for a $_GET or $_POST variable with the same name to find an existing model, and so on...
0

#8 User is online   qiang 

  • Yii Project Lead
  • Yii
  • Group: Yii Dev Team
  • Posts: 5,900
  • Joined: 04-October 08
  • Location:DC, USA

Posted 30 August 2010 - 09:41 PM

Your idea is definitely brilliant. It is in fact one of main inspirations of the newly checked-in enhancement.

We had an internal discussion about model binding. We chose not to implement it for several reasons:
  • It doesn't have an intuitive way of setting model scenario. Remember that model scenario determines which attributes can be massively assigned.
  • We are not sure where to put or configure the binder class. Your example of implementing IBindable in model classes is not quite good because it requires access to input data (e.g. $_POST). Strictly speaking, models should avoid directly accessing input data because they may be reused in different places who input could be quite different. For this reason, the most appropriate place is controller.
  • With model binder, you still need to detect whether the model data is submitted or not. If not, you will display the initial edit page. Otherwise, you will attempt saving. This is not reflected in your sample code in actionEdit.
  • Having model class type hinting, it means PHP needs to load the model class file when parsing the controller class, even if the model class is not used at all. This is probably not a big issue, though.
  • Using model binder also adds learning curve and makes code less easy to read since the relevant code now scatters in two places (controller action and some *distant* class method).

1

#9 User is offline   Mike 

  • Elite Member
  • PipPipPipPipPip
  • Yii
  • Group: Members
  • Posts: 3,016
  • Joined: 06-October 08
  • Location:Upper Palatinate

Posted 31 August 2010 - 03:15 AM

This new feature relies on Reflection. I remember that long ago i read, that Reflection is bad (at least in productive code). The reasons i remember:

- It's slow

but maybe more important:

- We add a "meta" level. Leaving pure PHP and creating artificial features by observing the code + remarks themself.

If we open that door, we could take it to the extreme: Where does this code inspection end? We could end up with our own completely artificial language where half of the code is executed on this "meta level" and e.g. contained in comments.

In my opinion this adds much obscurity and feels like "raping" PHP. Not to forget the potential performance issues. This new feature now uses Reflection on every request. I can think about a lot of requests that don't require that feature and instead should be as fast as possible.

So is this really the way the framework should go? And shouldn't that feature be optional?
0

#10 User is offline   Y!! 

  • Advanced Member
  • Yii
  • Group: Yii Dev Team
  • Posts: 978
  • Joined: 18-June 09

Posted 31 August 2010 - 06:04 AM

View PostMike, on 31 August 2010 - 03:15 AM, said:

This new feature relies on Reflection. I remember that long ago i read, that Reflection is bad (at least in productive code). The reasons i remember:

- It's slow

but maybe more important:

- We add a "meta" level. Leaving pure PHP and creating artificial features by observing the code + remarks themself.

If we open that door, we could take it to the extreme: Where does this code inspection end? We could end up with our own completely artificial language where half of the code is executed on this "meta level" and e.g. contained in comments.

In my opinion this adds much obscurity and feels like "raping" PHP. Not to forget the potential performance issues. This new feature now uses Reflection on every request. I can think about a lot of requests that don't require that feature and instead should be as fast as possible.

So is this really the way the framework should go? And shouldn't that feature be optional?


Mike I agree with you on every point, especially about the "meta level". Still I think this new feature is very good and I'm happy with it. It's optional, yes you have the Reflection, but it shouldn't be a performance problem since the logic and everything is very simple. The other thing is, there is basically no other way to do it. I think the non-reflection way would be to implement it directly in a router (CUrlManager), but that would only work with the current path url format and it would cause much more trouble in the end I guess.

Personally, I think we can go into that direction IF:

  • Everything is as strict as possible (see this patch that resulted in internal discussion)
  • If there is no other reasonable way using pure PHP (eg we could implement ASP.net style action-filter declerations, but what for? Filtering works good with current implementation)
  • If it helps to write less recurring code
  • If the performance impact is reasonable small


Also, I think it's an option adding a special property to CApplication in order to globally disable any Reflection-feature. So one is able to choose wether to use any additional magic that may decrease performance or not. What you think?
0

#11 User is online   qiang 

  • Yii Project Lead
  • Yii
  • Group: Yii Dev Team
  • Posts: 5,900
  • Joined: 04-October 08
  • Location:DC, USA

Posted 31 August 2010 - 06:31 AM

Reflection is fine as long as we don't overuse it. For this new feature, I did some performance test. The result doesn't show any obvious performance impact using reflection.

We already used reflection to determine the class file location. A global setting to turn it off means this part of code will stop work (what's the alternative then?)
0

#12 User is offline   Y!! 

  • Advanced Member
  • Yii
  • Group: Yii Dev Team
  • Posts: 978
  • Joined: 18-June 09

Posted 31 August 2010 - 06:40 AM

View Postqiang, on 31 August 2010 - 06:31 AM, said:

We already used reflection to determine the class file location. A global setting to turn it off means this part of code will stop work (what's the alternative then?)


Well what I mean is, in case of the new feature, simply use the old implementation if globally disabled, so that ReflectionMethod() doesn't even get called. If we add more of such reflection features in future (I mean doc-hacks etc), it's easy to fully disable it.
0

#13 User is offline   mindplay 

  • Advanced Member
  • PipPipPip
  • Yii
  • Group: Members
  • Posts: 397
  • Joined: 03-September 09
  • Location:New York

Posted 31 August 2010 - 07:50 AM

I only recently discovered how powerful reflection really can be. I've never seen it put to use in PHP like they did in ASP.NET MVC 2.0, but it works amazingly well, mainly thanks to source code annotations - but a lot of meta-data is extracted from method prototypes and model classes as well.

Once you understand how the meta-data is consumed by various framework components and helpers, it makes a lot of sense. The same is true for Yii, where your User model attributes by default get collected into an array named "User" in your form inputs - get_class() makes this possible, and that happens to be the simplest form of reflection.

So you've already started down this road.

And I'm not sure most people can imagine how far that road goes - if you haven't been there, you just can't know. Personally, I had no idea. But once you've worked with this, it makes so much sense it's eerie. Why weren't we doing this all along? That's been my reaction.

Of course, people are different. But personally, I like everything as DRY as possible. Ideally, I don't want to see the word "username" in my User model more than once - unless I'm looking at a method that actually accesses the username. Source code annotations and reflection makes this possible.

In my opinion it's no more magical, nor any harder to understand, than a whole bunch of required callback methods you're forced to implement, in order to provide the exact same metadata. The main difference is, source code annotations are more maintainable, because they require less code, they're less repetitive, and they're easier to find in the source code.

I'm not trying to sell you on the idea, I'm just sharing what I experienced.

I will continue work on my source code annotations library, and we'll see what you think of it when it's done :-)
0

#14 User is online   qiang 

  • Yii Project Lead
  • Yii
  • Group: Yii Dev Team
  • Posts: 5,900
  • Joined: 04-October 08
  • Location:DC, USA

Posted 31 August 2010 - 08:01 AM

I'm not underestimating the power of annotation. We have been using annotation to generate WSDL for soap service (http://www.yiiframew...pics.webservice) since many years ago (back to prado era).

While annotation is certainly powerful, PHP doesn't have native support for it. The current way of implementing annotation is more like a hack. We are not quite sure its side effects (e.g. what if comments are stripped off by some optimizers?) Also, it doesn't allow dynamic effects (e.g. how to define a validation rule on-the-fly?) Last but not least, comment-based annotation needs a set of newly invented grammars. They add additional learning curve, and are hard to be supported by IDE (unless the framework behind it becomes a de facto standard).
0

#15 User is offline   knut 

  • Junior Member
  • Pip
  • Yii
  • Group: Members
  • Posts: 45
  • Joined: 10-October 08
  • Location:Notodden, NORWAY

Posted 31 August 2010 - 04:12 PM

Guilherme Blanco and Pierrick Charron recently published an RFC + patch for annotations in PHP trunk: http://wiki.php.net/rfc/annotations

Maybe it will make it to PHP some day, but hard to tell from the internals list yet - not many comments.

I like the fact that Yii holds back on using the latest PHP features. I see Symfony 2 will require PHP >= 5.3.2 last time I checked.
0

#16 User is offline   Mike 

  • Elite Member
  • PipPipPipPipPip
  • Yii
  • Group: Members
  • Posts: 3,016
  • Joined: 06-October 08
  • Location:Upper Palatinate

Posted 01 September 2010 - 03:33 AM

@mindplay:
I see your point but Qiang raised very important concerns. I consider the potential problem with opcode caching the most important one. By its scripted nature it's essential for PHP (and any PHP framework) to play well with any opcode cache.

@Qiang:
With your above comment i'm surprised to see that the new implementation in CInlineAction already reflects the annotations. From framework/web/CInlineAction.php:

    public function run()
    {
        $controller=$this->getController();
        $methodName='action'.$this->getId();
        $method=new ReflectionMethod($controller,$methodName);
        if(($n=$method->getNumberOfParameters())>0)
        {
            if(preg_match_all('/^\s*\**\s*@param\s+(\w+)\s+\$(\w+)/m',$method->getDocComment(),$matches))


So comment reflection has now wandered into some of the most crucial parts of the core.

All this is done to save 3 lines of code per GET parameter:

if(isset($_GET['category']))
    $category=(int)$_GET['category'];
else
    throw new CHttpException(404,'invalid request');


I wonder if a simple helper method in CController to fetch GET and POST parameters wouldn't have been sufficient:

// In CController:
// Fetch $_GET parameters
//
//    $require: wether this param is required
//    $type: Perform type check if set
//
// Throws exception if a required param not found or wrong type
//
public function get($name,$required=false,$type=null) { /* TBD */ };

// Same implementation for $_POST:
public function post($name,$required=false,$type=null) { /* TBD */ };

// Usage:
// Make sure $_GET['category'] is supplied and an integer.
$category = $this->get('category',true,'integer'); 

// Check for optional $_POST['delete']
if ($this->post('delete') !== null)
   $this->deleteEntry();


No obscurity, no reflection, no magic. And much the same functionality. It looks, like it would require 1 more line per parameter in code than comment reflection. But that's not true: comment reflection also requires 1 line per parameter, even if it's in the doc comments.

So what do you think?
0

#17 User is offline   samdark 

  • Having fun
  • Yii
  • Group: Yii Dev Team
  • Posts: 3,778
  • Joined: 17-January 09
  • Location:Russia

Posted 01 September 2010 - 04:26 AM

Quote

I consider the potential problem with opcode caching the most important one.

It's not really a problem if one reads docs and uses --with-eaccelerator-doc-comment-inclusion to compile eaccelerator.
Yii 1.1 Application Development Cookbook

Enjoying Yii? Star us at github: 1.1 and 2.0.
0

#18 User is offline   Mike 

  • Elite Member
  • PipPipPipPipPip
  • Yii
  • Group: Members
  • Posts: 3,016
  • Joined: 06-October 08
  • Location:Upper Palatinate

Posted 01 September 2010 - 04:34 AM

I use APC :D.

Regardless: It's simply not guaranteed that all accelerators will keep comment blocks forever. By definition they are comments, not code. So an accelerator can strip them off if some optimization algorithm requires to do so to get max performance.

EDIT:
Cons moved to the vote thread.

This post has been edited by Mike: 01 September 2010 - 04:36 AM

0

#19 User is offline   mindplay 

  • Advanced Member
  • PipPipPip
  • Yii
  • Group: Members
  • Posts: 397
  • Joined: 03-September 09
  • Location:New York

Posted 01 September 2010 - 09:37 AM

View PostMike, on 01 September 2010 - 04:34 AM, said:

Regardless: It's simply not guaranteed that all accelerators will keep comment blocks forever. By definition they are comments, not code. So an accelerator can strip them off if some optimization algorithm requires to do so to get max performance.


My implementation uses the PHP tokenizer to parse the source code and extract annotations - the performance overhead is addressed by caching the parsed annotation data, avoiding the overhead of loading the parser again, once a file has been parsed once, on subsequent runs.

Bytecode caching should impose no limitations on this approach as far as I can figure?

I just wrote a very long e-mail to the two contributors working on the PHP syntax extension for annotations - this would of course be far more valuable than a custom implementation, especially in terms of IDE adaptation. Although it's a great start, I hope that the current implementation does not make it to release - it is lacking in a number of areas, and I'm afraid it would do more harm than good...

Update: I received a reply from the developer of the PHP extension, basically rejecting every feature and criticism I submitted. I'm afraid, if this library gets released, it will basically be as it is now. If you'd like to see the comments I posted, you can see them here.
1

#20 User is offline   mindplay 

  • Advanced Member
  • PipPipPip
  • Yii
  • Group: Members
  • Posts: 397
  • Joined: 03-September 09
  • Location:New York

Posted 07 September 2010 - 08:03 AM

Please see here for an update about my annotation library.
0

Share this topic:


Page 1 of 1
  • You cannot start a new topic
  • You cannot reply to this topic

1 User(s) are reading this topic
0 members, 1 guests, 0 anonymous users