From 7f2542c0fc59a2e312bdd0de836e13235b8be860 Mon Sep 17 00:00:00 2001 From: Eyad Abdullah Date: Mon, 23 Sep 2024 16:11:19 +0200 Subject: [PATCH] fix: reflection errors, and field mapping. --- Classes/Domain/Model/Event.php | 2 +- Classes/Mapping/AbstractMapping.php | 106 +++++++++++-------- Classes/Mapping/FalMapping.php | 33 +++--- Classes/Service/ApiClient.php | 2 +- Classes/Service/EventExecutionService.php | 10 +- Classes/Service/LoggingService.php | 3 +- Classes/Utility/ConstantsUtility.php | 2 +- Resources/Private/Templates/Event/Check.html | 20 +++- 8 files changed, 110 insertions(+), 68 deletions(-) diff --git a/Classes/Domain/Model/Event.php b/Classes/Domain/Model/Event.php index 261c137..1eb380b 100644 --- a/Classes/Domain/Model/Event.php +++ b/Classes/Domain/Model/Event.php @@ -220,7 +220,7 @@ public function setProcessing(bool $processing): void { $queryBuilder = GeneralUtility::makeInstance(ConnectionPool::class)->getQueryBuilderForTable('tx_fourallportal_domain_model_event'); $query = $queryBuilder->update('tx_fourallportal_domain_model_event') - ->set('processing', $processing, PDO::PARAM_INT) + ->set('processing', $processing ? 1 : 0,PDO::PARAM_INT) ->where($queryBuilder->expr()->eq('uid', $this->uid)); $query->executeStatement(); $this->processing = $processing; diff --git a/Classes/Mapping/AbstractMapping.php b/Classes/Mapping/AbstractMapping.php index f2f556c..962de85 100644 --- a/Classes/Mapping/AbstractMapping.php +++ b/Classes/Mapping/AbstractMapping.php @@ -14,9 +14,11 @@ use ReflectionClass; use ReflectionException; use ReflectionMethod; +use ReflectionProperty; use RuntimeException; use TYPO3\CMS\Core\Database\ConnectionPool; use TYPO3\CMS\Core\Domain\Repository\PageRepository; +use TYPO3\CMS\Core\Resource\File; use TYPO3\CMS\Core\Resource\FileInterface; use TYPO3\CMS\Core\Resource\StorageRepository; use TYPO3\CMS\Core\TypoScript\FrontendTypoScript; @@ -62,11 +64,6 @@ public function injectPersistenceManager(PersistenceManager $persistenceManager) $this->persistenceManager = $persistenceManager; } - public function injectAccessiblePropertyMapper(AccessiblePropertyMapper $accessiblePropertyMapper) - { - $this->accessiblePropertyMapper = $accessiblePropertyMapper; - } - public function injectStorageRepository(StorageRepository $storageRepository) { $this->storageRepository = $storageRepository; @@ -292,19 +289,18 @@ protected function removeObjectFromRepository(DomainObjectInterface $object): vo /** * @param array $data - * @param AbstractEntity|FileInterface $object + * @param AbstractEntity|File $object * @param Module $module * @param DimensionMapping|null $dimensionMapping * @return bool - * @throws InvalidSourceException - * @throws ReflectionException - * @throws TypeConverterException + * @throws ApiException */ - protected function mapPropertiesFromDataToObject(array $data, AbstractEntity|FileInterface $object, Module $module, DimensionMapping $dimensionMapping = null): bool + protected function mapPropertiesFromDataToObject(array $data, AbstractEntity|File $object, Module $module, DimensionMapping $dimensionMapping = null): bool { if (!$data['result']) { return true; } + $map = MappingRegister::resolvePropertyMapForMapper(static::class); $properties = $data['result'][0]['properties']; $responseValueReader = new ResponseDataFieldValueReader(); @@ -328,10 +324,7 @@ protected function mapPropertiesFromDataToObject(array $data, AbstractEntity|Fil $propertyMappingProblemsOccurred = $this->mapPropertyValueToObject($targetPropertyName, $propertyValue, $object); $mappingProblemsOccurred = $mappingProblemsOccurred ?: $propertyMappingProblemsOccurred; } - } catch (PropertyNotAccessibleException $error) { - $message = 'Error mapping ' . $module->getModuleName() . ':' . $objectId . ':' . $importedName . ' - ' . $error->getMessage(); - $this->loggingService->logObjectActivity($data['result'][0]['id'], $message, 3 /*GeneralUtility::SYSLOG_SEVERITY_WARNING*/); - } catch (DeferralException $error) { + } catch (PropertyNotAccessibleException|DeferralException|ReflectionException|InvalidSourceException|TypeConverterException $error) { $message = 'Error mapping ' . $module->getModuleName() . ':' . $objectId . ':' . $importedName . ' - ' . $error->getMessage(); $this->loggingService->logObjectActivity($data['result'][0]['id'], $message, 3 /*GeneralUtility::SYSLOG_SEVERITY_WARNING*/); } @@ -373,10 +366,13 @@ protected function mapPropertyValueToObject(string $propertyName, mixed $propert $objectId = $object->getRemoteId(); } - $array = (new ReflectionMethod(get_class($object), 'set' . ucfirst($propertyName)))->getParameters(); - if ($propertyValue === null && reset($array)->allowsNull()) { - ObjectAccess::setProperty($object, $propertyName, null); - return false; + $setterName = 'set' . ucfirst($propertyName); + if (method_exists($objectId, $setterName)) { + $array = (new ReflectionMethod(get_class($object),))->getParameters(); + if ($propertyValue === null && reset($array)->allowsNull()) { + ObjectAccess::setProperty($object, $propertyName, null); + return false; + } } $configuration = new PropertyMappingConfiguration(); $mappingProblemsOccurred = false; @@ -388,7 +384,11 @@ protected function mapPropertyValueToObject(string $propertyName, mixed $propert $propertyMapper = $this->getAccessiblePropertyMapper(); $targetType = $this->determineDataTypeForProperty($propertyName, $object); - $array1 = (new ReflectionMethod(get_class($object), 'set' . ucfirst($propertyName)))->getParameters(); + if ($targetType === null) { + $classSchema = get_class($object); + $this->loggingService->logObjectActivity($objectId, "Type of property {$propertyName} on {$classSchema} could not be determined", 3); + return false; + } if (strpos($targetType, '<')) { $childType = substr($targetType, strpos($targetType, '<') + 1, -1); $childType = trim($childType, '\\'); @@ -471,15 +471,22 @@ protected function mapPropertyValueToObject(string $propertyName, mixed $propert } } } - } elseif ($propertyValue === null && !reset($array1)->allowsNull()) { - $message = sprintf( - 'Property "%s" on object "%s->%s" does not allow NULL as value, but NULL was resolved. Please verify PIM response data consistency!', - $propertyName, - get_class($object), - method_exists($object, 'getRemoteId') ? $object->getRemoteId() : $object->getUid() - ); - $this->loggingService->logObjectActivity($objectId, $message, 4 /*GeneralUtility::SYSLOG_SEVERITY_FATAL*/); - return false; + } elseif ($propertyValue === null) { + try { + $parameters = (new \ReflectionMethod(get_class($object), 'set' . ucfirst($propertyName)))->getParameters(); + } catch (\ReflectionException $e) { + // ignore + } + if (!empty($parameters) && !$parameters[0]->allowsNull()) { + $message = sprintf( + 'Property "%s" on object "%s->%s" does not allow NULL as value, but NULL was resolved. Please verify PIM response data consistency!', + $propertyName, + get_class($object), + method_exists($object, 'getRemoteId') ? $object->getRemoteId() : $object->getUid() + ); + $this->loggingService->logObjectActivity($objectId, $message, 4 /*GeneralUtility::SYSLOG_SEVERITY_FATAL*/); + return false; + } } $setOnObject = $object; @@ -506,33 +513,38 @@ protected function mapPropertyValueToObject(string $propertyName, mixed $propert * @throws NoSuchPropertyException * @throws UnknownClassException */ - protected function determineDataTypeForProperty($propertyName, $object): bool|string + protected function determineDataTypeForProperty(string $propertyName, object $object): string|null { - if (property_exists(get_class($object), $propertyName)) { - $property = GeneralUtility::makeInstance(ReflectionService::class); - $classSchema = $property->getClassSchema($object); - $varTags = $classSchema->getProperty('var'); - if ($property->getPrimaryType()->isCollection()) { - return strpos($varTags[0], ' ') !== false ? substr($varTags[0], 0, strpos($varTags[0], ' ')) : $varTags[0]; + + if (property_exists($object, $propertyName)) { + $property = new ReflectionProperty($object, $propertyName); + $type = $property->getType(); + + if ($type !== null) { + return $type->getName(); + } + + $varTags = $property->getAttributes('var'); + if (!empty($varTags)) { + return explode(' ', $varTags[0]->getArguments()[0])[0]; } } - if (method_exists(get_class($object), 'set' . ucfirst($propertyName))) { - /** @see .build/vendor/typo3/cms-core/Documentation/Changelog/9.0/Breaking-57594-OptimizeReflectionServiceCacheHandling.rst */ - $method = $classSchema->getMethod('set' . ucfirst($propertyName)); + $setterMethod = 'set' . ucfirst($propertyName); + if (method_exists($object, $setterMethod)) { + $method = new ReflectionMethod($object, $setterMethod); $parameters = $method->getParameters(); - if ($parameters[0]->getType() !== null) { - return (string)$parameters[0]->getType(); + + if (!empty($parameters) && $parameters[0]->hasType()) { + return $parameters[0]->getType()->getName(); } - $varTags = $method->getParameter('param'); - if (!empty($varTags)) { - $array = explode(' ', $varTags[0]); - return reset($array); + $paramTags = $method->getAttributes('param'); + if (!empty($paramTags)) { + return explode(' ', $paramTags[0]->getArguments()[0])[0]; } } - - throw new RuntimeException('Type of property ' . $propertyName . ' on ' . get_class($object) . ' could not be determined'); + return null; } /** @@ -558,7 +570,7 @@ public function getEntityClassName(): string */ protected function getAccessiblePropertyMapper(): AccessiblePropertyMapper { - return $this->accessiblePropertyMapper; + return GeneralUtility::makeInstance(AccessiblePropertyMapper::class); } /** diff --git a/Classes/Mapping/FalMapping.php b/Classes/Mapping/FalMapping.php index ae9295e..0453def 100644 --- a/Classes/Mapping/FalMapping.php +++ b/Classes/Mapping/FalMapping.php @@ -8,12 +8,13 @@ use Crossmedia\Fourallportal\Domain\Model\Server; use Crossmedia\Fourallportal\Error\ApiException; use Crossmedia\Fourallportal\Service\ApiClient; +use Crossmedia\Fourallportal\Service\LoggingService; use Crossmedia\Fourallportal\ValueReader\ResponseDataFieldValueReader; use DateTime; use Doctrine\DBAL\Exception; -use ReflectionException; use RuntimeException; use TYPO3\CMS\Core\Charset\CharsetConverter; +use TYPO3\CMS\Core\Core\Environment; use TYPO3\CMS\Core\Database\ConnectionPool; use TYPO3\CMS\Core\Resource\Driver\DriverInterface; use TYPO3\CMS\Core\Resource\Driver\LocalDriver; @@ -32,8 +33,7 @@ use TYPO3\CMS\Core\Utility\GeneralUtility; use TYPO3\CMS\Extbase\Domain\Model\FileReference; use TYPO3\CMS\Extbase\DomainObject\AbstractEntity; -use TYPO3\CMS\Extbase\Property\Exception\InvalidSourceException; -use TYPO3\CMS\Extbase\Property\Exception\TypeConverterException; +use TYPO3\CMS\Extbase\Persistence\PersistenceManagerInterface; use TYPO3\CMS\Extbase\Reflection\Exception\PropertyNotAccessibleException; use TYPO3\CMS\Extbase\Reflection\ObjectAccess; @@ -57,10 +57,7 @@ public function getEntityClassName(): string * @throws InsufficientFolderReadPermissionsException * @throws InsufficientFolderWritePermissionsException * @throws InvalidFileNameException - * @throws InvalidSourceException * @throws PropertyNotAccessibleException - * @throws ReflectionException - * @throws TypeConverterException */ public function import(array $data, Event $event): bool { @@ -77,6 +74,7 @@ public function import(array $data, Event $event): bool ->where($queryBuilder->expr()->eq('remote_id', $queryBuilder->quote($objectId))); $deferAfterProcessing = false; + $logging = $this->loggingService = GeneralUtility::makeInstance(LoggingService::class); switch ($event->getEventType()) { case 'delete': @@ -88,6 +86,7 @@ public function import(array $data, Event $event): bool } // handle multiple files in the system foreach ($records as $record) { + /** @var File $object */ $object = $repository->findByUid($record['uid']); if (!$object || !$record) { // Object is already deleted, return false meaning no deferral after processing. @@ -108,14 +107,24 @@ public function import(array $data, Event $event): bool break; case 'update': case 'create': + $message = sprintf("start creating object with id %s", $objectId); + $logging->logEventActivity($event, $message); + + /** @var File $object */ $object = $this->downloadFileAndGetFileObject($objectId, $data, $event); + $message = sprintf("object with id %s downloaded successfully", $objectId); + $logging->logEventActivity($event, $message); + $deferAfterProcessing = $this->mapPropertiesFromDataToObject($data, $object, $event->getModule()); + + $message = sprintf("object with id %s create successfully", $objectId); + $logging->logEventActivity($event, $message); break; default: throw new RuntimeException('Unknown event type: ' . $event->getEventType()); } - $this->persistenceManager->persistAll(); + GeneralUtility::makeInstance(PersistenceManagerInterface::class)->persistAll(); if ($object) { $this->processRelationships($object, $data, $event); @@ -221,16 +230,14 @@ protected function collectTablesAndRecordsWithRelationsToFileReferences(array $e /** * @param array $data - * @param AbstractEntity|FileInterface $object + * @param AbstractEntity|File $object * @param Module $module * @param DimensionMapping|null $dimensionMapping * @return bool + * @throws ApiException * @throws PropertyNotAccessibleException - * @throws ReflectionException - * @throws InvalidSourceException - * @throws TypeConverterException */ - protected function mapPropertiesFromDataToObject(array $data, AbstractEntity|FileInterface $object, Module $module, DimensionMapping $dimensionMapping = null): bool + protected function mapPropertiesFromDataToObject(array $data, AbstractEntity|File $object, Module $module, DimensionMapping $dimensionMapping = null): bool { $deferAfterProcessing = parent::mapPropertiesFromDataToObject($data, $object, $module, $dimensionMapping); $metadata = []; @@ -512,7 +519,7 @@ public function check(ApiClient $client, Module $module, array $status): array

', $ids[0]); } else { - $publicTempFile = 'typo3/typo3temp/assets/images/' . basename($temporaryFile); + $publicTempFile = Environment::getPublicPath() . '/typo3/typo3temp/assets/images/' . basename($temporaryFile); rename($temporaryFile, $publicTempFile); $temporaryFileRelativePath = substr($publicTempFile, strlen('typo3/') - 1); $receivedBytes = filesize($publicTempFile); diff --git a/Classes/Service/ApiClient.php b/Classes/Service/ApiClient.php index b557194..3c2e912 100644 --- a/Classes/Service/ApiClient.php +++ b/Classes/Service/ApiClient.php @@ -205,7 +205,7 @@ public function saveDerivate(string $filename, string $objectId, string $usage = curl_close($ch); fclose($fp); - if ($expectedFileSize > 0 && $expectedFileSize != filesize($temporaryFilename)) { + if (!empty($expectedFileSize) && $expectedFileSize > 0 && $expectedFileSize != filesize($temporaryFilename)) { unlink($temporaryFilename); $message = 'The downloaded file does not match the expected filesize'; $this->loggingService->logFileTransferActivity($uri, $temporaryFilename . ': ' . $message, 4 /*GeneralUtility::SYSLOG_SEVERITY_ERROR*/); diff --git a/Classes/Service/EventExecutionService.php b/Classes/Service/EventExecutionService.php index 6e507dc..306ade7 100644 --- a/Classes/Service/EventExecutionService.php +++ b/Classes/Service/EventExecutionService.php @@ -537,6 +537,11 @@ public function processEvent(Event $event, bool $updateEventId = true, ?SyncPara ); $event->setProcessing(true); + try { + $this->persistenceManager->update($event); + } catch (UnknownObjectException $e) { + // ignore + } if (method_exists($this->response, 'send')) { $this->response->send(); @@ -603,7 +608,7 @@ public function processEvent(Event $event, bool $updateEventId = true, ?SyncPara $event->setStatus('failed'); } $this->loggingService->logEventActivity($event, 'Event was deferred', 2 /* GeneralUtility::SYSLOG_SEVERITY_WARNING */); - } catch (Exception $exception) { + } catch (\Exception | \ReflectionException $exception) { $event->setStatus('failed'); $event->setRetries(0); $event->setMessage($exception->getMessage() . ' (code: ' . $exception->getCode() . ')' . $exception->getFile() . ':' . $exception->getLine()); @@ -611,6 +616,8 @@ public function processEvent(Event $event, bool $updateEventId = true, ?SyncPara $event->getModule()->setLastEventId(max($event->getEventId(), $event->getModule()->getLastEventId())); } $this->loggingService->logEventActivity($event, 'System error: ' . $exception->getMessage(), 2 /* GeneralUtility::SYSLOG_SEVERITY_WARNING */); + } finally { + $event->setProcessing(false); } // $responseMetadata = $client->getLastResponse(); // $event->setHeaders($responseMetadata['headers']); @@ -620,6 +627,7 @@ public function processEvent(Event $event, bool $updateEventId = true, ?SyncPara // $event->setProcessing(false); // $this->eventRepository->update($event); try { + $this->persistenceManager->update($event); $this->persistenceManager->persistAll(); } catch (Exception $exception) { $this->loggingService->logEventActivity($event, 'System error: ' . $exception->getMessage(), 2 /* GeneralUtility::SYSLOG_SEVERITY_WARNING */); diff --git a/Classes/Service/LoggingService.php b/Classes/Service/LoggingService.php index 36cc221..288b05f 100644 --- a/Classes/Service/LoggingService.php +++ b/Classes/Service/LoggingService.php @@ -5,6 +5,7 @@ use Crossmedia\Fourallportal\Domain\Model\Event; use Crossmedia\Fourallportal\Domain\Model\LogEntry; use Crossmedia\Fourallportal\Utility\ConstantsUtility; +use TYPO3\CMS\Core\Core\Environment; use TYPO3\CMS\Core\SingletonInterface; use TYPO3\CMS\Core\Utility\GeneralUtility; @@ -147,7 +148,7 @@ protected function writeEntry(string $logFile, string $message, int $severity): protected function resolveLogFilePath(string $type, string $identity = null): string { - $logFilePath = ConstantsUtility::LOG_BASEDIR . $type; + $logFilePath = Environment::getVarPath() . ConstantsUtility::LOG_BASEDIR . $type; if ($identity) { $logFilePath .= '/' . $identity . '.log'; } else { diff --git a/Classes/Utility/ConstantsUtility.php b/Classes/Utility/ConstantsUtility.php index 1d30b48..bf9c4b1 100644 --- a/Classes/Utility/ConstantsUtility.php +++ b/Classes/Utility/ConstantsUtility.php @@ -10,5 +10,5 @@ class ConstantsUtility public final const TEXT_SCHEMA = 'schema'; public final const TEXT_CONNECTION = 'connection'; public final const TEXT_ERRORS = 'error'; - public final const LOG_BASEDIR = 'typo3temp/var/logs/fourallportal/'; + public final const LOG_BASEDIR = 'logs/fourallportal/'; } \ No newline at end of file diff --git a/Resources/Private/Templates/Event/Check.html b/Resources/Private/Templates/Event/Check.html index 7aaa1b2..8b4caeb 100644 --- a/Resources/Private/Templates/Event/Check.html +++ b/Resources/Private/Templates/Event/Check.html @@ -7,6 +7,20 @@ + + .pre-4ap { + display: block; + padding: 8.5px; + font-size: 11px; + line-height: 1.5; + word-break: break-all; + word-wrap: break-word; + background-color: #f5f5f5; + border: 1px solid #ccc; + border-radius: 2px; + } + +

Event history, object @@ -48,13 +62,13 @@

Object log

Request payload

- {event.payload} + 
{event.payload -> f:format.raw()}

Response headers

- {event.headers} + 
{event.headers -> f:format.raw()}

Response body

- {event.response} + 
{event.response -> f:format.raw()}