[2.0.x] Issue #11286 - Model property visibility and setter functionality

CHANGELOG.md

@@ -1,4 +1,5 @@
 # [2.0.11](https://github.com/phalcon/cphalcon/releases/tag/phalcon-v2.0.11) (????-??-??)
+- Fix Model magic set functionality to maintain variable visibility and utilize setter methods.[#11286](https://github.com/phalcon/cphalcon/issues/11286)
 - Added a `prepareSave` event to model saving 
 - Added support for OnUpdate and OnDelete foreign key events to the MySQL adapter
 - Added ability to setLogLevel on multiple logs [#10429](https://github.com/phalcon/cphalcon/pull/10429)

phalcon/mvc/model.zep

@@ -483,10 +483,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;
 				}
 			}
@@ -4148,14 +4145,65 @@ abstract class Model implements EntityInterface, ModelInterface, ResultInterface
 			return value;
 		}
 
-		/**
-		 * Fallback assigning the value to the instance
-		 */
+		// Use possible setter.
+		if this->_possibleSetter(property, value) {
+			return 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.");
+		}
+
 		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
 	 *

unit-tests/ModelsGetterSetterTest.php

@@ -0,0 +1,127 @@
+<?php
+
+/*
+  +------------------------------------------------------------------------+
+  | Phalcon Framework                                                      |
+  +------------------------------------------------------------------------+
+  | Copyright (c) 2011-2016 Phalcon Team (http://www.phalconphp.com)       |
+  +------------------------------------------------------------------------+
+  | This source file is subject to the New BSD License that is bundled     |
+  | with this package in the file docs/LICENSE.txt.                        |
+  |                                                                        |
+  | If you did not receive a copy of the license and are unable to         |
+  | obtain it through the world-wide-web, please send an email             |
+  | to [email protected] so we can send you a copy immediately.       |
+  +------------------------------------------------------------------------+
+  | Authors: Nochum Sossonko                                               |
+  +------------------------------------------------------------------------+
+*/
+
+class ModelsGetterSetterTest extends PHPUnit_Framework_TestCase
+{
+
+	public function __construct()
+	{
+		spl_autoload_register(array($this, 'modelsAutoloader'));
+	}
+
+	public function __destruct()
+	{
+		spl_autoload_unregister(array($this, 'modelsAutoloader'));
+	}
+
+	public function modelsAutoloader($className)
+	{
+		$className = str_replace('\\', DIRECTORY_SEPARATOR, $className);
+		if (file_exists('unit-tests/models/' . $className . '.php')) {
+			require 'unit-tests/models/' . $className . '.php';
+		}
+	}
+
+	protected function _getDI()
+	{
+
+		Phalcon\DI::reset();
+
+		$di = new Phalcon\DI();
+
+		$di->set('modelsManager', function(){
+			return new Phalcon\Mvc\Model\Manager();
+		});
+
+		$di->set('modelsMetadata', function(){
+			return new Phalcon\Mvc\Model\Metadata\Memory();
+		});
+
+		return $di;
+	}
+
+	public function testModelsMysql()
+	{
+
+		$di = $this->_getDI();
+
+		require 'unit-tests/config.db.php';
+		if (empty($configMysql)) {
+			$this->markTestSkipped('Test skipped');
+			return;
+		}
+
+		$connection = new Phalcon\Db\Adapter\Pdo\Mysql($configMysql);
+
+		$di->set('db', $connection, true);
+
+		$this->_executeSetGet();
+	}
+
+	/*public function testModelsPostgresql()
+	{
+
+		$di = $this->_getDI();
+
+		$di->set('db', function(){
+			require 'unit-tests/config.db.php';
+			return new Phalcon\Db\Adapter\Pdo\Postgresql($configPostgresql);
+		}, true);
+
+		$this->_executeSetGet();
+	}
+
+	public function testModelsSqlite()
+	{
+
+		$di = $this->_getDI();
+
+		$di->set('db', function(){
+			require 'unit-tests/config.db.php';
+			return new Phalcon\Db\Adapter\Pdo\Sqlite($configSqlite);
+		}, true);
+
+		$this->_executeSetGet();
+	}*/
+
+	public function _executeSetGet()
+	{
+		$robot = Boutique\Robots::findFirst();
+		$testText = 'executeSetGet Test';
+		$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

@@ -5,6 +5,8 @@
 class Robots extends \Phalcon\Mvc\Model
 {
 
+	const setterEpilogue = " setText";
+
 	/**
 	 * @Primary
 	 * @Identity
@@ -35,5 +37,18 @@ class Robots extends \Phalcon\Mvc\Model
 	/**
 	 * @Column(type="text", nullable=false)
 	 */
-	public $text;
-}
+	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);
+	}
+}