diff --git a/CHANGELOG.md b/CHANGELOG.md index 6dd2075..beaf41a 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -21,6 +21,10 @@ project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.html). for virtual setters; creates and attaches the missing translation for the exact locale, without locale fallback, regardless of `auto_create_translations` +- Translations of translatable resources are now fetch-joined (experimental + `TranslationsEagerLoadingExtension`), so listing N resources issues one query + instead of one translation query per entity per locale; disable with the + `eager_load_translations` option (#10) ### Changed - Modern bundle layout: the bundle class extends `AbstractBundle` and services diff --git a/README.md b/README.md index e8fd465..34201db 100644 --- a/README.md +++ b/README.md @@ -55,6 +55,11 @@ api_platform_translation: # 3.0; leaving the option unset is deprecated since 2.1. auto_create_translations: true + # Whether queries for translatable resources fetch-join the translations, + # so listing N resources issues one query instead of one per entity per + # locale. Set false to restore lazy loading. + eager_load_translations: true + # Ordered sources the request locale is resolved from; the first source # producing a locale wins. Remove a source to disable it. locale_resolution: @@ -359,10 +364,6 @@ Limitations: virtual getters, so built-in API Platform filters (`SearchFilter`, `OrderFilter`, ...) cannot target them on the resource. Filtering on translation fields requires a custom filter joining the translation entity. -- **Collections issue one translation query per item.** The `translations` - association is `EXTRA_LAZY` and resolved per entity, so listing N resources - triggers N additional queries for their translations; there is no built-in - eager loading yet. ## Contribution diff --git a/UPGRADE-2.1.md b/UPGRADE-2.1.md index 469f348..927309d 100644 --- a/UPGRADE-2.1.md +++ b/UPGRADE-2.1.md @@ -48,5 +48,14 @@ These only affect you if you referenced bundle files by path: The bundle now has a configuration tree under `api_platform_translation` (`enabled_locales`, `fallback_locale`, `auto_create_translations`, -`locale_resolution`). All defaults preserve 2.0 behavior; see the README +`eager_load_translations`, `locale_resolution`). All defaults except +`eager_load_translations` preserve 2.0 behavior; see the README "Configuration" section. + +## Eager loading of translations + +Queries for translatable resources now fetch-join the translations +(experimental), so listing N resources issues one query instead of one +translation query per entity per locale. Responses are unchanged; only the +generated SQL differs. Set `eager_load_translations: false` to restore the +2.0 lazy behavior. diff --git a/config/services.php b/config/services.php index 694febc..1dc2443 100644 --- a/config/services.php +++ b/config/services.php @@ -4,6 +4,8 @@ namespace Symfony\Component\DependencyInjection\Loader\Configurator; +use ApiPlatform\Doctrine\Orm\Extension\QueryCollectionExtensionInterface; +use Locastic\ApiPlatformTranslationBundle\Doctrine\Orm\Extension\TranslationsEagerLoadingExtension; use Locastic\ApiPlatformTranslationBundle\EventListener\AssignLocaleListener; use Locastic\ApiPlatformTranslationBundle\Serializer\TranslatableItemDenormalizer; use Locastic\ApiPlatformTranslationBundle\Translation\Translator; @@ -37,6 +39,19 @@ $services->alias(TranslatableItemDenormalizer::class, 'locastic_api_platform_translation.serializer.translatable_denormalizer'); + // Doctrine ORM: eager loading of translations (the interface lives in + // api-platform/doctrine-orm, which api-platform/symfony does not require) + if (interface_exists(QueryCollectionExtensionInterface::class)) { + $services->set('locastic_api_platform_translation.doctrine.orm.query_extension.translations_eager_loading', TranslationsEagerLoadingExtension::class) + ->args([ + param('locastic_api_platform_translation.eager_load_translations'), + ]) + ->tag('api_platform.doctrine.orm.query_extension.collection', ['priority' => -18]) + ->tag('api_platform.doctrine.orm.query_extension.item', ['priority' => -8]); + + $services->alias(TranslationsEagerLoadingExtension::class, 'locastic_api_platform_translation.doctrine.orm.query_extension.translations_eager_loading'); + } + // Filters $services->set('locastic_api_platform_translation.filter.translation_groups') ->parent('api_platform.serializer.group_filter') diff --git a/src/ApiPlatformTranslationBundle.php b/src/ApiPlatformTranslationBundle.php index 935c123..4063234 100644 --- a/src/ApiPlatformTranslationBundle.php +++ b/src/ApiPlatformTranslationBundle.php @@ -31,6 +31,10 @@ public function configure(DefinitionConfigurator $definition): void ->info('Whether reading a translation that does not exist creates an empty one and attaches it to the entity (with cascade persist, a read can then insert empty rows). Set false so reads never write: missing translations just show empty fields. Defaults to true; the default flips to false in 3.0.') ->defaultNull() ->end() + ->booleanNode('eager_load_translations') + ->info('Whether queries for translatable resources fetch-join the translations, so a collection issues one query instead of one per entity per locale. Set false to restore lazy loading.') + ->defaultTrue() + ->end() ->arrayNode('locale_resolution') ->info('Ordered list of sources the request locale is resolved from; the first source producing a locale wins. Remove a source to disable it.') ->performNoDeepMerging() @@ -47,7 +51,7 @@ public function configure(DefinitionConfigurator $definition): void } /** - * @param array{enabled_locales: list, fallback_locale: ?string, auto_create_translations: ?bool, locale_resolution: list} $config + * @param array{enabled_locales: list, fallback_locale: ?string, auto_create_translations: ?bool, eager_load_translations: bool, locale_resolution: list} $config */ public function loadExtension(array $config, ContainerConfigurator $container, ContainerBuilder $builder): void { @@ -63,6 +67,7 @@ public function loadExtension(array $config, ContainerConfigurator $container, C ->set('locastic_api_platform_translation.enabled_locales', $config['enabled_locales'] ?: '%kernel.enabled_locales%') ->set('locastic_api_platform_translation.fallback_locale', $config['fallback_locale'] ?? '%kernel.default_locale%') ->set('locastic_api_platform_translation.auto_create_translations', $config['auto_create_translations'] ?? true) + ->set('locastic_api_platform_translation.eager_load_translations', $config['eager_load_translations']) ->set('locastic_api_platform_translation.locale_resolution', $config['locale_resolution']); $container->import('../config/services.php'); diff --git a/src/DependencyInjection/ApiPlatformTranslationExtension.php b/src/DependencyInjection/ApiPlatformTranslationExtension.php index 41a38fd..9aca7d6 100644 --- a/src/DependencyInjection/ApiPlatformTranslationExtension.php +++ b/src/DependencyInjection/ApiPlatformTranslationExtension.php @@ -30,6 +30,7 @@ public function load(array $configs, ContainerBuilder $container): void $container->setParameter('locastic_api_platform_translation.enabled_locales', '%kernel.enabled_locales%'); $container->setParameter('locastic_api_platform_translation.fallback_locale', '%kernel.default_locale%'); $container->setParameter('locastic_api_platform_translation.auto_create_translations', true); + $container->setParameter('locastic_api_platform_translation.eager_load_translations', true); $container->setParameter('locastic_api_platform_translation.locale_resolution', ['query_param', 'accept_language']); $loader = new PhpFileLoader($container, new FileLocator(\dirname(__DIR__, 2).DIRECTORY_SEPARATOR.'config')); diff --git a/src/Doctrine/Orm/Extension/TranslationsEagerLoadingExtension.php b/src/Doctrine/Orm/Extension/TranslationsEagerLoadingExtension.php new file mode 100644 index 0000000..56e3062 --- /dev/null +++ b/src/Doctrine/Orm/Extension/TranslationsEagerLoadingExtension.php @@ -0,0 +1,90 @@ +joinTranslations($queryBuilder, $queryNameGenerator, $resourceClass); + } + + /** + * @param array $identifiers + */ + public function applyToItem(QueryBuilder $queryBuilder, QueryNameGeneratorInterface $queryNameGenerator, string $resourceClass, array $identifiers, ?Operation $operation = null, array $context = []): void + { + $this->joinTranslations($queryBuilder, $queryNameGenerator, $resourceClass); + } + + private function joinTranslations(QueryBuilder $queryBuilder, QueryNameGeneratorInterface $queryNameGenerator, string $resourceClass): void + { + if (!$this->enabled || !is_a($resourceClass, TranslatableInterface::class, true)) { + return; + } + + $rootAlias = $queryBuilder->getRootAliases()[0] ?? null; + if (null === $rootAlias || $this->hasFetchJoin($queryBuilder, $rootAlias)) { + return; + } + + $joinAlias = $queryNameGenerator->generateJoinAlias('translations'); + $queryBuilder + ->leftJoin(\sprintf('%s.translations', $rootAlias), $joinAlias) + ->addSelect($joinAlias); + } + + /** + * Only an unconstrained, selected join counts: a filter may join translations + * WITH a locale condition, and selecting that alias would hydrate a partial + * collection, so such joins are ignored and a dedicated one is added. + */ + private function hasFetchJoin(QueryBuilder $queryBuilder, string $rootAlias): bool + { + $selected = []; + foreach ($queryBuilder->getDQLPart('select') as $select) { + foreach ($select->getParts() as $part) { + if (\is_string($part)) { + $selected[] = $part; + } + } + } + + /** @var array $joinParts */ + $joinParts = $queryBuilder->getDQLPart('join'); + foreach ($joinParts[$rootAlias] ?? [] as $join) { + if (\sprintf('%s.translations', $rootAlias) === $join->getJoin() + && null === $join->getCondition() + && \in_array($join->getAlias(), $selected, true) + ) { + return true; + } + } + + return false; + } +} diff --git a/tests/DependencyInjection/ApiPlatformTranslationExtensionTest.php b/tests/DependencyInjection/ApiPlatformTranslationExtensionTest.php index 0b93604..b171bd0 100644 --- a/tests/DependencyInjection/ApiPlatformTranslationExtensionTest.php +++ b/tests/DependencyInjection/ApiPlatformTranslationExtensionTest.php @@ -28,6 +28,7 @@ public function testDeprecatedExtensionStillRegistersServices(): void $this->assertTrue($container->hasDefinition('locastic_api_platform_translation.filter.translation_groups')); $this->assertSame('%kernel.enabled_locales%', $container->getParameter('locastic_api_platform_translation.enabled_locales')); $this->assertTrue($container->getParameter('locastic_api_platform_translation.auto_create_translations')); + $this->assertTrue($container->getParameter('locastic_api_platform_translation.eager_load_translations')); $this->assertSame('api_platform_translation', $extension->getAlias()); } } diff --git a/tests/Doctrine/Orm/Extension/TranslationsEagerLoadingExtensionTest.php b/tests/Doctrine/Orm/Extension/TranslationsEagerLoadingExtensionTest.php new file mode 100644 index 0000000..a42a1d0 --- /dev/null +++ b/tests/Doctrine/Orm/Extension/TranslationsEagerLoadingExtensionTest.php @@ -0,0 +1,177 @@ += 80400) { + $config->enableNativeLazyObjects(true); + } else { + $config->setProxyDir(sys_get_temp_dir()); + $config->setProxyNamespace('LocasticApiPlatformTranslationTestProxies'); + } + $connection = DriverManager::getConnection(['driver' => 'pdo_sqlite', 'memory' => true], $config); + $this->em = new EntityManager($connection, $config); + + $schemaTool = new SchemaTool($this->em); + $schemaTool->createSchema($this->em->getMetadataFactory()->getAllMetadata()); + } + + public function testCollectionQueryFetchJoinsTranslations(): void + { + $queryBuilder = $this->createQueryBuilder(); + + (new TranslationsEagerLoadingExtension()) + ->applyToCollection($queryBuilder, new QueryNameGenerator(), IntegrationArticle::class); + + self::assertSame( + \sprintf('SELECT o, translations_a1 FROM %s o LEFT JOIN o.translations translations_a1', IntegrationArticle::class), + $queryBuilder->getDQL(), + ); + } + + public function testItemQueryFetchJoinsTranslations(): void + { + $queryBuilder = $this->createQueryBuilder(); + + (new TranslationsEagerLoadingExtension()) + ->applyToItem($queryBuilder, new QueryNameGenerator(), IntegrationArticle::class, ['id' => 1]); + + self::assertStringContainsString('LEFT JOIN o.translations translations_a1', $queryBuilder->getDQL()); + } + + public function testHydratedCollectionsAreInitialized(): void + { + $this->seedArticle(['en' => 'EN one', 'de' => 'DE one']); + $this->seedArticle(['en' => 'EN two', 'de' => 'DE two']); + + $queryBuilder = $this->createQueryBuilder(); + (new TranslationsEagerLoadingExtension()) + ->applyToCollection($queryBuilder, new QueryNameGenerator(), IntegrationArticle::class); + + /** @var list $articles */ + $articles = $queryBuilder->getQuery()->getResult(); + + self::assertCount(2, $articles); + foreach ($articles as $article) { + $translations = $article->getTranslations(); + self::assertInstanceOf(PersistentCollection::class, $translations); + // An initialized PersistentCollection resolves matching() in memory, + // so every later getTranslation() is query-free. + self::assertTrue($translations->isInitialized()); + self::assertCount(2, $translations); + } + self::assertSame('DE one', $articles[0]->getTranslation('de')->getTitle()); + } + + public function testSkipsNonTranslatableResources(): void + { + $queryBuilder = $this->createQueryBuilder(); + $dql = $queryBuilder->getDQL(); + + (new TranslationsEagerLoadingExtension()) + ->applyToCollection($queryBuilder, new QueryNameGenerator(), DummyNotTranslatable::class); + + self::assertSame($dql, $queryBuilder->getDQL()); + } + + public function testDoesNothingWhenDisabled(): void + { + $queryBuilder = $this->createQueryBuilder(); + $dql = $queryBuilder->getDQL(); + + (new TranslationsEagerLoadingExtension(false)) + ->applyToCollection($queryBuilder, new QueryNameGenerator(), IntegrationArticle::class); + + self::assertSame($dql, $queryBuilder->getDQL()); + } + + public function testDoesNotDuplicateAnExistingFetchJoin(): void + { + $queryBuilder = $this->createQueryBuilder(); + $queryNameGenerator = new QueryNameGenerator(); + $extension = new TranslationsEagerLoadingExtension(); + + $extension->applyToCollection($queryBuilder, $queryNameGenerator, IntegrationArticle::class); + $dql = $queryBuilder->getDQL(); + $extension->applyToCollection($queryBuilder, $queryNameGenerator, IntegrationArticle::class); + + self::assertSame($dql, $queryBuilder->getDQL()); + } + + public function testAddsItsOwnJoinNextToALocaleConstrainedFilterJoin(): void + { + $queryBuilder = $this->createQueryBuilder() + ->leftJoin('o.translations', 't', Join::WITH, "t.locale = 'en'"); + + (new TranslationsEagerLoadingExtension()) + ->applyToCollection($queryBuilder, new QueryNameGenerator(), IntegrationArticle::class); + + // Selecting the constrained alias would hydrate a partial collection, so + // the filter join must be left alone and an unconstrained one added. + self::assertSame( + \sprintf("SELECT o, translations_a1 FROM %s o LEFT JOIN o.translations t WITH t.locale = 'en' LEFT JOIN o.translations translations_a1", IntegrationArticle::class), + $queryBuilder->getDQL(), + ); + } + + private function createQueryBuilder(): QueryBuilder + { + return $this->em->createQueryBuilder() + ->select('o') + ->from(IntegrationArticle::class, 'o'); + } + + /** + * @param array $localeToTitle + */ + private function seedArticle(array $localeToTitle): void + { + $article = new IntegrationArticle(); + foreach ($localeToTitle as $locale => $title) { + $translation = new IntegrationArticleTranslation(); + $translation->setLocale($locale); + $translation->setTitle($title); + $article->addTranslation($translation); + } + + $this->em->persist($article); + $this->em->flush(); + $this->em->clear(); + } +} diff --git a/tests/Functional/BundleInitializationTest.php b/tests/Functional/BundleInitializationTest.php index 2eac886..fe6846a 100644 --- a/tests/Functional/BundleInitializationTest.php +++ b/tests/Functional/BundleInitializationTest.php @@ -5,6 +5,7 @@ namespace Locastic\ApiPlatformTranslationBundle\Tests\Functional; use ApiPlatform\Serializer\Filter\GroupFilter; +use Locastic\ApiPlatformTranslationBundle\Doctrine\Orm\Extension\TranslationsEagerLoadingExtension; use Locastic\ApiPlatformTranslationBundle\EventListener\AssignLocaleListener; use Locastic\ApiPlatformTranslationBundle\Tests\Fixtures\TestKernel; use Locastic\ApiPlatformTranslationBundle\Translation\Translator; @@ -40,5 +41,9 @@ public function testContainerCompilesAndRegistersServices(): void GroupFilter::class, $container->get('locastic_api_platform_translation.filter.translation_groups') ); + $this->assertInstanceOf( + TranslationsEagerLoadingExtension::class, + $container->get('locastic_api_platform_translation.doctrine.orm.query_extension.translations_eager_loading') + ); } } diff --git a/tests/Functional/ConfigurationTest.php b/tests/Functional/ConfigurationTest.php index 9d1e68a..388175f 100644 --- a/tests/Functional/ConfigurationTest.php +++ b/tests/Functional/ConfigurationTest.php @@ -52,6 +52,7 @@ public function testDefaultsWithBareFramework(): void $this->assertSame([], $container->getParameter('locastic_api_platform_translation.enabled_locales')); $this->assertSame('en', $container->getParameter('locastic_api_platform_translation.fallback_locale')); $this->assertTrue($container->getParameter('locastic_api_platform_translation.auto_create_translations')); + $this->assertTrue($container->getParameter('locastic_api_platform_translation.eager_load_translations')); $this->assertSame( [Translator::RESOLUTION_QUERY_PARAM, Translator::RESOLUTION_ACCEPT_LANGUAGE], $container->getParameter('locastic_api_platform_translation.locale_resolution') @@ -69,6 +70,17 @@ public function testAutoCreateTranslationsCanBeDisabled(): void $this->assertFalse($container->getParameter('locastic_api_platform_translation.auto_create_translations')); } + public function testEagerLoadTranslationsCanBeDisabled(): void + { + self::$translationConfig = [ + 'eager_load_translations' => false, + ]; + self::bootKernel(); + $container = static::getContainer(); + + $this->assertFalse($container->getParameter('locastic_api_platform_translation.eager_load_translations')); + } + public function testDefaultsInheritFrameworkLocales(): void { self::$frameworkConfig = [