[Thomas Jensen]

Hi,

Imagine this case:
We have a blog with a comment function.
An operation called "editComment" and a task called "editOwnComment". The task has a biz rule like: return (Yii::app()->user->id == $params["comment"]->userId);.
When displaying a post with several comments, foreach comment we check if the user has the right to edit this comment. If he has, we show a little pencil icon:

foreach ($comments as $comment) {
	if (Yii::app()->user->checkAccess('editComment',array('comment' => $comment))) {
		//display pencil and link to edit
	}
}

We'll then expect that all comments, which the user is associated with, will have the pencil icon. But thats not the case.
Because of Yii's caching function, only the first comment will either have a pencil or not, the rest will follow that example.

Is this an issue? It really seems stupid not to re-evaluate the bizrule, when the parameters have changed.

It could be achieved by doing something like this:

public function checkAccess($operation,$params=array(),$allowCaching=true)
{
	if($allowCaching && isset($this->_access[$operation]) && $this->_access[$operation]['params'] == $params)
		return $this->_access[$operation]['value'];
	else {
		$this->_access[$operation]['params'] == $params;
		return $this->_access[$operation]['value']=Yii::app()->getAuthManager()->checkAccess($operation,$this->getId(),$params);
	}
}

But that wouldn't be good enough as you should still be able to cache values for each different set of parameters.

[Sander]

I had the same problem a while back and solved it with a custom subclass of CWebUser, like this:

class CoreWebUser extends CWebUser {

	private $accessCache = array();
	
	/**
	 * Overrides {@link CWebUser::checkAccess()} to allow caching on a parameter
	 * level. The original method only checks for <var>$operation</var>, while
	 * ignoring the parameters that are passed on to the associated bizrules.
	 * We need more dynamic testing, though keeping a cache would still be VERY 
	 * useful, so this is my attempt of unifying these functionalities.
	 */
	public function checkAccess($operation, $params=array(), $fromCache=true) {
		$param_id = md5(serialize($params));
		if(!$fromCache || !isset($this->accessCache[$operation][$param_id])) {
			$this->accessCache[$operation][$param_id] = parent::checkAccess($operation, $params, false);
		}
		return $this->accessCache[$operation][$param_id];
	}

}

You do need to make sure to supply the arguments to $params in a consistent order, because that will probably influence the generated md5 string:

md5(array('a','b')) !== md5(array('b','a'))

I haven't explicitly tested this, though, maybe php is smart enough to serialize them the same way.

[Thomas Jensen]

I had the same idea, but wasn't sure how to make a md5 string of more than one parameter, but of course, serialize is the answer.
I hope this or similar will be implemented in the framework.

Thanks a lot.

[Sander]

NP. The md5 by the way isn't strictly necessary, it just felt wrong to me to have array indices with potentially VERY long strings of text (for example if an array of CActiveRecords is supplied in $params). Just a thought.

[Thomas Jensen]

One of my thoughts too
But, isn't it a performance problem? To make an md5 hash of such an amount of data?

[Sander]

Could be, but the char-for-char string comparison of large pieces of text could also very well be just as expensive. I don't know which approach requires more resources, and I don't think it matters enough to actually benchmark it. But you're welcome to do that if you want to