uniqueValidator on update

Hi there,

me again…a have a Problem with uniqueValidator. (may be i overlook

somethink, last night was very short)  I have a table with

one column set to be unique. It works wonderfull by inserting new

Records, duplicated entries will be catched.

But if i try to update an existing entry (with no change of the unique column,

and no other entry with same content in table) the validator fires saying there

are an existing element. Means it find himself as duplicate…

my rules from model look like this:



	array('Title','length','max'=>150),


	array('Title', 'required'),


	array('Title', 'unique'),





I've looked in the implemantion of CUniqueValidator and noticed that the

validator don't check if the found element has the same pk like

the object to validate.

Greetings from Germany

me23

Yes, that's a known problem. I can't think of a good way to solve it because CUniqueValidator doesn't know the original PK value. FYI, you can set 'on' option so that this rule only applies when inserting a record.

Hi qiang,

thanks for the quick answer.

Please correct me if i oversee something but the following works for me (but i have only test it with an single PK, not with multi-column pk);



protected function validateAttribute($object,$attribute)


	{


		$params = array();


		$pkCondition = "";


		$value=$object->$attribute;


		$params[':value']=$value;


		if($this->allowEmpty && ($value===null || $value===''))


			return;


		$columnName=$object->getTableSchema()->getColumn($attribute)->rawName;


		$pk = $object->getTableSchema()->primaryKey;


		if(null!==$pk){


			if(is_array($pk)){


				foreach($pk as $k){


					if(""!=$pkCondition){


						$pkCondition .= " AND ";


					}


					$pkCondition .= "$k!=:$k";


					$params[':'.$k]=$object->$k;


				}


			}


			else{


				$pkCondition = "$pk!=:$pk";


				$params[':'.$pk]=$object->$pk;


			}


			if(''!=$pkCondition){


				$pkCondition = ' AND (' . $pkCondition . ")";


			}


		}


		if($this->caseSensitive)


			$exists=$object->exists("$columnName=:value" . $pkCondition,$params);


		else


			$exists=$object->exists("LOWER($columnName)=LOWER(:value)" . $pkCondition,$params);


		if($exists)


		{


			$message=$this->message!==null?$this->message:Yii::t('yii','{attribute} "{value}" has already been taken.');


			$this->addError($object,$attribute,$message,array('{value}'=>$value));


		}


	}


Greetings from Germany

me23

Could you please explain your idea? I can't see much difference in your code.

Hi qiang,

the idea is to search only in records which not have the id of the object we want to check.

Here a short example to explain it better.

Assume a Table with 3 Colums

Id => as PK

Title => string and should be unique.

Content => string

Now lets say we want to change the content-column of record to 'other content' with starting values like this:

Id=5

Title='title1'

content='some content'

The validator runs on update and tries to find records with the condition like "Title=title1".

This will be true because the validator will find our record with id 5.

The update will fail here.

My idea is to extend the condition to somethink like this "Title ='title1' AND Id!=5".

So the validator will not found the same record which we want to check.

I improved my last code to check also if the object to check is new, because the validator will then fail on inserts. because of the condition like "Title='title1' and Id!=Null"…

So i think the code should look like this:



protected function validateAttribute($object,$attribute)


	{


		$params = array();


		$pkCondition = "";


		$value=$object->$attribute;


		$params[':value']=$value;


		if($this->allowEmpty && ($value===null || $value===''))


			return;


		$columnName=$object->getTableSchema()->getColumn($attribute)->rawName;


		if(!$object->isNewRecord){


			$pk = $object->getTableSchema()->primaryKey;


			if(null!==$pk){


				if(is_array($pk)){


					foreach($pk as $k){


						if(""!=$pkCondition){


							$pkCondition .= " AND ";


						}


						$pkCondition .= "$k!=:$k";


						$params[':'.$k]=$object->$k;


					}


				}


				else{


					$pkCondition = "$pk!=:$pk";


					$params[':'.$pk]=$object->$pk;


				}


				if(''!=$pkCondition){


					$pkCondition = ' AND (' . $pkCondition . ")";


				}


			}


		}


		if($this->caseSensitive)


			$exists=$object->exists("$columnName=:value" . $pkCondition,$params);


		else


			$exists=$object->exists("LOWER($columnName)=LOWER(:value)" . $pkCondition,$params);


		if($exists)


		{


			$message=$this->message!==null?$this->message:Yii::t('yii','{attribute} "{value}" has already been taken.');


			$this->addError($object,$attribute,$message,array('{value}'=>$value));


		}


Greetings

me23

What about multiple uniqueValidators? Only dirty attributes should be validated on update. I guess one possible but less than optimal solution is to add a dirty flag to every attribute. Or probably a better way: pass the pk (hidden) to the view and back again for comparison with the original record. (Assuming the view displays multiple records).

/Tommy

Edit: Thinking error. I guess we already pass the pk with an update request.  ;D

I see. I was thinking about PK change and couldn't find out a good solution.

Dirty flag would mean performance degradation. I think in case PK uniqueness is needed, users can always write a custom validation method to accomplish the task (with the help of keeping previous PK value).

we can also check if the columnname to be checked equals one of the pk-columns and bypass my condition-adding in this case…

me23

me23, could you please create a ticket about this?

sorry, qiang i don't have a gmail-account…

No problem. I just checked in a fix based on your idea.

Thanks qiang,

but it don't work like expected, it fires again on update.

I think Line 65

	$exists=$objects[0]->getPrimaryKey()===$object->getPrimaryKey();

should be:

	$exists=$objects[0]->getPrimaryKey()!==$object->getPrimaryKey();

and i got a new idea to validate uniqueness on multiple columns:

i think it could be used as follows:



return array(


			...


			array('Col1+Col2', 'unique'),


		);


and the modified validator could look like this:



protected function validateAttribute($object,$attribute)


	{


		$attributes = null;


		$criteria=array('condition'=>'');


		if(false !== strpos($attribute, "+")){


			$attributes = explode("+", $attribute);


		}


		else{


			$attributes = array($attribute);


		}


		foreach($attributes as $attribute){


			$value=$object->$attribute;


			if($this->allowEmpty && ($value===null || $value===''))


				return;


	


			$column=$object->getTableSchema()->getColumn($attribute);


			if($column===null)


				throw new CException(Yii::t('yii','{class} does not have attribute "{attribute}".',


					array('{class}'=>get_class($object), '{attribute}'=>$attribute)));


	


			$columnName=$column->rawName;


			if(''!=$criteria['condition']){


				$criteria['condition'].= " AND ";


			}


			$criteria['condition'].=$this->caseSensitive ? "$columnName=:$attribute" : "LOWER($columnName)=LOWER(:$attribute)";


			$criteria['params'][':'.$attribute]=$value;


		}


		if($column->isPrimaryKey)


			$exists=$object->exists($criteria);


		else


		{


			// need to exclude the current record based on PK


			$criteria['limit']=2;


			$objects=$object->findAll($criteria);


			$n=count($objects);


			if($n===1){


				if(''==$object->getPrimaryKey()){


					$exists = true;


				}


				else{


					$exists=$objects[0]->getPrimaryKey()!==$object->getPrimaryKey();


				}


			}


			else


				$exists=$n>1;


		}





		if($exists)


		{


			$message=$this->message!==null?$this->message:Yii::t('yii','{attribute} "{value}" has already been taken.');


			$this->addError($object,$attribute,$message,array('{value}'=>$value));


		}


	}


i've made a little test and it seems to work…

what do you think?

Greetings from Germany

me23

I've fixed the bug you found. Thanks!

Regarding the change about multiple columns, although your approach works, it has side effects (conflict with the newly implemented scenario-based massive assignment) and also requires new syntax.

I think uniqueness based on multiple columns is not widely needed. So maybe it should leave to developers to implement it in either a method-based validator or an individual extension (make the columns to be validated as a property).

ok and thanks again!

Validating multiple columns for uniqueness might be useful when a user can create objects with a name and the name has to be unique per user. Therefore the uniqueness of the user-objectName has to be checked.

I took me23’s code and packaged it a Validation Extention. one question though…

While CValidator::addError() takes a single attribute, is there a way to add s single error message that references multiple model attributes?

/Kaf

Update: The Validator extension was uploaded to the Extensions section at http://www.yiiframework.com/extension/unique-multiple-columns-validate/

I use compound keys so validating uniqueness across multiple fields is very important. I think if there was even a way to late bind the values passed to ‘params’ in the ‘criteria’ array that would be sufficient. Granted writing a custom validation method will work, but it is a little painful considering this built in validator almost does the job. I’m glad I found this thread so that I now know what I need to do to get my code to work.

There’s another thread with someone having the same problem I’m having and he also proposed a modification to CUniqueValidator: http://www.yiiframework.com/forum/index.php?/topic/7524-extension-for-cuniquevalidator/

What if the ‘attributeName’ option could accept a string column name or an array of string column names? The validation could still be triggered off of one column, but ‘attributeName’ would allow more than one column to be checked for uniqueness.