From b6b9d5818d331bd1e300d11bd8a017560d2102a2 Mon Sep 17 00:00:00 2001 From: lukmzig Date: Thu, 28 Nov 2024 10:08:55 +0100 Subject: [PATCH] fix: rather skip setter than use null value for it --- src/DataObject/Data/Adapter/BlockAdapter.php | 34 +++++++++++++++-- .../Adapter/ClassificationStoreAdapter.php | 15 +++++++- .../Data/Adapter/EncryptedFieldAdapter.php | 37 ++++++++++++++++--- .../Data/Adapter/FieldCollectionsAdapter.php | 15 +++++++- .../Data/Adapter/LocalizedFieldsAdapter.php | 29 ++++++++++----- .../Data/Adapter/ObjectBricksAdapter.php | 15 +++++++- src/DataObject/Service/DataAdapterService.php | 10 +++++ .../Service/DataAdapterServiceInterface.php | 2 + src/DataObject/Service/DataService.php | 30 +-------------- .../Service/DataServiceInterface.php | 8 ---- .../Util/Trait/ValidateFieldTypeTrait.php | 32 ++++++++++++++++ src/Updater/Service/UpdateService.php | 20 +++++++--- 12 files changed, 182 insertions(+), 65 deletions(-) create mode 100644 src/DataObject/Util/Trait/ValidateFieldTypeTrait.php diff --git a/src/DataObject/Data/Adapter/BlockAdapter.php b/src/DataObject/Data/Adapter/BlockAdapter.php index 17afbe37..c6ed0939 100644 --- a/src/DataObject/Data/Adapter/BlockAdapter.php +++ b/src/DataObject/Data/Adapter/BlockAdapter.php @@ -21,7 +21,9 @@ use Pimcore\Bundle\StudioBackendBundle\DataObject\Data\FieldContextData; use Pimcore\Bundle\StudioBackendBundle\DataObject\Data\SetterDataInterface; use Pimcore\Bundle\StudioBackendBundle\DataObject\Service\DataAdapterLoaderInterface; +use Pimcore\Bundle\StudioBackendBundle\DataObject\Service\DataAdapterServiceInterface; use Pimcore\Bundle\StudioBackendBundle\DataObject\Service\DataServiceInterface; +use Pimcore\Bundle\StudioBackendBundle\DataObject\Util\Trait\ValidateFieldTypeTrait; use Pimcore\Bundle\StudioBackendBundle\Exception\Api\InvalidDataTypeException; use Pimcore\Model\DataObject\ClassDefinition\Data; use Pimcore\Model\DataObject\ClassDefinition\Data\Block; @@ -37,7 +39,10 @@ #[AutoconfigureTag(DataAdapterLoaderInterface::ADAPTER_TAG)] final readonly class BlockAdapter implements SetterDataInterface, DataNormalizerInterface { + use ValidateFieldTypeTrait; + public function __construct( + private DataAdapterServiceInterface $dataAdapterService, private DataServiceInterface $dataService ) { } @@ -146,13 +151,24 @@ private function processBlockElement( $fieldContextData = $this->createFieldContextData($element, $fieldDefinition, $contextData); foreach ($fieldDefinitions as $elementName => $fd) { - $resultElement[$elementName] = $this->createBlockElement( + $adapter = $this->dataAdapterService->tryDataAdapter($fd->getFieldType()); + if (!$adapter) { + continue; + } + + $value = $this->createBlockElement( + $adapter, $element, $fd, $elementName, $blockElement, $fieldContextData ); + if (!$value) { + continue; + } + + $resultElement[$elementName] = $value; } return $resultElement; @@ -162,19 +178,31 @@ private function processBlockElement( * @throws Exception */ private function createBlockElement( + SetterDataInterface $adapter, Concrete $element, Data $fieldDefinition, string $elementName, ?array $blockElement, ?FieldContextData $fieldContextData = null - ): BlockElement { + ): ?BlockElement { $elementType = $fieldDefinition->getFieldtype(); $elementData = $blockElement[$elementName] ?? null; + $data = $adapter->getDataForSetter( + $element, + $fieldDefinition, + $elementName, + [$elementName => $elementData], + $fieldContextData + ); + if (!$this->validateEncryptedField($fieldDefinition, $data)) { + return null; + } + return new BlockElement( $elementName, $elementType, - $this->dataService->getAdapterSetterValue( + $adapter->getDataForSetter( $element, $fieldDefinition, $elementName, diff --git a/src/DataObject/Data/Adapter/ClassificationStoreAdapter.php b/src/DataObject/Data/Adapter/ClassificationStoreAdapter.php index ca9601a4..6d37d966 100644 --- a/src/DataObject/Data/Adapter/ClassificationStoreAdapter.php +++ b/src/DataObject/Data/Adapter/ClassificationStoreAdapter.php @@ -26,7 +26,9 @@ use Pimcore\Bundle\StudioBackendBundle\DataObject\Data\FieldContextData; use Pimcore\Bundle\StudioBackendBundle\DataObject\Data\SetterDataInterface; use Pimcore\Bundle\StudioBackendBundle\DataObject\Service\DataAdapterLoaderInterface; +use Pimcore\Bundle\StudioBackendBundle\DataObject\Service\DataAdapterServiceInterface; use Pimcore\Bundle\StudioBackendBundle\DataObject\Service\DataServiceInterface; +use Pimcore\Bundle\StudioBackendBundle\DataObject\Util\Trait\ValidateFieldTypeTrait; use Pimcore\Bundle\StudioBackendBundle\Exception\Api\DatabaseException; use Pimcore\Model\DataObject\ClassDefinition\Data; use Pimcore\Model\DataObject\ClassDefinition\Data\Classificationstore as ClassificationstoreDefinition; @@ -46,8 +48,11 @@ #[AutoconfigureTag(DataAdapterLoaderInterface::ADAPTER_TAG)] final readonly class ClassificationStoreAdapter implements SetterDataInterface, DataNormalizerInterface { + use ValidateFieldTypeTrait; + public function __construct( private DefinitionCacheResolverInterface $definitionCacheResolver, + private DataAdapterServiceInterface $dataAdapterService, private DataServiceInterface $dataService, private GroupConfigResolverInterface $groupConfigResolver, private ServiceResolverInterface $serviceResolver, @@ -180,12 +185,20 @@ private function processGroupKeys( continue; } - $setterData = $this->dataService->getAdapterSetterValue( + $adapter = $this->dataAdapterService->tryDataAdapter($fieldDefinition->getFieldType()); + if ($adapter === null) { + continue; + } + + $setterData = $adapter->getDataForSetter( $element, $fieldDefinition, $fieldDefinition->getName(), [$fieldDefinition->getName() => $value] ); + if (!$this->validateEncryptedField($fieldDefinition, $setterData)) { + continue; + } $container->setLocalizedKeyValue($groupId, $keyId, $setterData, $language); } diff --git a/src/DataObject/Data/Adapter/EncryptedFieldAdapter.php b/src/DataObject/Data/Adapter/EncryptedFieldAdapter.php index d88f01c1..af28f2d1 100644 --- a/src/DataObject/Data/Adapter/EncryptedFieldAdapter.php +++ b/src/DataObject/Data/Adapter/EncryptedFieldAdapter.php @@ -19,8 +19,9 @@ use Pimcore\Bundle\StudioBackendBundle\DataObject\Data\FieldContextData; use Pimcore\Bundle\StudioBackendBundle\DataObject\Data\SetterDataInterface; use Pimcore\Bundle\StudioBackendBundle\DataObject\Service\DataAdapterLoaderInterface; -use Pimcore\Bundle\StudioBackendBundle\DataObject\Service\DataServiceInterface; +use Pimcore\Bundle\StudioBackendBundle\DataObject\Service\DataAdapterServiceInterface; use Pimcore\Model\DataObject\ClassDefinition\Data; +use Pimcore\Model\DataObject\ClassDefinition\Data\EncryptedField as EncryptedFieldDefinition; use Pimcore\Model\DataObject\Concrete; use Pimcore\Model\DataObject\Data\EncryptedField; use Symfony\Component\DependencyInjection\Attribute\AutoconfigureTag; @@ -32,7 +33,7 @@ final readonly class EncryptedFieldAdapter implements SetterDataInterface { public function __construct( - private DataServiceInterface $dataService + private DataAdapterServiceInterface $dataAdapterService ) { } @@ -43,7 +44,7 @@ public function getDataForSetter( array $data, ?FieldContextData $contextData = null ): ?EncryptedField { - if (!($fieldDefinition instanceof Data\EncryptedField)) { + if (!$fieldDefinition instanceof EncryptedFieldDefinition) { return null; } @@ -52,9 +53,33 @@ public function getDataForSetter( return null; } - return new EncryptedField( - $fieldDefinition->getDelegate(), - $this->dataService->getAdapterSetterValue($element, $delegateFieldDefinition, $key, $data, $contextData) + return $this->handleDelegatedField( + $element, + $delegateFieldDefinition, + $fieldDefinition, + $key, + $data, + $contextData ); } + + private function handleDelegatedField( + Concrete $element, + Data $delegateFieldDefinition, + EncryptedFieldDefinition $fieldDefinition, + string $key, + array $data, + ?FieldContextData $contextData = null + ): ?EncryptedField + { + $adapter = $this->dataAdapterService->tryDataAdapter($fieldDefinition->getFieldType()); + if ($adapter instanceof SetterDataInterface) { + return new EncryptedField( + $fieldDefinition->getDelegate(), + $adapter->getDataForSetter($element, $delegateFieldDefinition, $key, $data, $contextData) + ); + } + + return null; + } } diff --git a/src/DataObject/Data/Adapter/FieldCollectionsAdapter.php b/src/DataObject/Data/Adapter/FieldCollectionsAdapter.php index 446de1aa..155b203c 100644 --- a/src/DataObject/Data/Adapter/FieldCollectionsAdapter.php +++ b/src/DataObject/Data/Adapter/FieldCollectionsAdapter.php @@ -22,7 +22,9 @@ use Pimcore\Bundle\StudioBackendBundle\DataObject\Data\FieldContextData; use Pimcore\Bundle\StudioBackendBundle\DataObject\Data\SetterDataInterface; use Pimcore\Bundle\StudioBackendBundle\DataObject\Service\DataAdapterLoaderInterface; +use Pimcore\Bundle\StudioBackendBundle\DataObject\Service\DataAdapterServiceInterface; use Pimcore\Bundle\StudioBackendBundle\DataObject\Service\DataServiceInterface; +use Pimcore\Bundle\StudioBackendBundle\DataObject\Util\Trait\ValidateFieldTypeTrait; use Pimcore\Model\DataObject\ClassDefinition\Data; use Pimcore\Model\DataObject\ClassDefinition\Data\Fieldcollections; use Pimcore\Model\DataObject\Concrete; @@ -37,7 +39,10 @@ #[AutoconfigureTag(DataAdapterLoaderInterface::ADAPTER_TAG)] final readonly class FieldCollectionsAdapter implements SetterDataInterface, DataNormalizerInterface { + use ValidateFieldTypeTrait; + public function __construct( + private DataAdapterServiceInterface $dataAdapterService, private DataServiceInterface $dataService, private DefinitionResolverInterface $fieldCollectionDefinition, private Factory $modelFactory @@ -144,17 +149,23 @@ private function processCollectionRaw( foreach ($collectionDef?->getFieldDefinitions() as $elementName => $fd) { $elementValue = $blockElement[$elementName] ?? null; - if (!$elementValue) { + $adapter = $this->dataAdapterService->tryDataAdapter($fd->getFieldType()); + if (!$elementValue || !$adapter) { continue; } - $collectionData[$elementName] = $this->dataService->getAdapterSetterValue( + $value = $adapter->getDataForSetter( $element, $fd, $elementName, [$elementName => $elementValue], $fieldContextData ); + if (!$this->validateEncryptedField($fieldDefinition, $value)) { + continue; + } + + $collectionData[$elementName] = $value; } return $collectionData; diff --git a/src/DataObject/Data/Adapter/LocalizedFieldsAdapter.php b/src/DataObject/Data/Adapter/LocalizedFieldsAdapter.php index 905805fb..af5c9673 100644 --- a/src/DataObject/Data/Adapter/LocalizedFieldsAdapter.php +++ b/src/DataObject/Data/Adapter/LocalizedFieldsAdapter.php @@ -21,7 +21,9 @@ use Pimcore\Bundle\StudioBackendBundle\DataObject\Data\FieldContextData; use Pimcore\Bundle\StudioBackendBundle\DataObject\Data\SetterDataInterface; use Pimcore\Bundle\StudioBackendBundle\DataObject\Service\DataAdapterLoaderInterface; +use Pimcore\Bundle\StudioBackendBundle\DataObject\Service\DataAdapterServiceInterface; use Pimcore\Bundle\StudioBackendBundle\DataObject\Service\DataServiceInterface; +use Pimcore\Bundle\StudioBackendBundle\DataObject\Util\Trait\ValidateFieldTypeTrait; use Pimcore\Bundle\StudioBackendBundle\Exception\Api\DatabaseException; use Pimcore\Bundle\StudioBackendBundle\Exception\Api\InvalidArgumentException; use Pimcore\Bundle\StudioBackendBundle\Security\Service\SecurityServiceInterface; @@ -42,7 +44,10 @@ #[AutoconfigureTag(DataAdapterLoaderInterface::ADAPTER_TAG)] final readonly class LocalizedFieldsAdapter implements SetterDataInterface, DataNormalizerInterface { + use ValidateFieldTypeTrait; + public function __construct( + private DataAdapterServiceInterface $dataAdapterService, private DataServiceInterface $dataService, private SecurityServiceInterface $securityService, ) { @@ -73,17 +78,23 @@ public function getDataForSetter( continue; } - $localizedField->setLocalizedValue( + $adapter = $this->dataAdapterService->tryDataAdapter($childFieldDefinition->getFieldType()); + if (!$adapter) { + continue; + } + + $value = $adapter->getDataForSetter( + $element, + $childFieldDefinition, $name, - $this->dataService->getAdapterSetterValue( - $element, - $childFieldDefinition, - $name, - [$name => $fieldData], - new FieldContextData(language: $language) - ), - $language + [$name => $fieldData], + new FieldContextData(language: $language) ); + if (!$this->validateEncryptedField($childFieldDefinition, $value)) { + continue; + } + + $localizedField->setLocalizedValue($name, $value, $language); } } diff --git a/src/DataObject/Data/Adapter/ObjectBricksAdapter.php b/src/DataObject/Data/Adapter/ObjectBricksAdapter.php index b5a24bae..c472000f 100644 --- a/src/DataObject/Data/Adapter/ObjectBricksAdapter.php +++ b/src/DataObject/Data/Adapter/ObjectBricksAdapter.php @@ -22,7 +22,9 @@ use Pimcore\Bundle\StudioBackendBundle\DataObject\Data\FieldContextData; use Pimcore\Bundle\StudioBackendBundle\DataObject\Data\SetterDataInterface; use Pimcore\Bundle\StudioBackendBundle\DataObject\Service\DataAdapterLoaderInterface; +use Pimcore\Bundle\StudioBackendBundle\DataObject\Service\DataAdapterServiceInterface; use Pimcore\Bundle\StudioBackendBundle\DataObject\Service\DataServiceInterface; +use Pimcore\Bundle\StudioBackendBundle\DataObject\Util\Trait\ValidateFieldTypeTrait; use Pimcore\Model\DataObject\ClassDefinition\Data; use Pimcore\Model\DataObject\ClassDefinition\Data\Objectbricks; use Pimcore\Model\DataObject\Concrete; @@ -38,7 +40,10 @@ #[AutoconfigureTag(DataAdapterLoaderInterface::ADAPTER_TAG)] final readonly class ObjectBricksAdapter implements SetterDataInterface, DataNormalizerInterface { + use ValidateFieldTypeTrait; + public function __construct( + private DataAdapterServiceInterface $dataAdapterService, private DataServiceInterface $dataService, private DefinitionResolverInterface $definitionResolver ) { @@ -175,18 +180,24 @@ private function getCollectionData( ): array { $collectionData = []; foreach ($collectionDef->getFieldDefinitions() as $fd) { + $adapter = $this->dataAdapterService->tryDataAdapter($fd->getFieldType()); $fieldName = $fd->getName(); - if (!array_key_exists($fieldName, $rawData)) { + if (!array_key_exists($fieldName, $rawData) || !$adapter) { continue; } - $collectionData[$fd->getName()] = $this->dataService->getAdapterSetterValue( + $value = $adapter->getDataForSetter( $element, $fd, $fieldName, [$fieldName => $rawData[$fieldName]], new FieldContextData($brick) ); + if (!$this->validateEncryptedField($fd, $value)) { + continue; + } + + $collectionData[$fieldName] = $value; } return $collectionData; diff --git a/src/DataObject/Service/DataAdapterService.php b/src/DataObject/Service/DataAdapterService.php index b5564d49..76813f0a 100644 --- a/src/DataObject/Service/DataAdapterService.php +++ b/src/DataObject/Service/DataAdapterService.php @@ -63,4 +63,14 @@ public function getDataAdapter(string $fieldDefinitionType): SetterDataInterface $this->getFieldDefinitionAdapterClass($fieldDefinitionType) ); } + + // ToDo: Consider removing this method and using getDataAdapter when field types from bundles are implemented + public function tryDataAdapter(string $fieldDefinitionType): ?SetterDataInterface + { + try { + return $this->getDataAdapter($fieldDefinitionType); + } catch (InvalidArgumentException) { + return null; + } + } } diff --git a/src/DataObject/Service/DataAdapterServiceInterface.php b/src/DataObject/Service/DataAdapterServiceInterface.php index b38c842e..c3968ab6 100644 --- a/src/DataObject/Service/DataAdapterServiceInterface.php +++ b/src/DataObject/Service/DataAdapterServiceInterface.php @@ -35,4 +35,6 @@ public function getFieldDefinitionAdapterClass(string $fieldDefinitionType): str * @throws InvalidArgumentException */ public function getDataAdapter(string $fieldDefinitionType): SetterDataInterface; + + public function tryDataAdapter(string $fieldDefinitionType): ?SetterDataInterface; } diff --git a/src/DataObject/Service/DataService.php b/src/DataObject/Service/DataService.php index d7f0eeb1..8e5a7e50 100644 --- a/src/DataObject/Service/DataService.php +++ b/src/DataObject/Service/DataService.php @@ -19,9 +19,6 @@ use Exception; use Pimcore\Bundle\StudioBackendBundle\DataObject\Data\ClassData; use Pimcore\Bundle\StudioBackendBundle\DataObject\Data\DataNormalizerInterface; -use Pimcore\Bundle\StudioBackendBundle\DataObject\Data\FieldContextData; -use Pimcore\Bundle\StudioBackendBundle\DataObject\Data\SetterDataInterface; -use Pimcore\Bundle\StudioBackendBundle\Exception\Api\InvalidArgumentException; use Pimcore\Bundle\StudioBackendBundle\Exception\Api\NotFoundException; use Pimcore\Model\DataObject\ClassDefinition\Data; use Pimcore\Model\DataObject\Concrete; @@ -90,36 +87,11 @@ public function getNormalizedValue( return null; } - $adapter = $this->getDataAdapter($fieldDefinition->getFieldType()); + $adapter = $this->dataAdapterService->tryDataAdapter($fieldDefinition->getFieldType()); if ($adapter instanceof DataNormalizerInterface) { return $adapter->normalize($value, $fieldDefinition); } return $fieldDefinition->normalize($value); } - - public function getAdapterSetterValue( - Concrete $element, - Data $fieldDefinition, - string $key, - array $data, - ?FieldContextData $contextData = null - ): ?array { - $adapter = $this->getDataAdapter($fieldDefinition->getFieldType()); - if ($adapter instanceof SetterDataInterface) { - return $adapter->getDataForSetter($element, $fieldDefinition, $key, $data, $contextData); - } - - return null; - } - - private function getDataAdapter(string $fieldType): ?SetterDataInterface - { - try { - return $this->dataAdapterService->getDataAdapter($fieldType); - } catch (InvalidArgumentException) { - // ToDo: Consider removing catch and throwing an exception when field types from bundles are implemented - return null; - } - } } diff --git a/src/DataObject/Service/DataServiceInterface.php b/src/DataObject/Service/DataServiceInterface.php index b0f7317e..ed0b497d 100644 --- a/src/DataObject/Service/DataServiceInterface.php +++ b/src/DataObject/Service/DataServiceInterface.php @@ -41,12 +41,4 @@ public function getNormalizedValue( mixed $value, Data $fieldDefinition ): mixed; - - public function getAdapterSetterValue( - Concrete $element, - Data $fieldDefinition, - string $key, - array $data, - ?FieldContextData $contextData = null - ): ?array; } diff --git a/src/DataObject/Util/Trait/ValidateFieldTypeTrait.php b/src/DataObject/Util/Trait/ValidateFieldTypeTrait.php new file mode 100644 index 00000000..a7f6b6bd --- /dev/null +++ b/src/DataObject/Util/Trait/ValidateFieldTypeTrait.php @@ -0,0 +1,32 @@ +getClass(); foreach ($editableData as $key => $value) { $fieldDefinition = $class->getFieldDefinition($key); - if ($fieldDefinition === null) { + if ($fieldDefinition === null || !array_key_exists($key, $editableData)) { continue; } - $data = array_key_exists($key, $editableData) - ? $this->dataService->getAdapterSetterValue($element, $fieldDefinition, $key, $editableData) - : null; + $adapter = $this->dataAdapterService->tryDataAdapter($fieldDefinition->getFieldtype()); + if ($adapter === null) { + continue; + } + + $value = $adapter->getDataForSetter($element, $fieldDefinition, $key, $editableData); + if (!$this->validateEncryptedField($fieldDefinition, $value)) { + continue; + } - $element->setValue($key, $data); + $element->setValue($key, $value); } } catch (Exception $e) {