From 6d967d3f043317be88ba6ecf5f01f4eaa699ec6e Mon Sep 17 00:00:00 2001 From: Bertrand Drouhard Date: Fri, 27 Oct 2017 08:59:08 +0000 Subject: [PATCH 1/5] Explicit error message in the Mapper for keyExists function --- Configurability/ConfigurableServiceHelper.php | 14 ++++++++------ 1 file changed, 8 insertions(+), 6 deletions(-) diff --git a/Configurability/ConfigurableServiceHelper.php b/Configurability/ConfigurableServiceHelper.php index 560f846a..e19b2add 100644 --- a/Configurability/ConfigurableServiceHelper.php +++ b/Configurability/ConfigurableServiceHelper.php @@ -30,7 +30,7 @@ class ConfigurableServiceHelper const CONF_MESSAGE = 'message'; const CONF_RECOVERABLE = 'recoverable'; - const CONF_NO_RESULTS = 'noResults'; + const CONF_NO_RESULTS = 'no_results'; const STEP_DEFINE = 'define'; const STEP_VALIDATE = 'validate'; @@ -96,11 +96,13 @@ public function createContext(array $options, MessageInterface $message = null, return $context; } - public function resolveArray($input, &$context){ + public function resolveArray($input, &$context) + { $output = []; - foreach($input as $key => $value){ - $output[$key] = $this->resolve($value,$context); + foreach ($input as $key => $value) { + $output[$key] = $this->resolve($value, $context); } + return $output; } @@ -163,6 +165,7 @@ public function executeStep($stepAction, &$stepActionParams, &$options, array &$ break; case self::STEP_VALIDATE: $this->validate($stepActionParams, $context); + return true; break; default: @@ -196,7 +199,6 @@ public function define(array $definitions, array &$context) public function validate(array $stepConfig, array &$context) { - $config = $this->validateResolver->resolve($stepConfig); $rule = $config[self::CONF_RULE]; @@ -206,7 +208,7 @@ public function validate(array $stepConfig, array &$context) $display_message = $config[self::CONF_DISPLAY_MESSAGE]; $evaluation = $this->resolve($rule, $context); - if ($evaluation !== true) { + if (true !== $evaluation) { $message = $this->resolve($message, $context); if ($no_results) { throw new NoResultsException($message); From 86b2b3b7a3661bb67fde9f31ea026721ffce0d4d Mon Sep 17 00:00:00 2001 From: Bertrand Drouhard Date: Fri, 27 Oct 2017 09:01:30 +0000 Subject: [PATCH 2/5] Unify parameter name for no_results instead of noResults --- Tools/Mapper/Mapper.php | 15 ++++++++++----- 1 file changed, 10 insertions(+), 5 deletions(-) diff --git a/Tools/Mapper/Mapper.php b/Tools/Mapper/Mapper.php index 810b43c6..3ce4922c 100644 --- a/Tools/Mapper/Mapper.php +++ b/Tools/Mapper/Mapper.php @@ -42,7 +42,7 @@ public function addMappings(array $mappings) */ public function formatDate($format, \DateTime $date = null) { - if ($date === null) { + if (null === $date) { return null; } @@ -87,7 +87,7 @@ public function resolve(&$mapping, &$obj, &$context) foreach ($mapping as $key => $value) { $resolved = $this->resolve($value, $obj, $context); - if ($resolved !== null) { + if (null !== $resolved) { $res[$key] = $resolved; } } @@ -146,6 +146,10 @@ public function mapAll($elements, $mappingName, $context = []) */ public function keyExists(array $obj, $key) { + if (!is_string($key) and !is_integer($key)) { + throw new \RuntimeException('keyExists expected either a string or an integer, \''.print_r($key, true).'\' was given.'); + } + return array_key_exists($key, $obj); } @@ -302,11 +306,11 @@ public function serializeWithGroup($data, $format, $group) } /** - * Return the n-th section of the given string splitted by piece of the given length + * Return the n-th section of the given string splitted by piece of the given length. * * @param string $string - * @param int $length - * @param int $section + * @param int $length + * @param int $section * * @return string */ @@ -320,6 +324,7 @@ public function wordWrap($string, $length, $section) if (isset($lines[$section])) { $result = $lines[$section]; } + return $result; } From 8a0014e7761d89f8fd146a8f0abaf60b0f34f0df Mon Sep 17 00:00:00 2001 From: Bertrand Drouhard Date: Mon, 6 Nov 2017 16:44:47 +0000 Subject: [PATCH 3/5] Remove changes in ConfigurableServiceHelper.php after split of the Pull request --- Configurability/ConfigurableServiceHelper.php | 14 ++++++-------- 1 file changed, 6 insertions(+), 8 deletions(-) diff --git a/Configurability/ConfigurableServiceHelper.php b/Configurability/ConfigurableServiceHelper.php index e19b2add..560f846a 100644 --- a/Configurability/ConfigurableServiceHelper.php +++ b/Configurability/ConfigurableServiceHelper.php @@ -30,7 +30,7 @@ class ConfigurableServiceHelper const CONF_MESSAGE = 'message'; const CONF_RECOVERABLE = 'recoverable'; - const CONF_NO_RESULTS = 'no_results'; + const CONF_NO_RESULTS = 'noResults'; const STEP_DEFINE = 'define'; const STEP_VALIDATE = 'validate'; @@ -96,13 +96,11 @@ public function createContext(array $options, MessageInterface $message = null, return $context; } - public function resolveArray($input, &$context) - { + public function resolveArray($input, &$context){ $output = []; - foreach ($input as $key => $value) { - $output[$key] = $this->resolve($value, $context); + foreach($input as $key => $value){ + $output[$key] = $this->resolve($value,$context); } - return $output; } @@ -165,7 +163,6 @@ public function executeStep($stepAction, &$stepActionParams, &$options, array &$ break; case self::STEP_VALIDATE: $this->validate($stepActionParams, $context); - return true; break; default: @@ -199,6 +196,7 @@ public function define(array $definitions, array &$context) public function validate(array $stepConfig, array &$context) { + $config = $this->validateResolver->resolve($stepConfig); $rule = $config[self::CONF_RULE]; @@ -208,7 +206,7 @@ public function validate(array $stepConfig, array &$context) $display_message = $config[self::CONF_DISPLAY_MESSAGE]; $evaluation = $this->resolve($rule, $context); - if (true !== $evaluation) { + if ($evaluation !== true) { $message = $this->resolve($message, $context); if ($no_results) { throw new NoResultsException($message); From 22f03a91eabc2bae243aaa2199c7c0a369b33570 Mon Sep 17 00:00:00 2001 From: Bertrand Drouhard Date: Mon, 6 Nov 2017 17:16:02 +0000 Subject: [PATCH 4/5] Add Tests for mapper keyExists --- Tests/Functional/Tools/Mapper/MapperTest.php | 23 +++++++++++++++++--- Tools/Mapper/Mapper.php | 10 ++++++--- 2 files changed, 27 insertions(+), 6 deletions(-) diff --git a/Tests/Functional/Tools/Mapper/MapperTest.php b/Tests/Functional/Tools/Mapper/MapperTest.php index 4b542639..12db0fc7 100644 --- a/Tests/Functional/Tools/Mapper/MapperTest.php +++ b/Tests/Functional/Tools/Mapper/MapperTest.php @@ -37,7 +37,7 @@ public function dataProviderForCorrectMappings() 'x' => "obj.get('x') + 1", 'y' => "obj.get('x') + 2", 'z' => "obj.get('x') + 3", - 'array' => "obj.get('a')" + 'array' => "obj.get('a')", ], ], 'mapping_name' => 'x_to_xyz', @@ -47,7 +47,7 @@ public function dataProviderForCorrectMappings() 'x' => 11, 'y' => 12, 'z' => 13, - 'array' => [] + 'array' => [], ], ], 'Test null values' => [ @@ -55,7 +55,7 @@ public function dataProviderForCorrectMappings() 'mapping_name' => [ 'x' => "obj.get('x') + 1", 'y' => "obj.get('y')", - 'z' => "obj.get('y').get('z')" + 'z' => "obj.get('y').get('z')", ], ], 'mapping_name' => 'mapping_name', @@ -123,6 +123,23 @@ public function dataProviderForCorrectMappings() 'date_3' => '2015-01-01T20:00:00.000', ], ], + 'Test keyExists' => [ + 'mappings' => [ + 'mapping_name' => [ + 'test_key_1' => "mapper.keyExists(obj.get('test_array'), 'A')", + 'test_key_2' => "mapper.keyExists(obj.get('test_array'), 'C')", + ], + ], + 'mapping_name' => 'mapping_name', + 'mapped_values' => new SerializableArray([ + 'test_array' => ['A' => 0, 'B' => 0], + ]), + 'context' => [], + 'expected_value' => [ + 'test_key_1' => true, + 'test_key_2' => false, + ], + ], 'Test mapping getting information from the context' => [ 'mappings' => [ 'x_to_xyz' => [ diff --git a/Tools/Mapper/Mapper.php b/Tools/Mapper/Mapper.php index 3ce4922c..22de2d3b 100644 --- a/Tools/Mapper/Mapper.php +++ b/Tools/Mapper/Mapper.php @@ -144,13 +144,17 @@ public function mapAll($elements, $mappingName, $context = []) * * @return bool */ - public function keyExists(array $obj, $key) + public function keyExists(array $array, $key) { + if (!is_array($array)) { + throw new \RuntimeException('keyExists expected the first argument to be an array, \''.print_r($array, true).'\' was given.'); + } + if (!is_string($key) and !is_integer($key)) { - throw new \RuntimeException('keyExists expected either a string or an integer, \''.print_r($key, true).'\' was given.'); + throw new \RuntimeException('keyExists expected the key (second argument) to be either a string or an integer, \''.print_r($key, true).'\' was given.'); } - return array_key_exists($key, $obj); + return array_key_exists($key, $array); } /** From eca24f392eca39aa0882d86e97dcf3efd0c49f42 Mon Sep 17 00:00:00 2001 From: Bertrand Drouhard Date: Mon, 6 Nov 2017 18:09:07 +0000 Subject: [PATCH 5/5] Added tests in th Mapper for keyExists exceptions --- Tests/Functional/Tools/Mapper/MapperTest.php | 34 ++++++++++++++++++++ Tools/Mapper/Mapper.php | 6 ++-- 2 files changed, 37 insertions(+), 3 deletions(-) diff --git a/Tests/Functional/Tools/Mapper/MapperTest.php b/Tests/Functional/Tools/Mapper/MapperTest.php index 12db0fc7..5f5cefc3 100644 --- a/Tests/Functional/Tools/Mapper/MapperTest.php +++ b/Tests/Functional/Tools/Mapper/MapperTest.php @@ -195,6 +195,40 @@ public function testMapForEmptyMappingName() ); } + /** + * @covers ::map + */ + public function testKeyExistsForExceptions() + { + $this->expectException(\InvalidArgumentException::class); + $this->mapper->addMappings([ + 'mapping_name' => [ + 'test_key_1' => "mapper.keyExists(obj.get('test_array'), obj.get('test_key'))", + ], + ]); + + // Testing 1st argument + $this->expectExceptionMessage('keyExists expected the first argument to be an array'); + $this->mapper->map( + new SerializableArray([ + 'test_array' => 'A', + 'test_key' => 'B', + ]), + 'mapping_name' + ); + + // Testing 2nd argument + $this->expectExceptionMessage('keyExists expected the key (second argument) to be either a string or an integer'); + $this->mapper->map( + new SerializableArray([ + 'test_array' => ['A' => 0, 'B' => 0], + 'test_key' => [10], + ]), + 'mapping_name' + ); + + } + /** * @covers ::map */ diff --git a/Tools/Mapper/Mapper.php b/Tools/Mapper/Mapper.php index 22de2d3b..66e1ebaa 100644 --- a/Tools/Mapper/Mapper.php +++ b/Tools/Mapper/Mapper.php @@ -144,14 +144,14 @@ public function mapAll($elements, $mappingName, $context = []) * * @return bool */ - public function keyExists(array $array, $key) + public function keyExists($array, $key) { if (!is_array($array)) { - throw new \RuntimeException('keyExists expected the first argument to be an array, \''.print_r($array, true).'\' was given.'); + throw new \InvalidArgumentException('keyExists expected the first argument to be an array, \''.print_r($array, true).'\' was given.'); } if (!is_string($key) and !is_integer($key)) { - throw new \RuntimeException('keyExists expected the key (second argument) to be either a string or an integer, \''.print_r($key, true).'\' was given.'); + throw new \InvalidArgumentException('keyExists expected the key (second argument) to be either a string or an integer, \''.print_r($key, true).'\' was given.'); } return array_key_exists($key, $array);