Skip to content

Commit 28a524c

Browse files
JacobBrownAustinJacobBrownAustin
committed
ACPT-1186
Refactored to avoid cloning. Using recursive CollectedObject objects instead
1 parent 83e2336 commit 28a524c

File tree

12 files changed

+499
-249
lines changed

12 files changed

+499
-249
lines changed

‎dev/tests/integration/testsuite/Magento/Framework/ObjectManager/ResetAfterRequestTest.php

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -11,6 +11,7 @@
1111
use Magento\Framework\ObjectManagerInterface;
1212
use Magento\GraphQl\App\State\Collector;
1313
use Magento\GraphQl\App\State\Comparator;
14+
use Magento\GraphQl\App\State\CompareType;
1415

1516
/**
1617
* Test that verifies that resetState method for classes cause the state to be the same as it was initially constructed
@@ -148,9 +149,9 @@ public function testResetAfterRequestClasses(string $className)
148149
}
149150
try {
150151
/** @var ResetAfterRequestInterface $object */
151-
$beforeProperties = $this->collector->getPropertiesFromObject($object, true);
152+
$beforeProperties = $this->collector->getPropertiesFromObject($object, CompareType::CompareBetweenRequests);
152153
$object->_resetState();
153-
$afterProperties = $this->collector->getPropertiesFromObject($object, true);
154+
$afterProperties = $this->collector->getPropertiesFromObject($object, CompareType::CompareBetweenRequests);
154155
$differences = [];
155156
foreach ($afterProperties as $propertyName => $propertyValue) {
156157
if ($propertyValue instanceof ObjectManagerInterface) {

‎dev/tests/integration/testsuite/Magento/GraphQl/App/GraphQlStateTest.php

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -110,7 +110,7 @@ private function request(string $query, string $operationName, bool $firstReques
110110
$this->comparator->rememberObjectsStateBefore($firstRequest);
111111
$response = $this->doRequest($query);
112112
$this->comparator->rememberObjectsStateAfter($firstRequest);
113-
$result = $this->comparator->compare($operationName);
113+
$result = $this->comparator->compareBetweenRequests($operationName);
114114
$this->assertEmpty(
115115
$result,
116116
sprintf(

‎dev/tests/integration/testsuite/Magento/GraphQl/App/State/CollectedObject.php

Lines changed: 51 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,51 @@
1+
<?php
2+
/**
3+
* Copyright © Magento, Inc. All rights reserved.
4+
* See COPYING.txt for license details.
5+
*/
6+
declare(strict_types=1);
7+
8+
namespace Magento\GraphQl\App\State;
9+
10+
/**
11+
* Collects shared objects from ObjectManager and clones properties for later comparison
12+
*/
13+
class CollectedObject
14+
{
15+
public function __construct(
16+
private readonly string $className,
17+
private readonly array $properties,
18+
private readonly int $objectId,
19+
) {
20+
}
21+
22+
public function getClassName() : string
23+
{
24+
return $this->className;
25+
}
26+
27+
public function getProperties() : array
28+
{
29+
return $this->properties;
30+
}
31+
32+
public function getObjectId() : int
33+
{
34+
return $this->objectId;
35+
}
36+
37+
private static ?CollectedObject $skippedObject = null;
38+
public static function getSkippedObject() {
39+
if (!self::$skippedObject) {
40+
self::$skippedObject = new CollectedObject('(skipped)', [], 0);
41+
}
42+
return self::$skippedObject;
43+
}
44+
private static ?CollectedObject $recursionEndObject = null;
45+
public static function getRecursionEndObject() {
46+
if (!self::$recursionEndObject) {
47+
self::$recursionEndObject = new CollectedObject('(end of recursion level)', [], 0);
48+
}
49+
return self::$recursionEndObject;
50+
}
51+
}

‎dev/tests/integration/testsuite/Magento/GraphQl/App/State/CollectedObjectConstructedAndCurrent.php

Lines changed: 36 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,36 @@
1+
<?php
2+
/**
3+
* Copyright © Magento, Inc. All rights reserved.
4+
* See COPYING.txt for license details.
5+
*/
6+
declare(strict_types=1);
7+
8+
namespace Magento\GraphQl\App\State;
9+
10+
/**
11+
* Returned by Collector
12+
*/
13+
class CollectedObjectConstructedAndCurrent
14+
{
15+
public function __construct(
16+
private object $object,
17+
private CollectedObject $constructedCollected,
18+
private CollectedObject $currentCollected,
19+
) {
20+
}
21+
22+
public function getObject() : object
23+
{
24+
return $this->object;
25+
}
26+
27+
public function getConstructedCollected() : CollectedObject
28+
{
29+
return $this->constructedCollected;
30+
}
31+
32+
public function getCurrentCollected() : CollectedObject
33+
{
34+
return $this->currentCollected;
35+
}
36+
}

‎dev/tests/integration/testsuite/Magento/GraphQl/App/State/Collector.php

Lines changed: 105 additions & 67 deletions
Original file line numberDiff line numberDiff line change
@@ -11,44 +11,62 @@
1111
use Magento\Framework\ObjectManagerInterface;
1212

1313
/**
14-
* Collects shared objects from ObjectManager and clones properties for later comparison
14+
* Collects shared objects from ObjectManager and copies properties for later comparison
1515
*/
1616
class Collector
1717
{
18-
/**
19-
* @var ObjectManagerInterface
20-
*/
21-
private ObjectManagerInterface $objectManager;
18+
private array $skipListFromConstructed;
19+
private array $skipListBetweenRequests;
2220

23-
/**
24-
* @param ObjectManagerInterface $objectManager
25-
*/
26-
public function __construct(ObjectManagerInterface $objectManager)
27-
{
28-
$this->objectManager = $objectManager;
21+
public function __construct(
22+
private ObjectManagerInterface $objectManager,
23+
SkipListAndFilterList $skipListAndFilterList
24+
) {
25+
$this->skipListFromConstructed =
26+
$skipListAndFilterList->getSkipList('', CompareType::CompareConstructedAgainstCurrent);
27+
$this->skipListBetweenRequests = $skipListAndFilterList->getSkipList('', CompareType::CompareBetweenRequests);
2928
}
3029

3130
/**
32-
* Recursively clone objects in array.
33-
*
34-
* TODO: avoid infinite recursion when $array is cyclic.
31+
* Recursively copy objects in array.
3532
*
3633
* @param array $array
37-
* @param bool $callResetState
34+
* @param CompareType $compareType
35+
* @param int $recursionLevel
36+
* @param int $arrayRecursionLevel
3837
* @return array
3938
*/
40-
private function cloneArray(array $array, bool $callResetState) : array
39+
private function copyArray(
40+
array $array,
41+
CompareType $compareType,
42+
int $recursionLevel,
43+
int $arrayRecursionLevel = 100
44+
) : array
4145
{
4246
return array_map(
43-
function ($element) use ($callResetState) {
47+
function ($element) use (
48+
$compareType,
49+
$recursionLevel,
50+
$arrayRecursionLevel,
51+
) {
4452
if (is_object($element)) {
45-
if ($callResetState && $element instanceof ResetAfterRequestInterface) {
46-
$element->_resetState();
47-
}
48-
return clone $element;
53+
return $this->getPropertiesFromObject(
54+
$element,
55+
$compareType,
56+
$recursionLevel - 1,
57+
);
4958
}
5059
if (is_array($element)) {
51-
return $this->cloneArray($element, $callResetState);
60+
if ($arrayRecursionLevel) {
61+
return $this->copyArray(
62+
$element,
63+
$compareType,
64+
$recursionLevel,
65+
$arrayRecursionLevel - 1,
66+
);
67+
} else {
68+
return '(end of array recursion level)';
69+
}
5270
}
5371
return $element;
5472
},
@@ -59,42 +77,41 @@ function ($element) use ($callResetState) {
5977
/**
6078
* Gets shared objects from ObjectManager using reflection and clones properties that are objects
6179
*
62-
* @return array
63-
* @throws \Exception
80+
* @param ShouldResetState $shouldResetState
81+
* @return CollectedObject[]
6482
*/
65-
public function getSharedObjects(): array
83+
public function getSharedObjects(ShouldResetState $shouldResetState): array
6684
{
6785
$sharedObjects = [];
6886
$obj = new \ReflectionObject($this->objectManager);
6987
if (!$obj->hasProperty('_sharedInstances')) {
7088
throw new \Exception('Cannot get shared objects from ' . get_class($this->objectManager));
7189
}
72-
do {
73-
$property = $obj->getProperty('_sharedInstances');
74-
$property->setAccessible(true);
75-
$didClone = false;
76-
foreach ($property->getValue($this->objectManager) as $serviceName => $object) {
77-
if (array_key_exists($serviceName, $sharedObjects)) {
78-
continue;
79-
}
80-
if ($object instanceof \Magento\Framework\ObjectManagerInterface) {
81-
continue;
82-
}
83-
if ($object instanceof ResetAfterRequestInterface) {
84-
$object->_resetState();
85-
}
86-
$properties = $this->getPropertiesFromObject($object, true, $didClone);
87-
$sharedObjects[$serviceName] = [$object, $properties];
90+
$property = $obj->getProperty('_sharedInstances');
91+
$property->setAccessible(true);
92+
foreach ($property->getValue($this->objectManager) as $serviceName => $object) {
93+
if (array_key_exists($serviceName, $sharedObjects)) {
94+
continue;
95+
}
96+
if (ShouldResetState::DoResetState == $shouldResetState &&
97+
($object instanceof ResetAfterRequestInterface)) {
98+
$object->_resetState();
8899
}
89-
// Note: We have to check again because sometimes cloning objects can indirectly cause adding to Object Manager
90-
} while ($didClone);
100+
if ($object instanceof \Magento\Framework\ObjectManagerInterface) {
101+
continue;
102+
}
103+
$sharedObjects[$serviceName] =
104+
$this->getPropertiesFromObject($object, CompareType::CompareBetweenRequests);
105+
}
91106
return $sharedObjects;
92107
}
93108

94109
/**
95110
* Gets all the objects' properties as they were originally constructed, and current, as well as object itself
96111
*
97-
* @return array
112+
* This also calls _resetState on any ResetAfterRequestInterface
113+
*
114+
* @return CollectedObjectConstructedAndCurrent[]
98115
*/
99116
public function getPropertiesConstructedAndCurrent(): array
100117
{
@@ -115,42 +132,63 @@ public function getPropertiesConstructedAndCurrent(): array
115132
$collectedCount = gc_collect_cycles();
116133
$objects = [];
117134
foreach ($objectManager->getWeakMap() as $object => $propertiesBefore) {
118-
$objects[] = [
119-
'object' => $object,
120-
'constructedProperties' => $propertiesBefore,
121-
'currentProperties' => $this->getPropertiesFromObject($object),
122-
];
135+
$objects[] = new CollectedObjectConstructedAndCurrent(
136+
$object,
137+
$propertiesBefore,
138+
$this->getPropertiesFromObject($object, CompareType::CompareConstructedAgainstCurrent),
139+
);
123140
}
124141
return $objects;
125142
}
126143

127-
public function getPropertiesFromObject(object $object, $doClone = false, &$didClone = null): array
128-
{
129-
$callResetState = true;
144+
/**
145+
* Gets properties from object and returns CollectedObject
146+
*
147+
* @param object $object
148+
* @param CompareType $compareType
149+
* @param int $recursionLevel
150+
* @return CollectedObject
151+
*/
152+
public function getPropertiesFromObject(
153+
object $object,
154+
CompareType $compareType,
155+
int $recursionLevel = 1,
156+
): CollectedObject {
157+
if ($recursionLevel < 0) {
158+
return CollectedObject::getRecursionEndObject();
159+
}
160+
$className = get_class($object);
161+
$skipList = $compareType == CompareType::CompareBetweenRequests ?
162+
$this->skipListBetweenRequests : $this->skipListFromConstructed ;
163+
if (array_key_exists($className, $skipList)) {
164+
return CollectedObject::getSkippedObject();
165+
}
130166
$objReflection = new \ReflectionObject($object);
131167
$properties = [];
132168
foreach ($objReflection->getProperties() as $property) {
133-
$propName = $property->getName();
169+
$propertyName = $property->getName();
134170
$property->setAccessible(true);
135171
$value = $property->getValue($object);
136-
if (!$doClone) {
137-
$properties[$propName] = $value;
138-
continue;
139-
}
140172
if (is_object($value)) {
141-
$didClone = true;
142-
if ($callResetState && $value instanceof ResetAfterRequestInterface) {
143-
// Calling _resetState helps us avoid adding skip/filter for these classes.
144-
$value->_resetState();
145-
}
146-
$properties[$propName] = clone $value;
173+
$properties[$propertyName] = $this->getPropertiesFromObject(
174+
$value,
175+
$compareType,
176+
$recursionLevel - 1,
177+
);
147178
} elseif (is_array($value)) {
148-
$didClone = true;
149-
$properties[$propName] = $this->cloneArray($value, $callResetState);
179+
$properties[$propertyName] = $this->copyArray(
180+
$value,
181+
$compareType,
182+
$recursionLevel,
183+
);
150184
} else {
151-
$properties[$propName] = $value;
185+
$properties[$propertyName] = $value;
152186
}
153187
}
154-
return $properties;
188+
return new CollectedObject(
189+
$className,
190+
$properties,
191+
spl_object_id($object),
192+
);
155193
}
156194
}

0 commit comments

Comments
 (0)