From f332acb8b97b1ac6ef247ddb3354a38fde8c3801 Mon Sep 17 00:00:00 2001 From: lukmzig Date: Fri, 21 Mar 2025 11:33:57 +0100 Subject: [PATCH 1/3] add catch for full path exception --- src/DataObject/Service/DataObjectService.php | 17 ++--------------- .../Service/DataObjectServiceInterface.php | 5 ----- src/Exception/Api/ElementExistsException.php | 2 +- src/Patcher/Adapter/KeyAdapter.php | 2 +- src/Patcher/Service/PatchService.php | 5 +++++ src/Updater/Service/UpdateService.php | 5 +++++ src/Util/Trait/ElementProviderTrait.php | 7 +++++++ 7 files changed, 21 insertions(+), 22 deletions(-) diff --git a/src/DataObject/Service/DataObjectService.php b/src/DataObject/Service/DataObjectService.php index 886167cc4..dd3d82da1 100644 --- a/src/DataObject/Service/DataObjectService.php +++ b/src/DataObject/Service/DataObjectService.php @@ -91,13 +91,6 @@ public function __construct( ) { } - public function getDataObjectFullPath( - string $parentFullPath, - string $key - ): string { - return str_ends_with($parentFullPath, '/') === true ? $parentFullPath . $key : $parentFullPath . '/' . $key; - } - /** * @throws DatabaseException * @throws ElementSavingFailedException @@ -112,15 +105,9 @@ public function addDataObject( ): int { $user = $this->securityService->getCurrentUser(); $parent = $this->getValidParent($user, $parentId); - $fullPath = $this->getDataObjectFullPath( - $parent->getFullPath(), - $parameters->getKey() - ); + $fullPath = $this->getElementFullPath($parent->getFullPath(), $parameters->getKey()); if ($this->dataObjectServiceResolver->pathExists($fullPath)) { - throw new ElementExistsException( - $fullPath, - HttpResponseErrorKeys::ELEMENT_EXISTS->value - ); + throw new ElementExistsException($fullPath, HttpResponseErrorKeys::ELEMENT_EXISTS->value); } $class = $this->getValidClass($this->classDefinitionResolver, $parameters->getClassId()); diff --git a/src/DataObject/Service/DataObjectServiceInterface.php b/src/DataObject/Service/DataObjectServiceInterface.php index 1296249c9..2f12b62f3 100644 --- a/src/DataObject/Service/DataObjectServiceInterface.php +++ b/src/DataObject/Service/DataObjectServiceInterface.php @@ -99,9 +99,4 @@ public function getDataObjectElementByPath( * @throws ForbiddenException|InvalidQueryTypeException|NotFoundException|UserNotFoundException */ public function setTreeSorting(DataObjectModel $parent, QueryInterface $dataObjectQuery): void; - - public function getDataObjectFullPath( - string $parentFullPath, - string $key - ): string; } diff --git a/src/Exception/Api/ElementExistsException.php b/src/Exception/Api/ElementExistsException.php index 4baa5a940..4499f82e1 100644 --- a/src/Exception/Api/ElementExistsException.php +++ b/src/Exception/Api/ElementExistsException.php @@ -28,7 +28,7 @@ final class ElementExistsException extends AbstractApiException public function __construct(?string $error = null, string $errorKey = HttpResponseErrorKeys::ELEMENT_EXISTS->value) { $message = sprintf( - 'Failed to create new element: %s', + 'Failed to create/update element: %s', $error ?? 'Unknown error' ); diff --git a/src/Patcher/Adapter/KeyAdapter.php b/src/Patcher/Adapter/KeyAdapter.php index f51ff87ae..48fb3c70f 100644 --- a/src/Patcher/Adapter/KeyAdapter.php +++ b/src/Patcher/Adapter/KeyAdapter.php @@ -29,7 +29,7 @@ #[AutoconfigureTag(TaggedIteratorAdapter::ADAPTER_TAG)] final readonly class KeyAdapter implements PatchAdapterInterface { - private const INDEX_KEY = 'key'; + private const string INDEX_KEY = 'key'; public function patch(ElementInterface $element, array $data, UserInterface $user): void { diff --git a/src/Patcher/Service/PatchService.php b/src/Patcher/Service/PatchService.php index f34cb4bf4..e4ecc6556 100644 --- a/src/Patcher/Service/PatchService.php +++ b/src/Patcher/Service/PatchService.php @@ -27,6 +27,7 @@ use Pimcore\Bundle\StudioBackendBundle\Element\ExecutionEngine\Util\JobSteps; use Pimcore\Bundle\StudioBackendBundle\Element\Service\ElementSaveServiceInterface; use Pimcore\Bundle\StudioBackendBundle\Element\Service\ElementServiceInterface; +use Pimcore\Bundle\StudioBackendBundle\Exception\Api\ElementExistsException; use Pimcore\Bundle\StudioBackendBundle\Exception\Api\ElementSavingFailedException; use Pimcore\Bundle\StudioBackendBundle\Exception\Api\ForbiddenException; use Pimcore\Bundle\StudioBackendBundle\Exception\Api\InvalidArgumentException; @@ -36,9 +37,11 @@ use Pimcore\Bundle\StudioBackendBundle\MappedParameter\PatchFolderParameter; use Pimcore\Bundle\StudioBackendBundle\Updater\Service\UpdateServiceInterface; use Pimcore\Bundle\StudioBackendBundle\Util\Constant\DataObject\FieldKeys; +use Pimcore\Bundle\StudioBackendBundle\Util\Constant\HttpResponseErrorKeys; use Pimcore\Bundle\StudioBackendBundle\Util\Constant\PatchDataKeys; use Pimcore\Bundle\StudioBackendBundle\Util\Constant\PatcherActions; use Pimcore\Model\DataObject\Concrete; +use Pimcore\Model\Element\DuplicateFullPathException; use Pimcore\Model\Element\ElementDescriptor; use Pimcore\Model\Element\ElementInterface; use Pimcore\Model\UserInterface; @@ -143,6 +146,8 @@ public function patchElement( $user, $elementPatchData[ElementSaveServiceInterface::INDEX_TASK] ?? null ); + } catch (DuplicateFullPathException) { + throw new ElementExistsException($element->getRealFullPath(), HttpResponseErrorKeys::ELEMENT_EXISTS->value); } catch (Exception $exception) { throw new ElementSavingFailedException($element->getId(), $exception->getMessage()); } diff --git a/src/Updater/Service/UpdateService.php b/src/Updater/Service/UpdateService.php index 35f203942..8226390d3 100644 --- a/src/Updater/Service/UpdateService.php +++ b/src/Updater/Service/UpdateService.php @@ -21,13 +21,16 @@ use Pimcore\Bundle\StudioBackendBundle\DataObject\Service\DataServiceInterface; use Pimcore\Bundle\StudioBackendBundle\DataObject\Util\Trait\ValidateObjectDataTrait; use Pimcore\Bundle\StudioBackendBundle\Element\Service\ElementSaveServiceInterface; +use Pimcore\Bundle\StudioBackendBundle\Exception\Api\ElementExistsException; use Pimcore\Bundle\StudioBackendBundle\Exception\Api\ElementSavingFailedException; use Pimcore\Bundle\StudioBackendBundle\Exception\Api\FieldValidationFailedException; use Pimcore\Bundle\StudioBackendBundle\Exception\Api\NotFoundException; use Pimcore\Bundle\StudioBackendBundle\Security\Service\SecurityServiceInterface; +use Pimcore\Bundle\StudioBackendBundle\Util\Constant\HttpResponseErrorKeys; use Pimcore\Bundle\StudioBackendBundle\Util\Trait\ElementProviderTrait; use Pimcore\Model\DataObject\Concrete; use Pimcore\Model\Document; +use Pimcore\Model\Element\DuplicateFullPathException; use Pimcore\Model\Element\ElementInterface; use Pimcore\Model\Element\ValidationException; @@ -77,6 +80,8 @@ public function update(string $elementType, int $id, array $data): void try { $this->elementSaveService->save($element, $user, $task); + } catch (DuplicateFullPathException) { + throw new ElementExistsException($element->getRealFullPath(), HttpResponseErrorKeys::ELEMENT_EXISTS->value); } catch (ValidationException $e) { throw new FieldValidationFailedException($e->getMessage(), previous: $e); } catch (Exception $e) { diff --git a/src/Util/Trait/ElementProviderTrait.php b/src/Util/Trait/ElementProviderTrait.php index 755fdbd26..59b1f4297 100644 --- a/src/Util/Trait/ElementProviderTrait.php +++ b/src/Util/Trait/ElementProviderTrait.php @@ -125,4 +125,11 @@ private function getCoreElementType(string $type): string return $type; } + + private function getElementFullPath( + string $parentFullPath, + string $key + ): string { + return str_ends_with($parentFullPath, '/') === true ? $parentFullPath . $key : $parentFullPath . '/' . $key; + } } From bbff0fae1941f33e53bffe30fd827c14a14ba238 Mon Sep 17 00:00:00 2001 From: lukmzig Date: Mon, 24 Mar 2025 08:50:54 +0100 Subject: [PATCH 2/3] adapt exception message --- src/DataObject/Service/DataObjectService.php | 2 +- src/Element/Service/ElementFolderService.php | 4 ++-- src/Exception/Api/ElementExistsException.php | 16 +++++++++++----- src/Patcher/Service/PatchService.php | 4 +++- src/Updater/Service/UpdateService.php | 4 +++- 5 files changed, 20 insertions(+), 10 deletions(-) diff --git a/src/DataObject/Service/DataObjectService.php b/src/DataObject/Service/DataObjectService.php index dd3d82da1..b501764db 100644 --- a/src/DataObject/Service/DataObjectService.php +++ b/src/DataObject/Service/DataObjectService.php @@ -107,7 +107,7 @@ public function addDataObject( $parent = $this->getValidParent($user, $parentId); $fullPath = $this->getElementFullPath($parent->getFullPath(), $parameters->getKey()); if ($this->dataObjectServiceResolver->pathExists($fullPath)) { - throw new ElementExistsException($fullPath, HttpResponseErrorKeys::ELEMENT_EXISTS->value); + throw new ElementExistsException(error: $fullPath); } $class = $this->getValidClass($this->classDefinitionResolver, $parameters->getClassId()); diff --git a/src/Element/Service/ElementFolderService.php b/src/Element/Service/ElementFolderService.php index dc5e78d6b..61b089937 100644 --- a/src/Element/Service/ElementFolderService.php +++ b/src/Element/Service/ElementFolderService.php @@ -68,8 +68,8 @@ public function createFolderByType( if ($existingElement) { throw new ElementExistsException( - 'Folder already exists', - HttpResponseErrorKeys::FOLDER_EXISTS->value + error: 'Folder already exists', + errorKey: HttpResponseErrorKeys::FOLDER_EXISTS->value ); } diff --git a/src/Exception/Api/ElementExistsException.php b/src/Exception/Api/ElementExistsException.php index 4499f82e1..2e6bd948c 100644 --- a/src/Exception/Api/ElementExistsException.php +++ b/src/Exception/Api/ElementExistsException.php @@ -25,12 +25,18 @@ */ final class ElementExistsException extends AbstractApiException { - public function __construct(?string $error = null, string $errorKey = HttpResponseErrorKeys::ELEMENT_EXISTS->value) + public function __construct( + ?string $message = null, + ?string $error = null, + string $errorKey = HttpResponseErrorKeys::ELEMENT_EXISTS->value, + ) { - $message = sprintf( - 'Failed to create/update element: %s', - $error ?? 'Unknown error' - ); + if ($message === null) { + $message = sprintf( + 'Failed to create new element: %s', + $error ?? 'Unknown error' + ); + } parent::__construct( HttpResponseCodes::CONFLICT->value, diff --git a/src/Patcher/Service/PatchService.php b/src/Patcher/Service/PatchService.php index e4ecc6556..d4c8d30d5 100644 --- a/src/Patcher/Service/PatchService.php +++ b/src/Patcher/Service/PatchService.php @@ -147,7 +147,9 @@ public function patchElement( $elementPatchData[ElementSaveServiceInterface::INDEX_TASK] ?? null ); } catch (DuplicateFullPathException) { - throw new ElementExistsException($element->getRealFullPath(), HttpResponseErrorKeys::ELEMENT_EXISTS->value); + throw new ElementExistsException( + message: sprintf('Element with full path [%s] already exists', $element->getRealFullPath()) + ); } catch (Exception $exception) { throw new ElementSavingFailedException($element->getId(), $exception->getMessage()); } diff --git a/src/Updater/Service/UpdateService.php b/src/Updater/Service/UpdateService.php index 8226390d3..d0c9c0fe5 100644 --- a/src/Updater/Service/UpdateService.php +++ b/src/Updater/Service/UpdateService.php @@ -81,7 +81,9 @@ public function update(string $elementType, int $id, array $data): void try { $this->elementSaveService->save($element, $user, $task); } catch (DuplicateFullPathException) { - throw new ElementExistsException($element->getRealFullPath(), HttpResponseErrorKeys::ELEMENT_EXISTS->value); + throw new ElementExistsException( + message: sprintf('Element with full path [%s] already exists', $element->getRealFullPath()) + ); } catch (ValidationException $e) { throw new FieldValidationFailedException($e->getMessage(), previous: $e); } catch (Exception $e) { From 67cd994bb1e211848e16b4e6bf625a3b9c93a2a5 Mon Sep 17 00:00:00 2001 From: lukmzig <30526586+lukmzig@users.noreply.github.com> Date: Mon, 24 Mar 2025 08:58:41 +0000 Subject: [PATCH 3/3] Apply php-cs-fixer changes --- src/DataObject/Service/DataObjectService.php | 1 - src/Exception/Api/ElementExistsException.php | 3 +-- src/Patcher/Service/PatchService.php | 2 +- src/Updater/Service/UpdateService.php | 2 +- 4 files changed, 3 insertions(+), 5 deletions(-) diff --git a/src/DataObject/Service/DataObjectService.php b/src/DataObject/Service/DataObjectService.php index b501764db..e44a2dbd2 100644 --- a/src/DataObject/Service/DataObjectService.php +++ b/src/DataObject/Service/DataObjectService.php @@ -48,7 +48,6 @@ use Pimcore\Bundle\StudioBackendBundle\Util\Constant\ElementPermissions; use Pimcore\Bundle\StudioBackendBundle\Util\Constant\ElementTypes; use Pimcore\Bundle\StudioBackendBundle\Util\Constant\HttpResponseCodes; -use Pimcore\Bundle\StudioBackendBundle\Util\Constant\HttpResponseErrorKeys; use Pimcore\Bundle\StudioBackendBundle\Util\Trait\ElementProviderTrait; use Pimcore\Bundle\StudioBackendBundle\Util\Trait\UserPermissionTrait; use Pimcore\Model\DataObject\AbstractObject; diff --git a/src/Exception/Api/ElementExistsException.php b/src/Exception/Api/ElementExistsException.php index 2e6bd948c..cf9854935 100644 --- a/src/Exception/Api/ElementExistsException.php +++ b/src/Exception/Api/ElementExistsException.php @@ -29,8 +29,7 @@ public function __construct( ?string $message = null, ?string $error = null, string $errorKey = HttpResponseErrorKeys::ELEMENT_EXISTS->value, - ) - { + ) { if ($message === null) { $message = sprintf( 'Failed to create new element: %s', diff --git a/src/Patcher/Service/PatchService.php b/src/Patcher/Service/PatchService.php index d4c8d30d5..a62bfc589 100644 --- a/src/Patcher/Service/PatchService.php +++ b/src/Patcher/Service/PatchService.php @@ -37,7 +37,6 @@ use Pimcore\Bundle\StudioBackendBundle\MappedParameter\PatchFolderParameter; use Pimcore\Bundle\StudioBackendBundle\Updater\Service\UpdateServiceInterface; use Pimcore\Bundle\StudioBackendBundle\Util\Constant\DataObject\FieldKeys; -use Pimcore\Bundle\StudioBackendBundle\Util\Constant\HttpResponseErrorKeys; use Pimcore\Bundle\StudioBackendBundle\Util\Constant\PatchDataKeys; use Pimcore\Bundle\StudioBackendBundle\Util\Constant\PatcherActions; use Pimcore\Model\DataObject\Concrete; @@ -47,6 +46,7 @@ use Pimcore\Model\UserInterface; use function array_key_exists; use function count; +use function sprintf; /** * @internal diff --git a/src/Updater/Service/UpdateService.php b/src/Updater/Service/UpdateService.php index d0c9c0fe5..f4da76bae 100644 --- a/src/Updater/Service/UpdateService.php +++ b/src/Updater/Service/UpdateService.php @@ -26,13 +26,13 @@ use Pimcore\Bundle\StudioBackendBundle\Exception\Api\FieldValidationFailedException; use Pimcore\Bundle\StudioBackendBundle\Exception\Api\NotFoundException; use Pimcore\Bundle\StudioBackendBundle\Security\Service\SecurityServiceInterface; -use Pimcore\Bundle\StudioBackendBundle\Util\Constant\HttpResponseErrorKeys; use Pimcore\Bundle\StudioBackendBundle\Util\Trait\ElementProviderTrait; use Pimcore\Model\DataObject\Concrete; use Pimcore\Model\Document; use Pimcore\Model\Element\DuplicateFullPathException; use Pimcore\Model\Element\ElementInterface; use Pimcore\Model\Element\ValidationException; +use function sprintf; /** * @internal