diff --git a/Classes/Domain/Model/Event.php b/Classes/Domain/Model/Event.php index 721a80dd..c072e21f 100644 --- a/Classes/Domain/Model/Event.php +++ b/Classes/Domain/Model/Event.php @@ -426,21 +426,7 @@ public function addCategory(Category $category): void */ public function getCategories(): array { - $categories = $this->categories->toArray(); - - usort($categories, function (Category $catA, Category $catB) { - return $catA->getSorting() <=> $catB->getSorting(); - }); - - return $categories; - } - - /** - * @param ObjectStorage $categories - */ - public function setCategories(ObjectStorage $categories): void - { - $this->categories = $categories; + return $this->getSortedCategory($this->categories); } /** @@ -448,21 +434,7 @@ public function setCategories(ObjectStorage $categories): void */ public function getFeatures(): array { - $features = $this->features->toArray(); - - usort($features, function (Category $catA, Category $catB) { - return $catA->getSorting() <=> $catB->getSorting(); - }); - - return $features; - } - - /** - * @param ObjectStorage $features - */ - public function setFeatures(ObjectStorage $features): void - { - $this->features = $features; + return $this->getSortedCategory($this->features); } public function setLanguageUid(int $languageUid): void @@ -489,4 +461,15 @@ public function setSourceUrl(string $url): void { $this->sourceUrl = $url; } + + private function getSortedCategory(ObjectStorage $categories): array + { + $categories = $categories->toArray(); + + usort($categories, function (Category $catA, Category $catB) { + return $catA->getSorting() <=> $catB->getSorting(); + }); + + return $categories; + } } diff --git a/Classes/Service/DestinationDataImportService.php b/Classes/Service/DestinationDataImportService.php index 32d615eb..d54305b7 100644 --- a/Classes/Service/DestinationDataImportService.php +++ b/Classes/Service/DestinationDataImportService.php @@ -22,6 +22,8 @@ use Wrm\Events\Service\DestinationDataImportService\CategoriesAssignment; use Wrm\Events\Service\DestinationDataImportService\CategoriesAssignment\Import as CategoryImport; use Wrm\Events\Service\DestinationDataImportService\DataFetcher; +use Wrm\Events\Service\DestinationDataImportService\DataHandler; +use Wrm\Events\Service\DestinationDataImportService\DataHandler\Assignment; use Wrm\Events\Service\DestinationDataImportService\DatesFactory; use Wrm\Events\Service\DestinationDataImportService\Events\CategoriesAssignEvent; use Wrm\Events\Service\DestinationDataImportService\Events\EventImportEvent; @@ -111,6 +113,11 @@ class DestinationDataImportService */ private $cacheManager; + /** + * @var DataHandler + */ + private $dataHandler; + /** * @var EventDispatcher */ @@ -131,6 +138,7 @@ class DestinationDataImportService * @param LocationAssignment $locationAssignment * @param Slugger $slugger * @param CacheManager $cacheManager + * @param DataHandler $dataHandler * @param EventDispatcher $eventDispatcher */ public function __construct( @@ -147,6 +155,7 @@ public function __construct( LocationAssignment $locationAssignment, Slugger $slugger, CacheManager $cacheManager, + DataHandler $dataHandler, EventDispatcher $eventDispatcher ) { $this->eventRepository = $eventRepository; @@ -162,6 +171,7 @@ public function __construct( $this->locationAssignment = $locationAssignment; $this->slugger = $slugger; $this->cacheManager = $cacheManager; + $this->dataHandler = $dataHandler; $this->eventDispatcher = $eventDispatcher; } @@ -236,16 +246,6 @@ public function processData(array $data): int $this->locationAssignment->getLocation($event) ); - // Set Categories - if ($event['categories'] ?? false) { - $this->setCategories($event['categories']); - } - - // Set Features - if ($event['features']) { - $this->setFeatures($event['features']); - } - // Set Organizer if ($event['addresses'] ?? false) { $this->setOrganizer($event['addresses']); @@ -291,6 +291,16 @@ public function processData(array $data): int $this->logger->info('Persist database'); $this->eventRepository->update($this->tmpCurrentEvent); $this->persistenceManager->persistAll(); + + // Apply changes via DataHandler (The new way) + $this->logger->info('Apply changes via DataHandler'); + if ($event['categories'] ?? false) { + $this->setCategories($event['categories']); + } + if ($event['features']) { + $this->setFeatures($event['features']); + } + $this->logger->info('Update slugs'); $this->slugger->update('tx_events_domain_model_event'); $this->slugger->update('tx_events_domain_model_date'); @@ -317,7 +327,11 @@ private function setCategories(array $categories): void ); $this->eventDispatcher->dispatch($event); - $this->tmpCurrentEvent->setCategories($event->getCategories()); + $this->dataHandler->storeAssignments(new Assignment( + $this->tmpCurrentEvent->getUid(), + 'categories', + $event->getCategories()->toArray() + )); } private function setFeatures(array $features): void @@ -329,7 +343,11 @@ private function setFeatures(array $features): void true )); - $this->tmpCurrentEvent->setFeatures($features); + $this->dataHandler->storeAssignments(new Assignment( + $this->tmpCurrentEvent->getUid(), + 'features', + $features->toArray() + )); } private function setDates( diff --git a/Classes/Service/DestinationDataImportService/CategoriesAssignment.php b/Classes/Service/DestinationDataImportService/CategoriesAssignment.php index ab290b16..4ddf8ede 100644 --- a/Classes/Service/DestinationDataImportService/CategoriesAssignment.php +++ b/Classes/Service/DestinationDataImportService/CategoriesAssignment.php @@ -2,6 +2,7 @@ namespace Wrm\Events\Service\DestinationDataImportService; +use TYPO3\CMS\Extbase\Persistence\Generic\PersistenceManager; use TYPO3\CMS\Extbase\Persistence\ObjectStorage; use Wrm\Events\Domain\Model\Category; use Wrm\Events\Domain\Repository\CategoryRepository; @@ -20,10 +21,17 @@ class CategoriesAssignment */ private $repository; + /** + * @var PersistenceManager + */ + private $persistenceManager; + public function __construct( - CategoryRepository $repository + CategoryRepository $repository, + PersistenceManager $persistenceManager ) { $this->repository = $repository; + $this->persistenceManager = $persistenceManager; } /** @@ -58,6 +66,8 @@ public function getCategories( $categories->attach($category); } + $this->persistenceManager->persistAll(); + return $categories; } } diff --git a/Classes/Service/DestinationDataImportService/DataHandler.php b/Classes/Service/DestinationDataImportService/DataHandler.php new file mode 100644 index 00000000..36e53f52 --- /dev/null +++ b/Classes/Service/DestinationDataImportService/DataHandler.php @@ -0,0 +1,68 @@ + + * + * This program is free software; you can redistribute it and/or + * modify it under the terms of the GNU General Public License + * as published by the Free Software Foundation; either version 2 + * of the License, or (at your option) any later version. + * + * This program is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * GNU General Public License for more details. + * + * You should have received a copy of the GNU General Public License + * along with this program; if not, write to the Free Software + * Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA + * 02110-1301, USA. + */ + +namespace Wrm\Events\Service\DestinationDataImportService; + +use TYPO3\CMS\Core\DataHandling\DataHandler as Typo3DataHandler; +use TYPO3\CMS\Core\Log\Logger; +use TYPO3\CMS\Core\Log\LogManager; +use TYPO3\CMS\Core\Utility\GeneralUtility; +use Wrm\Events\Service\DestinationDataImportService\DataHandler\Assignment; + +final class DataHandler +{ + /** + * @var Logger + */ + private $logger; + + public function __construct( + LogManager $logManager + ) { + $this->logger = $logManager->getLogger(__CLASS__); + } + + public function storeAssignments( + Assignment $assignment + ): void { + $data = [ + 'tx_events_domain_model_event' => [ + $assignment->getUid() => [ + $assignment->getColumnName() => implode(',', $assignment->getUids()), + ], + ], + ]; + + $this->logger->debug('Import assignment.', $data); + $dataHandler = GeneralUtility::makeInstance(Typo3DataHandler::class); + $dataHandler->start($data, []); + $dataHandler->process_datamap(); + + if ($dataHandler->errorLog !== []) { + $this->logger->error('Error during import of assignments.', [ + 'assignment' => $assignment, + 'errors' => $dataHandler->errorLog, + ]); + } + } +} diff --git a/Classes/Service/DestinationDataImportService/DataHandler/Assignment.php b/Classes/Service/DestinationDataImportService/DataHandler/Assignment.php new file mode 100644 index 00000000..205d68b7 --- /dev/null +++ b/Classes/Service/DestinationDataImportService/DataHandler/Assignment.php @@ -0,0 +1,82 @@ + + * + * This program is free software; you can redistribute it and/or + * modify it under the terms of the GNU General Public License + * as published by the Free Software Foundation; either version 2 + * of the License, or (at your option) any later version. + * + * This program is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * GNU General Public License for more details. + * + * You should have received a copy of the GNU General Public License + * along with this program; if not, write to the Free Software + * Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA + * 02110-1301, USA. + */ + +namespace Wrm\Events\Service\DestinationDataImportService\DataHandler; + +use InvalidArgumentException; +use TYPO3\CMS\Extbase\DomainObject\AbstractDomainObject; + +final class Assignment +{ + /** + * @var int + */ + private $uid; + + /** + * @var string + */ + private $columnName; + + /** + * @var int[] + */ + private $uids; + + /** + * @param AbstractDomainObject[] $assignments + */ + public function __construct( + int $uid, + string $columnName, + array $assignments + ) { + $this->uid = $uid; + $this->columnName = $columnName; + $this->uids = array_map(static function (AbstractDomainObject $model): int { + $uid = $model->getUid(); + if (is_int($uid) === false) { + throw new InvalidArgumentException('Only object with uid supported.', 1698936965); + } + return $uid; + }, $assignments); + } + + public function getUid(): int + { + return $this->uid; + } + + public function getColumnName(): string + { + return $this->columnName; + } + + /** + * @return int[] + */ + public function getUids(): array + { + return $this->uids; + } +} diff --git a/Documentation/Changelog/3.5.2.rst b/Documentation/Changelog/3.5.2.rst index 09e265b5..579597eb 100644 --- a/Documentation/Changelog/3.5.2.rst +++ b/Documentation/Changelog/3.5.2.rst @@ -19,6 +19,21 @@ Fixes An unexpected value (3) was submitted on selection. We now migrated the property to look and act the same as hidden input. +* Fix broken assignment of features and categories. + The import contained a bug that lead to summing up relations between events and + categories and features. + The same relations would be added over and over again with every new import. + This leads to performance issues when editing or storing events through TYPO3 + backend. + The import is adjusted to no longer use Extbase but DataHandler for this task. + This circumvents the issues introduced by Extbase. + This is the first step to migration of import to be fully using DataHandler + + doctrine DBAL in the future. + One drawback is the slowdown of the import. + But we don't expect people to run it that often. + Also a working import with proper logging (history, permissions, etc.) seems better + than a broken but fast import. + Tasks ----- diff --git a/Tests/Functional/Import/DestinationDataTest/Assertions/ImportsFeaturesAddsNewFeatures.csv b/Tests/Functional/Import/DestinationDataTest/Assertions/ImportsFeaturesAddsNewFeatures.csv index 36a05a39..5b3e33f9 100644 --- a/Tests/Functional/Import/DestinationDataTest/Assertions/ImportsFeaturesAddsNewFeatures.csv +++ b/Tests/Functional/Import/DestinationDataTest/Assertions/ImportsFeaturesAddsNewFeatures.csv @@ -1,14 +1,14 @@ "tx_events_domain_model_event",,,,,, ,"uid","pid","features",,, -,1,2,1,,, -,2,2,1,,, +,1,2,6,,, +,2,2,4,,, ,3,2,0,,, "sys_category",,,,,, ,"uid","pid","title","parent","hidden", ,1,2,"Top Category",0,0, ,2,2,"Event Category Parent",1,0, ,4,2,"Event Feature Parent",1,0, -,5,3,"vorhandenes Feature",4,0, +,5,3,"vorhandenes Feature",4,1, ,6,3,"Barrierefrei",4,1, ,7,3,"Zielgruppe Jugendliche",4,1, ,8,3,"Karten an der Abendkasse",4,1, diff --git a/Tests/Functional/Import/DestinationDataTest/Fixtures/Database/ExistingFeatures.php b/Tests/Functional/Import/DestinationDataTest/Fixtures/Database/ExistingFeatures.php new file mode 100644 index 00000000..6a43fa23 --- /dev/null +++ b/Tests/Functional/Import/DestinationDataTest/Fixtures/Database/ExistingFeatures.php @@ -0,0 +1,22 @@ + [ + [ + 'uid' => 1, + 'pid' => 2, + 'global_id' => 'e_100347853', + 'features' => 1, + ], + ], + 'sys_category_record_mm' => [ + [ + 'uid_local' => 5, + 'uid_foreign' => 1, + 'tablenames' => 'tx_events_domain_model_event', + 'fieldname' => 'features', + 'sorting' => 0, + 'sorting_foreign' => 1, + ], + ], +]; diff --git a/Tests/Functional/Import/DestinationDataTest/Fixtures/Database/FeaturesImportConfiguration.php b/Tests/Functional/Import/DestinationDataTest/Fixtures/Database/FeaturesImportConfiguration.php index adf7472a..ecde4233 100644 --- a/Tests/Functional/Import/DestinationDataTest/Fixtures/Database/FeaturesImportConfiguration.php +++ b/Tests/Functional/Import/DestinationDataTest/Fixtures/Database/FeaturesImportConfiguration.php @@ -42,7 +42,7 @@ 'pid' => '3', 'title' => 'vorhandenes Feature', 'parent' => '4', - 'hidden' => '0', + 'hidden' => '1', ], ], ]; diff --git a/Tests/Functional/Import/DestinationDataTest/ImportsFeaturesTest.php b/Tests/Functional/Import/DestinationDataTest/ImportsFeaturesTest.php index 932ae8a9..a2b037f0 100644 --- a/Tests/Functional/Import/DestinationDataTest/ImportsFeaturesTest.php +++ b/Tests/Functional/Import/DestinationDataTest/ImportsFeaturesTest.php @@ -27,4 +27,23 @@ public function addsNewFeatures(): void $this->assertCSVDataSet('EXT:events/Tests/Functional/Import/DestinationDataTest/Assertions/ImportsFeaturesAddsNewFeatures.csv'); $this->assertEmptyLog(); } + + /** + * @test + */ + public function addsNewFeaturesToExistingOnes(): void + { + $this->setUpConfiguration([ + 'restUrl = https://example.com/some-path/', + ]); + $this->importPHPDataSet(__DIR__ . '/Fixtures/Database/FeaturesImportConfiguration.php'); + $this->importPHPDataSet(__DIR__ . '/Fixtures/Database/ExistingFeatures.php'); + $this->setUpResponses([ + new Response(200, [], file_get_contents(__DIR__ . '/Fixtures/ResponseWithFeatures.json') ?: ''), + ]); + $tester = $this->executeCommand(); + + $this->assertCSVDataSet('EXT:events/Tests/Functional/Import/DestinationDataTest/Assertions/ImportsFeaturesAddsNewFeatures.csv'); + $this->assertEmptyLog(); + } } diff --git a/Tests/Unit/Domain/Model/EventTest.php b/Tests/Unit/Domain/Model/EventTest.php index 924a3a5b..fd72c325 100644 --- a/Tests/Unit/Domain/Model/EventTest.php +++ b/Tests/Unit/Domain/Model/EventTest.php @@ -42,7 +42,7 @@ public function returnsSortedFeatures(): void $storage->attach($feature2); $subject = new Event(); - $subject->setFeatures($storage); + $subject->_setProperty('features', $storage); self::assertSame([ $feature2, @@ -56,7 +56,7 @@ public function returnsSortedFeatures(): void public function returnsEmptyFeaturesStorage(): void { $subject = new Event(); - $subject->setFeatures(new ObjectStorage()); + $subject->_setProperty('features', new ObjectStorage()); self::assertSame([], $subject->getFeatures()); } diff --git a/phpstan-baseline.neon b/phpstan-baseline.neon index 7db735c7..2951d9f9 100644 --- a/phpstan-baseline.neon +++ b/phpstan-baseline.neon @@ -1,20 +1,5 @@ parameters: ignoreErrors: - - - message: "#^Instantiated class TYPO3\\\\CMS\\\\Core\\\\Http\\\\PropagateResponseException not found\\.$#" - count: 1 - path: Classes/Controller/AbstractController.php - - - - message: "#^Parameter \\#1 \\$request of method TYPO3\\\\CMS\\\\Frontend\\\\Controller\\\\ErrorController\\:\\:pageNotFoundAction\\(\\) expects Psr\\\\Http\\\\Message\\\\ServerRequestInterface, TYPO3\\\\CMS\\\\Extbase\\\\Mvc\\\\Request given\\.$#" - count: 1 - path: Classes/Controller/AbstractController.php - - - - message: "#^Throwing object of an unknown class TYPO3\\\\CMS\\\\Core\\\\Http\\\\PropagateResponseException\\.$#" - count: 1 - path: Classes/Controller/AbstractController.php - - message: "#^Cannot call method typoLink_URL\\(\\) on TYPO3\\\\CMS\\\\Frontend\\\\ContentObject\\\\ContentObjectRenderer\\|null\\.$#" count: 1 @@ -25,61 +10,16 @@ parameters: count: 1 path: Classes/Domain/Model/Category.php - - - message: "#^Parameter \\#1 \\$categories of method Wrm\\\\Events\\\\Domain\\\\Model\\\\Event\\:\\:setCategories\\(\\) expects TYPO3\\\\CMS\\\\Extbase\\\\Persistence\\\\ObjectStorage\\, TYPO3\\\\CMS\\\\Extbase\\\\Persistence\\\\ObjectStorage\\ given\\.$#" - count: 1 - path: Classes/Service/DestinationDataImportService.php - - - - message: "#^Parameter \\#1 \\$callback of function array_map expects \\(callable\\(mixed\\)\\: mixed\\)\\|null, 'strval' given\\.$#" - count: 1 - path: Classes/Domain/DestinationData/ImportFactory.php - - - - message: "#^Parameter \\#2 \\$array of function array_map expects array, mixed given\\.$#" - count: 1 - path: Classes/Domain/DestinationData/ImportFactory.php - - message: "#^Parameter \\#2 \\$callback of function usort expects callable\\(TEntity, TEntity\\)\\: int, Closure\\(Wrm\\\\Events\\\\Domain\\\\Model\\\\Category, Wrm\\\\Events\\\\Domain\\\\Model\\\\Category\\)\\: int\\<\\-1, 1\\> given\\.$#" - count: 2 - path: Classes/Domain/Model/Event.php - - - - message: "#^Method Wrm\\\\Events\\\\Domain\\\\Model\\\\Event\\:\\:getCategories\\(\\) should return array\\ but returns array\\\\.$#" - count: 1 - path: Classes/Domain/Model/Event.php - - - - message: "#^Method Wrm\\\\Events\\\\Domain\\\\Model\\\\Event\\:\\:getFeatures\\(\\) should return array\\ but returns array\\\\.$#" count: 1 path: Classes/Domain/Model/Event.php - - - message: "#^Call to method deleteFile\\(\\) on an unknown class TYPO3\\\\CMS\\\\Core\\\\Resource\\\\Storage\\.$#" - count: 1 - path: Classes/Service/Cleanup/Files.php - - - - message: "#^Call to method getFile\\(\\) on an unknown class TYPO3\\\\CMS\\\\Core\\\\Resource\\\\Storage\\.$#" - count: 1 - path: Classes/Service/Cleanup/Files.php - - - - message: "#^Call to method hasFile\\(\\) on an unknown class TYPO3\\\\CMS\\\\Core\\\\Resource\\\\Storage\\.$#" - count: 1 - path: Classes/Service/Cleanup/Files.php - - message: "#^Argument of an invalid type Doctrine\\\\DBAL\\\\Result\\|int supplied for foreach, only iterables are supported\\.$#" count: 1 path: Classes/Service/Cleanup/Files.php - - - message: "#^Argument of an invalid type Doctrine\\\\DBAL\\\\Driver\\\\ResultStatement\\|int supplied for foreach, only iterables are supported\\.$#" - count: 1 - path: Classes/Service/Cleanup/Files.php - - message: "#^Cannot call method fetchAllAssociativeIndexed\\(\\) on Doctrine\\\\DBAL\\\\Result\\|int\\.$#" count: 1 @@ -91,30 +31,25 @@ parameters: path: Classes/Service/Cleanup/Files.php - - message: "#^Method Wrm\\\\Events\\\\Service\\\\DestinationDataImportService\\\\CategoriesAssignment\\:\\:getCategories\\(\\) should return TYPO3\\\\CMS\\\\Extbase\\\\Persistence\\\\ObjectStorage\\ but returns TYPO3\\\\CMS\\\\Extbase\\\\Persistence\\\\ObjectStorage\\\\.$#" + message: "#^Parameter \\#1 \\$uid of class Wrm\\\\Events\\\\Service\\\\DestinationDataImportService\\\\DataHandler\\\\Assignment constructor expects int, int\\|null given\\.$#" count: 2 - path: Classes/Service/DestinationDataImportService/CategoriesAssignment.php + path: Classes/Service/DestinationDataImportService.php - - message: "#^Argument of an invalid type Doctrine\\\\DBAL\\\\Driver\\\\ResultStatement\\|int supplied for foreach, only iterables are supported\\.$#" - count: 1 - path: Classes/Updates/MigrateOldLocations.php + message: "#^Parameter \\#3 \\$assignments of class Wrm\\\\Events\\\\Service\\\\DestinationDataImportService\\\\DataHandler\\\\Assignment constructor expects array\\, array\\ given\\.$#" + count: 2 + path: Classes/Service/DestinationDataImportService.php - - message: "#^Cannot call method fetchAssociative\\(\\) on Doctrine\\\\DBAL\\\\Driver\\\\ResultStatement\\|int\\.$#" - count: 1 - path: Classes/Updates/MigrateOldLocations.php + message: "#^Method Wrm\\\\Events\\\\Service\\\\DestinationDataImportService\\\\CategoriesAssignment\\:\\:getCategories\\(\\) should return TYPO3\\\\CMS\\\\Extbase\\\\Persistence\\\\ObjectStorage\\ but returns TYPO3\\\\CMS\\\\Extbase\\\\Persistence\\\\ObjectStorage\\\\.$#" + count: 2 + path: Classes/Service/DestinationDataImportService/CategoriesAssignment.php - message: "#^Method Wrm\\\\Events\\\\Service\\\\DestinationDataImportService\\\\FilesAssignment\\:\\:getImages\\(\\) should return TYPO3\\\\CMS\\\\Extbase\\\\Persistence\\\\ObjectStorage\\ but returns TYPO3\\\\CMS\\\\Extbase\\\\Persistence\\\\ObjectStorage\\\\.$#" count: 1 path: Classes/Service/DestinationDataImportService/FilesAssignment.php - - - message: "#^Cannot call method fetchAllAssociativeIndexed\\(\\) on Doctrine\\\\DBAL\\\\Driver\\\\ResultStatement\\|int\\.$#" - count: 1 - path: Classes/Service/Cleanup/Files.php - - message: "#^Argument of an invalid type Doctrine\\\\DBAL\\\\Result\\|int supplied for foreach, only iterables are supported\\.$#" count: 1 @@ -139,9 +74,3 @@ parameters: message: "#^Offset 'uid' does not exist on array\\{location\\: int\\}\\.$#" count: 1 path: Classes/Updates/MigrateOldLocations.php - - - - - message: "#^Cannot call method getEnd\\(\\) on mixed\\.$#" - count: 1 - path: Classes/Caching/PageCacheTimeout.php