diff --git a/src/Override/OverrideAwareTranslator.php b/src/Override/OverrideAwareTranslator.php index 0eeb7c0..c06b09b 100644 --- a/src/Override/OverrideAwareTranslator.php +++ b/src/Override/OverrideAwareTranslator.php @@ -5,6 +5,7 @@ namespace CylleneDigital\AiTranslationBundle\Override; use CylleneDigital\AiTranslationBundle\Catalogue\TranslationCatalogue; +use CylleneDigital\AiTranslationBundle\Entity\TranslationOverride; use CylleneDigital\AiTranslationBundle\Locale\LocaleFallback; use CylleneDigital\AiTranslationBundle\Repository\TranslationOverrideRepository; use CylleneDigital\AiTranslationBundle\Scope\ScopeProviderInterface; @@ -307,7 +308,10 @@ private function fetchOverrides(string $locale, string $scope): array // 3. within a rank, additional-root catalogues ("@label/…" — themes and // friends, whose files also win at runtime) shadow the main root's; // 4. catalogue/key order otherwise. - usort($overrides, static fn ($a, $b): int => [-LocaleFallback::rank($a->getLocale()), '' === $a->getScope(), !str_starts_with($a->getCatalogue(), '@'), $a->getCatalogue(), $a->getKey()] <=> [-LocaleFallback::rank($b->getLocale()), '' === $b->getScope(), !str_starts_with($b->getCatalogue(), '@'), $b->getCatalogue(), $b->getKey()]); + // The sort key of each row is built once: ranking both locales again in every + // comparison weighed more than the query on a large locale. + $sortKeys = array_map(static fn (TranslationOverride $override): array => [-LocaleFallback::rank($override->getLocale()), '' === $override->getScope(), !str_starts_with($override->getCatalogue(), '@'), $override->getCatalogue(), $override->getKey()], $overrides); + uksort($overrides, static fn (int $a, int $b): int => $sortKeys[$a] <=> $sortKeys[$b]); $result = []; /** @var array> $winningLocale domain => key => the locale that answered */ diff --git a/src/Override/OverrideReader.php b/src/Override/OverrideReader.php index 5e4fa68..a5d16d4 100644 --- a/src/Override/OverrideReader.php +++ b/src/Override/OverrideReader.php @@ -67,11 +67,11 @@ public function getTranslationsForCatalogues(array $catalogues, string $locale, } } - foreach ($this->repository->findByLocaleAndCatalogues($locale, $catalogues, $scope) as $override) { - $catalogue = $override->getCatalogue(); - $key = $override->getKey(); + foreach ($this->repository->findValuesByLocaleAndCatalogues($locale, $catalogues, $scope) as $row) { + $catalogue = $row['catalogue']; + $key = $row['key']; $translations[$catalogue][$key] ??= $empty; - $translations[$catalogue][$key]['override'] = $override->getValue(); + $translations[$catalogue][$key]['override'] = $row['value']; $translations[$catalogue][$key]['hasOverride'] = true; } @@ -106,13 +106,21 @@ public function getEffectiveOverridesByCatalogue(string $locale, string $scope = $overrides = $this->repository->findForRuntime($locale, $scope); // Least specific locale first, and within a locale the global rows before the - // scope's own ones: the later passes overwrite. - usort($overrides, static fn (TranslationOverride $a, TranslationOverride $b): int => [LocaleFallback::rank($a->getLocale()), '' === $a->getScope() ? 0 : 1] <=> [LocaleFallback::rank($b->getLocale()), '' === $b->getScope() ? 0 : 1]); + // scope's own ones: the later passes overwrite. Bucketed, not sorted: a comparison + // that ranks both locales again weighed more than the query on a large locale. + $passes = []; + foreach ($overrides as $override) { + $passes[LocaleFallback::rank($override->getLocale()) * 2 + ('' === $override->getScope() ? 0 : 1)][] = $override; + } + + ksort($passes); $byCatalogue = []; - foreach ($overrides as $override) { - $byCatalogue[$override->getCatalogue()][$override->getKey()] = $override->getValue(); + foreach ($passes as $pass) { + foreach ($pass as $override) { + $byCatalogue[$override->getCatalogue()][$override->getKey()] = $override->getValue(); + } } return $byCatalogue; @@ -374,12 +382,22 @@ private function inherited(string $locale, string $scope, ?array $catalogues = n static fn (array $row): bool => $row['locale'] !== $locale || $row['scope'] !== $scope, )); - usort($rows, static fn (array $a, array $b): int => [-LocaleFallback::rank($a['locale']), '' === $a['scope']] <=> [-LocaleFallback::rank($b['locale']), '' === $b['scope']]); + // The more specific locale first, and within a locale the scope's row before the + // global one. Bucketed, not sorted: a comparison that ranks both locales again + // weighed more than the query on a large locale. + $passes = []; + foreach ($rows as $row) { + $passes[-LocaleFallback::rank($row['locale']) * 2 + ('' === $row['scope'] ? 1 : 0)][] = $row; + } + + ksort($passes); $inherited = []; - foreach ($rows as $row) { - $inherited[$row['catalogue']][(string) $row['key']] ??= ['value' => $row['value'], 'locale' => $row['locale'], 'scope' => $row['scope']]; + foreach ($passes as $pass) { + foreach ($pass as $row) { + $inherited[$row['catalogue']][(string) $row['key']] ??= ['value' => $row['value'], 'locale' => $row['locale'], 'scope' => $row['scope']]; + } } return $inherited; diff --git a/src/Repository/TranslationOverrideRepository.php b/src/Repository/TranslationOverrideRepository.php index 6b5d22d..8f86dca 100644 --- a/src/Repository/TranslationOverrideRepository.php +++ b/src/Repository/TranslationOverrideRepository.php @@ -88,20 +88,23 @@ public function lockOneByKey(string $key, string $catalogue, string $locale, str /** * The overrides of these catalogues in one (locale, scope): one query for a whole view, - * filtered in SQL — a single catalogue costs what it always cost. + * filtered in SQL — a single catalogue costs what it always cost. Scalar rows, not + * entities: a view reads thousands of them, and building an entity per row (and + * leaving it in the host's unit of work) cost more than the query itself. * * @param list $catalogues * - * @return list + * @return list */ - public function findByLocaleAndCatalogues(string $locale, array $catalogues, string $scope = ''): array + public function findValuesByLocaleAndCatalogues(string $locale, array $catalogues, string $scope = ''): array { if ([] === $catalogues) { return []; } - /** @var list $result */ - $result = $this->createQueryBuilder('t') + /** @var list $rows */ + $rows = $this->createQueryBuilder('t') + ->select('t.key', 't.catalogue', 't.value') ->andWhere('t.locale = :locale') ->andWhere('t.catalogue IN (:catalogues)') ->andWhere('t.scope = :scope') @@ -111,9 +114,9 @@ public function findByLocaleAndCatalogues(string $locale, array $catalogues, str ->orderBy('t.catalogue', 'ASC') ->addOrderBy('t.key', 'ASC') ->getQuery() - ->getResult(); + ->getArrayResult(); - return $result; + return $rows; } /** diff --git a/tests/Unit/Estimation/GenerationCostEstimatorTest.php b/tests/Unit/Estimation/GenerationCostEstimatorTest.php index 00cf153..1d04b28 100644 --- a/tests/Unit/Estimation/GenerationCostEstimatorTest.php +++ b/tests/Unit/Estimation/GenerationCostEstimatorTest.php @@ -218,8 +218,11 @@ private function createEstimator(array $frMessages, array $frShopMessages = [], $author->method('getAuthorIdentifier')->willReturn(null); $overrideRepository = $this->createStub(TranslationOverrideRepository::class); - $overrideRepository->method('findByLocaleAndCatalogues')->willReturnCallback(static fn (string $locale, array $catalogues, string $scope = ''): array => null !== $overrides - ? array_merge(...array_map(static fn (string $catalogue): array => $overrides($locale, $catalogue, $scope), array_values(array_filter($catalogues, is_string(...))))) + $overrideRepository->method('findValuesByLocaleAndCatalogues')->willReturnCallback(static fn (string $locale, array $catalogues, string $scope = ''): array => null !== $overrides + ? array_map( + static fn (TranslationOverride $override): array => ['key' => $override->getKey(), 'catalogue' => $override->getCatalogue(), 'value' => $override->getValue()], + array_merge(...array_map(static fn (string $catalogue): array => $overrides($locale, $catalogue, $scope), array_values(array_filter($catalogues, is_string(...))))), + ) : []); $suggestionRepository = $this->createStub(TranslationSuggestionRepository::class); diff --git a/tests/Unit/Override/TranslationManagerTest.php b/tests/Unit/Override/TranslationManagerTest.php index 4c45b23..40fb751 100644 --- a/tests/Unit/Override/TranslationManagerTest.php +++ b/tests/Unit/Override/TranslationManagerTest.php @@ -89,9 +89,9 @@ public function testTranslationsMergeFileEntriesWithOverrides(): void $orphan->setValue('Orpheline'); $this->repository - ->method('findByLocaleAndCatalogues') + ->method('findValuesByLocaleAndCatalogues') ->with('fr_FR', ['shop/Product/messages']) - ->willReturn([$override, $orphan]); + ->willReturn(self::rows($override, $orphan)); $manager = $this->createManager(); $translations = $manager->getTranslationsForCatalogue('shop/Product/messages', 'fr_FR'); @@ -110,7 +110,7 @@ public function testTranslationsMergeFileEntriesWithOverrides(): void public function testAKeyOnlyTranslatedInAnotherLanguageIsMissing(): void { - $this->repository->method('findByLocaleAndCatalogues')->willReturn([]); + $this->repository->method('findValuesByLocaleAndCatalogues')->willReturn([]); $manager = $this->createManager(); @@ -215,10 +215,10 @@ public function testAScopedBrowseShowsTheGlobalOverrideAsInherited(): void $scoped->setValue('Ajouter au panier (B2B)'); $this->repository - ->method('findByLocaleAndCatalogues') + ->method('findValuesByLocaleAndCatalogues') ->willReturnCallback(static fn (string $locale, array $catalogues, string $scope = ''): array => match ($scope) { - '' => [$global], - 'b2b' => [$scoped], + '' => self::rows($global), + 'b2b' => self::rows($scoped), default => [], }); // What the entry can inherit from — its own b2b row included, set apart by the reader. @@ -312,4 +312,14 @@ private function suggestionRepository(): TranslationSuggestionRepository return $repository; } + + /** + * What the view reads of these overrides: scalar rows. + * + * @return list + */ + private static function rows(TranslationOverride ...$overrides): array + { + return array_map(static fn (TranslationOverride $override): array => ['key' => $override->getKey(), 'catalogue' => $override->getCatalogue(), 'value' => $override->getValue()], array_values($overrides)); + } } diff --git a/tests/Unit/Suggestion/TranslationAiServiceTest.php b/tests/Unit/Suggestion/TranslationAiServiceTest.php index 33d8c20..f577eb7 100644 --- a/tests/Unit/Suggestion/TranslationAiServiceTest.php +++ b/tests/Unit/Suggestion/TranslationAiServiceTest.php @@ -115,8 +115,11 @@ protected function setUp(): void $this->staged[] = $suggestion; }); $this->suggestionRepository->method('findPending')->willReturnCallback(fn (): array => $this->pending); - $this->overrideRepository->method('findByLocaleAndCatalogues')->willReturnCallback(fn (string $locale, array $catalogues, string $scope = ''): array => null !== $this->overrides - ? array_merge(...array_map(fn (string $catalogue): array => ($this->overrides)($locale, $catalogue, $scope), array_values(array_filter($catalogues, is_string(...))))) + $this->overrideRepository->method('findValuesByLocaleAndCatalogues')->willReturnCallback(fn (string $locale, array $catalogues, string $scope = ''): array => null !== $this->overrides + ? array_map( + static fn (TranslationOverride $override): array => ['key' => $override->getKey(), 'catalogue' => $override->getCatalogue(), 'value' => $override->getValue()], + array_merge(...array_map(fn (string $catalogue): array => ($this->overrides)($locale, $catalogue, $scope), array_values(array_filter($catalogues, is_string(...))))), + ) : []); // The chain read behind the inherited values, served from the same per-test rows. $this->overrideRepository->method('findInheritable')->willReturnCallback(function (array $locales, string $scope, ?array $catalogues = null): array {