[Tree] [materializedPath] Deleting parent removes children when onDelete="restrict"

[FrKevin]

When onDelete="restrict", I don't know if this is the expected behaviour, but deleting the parent also deletes the children. I assumed I would get an SQL error, and the children are still in the database.

Example Entity

/**
 * @ORM\Entity(repositoryClass="Gedmo\Tree\Entity\Repository\MaterializedPathRepository")
 * @Gedmo\Tree(type="materializedPath")
 */
class Category
{
    /**
     * @ORM\Id
     * @ORM\GeneratedValue
     */
    private $id;

    /**
     * @Gedmo\TreePath
     * @ORM\Column(name="path", type="string", length=3000, nullable=true)
     */
    private $path;

    /**
     * @Gedmo\TreePathSource
     * @ORM\Column(name="title", type="string", length=64)
     */
    private $title;

    /**
     * @Gedmo\TreeParent
     * @ORM\ManyToOne(targetEntity="Category", inversedBy="children")
     * @ORM\JoinColumns({
     *   @ORM\JoinColumn(name="parent_id", referencedColumnName="id", onDelete="restrict")
     * })
     */
    private Category $parent;

    /**
     * @Gedmo\TreeLevel
     * @ORM\Column(name="lvl", type="integer", nullable=true)
     */
    private $level;

    /**
     * @ORM\OneToMany(targetEntity="Category", mappedBy="parent")
     */
    private $children;

    public function getId()
    {
        return $this->id;
    }

    public function setTitle($title)
    {
        $this->title = $title;
    }

    public function getTitle()
    {
        return $this->title;
    }

    public function setParent(Category $parent = null)
    {
        $this->parent = $parent;
    }

    public function getParent()
    {
        return $this->parent;
    }

    public function setPath($path)
    {
        $this->path = $path;
    }

    public function getPath()
    {
        return $this->path;
    }

    public function getLevel()
    {
        return $this->level;
    }
}

Example of PHP Code

 $em = $this->getDoctrine()->getEntityManager();
 
 $category1 = (new Category())->setTitle('category_1');
 $category2 = (new Category())->setTitle('category_2')->setParent($category1);

$em->persist($category1);
$em->persist($category2);

$em->flush();

$em->remove($category1); 
$em->flush();

With this code I no longer get fatal postgresql error, $category1 and $category2 deleted even though parent is set to restrict delete.

Searching through the code, I found the function that I think is broken :

• https://github.com/doctrine-extensions/DoctrineExtensions/blob/main/src/Tree/TreeListener.php#L179
• https://github.com/doctrine-extensions/DoctrineExtensions/blob/main/src/Tree/Strategy/ORM/MaterializedPath.php#L31

Here we delete all the children. We do not check if the parent is set to restrict delete.

[mbabker]

IIRC the JoinColumn::$onDelete config is only used to configure the foreign key column in the database, the ORM doesn't use that to make any decisions about mapping relationships. Actually, the only place I can find the ORM using that information is in the UnitOfWork when it is determining the order of operations for DELETE statements, and it only takes into consideration "cascade" and "set null".

It does look like right now it's known the ORM might not work as expected with onDelete="restrict", see doctrine/orm#10783 for that.

So I don't think we have an actionable bug within this package at the moment, if something changes in the ORM around this situation then things can be updated here if need be.

[joosee7]

It seems there might be some confusion here. While it's true that JoinColumn::$onDelete primarily configures the behavior of the foreign key column in the database, its impact extends beyond that. In MySQL, onDelete="restrict" should enforce that a referenced row cannot be deleted while it's still referenced by another table.

However, if this isn't being reflected as expected in the ORM, it could indeed be considered a bug. While the ORM may currently handle onDelete="cascade" and "set null" appropriately, the fact that onDelete="restrict" isn't behaving as anticipated could signify a bug.

The reported behavior should ideally align with database behavior for consistency and predictability.

[github-actions]

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.

[FrKevin]

When onDelete="restrict", I don't know if this is the expected behaviour, but deleting the parent also deletes the children. I assumed I would get an SQL error, and the children are still in the database.

Example Entity
Example of PHP Code

 $em = $this->getDoctrine()->getEntityManager();
 
 $category1 = (new Category())->setTitle('category_1');
 $category2 = (new Category())->setTitle('category_2')->setParent($category1);

$em->persist($category1);
$em->persist($category2);

$em->flush();

$em->remove($category1); 
$em->flush();

With this code I no longer get fatal postgresql error, $category1 and $category2 deleted even though parent is set to restrict delete.

Searching through the code, I found the function that I think is broken :

• https://github.com/doctrine-extensions/DoctrineExtensions/blob/main/src/Tree/TreeListener.php#L179
• https://github.com/doctrine-extensions/DoctrineExtensions/blob/main/src/Tree/Strategy/ORM/MaterializedPath.php#L31

Here we delete all the children. We do not check if the parent is set to restrict delete.

For me it's a bug,

Without Tree : we have fatal postgresql error
With tree : I no longer get fatal postgresql error, $category1 and $category2 deleted even though parent is set to restrict delete.

[mbabker]

For me it's a bug

In all fairness, I do agree there is a bug here. But, IMO, that bug exists because the ORM itself also doesn't support onDelete="restrict" configurations. It would be better for the ORM to add support versus trying to monkey-patch around it in this library (which, if I remember right from the quick search I did on my first comment, the ORM handles other cascades directly in the UnitOfWork and that's not something a downstream package can easily hook into).