ACPT-1186

* fixing static test failures
* adding "readonly" to more member variable definitions
* use proper reset method for objects in the weakMap

Author: JacobBrownAustin

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

@@ -3,6 +3,8 @@
  * Copyright © Magento, Inc. All rights reserved.
  * See COPYING.txt for license details.
  */
+declare(strict_types=1);
+
 namespace Magento\Framework\ObjectManager;
 
 use Magento\Framework\App\Utility\Classes;
@@ -26,7 +28,7 @@ class ResetAfterRequestTest extends \PHPUnit\Framework\TestCase
     private ?Collector $collector;
 
     /**
-     * @return void
+     * @inheritDoc
      */
     protected function setUp(): void
     {

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

@@ -8,7 +8,7 @@
 namespace Magento\GraphQl\App\State;
 
 /**
- * Collects shared objects from ObjectManager and clones properties for later comparison
+ * Immutable recursive data structure that holds copy of properties from collected objects.  Created by Collector.
  */
 class CollectedObject
 {

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

@@ -13,9 +13,9 @@
 class CollectedObjectConstructedAndCurrent
 {
     public function __construct(
-        private object $object,
-        private CollectedObject $constructedCollected,
-        private CollectedObject $currentCollected,
+        private readonly object $object,
+        private readonly CollectedObject $constructedCollected,
+        private readonly CollectedObject $currentCollected,
     ) {
     }
 

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

@@ -15,13 +15,13 @@
  */
 class Collector
 {
-    private array $skipListFromConstructed;
-    private array $skipListBetweenRequests;
+    private readonly array $skipListFromConstructed;
+    private readonly array $skipListBetweenRequests;
 
     /** @var ObjectManager $objectManager Note: Using ObjectManagerInterface for DI to get correct instance */
 
     public function __construct(
-        private ObjectManagerInterface $objectManager,
+        private readonly ObjectManagerInterface $objectManager,
         SkipListAndFilterList $skipListAndFilterList
     ) {
         $this->skipListFromConstructed =
@@ -116,12 +116,7 @@ public function getPropertiesConstructedAndCurrent(): array
             throw new \Exception("Not the correct type of ObjectManager");
         }
         // Calling _resetState helps us avoid adding skip/filter for these classes.
-        foreach ($objectManager->getWeakMap() as $object => $propertiesBefore) {
-            if ($object instanceof ResetAfterRequestInterface) {
-                $object->_resetState();
-            }
-            unset($object);
-        }
+        $objectManager->resetStateWeakMapObjects();
         /* Note: We must force garbage collection to clean up cyclic referenced objects after _resetState()
         Otherwise, they may still show up in the WeakMap. */
         gc_collect_cycles();
@@ -143,6 +138,7 @@ public function getPropertiesConstructedAndCurrent(): array
      * @param CompareType $compareType
      * @param int $recursionLevel
      * @return CollectedObject
+     * @SuppressWarnings(PHPMD.CyclomaticComplexity)
      */
     public function getPropertiesFromObject(
         object $object,

dev/tests/integration/testsuite/Magento/GraphQl/App/State/Comparator.php

@@ -8,7 +8,7 @@
 namespace Magento\GraphQl\App\State;
 
 /**
- * Compare object state between requests
+ * Compare object state between requests and between first instantiation by ObjectManager
  */
 class Comparator
 {
@@ -20,8 +20,10 @@ class Comparator
      */
     private array $objectsStateAfter = [];
 
-    public function __construct(private Collector $collector, private SkipListAndFilterList $skipListAndFilterList)
-    {
+    public function __construct(
+        private readonly Collector $collector,
+        private readonly SkipListAndFilterList $skipListAndFilterList
+    ) {
     }
 
     /**
@@ -94,10 +96,6 @@ public function compareConstructedAgainstCurrent(string $operationName): array
         $compareResults = [];
         $skipList = $this->skipListAndFilterList
             ->getSkipList($operationName, CompareType::CompareConstructedAgainstCurrent);
-        $filterList = $this->skipListAndFilterList->getFilterList();
-        $filterListParentClasses = $filterList['parents'] ?? [];
-        $filterListServices = $filterList['services'] ?? [];
-        $filterListAll = $filterList['all'] ?? [];
         foreach ($this->collector->getPropertiesConstructedAndCurrent() as $objectAndProperties) {
             $object = $objectAndProperties->getObject();
             $constructedObject = $objectAndProperties->getConstructedCollected();
@@ -128,6 +126,7 @@ public function compareConstructedAgainstCurrent(string $operationName): array
      * @param string $serviceName
      * @return array
      * @SuppressWarnings(PHPMD.CyclomaticComplexity)
+     * @SuppressWarnings(PHPMD.NPathComplexity)
      */
     private function compare(
         CollectedObject $before,

dev/tests/integration/testsuite/Magento/GraphQl/App/State/DynamicFactoryDecorator.php

@@ -13,14 +13,21 @@
 use WeakReference;
 
 /**
- * Dynamic Factory Decorator for State test.
+ * Dynamic Factory Decorator for State test.  Stores collected properties from created objects in WeakMap
  */
 class DynamicFactoryDecorator extends Developer implements ResetAfterRequestInterface
 {
-    private WeakMap $weakMap; // values are CollectedObject
-    private Collector $collector;
-    private array $skipList;
+    /** @var WeakMap $weakMap Where CollectedObject is stored after object is created by us */
+    private WeakMap $weakMap;
+    private readonly Collector $collector;
+    private readonly array $skipList;
 
+    /**
+     * Constructs this instance by copying $developer
+     *
+     * @param Developer $developer
+     * @param ObjectManager $objectManager
+     */
     public function __construct(Developer $developer, ObjectManager $objectManager)
     {
         /* Note: PHP doesn't have copy constructors, so we have to use get_object_vars,
@@ -59,17 +66,19 @@ public function create($type, array $arguments = [])
      */
     public function _resetState(): void
     {
-        /* Note: we can't iterate resetAfterWeakMap itself because it gets indirectly modified (shrinks) as some of the
+        /* Note: we can't iterate weakMap itself because it gets indirectly modified (shrinks) as some of the
          * service classes that get reset will destruct some of the other service objects.  The iterator to WeakMap
          * returns actual objects, not WeakReferences.  Therefore, we create a temporary list of weak references which
          *  is safe to iterate. */
-        $temporaryWeakReferenceList = [];
-        foreach($this->weakMap as $weakMapObject => $value) {
-            $temporaryWeakReferenceList[] = WeakReference::create($weakMapObject);
+        $weakReferenceListToReset = [];
+        foreach ($this->weakMap as $weakMapObject => $value) {
+            if ($weakMapObject instanceof ResetAfterRequestInterface) {
+                $weakReferenceListToReset[] = WeakReference::create($weakMapObject);
+            }
             unset($weakMapObject);
             unset($value);
         }
-        foreach ($temporaryWeakReferenceList as $weakReference) {
+        foreach ($weakReferenceListToReset as $weakReference) {
             $object = $weakReference->get();
             if (!$object) {
                 continue;

dev/tests/integration/testsuite/Magento/GraphQl/App/State/ObjectManager.php

@@ -12,7 +12,7 @@
 use Weakmap;
 
 /**
- * Collects shared objects from ObjectManager and clones properties for later comparison
+ * ObjectManager decorator used by GraphQlStateTest for resetting objects and getting initial properties from objects
  */
 class ObjectManager extends TestFrameworkObjectManager
 {