From e1cda00a982f96cd9b233d6b627cf980a297bb67 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Julien=20Fastr=C3=A9?= Date: Tue, 16 Dec 2025 14:53:23 +0100 Subject: [PATCH] Optimize motive import with in-memory caching for deduplication. - Introduced an in-memory cache to store and reuse created or found motives during import, reducing redundant database operations. - Updated the logic to ensure parent motives are created or retrieved via the cache. - Modified the test suite to verify parent and child motive relationships with deduplication. - Ensured labels are consistently formatted and trimmed during the import process. --- .../Import/ImportMotivesFromDirectory.php | 115 +++++++++++++----- .../Import/ImportMotivesFromDirectoryTest.php | 74 +++++++++++ 2 files changed, 160 insertions(+), 29 deletions(-) diff --git a/src/Bundle/ChillTicketBundle/src/Service/Import/ImportMotivesFromDirectory.php b/src/Bundle/ChillTicketBundle/src/Service/Import/ImportMotivesFromDirectory.php index 3d9bf9d84..d61908366 100644 --- a/src/Bundle/ChillTicketBundle/src/Service/Import/ImportMotivesFromDirectory.php +++ b/src/Bundle/ChillTicketBundle/src/Service/Import/ImportMotivesFromDirectory.php @@ -21,11 +21,84 @@ use Symfony\Component\Yaml\Yaml; final readonly class ImportMotivesFromDirectory { + /** + * Cache en mémoire des Motive créés/trouvés pendant l'import. + * Clé: "$lang|lower(trim(label))" → valeur: Motive. + */ + private \ArrayObject $parentsAndMotives; + public function __construct( private EntityManagerInterface $entityManager, private StoredObjectManagerInterface $storedObjectManager, private MotiveRepository $motiveRepository, - ) {} + ) { + // Objet mutable pour rester compatible avec la classe readonly + $this->parentsAndMotives = new \ArrayObject(); + } + + private function buildCacheKey(string $lang, string $label): string + { + return $lang.'|'.mb_strtolower(trim($label)); + } + + private function getFromCache(string $lang, string $label): ?Motive + { + $key = $this->buildCacheKey($lang, $label); + + if ($this->parentsAndMotives->offsetExists($key)) { + return $this->parentsAndMotives->offsetGet($key); + } + + return null; + } + + private function putInCache(string $lang, string $label, Motive $motive): void + { + $this->parentsAndMotives->offsetSet($this->buildCacheKey($lang, $label), $motive); + } + + /** + * Trouve un Motive par label/lang en utilisant d'abord le cache, puis le repository. + * Si inexistant, le crée, le persiste, l’ajoute au cache et le retourne. + * + * @param array $baseLabels tableau de labels (sera copié/ajusté pour la langue courante) + */ + private function findOrCreateMotiveByLabel(string $name, string $lang, array $baseLabels): Motive + { + $name = trim($name); + + if ('' === $name) { + throw new \InvalidArgumentException('Empty motive name provided to findOrCreateMotiveByLabel'); + } + + if (null !== ($cached = $this->getFromCache($lang, $name))) { + return $cached; + } + + $existing = $this->motiveRepository->findByLabel($name, $lang); + if (\count($existing) > 1) { + throw new \RuntimeException(sprintf('Multiple motives found with label "%s" for lang "%s".', $name, $lang)); + } + $motive = $existing[0] ?? new Motive(); + + // Préparer les labels: copie, trim, puis forcer la valeur de la langue courante + $labels = $baseLabels; + foreach ($labels as $k => $v) { + if (\is_string($v)) { + $labels[$k] = trim($v); + } + } + $labels[$lang] = $name; // forcer le label courant + $motive->setLabel($labels); + + if (!isset($existing[0])) { + $this->entityManager->persist($motive); + } + + $this->putInCache($lang, $name, $motive); + + return $motive; + } public function import(string $directory, string $lang): void { @@ -73,37 +146,21 @@ final readonly class ImportMotivesFromDirectory [$parentName, $childName] = array_map('trim', explode('>', $childName, 2)); } - // Find or create the current motive (child or standalone) - $existing = $this->motiveRepository->findByLabel($childName, $lang); - if (\count($existing) > 1) { - throw new \RuntimeException(sprintf('Item %d: multiple motives found with label "%s" for lang "%s".', $index, $childName, $lang)); - } - $motive = $existing[0] ?? new Motive(); - - // Ensure the stored label for the current language is the child/standalone name (trimmed) - $labelArray[$lang] = $childName; - $motive->setLabel($labelArray); + // Find or create the current motive (child or standalone) en utilisant le cache + $motive = $this->getFromCache($lang, $childName) + ?? $this->findOrCreateMotiveByLabel($childName, $lang, $labelArray); + // S’assurer que le label courant reflète bien le nom de l’enfant/standalone + // (utile si l’entité existait déjà mais avec un label non trim) + $labelsForChild = $motive->getLabel(); + $labelsForChild[$lang] = $childName; + $motive->setLabel($labelsForChild); // If a parent is defined, ensure it exists and link it if (null !== $parentName && '' !== $parentName) { - $parentCandidates = $this->motiveRepository->findByLabel($parentName, $lang); - if (\count($parentCandidates) > 1) { - throw new \RuntimeException(sprintf('Item %d: multiple parent motives found with label "%s" for lang "%s".', $index, $parentName, $lang)); - } - $parent = $parentCandidates[0] ?? null; - if (null === $parent) { - $parent = new Motive(); - $parentLabel = $labelArray; - $parentLabel[$lang] = $parentName; - // Make sure all labels are trimmed - foreach ($parentLabel as $k => $v) { - if (\is_string($v)) { - $parentLabel[$k] = trim($v); - } - } - $parent->setLabel($parentLabel); - $this->entityManager->persist($parent); - } + // Trouver/créer le parent via cache pour éviter les doublons + $parentLabel = $labelArray; + $parent = $this->getFromCache($lang, $parentName) + ?? $this->findOrCreateMotiveByLabel($parentName, $lang, $parentLabel); $motive->setParent($parent); } diff --git a/src/Bundle/ChillTicketBundle/tests/Service/Import/ImportMotivesFromDirectoryTest.php b/src/Bundle/ChillTicketBundle/tests/Service/Import/ImportMotivesFromDirectoryTest.php index ba52d32fc..8939940b7 100644 --- a/src/Bundle/ChillTicketBundle/tests/Service/Import/ImportMotivesFromDirectoryTest.php +++ b/src/Bundle/ChillTicketBundle/tests/Service/Import/ImportMotivesFromDirectoryTest.php @@ -168,4 +168,78 @@ YAML; @unlink($tmpBase.DIRECTORY_SEPARATOR.'motives.yaml'); @rmdir($tmpBase); } + + public function testImportReusesParentAcrossItems(): void + { + // 1) Prépare un répertoire temporaire avec deux items partageant le même parent + $tmpBase = sys_get_temp_dir().DIRECTORY_SEPARATOR.'chill_ticket_import_'.bin2hex(random_bytes(6)); + $this->assertTrue(mkdir($tmpBase, 0777, true)); + + $yaml = <<<'YAML' +- label: + fr: "Parent > A" + ordering: 1 +- label: + fr: "Parent > B" + ordering: 2 +YAML; + file_put_contents($tmpBase.DIRECTORY_SEPARATOR.'motives.yaml', $yaml); + + // 2) Repository: parent/children absents initialement + $repoProphecy = $this->prophesize(MotiveRepository::class); + $repoProphecy->findByLabel('A', 'fr')->willReturn([])->shouldBeCalledTimes(1); + $repoProphecy->findByLabel('Parent', 'fr')->willReturn([])->shouldBeCalledTimes(1); + $repoProphecy->findByLabel('B', 'fr')->willReturn([])->shouldBeCalledTimes(1); + + // 3) Capture des persist() pour compter les entités créées + $persistedMotives = []; + $emProphecy = $this->prophesize(EntityManagerInterface::class); + $emProphecy->persist(Argument::type(Motive::class)) + ->will(function ($args) use (&$persistedMotives) { + $persistedMotives[] = $args[0]; + }) + ->shouldBeCalled(); + $emProphecy->flush()->shouldBeCalled(); + + // 4) Exécute l'import + $somMock = $this->createMock(StoredObjectManagerInterface::class); + $importer = new ImportMotivesFromDirectory( + $emProphecy->reveal(), + $somMock, + $repoProphecy->reveal(), + ); + $importer->import($tmpBase, 'fr'); + + // 5) Vérifications: un seul parent créé et partagé (déduplication par instance) + $unique = []; + foreach ($persistedMotives as $m) { + if (!$m instanceof Motive) { + continue; + } + $unique[spl_object_hash($m)] = $m; + } + + $parents = []; + $children = []; + foreach ($unique as $m) { + $label = $m->getLabel(); + $fr = $label['fr'] ?? null; + if ('Parent' === $fr) { + $parents[] = $m; + } + if ('A' === $fr || 'B' === $fr) { + $children[] = $m; + } + } + + $this->assertCount(1, $parents, 'Le parent doit être créé une seule fois'); + $this->assertCount(2, $children, 'Les deux enfants doivent être créés'); + $this->assertSame($parents[0], $children[0]->getParent()); + $this->assertSame($parents[0], $children[1]->getParent()); + $this->assertTrue($parents[0]->isParent()); + + // 6) Nettoyage + @unlink($tmpBase.DIRECTORY_SEPARATOR.'motives.yaml'); + @rmdir($tmpBase); + } }