From 827be83a870da3509fa270156530e09d8100bb98 Mon Sep 17 00:00:00 2001 From: silverbackdan Date: Tue, 13 Oct 2026 17:47:01 +0100 Subject: [PATCH 1/2] Stop worker-mode memory growth: reset what a request collects ResourceMetadataProvider kept every serialised resource and its _metadata, and API Platform's skolem IRI converter (not tagged kernel.reset upstream, api-platform/core#8681) kept every _metadata object it named, each keeping the other's objects alive. The provider is now a WeakMap and reset, the skolem converter is tagged by ApiPlatformCompilerPass, PropagateUpdatesListener and the user email factories are reset, and a test fails for any bundle service that implements ResetInterface without the kernel.reset tag. --- CHANGELOG.md | 6 + CLAUDE.md | 3 + features/bootstrap/DoctrineContext.php | 28 +++++ features/main/worker_mode_memory.feature | 8 ++ .../CompilerPass/ApiPlatformCompilerPass.php | 15 +++ .../Doctrine/PropagateUpdatesListener.php | 5 +- .../User/Mailer/AbstractUserEmailFactory.php | 10 +- src/Resources/config/services.php | 27 +++-- .../ResourceMetadataProvider.php | 33 +++--- .../ApiPlatformCompilerPassTest.php | 29 +++++ .../ResetInterfaceServicesTest.php | 56 ++++++++++ .../Doctrine/PropagateUpdatesListenerTest.php | 105 ++++++++++++++++++ .../Mailer/AbstractUserEmailFactoryTest.php | 37 ++++++ .../ResourceMetadataProviderTest.php | 67 +++++++++++ 14 files changed, 399 insertions(+), 30 deletions(-) create mode 100644 features/main/worker_mode_memory.feature create mode 100644 tests/DependencyInjection/ResetInterfaceServicesTest.php create mode 100644 tests/EventListener/Doctrine/PropagateUpdatesListenerTest.php create mode 100644 tests/Serializer/ResourceMetadata/ResourceMetadataProviderTest.php diff --git a/CHANGELOG.md b/CHANGELOG.md index b5570771d..dfa622712 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -4,6 +4,12 @@ Every change that reaches `main` adds a line under **Unreleased**. When a versio ## Unreleased +### Breaking +- `ResourceMetadataProvider` no longer has the public `$metadatas` property or `getMetadatas()`: it keeps each resource's metadata in a `WeakMap` and is reset between requests (see Fixed) ([#447](https://github.com/components-web-app/api-components-bundle/pull/447)) + +### Fixed +- Memory no longer grows request after request in a worker (FrankenPHP worker mode). `ResourceMetadataProvider` kept every serialised resource and its `_metadata`, and API Platform's skolem IRI converter, which is not tagged `kernel.reset` (api-platform/core#8681), kept every `_metadata` object it gave a `/.well-known/genid/` IRI, each keeping the other's objects alive. Both are now reset between requests. `PropagateUpdatesListener` is reset too, so a flush that fails no longer leaves its entities to be purged and published by the next request's flush, and the user email factories no longer keep the last user they emailed ([#447](https://github.com/components-web-app/api-components-bundle/pull/447)) + ## [2.0.0-alpha.20](https://github.com/components-web-app/api-components-bundle/compare/2.0.0-alpha.19...2.0.0-alpha.20) - 2026-10-13 ### Added diff --git a/CLAUDE.md b/CLAUDE.md index 0f9671032..91b3c4632 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -386,6 +386,9 @@ The bundle does not set `use_symfony_listeners`. API Platform's default (`false` - Services use `silverback.api_components.*` ids with FQCN aliases, **except** AP state providers/processors referenced by class on an operation and `controller.service_arguments` actions. Those keep the **FQCN as primary id**, or AP throws `ProviderNotFoundException` and argument injection fails. - **Never rely on autoconfiguration, registration order or decoration inference in a bundle.** Tag and order explicitly; several definitions use `->autoconfigure(false)`. - **Services holding request-scoped state must be tagged `kernel.reset`** explicitly, or scope the state with a `WeakMap`. FrankenPHP worker mode keeps services alive across requests. Add each such service to `tests/DependencyInjection/ServicesResetterTest.php`. +- **A class that implements `ResetInterface` must be tagged `kernel.reset`, on every service of it, children of a parent definition included** (Symfony never inherits tags from a parent). `ResetInterfaceServicesTest` loads every bundle config file and fails on any that is not. Implementing the interface alone resets nothing. +- **A map keyed by objects that live for one request (entities, `_metadata`) is a `WeakMap` and is also reset** (#447). The `WeakMap` alone is not enough when a value can reference its key, as `ResourceMetadata`'s violations reference the resource. Never key by `spl_object_id`: it holds nothing and is reused once the object is freed. `worker_mode_memory.feature` sends requests through one kernel and asserts both maps are empty after each services reset. +- **API Platform's `api_platform.symfony.iri_converter.skolem` is tagged `kernel.reset` by `ApiPlatformCompilerPass`** until api-platform/core#8681 is released and the bundle requires it; then remove that step. It keeps every anonymous object it gives a `/.well-known/genid/` IRI (each `_metadata` object) for the life of the worker. - Optional collaborators (logger, purger) are wired as `new Reference(..., NULL_ON_INVALID_REFERENCE)`. - **Config nodes:** never put `isRequired()` on a child of a node with `addDefaultsIfNotSet()` or `canBeDisabled()`. The default is inserted without finalising, so the check never runs (#214). Give the child a real default or make the parent required. Use `->validate()` for invalid combinations. Env placeholders skip node validators, so validate again at run time. - `make:rename-component` generates through `doctrine.migrations.dependency_factory` and reads table names from `ClassMetadata`. **Never `LIKE`-match an IRI in a `json` column** (slashes are escaped). AP's `getIriFromResource()` returns a `/.well-known/genid/` blank node, not an exception, for a non-resource class. diff --git a/features/bootstrap/DoctrineContext.php b/features/bootstrap/DoctrineContext.php index 68fc649a0..fe6a77a6e 100644 --- a/features/bootstrap/DoctrineContext.php +++ b/features/bootstrap/DoctrineContext.php @@ -2358,6 +2358,34 @@ public function theManifestDepthShouldContainAnIriMatching(int $depth, string $p throw new \RuntimeException(\sprintf('Manifest depth %d has no IRI matching %s. Has: %s', $depth, $pattern, implode(', ', $iris))); } + /** + * @Then requests to :urls through one kernel leave nothing in the skolem IRI or resource metadata maps after a services reset + */ + public function requestsThroughOneKernelLeaveNothingAfterAServicesReset(string $urls): void + { + $container = $this->kernel->getContainer(); + $testContainer = $container->get('test.service_container'); + $skolem = $testContainer->get('api_platform.symfony.iri_converter.skolem'); + $provider = $testContainer->get('silverback.api_components.serializer.resource_metadata_provider'); + $sizes = static fn (): array => [ + 'skolem' => \count((new \ReflectionProperty($skolem, 'objectHashMap'))->getValue($skolem)), + 'metadata' => \count((new \ReflectionProperty($provider, 'metadatas'))->getValue($provider)), + ]; + + $this->manager->clear(); + foreach (explode(',', $urls) as $url) { + $status = $this->kernel->handle(Request::create($url, 'GET', server: ['HTTP_ACCEPT' => 'application/ld+json']))->getStatusCode(); + $during = $sizes(); + if (200 !== $status || 0 === $during['skolem'] || 0 === $during['metadata']) { + throw new \RuntimeException(\sprintf('%s returned %d and left %d skolem and %d metadata entries; expected 200 with entries in both.', $url, $status, $during['skolem'], $during['metadata'])); + } + $container->get('services_resetter')->reset(); + if ([0, 0] !== array_values($after = $sizes())) { + throw new \RuntimeException(\sprintf('After a services reset following %s, %d skolem and %d metadata entries remain.', $url, $after['skolem'], $after['metadata'])); + } + } + } + /** * @Then every IRI in the manifest should resolve */ diff --git a/features/main/worker_mode_memory.feature b/features/main/worker_mode_memory.feature new file mode 100644 index 000000000..7ea91c041 --- /dev/null +++ b/features/main/worker_mode_memory.feature @@ -0,0 +1,8 @@ +Feature: Nothing serialised in one request is kept for the next in worker mode + In order that a long-running worker does not grow until it hits its memory limit + As a FrankenPHP worker + I need every service that collects objects during a request to let them go when services are reset + + Scenario: The skolem IRI map and the resource metadata map are empty after each request's services reset + Given there is a PageData resource with a published component in a pageDataProperty position and the route path "/worker" + Then requests to "/_/routes//worker,/_/resource_manifest//worker,/_/routes,/_/routes//worker" through one kernel leave nothing in the skolem IRI or resource metadata maps after a services reset diff --git a/src/DependencyInjection/CompilerPass/ApiPlatformCompilerPass.php b/src/DependencyInjection/CompilerPass/ApiPlatformCompilerPass.php index 10d29a24c..9eb179b32 100644 --- a/src/DependencyInjection/CompilerPass/ApiPlatformCompilerPass.php +++ b/src/DependencyInjection/CompilerPass/ApiPlatformCompilerPass.php @@ -26,6 +26,7 @@ class ApiPlatformCompilerPass implements CompilerPassInterface private const string HTTP_CACHE_FLUSHER = 'silverback.api_components.http_cache.flusher'; private const string INVALIDATION_CLIENT_PREFIX = 'api_platform.invalidation_http_client.'; private const string EXCEPTION_TO_STATUS = 'api_platform.exception_to_status'; + private const string SKOLEM_IRI_CONVERTER = 'api_platform.symfony.iri_converter.skolem'; public function process(ContainerBuilder $container): void { @@ -44,6 +45,8 @@ public function process(ContainerBuilder $container): void $this->appendApiPlatformDefaultExceptionStatuses($container); + $this->resetSkolemIriConverter($container); + $apiPlatformMercurePublishListener = 'api_platform.doctrine.orm.listener.mercure.publish'; if ($container->hasDefinition($apiPlatformMercurePublishListener)) { // we have implemented fully custom logic @@ -60,6 +63,18 @@ private function appendApiPlatformDefaultExceptionStatuses(ContainerBuilder $con $container->setParameter(self::EXCEPTION_TO_STATUS, $container->getParameter(self::EXCEPTION_TO_STATUS) + $defaults); } + private function resetSkolemIriConverter(ContainerBuilder $container): void + { + if (!$container->hasDefinition(self::SKOLEM_IRI_CONVERTER)) { + return; + } + + $definition = $container->getDefinition(self::SKOLEM_IRI_CONVERTER); + if (!$definition->hasTag('kernel.reset')) { + $definition->addTag('kernel.reset', ['method' => 'reset']); + } + } + private function configureHttpCacheFlusher(ContainerBuilder $container): void { if (!$container->hasDefinition(self::HTTP_CACHE_FLUSHER)) { diff --git a/src/EventListener/Doctrine/PropagateUpdatesListener.php b/src/EventListener/Doctrine/PropagateUpdatesListener.php index 43ef90d2b..91a138a6e 100644 --- a/src/EventListener/Doctrine/PropagateUpdatesListener.php +++ b/src/EventListener/Doctrine/PropagateUpdatesListener.php @@ -35,8 +35,9 @@ use Silverback\ApiComponentsBundle\Repository\Core\ComponentPositionRepository; use Symfony\Component\PropertyAccess\PropertyAccess; use Symfony\Component\PropertyAccess\PropertyAccessor; +use Symfony\Contracts\Service\ResetInterface; -class PropagateUpdatesListener +class PropagateUpdatesListener implements ResetInterface { private PropertyAccessor $propertyAccessor; private ObjectRepository|EntityRepository $collectionRepository; @@ -292,7 +293,7 @@ private function addToPropagators(object $item, string $type): void } } - private function reset(): void + public function reset(): void { $this->updatedResources = new \SplObjectStorage(); $this->pageDataPropertiesChanged = []; diff --git a/src/Factory/User/Mailer/AbstractUserEmailFactory.php b/src/Factory/User/Mailer/AbstractUserEmailFactory.php index 47d0a5b91..c353c2d28 100644 --- a/src/Factory/User/Mailer/AbstractUserEmailFactory.php +++ b/src/Factory/User/Mailer/AbstractUserEmailFactory.php @@ -27,11 +27,12 @@ use Symfony\Component\Mime\Address; use Symfony\Component\Mime\Exception\RfcComplianceException as SymfonyRfcComplianceException; use Symfony\Component\Mime\RawMessage; +use Symfony\Contracts\Service\ResetInterface; /** * @author Daniel West */ -abstract class AbstractUserEmailFactory +abstract class AbstractUserEmailFactory implements ResetInterface { public const MESSAGE_ID_PREFIX = 'xx'; @@ -43,7 +44,7 @@ abstract class AbstractUserEmailFactory protected ?string $redirectPathQueryKey; protected array $emailContext; protected ?RawMessage $message; - private AbstractUser $user; + private ?AbstractUser $user = null; public function __construct(ContainerInterface $container, EventDispatcherInterface $eventDispatcher, string $subject, bool $enabled = true, ?string $defaultRedirectPath = null, ?string $redirectPathQueryKey = null, array $emailContext = []) { @@ -64,6 +65,11 @@ protected static function getContextKeys(): ?array ]; } + public function reset(): void + { + $this->user = null; + } + protected function initUser(AbstractUser $user): void { if (!$user->getUsername()) { diff --git a/src/Resources/config/services.php b/src/Resources/config/services.php index f7473630f..6006c1b2b 100644 --- a/src/Resources/config/services.php +++ b/src/Resources/config/services.php @@ -289,7 +289,8 @@ $services ->set('silverback.api_components.factory.user.mailer.change_email_confirmation_email') ->class(ChangeEmailConfirmationEmailFactory::class) - ->parent(AbstractUserEmailFactory::class); + ->parent(AbstractUserEmailFactory::class) + ->tag('kernel.reset', ['method' => 'reset']); $services->alias(ChangeEmailConfirmationEmailFactory::class, 'silverback.api_components.factory.user.mailer.change_email_confirmation_email'); $services @@ -748,13 +749,15 @@ $services ->set('silverback.api_components.factory.user.mailer.password_changed_email') ->class(PasswordChangedEmailFactory::class) - ->parent(AbstractUserEmailFactory::class); + ->parent(AbstractUserEmailFactory::class) + ->tag('kernel.reset', ['method' => 'reset']); $services->alias(PasswordChangedEmailFactory::class, 'silverback.api_components.factory.user.mailer.password_changed_email'); $services ->set('silverback.api_components.factory.user.mailer.password_reset_email') ->class(PasswordResetEmailFactory::class) - ->parent(AbstractUserEmailFactory::class); + ->parent(AbstractUserEmailFactory::class) + ->tag('kernel.reset', ['method' => 'reset']); $services->alias(PasswordResetEmailFactory::class, 'silverback.api_components.factory.user.mailer.password_reset_email'); $services @@ -1103,7 +1106,8 @@ $services ->set('silverback.api_components.serializer.resource_metadata_provider') - ->class(ResourceMetadataProvider::class); + ->class(ResourceMetadataProvider::class) + ->tag('kernel.reset', ['method' => 'reset']); $services->alias(ResourceMetadataProvider::class, 'silverback.api_components.serializer.resource_metadata_provider'); $services @@ -1660,7 +1664,8 @@ $services ->set('silverback.api_components.factory.user.mailer.user_enabled_email') ->class(UserEnabledEmailFactory::class) - ->parent(AbstractUserEmailFactory::class); + ->parent(AbstractUserEmailFactory::class) + ->tag('kernel.reset', ['method' => 'reset']); $services->alias(UserEnabledEmailFactory::class, 'silverback.api_components.factory.user.mailer.user_enabled_email'); $services @@ -1732,7 +1737,8 @@ $services ->set('silverback.api_components.factory.user.mailer.username_changed_email') ->class(UsernameChangedEmailFactory::class) - ->parent(AbstractUserEmailFactory::class); + ->parent(AbstractUserEmailFactory::class) + ->tag('kernel.reset', ['method' => 'reset']); $services->alias(UsernameChangedEmailFactory::class, 'silverback.api_components.factory.user.mailer.username_changed_email'); $services @@ -1843,13 +1849,15 @@ $services ->set('silverback.api_components.factory.user.mailer.verify_email') ->class(VerifyEmailFactory::class) - ->parent(AbstractUserEmailFactory::class); + ->parent(AbstractUserEmailFactory::class) + ->tag('kernel.reset', ['method' => 'reset']); $services->alias(VerifyEmailFactory::class, 'silverback.api_components.factory.user.mailer.verify_email'); $services ->set('silverback.api_components.factory.user.mailer.welcome_email') ->class(WelcomeEmailFactory::class) - ->parent(AbstractUserEmailFactory::class); + ->parent(AbstractUserEmailFactory::class) + ->tag('kernel.reset', ['method' => 'reset']); $services->alias(WelcomeEmailFactory::class, 'silverback.api_components.factory.user.mailer.welcome_email'); $services @@ -2088,7 +2096,8 @@ new Reference(CwaCollectorData::class), ]) ->tag('doctrine.event_listener', ['event' => DoctrineEvents::onFlush]) - ->tag('doctrine.event_listener', ['event' => DoctrineEvents::postFlush]); + ->tag('doctrine.event_listener', ['event' => DoctrineEvents::postFlush]) + ->tag('kernel.reset', ['method' => 'reset']); $services->alias(PropagateUpdatesListener::class, 'silverback.api_components.event_listener.doctrine.propagate_updates_listener'); $services diff --git a/src/Serializer/ResourceMetadata/ResourceMetadataProvider.php b/src/Serializer/ResourceMetadata/ResourceMetadataProvider.php index 88e982041..e323e7e7c 100644 --- a/src/Serializer/ResourceMetadata/ResourceMetadataProvider.php +++ b/src/Serializer/ResourceMetadata/ResourceMetadataProvider.php @@ -11,33 +11,32 @@ namespace Silverback\ApiComponentsBundle\Serializer\ResourceMetadata; -class ResourceMetadataProvider +use Symfony\Contracts\Service\ResetInterface; + +class ResourceMetadataProvider implements ResetInterface { - public array $metadatas = []; + /** + * @var \WeakMap + */ + private \WeakMap $metadatas; + + public function __construct() + { + $this->metadatas = new \WeakMap(); + } public function findResourceMetadata(object $object): ResourceMetadata { - $hash = spl_object_id($object); - if ($this->resourceMetadataExists($object)) { - return $this->metadatas[$hash]['metadata']; - } - $this->metadatas[$hash] = [ - 'resource' => $object, - 'metadata' => new ResourceMetadata(), - ]; - - return $this->metadatas[$hash]['metadata']; + return $this->metadatas[$object] ??= new ResourceMetadata(); } public function resourceMetadataExists(object $object): bool { - $hash = spl_object_id($object); - - return isset($this->metadatas[$hash]); + return isset($this->metadatas[$object]); } - public function getMetadatas(): array + public function reset(): void { - return $this->metadatas; + $this->metadatas = new \WeakMap(); } } diff --git a/tests/DependencyInjection/CompilerPass/ApiPlatformCompilerPassTest.php b/tests/DependencyInjection/CompilerPass/ApiPlatformCompilerPassTest.php index 66143b573..eaa4c5619 100644 --- a/tests/DependencyInjection/CompilerPass/ApiPlatformCompilerPassTest.php +++ b/tests/DependencyInjection/CompilerPass/ApiPlatformCompilerPassTest.php @@ -125,6 +125,35 @@ public function test_a_configured_status_for_a_default_exception_is_kept(): void self::assertSame(400, $exceptionToStatus[SerializerExceptionInterface::class]); } + public function test_the_skolem_iri_converter_is_reset_between_requests(): void + { + $container = $this->createContainer(); + $container->setDefinition('api_platform.symfony.iri_converter.skolem', new Definition()); + + (new ApiPlatformCompilerPass())->process($container); + + self::assertSame([['method' => 'reset']], $container->getDefinition('api_platform.symfony.iri_converter.skolem')->getTag('kernel.reset')); + } + + public function test_a_skolem_iri_converter_already_tagged_kernel_reset_keeps_its_one_tag(): void + { + $container = $this->createContainer(); + $container->setDefinition('api_platform.symfony.iri_converter.skolem', (new Definition())->addTag('kernel.reset', ['method' => 'reset'])); + + (new ApiPlatformCompilerPass())->process($container); + + self::assertSame([['method' => 'reset']], $container->getDefinition('api_platform.symfony.iri_converter.skolem')->getTag('kernel.reset')); + } + + public function test_without_a_skolem_iri_converter_nothing_is_defined(): void + { + $container = $this->createContainer(); + + (new ApiPlatformCompilerPass())->process($container); + + self::assertFalse($container->hasDefinition('api_platform.symfony.iri_converter.skolem')); + } + private function createContainer(): ContainerBuilder { $container = new ContainerBuilder(); diff --git a/tests/DependencyInjection/ResetInterfaceServicesTest.php b/tests/DependencyInjection/ResetInterfaceServicesTest.php new file mode 100644 index 000000000..4be411725 --- /dev/null +++ b/tests/DependencyInjection/ResetInterfaceServicesTest.php @@ -0,0 +1,56 @@ + + * + * For the full copyright and license information, please view the LICENSE + * file that was distributed with this source code. + */ + +namespace Silverback\ApiComponentsBundle\Tests\DependencyInjection; + +use PHPUnit\Framework\TestCase; +use Symfony\Component\Config\FileLocator; +use Symfony\Component\DependencyInjection\ContainerBuilder; +use Symfony\Component\DependencyInjection\Loader\PhpFileLoader; +use Symfony\Contracts\Service\ResetInterface; + +class ResetInterfaceServicesTest extends TestCase +{ + private const array CONFIG_FILES = [ + 'services.php', + 'services_normalizers.php', + 'services_doctrine_orm_http_cache_purger.php', + 'services_doctrine_orm_mercure_publisher.php', + 'services_orphaned_resources.php', + 'services_orphaned_files.php', + 'services_maker.php', + ]; + + public function test_every_bundle_service_that_implements_reset_interface_is_tagged_kernel_reset(): void + { + $container = new ContainerBuilder(); + $loader = new PhpFileLoader($container, new FileLocator(__DIR__ . '/../../src/Resources/config')); + foreach (self::CONFIG_FILES as $file) { + $loader->load($file); + } + + $checked = 0; + $untagged = []; + foreach ($container->getDefinitions() as $id => $definition) { + $class = $definition->getClass() ?? $id; + if ($definition->isAbstract() || !class_exists($class) || !is_subclass_of($class, ResetInterface::class)) { + continue; + } + ++$checked; + if (!$definition->hasTag('kernel.reset') && !$definition->hasTag('data_collector')) { + $untagged[] = \sprintf('%s (%s)', $id, $class); + } + } + + self::assertGreaterThan(5, $checked); + self::assertSame([], $untagged, 'These services implement ResetInterface but are not tagged kernel.reset, so worker mode never resets them.'); + } +} diff --git a/tests/EventListener/Doctrine/PropagateUpdatesListenerTest.php b/tests/EventListener/Doctrine/PropagateUpdatesListenerTest.php new file mode 100644 index 000000000..0c63ce381 --- /dev/null +++ b/tests/EventListener/Doctrine/PropagateUpdatesListenerTest.php @@ -0,0 +1,105 @@ + + * + * For the full copyright and license information, please view the LICENSE + * file that was distributed with this source code. + */ + +namespace Silverback\ApiComponentsBundle\Tests\EventListener\Doctrine; + +use ApiPlatform\Metadata\IriConverterInterface; +use ApiPlatform\Metadata\ResourceClassResolverInterface; +use Doctrine\ORM\EntityManagerInterface; +use Doctrine\ORM\EntityRepository; +use Doctrine\ORM\Event\OnFlushEventArgs; +use Doctrine\ORM\Event\PostFlushEventArgs; +use Doctrine\ORM\Mapping\ClassMetadata; +use Doctrine\ORM\UnitOfWork; +use Doctrine\Persistence\ManagerRegistry; +use PHPUnit\Framework\TestCase; +use Silverback\ApiComponentsBundle\DataProvider\PageDataProvider; +use Silverback\ApiComponentsBundle\EventListener\Doctrine\PropagateUpdatesListener; +use Silverback\ApiComponentsBundle\HttpCache\ResourceChangedPropagatorInterface; +use Silverback\ApiComponentsBundle\Repository\Core\ComponentPositionRepository; +use Silverback\ApiComponentsBundle\Tests\Functional\TestBundle\Entity\PageData; +use Symfony\Contracts\Service\ResetInterface; + +class PropagateUpdatesListenerTest extends TestCase +{ + public function test_a_reset_after_a_flush_that_never_completed_leaves_nothing_for_the_next_flush(): void + { + $positionRepository = $this->createMock(ComponentPositionRepository::class); + $positionRepository->expects(self::never())->method('findByPageDataProperties'); + $collectionRepository = $this->createMock(EntityRepository::class); + $collectionRepository->expects(self::never())->method('findBy'); + $listener = $this->listener($positionRepository, $collectionRepository); + + $listener->onFlush(new OnFlushEventArgs($this->entityManager([new PageData()], ['title' => ['old', 'new']]))); + + self::assertInstanceOf(ResetInterface::class, $listener); + $listener->reset(); + + $listener->postFlush(new PostFlushEventArgs($this->entityManager([], []))); + } + + public function test_without_a_reset_the_next_flush_propagates_what_the_unfinished_flush_collected(): void + { + $positionRepository = $this->createMock(ComponentPositionRepository::class); + $positionRepository->expects(self::once())->method('findByPageDataProperties')->with(['title'])->willReturn([]); + $collectionRepository = $this->createMock(EntityRepository::class); + $collectionRepository->expects(self::once())->method('findBy')->willReturn([]); + $listener = $this->listener($positionRepository, $collectionRepository); + + $listener->onFlush(new OnFlushEventArgs($this->entityManager([new PageData()], ['title' => ['old', 'new']]))); + $listener->postFlush(new PostFlushEventArgs($this->entityManager([], []))); + } + + private function listener(ComponentPositionRepository $positionRepository, EntityRepository $collectionRepository): PropagateUpdatesListener + { + $registry = $this->createStub(ManagerRegistry::class); + $registry->method('getRepository')->willReturn($collectionRepository); + $iriConverter = $this->createStub(IriConverterInterface::class); + $iriConverter->method('getIriFromResource')->willReturn('/page_data/page_datas/1'); + $resourceClassResolver = $this->createStub(ResourceClassResolverInterface::class); + $resourceClassResolver->method('getResourceClass')->willReturn(PageData::class); + $pageDataProvider = $this->createStub(PageDataProvider::class); + $pageDataProvider->method('findPageDataComponentMetadata')->willReturn([]); + + return new PropagateUpdatesListener( + $iriConverter, + $registry, + [$this->createStub(ResourceChangedPropagatorInterface::class)], + $resourceClassResolver, + $pageDataProvider, + $positionRepository, + ); + } + + /** + * @param list $insertions + * @param array> $changeSet + */ + private function entityManager(array $insertions, array $changeSet): EntityManagerInterface + { + $unitOfWork = $this->createStub(UnitOfWork::class); + $unitOfWork->method('getScheduledEntityInsertions')->willReturn($insertions); + $unitOfWork->method('getScheduledEntityUpdates')->willReturn([]); + $unitOfWork->method('getScheduledEntityDeletions')->willReturn([]); + $unitOfWork->method('getScheduledCollectionUpdates')->willReturn([]); + $unitOfWork->method('getScheduledCollectionDeletions')->willReturn([]); + $unitOfWork->method('getEntityChangeSet')->willReturn($changeSet); + + $classMetadata = $this->createStub(ClassMetadata::class); + $classMetadata->method('getAssociationMappings')->willReturn([]); + + $entityManager = $this->createStub(EntityManagerInterface::class); + $entityManager->method('getUnitOfWork')->willReturn($unitOfWork); + $entityManager->method('getClassMetadata')->willReturn($classMetadata); + + return $entityManager; + } +} diff --git a/tests/Factory/User/Mailer/AbstractUserEmailFactoryTest.php b/tests/Factory/User/Mailer/AbstractUserEmailFactoryTest.php index d19d96acd..13f802483 100644 --- a/tests/Factory/User/Mailer/AbstractUserEmailFactoryTest.php +++ b/tests/Factory/User/Mailer/AbstractUserEmailFactoryTest.php @@ -31,6 +31,7 @@ use Symfony\Component\HttpFoundation\RequestStack; use Symfony\Component\Mime\Address; use Symfony\Component\Mime\RawMessage; +use Symfony\Contracts\Service\ResetInterface; use Twig\Environment; use Twig\Loader\LoaderInterface; @@ -74,6 +75,42 @@ protected function getTemplate(): string ); } + public function test_a_reset_forgets_the_user_so_the_next_request_cannot_email_them(): void + { + $factory = new class($this->containerInterfaceMock, $this->eventDispatcherMock, '-') extends AbstractUserEmailFactory { + public function create(AbstractUser $user, array $context = []): ?RawMessage + { + $this->initUser($user); + + return null; + } + + public function createWithoutUser(): ?RawMessage + { + return $this->createEmailMessage(); + } + + protected function getTemplate(): string + { + return ''; + } + }; + $user = new class extends AbstractUser { + }; + $user->setUsername('my_username')->setEmailAddress('email@address.com'); + $factory->create($user); + $userReference = \WeakReference::create($user); + unset($user); + + self::assertInstanceOf(ResetInterface::class, $factory); + $factory->reset(); + gc_collect_cycles(); + + self::assertNull($userReference->get()); + $this->expectException(BadMethodCallException::class); + $factory->createWithoutUser(); + } + public function test_exception_thrown_if_user_has_no_username(): void { $userEmailFactory = new DummyUserEmailFactory($this->containerInterfaceMock, $this->eventDispatcherMock, 'email subject'); diff --git a/tests/Serializer/ResourceMetadata/ResourceMetadataProviderTest.php b/tests/Serializer/ResourceMetadata/ResourceMetadataProviderTest.php new file mode 100644 index 000000000..f9cbb39bc --- /dev/null +++ b/tests/Serializer/ResourceMetadata/ResourceMetadataProviderTest.php @@ -0,0 +1,67 @@ + + * + * For the full copyright and license information, please view the LICENSE + * file that was distributed with this source code. + */ + +namespace Silverback\ApiComponentsBundle\Tests\Serializer\ResourceMetadata; + +use PHPUnit\Framework\TestCase; +use Silverback\ApiComponentsBundle\Serializer\ResourceMetadata\ResourceMetadataProvider; +use Symfony\Component\Validator\ConstraintViolation; +use Symfony\Component\Validator\ConstraintViolationList; +use Symfony\Contracts\Service\ResetInterface; + +class ResourceMetadataProviderTest extends TestCase +{ + public function test_a_resource_has_one_metadata_until_reset(): void + { + $provider = new ResourceMetadataProvider(); + $resource = new \stdClass(); + + self::assertFalse($provider->resourceMetadataExists($resource)); + $metadata = $provider->findResourceMetadata($resource); + self::assertTrue($provider->resourceMetadataExists($resource)); + self::assertSame($metadata, $provider->findResourceMetadata($resource)); + self::assertNotSame($metadata, $provider->findResourceMetadata(new \stdClass())); + + self::assertInstanceOf(ResetInterface::class, $provider); + $provider->reset(); + + self::assertFalse($provider->resourceMetadataExists($resource)); + self::assertNotSame($metadata, $provider->findResourceMetadata($resource)); + } + + public function test_the_provider_keeps_neither_a_freed_resource_nor_its_metadata(): void + { + $provider = new ResourceMetadataProvider(); + $resource = new \stdClass(); + $resourceReference = \WeakReference::create($resource); + $metadataReference = \WeakReference::create($provider->findResourceMetadata($resource)); + + unset($resource); + gc_collect_cycles(); + + self::assertNull($resourceReference->get()); + self::assertNull($metadataReference->get()); + } + + public function test_metadata_whose_violations_reference_the_resource_is_freed_by_a_reset(): void + { + $provider = new ResourceMetadataProvider(); + $resource = new \stdClass(); + $provider->findResourceMetadata($resource)->setViolations(new ConstraintViolationList([new ConstraintViolation('invalid', null, [], $resource, 'title', null)])); + $resourceReference = \WeakReference::create($resource); + + unset($resource); + $provider->reset(); + gc_collect_cycles(); + + self::assertNull($resourceReference->get()); + } +} From 8750dcc71fbdca30a7388d70f7c571766caae1d3 Mon Sep 17 00:00:00 2001 From: silverbackdan Date: Tue, 13 Oct 2026 17:47:22 +0100 Subject: [PATCH 2/2] Link the changelog lines to #449 --- CHANGELOG.md | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index dfa622712..2829977ba 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -5,10 +5,10 @@ Every change that reaches `main` adds a line under **Unreleased**. When a versio ## Unreleased ### Breaking -- `ResourceMetadataProvider` no longer has the public `$metadatas` property or `getMetadatas()`: it keeps each resource's metadata in a `WeakMap` and is reset between requests (see Fixed) ([#447](https://github.com/components-web-app/api-components-bundle/pull/447)) +- `ResourceMetadataProvider` no longer has the public `$metadatas` property or `getMetadatas()`: it keeps each resource's metadata in a `WeakMap` and is reset between requests (see Fixed) ([#449](https://github.com/components-web-app/api-components-bundle/pull/449)) ### Fixed -- Memory no longer grows request after request in a worker (FrankenPHP worker mode). `ResourceMetadataProvider` kept every serialised resource and its `_metadata`, and API Platform's skolem IRI converter, which is not tagged `kernel.reset` (api-platform/core#8681), kept every `_metadata` object it gave a `/.well-known/genid/` IRI, each keeping the other's objects alive. Both are now reset between requests. `PropagateUpdatesListener` is reset too, so a flush that fails no longer leaves its entities to be purged and published by the next request's flush, and the user email factories no longer keep the last user they emailed ([#447](https://github.com/components-web-app/api-components-bundle/pull/447)) +- Memory no longer grows request after request in a worker (FrankenPHP worker mode). `ResourceMetadataProvider` kept every serialised resource and its `_metadata`, and API Platform's skolem IRI converter, which is not tagged `kernel.reset` (api-platform/core#8681), kept every `_metadata` object it gave a `/.well-known/genid/` IRI, each keeping the other's objects alive. Both are now reset between requests. `PropagateUpdatesListener` is reset too, so a flush that fails no longer leaves its entities to be purged and published by the next request's flush, and the user email factories no longer keep the last user they emailed ([#449](https://github.com/components-web-app/api-components-bundle/pull/449)) ## [2.0.0-alpha.20](https://github.com/components-web-app/api-components-bundle/compare/2.0.0-alpha.19...2.0.0-alpha.20) - 2026-10-13