From 7bac7b0c4316826494ebeca886251edd20573e3f Mon Sep 17 00:00:00 2001 From: Patrick McCarren Date: Fri, 21 Jul 2017 15:26:13 -0400 Subject: [PATCH 1/6] Only cast collection properties as int if numeric This allows string based keys, such as uuids, to remain intact. --- src/ModeResolver/IdsModeResolver.php | 6 ++--- src/Utility.php | 28 +++++++++++++++++++--- tests/Controller.php | 13 +++++----- tests/ModeResolver/IdsModeResolverTest.php | 20 ++++++++++++++++ 4 files changed, 55 insertions(+), 12 deletions(-) diff --git a/src/ModeResolver/IdsModeResolver.php b/src/ModeResolver/IdsModeResolver.php index 74a79a8..2b26366 100644 --- a/src/ModeResolver/IdsModeResolver.php +++ b/src/ModeResolver/IdsModeResolver.php @@ -30,15 +30,15 @@ public function resolve($property, &$object, &$root, $fullPropertyPath) // model, because the first item returned was a property // We therefore just return the single ID if (Utility::isPrimitive($firstElement)) { - return (int) Utility::getProperty($object, 'id'); + return Utility::getProperty($object, 'id', true); } return array_map(function ($entry) { - return (int) Utility::getProperty($entry, 'id'); + return Utility::getProperty($entry, 'id', true); }, $object); } elseif ($object instanceof Collection) { return $object->map(function ($entry) { - return (int) Utility::getProperty($entry, 'id'); + return Utility::getProperty($entry, 'id', true); }); // The relation is not a collection, but rather // a singular relation diff --git a/src/Utility.php b/src/Utility.php index 31cb312..e952eb7 100644 --- a/src/Utility.php +++ b/src/Utility.php @@ -13,13 +13,19 @@ class Utility * @param string $property * @return mixed */ - public static function getProperty($objectOrArray, $property) + public static function getProperty($objectOrArray, $property, $format = false) { if (is_array($objectOrArray)) { - return $objectOrArray[$property]; + $ret = $objectOrArray[$property]; } else { - return $objectOrArray->{$property}; + $ret = $objectOrArray->{$property}; } + + if (!$format) { + return $ret; + } + + return static::formatProperty($ret); } /** @@ -75,4 +81,20 @@ public static function isCollection($input) { return is_array($input) || $input instanceof Collection; } + + /** + * Format a given property. + * @param mixed $property + * @param bool $numericIntegers If true and property passes is_numeric test, cast as an integer + * @return mixed + */ + private static function formatProperty($property, $numericIntegers = true) + { + // handle default integer based properties + if ($numericIntegers && is_numeric($property)) { + return (int) $property; + } + + return $property; + } } diff --git a/tests/Controller.php b/tests/Controller.php index 7f16e96..248c55c 100644 --- a/tests/Controller.php +++ b/tests/Controller.php @@ -23,14 +23,14 @@ public function getEloquentCollection(array $eagerLoad = array(), array $modes = return $this->architect->parseData($collection, $modes, 'collection'); } - public function getCollection($rows, array $modes = [], $children = false, $childrensChildren = false, $array = false) + public function getCollection($rows, array $modes = [], $children = false, $childrensChildren = false, $array = false, $stringKeys = false) { - $collection = $this->createCollection($rows, $children, $childrensChildren, $array); + $collection = $this->createCollection($rows, $children, $childrensChildren, $array, $stringKeys); return $this->architect->parseData($collection, $modes, 'collection'); } - private function createCollection($rows = 2, $children = false, $childrensChildren = false, $array = false, $level = 1) + private function createCollection($rows = 2, $children = false, $childrensChildren = false, $array = false, $stringKeys = false, $level = 1) { if (!array_key_exists($level, $this->idCounts)) { $this->idCounts[$level] = 0; @@ -40,18 +40,19 @@ private function createCollection($rows = 2, $children = false, $childrensChildr for ($i=1;$i<=$rows;$i++) { $this->idCounts[$level]++; + $newId = (!$stringKeys ? $this->idCounts[$level] : 'aaa'.dechex($this->idCounts[$level])); $tmp = [ - 'id' => $this->idCounts[$level], + 'id' => $newId, 'title' => 'Resource ' . $i, 'singleChildren' => [ - 'id' => $this->idCounts[$level], + 'id' => $newId, 'name' => 'Single child' ] ]; if (is_int($children)) { $key = $level === 1 ? 'children' : 'nestedChildren'; - $tmp[$key] = $this->createCollection($children, $childrensChildren, false, $array, ($level+1)); + $tmp[$key] = $this->createCollection($children, $childrensChildren, false, $array, $stringKeys, ($level+1)); } $data[] = $tmp; diff --git a/tests/ModeResolver/IdsModeResolverTest.php b/tests/ModeResolver/IdsModeResolverTest.php index e2bbe6b..631b967 100644 --- a/tests/ModeResolver/IdsModeResolverTest.php +++ b/tests/ModeResolver/IdsModeResolverTest.php @@ -91,6 +91,26 @@ public function testIdsModeResolverOnNestedChildrenOnArrayCollections() ], $second); } + public function testStringIdsModeResolverOnNestedChildrenOnArrayCollections() + { + $controller = new Controller; + + $modes = [ + 'children.nestedChildren' => 'ids' + ]; + $parsed = $controller->getCollection(2, $modes, 2, 2, true, true)['collection']; + + $first = $parsed[0]['children'][0]['nestedChildren']; + $this->assertEquals([ + 'aaa1', 'aaa2' + ], $first); + + $second = $parsed[1]['children'][1]['nestedChildren']; + $this->assertEquals([ + 'aaa7', 'aaa8' + ], $second); + } + public function testIdsModeResolverOnEloquentCollections() { $controller = new Controller; From da87081a4cfab73c90047a4c4772b66e13eac6e2 Mon Sep 17 00:00:00 2001 From: Patrick McCarren Date: Fri, 21 Jul 2017 15:26:13 -0400 Subject: [PATCH 2/6] fix: only cast properties as int if numeric This allows string based keys, such as uuids, to remain intact. Closes esbenp/architect#6 --- src/ModeResolver/IdsModeResolver.php | 6 ++--- src/Utility.php | 28 +++++++++++++++++++--- tests/Controller.php | 13 +++++----- tests/ModeResolver/IdsModeResolverTest.php | 20 ++++++++++++++++ 4 files changed, 55 insertions(+), 12 deletions(-) diff --git a/src/ModeResolver/IdsModeResolver.php b/src/ModeResolver/IdsModeResolver.php index 74a79a8..2b26366 100644 --- a/src/ModeResolver/IdsModeResolver.php +++ b/src/ModeResolver/IdsModeResolver.php @@ -30,15 +30,15 @@ public function resolve($property, &$object, &$root, $fullPropertyPath) // model, because the first item returned was a property // We therefore just return the single ID if (Utility::isPrimitive($firstElement)) { - return (int) Utility::getProperty($object, 'id'); + return Utility::getProperty($object, 'id', true); } return array_map(function ($entry) { - return (int) Utility::getProperty($entry, 'id'); + return Utility::getProperty($entry, 'id', true); }, $object); } elseif ($object instanceof Collection) { return $object->map(function ($entry) { - return (int) Utility::getProperty($entry, 'id'); + return Utility::getProperty($entry, 'id', true); }); // The relation is not a collection, but rather // a singular relation diff --git a/src/Utility.php b/src/Utility.php index 31cb312..e952eb7 100644 --- a/src/Utility.php +++ b/src/Utility.php @@ -13,13 +13,19 @@ class Utility * @param string $property * @return mixed */ - public static function getProperty($objectOrArray, $property) + public static function getProperty($objectOrArray, $property, $format = false) { if (is_array($objectOrArray)) { - return $objectOrArray[$property]; + $ret = $objectOrArray[$property]; } else { - return $objectOrArray->{$property}; + $ret = $objectOrArray->{$property}; } + + if (!$format) { + return $ret; + } + + return static::formatProperty($ret); } /** @@ -75,4 +81,20 @@ public static function isCollection($input) { return is_array($input) || $input instanceof Collection; } + + /** + * Format a given property. + * @param mixed $property + * @param bool $numericIntegers If true and property passes is_numeric test, cast as an integer + * @return mixed + */ + private static function formatProperty($property, $numericIntegers = true) + { + // handle default integer based properties + if ($numericIntegers && is_numeric($property)) { + return (int) $property; + } + + return $property; + } } diff --git a/tests/Controller.php b/tests/Controller.php index 7f16e96..248c55c 100644 --- a/tests/Controller.php +++ b/tests/Controller.php @@ -23,14 +23,14 @@ public function getEloquentCollection(array $eagerLoad = array(), array $modes = return $this->architect->parseData($collection, $modes, 'collection'); } - public function getCollection($rows, array $modes = [], $children = false, $childrensChildren = false, $array = false) + public function getCollection($rows, array $modes = [], $children = false, $childrensChildren = false, $array = false, $stringKeys = false) { - $collection = $this->createCollection($rows, $children, $childrensChildren, $array); + $collection = $this->createCollection($rows, $children, $childrensChildren, $array, $stringKeys); return $this->architect->parseData($collection, $modes, 'collection'); } - private function createCollection($rows = 2, $children = false, $childrensChildren = false, $array = false, $level = 1) + private function createCollection($rows = 2, $children = false, $childrensChildren = false, $array = false, $stringKeys = false, $level = 1) { if (!array_key_exists($level, $this->idCounts)) { $this->idCounts[$level] = 0; @@ -40,18 +40,19 @@ private function createCollection($rows = 2, $children = false, $childrensChildr for ($i=1;$i<=$rows;$i++) { $this->idCounts[$level]++; + $newId = (!$stringKeys ? $this->idCounts[$level] : 'aaa'.dechex($this->idCounts[$level])); $tmp = [ - 'id' => $this->idCounts[$level], + 'id' => $newId, 'title' => 'Resource ' . $i, 'singleChildren' => [ - 'id' => $this->idCounts[$level], + 'id' => $newId, 'name' => 'Single child' ] ]; if (is_int($children)) { $key = $level === 1 ? 'children' : 'nestedChildren'; - $tmp[$key] = $this->createCollection($children, $childrensChildren, false, $array, ($level+1)); + $tmp[$key] = $this->createCollection($children, $childrensChildren, false, $array, $stringKeys, ($level+1)); } $data[] = $tmp; diff --git a/tests/ModeResolver/IdsModeResolverTest.php b/tests/ModeResolver/IdsModeResolverTest.php index e2bbe6b..631b967 100644 --- a/tests/ModeResolver/IdsModeResolverTest.php +++ b/tests/ModeResolver/IdsModeResolverTest.php @@ -91,6 +91,26 @@ public function testIdsModeResolverOnNestedChildrenOnArrayCollections() ], $second); } + public function testStringIdsModeResolverOnNestedChildrenOnArrayCollections() + { + $controller = new Controller; + + $modes = [ + 'children.nestedChildren' => 'ids' + ]; + $parsed = $controller->getCollection(2, $modes, 2, 2, true, true)['collection']; + + $first = $parsed[0]['children'][0]['nestedChildren']; + $this->assertEquals([ + 'aaa1', 'aaa2' + ], $first); + + $second = $parsed[1]['children'][1]['nestedChildren']; + $this->assertEquals([ + 'aaa7', 'aaa8' + ], $second); + } + public function testIdsModeResolverOnEloquentCollections() { $controller = new Controller; From 40544061f6b5671fdfe445f767a10af5c99f5dc5 Mon Sep 17 00:00:00 2001 From: Patrick McCarren Date: Fri, 21 Jul 2017 15:46:09 -0400 Subject: [PATCH 3/6] test: use Ubuntu Trusty for Travis Builds - Use Ubuntu Trusty instead of Ubuntu Precise. - Add PHP 7.1 build Closes esbenp/architect#8 --- .travis.yml | 2 ++ 1 file changed, 2 insertions(+) diff --git a/.travis.yml b/.travis.yml index fb7cd20..6708167 100644 --- a/.travis.yml +++ b/.travis.yml @@ -1,9 +1,11 @@ language: php +dist: trusty install: composer install php: - 5.5 - 5.6 + - 7.1 - hhvm script: From 059fbe8ad66c03532dd2c3c75de8b2bba7eac1df Mon Sep 17 00:00:00 2001 From: Patrick McCarren Date: Fri, 21 Jul 2017 17:15:28 -0400 Subject: [PATCH 4/6] fix: format properties for sideload mode --- src/ModeResolver/SideloadModeResolver.php | 4 +- tests/ModeResolver/IdsModeResolverTest.php | 2 +- .../ModeResolver/SideloadModeResolverTest.php | 40 +++++++++++++++++++ 3 files changed, 43 insertions(+), 3 deletions(-) diff --git a/src/ModeResolver/SideloadModeResolver.php b/src/ModeResolver/SideloadModeResolver.php index c898e7a..6185334 100644 --- a/src/ModeResolver/SideloadModeResolver.php +++ b/src/ModeResolver/SideloadModeResolver.php @@ -102,13 +102,13 @@ private function mergeRootCollection(&$collection, $object) */ private function addResourceToRootCollectionIfNonExistant(&$collection, $resource) { - $identifier = Utility::getProperty($resource, 'id'); + $identifier = Utility::getProperty($resource, 'id', true); $exists = false; $copy = $collection instanceof Collection ? $collection->toArray() : $collection; foreach ($copy as $rootResource) { - if ((int) Utility::getProperty($rootResource, 'id') === (int) $identifier) { + if (Utility::getProperty($rootResource, 'id', true) === $identifier) { $exists = true; break; } diff --git a/tests/ModeResolver/IdsModeResolverTest.php b/tests/ModeResolver/IdsModeResolverTest.php index 631b967..62f13fb 100644 --- a/tests/ModeResolver/IdsModeResolverTest.php +++ b/tests/ModeResolver/IdsModeResolverTest.php @@ -91,7 +91,7 @@ public function testIdsModeResolverOnNestedChildrenOnArrayCollections() ], $second); } - public function testStringIdsModeResolverOnNestedChildrenOnArrayCollections() + public function testIdsModeResolverOnNestedChildrenOnArrayCollectionsAndStringKeys() { $controller = new Controller; diff --git a/tests/ModeResolver/SideloadModeResolverTest.php b/tests/ModeResolver/SideloadModeResolverTest.php index 3eb5bd4..45a6b18 100644 --- a/tests/ModeResolver/SideloadModeResolverTest.php +++ b/tests/ModeResolver/SideloadModeResolverTest.php @@ -77,6 +77,46 @@ public function testSideloadModeResolverOnNestedChildrenOnVanillaCollections() ]); } + public function testSideloadModeResolverOnNestedChildrenOnVanillaCollectionsAndStringKeys() + { + $controller = new Controller; + + $modes = [ + 'children.nestedChildren' => 'sideload' + ]; + $parsed = $controller->getCollection(2, $modes, 2, 2, false, true); + $collection = $parsed['collection']; + $nestedChildren = $parsed['children.nestedChildren']; + + $first = $collection->get(0); + $this->assertEquals([ + 'aaa1', 'aaa2', 'aaa3', 'aaa4', 'aaa1', 'aaa2', 'aaa3', 'aaa4' + ], [ + $first['children']->get(0)['nestedChildren']->get(0), + $first['children']->get(0)['nestedChildren']->get(1), + $first['children']->get(1)['nestedChildren']->get(0), + $first['children']->get(1)['nestedChildren']->get(1), + $nestedChildren->get(0)['id'], + $nestedChildren->get(1)['id'], + $nestedChildren->get(2)['id'], + $nestedChildren->get(3)['id'] + ]); + + $second = $collection->get(1); + $this->assertEquals([ + 'aaa5', 'aaa6', 'aaa7', 'aaa8', 'aaa5', 'aaa6', 'aaa7', 'aaa8' + ], [ + $second['children']->get(0)['nestedChildren']->get(0), + $second['children']->get(0)['nestedChildren']->get(1), + $second['children']->get(1)['nestedChildren']->get(0), + $second['children']->get(1)['nestedChildren']->get(1), + $nestedChildren->get(4)['id'], + $nestedChildren->get(5)['id'], + $nestedChildren->get(6)['id'], + $nestedChildren->get(7)['id'] + ]); + } + public function testSideloadModeResolverOnParentAndNestedChildrenOnVanillaCollections() { $controller = new Controller; From d2a1b34b564bdb73c65a4a154f0a07dda236a045 Mon Sep 17 00:00:00 2001 From: Patrick McCarren Date: Tue, 15 May 2018 16:26:57 -0400 Subject: [PATCH 5/6] fix: nested relations that match same model multiple times --- src/Architect.php | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/src/Architect.php b/src/Architect.php index 993f7c6..4530931 100644 --- a/src/Architect.php +++ b/src/Architect.php @@ -96,6 +96,14 @@ private function parseResource(array $modes, &$resource, &$root, $fullPropertyPa continue; } + // if we failed to load the relation and the resource is a + // string (the id), we can't do anything, so return + // FIXME: Find root cause of error "Indirect modification of overloaded property" + // (Note: encountered with relations 3 levels deep, with same model relating to multiple) + if ($mode === 'sideload' && getType($resource->{$property}) === 'string') { + return; + } + $object = &$resource->{$property}; } From ba7cbdf7a399cbbd9470c6a42f0d9c35bc5877dc Mon Sep 17 00:00:00 2001 From: Patrick McCarren Date: Tue, 21 Jan 2020 11:55:07 -0500 Subject: [PATCH 6/6] fix: overloaded property error --- src/Architect.php | 13 ++++--------- 1 file changed, 4 insertions(+), 9 deletions(-) diff --git a/src/Architect.php b/src/Architect.php index 4530931..12759a8 100644 --- a/src/Architect.php +++ b/src/Architect.php @@ -92,19 +92,14 @@ private function parseResource(array $modes, &$resource, &$root, $fullPropertyPa $object = &$resource[$property]; } else { - if ($resource->{$property} === null) { + if (!is_object($resource)) { continue; } - // if we failed to load the relation and the resource is a - // string (the id), we can't do anything, so return - // FIXME: Find root cause of error "Indirect modification of overloaded property" - // (Note: encountered with relations 3 levels deep, with same model relating to multiple) - if ($mode === 'sideload' && getType($resource->{$property}) === 'string') { - return; + $object = $resource->{$property}; + if (!is_object($object)) { + continue; } - - $object = &$resource->{$property}; } if (empty($steps)) {