Skip to content

Commit 83e2336

Browse files
JacobBrownAustinJacobBrownAustin
committed
ACPT-1186: Add ability to find non-changing mutable state in GraphQlStateTest
1 parent 92fa2db commit 83e2336

File tree

11 files changed

+410
-95
lines changed

11 files changed

+410
-95
lines changed

‎app/code/Magento/Customer/Model/ResourceModel/Customer.php

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -112,6 +112,7 @@ public function __construct(
112112
$this->storeManager = $storeManager;
113113
$this->encryptor = $encryptor ?? ObjectManager::getInstance()
114114
->get(EncryptorInterface::class);
115+
$this->getEntityIdField();
115116
}
116117

117118
/**

‎app/code/Magento/Eav/Model/Entity/Attribute/Source/Table.php

Lines changed: 7 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,7 @@
88
use Magento\Framework\App\ObjectManager;
99
use Magento\Framework\ObjectManager\ResetAfterRequestInterface;
1010
use Magento\Store\Model\StoreManagerInterface;
11+
use Magento\Store\Model\StoreManagerInterface\Proxy as StoreManagerInterfaceProxy;
1112

1213
/**
1314
* Eav attribute default source when values are coming from another table
@@ -42,14 +43,18 @@ class Table extends \Magento\Eav\Model\Entity\Attribute\Source\AbstractSource im
4243
/**
4344
* @param \Magento\Eav\Model\ResourceModel\Entity\Attribute\Option\CollectionFactory $attrOptionCollectionFactory
4445
* @param \Magento\Eav\Model\ResourceModel\Entity\Attribute\OptionFactory $attrOptionFactory
46+
* @param StoreManagerInterfaceProxy|null $StoreManagerInterfaceProxy
4547
* @codeCoverageIgnore
4648
*/
4749
public function __construct(
4850
\Magento\Eav\Model\ResourceModel\Entity\Attribute\Option\CollectionFactory $attrOptionCollectionFactory,
49-
\Magento\Eav\Model\ResourceModel\Entity\Attribute\OptionFactory $attrOptionFactory
51+
\Magento\Eav\Model\ResourceModel\Entity\Attribute\OptionFactory $attrOptionFactory,
52+
StoreManagerInterfaceProxy $StoreManagerInterfaceProxy = null
5053
) {
5154
$this->_attrOptionCollectionFactory = $attrOptionCollectionFactory;
5255
$this->_attrOptionFactory = $attrOptionFactory;
56+
$this->storeManager = $StoreManagerInterfaceProxy
57+
?? ObjectManager::getInstance()->get(StoreManagerInterfaceProxy::class);
5358
}
5459

5560
/**
@@ -63,7 +68,7 @@ public function getAllOptions($withEmpty = true, $defaultValues = false)
6368
{
6469
$storeId = $this->getAttribute()->getStoreId();
6570
if ($storeId === null) {
66-
$storeId = $this->getStoreManager()->getStore()->getId();
71+
$storeId = $this->storeManager->getStore()->getId();
6772
}
6873
if (!is_array($this->_options)) {
6974
$this->_options = [];
@@ -93,21 +98,6 @@ public function getAllOptions($withEmpty = true, $defaultValues = false)
9398
return $options;
9499
}
95100

96-
/**
97-
* Get StoreManager dependency
98-
*
99-
* @return StoreManagerInterface
100-
* @deprecated 100.1.6
101-
* @see we don't recommend this approach anymore
102-
*/
103-
private function getStoreManager()
104-
{
105-
if ($this->storeManager === null) {
106-
$this->storeManager = ObjectManager::getInstance()->get(StoreManagerInterface::class);
107-
}
108-
return $this->storeManager;
109-
}
110-
111101
/**
112102
* Retrieve Option values array by ids
113103
*

‎app/code/Magento/Theme/Model/Theme/ThemeProvider.php

Lines changed: 7 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -9,6 +9,7 @@
99
use Magento\Framework\Serialize\Serializer\Json;
1010
use Magento\Framework\View\Design\Theme\ListInterface;
1111
use Magento\Framework\App\DeploymentConfig;
12+
use Magento\Framework\App\DeploymentConfig\Proxy as DeploymentConfigProxy;
1213

1314
/**
1415
* Provide data for theme grid and for theme edit page
@@ -57,17 +58,21 @@ class ThemeProvider implements \Magento\Framework\View\Design\Theme\ThemeProvide
5758
* @param \Magento\Theme\Model\ThemeFactory $themeFactory
5859
* @param \Magento\Framework\App\CacheInterface $cache
5960
* @param Json $serializer
61+
* @param DeploymentConfigProxy|null $deploymentConfigProxy
6062
*/
6163
public function __construct(
6264
\Magento\Theme\Model\ResourceModel\Theme\CollectionFactory $collectionFactory,
6365
\Magento\Theme\Model\ThemeFactory $themeFactory,
6466
\Magento\Framework\App\CacheInterface $cache,
65-
Json $serializer = null
67+
Json $serializer = null,
68+
DeploymentConfigProxy $deploymentConfigProxy = null
6669
) {
6770
$this->collectionFactory = $collectionFactory;
6871
$this->themeFactory = $themeFactory;
6972
$this->cache = $cache;
7073
$this->serializer = $serializer ?: ObjectManager::getInstance()->get(Json::class);
74+
$this->deploymentConfig = $deploymentConfigProxy
75+
?? ObjectManager::getInstance()->get(DeploymentConfigProxy::class);
7176
}
7277

7378
/**
@@ -79,7 +84,7 @@ public function getThemeByFullPath($fullPath)
7984
return $this->themes[$fullPath];
8085
}
8186

82-
if (! $this->getDeploymentConfig()->isDbAvailable()) {
87+
if (! $this->deploymentConfig->isDbAvailable()) {
8388
return $this->getThemeList()->getThemeByFullPath($fullPath);
8489
}
8590

@@ -179,18 +184,4 @@ private function getThemeList()
179184
}
180185
return $this->themeList;
181186
}
182-
183-
/**
184-
* Get deployment config
185-
*
186-
* @deprecated 100.1.3
187-
* @return DeploymentConfig
188-
*/
189-
private function getDeploymentConfig()
190-
{
191-
if ($this->deploymentConfig === null) {
192-
$this->deploymentConfig = ObjectManager::getInstance()->get(DeploymentConfig::class);
193-
}
194-
return $this->deploymentConfig;
195-
}
196187
}

‎dev/tests/integration/framework/Magento/TestFramework/ObjectManager.php

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -77,7 +77,7 @@ private function clearMappedTableNames()
7777
$reflection = new \ReflectionClass($resourceConnection);
7878
$dataProperty = $reflection->getProperty('mappedTableNames');
7979
$dataProperty->setAccessible(true);
80-
$dataProperty->setValue($resourceConnection, null);
80+
$dataProperty->setValue($resourceConnection, []);
8181
}
8282
}
8383

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

Lines changed: 40 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -8,10 +8,12 @@
88
namespace Magento\GraphQl\App;
99

1010
use Magento\Framework\App\Http as HttpApp;
11+
use Magento\Framework\App\ObjectManager as AppObjectManager;
1112
use Magento\Framework\App\Request\HttpFactory as RequestFactory;
1213
use Magento\Framework\App\Response\Http as HttpResponse;
1314
use Magento\Framework\ObjectManagerInterface;
1415
use Magento\GraphQl\App\State\Comparator;
16+
use Magento\GraphQl\App\State\ObjectManager;
1517
use Magento\TestFramework\Helper\Bootstrap;
1618

1719
/**
@@ -30,7 +32,11 @@ class GraphQlStateTest extends \PHPUnit\Framework\TestCase
3032
private const CONTENT_TYPE = 'application/json';
3133

3234
/** @var ObjectManagerInterface */
33-
private ObjectManagerInterface $objectManager;
35+
private ObjectManagerInterface $objectManagerBeforeTest;
36+
37+
/** @var ObjectManagerInterface */
38+
private ObjectManagerInterface $objectManagerForTest;
39+
3440

3541
/** @var Comparator */
3642
private Comparator $comparator;
@@ -39,16 +45,34 @@ class GraphQlStateTest extends \PHPUnit\Framework\TestCase
3945
private RequestFactory $requestFactory;
4046

4147
/**
42-
* @return void
48+
* @inheritDoc
4349
*/
4450
protected function setUp(): void
4551
{
46-
$this->objectManager = Bootstrap::getObjectManager();
47-
$this->comparator = $this->objectManager->create(Comparator::class);
48-
$this->requestFactory = $this->objectManager->get(RequestFactory::class);
52+
$this->objectManagerBeforeTest = Bootstrap::getObjectManager();
53+
$this->objectManagerForTest = new ObjectManager($this->objectManagerBeforeTest);
54+
$this->objectManagerForTest->getFactory()->setObjectManager($this->objectManagerForTest);
55+
AppObjectManager::setInstance($this->objectManagerForTest);
56+
Bootstrap::setObjectManager($this->objectManagerForTest);
57+
$application = Bootstrap::getInstance()->getBootstrap()->getApplication();
58+
$application->reinitialize();
59+
$application->loadArea('graphql');
60+
$this->comparator = $this->objectManagerForTest->create(Comparator::class);
61+
$this->requestFactory = $this->objectManagerForTest->get(RequestFactory::class);
4962
parent::setUp();
5063
}
5164

65+
/**
66+
* @inheritDoc
67+
*/
68+
protected function tearDown(): void
69+
{
70+
$this->objectManagerBeforeTest->getFactory()->setObjectManager($this->objectManagerBeforeTest);
71+
AppObjectManager::setInstance($this->objectManagerBeforeTest);
72+
Bootstrap::setObjectManager($this->objectManagerBeforeTest);
73+
parent::tearDown();
74+
}
75+
5276
/**
5377
* Runs various GraphQL queries and checks if state of shared objects in Object Manager have changed
5478
*
@@ -95,6 +119,15 @@ private function request(string $query, string $operationName, bool $firstReques
95119
var_export($result, true)
96120
)
97121
);
122+
$result = $this->comparator->compareConstructedAgainstCurrent($operationName);
123+
$this->assertEmpty(
124+
$result,
125+
sprintf(
126+
'%d objects changed state since constructed. Details: %s',
127+
count($result),
128+
var_export($result, true)
129+
)
130+
);
98131
return $response;
99132
}
100133

@@ -111,8 +144,8 @@ private function doRequest(string $query)
111144
$request->setMethod('POST');
112145
$request->setPathInfo('/graphql');
113146
$request->getHeaders()->addHeaders(['content_type' => self::CONTENT_TYPE]);
114-
$unusedResponse = $this->objectManager->create(HttpResponse::class);
115-
$httpApp = $this->objectManager->create(
147+
$unusedResponse = $this->objectManagerForTest->create(HttpResponse::class);
148+
$httpApp = $this->objectManagerForTest->create(
116149
HttpApp::class,
117150
['request' => $request, 'response' => $unusedResponse]
118151
);

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

Lines changed: 52 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,7 @@
77

88
namespace Magento\GraphQl\App\State;
99

10+
use Magento\Framework\ObjectManager\ResetAfterRequestInterface;
1011
use Magento\Framework\ObjectManagerInterface;
1112

1213
/**
@@ -30,18 +31,24 @@ public function __construct(ObjectManagerInterface $objectManager)
3031
/**
3132
* Recursively clone objects in array.
3233
*
34+
* TODO: avoid infinite recursion when $array is cyclic.
35+
*
3336
* @param array $array
37+
* @param bool $callResetState
3438
* @return array
3539
*/
36-
private function cloneArray(array $array) : array
40+
private function cloneArray(array $array, bool $callResetState) : array
3741
{
3842
return array_map(
39-
function ($element) {
43+
function ($element) use ($callResetState) {
4044
if (is_object($element)) {
45+
if ($callResetState && $element instanceof ResetAfterRequestInterface) {
46+
$element->_resetState();
47+
}
4148
return clone $element;
4249
}
4350
if (is_array($element)) {
44-
return $this->cloneArray($element);
51+
return $this->cloneArray($element, $callResetState);
4552
}
4653
return $element;
4754
},
@@ -73,6 +80,9 @@ public function getSharedObjects(): array
7380
if ($object instanceof \Magento\Framework\ObjectManagerInterface) {
7481
continue;
7582
}
83+
if ($object instanceof ResetAfterRequestInterface) {
84+
$object->_resetState();
85+
}
7686
$properties = $this->getPropertiesFromObject($object, true, $didClone);
7787
$sharedObjects[$serviceName] = [$object, $properties];
7888
}
@@ -81,8 +91,42 @@ public function getSharedObjects(): array
8191
return $sharedObjects;
8292
}
8393

94+
/**
95+
* Gets all the objects' properties as they were originally constructed, and current, as well as object itself
96+
*
97+
* @return array
98+
*/
99+
public function getPropertiesConstructedAndCurrent(): array
100+
{
101+
/** @var ObjectManager $objectManager */
102+
$objectManager = $this->objectManager;
103+
if (!($objectManager instanceof ObjectManager)) {
104+
throw new \Exception("Not the correct type of ObjectManager");
105+
}
106+
// Calling _resetState helps us avoid adding skip/filter for these classes.
107+
foreach ($objectManager->getWeakMap() as $object => $propertiesBefore) {
108+
if ($object instanceof ResetAfterRequestInterface) {
109+
$object->_resetState();
110+
}
111+
unset($object);
112+
}
113+
/* Note: We must force garbage collection to clean up cyclic referenced objects after _resetState()
114+
Otherwise, they may still show up in the WeakMap. */
115+
$collectedCount = gc_collect_cycles();
116+
$objects = [];
117+
foreach ($objectManager->getWeakMap() as $object => $propertiesBefore) {
118+
$objects[] = [
119+
'object' => $object,
120+
'constructedProperties' => $propertiesBefore,
121+
'currentProperties' => $this->getPropertiesFromObject($object),
122+
];
123+
}
124+
return $objects;
125+
}
126+
84127
public function getPropertiesFromObject(object $object, $doClone = false, &$didClone = null): array
85128
{
129+
$callResetState = true;
86130
$objReflection = new \ReflectionObject($object);
87131
$properties = [];
88132
foreach ($objReflection->getProperties() as $property) {
@@ -95,10 +139,14 @@ public function getPropertiesFromObject(object $object, $doClone = false, &$didC
95139
}
96140
if (is_object($value)) {
97141
$didClone = true;
142+
if ($callResetState && $value instanceof ResetAfterRequestInterface) {
143+
// Calling _resetState helps us avoid adding skip/filter for these classes.
144+
$value->_resetState();
145+
}
98146
$properties[$propName] = clone $value;
99147
} elseif (is_array($value)) {
100148
$didClone = true;
101-
$properties[$propName] = $this->cloneArray($value);
149+
$properties[$propName] = $this->cloneArray($value, $callResetState);
102150
} else {
103151
$properties[$propName] = $value;
104152
}

0 commit comments

Comments
 (0)