Merge pull request #74 from Setono/fix-calculator

Fixing bug in ProductVariantPricesCalculator

Author: loevgaard

src/Calculator/ProductVariantPricesCalculator.php

@@ -27,12 +27,12 @@ public function __construct(private readonly RuntimePromotionsApplicatorInterfac
 
     public function calculate(ProductVariantInterface $productVariant, array $context): int
     {
-        $hash = spl_object_hash($productVariant);
-        if (!isset($this->computedPriceCache[$hash])) {
-            $this->computedPriceCache[$hash] = $this->getPrice($productVariant, $context);
+        $cacheKey = self::generateCacheKey($productVariant, $context);
+        if (!array_key_exists($cacheKey, $this->computedPriceCache)) {
+            $this->computedPriceCache[$cacheKey] = $this->getPrice($productVariant, $context);
         }
 
-        return $this->computedPriceCache[$hash];
+        return $this->computedPriceCache[$cacheKey];
     }
 
     public function calculateOriginal(ProductVariantInterface $productVariant, array $context): int
@@ -57,15 +57,14 @@ private function getPrice(ProductVariantInterface $productVariant, array $contex
     }
 
     /**
-     * @psalm-assert ChannelInterface $context['channel']
-     *
      * @return array{price: int, originalPrice: int, minimumPrice: int}
      */
     private function getPersistedPrices(ProductVariantInterface $productVariant, array $context): array
     {
-        $hash = spl_object_hash($productVariant);
+        $cacheKey = self::generateCacheKey($productVariant, $context);
 
-        if (!isset($this->persistedPricesCache[$hash])) {
+        if (!isset($this->persistedPricesCache[$cacheKey])) {
+            // todo remove these assertions when this issue is fixed: https://github.com/vimeo/psalm/issues/11248
             Assert::keyExists($context, 'channel');
             Assert::isInstanceOf($context['channel'], ChannelInterface::class);
 
@@ -80,14 +79,14 @@ private function getPersistedPrices(ProductVariantInterface $productVariant, arr
                 throw MissingChannelConfigurationException::createForProductVariantChannelPricing($productVariant, $context['channel']);
             }
 
-            $this->persistedPricesCache[$hash] = [
+            $this->persistedPricesCache[$cacheKey] = [
                 'price' => $price,
                 'originalPrice' => $channelPricing->getOriginalPrice() ?? $price,
                 'minimumPrice' => self::getMinimumPrice($channelPricing),
             ];
         }
 
-        return $this->persistedPricesCache[$hash];
+        return $this->persistedPricesCache[$cacheKey];
     }
 
     private static function getMinimumPrice(ChannelPricingInterface $channelPricing): int
@@ -104,4 +103,15 @@ private static function getMinimumPrice(ChannelPricingInterface $channelPricing)
 
         return $minimumPrice;
     }
+
+    /**
+     * @psalm-assert ChannelInterface $context['channel']
+     */
+    private static function generateCacheKey(ProductVariantInterface $productVariant, array $context): string
+    {
+        Assert::keyExists($context, 'channel');
+        Assert::isInstanceOf($context['channel'], ChannelInterface::class);
+
+        return sprintf('%s%s', (string) $context['channel']->getCode(), (string) $productVariant->getCode());
+    }
 }

tests/Calculator/ProductVariantPricesCalculatorTest.php

@@ -10,6 +10,8 @@
 use Setono\SyliusCatalogPromotionPlugin\Calculator\ProductVariantPricesCalculator;
 use Setono\SyliusCatalogPromotionPlugin\Model\ProductInterface;
 use Sylius\Component\Core\Model\Channel;
+use Sylius\Component\Core\Model\ChannelInterface;
+use Sylius\Component\Core\Model\ChannelPricing;
 use Sylius\Component\Core\Model\ChannelPricingInterface;
 use Sylius\Component\Core\Model\ProductVariantInterface;
 
@@ -33,6 +35,7 @@ public function it_delegates_to_decorated(): void
         $product->hasPreQualifiedCatalogPromotions()->willReturn(false);
 
         $productVariant = $this->prophesize(ProductVariantInterface::class);
+        $productVariant->getCode()->willReturn('product_variant_1');
         $productVariant->getChannelPricingForChannel($channel)->willReturn($channelPricing->reveal());
         $productVariant->getProduct()->willReturn($product->reveal());
 
@@ -65,6 +68,7 @@ public function it_calculates_catalog_promotion(): void
         $product->getPreQualifiedCatalogPromotions()->willReturn(['promo1', 'promo2']);
 
         $productVariant = $this->prophesize(ProductVariantInterface::class);
+        $productVariant->getCode()->willReturn('product_variant_1');
         $productVariant->getChannelPricingForChannel($channel)->willReturn($channelPricing->reveal());
         $productVariant->getProduct()->willReturn($product->reveal());
 
@@ -99,6 +103,7 @@ public function it_respects_minimum_price(): void
         $product->getPreQualifiedCatalogPromotions()->willReturn(['promo1']);
 
         $productVariant = $this->prophesize(ProductVariantInterface::class);
+        $productVariant->getCode()->willReturn('product_variant_1');
         $productVariant->getChannelPricingForChannel($channel)->willReturn($channelPricing->reveal());
         $productVariant->getProduct()->willReturn($product->reveal());
 
@@ -111,4 +116,61 @@ public function it_respects_minimum_price(): void
             'channel' => $channel,
         ]));
     }
+
+    /**
+     * @test
+     */
+    public function it_works_with_different_channels(): void
+    {
+        $channel1 = self::createChannel('channel1');
+        $channel2 = self::createChannel('channel2');
+
+        $channelPricing1 = self::createChannelPricing(100);
+        $channelPricing2 = self::createChannelPricing(200);
+
+        $product = $this->prophesize(ProductInterface::class);
+        $product->hasPreQualifiedCatalogPromotions()->willReturn(true);
+        $product->getPreQualifiedCatalogPromotions()->willReturn(['promo1']);
+
+        $productVariant = $this->prophesize(ProductVariantInterface::class);
+        $productVariant->getCode()->willReturn('product_variant_1');
+        $productVariant->getChannelPricingForChannel($channel1)->willReturn($channelPricing1);
+        $productVariant->getChannelPricingForChannel($channel2)->willReturn($channelPricing2);
+        $productVariant->getProduct()->willReturn($product->reveal());
+
+        $runtimePromotionsApplicator = $this->prophesize(RuntimePromotionsApplicatorInterface::class);
+        $runtimePromotionsApplicator->apply($product->reveal(), 100, 100)->willReturn(80);
+        $runtimePromotionsApplicator->apply($product->reveal(), 200, 200)->willReturn(160);
+
+        $calculator = new ProductVariantPricesCalculator($runtimePromotionsApplicator->reveal());
+
+        $this->assertSame(80, $calculator->calculate($productVariant->reveal(), [
+            'channel' => $channel1,
+        ]));
+
+        $this->assertSame(160, $calculator->calculate($productVariant->reveal(), [
+            'channel' => $channel2,
+        ]));
+    }
+
+    private static function createChannel(string $code): ChannelInterface
+    {
+        $channel = new Channel();
+        $channel->setCode($code);
+
+        return $channel;
+    }
+
+    private static function createChannelPricing(int $price, int $originalPrice = null, int $minimumPrice = null): ChannelPricingInterface
+    {
+        $channelPricing = new ChannelPricing();
+        $channelPricing->setPrice($price);
+        $channelPricing->setOriginalPrice($originalPrice);
+
+        if (method_exists($channelPricing, 'setMinimumPrice')) {
+            $channelPricing->setMinimumPrice($minimumPrice);
+        }
+
+        return $channelPricing;
+    }
 }