CAction and parameter binding

Hi!

They say that submitting a patch is a lot better than submitting a bug report or a feature request. I believe that is true :)

Yesterday I posted a question about the shortcomings of the new and shiny parameter binding feature in Yii 1.1.4. In essence, I pointed out that parameter binding is not functioning properly when the action was defined as an extension of CAction.

Since the issue did not seem to arouse much interest, I decided to fix the thing myself.

In short, the small changes I made enable the use of parameter binding feature with CAction classes. In terms of code I consolidated the reflection logic from CInlineAction->run method into a single CController method and added 3 lines to the CController->runAction method.

I am wondering if any of the core developers is interested in helping me submit this patch into the development tree? I am prepared to improve the existing solution if it has any shortcomings.

The relevant code changes are as follows.

The refactored CInlineAction


/**

 * CInlineAction represents an action that is defined as a controller method.

 *

 * The method name is like 'actionXYZ' where 'XYZ' stands for the action name.

 *

 * @author Qiang Xue <qiang.xue@gmail.com>

 * @version $Id: CInlineAction.php 2414 2010-09-02 14:40:22Z qiang.xue $

 * @package system.web.actions

 * @since 1.0

 */

class CInlineAction extends CAction

{

	/**

	 * Runs the action.

	 * The action method defined in the controller is invoked.

	 * This method is required by {@link CAction}.

	 */

	public function run()

	{

		$controller=$this->getController();

		$methodName='action'.$this->getId();

		$controller->runActionWithParameterBindings($controller,$methodName);

	}

}

The CController method that consolidates parameter binding magic, almost unchanged.


	/**

	 * Runs a specified method of a CController or CAction with

	 * or without parameter binding, depending on whether that

	 * method has any arguments declared or not. If the method has

	 * arguments defined and any correresponding GET variable is missing,

	 * an exception gets thrown.

	 * @param $parent the object which method is to run

	 * @param $methodName the name of the method to be run

	 * @throws CHttpException if a binded variable is missing from GET

	 * @since

	 */

	public function runActionWithParameterBindings($parent,$methodName)

	{

		$method=new ReflectionMethod($parent,$methodName);

		if(($n=$method->getNumberOfParameters())>0)

		{

			$params=array();

			foreach($method->getParameters() as $i=>$param)

			{

				$name=$param->getName();

				if(isset($_GET[$name]))

					$params[]=$_GET[$name];

				else if($param->isDefaultValueAvailable())

					$params[]=$param->getDefaultValue();

				else

					throw new CHttpException(400,Yii::t('yii','Your request is invalid.'));

			}

			$method->invokeArgs($parent,$params);

		}

		else

			$parent->$methodName();

	}

The updated runAction method of CController, 3 lines added.


	/**

	 * Runs the action after passing through all filters.

	 * This method is invoked by {@link runActionWithFilters} after all possible filters have been executed

	 * and the action starts to run.

	 * @param CAction action to run

	 */

	public function runAction($action)

	{

		$priorAction=$this->_action;

		$this->_action=$action;

		if($this->beforeAction($action))

		{

			if ($action instanceof CInlineAction)

				$action->run();

			else

				$this->runActionWithParameterBindings($action, 'run');

			$this->afterAction($action);

		}

		$this->_action=$priorAction;

	}

I could have done it more elegantly but I wanted to keep the architectural design intact – all controller action methods are still called via the CInlineAction proxy.

All feedback is welcome.

The problem is that you cannot have run() with different parameter declarations in action classes, unless we remove run() from IAction.

Actually you can. If all new method parameters have default values declared, then it conforms to any interface specification.




interface IAction {

   public function run();

}


class CAction implements IAction {

   public function run($yii='1.1.4', $nothing=null) {

      echo $yii;

   }

}



It’s valid PHP because the implementation of an interface must not introduce unknown parameters and default values rule that possibility out.

A requirement to declare default values for binded parameters is not necessarily a bad thing. It would serve the same goal as explicitly initializing all variables did in the days of register_globals.

thanks Saul. You save me :)

Saul, that’s an interesting patch. It appears you haven’t submitted it to the codebase yet though?

To do so, create a new issue here: http://code.google.com/p/yii/issues/list

Describe the situation, and provide a patch/diff file, so that developers can easily merge your patch in, instead of having to search/copy/paste.

It would be great to see this solution (or something that achieves the same goal) implemented for CAction!

Cheers

As I’ve implemented this change into my own fork of Yii, I’ve submitted the changeset (with allowances for changes in Yii trunk that aren’t reflected in Saul’s code) as a patch.

Thanks for doing the hard work Saul :slight_smile:

http://code.google.com/p/yii/issues/detail?id=1996 << Anyone that wants this same feature, please star the issue so that it gets ranked more highly in the issue tracker

EDIT: to reiterate what Saul said above, parameter binding the run() method in a class extended from CAction will not work unless you default each parameter, i.e. - run($param1=-1, $param2=‘test’)

Otherwise, you’ll just get a fatal error.