From e0f65ffd88a14eb6312be64cf1a9ac8133df93e4 Mon Sep 17 00:00:00 2001 From: Pirmin Mattmann Date: Sat, 22 Jun 2024 14:42:51 +0200 Subject: [PATCH] unify AssertNoLoop-Validators --- api/src/Entity/ChecklistItem.php | 8 +++- api/src/Entity/ContentNode.php | 8 +++- api/src/Entity/HasParentInterface.php | 7 +++ .../{ContentNode => }/AssertNoLoop.php | 2 +- .../AssertNoLoopValidator.php | 8 ++-- .../Validator/ChecklistItem/AssertNoLoop.php | 10 ---- .../ChecklistItem/AssertNoLoopValidator.php | 46 ------------------- .../AssertNoLoopValidatorTest.php | 6 +-- 8 files changed, 27 insertions(+), 68 deletions(-) create mode 100644 api/src/Entity/HasParentInterface.php rename api/src/Validator/{ContentNode => }/AssertNoLoop.php (83%) rename api/src/Validator/{ContentNode => }/AssertNoLoopValidator.php (88%) delete mode 100644 api/src/Validator/ChecklistItem/AssertNoLoop.php delete mode 100644 api/src/Validator/ChecklistItem/AssertNoLoopValidator.php rename api/tests/Validator/{ContentNode => }/AssertNoLoopValidatorTest.php (95%) diff --git a/api/src/Entity/ChecklistItem.php b/api/src/Entity/ChecklistItem.php index c6c453806b8..f9a42a65c08 100644 --- a/api/src/Entity/ChecklistItem.php +++ b/api/src/Entity/ChecklistItem.php @@ -16,8 +16,8 @@ use App\InputFilter; use App\Repository\ChecklistItemRepository; use App\Util\EntityMap; +use App\Validator\AssertNoLoop; use App\Validator\ChecklistItem\AssertBelongsToChecklist; -use App\Validator\ChecklistItem\AssertNoLoop; use Doctrine\Common\Collections\ArrayCollection; use Doctrine\Common\Collections\Collection; use Doctrine\ORM\Mapping as ORM; @@ -67,7 +67,7 @@ )] #[ApiFilter(filterClass: SearchFilter::class, properties: ['checklist'])] #[ORM\Entity(repositoryClass: ChecklistItemRepository::class)] -class ChecklistItem extends BaseEntity implements BelongsToCampInterface, CopyFromPrototypeInterface { +class ChecklistItem extends BaseEntity implements BelongsToCampInterface, CopyFromPrototypeInterface, HasParentInterface { public const CHECKLIST_SUBRESOURCE_URI_TEMPLATE = '/checklists/{checklistId}/checklist_items.{_format}'; /** @@ -140,6 +140,10 @@ public function getCamp(): ?Camp { return $this->checklist?->getCamp(); } + public function getParent(): ?HasParentInterface { + return $this->parent; + } + /** * @return ChecklistItem[] */ diff --git a/api/src/Entity/ContentNode.php b/api/src/Entity/ContentNode.php index 0b29d177537..7c40ff24d48 100644 --- a/api/src/Entity/ContentNode.php +++ b/api/src/Entity/ContentNode.php @@ -14,9 +14,9 @@ use App\Util\ClassInfoTrait; use App\Util\EntityMap; use App\Util\JsonMergePatch; +use App\Validator\AssertNoLoop; use App\Validator\ContentNode\AssertAttachedToRoot; use App\Validator\ContentNode\AssertContentTypeCompatible; -use App\Validator\ContentNode\AssertNoLoop; use App\Validator\ContentNode\AssertNoRootChange; use App\Validator\ContentNode\AssertSlotSupportedByParent; use Doctrine\Common\Collections\ArrayCollection; @@ -49,7 +49,7 @@ #[ORM\InheritanceType('SINGLE_TABLE')] #[ORM\DiscriminatorColumn(name: 'strategy', type: 'string')] #[ORM\UniqueConstraint(name: 'contentnode_parentid_slot_position_unique', columns: ['parentid', 'slot', 'position'])] -abstract class ContentNode extends BaseEntity implements BelongsToContentNodeTreeInterface, CopyFromPrototypeInterface { +abstract class ContentNode extends BaseEntity implements BelongsToContentNodeTreeInterface, CopyFromPrototypeInterface, HasParentInterface { use ClassInfoTrait; /** @@ -180,6 +180,10 @@ public function getRoot(): ?ColumnLayout { return $this->root; } + public function getParent(): ?HasParentInterface { + return $this->parent; + } + /** * Holds the actual data of the content node. */ diff --git a/api/src/Entity/HasParentInterface.php b/api/src/Entity/HasParentInterface.php new file mode 100644 index 00000000000..87ec5c2d001 --- /dev/null +++ b/api/src/Entity/HasParentInterface.php @@ -0,0 +1,7 @@ +context->getObject(); - /** @var ContentNode $parent */ + /** @var HasParentInterface $parent */ $parent = $value; // $seen keeps track of all parents that we have visited. This is for a safety @@ -36,7 +36,7 @@ public function validate($value, Constraint $constraint): void { } $seen[] = $parent->getId(); - $parent = $parent->parent; + $parent = $parent->getParent(); } } } diff --git a/api/src/Validator/ChecklistItem/AssertNoLoop.php b/api/src/Validator/ChecklistItem/AssertNoLoop.php deleted file mode 100644 index f60e2bba67a..00000000000 --- a/api/src/Validator/ChecklistItem/AssertNoLoop.php +++ /dev/null @@ -1,10 +0,0 @@ -context->getObject(); - if (!$object instanceof ChecklistItem) { - throw new UnexpectedValueException($object, ChecklistItem::class); - } - - /** @var ChecklistItem $parent */ - $parent = $value; - - // $seen keeps track of all parents that we have visited. This is for a safety - // bailout mechanism to avoid an infinite loop in case there is flawed data in the DB - $seen = []; - - while (null !== $parent && !in_array($parent->getId(), $seen)) { - if ($parent->getId() === $object->getId()) { - $this->context->buildViolation($constraint->message) - ->addViolation() - ; - - return; - } - - $seen[] = $parent->getId(); - $parent = $parent->parent; - } - } -} diff --git a/api/tests/Validator/ContentNode/AssertNoLoopValidatorTest.php b/api/tests/Validator/AssertNoLoopValidatorTest.php similarity index 95% rename from api/tests/Validator/ContentNode/AssertNoLoopValidatorTest.php rename to api/tests/Validator/AssertNoLoopValidatorTest.php index ead7803414b..527f4bfb945 100644 --- a/api/tests/Validator/ContentNode/AssertNoLoopValidatorTest.php +++ b/api/tests/Validator/AssertNoLoopValidatorTest.php @@ -1,10 +1,10 @@