From cd691f9a6e3f97812c98e7385208e3dc3661bb4d Mon Sep 17 00:00:00 2001 From: Remi Collet Date: Thu, 10 Oct 2019 10:57:31 +0200 Subject: [PATCH 1/3] fix Using array_key_exists() on objects --- src/Model/ViewModel.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/Model/ViewModel.php b/src/Model/ViewModel.php index 04e76498..6c330dab 100644 --- a/src/Model/ViewModel.php +++ b/src/Model/ViewModel.php @@ -239,7 +239,7 @@ public function clearOptions() public function getVariable($name, $default = null) { $name = (string) $name; - if (array_key_exists($name, $this->variables)) { + if (isset($this->variables[$name])) { return $this->variables[$name]; } From c711fbddaedc322cf5cbdfaa5f4dce8c201e35d7 Mon Sep 17 00:00:00 2001 From: webimpress Date: Thu, 10 Oct 2019 23:01:54 +0100 Subject: [PATCH 2/3] Fixes failing tests on PHP 7.4 From PHP 7.4 docs: Calling get_object_vars() on an ArrayObject instance will now always return the properties of the ArrayObject itself (or a subclass). Previously it returned the values of the wrapped array/object unless the STD_PROP_LIST flag was specified. Other affected operations are: * ReflectionObject::getProperties() * reset(), current(), etc. Use Iterator methods instead. * Potentially others working on object properties as a list. --- .../application/views/scripts/partialLoopChildObject.phtml | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/test/Helper/_files/modules/application/views/scripts/partialLoopChildObject.phtml b/test/Helper/_files/modules/application/views/scripts/partialLoopChildObject.phtml index e5babd65..1463771b 100644 --- a/test/Helper/_files/modules/application/views/scripts/partialLoopChildObject.phtml +++ b/test/Helper/_files/modules/application/views/scripts/partialLoopChildObject.phtml @@ -1,12 +1,12 @@ vars(); +$vars = (array) $this->vars(); if (empty($vars)) { echo "No object model passed"; } elseif (isset($vars['message'])) { echo $vars['message']; } else { - $objKey = current($this->vars())->helper->getObjectKey(); + $objKey = current($vars)->helper->getObjectKey(); echo 'This is an iteration with objectKey: ' . $objKey; } From 81ed6ca3c83f295fabdb67f7d9e5e2c9de0a41be Mon Sep 17 00:00:00 2001 From: webimpress Date: Thu, 10 Oct 2019 23:08:36 +0100 Subject: [PATCH 3/3] Correct the fix so the behaviour of the original code is not changed Added tests to verify the change. --- src/Model/ViewModel.php | 9 +++++-- test/Model/ViewModelTest.php | 52 ++++++++++++++++++++++++++++++++++++ 2 files changed, 59 insertions(+), 2 deletions(-) diff --git a/src/Model/ViewModel.php b/src/Model/ViewModel.php index 6c330dab..5609a091 100644 --- a/src/Model/ViewModel.php +++ b/src/Model/ViewModel.php @@ -239,8 +239,13 @@ public function clearOptions() public function getVariable($name, $default = null) { $name = (string) $name; - if (isset($this->variables[$name])) { - return $this->variables[$name]; + + if (is_array($this->variables)) { + if (array_key_exists($name, $this->variables)) { + return $this->variables[$name]; + } + } elseif ($this->variables->offsetExists($name)) { + return $this->variables->offsetGet($name); } return $default; diff --git a/test/Model/ViewModelTest.php b/test/Model/ViewModelTest.php index 318825f8..e922a0e8 100644 --- a/test/Model/ViewModelTest.php +++ b/test/Model/ViewModelTest.php @@ -361,4 +361,56 @@ public function testCloneWithArray() $this->assertEquals('foo', $model1->getVariable('a')); $this->assertEquals('bar', $model2->getVariable('a')); } + + public function variableValue() + { + return [ + // variables default expected + + // if it is set always get the value + [['foo' => 'bar'], 'baz', 'bar'], + [['foo' => 'bar'], null, 'bar'], + [new ArrayObject(['foo' => 'bar']), 'baz', 'bar'], + [new ArrayObject(['foo' => 'bar']), null, 'bar'], + + // if it is null always get null value + [['foo' => null], null, null], + [['foo' => null], 'baz', null], + [new ArrayObject(['foo' => null]), null, null], + [new ArrayObject(['foo' => null]), 'baz', null], + + // when it is not set always get default value + [[], 'baz', 'baz'], + [new ArrayObject(), 'baz', 'baz'], + ]; + } + + /** + * @dataProvider variableValue + * + * @param array|ArrayObject $variables + * @param string|null $default + * @param string|null $expected + */ + public function testGetVariableSetByConstruct($variables, $default, $expected) + { + $model = new ViewModel($variables); + + self::assertSame($expected, $model->getVariable('foo', $default)); + } + + /** + * @dataProvider variableValue + * + * @param array|ArrayObject $variables + * @param string|null $default + * @param string|null $expected + */ + public function testGetVariableSetBySetter($variables, $default, $expected) + { + $model = new ViewModel(); + $model->setVariables($variables); + + self::assertSame($expected, $model->getVariable('foo', $default)); + } }