From 307b35db424fd90916fb7b36a31a69863769b54f Mon Sep 17 00:00:00 2001 From: Andrii Kasian Date: Thu, 14 Dec 2023 18:10:45 -0600 Subject: [PATCH 01/12] Add 3rd part reset capability --- .../ObjectManager/Resetter/Resetter.php | 58 ++++++++++++------- .../Resetter/ResetterFactory.php | 13 +++-- .../ApplicationStateComparator/Resetter.php | 5 +- 3 files changed, 48 insertions(+), 28 deletions(-) diff --git a/lib/internal/Magento/Framework/ObjectManager/Resetter/Resetter.php b/lib/internal/Magento/Framework/ObjectManager/Resetter/Resetter.php index 1a3282c990414..7e6baf9e948c2 100644 --- a/lib/internal/Magento/Framework/ObjectManager/Resetter/Resetter.php +++ b/lib/internal/Magento/Framework/ObjectManager/Resetter/Resetter.php @@ -8,6 +8,8 @@ namespace Magento\Framework\ObjectManager\Resetter; use Magento\Framework\App\ObjectManager; +use Magento\Framework\Component\ComponentRegistrar; +use Magento\Framework\Component\ComponentRegistrarInterface; use Magento\Framework\ObjectManager\ResetAfterRequestInterface; use Magento\Framework\ObjectManagerInterface; use WeakMap; @@ -17,7 +19,8 @@ */ class Resetter implements ResetterInterface { - public const RESET_PATH = '/app/etc/reset.php'; + public const RESET_PATH = 'reset.json'; + private const RESET_STATE_METHOD = '_resetState'; /** @var WeakMap instances to be reset after request */ private WeakMap $resetAfterWeakMap; @@ -28,21 +31,6 @@ class Resetter implements ResetterInterface /** @var WeakMapSorter|null Note: We use temporal coupling here because of chicken/egg during bootstrapping */ private ?WeakMapSorter $weakMapSorter = null; - /** - * @var array - * - */ - private array $classList = [ - //phpcs:disable Magento2.PHP.LiteralNamespaces - 'Magento\Framework\GraphQl\Query\Fields' => true, - 'Magento\Store\Model\Store' => [ - "_baseUrlCache" => [], - "_configCache" => null, - "_configCacheBaseNodes" => [], - "_dirCache" => [], - "_urlCache" => [] - ] - ]; /** * @var array @@ -55,15 +43,34 @@ class Resetter implements ResetterInterface * @return void * @phpcs:disable Magento2.Functions.DiscouragedFunction */ - public function __construct() - { - if (\file_exists(BP . self::RESET_PATH)) { - // phpcs:ignore Magento2.Security.IncludeFile.FoundIncludeFile - $this->classList = array_replace($this->classList, (require BP . self::RESET_PATH)); + public function __construct( + private ComponentRegistrarInterface $componentRegistrar, + private array $classList = [], + ) { + foreach ($this->getPaths() as $resetPath) { + if (!\file_exists($resetPath)) { + continue; + } + $resetData = \json_decode(\file_get_contents($resetPath), true); + $this->classList = array_replace($this->classList, $resetData); } $this->resetAfterWeakMap = new WeakMap; } + /** + * Get paths for reset json + * + * @return \Generator + */ + private function getPaths(): \Generator + { + yield BP . '/app/etc/' . self::RESET_PATH; + foreach ($this->componentRegistrar->getPaths(ComponentRegistrar::MODULE) as $modulePath) { + yield $modulePath . '/etc/' . self::RESET_PATH; + } + } + + /** * Add instance to be reset later * @@ -72,7 +79,10 @@ public function __construct() */ public function addInstance(object $instance) : void { - if ($instance instanceof ResetAfterRequestInterface || isset($this->classList[\get_class($instance)])) { + if ($instance instanceof ResetAfterRequestInterface + || \method_exists($instance, self::RESET_STATE_METHOD) + || isset($this->classList[\get_class($instance)]) + ) { $this->resetAfterWeakMap[$instance] = true; } } @@ -124,6 +134,10 @@ public function setObjectManager(ObjectManagerInterface $objectManager) : void */ private function resetStateWithReflection(object $instance) { + if (\method_exists($instance, self::RESET_STATE_METHOD)) { + $instance->{self::RESET_STATE_METHOD}(); + return; + } $className = \get_class($instance); $reflectionClass = $this->reflectionCache[$className] ?? $this->reflectionCache[$className] = new \ReflectionClass($className); diff --git a/lib/internal/Magento/Framework/ObjectManager/Resetter/ResetterFactory.php b/lib/internal/Magento/Framework/ObjectManager/Resetter/ResetterFactory.php index 78d6caa630522..21f17feb1f8c8 100644 --- a/lib/internal/Magento/Framework/ObjectManager/Resetter/ResetterFactory.php +++ b/lib/internal/Magento/Framework/ObjectManager/Resetter/ResetterFactory.php @@ -7,6 +7,8 @@ namespace Magento\Framework\ObjectManager\Resetter; +use Magento\Framework\ObjectManagerInterface; + /** * Factory that creates Resetter based on environment variable. */ @@ -17,15 +19,18 @@ class ResetterFactory */ private static string $resetterClassName = Resetter::class; + public function __construct(private ObjectManagerInterface $objectManager) + { + } + /** - * Create resseter factory + * Create resseter instance * * @return ResetterInterface - * @phpcs:disable Magento2.Functions.StaticFunction */ - public static function create() : ResetterInterface + public function create() : ResetterInterface { - return new static::$resetterClassName; + return $this->objectManager->create(static::$resetterClassName); } /** diff --git a/lib/internal/Magento/Framework/TestFramework/ApplicationStateComparator/Resetter.php b/lib/internal/Magento/Framework/TestFramework/ApplicationStateComparator/Resetter.php index ddc190597d024..022f8ca32432d 100644 --- a/lib/internal/Magento/Framework/TestFramework/ApplicationStateComparator/Resetter.php +++ b/lib/internal/Magento/Framework/TestFramework/ApplicationStateComparator/Resetter.php @@ -7,6 +7,7 @@ namespace Magento\Framework\TestFramework\ApplicationStateComparator; +use Magento\Framework\Component\ComponentRegistrarInterface; use Magento\Framework\ObjectManager\Resetter\Resetter as OriginalResetter; use Magento\Framework\ObjectManagerInterface; use WeakMap; @@ -37,11 +38,11 @@ class Resetter extends OriginalResetter * @param array $classList * @return void */ - public function __construct() + public function __construct(ComponentRegistrarInterface $componentRegistrar, array $classList = []) { $this->collectedWeakMap = new WeakMap; $this->skipListAndFilterList = new SkipListAndFilterList; - parent::__construct(); + parent::__construct($componentRegistrar, $classList); } /** From b16b3f207bf0fdf419d439ee67be8fff8fb9c2b8 Mon Sep 17 00:00:00 2001 From: Andrii Dimov Date: Sat, 10 Feb 2024 21:36:54 -0600 Subject: [PATCH 02/12] ACPT-1493: Extension and SaaS Services compatibility -- fix test failures --- .../Magento/Framework/ObjectManager/Resetter/Resetter.php | 6 ++---- .../Framework/ObjectManager/Resetter/ResetterFactory.php | 3 +++ 2 files changed, 5 insertions(+), 4 deletions(-) diff --git a/lib/internal/Magento/Framework/ObjectManager/Resetter/Resetter.php b/lib/internal/Magento/Framework/ObjectManager/Resetter/Resetter.php index 7e6baf9e948c2..56da3cd9dd6ac 100644 --- a/lib/internal/Magento/Framework/ObjectManager/Resetter/Resetter.php +++ b/lib/internal/Magento/Framework/ObjectManager/Resetter/Resetter.php @@ -7,7 +7,6 @@ namespace Magento\Framework\ObjectManager\Resetter; -use Magento\Framework\App\ObjectManager; use Magento\Framework\Component\ComponentRegistrar; use Magento\Framework\Component\ComponentRegistrarInterface; use Magento\Framework\ObjectManager\ResetAfterRequestInterface; @@ -38,8 +37,8 @@ class Resetter implements ResetterInterface private array $reflectionCache = []; /** - * Constructor - * + * @param ComponentRegistrarInterface $componentRegistrar + * @param array $classList * @return void * @phpcs:disable Magento2.Functions.DiscouragedFunction */ @@ -70,7 +69,6 @@ private function getPaths(): \Generator } } - /** * Add instance to be reset later * diff --git a/lib/internal/Magento/Framework/ObjectManager/Resetter/ResetterFactory.php b/lib/internal/Magento/Framework/ObjectManager/Resetter/ResetterFactory.php index 21f17feb1f8c8..ca65c21787766 100644 --- a/lib/internal/Magento/Framework/ObjectManager/Resetter/ResetterFactory.php +++ b/lib/internal/Magento/Framework/ObjectManager/Resetter/ResetterFactory.php @@ -19,6 +19,9 @@ class ResetterFactory */ private static string $resetterClassName = Resetter::class; + /** + * @param ObjectManagerInterface $objectManager + */ public function __construct(private ObjectManagerInterface $objectManager) { } From 625c38f1ea08e2c3e0fd3f993a5fb06020c7dba4 Mon Sep 17 00:00:00 2001 From: Jacob Brown Date: Tue, 25 Jun 2024 08:23:54 -0500 Subject: [PATCH 03/12] ACPT-1929: Fixing race conditions in ACPT-1493 --- .../Framework/ObjectManager/Resetter/Resetter.php | 7 +++++-- .../ObjectManager/Resetter/ResetterFactory.php | 14 +++----------- .../ApplicationStateComparator/Resetter.php | 4 ++-- 3 files changed, 10 insertions(+), 15 deletions(-) diff --git a/lib/internal/Magento/Framework/ObjectManager/Resetter/Resetter.php b/lib/internal/Magento/Framework/ObjectManager/Resetter/Resetter.php index 56da3cd9dd6ac..77433de1907b8 100644 --- a/lib/internal/Magento/Framework/ObjectManager/Resetter/Resetter.php +++ b/lib/internal/Magento/Framework/ObjectManager/Resetter/Resetter.php @@ -37,15 +37,18 @@ class Resetter implements ResetterInterface private array $reflectionCache = []; /** - * @param ComponentRegistrarInterface $componentRegistrar + * @param ComponentRegistrarInterface|null $componentRegistrar * @param array $classList * @return void * @phpcs:disable Magento2.Functions.DiscouragedFunction */ public function __construct( - private ComponentRegistrarInterface $componentRegistrar, + private ?ComponentRegistrarInterface $componentRegistrar = null, private array $classList = [], ) { + if (null === $componentRegistrar) { + $componentRegistrar = new ComponentRegistrar(); + } foreach ($this->getPaths() as $resetPath) { if (!\file_exists($resetPath)) { continue; diff --git a/lib/internal/Magento/Framework/ObjectManager/Resetter/ResetterFactory.php b/lib/internal/Magento/Framework/ObjectManager/Resetter/ResetterFactory.php index ca65c21787766..4b3a81d2a326f 100644 --- a/lib/internal/Magento/Framework/ObjectManager/Resetter/ResetterFactory.php +++ b/lib/internal/Magento/Framework/ObjectManager/Resetter/ResetterFactory.php @@ -7,8 +7,6 @@ namespace Magento\Framework\ObjectManager\Resetter; -use Magento\Framework\ObjectManagerInterface; - /** * Factory that creates Resetter based on environment variable. */ @@ -19,21 +17,15 @@ class ResetterFactory */ private static string $resetterClassName = Resetter::class; - /** - * @param ObjectManagerInterface $objectManager - */ - public function __construct(private ObjectManagerInterface $objectManager) - { - } - /** * Create resseter instance * * @return ResetterInterface + * @phpcs:disable Magento2.Functions.StaticFunction */ - public function create() : ResetterInterface + public static function create() : ResetterInterface { - return $this->objectManager->create(static::$resetterClassName); + return new static::$resetterClassName; } /** diff --git a/lib/internal/Magento/Framework/TestFramework/ApplicationStateComparator/Resetter.php b/lib/internal/Magento/Framework/TestFramework/ApplicationStateComparator/Resetter.php index 022f8ca32432d..5af9589a97d97 100644 --- a/lib/internal/Magento/Framework/TestFramework/ApplicationStateComparator/Resetter.php +++ b/lib/internal/Magento/Framework/TestFramework/ApplicationStateComparator/Resetter.php @@ -34,11 +34,11 @@ class Resetter extends OriginalResetter /** * Constructor * - * @param ComponentRegistrarInterface $componentRegistrar + * @param ComponentRegistrarInterface|null $componentRegistrar * @param array $classList * @return void */ - public function __construct(ComponentRegistrarInterface $componentRegistrar, array $classList = []) + public function __construct(?ComponentRegistrarInterface $componentRegistrar = null, array $classList = []) { $this->collectedWeakMap = new WeakMap; $this->skipListAndFilterList = new SkipListAndFilterList; From 6b6084cacd05cc37c8d26444c53e20dc1018e431 Mon Sep 17 00:00:00 2001 From: Jacob Brown Date: Tue, 25 Jun 2024 12:10:47 -0500 Subject: [PATCH 04/12] ACPT-1929: Fixing race conditions in ACPT-1493 Fixing bug in my last commit --- .../Magento/Framework/ObjectManager/Resetter/Resetter.php | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/lib/internal/Magento/Framework/ObjectManager/Resetter/Resetter.php b/lib/internal/Magento/Framework/ObjectManager/Resetter/Resetter.php index 77433de1907b8..4257acb520bf8 100644 --- a/lib/internal/Magento/Framework/ObjectManager/Resetter/Resetter.php +++ b/lib/internal/Magento/Framework/ObjectManager/Resetter/Resetter.php @@ -30,7 +30,6 @@ class Resetter implements ResetterInterface /** @var WeakMapSorter|null Note: We use temporal coupling here because of chicken/egg during bootstrapping */ private ?WeakMapSorter $weakMapSorter = null; - /** * @var array */ @@ -46,8 +45,8 @@ public function __construct( private ?ComponentRegistrarInterface $componentRegistrar = null, private array $classList = [], ) { - if (null === $componentRegistrar) { - $componentRegistrar = new ComponentRegistrar(); + if (null === $this->componentRegistrar) { + $this->componentRegistrar = new ComponentRegistrar(); } foreach ($this->getPaths() as $resetPath) { if (!\file_exists($resetPath)) { From 781f34026eb80a3848983a1d6a1b168e4fdda0b3 Mon Sep 17 00:00:00 2001 From: Jacob Brown Date: Wed, 26 Jun 2024 10:55:44 -0500 Subject: [PATCH 05/12] ACPT-1929: Fixing race conditions in ACPT-1493 Adding exclicit exception for when JSON file is corrupt. This will give more useful error messages if people make broken json files. --- .../Magento/Framework/ObjectManager/Resetter/Resetter.php | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/lib/internal/Magento/Framework/ObjectManager/Resetter/Resetter.php b/lib/internal/Magento/Framework/ObjectManager/Resetter/Resetter.php index 4257acb520bf8..020f8fc1673b2 100644 --- a/lib/internal/Magento/Framework/ObjectManager/Resetter/Resetter.php +++ b/lib/internal/Magento/Framework/ObjectManager/Resetter/Resetter.php @@ -9,6 +9,7 @@ use Magento\Framework\Component\ComponentRegistrar; use Magento\Framework\Component\ComponentRegistrarInterface; +use Magento\Framework\Exception\LocalizedException; use Magento\Framework\ObjectManager\ResetAfterRequestInterface; use Magento\Framework\ObjectManagerInterface; use WeakMap; @@ -53,6 +54,9 @@ public function __construct( continue; } $resetData = \json_decode(\file_get_contents($resetPath), true); + if (!$resetData) { + throw new LocalizedException(__('Error parsing %1', $resetPath)); + } $this->classList = array_replace($this->classList, $resetData); } $this->resetAfterWeakMap = new WeakMap; From 77f1c9e7026b7f48a76e32097f57fdf844a1c597 Mon Sep 17 00:00:00 2001 From: Jacob Brown Date: Thu, 27 Jun 2024 16:29:22 -0500 Subject: [PATCH 06/12] ACPT-1929: Fixing issuls in ACPT-1493 * fixing inheritance issues * fixing issues with types (by removing feature) --- .../ObjectManager/Resetter/Resetter.php | 44 ++++++++++--------- 1 file changed, 24 insertions(+), 20 deletions(-) diff --git a/lib/internal/Magento/Framework/ObjectManager/Resetter/Resetter.php b/lib/internal/Magento/Framework/ObjectManager/Resetter/Resetter.php index 020f8fc1673b2..07cad6ecef6af 100644 --- a/lib/internal/Magento/Framework/ObjectManager/Resetter/Resetter.php +++ b/lib/internal/Magento/Framework/ObjectManager/Resetter/Resetter.php @@ -57,7 +57,7 @@ public function __construct( if (!$resetData) { throw new LocalizedException(__('Error parsing %1', $resetPath)); } - $this->classList = array_replace($this->classList, $resetData); + $this->classList += $resetData; } $this->resetAfterWeakMap = new WeakMap; } @@ -85,7 +85,7 @@ public function addInstance(object $instance) : void { if ($instance instanceof ResetAfterRequestInterface || \method_exists($instance, self::RESET_STATE_METHOD) - || isset($this->classList[\get_class($instance)]) + || $this->isObjectInClassList($instance) ) { $this->resetAfterWeakMap[$instance] = true; } @@ -129,8 +129,18 @@ public function setObjectManager(ObjectManagerInterface $objectManager) : void $this->objectManager = $objectManager; } + public function isObjectInClassList(object $object) + { + foreach ($this->classList as $key => $value) { + if ($object instanceof $key) { + return true; + } + } + return false; + } + /** - * State reset without reflection + * State reset using reflection (or RESET_STATE_METHOD instead if it exists) * * @param object $instance * @return void @@ -142,29 +152,23 @@ private function resetStateWithReflection(object $instance) $instance->{self::RESET_STATE_METHOD}(); return; } - $className = \get_class($instance); + foreach ($this->classList as $className => $value) { + if ($instance instanceof $className) { + $this->resetStateWithReflectionByClassName($instance, $className); + } + } + } + private function resetStateWithReflectionByClassName(object $instance, string $className) + { + $classResetValues = $this->classList[$className] ?? []; $reflectionClass = $this->reflectionCache[$className] ?? $this->reflectionCache[$className] = new \ReflectionClass($className); foreach ($reflectionClass->getProperties() as $property) { - $type = $property->getType()?->getName(); - if (empty($type) && preg_match('/@var\s+([^\s]+)/', $property->getDocComment(), $matches)) { - $type = $matches[1]; - if (\str_contains($type, '[]')) { - $type = 'array'; - } - } $name = $property->getName(); - if (!in_array($type, ['bool', 'array', 'null', 'true', 'false'], true)) { + if (!array_key_exists($name, $classResetValues)) { continue; } - $value = $this->classList[$className][$name] ?? - match ($type) { - 'bool' => false, - 'true' => true, - 'false' => false, - 'array' => [], - 'null' => null, - }; + $value = $classResetValues[$name]; $property->setAccessible(true); $property->setValue($instance, $value); $property->setAccessible(false); From 01ac7ceea30f43a4c7308a5375b6bdaa9fb6a9eb Mon Sep 17 00:00:00 2001 From: Jacob Brown Date: Thu, 27 Jun 2024 16:33:19 -0500 Subject: [PATCH 07/12] ACPT-1929: .... Fixing missing extends ResetAfterRequestInterface in two classes --- app/code/Magento/GraphQl/Model/Query/Resolver/Context.php | 3 ++- lib/internal/Magento/Framework/Url/QueryParamsResolver.php | 5 ++++- 2 files changed, 6 insertions(+), 2 deletions(-) diff --git a/app/code/Magento/GraphQl/Model/Query/Resolver/Context.php b/app/code/Magento/GraphQl/Model/Query/Resolver/Context.php index bae3ceabf2783..6b436de3e53b4 100644 --- a/app/code/Magento/GraphQl/Model/Query/Resolver/Context.php +++ b/app/code/Magento/GraphQl/Model/Query/Resolver/Context.php @@ -16,7 +16,8 @@ * @deprecated 100.3.3 * @see \Magento\GraphQl\Model\Query\Context */ -class Context extends \Magento\Framework\Model\AbstractExtensibleModel implements ContextInterface +class Context extends \Magento\Framework\Model\AbstractExtensibleModel implements ContextInterface, + ResetAfterRequestInterface { /**#@+ * Constants defined for type of context diff --git a/lib/internal/Magento/Framework/Url/QueryParamsResolver.php b/lib/internal/Magento/Framework/Url/QueryParamsResolver.php index 1e38df4941511..bf711cf404d74 100644 --- a/lib/internal/Magento/Framework/Url/QueryParamsResolver.php +++ b/lib/internal/Magento/Framework/Url/QueryParamsResolver.php @@ -5,7 +5,10 @@ */ namespace Magento\Framework\Url; -class QueryParamsResolver extends \Magento\Framework\DataObject implements QueryParamsResolverInterface +use Magento\Framework\ObjectManager\ResetAfterRequestInterface; + +class QueryParamsResolver extends \Magento\Framework\DataObject implements QueryParamsResolverInterface, + ResetAfterRequestInterface { /** * @inheritdoc From b9cb9d1be441546147bc56ea5874582adb410897 Mon Sep 17 00:00:00 2001 From: Jacob Brown Date: Fri, 28 Jun 2024 09:16:05 -0500 Subject: [PATCH 08/12] ACPT-1929 Fixing static test failures --- .../GraphQl/Model/Query/Resolver/Context.php | 3 ++- .../ObjectManager/Resetter/Resetter.php | 16 ++++++++++++++++ .../Framework/Url/QueryParamsResolver.php | 3 ++- 3 files changed, 20 insertions(+), 2 deletions(-) diff --git a/app/code/Magento/GraphQl/Model/Query/Resolver/Context.php b/app/code/Magento/GraphQl/Model/Query/Resolver/Context.php index 6b436de3e53b4..44f59ed81b51c 100644 --- a/app/code/Magento/GraphQl/Model/Query/Resolver/Context.php +++ b/app/code/Magento/GraphQl/Model/Query/Resolver/Context.php @@ -16,7 +16,8 @@ * @deprecated 100.3.3 * @see \Magento\GraphQl\Model\Query\Context */ -class Context extends \Magento\Framework\Model\AbstractExtensibleModel implements ContextInterface, +class Context extends \Magento\Framework\Model\AbstractExtensibleModel implements + ContextInterface, ResetAfterRequestInterface { /**#@+ diff --git a/lib/internal/Magento/Framework/ObjectManager/Resetter/Resetter.php b/lib/internal/Magento/Framework/ObjectManager/Resetter/Resetter.php index 07cad6ecef6af..a5d0c5c288ea7 100644 --- a/lib/internal/Magento/Framework/ObjectManager/Resetter/Resetter.php +++ b/lib/internal/Magento/Framework/ObjectManager/Resetter/Resetter.php @@ -129,6 +129,13 @@ public function setObjectManager(ObjectManagerInterface $objectManager) : void $this->objectManager = $objectManager; } + /** + * Checks if the object is in the class list uses inheritance (via instanceof) + * + * @param object $object + * @return bool + * @SuppressWarnings(PHPMD.UnusedLocalVariable) + */ public function isObjectInClassList(object $object) { foreach ($this->classList as $key => $value) { @@ -145,6 +152,7 @@ public function isObjectInClassList(object $object) * @param object $instance * @return void * @throws \ReflectionException + * @SuppressWarnings(PHPMD.UnusedLocalVariable) */ private function resetStateWithReflection(object $instance) { @@ -158,6 +166,14 @@ private function resetStateWithReflection(object $instance) } } } + + /** + * State reset using reflection using specific className + * + * @param object $instance + * @param string $className + * @return void + */ private function resetStateWithReflectionByClassName(object $instance, string $className) { $classResetValues = $this->classList[$className] ?? []; diff --git a/lib/internal/Magento/Framework/Url/QueryParamsResolver.php b/lib/internal/Magento/Framework/Url/QueryParamsResolver.php index bf711cf404d74..ed862cd0d8f1f 100644 --- a/lib/internal/Magento/Framework/Url/QueryParamsResolver.php +++ b/lib/internal/Magento/Framework/Url/QueryParamsResolver.php @@ -7,7 +7,8 @@ use Magento\Framework\ObjectManager\ResetAfterRequestInterface; -class QueryParamsResolver extends \Magento\Framework\DataObject implements QueryParamsResolverInterface, +class QueryParamsResolver extends \Magento\Framework\DataObject implements + QueryParamsResolverInterface, ResetAfterRequestInterface { /** From 4a42eeac71dbcdaf3ff7ddde75213be47baa574c Mon Sep 17 00:00:00 2001 From: Jacob Brown Date: Fri, 28 Jun 2024 14:33:52 -0500 Subject: [PATCH 09/12] ACPT-1929 Cache result of isObjectInClassList because now that its time is O(n) when search for matching classes, when same class is checked twice, it can be O(1) instead when using the cache --- .../ObjectManager/Resetter/Resetter.php | 22 ++++++++++++++----- 1 file changed, 17 insertions(+), 5 deletions(-) diff --git a/lib/internal/Magento/Framework/ObjectManager/Resetter/Resetter.php b/lib/internal/Magento/Framework/ObjectManager/Resetter/Resetter.php index a5d0c5c288ea7..5f7a1ef1e5445 100644 --- a/lib/internal/Magento/Framework/ObjectManager/Resetter/Resetter.php +++ b/lib/internal/Magento/Framework/ObjectManager/Resetter/Resetter.php @@ -31,11 +31,15 @@ class Resetter implements ResetterInterface /** @var WeakMapSorter|null Note: We use temporal coupling here because of chicken/egg during bootstrapping */ private ?WeakMapSorter $weakMapSorter = null; - /** - * @var array - */ + /** @var array */ private array $reflectionCache = []; + /** @var array */ + private array $isObjectInClassListCache = []; + + /** @var array */ + private readonly array $classList; + /** * @param ComponentRegistrarInterface|null $componentRegistrar * @param array $classList @@ -44,7 +48,7 @@ class Resetter implements ResetterInterface */ public function __construct( private ?ComponentRegistrarInterface $componentRegistrar = null, - private array $classList = [], + array $classList = [], ) { if (null === $this->componentRegistrar) { $this->componentRegistrar = new ComponentRegistrar(); @@ -57,8 +61,9 @@ public function __construct( if (!$resetData) { throw new LocalizedException(__('Error parsing %1', $resetPath)); } - $this->classList += $resetData; + $classList += $resetData; } + $this->classList = $classList; $this->resetAfterWeakMap = new WeakMap; } @@ -138,11 +143,18 @@ public function setObjectManager(ObjectManagerInterface $objectManager) : void */ public function isObjectInClassList(object $object) { + $className = \get_class($object); + $isObjectInClassListCachedValue = $this->isObjectInClassListCache[$className] ?? null; + if (null !== $isObjectInClassListCachedValue) { + return $isObjectInClassListCachedValue; + } foreach ($this->classList as $key => $value) { if ($object instanceof $key) { + $this->isObjectInClassListCache[$className] = true; return true; } } + $this->isObjectInClassListCache[$className] = false; return false; } From becf50ddef50cd3120df988a4c99bc690cb5b240 Mon Sep 17 00:00:00 2001 From: Abhishek Pathak <107833467+wip44850@users.noreply.github.com> Date: Wed, 3 Jul 2024 13:34:57 +0530 Subject: [PATCH 10/12] =?UTF-8?q?LYNX-460:=20Bundle=20products=20does=20no?= =?UTF-8?q?t=20show=20original=20price=20and=20the=20final=20=E2=80=A6=20(?= =?UTF-8?q?#268)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * LYNX-460: Bundle products does not show original price and the final price has no currency * LYNX-460: updated tests * LYNX-460: updated review point --- .../Model/Cart/BundleOptionDataProvider.php | 39 +- .../Magento/BundleGraphQl/etc/schema.graphqls | 4 +- .../BundleProductCartOriginalPricesTest.php | 516 ++++++++++++++++++ 3 files changed, 534 insertions(+), 25 deletions(-) create mode 100644 dev/tests/api-functional/testsuite/Magento/GraphQl/Bundle/BundleProductCartOriginalPricesTest.php diff --git a/app/code/Magento/BundleGraphQl/Model/Cart/BundleOptionDataProvider.php b/app/code/Magento/BundleGraphQl/Model/Cart/BundleOptionDataProvider.php index fdd69cc268f97..31bcdf57a139a 100644 --- a/app/code/Magento/BundleGraphQl/Model/Cart/BundleOptionDataProvider.php +++ b/app/code/Magento/BundleGraphQl/Model/Cart/BundleOptionDataProvider.php @@ -14,6 +14,7 @@ use Magento\Quote\Model\Quote\Item; use Magento\Framework\Pricing\Helper\Data; use Magento\Framework\Serialize\SerializerInterface; +use Magento\Bundle\Model\Product\OriginalPrice; /** * Data provider for bundled product options @@ -25,21 +26,6 @@ class BundleOptionDataProvider */ private const OPTION_TYPE = 'bundle'; - /** - * @var Data - */ - private $pricingHelper; - - /** - * @var SerializerInterface - */ - private $serializer; - - /** - * @var Configuration - */ - private $configuration; - /** @var Uid */ private $uidEncoder; @@ -47,17 +33,16 @@ class BundleOptionDataProvider * @param Data $pricingHelper * @param SerializerInterface $serializer * @param Configuration $configuration + * @param OriginalPrice $originalPrice * @param Uid|null $uidEncoder */ public function __construct( - Data $pricingHelper, - SerializerInterface $serializer, - Configuration $configuration, + private readonly Data $pricingHelper, + private readonly SerializerInterface $serializer, + private readonly Configuration $configuration, + private readonly OriginalPrice $originalPrice, Uid $uidEncoder = null ) { - $this->pricingHelper = $pricingHelper; - $this->serializer = $serializer; - $this->configuration = $configuration; $this->uidEncoder = $uidEncoder ?: ObjectManager::getInstance() ->get(Uid::class); } @@ -139,12 +124,12 @@ private function buildBundleOptionValues(array $selections, Item $item): array $values = []; $product = $item->getProduct(); + $currencyCode = $item->getQuote()->getQuoteCurrencyCode(); foreach ($selections as $selection) { $qty = (float) $this->configuration->getSelectionQty($product, $selection->getSelectionId()); if (!$qty) { continue; } - $selectionPrice = $this->configuration->getSelectionFinalPrice($item, $selection); $optionDetails = [ self::OPTION_TYPE, @@ -152,15 +137,21 @@ private function buildBundleOptionValues(array $selections, Item $item): array $selection->getData('selection_id'), (int) $selection->getData('selection_qty') ]; + $price = $this->pricingHelper->currency($selectionPrice, false, false); $values[] = [ 'id' => $selection->getSelectionId(), 'uid' => $this->uidEncoder->encode(implode('/', $optionDetails)), 'label' => $selection->getName(), 'quantity' => $qty, - 'price' => $this->pricingHelper->currency($selectionPrice, false, false), + 'price' => $price, + 'priceV2' => ['currency' => $currencyCode, 'value' => $price], + 'original_price' => [ + 'currency' => $currencyCode, + 'value' => $this->originalPrice + ->getSelectionOriginalPrice($item->getProduct(), $selection) + ], ]; } - return $values; } } diff --git a/app/code/Magento/BundleGraphQl/etc/schema.graphqls b/app/code/Magento/BundleGraphQl/etc/schema.graphqls index f75b731ea83e9..f2da27d604666 100644 --- a/app/code/Magento/BundleGraphQl/etc/schema.graphqls +++ b/app/code/Magento/BundleGraphQl/etc/schema.graphqls @@ -44,7 +44,9 @@ type SelectedBundleOptionValue @doc(description: "Contains details about a value uid: ID! @doc(description: "The unique ID for a `SelectedBundleOptionValue` object") label: String! @doc(description: "The display name of the value for the selected bundle product option.") quantity: Float! @doc(description: "The quantity of the value for the selected bundle product option.") - price: Float! @doc(description: "The price of the value for the selected bundle product option.") + price: Float! @deprecated(reason: "Use priceV2 instead.") @doc(description: "The price of the value for the selected bundle product option.") + priceV2: Money! @doc(description: "The price of the value for the selected bundle product option.") + original_price: Money! @doc(description: "The original price of the value for the selected bundle product option.") } type PriceDetails @doc(description: "Can be used to retrieve the main price details in case of bundle product") { diff --git a/dev/tests/api-functional/testsuite/Magento/GraphQl/Bundle/BundleProductCartOriginalPricesTest.php b/dev/tests/api-functional/testsuite/Magento/GraphQl/Bundle/BundleProductCartOriginalPricesTest.php new file mode 100644 index 0000000000000..d1d673258fd62 --- /dev/null +++ b/dev/tests/api-functional/testsuite/Magento/GraphQl/Bundle/BundleProductCartOriginalPricesTest.php @@ -0,0 +1,516 @@ +quoteIdToMaskedQuoteIdInterface = $objectManager->get(QuoteIdToMaskedQuoteIdInterface::class); + $this->fixtures = $objectManager->get(DataFixtureStorageManager::class)->getStorage(); + } + + #[ + DataFixture(ProductFixture::class, ['price' => 20], 'product1'), + DataFixture(ProductFixture::class, ['price' => 10], 'product2'), + DataFixture(BundleSelectionFixture::class, ['sku' => '$product1.sku$'], 'selection1'), + DataFixture(BundleSelectionFixture::class, ['sku' => '$product2.sku$'], 'selection2'), + DataFixture(BundleOptionFixture::class, ['product_links' => ['$selection1$']], 'opt1'), + DataFixture(BundleOptionFixture::class, ['product_links' => ['$selection2$']], 'opt2'), + DataFixture( + BundleProductFixture::class, + [ + 'sku' => 'bundle-product-fixed-price', + 'price_type' => Price::PRICE_TYPE_DYNAMIC, + '_options' => ['$opt1$', '$opt2$'] + ], + 'bundle_product_1' + ), + DataFixture( + BundleProductFixture::class, + [ + 'sku' => 'bundle-product-fixed-price-special-price', + 'price_type' => Price::PRICE_TYPE_DYNAMIC, + '_options' => ['$opt1$', '$opt2$'], + 'special_price' => 90 // it is the 90% of the original price + ], + 'bundle_product_2' + ), + DataFixture(GuestCartFixture::class, as: 'cart'), + DataFixture( + AddBundleProductToCart::class, + [ + 'cart_id' => '$cart.id$', + 'product_id' => '$bundle_product_1.id$', + 'selections' => [['$product1.id$'], ['$product2.id$']] + ] + ), + DataFixture( + AddBundleProductToCart::class, + [ + 'cart_id' => '$cart.id$', + 'product_id' => '$bundle_product_2.id$', + 'selections' => [['$product1.id$'], ['$product2.id$']] + ] + ) + ] + public function testBundleProductWithDynamicPrices() + { + $cart = $this->fixtures->get('cart'); + $maskedQuoteId = $this->quoteIdToMaskedQuoteIdInterface->execute((int) $cart->getId()); + $query = $this->getCartQuery($maskedQuoteId); + $response = $this->graphQlQuery($query); + + $expectedResponse = $this->getExpectedResponse( + 20, //prod1 original price + 10, //prod2 original price + 20, //prod1 price is same as original price in bundle 1 + 10, //prod2 price is same as original price in bundle 1 + 18, //prod1 price is 90% of original price in bundle 2 + 9 //prod2 price is 90% of original price in bundle 2 + ); + $this->assertEquals($expectedResponse, $response); + } + + #[ + DataFixture(ProductFixture::class, ['price' => 20], 'product1'), + DataFixture(ProductFixture::class, ['price' => 10], 'product2'), + DataFixture(BundleSelectionFixture::class, ['sku' => '$product1.sku$', 'price' => 15], 'selection1'), + DataFixture(BundleSelectionFixture::class, ['sku' => '$product2.sku$', 'price' => 8], 'selection2'), + DataFixture(BundleOptionFixture::class, ['product_links' => ['$selection1$']], 'opt1'), + DataFixture(BundleOptionFixture::class, ['product_links' => ['$selection2$']], 'opt2'), + DataFixture( + BundleProductFixture::class, + [ + 'sku' => 'bundle-product-fixed-price', + 'price_type' => Price::PRICE_TYPE_FIXED, + '_options' => ['$opt1$', '$opt2$'], + ], + 'bundle_product_1' + ), + DataFixture( + BundleProductFixture::class, + [ + 'sku' => 'bundle-product-fixed-price-special-price', + 'price_type' => Price::PRICE_TYPE_FIXED, + '_options' => ['$opt1$', '$opt2$'], + 'special_price' => 90 // it is the 90% of the original price + ], + 'bundle_product_2' + ), + DataFixture(GuestCartFixture::class, as: 'cart'), + DataFixture( + AddBundleProductToCart::class, + [ + 'cart_id' => '$cart.id$', + 'product_id' => '$bundle_product_1.id$', + 'selections' => [['$product1.id$'], ['$product2.id$']] + ] + ), + DataFixture( + AddBundleProductToCart::class, + [ + 'cart_id' => '$cart.id$', + 'product_id' => '$bundle_product_2.id$', + 'selections' => [['$product1.id$'], ['$product2.id$']] + ] + ) + ] + public function testBundleProductWithFixedPrices() + { + $cart = $this->fixtures->get('cart'); + $maskedQuoteId = $this->quoteIdToMaskedQuoteIdInterface->execute((int) $cart->getId()); + $query = $this->getCartQuery($maskedQuoteId); + $response = $this->graphQlQuery($query); + + $expectedResponse = $this->getExpectedResponse( + 15, //fixed price set for selection 1 + 8, //fixed price set for selection 2 + 15, //prod1 price is same as fixed price in bundle 1 + 8, //prod2 price is same as fixed price in bundle 1 + 13.5, //prod1 price is 90% of fixed price in bundle 2 + 7.2 //prod2 price is 90% of fixed price in bundle 2 + ); + $this->assertEquals($expectedResponse, $response); + } + + #[ + DataFixture(ProductFixture::class, ['price' => 20], 'product1'), + DataFixture(ProductFixture::class, ['price' => 10], 'product2'), + DataFixture( + BundleSelectionFixture::class, + [ + 'sku' => '$product1.sku$', + 'price' => 90, //90% of bundle price + 'price_type' => LinkInterface::PRICE_TYPE_PERCENT + ], + 'selection1' + ), + DataFixture( + BundleSelectionFixture::class, + [ + 'sku' => '$product2.sku$', + 'price' => 80, //80% of bundle price + 'price_type' => LinkInterface::PRICE_TYPE_PERCENT + ], + 'selection2' + ), + DataFixture(BundleOptionFixture::class, ['product_links' => ['$selection1$']], 'opt1'), + DataFixture(BundleOptionFixture::class, ['product_links' => ['$selection2$']], 'opt2'), + DataFixture( + BundleProductFixture::class, + [ + 'sku' => 'bundle-product-fixed-price', + 'price' => 15, + 'price_type' => Price::PRICE_TYPE_FIXED, + '_options' => ['$opt1$', '$opt2$'], + ], + 'bundle_product_1' + ), + DataFixture( + BundleProductFixture::class, + [ + 'sku' => 'bundle-product-fixed-price-special-price', + 'price' => 15, + 'price_type' => Price::PRICE_TYPE_FIXED, + '_options' => ['$opt1$', '$opt2$'], + 'special_price' => 90 // it is the 90% of the original price + ], + 'bundle_product_2' + ), + DataFixture(GuestCartFixture::class, as: 'cart'), + DataFixture( + AddBundleProductToCart::class, + [ + 'cart_id' => '$cart.id$', + 'product_id' => '$bundle_product_1.id$', + 'selections' => [['$product1.id$'], ['$product2.id$']] + ] + ), + DataFixture( + AddBundleProductToCart::class, + [ + 'cart_id' => '$cart.id$', + 'product_id' => '$bundle_product_2.id$', + 'selections' => [['$product1.id$'], ['$product2.id$']] + ] + ) + ] + public function testBundleProductWithPercentagePrices() + { + $cart = $this->fixtures->get('cart'); + $maskedQuoteId = $this->quoteIdToMaskedQuoteIdInterface->execute((int) $cart->getId()); + $query = $this->getCartQuery($maskedQuoteId); + $response = $this->graphQlQuery($query); + + $expectedResponse = $this->getExpectedResponse( + 13.5, //90% of selection 1 + 12, //80% of selection 1 + 13.5, //prod1 price is same as percentage price in bundle 1 + 12, //prod2 price is same as percentage price in bundle 1 + 12.15, //prod1 price is 90% of percentage price in bundle 2 + 10.8 //prod2 price is 90% of percentage price in bundle 2 + ); + $this->assertEquals($expectedResponse, $response); + } + + #[ + DataFixture(ProductFixture::class, ['price' => 20], 'product1'), + DataFixture(ProductFixture::class, ['price' => 10], 'product2'), + DataFixture(BundleSelectionFixture::class, ['sku' => '$product1.sku$'], 'selection1'), + DataFixture(BundleSelectionFixture::class, ['sku' => '$product2.sku$'], 'selection2'), + DataFixture(BundleOptionFixture::class, ['product_links' => ['$selection1$']], 'opt1'), + DataFixture(BundleOptionFixture::class, ['product_links' => ['$selection2$']], 'opt2'), + DataFixture( + BundleProductFixture::class, + [ + 'sku' => 'bundle-product-dynamic-price', + '_options' => ['$opt1$', '$opt2$'], + ], + 'bundle_product_1' + ), + DataFixture(GuestCartFixture::class, as: 'cart'), + DataFixture( + AddBundleProductToCart::class, + [ + 'cart_id' => '$cart.id$', + 'product_id' => '$bundle_product_1.id$', + 'selections' => [['$product1.id$'], ['$product2.id$']] + ] + ) + ] + public function testBundleProductWithoutSpecialPrice() + { + $cart = $this->fixtures->get('cart'); + $maskedQuoteId = $this->quoteIdToMaskedQuoteIdInterface->execute((int) $cart->getId()); + $query = $this->getCartQuery($maskedQuoteId); + $response = $this->graphQlQuery($query); + + $expectedResponse = $this->getExpectedResponseSingleBundle( + 20, //prod1 original price + 10 //prod2 original price + ); + + $this->assertEquals($expectedResponse, $response); + } + + #[ + DataFixture(ProductFixture::class, ['price' => 20, 'special_price' => 15], 'product1'), + DataFixture(ProductFixture::class, ['price' => 10, 'special_price' => 8], 'product2'), + DataFixture(BundleSelectionFixture::class, ['sku' => '$product1.sku$'], 'selection1'), + DataFixture(BundleSelectionFixture::class, ['sku' => '$product2.sku$'], 'selection2'), + DataFixture(BundleOptionFixture::class, ['product_links' => ['$selection1$']], 'opt1'), + DataFixture(BundleOptionFixture::class, ['product_links' => ['$selection2$']], 'opt2'), + DataFixture( + BundleProductFixture::class, + [ + 'sku' => 'bundle-product-dynamic-price', + '_options' => ['$opt1$', '$opt2$'], + ], + 'bundle_product_1' + ), + DataFixture(GuestCartFixture::class, as: 'cart'), + DataFixture( + AddBundleProductToCart::class, + [ + 'cart_id' => '$cart.id$', + 'product_id' => '$bundle_product_1.id$', + 'selections' => [['$product1.id$'], ['$product2.id$']] + ] + ) + ] + public function testBundleProductWithSpecialPrice() + { + $cart = $this->fixtures->get('cart'); + $maskedQuoteId = $this->quoteIdToMaskedQuoteIdInterface->execute((int) $cart->getId()); + $query = $this->getCartQuery($maskedQuoteId); + $response = $this->graphQlQuery($query); + + $expectedResponse = $this->getExpectedResponseSingleBundle( + 15, //prod1 special price + 8 //prod2 special price + ); + + $this->assertEquals($expectedResponse, $response); + } + + /** + * Generates GraphQl query for getting cart prices (priceV2 & original_price) of bundle products + * + * @param string $maskedQuoteId + * @return string + */ + private function getCartQuery(string $maskedQuoteId): string + { + return << [ + "items" => [ + 0 => [ + "bundle_options" => [ + 0 => [ + 'values' => [ + 0 => [ + "priceV2" => [ + "value" => $item1Option1PriceV2, + "currency" => "USD" + ], + "original_price" => [ + "value" => $product1OriginalPrice, + "currency" => "USD" + ] + ] + ] + ], + 1 => [ + 'values' => [ + 0 => [ + "priceV2" => [ + "value" => $item2Option1PriceV2, + "currency" => "USD" + ], + "original_price" => [ + "value" => $product2OriginalPrice, + "currency" => "USD" + ] + ] + ] + ] + ] + ], + 1 => [ + "bundle_options" => [ + 0 => [ + 'values' => [ + 0 => [ + "priceV2" => [ + "value" => $item1Option2PriceV2, + "currency" => "USD" + ], + "original_price" => [ + "value" => $product1OriginalPrice, + "currency" => "USD" + ] + ] + ] + ], + 1 => [ + 'values' => [ + 0 => [ + "priceV2" => [ + "value" => $item2Option2PriceV2, + "currency" => "USD" + ], + "original_price" => [ + "value" => $product2OriginalPrice, + "currency" => "USD" + ] + ] + ] + ] + ] + ] + ] + ] + ]; + } + + /** + * Returns expected result with priceV2 & original_price for single bundles in cart + * + * @param $item1SpecialPrice + * @param $item2SpecialPrice + * @return array[] + */ + private function getExpectedResponseSingleBundle( + $item1SpecialPrice, + $item2SpecialPrice + ): array { + return [ + "cart" => [ + "items" => [ + 0 => [ + "bundle_options" => [ + 0 => [ + 'values' => [ + 0 => [ + "priceV2" => [ + "value" => $item1SpecialPrice, + "currency" => "USD" + ], + "original_price" => [ + "value" => 20, + "currency" => "USD" + ] + ] + ] + ], + 1 => [ + 'values' => [ + 0 => [ + "priceV2" => [ + "value" => $item2SpecialPrice, + "currency" => "USD" + ], + "original_price" => [ + "value" => 10, + "currency" => "USD" + ] + ] + ] + ] + ] + ] + ] + ] + ]; + } +} From 77f66695d6c69535a14da9a3bcdd3831af600db8 Mon Sep 17 00:00:00 2001 From: Rafal Janicki Date: Fri, 5 Jul 2024 09:25:47 +0100 Subject: [PATCH 11/12] [LYNX-455] DisableSessionSetCookieTest failing with AppServer enabled --- .../TestFramework/TestCase/GraphQl/Client.php | 12 ++++- .../GraphQl/DisableSessionSetCookieTest.php} | 49 ++++++++++++++----- .../Framework/App/PageCache/Version.php | 3 +- 3 files changed, 50 insertions(+), 14 deletions(-) rename dev/tests/{Magento/GraphQl/PageCache/DisableSessionTest.php => api-functional/testsuite/Magento/GraphQl/GraphQl/DisableSessionSetCookieTest.php} (57%) diff --git a/dev/tests/api-functional/framework/Magento/TestFramework/TestCase/GraphQl/Client.php b/dev/tests/api-functional/framework/Magento/TestFramework/TestCase/GraphQl/Client.php index a2e7fcb95a430..8f50dc171c944 100644 --- a/dev/tests/api-functional/framework/Magento/TestFramework/TestCase/GraphQl/Client.php +++ b/dev/tests/api-functional/framework/Magento/TestFramework/TestCase/GraphQl/Client.php @@ -22,6 +22,8 @@ class Client public const GRAPHQL_METHOD_POST = 'POST'; /**#@-*/ + private const SET_COOKIE_HEADER_NAME = 'Set-Cookie'; + /** @var CurlClient */ private $curlClient; @@ -264,7 +266,15 @@ private function processResponseHeaders(string $headers): array foreach ($headerLines as $headerLine) { $headerParts = preg_split('/: /', $headerLine, 2); if (count($headerParts) == 2) { - $headersArray[trim($headerParts[0])] = trim($headerParts[1]); + $headerName = trim($headerParts[0]); + if ($headerName === self::SET_COOKIE_HEADER_NAME) { + if (!isset($headersArray[self::SET_COOKIE_HEADER_NAME])) { + $headersArray[self::SET_COOKIE_HEADER_NAME] = []; + } + $headersArray[self::SET_COOKIE_HEADER_NAME][] = trim($headerParts[1]); + } else { + $headersArray[$headerName] = trim($headerParts[1]); + } } elseif (preg_match('/HTTP\/[\.0-9]+/', $headerLine)) { $headersArray[trim('Status-Line')] = trim($headerLine); } diff --git a/dev/tests/Magento/GraphQl/PageCache/DisableSessionTest.php b/dev/tests/api-functional/testsuite/Magento/GraphQl/GraphQl/DisableSessionSetCookieTest.php similarity index 57% rename from dev/tests/Magento/GraphQl/PageCache/DisableSessionTest.php rename to dev/tests/api-functional/testsuite/Magento/GraphQl/GraphQl/DisableSessionSetCookieTest.php index 5614eccff88bf..f1cde88ba868b 100644 --- a/dev/tests/Magento/GraphQl/PageCache/DisableSessionTest.php +++ b/dev/tests/api-functional/testsuite/Magento/GraphQl/GraphQl/DisableSessionSetCookieTest.php @@ -14,17 +14,19 @@ */ declare(strict_types=1); -namespace Magento\GraphQl\PageCache; +namespace Magento\GraphQl\GraphQl; -use Magento\TestFramework\TestCase\GraphQlAbstract; -use Magento\TestFramework\Fixture\Config; use Magento\Framework\App\PageCache\Version; +use Magento\TestFramework\Fixture\Config; +use Magento\TestFramework\TestCase\GraphQlAbstract; /** * Test absence/presence of private_content_version cookie in GraphQl POST HTTP responses */ -class DisableSessionTest extends GraphQlAbstract +class DisableSessionSetCookieTest extends GraphQlAbstract { + private const PHPSESSID_COOKIE_NAME = 'PHPSESSID'; + #[ Config('graphql/session/disable', '1') ] @@ -33,10 +35,14 @@ public function testPrivateSessionContentCookieNotPresentWhenSessionDisabled() $result = $this->graphQlMutationWithResponseHeaders($this->getMutation()); $this->assertArrayHasKey('headers', $result); if (!empty($result['headers']['Set-Cookie'])) { - $this->assertStringNotContainsString( - Version::COOKIE_NAME, - $result['headers']['Set-Cookie'], - Version::COOKIE_NAME . ' should not be present in Set-Cookie header' + $this->assertFalse( + $this->isCookieSet($result['headers']['Set-Cookie'], self::PHPSESSID_COOKIE_NAME), + self::PHPSESSID_COOKIE_NAME . ' should not be present in HTTP response' + ); + + $this->assertFalse( + $this->isCookieSet($result['headers']['Set-Cookie'], Version::COOKIE_NAME), + Version::COOKIE_NAME . ' should not be present in HTTP response' ); } } @@ -49,11 +55,30 @@ public function testPrivateSessionContentCookiePresentWhenSessionEnabled() $result = $this->graphQlMutationWithResponseHeaders($this->getMutation()); $this->assertArrayHasKey('headers', $result); $this->assertArrayHasKey('Set-Cookie', $result['headers'], 'Set-Cookie HTTP response header should be present'); - $this->assertStringContainsString( - Version::COOKIE_NAME, - $result['headers']['Set-Cookie'], - Version::COOKIE_NAME . ' should be set by the server' + + $this->assertTrue( + $this->isCookieSet($result['headers']['Set-Cookie'], self::PHPSESSID_COOKIE_NAME), + self::PHPSESSID_COOKIE_NAME . ' should be present in HTTP response' ); + + $this->assertTrue( + $this->isCookieSet($result['headers']['Set-Cookie'], Version::COOKIE_NAME), + Version::COOKIE_NAME . ' should be present in HTTP response' + ); + } + + /** + * Checks if $cookieName was set by server in any of Set-Cookie header(s) + * + * @param array $setCookieHeader + * @param string $cookieName + * @return bool + */ + private function isCookieSet(array $setCookieHeader, string $cookieName): bool + { + return count(array_filter($setCookieHeader, function ($cookie) use ($cookieName) { + return str_starts_with($cookie, $cookieName . '='); + })) > 0; } /** diff --git a/lib/internal/Magento/Framework/App/PageCache/Version.php b/lib/internal/Magento/Framework/App/PageCache/Version.php index 0e19f572d176c..47ada94e2bdb4 100644 --- a/lib/internal/Magento/Framework/App/PageCache/Version.php +++ b/lib/internal/Magento/Framework/App/PageCache/Version.php @@ -72,7 +72,8 @@ public function process(): void return; } - if ($this->request->getOriginalPathInfo() === '/graphql' && $this->isSessionDisabled() === true) { + $originalPathInfo = $this->request->getOriginalPathInfo(); + if ($originalPathInfo && str_contains($originalPathInfo, '/graphql') && $this->isSessionDisabled() === true) { return; } From 67ac28d715fefae7ac984f28660ef118b5466d01 Mon Sep 17 00:00:00 2001 From: Jacob Brown Date: Fri, 12 Jul 2024 14:44:13 -0500 Subject: [PATCH 12/12] ACPT-1929 When multiple reseters match class in resetStateWithReflection, they are now run in order of inheritance so that subclasses can have specialized resetters. Also runs resetters for interfaces in proper order as well. --- .../ObjectManager/Resetter/Resetter.php | 65 ++++++++++++++++++- 1 file changed, 62 insertions(+), 3 deletions(-) diff --git a/lib/internal/Magento/Framework/ObjectManager/Resetter/Resetter.php b/lib/internal/Magento/Framework/ObjectManager/Resetter/Resetter.php index 5f7a1ef1e5445..27a2e15c196d6 100644 --- a/lib/internal/Magento/Framework/ObjectManager/Resetter/Resetter.php +++ b/lib/internal/Magento/Framework/ObjectManager/Resetter/Resetter.php @@ -40,6 +40,9 @@ class Resetter implements ResetterInterface /** @var array */ private readonly array $classList; + /** @var array */ + private array $sortedClassListsByClass = []; + /** * @param ComponentRegistrarInterface|null $componentRegistrar * @param array $classList @@ -172,11 +175,67 @@ private function resetStateWithReflection(object $instance) $instance->{self::RESET_STATE_METHOD}(); return; } - foreach ($this->classList as $className => $value) { - if ($instance instanceof $className) { - $this->resetStateWithReflectionByClassName($instance, $className); + $className = get_class($instance); + if (!array_key_exists($className, $this->sortedClassListsByClass)) { + $temporaryClassList = []; + foreach ($this->classList as $key => $value) { + if ($instance instanceof $key) { + $temporaryClassList[] = $key; + } } + $this->sortClasses($temporaryClassList); + $this->sortedClassListsByClass[$className] = $temporaryClassList; + } + foreach ($this->sortedClassListsByClass[$className] as $currentClassName) { + $this->resetStateWithReflectionByClassName($instance, $currentClassName); + } + } + + /** + * Sorts an array of strings that are class names and interface names + * + * Note: This sorting algorithm only takes arrays that are keyed by contiguous numbers starting at zero. + * Note: This sorting algorithm works with comparators that return false on unrelated items. + * + * @param array $array + * @return void + */ + private function sortClasses(array &$array) : void + { + $i = 0; + $count = count($array); + while ($i + 1 < $count) { + for ($j = $i + 1; $j < $count; $j++) { + if ($this->sortClassesComparitor($array[$i], $array[$j])) { + $swapTemp = $array[$i]; + $array[$i] = $array[$j]; + $array[$j] = $swapTemp; + continue 2; + } + } + $i++; + } + } + + /** + * Comparator for class/interface sorter that returns true if $b should come before $a. + * + * @param string $a + * @param string $b + * @return bool + */ + private function sortClassesComparitor(string $a, string $b) : bool + { + if (is_a($a, $b, true)) { + return true; + } + if (is_a($b, $a, true)) { + return false; + } + if (interface_exists($a) && class_exists($b)) { + return true; // Note: If they aren't related, classes should come before interfaces } + return false; // No relation } /**