From 3d790db5596a76e1c3b2832f179c12ceaf018f63 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jan=20B=C3=B6hmer?= Date: Tue, 17 Sep 2019 13:57:40 +0200 Subject: [PATCH] Fixed orphanRemoval problem with parts collection. Also ElementPermissionListener was improved, and multiple special cases were unified. --- src/Entity/Parts/Part.php | 5 +- src/Entity/Parts/PartTraits/InstockTrait.php | 2 +- src/Entity/Parts/PartTraits/OrderTrait.php | 2 +- .../ElementPermissionListener.php | 75 +++++++++---------- 4 files changed, 38 insertions(+), 46 deletions(-) diff --git a/src/Entity/Parts/Part.php b/src/Entity/Parts/Part.php index 641aa615..53a862af 100644 --- a/src/Entity/Parts/Part.php +++ b/src/Entity/Parts/Part.php @@ -78,9 +78,6 @@ use Symfony\Component\Validator\Constraints as Assert; /** * Part class. * - * DONT USE orphanRemoval on properties with ColumnSecurity!! An empty collection will be created as placeholder, - * and the partlots are deleted, even if we want dont want that! - * * The class properties are split over various traits in directory PartTraits. * Otherwise this class would be too big, to be maintained. * @@ -125,7 +122,7 @@ class Part extends AttachmentContainingDBElement protected $name = ''; /** - * @ORM\OneToMany(targetEntity="App\Entity\Attachments\PartAttachment", mappedBy="element", cascade={"persist", "remove"}, orphanRemoval=false) + * @ORM\OneToMany(targetEntity="App\Entity\Attachments\PartAttachment", mappedBy="element", cascade={"persist", "remove"}, orphanRemoval=true) * @ColumnSecurity(type="collection", prefix="attachments") * @Assert\Valid() */ diff --git a/src/Entity/Parts/PartTraits/InstockTrait.php b/src/Entity/Parts/PartTraits/InstockTrait.php index bc423e6f..1210a232 100644 --- a/src/Entity/Parts/PartTraits/InstockTrait.php +++ b/src/Entity/Parts/PartTraits/InstockTrait.php @@ -45,7 +45,7 @@ trait InstockTrait { /** * @var ?PartLot[]|Collection A list of part lots where this part is stored - * @ORM\OneToMany(targetEntity="PartLot", mappedBy="part", cascade={"persist", "remove"}, orphanRemoval=false) + * @ORM\OneToMany(targetEntity="PartLot", mappedBy="part", cascade={"persist", "remove"}, orphanRemoval=true) * @Assert\Valid() * @ColumnSecurity(type="collection", prefix="lots") */ diff --git a/src/Entity/Parts/PartTraits/OrderTrait.php b/src/Entity/Parts/PartTraits/OrderTrait.php index 8cae9618..4ba5c9ff 100644 --- a/src/Entity/Parts/PartTraits/OrderTrait.php +++ b/src/Entity/Parts/PartTraits/OrderTrait.php @@ -45,7 +45,7 @@ trait OrderTrait { /** * @var Orderdetail[] The details about how and where you can order this part. - * @ORM\OneToMany(targetEntity="App\Entity\PriceInformations\Orderdetail", mappedBy="part", cascade={"persist", "remove"}, orphanRemoval=false) + * @ORM\OneToMany(targetEntity="App\Entity\PriceInformations\Orderdetail", mappedBy="part", cascade={"persist", "remove"}, orphanRemoval=true) * @Assert\Valid() * @ColumnSecurity(prefix="orderdetails", type="collection") */ diff --git a/src/Security/EntityListeners/ElementPermissionListener.php b/src/Security/EntityListeners/ElementPermissionListener.php index 011ff9b8..9a1e7059 100644 --- a/src/Security/EntityListeners/ElementPermissionListener.php +++ b/src/Security/EntityListeners/ElementPermissionListener.php @@ -32,6 +32,7 @@ namespace App\Security\EntityListeners; use App\Entity\Base\DBElement; use App\Security\Annotations\ColumnSecurity; use Doctrine\Common\Annotations\Reader; +use Doctrine\Common\Collections\Collection; use Doctrine\Common\Persistence\Event\LifecycleEventArgs; use Doctrine\ORM\EntityManager; use Doctrine\ORM\EntityManagerInterface; @@ -65,10 +66,13 @@ class ElementPermissionListener /** * @PostLoad - * + * @ORM\PostUpdate() * This function is called after doctrine filled, the entity properties with db values. * We use this, to check if the user is allowed to access these properties, and if not, we write a placeholder * into the element properties, so that a user only gets non sensitve data. + * + * This function is also called after an entity was updated, so we dont show the original data to user, + * after an update. */ public function postLoadHandler(DBElement $element, LifecycleEventArgs $event) { @@ -80,16 +84,21 @@ class ElementPermissionListener /** * @var ColumnSecurity */ - $annotation = $this->reader->getPropertyAnnotation($property, - ColumnSecurity::class); + $annotation = $this->reader->getPropertyAnnotation( + $property, + ColumnSecurity::class + ); //Check if user is allowed to read info, otherwise apply placeholder if ((null !== $annotation) && !$this->security->isGranted($annotation->getReadOperationName(), $element)) { $property->setAccessible(true); $value = $annotation->getPlaceholder(); - if($value instanceof DBElement) { + + //Detach placeholder entities, so we dont get cascade errors + if ($value instanceof DBElement) { $this->em->detach($value); } + $property->setValue($element, $value); } } @@ -97,58 +106,44 @@ class ElementPermissionListener /** * @ORM\PreFlush() - * This function is called before flushing. We use it, to remove all placeholder DBElements (with name=???), - * so we dont get a no cascade persistance error. + * This function is called before flushing. We use it, to remove all placeholders. + * We do it here and not in preupdate, because this is called before calculating the changeset, + * and so we dont get problems with orphan removal. */ public function preFlushHandler(DBElement $element, PreFlushEventArgs $eventArgs) { - //$eventArgs->getEntityManager()->getUnitOfWork()-> + $em = $eventArgs->getEntityManager(); + $unitOfWork = $eventArgs->getEntityManager()->getUnitOfWork(); $reflectionClass = new ReflectionClass($element); $properties = $reflectionClass->getProperties(); + $old_data = $unitOfWork->getOriginalEntityData($element); + foreach ($properties as $property) { $annotation = $this->reader->getPropertyAnnotation( $property, ColumnSecurity::class ); + + $changed = false; + + //Only set the field if it has an annotation if (null !== $annotation) { - //Check if the current property is an DBElement $property->setAccessible(true); - $value = $property->getValue($element); - if ($value instanceof DBElement && !$this->security->isGranted($annotation->getEditOperationName(), $element)) { - $property->setValue($element, null); + + //If the user is not allowed to edit or read this property, reset all values. + if ((!$this->security->isGranted($annotation->getEditOperationName(), $element) + || !$this->security->isGranted($annotation->getReadOperationName(), $element))) { + //Set value to old value, so that there a no change to this property + $property->setValue($element, $old_data[$property->getName()]); + $changed = true; + } - } - } - } - /** - * @PreUpdate - * This function is called before Doctrine saves the property values to the database. - * We use this function to revert the changes made in postLoadHandler(), so nothing gets changed persistently. - */ - public function preUpdateHandler(DBElement $element, PreUpdateEventArgs $event) - { - $reflectionClass = new ReflectionClass($element); - $properties = $reflectionClass->getProperties(); - - foreach ($properties as $property) { - /** - * @var ColumnSecurity $annotation - */ - $annotation = $this->reader->getPropertyAnnotation($property, - ColumnSecurity::class); - - if (null !== $annotation) { - $field_name = $property->getName(); - - //Check if user is allowed to edit info, otherwise overwrite the new value - // so that nothing is changed in the DB. - if ($event->hasChangedField($field_name) && - (!$this->security->isGranted($annotation->getEditOperationName(), $element) - || !$this->security->isGranted($annotation->getReadOperationName(), $element))) { - $event->setNewValue($field_name, $event->getOldValue($field_name)); + if ($changed) { + //Schedule for update, so the post update method will be called + $unitOfWork->scheduleForUpdate($element); } } }