Small cleanup of cart functionality, re-use existing collection code from Doctrine2

src/Sylius/Bundle/CartBundle/Entity/Cart.php

@@ -32,17 +32,6 @@ class Cart extends BaseCart
         $this->items = new ArrayCollection();
     }
 
-    /**
-     * {@inheritdoc}
-     */
-    public function addItem(ItemInterface $item)
-    {
-        if (!$this->hasItem($item)) {
-            $this->items->add($item);
-            $item->setCart($this);
-        }
-    }
-
     /**
      * {@inheritdoc}
      */
@@ -62,6 +51,22 @@ class Cart extends BaseCart
         return $this->items->contains($item);
     }
 
+    /**
+     * {@inheritdoc}
+     */
+    public function searchItem(ItemInterface $item)
+    {
+        return $this->items->indexOf($item);
+    }
+
+    /**
+     * {@inheritdoc}
+     */
+    public function isEmpty()
+    {
+        return $this->items->isEmpty();
+    }
+
     /**
      * {@inheritdoc}
      */

src/Sylius/Bundle/CartBundle/Model/Cart.php

@@ -159,12 +159,20 @@ abstract class Cart implements CartInterface
      */
     public function addItem(ItemInterface $item)
     {
-        if (!$this->hasItem($item)) {
+        $exists = false;
+        foreach ($this->items as $existingItem) {
+            /** @var $existingItem ItemInterface */
+            if ($existingItem->equals($item)) {
+                $existingItem->incrementQuantity($item->getQuantity());
+                $exists = true;
+                break;
+            }
+        }
+
+        if (!$exists) {
             $item->setCart($this);
             $this->items[] = $item;
         }
-
-        return $this;
     }
 
     /**
@@ -172,8 +180,8 @@ abstract class Cart implements CartInterface
      */
     public function removeItem(ItemInterface $item)
     {
-        if ($this->hasItem($item)) {
+        $key = $this->searchItem($item);
-            $key = array_search($item, $this->items);
+        if (false !== $key) {
             unset($this->items[$key]);
             $item->setCart(null);
         }
@@ -184,7 +192,17 @@ abstract class Cart implements CartInterface
      */
     public function hasItem(ItemInterface $item)
     {
-        return $this === $item->getCart();
+        return false !== $this->searchItem($item);
+    }
+
+    /**
+     * @param ItemInterface $item
+     *
+     * @return boolean|integer
+     */
+    public function searchItem(ItemInterface $item)
+    {
+        return array_search($item, $this->items, true);
     }
 
     /**

src/Sylius/Bundle/CartBundle/Model/CartInterface.php

@@ -136,7 +136,7 @@ interface CartInterface
      *
      * @param \DateTime $expiresAt
      */
-    function setExpiresAt(\DateTime $expiresAt);
+    function setExpiresAt(\DateTime $expiresAt = null);
 
     /**
      * Bumps the expiration time.

src/Sylius/Bundle/CartBundle/Model/Item.php

@@ -92,6 +92,10 @@ abstract class Item implements ItemInterface
      */
     public function setQuantity($quantity)
     {
+        if (1 > $quantity) {
+            throw new \OutOfRangeException('Quantity value must be bigger than zero.');
+        }
+
         $this->quantity = $quantity;
     }
 
@@ -101,6 +105,11 @@ abstract class Item implements ItemInterface
     public function incrementQuantity($amount = 1)
     {
         $this->quantity += $amount;
+
+        // Quantity must be bigger than zero
+        if (1 > $this->quantity) {
+            $this->quantity = 1;
+        }
     }
 
     /**
@@ -108,6 +117,10 @@ abstract class Item implements ItemInterface
      */
     public function equals(ItemInterface $item)
     {
-        return $item === $this;
+        if ($item->getId() !== $this->getId()) {
+            return false;
+        }
+
+        return true;
     }
 }

src/Sylius/Bundle/CartBundle/Operator/CartOperator.php

@@ -46,16 +46,6 @@ abstract class CartOperator implements CartOperatorInterface
      */
     public function addItem(CartInterface $cart, ItemInterface $item)
     {
-        if (false === $cart->isEmpty()) {
-            foreach ($cart->getItems() as $existingItem) {
-                if ($existingItem->equals($item)) {
-                    $existingItem->incrementQuantity($item->getQuantity());
-
-                    return;
-                }
-            }
-        }
-
         $cart->addItem($item);
     }
 

src/Sylius/Bundle/CartBundle/Tests/Model/CartTest.php

@@ -22,8 +22,8 @@ class CartTest extends \PHPUnit_Framework_TestCase
     {
         $cart = $this->getCart();
 
-        $this->assertEquals(0, $cart->getTotalItems());
+        $this->assertTrue($cart->isEmpty());
-        $this->assertEquals(false, $cart->isLocked());
+        $this->assertFalse($cart->isLocked());
         $this->assertInstanceOf('DateTime', $cart->getExpiresAt());
     }
 
@@ -73,6 +73,9 @@ class CartTest extends \PHPUnit_Framework_TestCase
         $this->assertGreaterThan($expiresAt, $cart->getExpiresAt());
     }
 
+    /**
+     * @return \Sylius\Bundle\CartBundle\Model\Cart
+     */
     private function getCart()
     {
         return $this->getMockForAbstractClass('Sylius\Bundle\CartBundle\Model\Cart');

src/Sylius/Bundle/CartBundle/Tests/Model/ItemTest.php

@@ -43,14 +43,57 @@ class ItemTest extends \PHPUnit_Framework_TestCase
 
         $item->setCart($cart);
         $this->assertEquals($cart, $item->getCart());
-
     }
 
+    public function testEquals()
+    {
+        $item = $this->getItem();
+        $item->setId(1);
+
+        $duplicatedItem = clone $item;
+        $this->assertTrue($item->equals($duplicatedItem));
+
+        $duplicatedItem->setQuantity(2);
+        $this->assertTrue($item->equals($duplicatedItem));
+        $this->assertNotEquals($duplicatedItem->getQuantity(), $item->getQuantity());
+
+        $duplicatedItem->setId(2);
+        $this->assertFalse($item->equals($duplicatedItem));
+    }
+
+    public function testQuantityManagement()
+    {
+        $item = $this->getItem();
+        $item->setQuantity(5);
+        $this->assertEquals(5, $item->getQuantity());
+
+        $item->incrementQuantity(5);
+        $this->assertEquals(10, $item->getQuantity());
+
+        $item->incrementQuantity(-10);
+        $this->assertEquals(1, $item->getQuantity());
+    }
+
+    /**
+     * @expectedException \OutOfRangeException
+     */
+    public function testQuantityMustBeBiggerThanZero()
+    {
+        $item = $this->getItem();
+        $item->setQuantity(0);
+    }
+
+    /**
+     * @return \Sylius\Bundle\CartBundle\Model\Item
+     */
     private function getItem()
     {
         return $this->getMockForAbstractClass('Sylius\Bundle\CartBundle\Model\Item');
     }
 
+    /**
+     * @return \Sylius\Bundle\CartBundle\Model\CartInterface
+     */
     private function getMockCart()
     {
         return $this->getMock('Sylius\Bundle\CartBundle\Model\CartInterface');

src/Sylius/Bundle/CartBundle/Tests/Operator/CartOperatorTest.php

@@ -35,16 +35,21 @@ class CartOperatorTest extends \PHPUnit_Framework_TestCase
     public function testAddItemIncreasesQuantityIfThereAreItemsThatAreEqual()
     {
         $item = $this->getItem();
+        $item->setId(1);
         $item->setQuantity(3);
 
         $cart = $this->getCart();
 
         $cartOperator = $this->getCartOperator();
         $cartOperator->addItem($cart, $item);
-        $cartOperator->addItem($cart, $item);
+
+        $duplicatedItem = clone $item;
+        $duplicatedItem->setQuantity(2);
+
+        $cartOperator->addItem($cart, $duplicatedItem);
 
         $this->assertEquals(1, $cart->countItems());
-        $this->assertEquals(6, $item->getQuantity());
+        $this->assertEquals(5, $item->getQuantity());
     }
 
     public function testRefreshSetsTotalItems()
@@ -105,6 +110,11 @@ class CartOperatorTest extends \PHPUnit_Framework_TestCase
         $cartOperator->removeItem($cart, $item);
     }
 
+    /**
+     * @param object|null $cartManager
+     *
+     * @return \Sylius\Bundle\CartBundle\Operator\CartOperator
+     */
     private function getCartOperator($cartManager = null)
     {
         if (null === $cartManager) {
@@ -117,26 +127,41 @@ class CartOperatorTest extends \PHPUnit_Framework_TestCase
         ;
     }
 
+    /**
+     * @return \Sylius\Bundle\CartBundle\Model\Cart
+     */
     private function getCart()
     {
         return $this->getMockForAbstractClass('Sylius\Bundle\CartBundle\Model\Cart');
     }
 
+    /**
+     * @return \Sylius\Bundle\CartBundle\Model\Item
+     */
     private function getItem()
     {
         return $this->getMockForAbstractClass('Sylius\Bundle\CartBundle\Model\Item');
     }
 
+    /**
+     * @return \Sylius\Bundle\CartBundle\Model\CartManagerInterface
+     */
     private function getMockCartManager()
     {
         return $this->getMock('Sylius\Bundle\CartBundle\Model\CartManagerInterface');
     }
 
+    /**
+     * @return \Sylius\Bundle\CartBundle\Model\CartInterface
+     */
     private function getMockCart()
     {
         return $this->getMock('Sylius\Bundle\CartBundle\Model\CartInterface');
     }
 
+    /**
+     * @return \Sylius\Bundle\CartBundle\Model\ItemInterface
+     */
     private function getMockItem()
     {
         return $this->getMock('Sylius\Bundle\CartBundle\Model\ItemInterface');

src/Sylius/Bundle/CartBundle/Tests/autoload.php.dist

@@ -9,23 +9,10 @@
  * file that was distributed with this source code.
  */
 
-if (!file_exists($file = __DIR__.'/../vendor/.composer/autoload.php')) {
+if (file_exists($file = __DIR__.'/../vendor/autoload.php')) {
-
+   $autoload = require_once $file;
-$vendorDir = __DIR__.'/../vendor';
-require_once $vendorDir.'/symfony/src/Symfony/Component/ClassLoader/UniversalClassLoader.php';
-
-$loader = new \Symfony\Component\ClassLoader\UniversalClassLoader();
-$loader->registerNamespaces(array(
-    'Symfony'                => array($vendorDir.'/symfony/src', $vendorDir.'/bundles'),
-    'Doctrine\\Common'       => $vendorDir.'/doctrine-common/lib',
-    'Doctrine\\DBAL'         => $vendorDir.'/doctrine-dbal/lib',
-    'Doctrine'               => $vendorDir.'/doctrine/lib',
-));
-
-$loader->register();
-
 } else {
-    require_once $file;
+   throw new RuntimeException('Install dependencies to run test suite.');
 }
 
 spl_autoload_register(function($class) {