diff --git a/CLAUDE.md b/CLAUDE.md index 8ee652d..5c869c7 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -14,7 +14,7 @@ PHP 8.1+ is required (CI runs 8.1 and 8.2). Use the `8.1`/`8.2` shell switchers - Tests: `composer phpunit` (or `vendor/bin/phpunit`) runs both suites; pick one with `--testsuite unit` or `--testsuite functional` - `tests/Unit`: isolated classes with Prophecy doubles. - - `tests/Functional`: the bundle inside a real Symfony app (`tests/Functional/App/Kernel.php`: FrameworkBundle + DoctrineBundle + DAMA DoctrineTestBundle + this bundle), driven over HTTP with `KernelBrowser` against a SQLite database. `tests/bootstrap.php` creates the schema once per PHPUnit process, and DAMA's PHPUnit extension (registered in `phpunit.xml.dist`) runs every test in a transaction that it rolls back. So tests must not run DDL: MySQL commits a transaction on DDL. To check which queries a request ran, read DoctrineBundle's debug data holder (see `MetadataTest::getExecutedQueries()`). Extend `FunctionalTestCase`, which rethrows exceptions instead of rendering error pages and has helpers to read the stored metadata. Every request gets a new container, like PHP-FPM. The kernel always runs in the `test` environment with debug on, whatever `APP_ENV`/`APP_DEBUG` say (the Symfony CLI's `php` sets `APP_ENV=dev`). Its cache, log and database live in `sys_get_temp_dir()/setono-client-bundle-/`, so nothing is generated in the repo, and neither two checkouts nor parallel Infection processes share a container or a database. The bootstrap also removes the cached container, because Infection swaps in mutated code without changing file times. Set `SETONO_CLIENT_BUNDLE_DATABASE_URL` (not `DATABASE_URL`, on purpose) to run against another database: the bootstrap drops the bundle's tables in it, and all Infection processes share it, so run Infection with `--threads=1` then. + - `tests/Functional`: the bundle inside a real Symfony app (`tests/Functional/App/Kernel.php`: FrameworkBundle + DoctrineBundle + DAMA DoctrineTestBundle + this bundle), driven over HTTP with `KernelBrowser` against a SQLite database. `tests/bootstrap.php` creates the schema once per PHPUnit process, and DAMA's PHPUnit extension (registered in `phpunit.xml.dist`) runs every test in a transaction that it rolls back. So tests must not run DDL: MySQL commits a transaction on DDL. To check which queries a request ran, read DoctrineBundle's debug data holder (see `MetadataTest::getExecutedQueries()`). Extend `FunctionalTestCase`, which rethrows exceptions instead of rendering error pages and has helpers to read the stored metadata. Every request gets a new container, like PHP-FPM. The kernel always runs in the `test` environment with debug on, whatever `APP_ENV`/`APP_DEBUG` say (the Symfony CLI's `php` sets `APP_ENV=dev`). A test class can pick another environment by overriding `getEnvironment()`, like `CustomMetadataClassTest`. Every environment needs its schema created in `tests/bootstrap.php`, and one that maps the entities differently also needs its own DAMA connection key and no static metadata cache (see the `custom_metadata` block in the kernel). Its cache, log and database live in `sys_get_temp_dir()/setono-client-bundle-/`, so nothing is generated in the repo, and neither two checkouts nor parallel Infection processes share a container or a database. The bootstrap also removes the cached container, because Infection swaps in mutated code without changing file times. Set `SETONO_CLIENT_BUNDLE_DATABASE_URL` (not `DATABASE_URL`, on purpose) to run against another database: the bootstrap drops the bundle's tables in it, and all Infection processes share it, so run Infection with `--threads=1` then. - Single test: `vendor/bin/phpunit --filter after_loading_the_correct_parameter_has_been_set` or by path `vendor/bin/phpunit tests/Unit/DependencyInjection/SetonoClientExtensionTest.php` - Static analysis: `composer analyse` (PHPStan at `level: max`, config in `phpstan.neon.dist`; auto-loads the doctrine/symfony/phpunit/strict-rules extensions via `phpstan/extension-installer`) - Check style: `composer check-style` (ECS, Sylius Labs ruleset) @@ -53,7 +53,7 @@ Three concerns are each built as a **Symfony service-decoration chain** in `conf ### Entity mapping -`Entity/Metadata` (table `setono_client__metadata`, string id `clientId`, nullable `json` `metadata`) is mapped in `config/doctrine-mapping/Metadata.orm.xml` as a **mapped-superclass**. `EventListener/Doctrine/ConvertToEntityListener` listens on Doctrine's `loadClassMetadata` and flips `isMappedSuperclass = false` so the bundle's own `Metadata` becomes a concrete entity when the app provides no replacement. Apps can swap in a custom class via the `metadata_class` config option; `SetonoClientExtension::load()` enforces that it implements `Entity/MetadataInterface` and they are responsible for mapping it. +`Entity/Metadata` (table `setono_client__metadata`, string id `clientId`, nullable `json` `metadata`) is mapped in `config/doctrine-mapping/Metadata.orm.xml` as a **mapped-superclass**. `EventListener/Doctrine/ConvertToEntityListener` listens on Doctrine's `loadClassMetadata` and flips `isMappedSuperclass = false` on the bundle's own `Metadata`, so it becomes a concrete entity. Apps can swap in a custom class via the `metadata_class` config option; `SetonoClientExtension::load()` enforces that it implements `Entity/MetadataInterface`, stores its declared name (e.g. without a leading backslash) in the parameter, and the app is responsible for mapping it. When the custom class extends `Entity/Metadata`, the extension removes the listener: the bundle's class stays a mapped superclass and the app's entity inherits its fields (`tests/Functional/CustomMetadataClassTest.php` runs `MetadataTest` that way). A custom class that only implements the interface leaves the bundle's `Metadata` an entity, as before. ### Configuration diff --git a/README.md b/README.md index fb188f2..c49542e 100644 --- a/README.md +++ b/README.md @@ -238,6 +238,10 @@ setono_client: # Entity used to persist metadata. Override it with your own class to add mapped fields # or behaviour; it must implement Setono\ClientBundle\Entity\MetadataInterface. + # If your class extends Setono\ClientBundle\Entity\Metadata, map it as an entity: it inherits + # the client id and metadata fields, because the bundle's class then stays a mapped superclass. + # It doesn't inherit the table name, so map it to table="setono_client__metadata" to keep the + # metadata you have already stored. Otherwise your next migration drops that table. metadata_class: Setono\ClientBundle\Entity\Metadata ``` diff --git a/src/DependencyInjection/SetonoClientExtension.php b/src/DependencyInjection/SetonoClientExtension.php index 4af2c3c..1b60ad5 100644 --- a/src/DependencyInjection/SetonoClientExtension.php +++ b/src/DependencyInjection/SetonoClientExtension.php @@ -4,6 +4,7 @@ namespace Setono\ClientBundle\DependencyInjection; +use Setono\ClientBundle\Entity\Metadata; use Setono\ClientBundle\Entity\MetadataInterface; use Symfony\Component\Config\FileLocator; use Symfony\Component\DependencyInjection\ContainerBuilder; @@ -27,10 +28,19 @@ public function load(array $configs, ContainerBuilder $container): void )); } + // Doctrine only knows the class by its declared name, e.g. without a leading backslash + $metadataClass = (new \ReflectionClass($config['metadata_class']))->getName(); + $container->setParameter('setono_client.cookie.name', $config['cookie']['name']); $container->setParameter('setono_client.cookie.expiration', $config['cookie']['expiration']); - $container->setParameter('setono_client.metadata_class', $config['metadata_class']); + $container->setParameter('setono_client.metadata_class', $metadataClass); $loader->load('services.xml'); + + // An application entity that extends the bundle's Metadata inherits its fields, but only while the bundle's class + // stays a mapped superclass. Doctrine would otherwise see two entities without an inheritance mapping + if (is_subclass_of($metadataClass, Metadata::class)) { + $container->removeDefinition('setono_client.event_listener_doctrine.convert_to_entity'); + } } } diff --git a/src/EventListener/Doctrine/ConvertToEntityListener.php b/src/EventListener/Doctrine/ConvertToEntityListener.php index 2708c82..ab3a4ca 100644 --- a/src/EventListener/Doctrine/ConvertToEntityListener.php +++ b/src/EventListener/Doctrine/ConvertToEntityListener.php @@ -9,8 +9,9 @@ use Setono\ClientBundle\Entity\Metadata; /** - * If the user of the plugin doesn't add their own Metadata, - * this listener will make sure that Doctrine sees our Metadata as an entity + * The bundle maps its Metadata class as a mapped superclass, so an application can extend it with its own entity. + * This listener makes Doctrine see the bundle's Metadata class as an entity. SetonoClientExtension only registers it + * when the configured metadata class doesn't extend the bundle's class */ final class ConvertToEntityListener { @@ -19,12 +20,8 @@ final class ConvertToEntityListener */ public function loadClassMetadata(LoadClassMetadataEventArgs $eventArgs): void { - if (!is_a($eventArgs->getClassMetadata()->getName(), Metadata::class, true)) { - return; - } - $metadata = $eventArgs->getClassMetadata(); - if (!$metadata instanceof ClassMetadata) { + if (Metadata::class !== $metadata->getName() || !$metadata instanceof ClassMetadata) { return; } diff --git a/tests/Functional/App/Entity/CustomMetadata.php b/tests/Functional/App/Entity/CustomMetadata.php new file mode 100644 index 0000000..98313e2 --- /dev/null +++ b/tests/Functional/App/Entity/CustomMetadata.php @@ -0,0 +1,14 @@ +load(__NAMESPACE__ . '\\Controller\\', __DIR__ . '/Controller')->tag('controller.service_arguments') ->load(__NAMESPACE__ . '\\EventSubscriber\\', __DIR__ . '/EventSubscriber') ; + + if (self::ENVIRONMENT_CUSTOM_METADATA === $this->environment) { + $container->extension('setono_client', [ + 'metadata_class' => CustomMetadata::class, + ]); + + $container->extension('doctrine', [ + 'orm' => [ + 'mappings' => [ + 'App' => [ + 'type' => 'xml', + 'is_bundle' => false, + 'dir' => __DIR__ . '/config/doctrine', + 'prefix' => __NAMESPACE__ . '\\Entity', + ], + ], + ], + ]); + + // DAMA DoctrineTestBundle shares its static connection and its static metadata cache between kernels by the + // names of the connection and the cache, which are the same in both environments. But this environment has + // its own database and maps the bundle's Metadata as a mapped superclass + $container->extension('dama_doctrine_test', [ + 'connection_keys' => ['default' => self::ENVIRONMENT_CUSTOM_METADATA], + 'enable_static_meta_data_cache' => false, + ]); + } } protected function configureRoutes(RoutingConfigurator $routes): void diff --git a/tests/Functional/App/config/doctrine/CustomMetadata.orm.xml b/tests/Functional/App/config/doctrine/CustomMetadata.orm.xml new file mode 100644 index 0000000..eb9baff --- /dev/null +++ b/tests/Functional/App/config/doctrine/CustomMetadata.orm.xml @@ -0,0 +1,7 @@ + + + + diff --git a/tests/Functional/CustomMetadataClassTest.php b/tests/Functional/CustomMetadataClassTest.php new file mode 100644 index 0000000..cd1ba8a --- /dev/null +++ b/tests/Functional/CustomMetadataClassTest.php @@ -0,0 +1,40 @@ +browser->request('GET', '/metadata/set?key=foo&value=bar'); + + self::assertResponseIsSuccessful(); + self::assertSame('app_client_metadata', self::getEntityManager()->getClassMetadata(CustomMetadata::class)->getTableName()); + self::assertSame(['foo' => 'bar'], self::fetchStoredMetadata($this->getClientId())); + } + + /** + * @test + */ + public function it_keeps_the_bundles_metadata_a_mapped_superclass(): void + { + self::assertTrue(self::getEntityManager()->getClassMetadata(Metadata::class)->isMappedSuperclass); + } +} diff --git a/tests/Functional/DoctrineMappingTest.php b/tests/Functional/DoctrineMappingTest.php index ffade33..5d468df 100644 --- a/tests/Functional/DoctrineMappingTest.php +++ b/tests/Functional/DoctrineMappingTest.php @@ -4,7 +4,6 @@ namespace Setono\ClientBundle\Tests\Functional; -use Doctrine\ORM\Tools\SchemaValidator; use Setono\ClientBundle\Entity\Metadata; final class DoctrineMappingTest extends FunctionalTestCase @@ -23,15 +22,4 @@ public function it_maps_the_metadata_as_an_entity(): void self::assertSame('json', $classMetadata->getTypeOfField('metadata')); self::assertTrue($classMetadata->isNullable('metadata')); } - - /** - * @test - */ - public function its_mapping_is_valid_and_matches_the_database_schema(): void - { - $validator = new SchemaValidator(self::getEntityManager()); - - self::assertSame([], $validator->validateMapping()); - self::assertTrue($validator->schemaInSyncWithMetadata()); - } } diff --git a/tests/Functional/FunctionalTestCase.php b/tests/Functional/FunctionalTestCase.php index e652d16..936e684 100644 --- a/tests/Functional/FunctionalTestCase.php +++ b/tests/Functional/FunctionalTestCase.php @@ -26,7 +26,7 @@ abstract class FunctionalTestCase extends WebTestCase protected function setUp(): void { // Fixed, so APP_ENV and APP_DEBUG in the environment, e.g. from the Symfony CLI, don't change the application - $this->browser = static::createClient(['environment' => 'test', 'debug' => true]); + $this->browser = static::createClient(['environment' => static::getEnvironment(), 'debug' => true]); // An exception fails the test instead of becoming an error response $this->browser->catchExceptions(false); @@ -37,6 +37,11 @@ protected static function getKernelClass(): string return Kernel::class; } + protected static function getEnvironment(): string + { + return 'test'; + } + /** * Returns the cookie stored in the browser */ diff --git a/tests/Functional/MetadataTest.php b/tests/Functional/MetadataTest.php index 09fae06..d574052 100644 --- a/tests/Functional/MetadataTest.php +++ b/tests/Functional/MetadataTest.php @@ -4,10 +4,14 @@ namespace Setono\ClientBundle\Tests\Functional; +use Doctrine\ORM\Tools\SchemaValidator; use Setono\Client\Metadata; use Symfony\Bridge\Doctrine\Middleware\Debug\DebugDataHolder; -final class MetadataTest extends FunctionalTestCase +/** + * Not final, because CustomMetadataClassTest runs these tests with an application's own metadata entity + */ +class MetadataTest extends FunctionalTestCase { /** * @test @@ -160,6 +164,20 @@ public function it_keeps_the_row_of_a_concurrent_request_that_stored_it_first(): self::assertSame(['set_by' => 'concurrent request'], self::fetchStoredMetadata($this->getClientId())); } + /** + * The schema check fails if the mapping makes Doctrine see a difference to the schema it created itself, which would + * make every doctrine:migrations:diff generate a migration + * + * @test + */ + public function its_mapping_is_valid_and_matches_the_database_schema(): void + { + $validator = new SchemaValidator(self::getEntityManager()); + + self::assertSame([], $validator->validateMapping()); + self::assertTrue($validator->schemaInSyncWithMetadata()); + } + /** * @return array */ diff --git a/tests/Unit/DependencyInjection/Fixture/InterfaceOnlyMetadata.php b/tests/Unit/DependencyInjection/Fixture/InterfaceOnlyMetadata.php new file mode 100644 index 0000000..be3c321 --- /dev/null +++ b/tests/Unit/DependencyInjection/Fixture/InterfaceOnlyMetadata.php @@ -0,0 +1,31 @@ +load(['metadata_class' => CustomMetadata::class]); + + $this->assertContainerBuilderNotHasService('setono_client.event_listener_doctrine.convert_to_entity'); + } + + /** + * @test + */ + public function after_loading_with_a_metadata_class_that_only_implements_the_interface_the_doctrine_listener_is_kept(): void + { + $this->load(['metadata_class' => InterfaceOnlyMetadata::class]); + + $this->assertContainerBuilderHasService('setono_client.event_listener_doctrine.convert_to_entity', ConvertToEntityListener::class); + } + + /** + * @test + */ + public function after_loading_the_metadata_class_parameter_holds_the_declared_class_name(): void + { + $this->load(['metadata_class' => '\\' . CustomMetadata::class]); + + $this->assertContainerBuilderHasParameter('setono_client.metadata_class', CustomMetadata::class); + } + /** * @test */ diff --git a/tests/bootstrap.php b/tests/bootstrap.php index 343a11f..1568b00 100644 --- a/tests/bootstrap.php +++ b/tests/bootstrap.php @@ -17,7 +17,7 @@ (new Filesystem())->remove(Kernel::getVarDir() . '/cache'); // DAMA DoctrineTestBundle rolls back what each test writes, so the schema is created once, before it starts its transactions -foreach (['test'] as $environment) { +foreach (['test', Kernel::ENVIRONMENT_CUSTOM_METADATA] as $environment) { $kernel = new Kernel($environment, true); $kernel->boot();