From dd546a98882a1291863b0dbe5858c28410092237 Mon Sep 17 00:00:00 2001 From: Simon Welsh Date: Fri, 23 Dec 2011 11:04:44 +1300 Subject: [PATCH] BUGFIX Merge request arrays recursively --- control/Director.php | 4 +- control/HTTPRequest.php | 2 +- core/ArrayLib.php | 37 +++++++- tests/control/HTTPRequestTest.php | 133 ++++++++++++++++++++++++++- tests/core/ArrayLibTest.php | 143 +++++++++++++++++++++++++++++- 5 files changed, 312 insertions(+), 7 deletions(-) diff --git a/control/Director.php b/control/Director.php index 040c848b8..c1f59e28a 100644 --- a/control/Director.php +++ b/control/Director.php @@ -84,7 +84,7 @@ class Director { (isset($_SERVER['X-HTTP-Method-Override'])) ? $_SERVER['X-HTTP-Method-Override'] : $_SERVER['REQUEST_METHOD'], $url, $_GET, - array_merge((array)$_POST, (array)$_FILES), + ArrayLib::array_merge_recursive((array)$_POST, (array)$_FILES), @file_get_contents('php://input') ); @@ -193,7 +193,7 @@ class Director { } // Replace the superglobals with appropriate test values - $_REQUEST = array_merge((array)$getVars, (array)$postVars); + $_REQUEST = ArrayLib::array_merge_recursive((array)$getVars, (array)$postVars); $_GET = (array)$getVars; $_POST = (array)$postVars; $_SESSION = $session ? $session->inst_getAll() : array(); diff --git a/control/HTTPRequest.php b/control/HTTPRequest.php index c82043ab3..06bc90063 100644 --- a/control/HTTPRequest.php +++ b/control/HTTPRequest.php @@ -149,7 +149,7 @@ class SS_HTTPRequest implements ArrayAccess { * @return array */ function requestVars() { - return array_merge($this->getVars, $this->postVars); + return ArrayLib::array_merge_recursive($this->getVars, $this->postVars); } function getVar($name) { diff --git a/core/ArrayLib.php b/core/ArrayLib.php index 6195fe2b0..3e348df95 100644 --- a/core/ArrayLib.php +++ b/core/ArrayLib.php @@ -141,6 +141,39 @@ class ArrayLib { return false; // Never found $needle :( } + /** + * Recursively merges two or more arrays. + * + * Behaves similar to array_merge_recursive(), however it only merges values when both are arrays + * rather than creating a new array with both values, as the PHP version does. The same behaviour + * also occurs with numeric keys, to match that of what PHP does to generate $_REQUEST. + * + * @param array $array, ... + * @return array + */ + static function array_merge_recursive($array) { + $arrays = func_get_args(); + $merged = array(); + if(count($arrays) == 1) { + return $array; + } + while ($arrays) { + $array = array_shift($arrays); + if (!is_array($array)) { + trigger_error('ArrayLib::array_merge_recursive() encountered a non array argument', E_USER_WARNING); + return; + } + if (!$array) { + continue; + } + foreach ($array as $key => $value) { + if (is_array($value) && array_key_exists($key, $merged) && is_array($merged[$key])) { + $merged[$key] = ArrayLib::array_merge_recursive($merged[$key], $value); + } else { + $merged[$key] = $value; + } + } + } + return $merged; + } } - -?> \ No newline at end of file diff --git a/tests/control/HTTPRequestTest.php b/tests/control/HTTPRequestTest.php index c46a2dd9d..4ae6b1b0a 100644 --- a/tests/control/HTTPRequestTest.php +++ b/tests/control/HTTPRequestTest.php @@ -99,4 +99,135 @@ class HTTPRequestTest extends SapphireTest { ); } -} \ No newline at end of file + public function testRequestVars() { + $getVars = array( + 'first' => 'a', + 'second' => 'b', + ); + $postVars = array( + 'third' => 'c', + 'fourth' => 'd', + ); + $requestVars = array( + 'first' => 'a', + 'second' => 'b', + 'third' => 'c', + 'fourth' => 'd', + ); + $request = new SS_HTTPRequest( + 'POST', + 'admin/crm', + $getVars, + $postVars + ); + $this->assertEquals( + $requestVars, + $request->requestVars(), + 'GET parameters should supplement POST parameters' + ); + + $getVars = array( + 'first' => 'a', + 'second' => 'b', + ); + $postVars = array( + 'first' => 'c', + 'third' => 'd', + ); + $requestVars = array( + 'first' => 'c', + 'second' => 'b', + 'third' => 'd', + ); + $request = new SS_HTTPRequest( + 'POST', + 'admin/crm', + $getVars, + $postVars + ); + $this->assertEquals( + $requestVars, + $request->requestVars(), + 'POST parameters should override GET parameters' + ); + + $getVars = array( + 'first' => array( + 'first' => 'a', + ), + 'second' => array( + 'second' => 'b', + ), + ); + $postVars = array( + 'first' => array( + 'first' => 'c', + ), + 'third' => array( + 'third' => 'd', + ), + ); + $requestVars = array( + 'first' => array( + 'first' => 'c', + ), + 'second' => array( + 'second' => 'b', + ), + 'third' => array( + 'third' => 'd', + ), + ); + $request = new SS_HTTPRequest( + 'POST', + 'admin/crm', + $getVars, + $postVars + ); + $this->assertEquals( + $requestVars, + $request->requestVars(), + 'Nested POST parameters should override GET parameters' + ); + + $getVars = array( + 'first' => array( + 'first' => 'a', + ), + 'second' => array( + 'second' => 'b', + ), + ); + $postVars = array( + 'first' => array( + 'second' => 'c', + ), + 'third' => array( + 'third' => 'd', + ), + ); + $requestVars = array( + 'first' => array( + 'first' => 'a', + 'second' => 'c', + ), + 'second' => array( + 'second' => 'b', + ), + 'third' => array( + 'third' => 'd', + ), + ); + $request = new SS_HTTPRequest( + 'POST', + 'admin/crm', + $getVars, + $postVars + ); + $this->assertEquals( + $requestVars, + $request->requestVars(), + 'Nested GET parameters should supplement POST parameters' + ); + } +} diff --git a/tests/core/ArrayLibTest.php b/tests/core/ArrayLibTest.php index f67765f7d..c126eb7a6 100644 --- a/tests/core/ArrayLibTest.php +++ b/tests/core/ArrayLibTest.php @@ -45,5 +45,146 @@ class ArrayLibTest extends SapphireTest { ) ); } - + + function testArrayMergeRecursive() { + $first = array( + 'first' => 'a', + 'second' => 'b', + ); + $second = array( + 'third' => 'c', + 'fourth' => 'd', + ); + $expected = array( + 'first' => 'a', + 'second' => 'b', + 'third' => 'c', + 'fourth' => 'd', + ); + $this->assertEquals( + $expected, + ArrayLib::array_merge_recursive($first, $second), + 'First values should supplement second values' + ); + + $first = array( + 'first' => 'a', + 'second' => 'b', + ); + $second = array( + 'first' => 'c', + 'third' => 'd', + ); + $expected = array( + 'first' => 'c', + 'second' => 'b', + 'third' => 'd', + ); + $this->assertEquals( + $expected, + ArrayLib::array_merge_recursive($first, $second), + 'Second values should override first values' + ); + + $first = array( + 'first' => array( + 'first' => 'a', + ), + 'second' => array( + 'second' => 'b', + ), + ); + $second = array( + 'first' => array( + 'first' => 'c', + ), + 'third' => array( + 'third' => 'd', + ), + ); + $expected = array( + 'first' => array( + 'first' => 'c', + ), + 'second' => array( + 'second' => 'b', + ), + 'third' => array( + 'third' => 'd', + ), + ); + $this->assertEquals( + $expected, + ArrayLib::array_merge_recursive($first, $second), + 'Nested second values should override first values' + ); + + $first = array( + 'first' => array( + 'first' => 'a', + ), + 'second' => array( + 'second' => 'b', + ), + ); + $second = array( + 'first' => array( + 'second' => 'c', + ), + 'third' => array( + 'third' => 'd', + ), + ); + $expected = array( + 'first' => array( + 'first' => 'a', + 'second' => 'c', + ), + 'second' => array( + 'second' => 'b', + ), + 'third' => array( + 'third' => 'd', + ), + ); + $this->assertEquals( + $expected, + ArrayLib::array_merge_recursive($first, $second), + 'Nested first values should supplement second values' + ); + + $first = array( + 'first' => array( + 0 => 'a', + ), + 'second' => array( + 1 => 'b', + ), + ); + $second = array( + 'first' => array( + 0 => 'c', + ), + 'third' => array( + 2 => 'd', + ), + ); + $expected = array( + 'first' => array( + 0 => 'c', + ), + 'second' => array( + 1 => 'b', + ), + 'third' => array( + 2 => 'd', + ), + ); + + $this->assertEquals( + $expected, + ArrayLib::array_merge_recursive($first, $second), + 'Numeric keys should behave like string keys' + ); + } } \ No newline at end of file