How should I check for dupplicate entity in an object collection?

Question

I've been reading this post : Doctrine - how to check if a collection contains an entity

But I actually don't like the solution, as, doctrine already provide the contains() method, which have the advantage to keep logic directly into the object, and then to not load EXTRA_LAZY collections entirely.

So here a Cart Entity own a CartProduct collection as is :

/**
 * ...
 * @ORM\Entity(repositoryClass="App\Repository\CartRepository")
 */
abstract class Cart implements InheritanceInterface{
    ...
    /**
     * @ORM\OneToMany(targetEntity="CartProduct", mappedBy="cart", fetch="EXTRA_LAZY", cascade={"persist"})
     */
    private Collection $cartProducts;
    ...
    public function __construct()
    {
        $this->cartProducts = new ArrayCollection();
    }
    ...
}

(CartProduct have to be an Entity look at this simplify EA model. That's a standard way to proceed for related entity holding extra fields)

Now I want to add a new ProductCart Entity to my Cart class.

So I'm adding this method (generated by Symfony make:entity) :

abstract class Cart implements InheritanceInterface{
...
    public function addCartProduct(CartProduct $cartProduct): self
    {
        if(!$this->getCartProducts()->contains($cartProduct)) {
            $this->cartProducts->add($cartProduct);
            $cartProduct->setCart($this);
        }
        return $this;
    }
...

And then I test this code :

    public function testAddCartProduct()
    {
        $cart = new ShoppingCart($this->createMock(ShoppingCartState::class));
        $cart_product = new CartProduct();
        $cart_product->setProduct(new Product(self::NO_.'1', new Group('1')));
        $cart->addCartProduct($cart_product);
        $cart_product2 = new CartProduct();
        $cart_product2->setProduct(new Product(self::NO_.'1', new Group('1')));
        $cart->addCartProduct($cart_product2);
        $this->assertCount(1, $cart->getCartProducts());
    }

But when I run this test, it fail :

Failed asserting that actual size 2 matches expected size 1.

So I check, and the Cart.cartProducts Collection have two product which are exactly the same objects.

As it's an ArrayCollection, I suppose that it just use this method :

namespace Doctrine\Common\Collections;
class ArrayCollection implements Collection, Selectable {
    ...
    public function contains($element)
    {
        return in_array($element, $this->elements, true);
    }

So well, of course in this case it is just return false, And the objects are considered to be different.

So now, I wish I could use PersistentCollection instead of ArrayCollection when implementing the Collection object , because the PersistentCollection.contains() method looks better.

abstract class Cart implements InheritanceInterface{
...
 public function __construct()
    {
--      $this->cartProducts = new ArrayCollection();
++      $this->cartProducts = new PersistentCollection(...);
    }
}

But this require an EntityManager as a parameter, so, seams a little bit overkill to give an EntityManager to an Entity object...

So I finally, I don't know what is the better way to check for a dupplicate entity inside a collection.

Of course, I could implement myself a thing like :

abstract class Cart implements InheritanceInterface{
...
    public function addCartProduct(CartProduct $cartProduct): self
    {
        if(!$this->getCartProducts()->filter(
            function (CartProduct $cp)use($cartProduct){
                return $cp->getId() === $cartProduct->getId();
            })->count()) {
            $this->cartProducts->add($cartProduct);
            $cartProduct->setCart($this);
        }
        return $this;
    }
...

But it'll require to load every Entity and I really don't like the idea.

Answer 1

Personally I agree with your comment, I don't think the entity itself should have the responsibility to ensure there is no duplicate.

The entity cannot make a request like a repository could, and I don't see how you can be sure there is no duplicate in the database without querying it.

Calling contains will not trigger a fetch in your case, this means the collection will stay as is, which is not what you want anyway because you could have a previously persisted duplicate that will not be part of the collection because you marked it as EXTRA_LAZY.

You also don't want to fetch all the entities of the collection (and transform the results into objects) just to check if you have a collision.

So IMHO you should create a method in the repository of the entity to check for duplicates, a simple SELECT COUNT(id).

Then there is your real problem.

The way you make your test will never find a collision. When you do:

$cart = new ShoppingCart($this->createMock(ShoppingCartState::class));
$cart_product = new CartProduct();
$cart_product->setProduct(new Product(self::NO_.'1', new Group('1')));
$cart->addCartProduct($cart_product);
$cart_product2 = new CartProduct();
$cart_product2->setProduct(new Product(self::NO_.'1', new Group('1')));
$cart->addCartProduct($cart_product2);
$this->assertCount(1, $cart->getCartProducts());

You are creating two instances of CartProduct, that's why the call to contains doesn't find anything.

Because contains checks for the object reference, not the content, like you can see in its implementation:

public function contains($element)
{
    return in_array($element, $this->elements, true);
}

So in your test case what you're really testing is:

in_array(new CartProduct(), [new CartProduct()], true);

which will always return false.