Merge pull request #11320 from bmoore/2.1.x

Minor rewrite to assign and __set issue #11286

CHANGELOG.md

@@ -42,7 +42,8 @@
 - `Phalcon\Cli\Console` and `Phalcon\Mvc\Application` now inherit `Phalcon\Application`.
 - Fixed `afterFetch` event not being sent to behaviors
 - Fixed issue with radio not being checked when default value is 0 [#11358] (https://github.com/phalcon/cphalcon/issues/11358)
-- Fixed issue with `Model::__set` that was setting hidden attributes directly instead of calling setters [#11286](https://github.com/phalcon/cphalcon/issues/11286)
+- Fixed issue with `Model::__set` that was bypassing setters [#11286](https://github.com/phalcon/cphalcon/issues/11286)
+- Fixed issue with `Model::__set` that was setting hidden attributes directly when setters are not declared [#11286](https://github.com/phalcon/cphalcon/issues/11286)
 - Added `Phalcon\Cli\DispatcherInterface`, `Phalcon\Cli\TaskInterface`, `Phalcon\Cli\RouterInterface` and `Phalcon\Cli\Router\RouteInterface`.
 - Added methods update(), create() and createIfNotExist(array criteria) to `Phalcon\Mvc\Collection`
 

phalcon/mvc/model.zep

@@ -490,10 +490,7 @@ abstract class Model implements EntityInterface, ModelInterface, ResultInterface
 				}
 
 				// Try to find a possible getter
-				let possibleSetter = "set" . camelize(attributeField);
-				if method_exists(this, possibleSetter) {
-					this->{possibleSetter}(value);
-				} else {
+				if !this->_possibleSetter(attributeField, value) {
 					let this->{attributeField} = value;
 				}
 			}
@@ -4106,7 +4103,7 @@ abstract class Model implements EntityInterface, ModelInterface, ResultInterface
 	public function __set(string property, value)
 	{
 		var lowerProperty, related, modelName, manager, lowerKey,
-			relation, referencedModel, key, item, dirtyState, method;
+			relation, referencedModel, key, item, dirtyState;
 
 		/**
 		 * Values are probably relationships if they are objects
@@ -4159,23 +4156,65 @@ abstract class Model implements EntityInterface, ModelInterface, ResultInterface
 			return value;
 		}
 
-		/**
-		 * Check if the property has setters
-		 */
-		let method = "set" . camelize(property);
+		// Use possible setter.
+		if this->_possibleSetter(property, value) {
+			return value;
+		}
 
-		if method_exists(this, method) {
-			return this->{method}(value);
+		// Throw an exception if there is an attempt to set a non-public property.
+		if !this->_isVisible(property) {
+			throw new Exception("Property '" . property . "' does not have a setter.");
 		}
 
-		/**
-		 * Fallback assigning the value to the instance
-		 */
 		let this->{property} = value;
 
 		return value;
 	}
 
+	/**
+	 * Check for, and attempt to use, possible setter.
+	 *
+	 * @param string property
+	 * @param mixed value
+	 * @return string
+	 */
+	protected final function _possibleSetter(string property, value)
+	{
+		var possibleSetter;
+
+		let possibleSetter = "set" . camelize(property);
+		if method_exists(this, possibleSetter) {
+			this->{possibleSetter}(value);
+			return true;
+		}
+		return false;
+	}
+
+	/**
+	 * Check whether a property is declared private or protected.
+	 * This is a stop-gap because we do not want to have to declare all properties.
+	 *
+	 * @param string property
+	 * @return boolean
+	 */
+	protected final function _isVisible(property)
+	{
+		var reflectionClass, reflectionProp, e;
+
+		//Try reflection on the property.
+		let reflectionClass = new \ReflectionClass(this);
+		try {
+			let reflectionProp = reflectionClass->getProperty(property);
+			if !reflectionProp->isPublic() {
+				return false;
+			}
+		} catch \Exception, e {
+			// The property doesn't exist.
+			return true;
+		}
+		return true;
+	}
+
 	/**
 	 * Magic method to get related records using the relation alias as a property
 	 *
@@ -4383,18 +4422,18 @@ abstract class Model implements EntityInterface, ModelInterface, ResultInterface
 		return data;
 	}
 
-    /**
-    * Serializes the object for json_encode
-    *
+	/**
+	* Serializes the object for json_encode
+	*
 	*<code>
 	* echo json_encode($robot);
 	*</code>
-    *
-    * @return array
-    */
+	*
+	* @return array
+	*/
 	public function jsonSerialize() -> array
 	{
-	    return this->toArray();
+		return this->toArray();
 	}
 
 	/**

unit-tests/ModelsGetterSetterTest.php

@@ -107,8 +107,21 @@ public function _executeSetGet()
 		$robot->assign(array('text' => $testText));
 		$this->assertEquals($robot->text, $testText . $robot::setterEpilogue);
 		$this->assertEquals($robot->text, $robot->getText());
+
 		$testText = 'executeSetGet Test 2';
 		$robot->text = $testText;
 		$this->assertEquals($robot->text, $testText . $robot::setterEpilogue);
+		$this->assertEquals($robot->text, $robot->getText());
+
+		$testText = 'executeSetGet Test 3';
+		$robot = new Boutique\Robots();
+		try {
+			$exception_thrown = false;
+			$robot->serial = '1234';
+		} catch (Exception $e) {
+			$exception_thrown = true;
+			$this->assertEquals($e->getMessage(), "Property 'serial' does not have a setter.");
+		}
+		$this->assertEquals($exception_thrown, true);
 	}
 }

unit-tests/models/Boutique/Robots.php

@@ -39,11 +39,16 @@ class Robots extends \Phalcon\Mvc\Model
 	 */
 	protected $text;
 
+	/**
+	 * Test restriction to not set hidden properties without setters.
+	 */
+	protected $serial;
+
 	public function getText() {
 		return $this->text;
 	}
 
 	public function setText($value) {
 		return ($this->text = $value . self::setterEpilogue);
 	}
-}
+}