diff --git a/CHANGELOG.md b/CHANGELOG.md index f34be9418b..23fd668ce6 100644 --- a/CHANGELOG.md +++ b/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) diff --git a/phalcon/mvc/model.zep b/phalcon/mvc/model.zep index 77bcfc9174..0fe7fa7152 100644 --- a/phalcon/mvc/model.zep +++ b/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 * diff --git a/unit-tests/ModelsGetterSetterTest.php b/unit-tests/ModelsGetterSetterTest.php new file mode 100644 index 0000000000..2ce54ff5d2 --- /dev/null +++ b/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 license@phalconphp.com 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); + } +} diff --git a/unit-tests/models/Boutique/Robots.php b/unit-tests/models/Boutique/Robots.php index ece06c9c6a..02d228df85 100644 --- a/unit-tests/models/Boutique/Robots.php +++ b/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; -} \ No newline at end of file + 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); + } +}