Fix configuring a metadata class that extends the bundle's Metadata

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-<hash of the checkout><TEST_TOKEN>/`, 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-<hash of the checkout><TEST_TOKEN>/`, 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
 

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
 ```
 

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');
+        }
     }
 }

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;
         }
 

tests/Functional/App/Entity/CustomMetadata.php

@@ -0,0 +1,14 @@
+<?php
+
+declare(strict_types=1);
+
+namespace Setono\ClientBundle\Tests\Functional\App\Entity;
+
+use Setono\ClientBundle\Entity\Metadata;
+
+/**
+ * An application's own metadata entity, configured with the metadata_class option
+ */
+class CustomMetadata extends Metadata
+{
+}

tests/Functional/App/Kernel.php

@@ -9,6 +9,7 @@
 use Doctrine\DBAL\Connection;
 use Setono\ClientBundle\SetonoClientBundle;
 use Setono\ClientBundle\Tests\Functional\App\Controller\ClientController;
+use Setono\ClientBundle\Tests\Functional\App\Entity\CustomMetadata;
 use Symfony\Bundle\FrameworkBundle\FrameworkBundle;
 use Symfony\Bundle\FrameworkBundle\Kernel\MicroKernelTrait;
 use Symfony\Component\DependencyInjection\Loader\Configurator\ContainerConfigurator;
@@ -20,6 +21,11 @@ final class Kernel extends BaseKernel
 {
     use MicroKernelTrait;
 
+    /**
+     * In this environment the application stores the metadata in its own entity
+     */
+    public const ENVIRONMENT_CUSTOM_METADATA = 'custom_metadata';
+
     public function registerBundles(): iterable
     {
         yield new FrameworkBundle();
@@ -101,6 +107,33 @@ protected function configureContainer(ContainerConfigurator $container): void
             ->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

tests/Functional/App/config/doctrine/CustomMetadata.orm.xml

@@ -0,0 +1,7 @@
+<?xml version="1.0" encoding="UTF-8"?>
+<doctrine-mapping xmlns="http://doctrine-project.org/schemas/orm/doctrine-mapping"
+                  xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance"
+                  xsi:schemaLocation="http://doctrine-project.org/schemas/orm/doctrine-mapping
+                  http://doctrine-project.org/schemas/orm/doctrine-mapping.xsd">
+    <entity name="Setono\ClientBundle\Tests\Functional\App\Entity\CustomMetadata" table="app_client_metadata"/>
+</doctrine-mapping>

tests/Functional/CustomMetadataClassTest.php

@@ -0,0 +1,40 @@
+<?php
+
+declare(strict_types=1);
+
+namespace Setono\ClientBundle\Tests\Functional;
+
+use Setono\ClientBundle\Entity\Metadata;
+use Setono\ClientBundle\Tests\Functional\App\Entity\CustomMetadata;
+use Setono\ClientBundle\Tests\Functional\App\Kernel;
+
+/**
+ * Runs the metadata tests with an application entity that extends the bundle's Metadata class
+ */
+final class CustomMetadataClassTest extends MetadataTest
+{
+    protected static function getEnvironment(): string
+    {
+        return Kernel::ENVIRONMENT_CUSTOM_METADATA;
+    }
+
+    /**
+     * @test
+     */
+    public function it_stores_the_metadata_in_the_configured_entity(): void
+    {
+        $this->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);
+    }
+}

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());
-    }
 }

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
      */

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<array-key, mixed>
      */

tests/Unit/DependencyInjection/Fixture/InterfaceOnlyMetadata.php

@@ -0,0 +1,31 @@
+<?php
+
+declare(strict_types=1);
+
+namespace Setono\ClientBundle\Tests\Unit\DependencyInjection\Fixture;
+
+use Setono\ClientBundle\Entity\MetadataInterface;
+
+/**
+ * A metadata class that implements the interface without extending the bundle's Metadata class
+ */
+final class InterfaceOnlyMetadata implements MetadataInterface
+{
+    public function getClientId(): ?string
+    {
+        return null;
+    }
+
+    public function setClientId(string $clientId): void
+    {
+    }
+
+    public function getMetadata(): array
+    {
+        return [];
+    }
+
+    public function setMetadata(array $metadata): void
+    {
+    }
+}

tests/Unit/DependencyInjection/SetonoClientExtensionTest.php

@@ -10,7 +10,9 @@
 use Setono\ClientBundle\CookieProvider\RequestBasedCookieProvider;
 use Setono\ClientBundle\DependencyInjection\SetonoClientExtension;
 use Setono\ClientBundle\Entity\Metadata;
+use Setono\ClientBundle\EventListener\Doctrine\ConvertToEntityListener;
 use Setono\ClientBundle\Tests\Unit\DependencyInjection\Fixture\CustomMetadata;
+use Setono\ClientBundle\Tests\Unit\DependencyInjection\Fixture\InterfaceOnlyMetadata;
 
 final class SetonoClientExtensionTest extends AbstractExtensionTestCase
 {
@@ -78,6 +80,36 @@ public function after_loading_the_services_hooking_into_symfony_and_doctrine_are
         ]);
     }
 
+    /**
+     * @test
+     */
+    public function after_loading_with_a_metadata_class_that_extends_the_bundles_class_the_doctrine_listener_is_removed(): void
+    {
+        $this->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
      */

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();