Doctrine 2 ManyToMany - the correct way

User.php

<?php

use Doctrine\ORM\Mapping as ORM;
use Doctrine\Common\Collections\ArrayCollection;

/**
 * @ORM\Entity()
 * @ORM\Table(name="user")
 */
class User
{
    /**
     * @var int|null
     * @ORM\Id()
     * @ORM\GeneratedValue(strategy="AUTO")
     * @ORM\Column(type="integer", name="id")
     */
    protected $id;

    /**
     * @var \Doctrine\Common\Collections\Collection|UserGroup[]
     *
     * @ORM\ManyToMany(targetEntity="UserGroup", inversedBy="users")
     * @ORM\JoinTable(
     *  name="user_usergroup",
     *  joinColumns={
     *      @ORM\JoinColumn(name="user_id", referencedColumnName="id")
     *  },
     *  inverseJoinColumns={
     *      @ORM\JoinColumn(name="usergroup_id", referencedColumnName="id")
     *  }
     * )
     */
    protected $userGroups;

    /**
     * Default constructor, initializes collections
     */
    public function __construct()
    {
        $this->userGroups = new ArrayCollection();
    }

    /**
     * @param UserGroup $userGroup
     */
    public function addUserGroup(UserGroup $userGroup)
    {
        if ($this->userGroups->contains($userGroup)) {
            return;
        }

        $this->userGroups->add($userGroup);
        $userGroup->addUser($this);
    }

    /**
     * @param UserGroup $userGroup
     */
    public function removeUserGroup(UserGroup $userGroup)
    {
        if (!$this->userGroups->contains($userGroup)) {
            return;
        }

        $this->userGroups->removeElement($userGroup);
        $userGroup->removeUser($this);
    }
}

UserGroup.php

<?php

use Doctrine\ORM\Mapping as ORM;
use Doctrine\Common\Collections\ArrayCollection;

/**
 * @ORM\Entity()
 * @ORM\Table(name="usergroup")
 */
class UserGroup
{
    /**
     * @var int|null
     * @ORM\Id()
     * @ORM\GeneratedValue(strategy="AUTO")
     * @ORM\Column(type="integer", name="id")
     */
    protected $id;

    /**
     * @var \Doctrine\Common\Collections\Collection|User[]
     *
     * @ORM\ManyToMany(targetEntity="User", mappedBy="userGroups")
     */
    protected $users;

    /**
     * Default constructor, initializes collections
     */
    public function __construct()
    {
        $this->users = new ArrayCollection();
    }

    /**
     * @param User $user
     */
    public function addUser(User $user)
    {
        if ($this->users->contains($user)) {
            return;
        }

        $this->users->add($user);
        $user->addUserGroup($this);
    }

    /**
     * @param User $user
     */
    public function removeUser(User $user)
    {
        if (!$this->users->contains($user)) {
            return;
        }

        $this->users->removeElement($user);
        $user->removeUserGroup($this);
    }
}

@nimasdj What is $admin? For your logic to function correctly per your example, you would use.

$user = $em->find('Entities\User', 1);
$userGroups = $user->getUserGroups();
foreach ($userGroups as $userGroup) {
    $userGroup->removeUser($user); //remove associated user from user groups
}
$em->remove($user);

However the $userGroup->removeUser($user) iteration is not needed if you have foreign key onDelete="CASCADE" specified on the entity join column (and in the database). Optionally you could also use orphan removal in your ManyToMany declaration to ensure the association is not recreated.

It would be helpful to have the same thing for OneToMany & ManyToOne (Bidirectional).

Thanks a lot +1

Its model-wise safe. But am I correct, that this causes initialization of the collections on both sides?
Example:
group-add-user: selects selects all users from group (contains($user)) and then inverse-side, selects all groups from user (contains($group)).

Does doctrine - on "contains" - just make a query to "check" or does it initialize a collection? If the second is the case, it does only contain proxies until you actually access an item, right?

Change
use Doctrine\Common\Collections\ArrayCollection; for
use Doctrine\Common\Collections\Collection;

and the methods:
public function getUser() or getUserGroups(), for example, return a array() and not a PersistentCollection.

A couple questions, maybe this is ignorance but...

1. Why do a negative check in the IF?
2. Why return nothing as opposed to true or false like the Collection methods do?

I see this a lot

    /**
     * @param User $user
     */
    public function addUser(User $user)
    {
        if (!$this->users->contains($user)) {
            $this->users[] = $user;
        }
    }

    /**
     * @param User $user
     */
    public function removeUser(User $user)
    {
        if ($this->users->contains($user)) {
            $this->users->removeElement($user);
        }
    }

And I am just curious, why not this instead?

    /**
     * @param User $user
     */
    public function addUser(User $user)
    {
        if ($this->users->contains($user)) {
            return true;
        }

        return $this->users->add($user);
    }

    /**
     * @param User $user
     */
    public function removeUser(User $user)
    {
        if ($this->users->contains($user)) {
            return $this->users->removeElement($user);
        }
        
        return true;
    }

This way, no matter what, the add and remove methods always return a boolean.

I am genuinely curious if there is some compatibility reasons behind it or if some of that functionality just didn't exist at one time and everyone is used to it or whatever it may be?

@kemo I think I found a way to improve performance for large tables. It looks like by default $collection->contains loads all the related objects in memory. I set the fetch option (in the ManyToMany binding) to EXTRA_LAZY and that seemed to reduce the loaded entities quite drastically! See: https://www.doctrine-project.org/projects/doctrine-orm/en/2.6/tutorials/extra-lazy-associations.html

@haydenk a specific return value is outside the scope for this gist, which is simply demonstrating how to handle the bi-directional object associations.
Ultimately the return values would depend on your desired behavior. You could also return $this; to allow for method chaining,
or return $this->someOtherMthod($object); to perform additional business logic and it's return value. Really is up to you.

@haydenk I also usually do returns as soon as I can so the processor doesn't have to read the entire function before continuing.
Also, not doing that you often see unneeded extra indentation in your whole function.

How would you query for a user that is in multiple groups?
eg.
User1 is in Group1 and Group2,
User2 is in Group1,
User3 is in Group2

The query should return only User1.

@werdck I would look into Doctrine specific methods. In that scenario the ORM would load the data for you and groups would be a relationship you can search through the array or collection.

https://www.doctrine-project.org/projects/doctrine-collections/en/stable/index.html