Skip to content

Fix production cache:clear crash when DoctrineBundle's metadata warmer runs#136

Merged
xavierlacot merged 3 commits into
jolicode:mainfrom
vvasiloi:fix/metadata-warmer-doctrine-conflict
Jul 17, 2026
Merged

Fix production cache:clear crash when DoctrineBundle's metadata warmer runs#136
xavierlacot merged 3 commits into
jolicode:mainfrom
vvasiloi:fix/metadata-warmer-doctrine-conflict

Conversation

@vvasiloi

@vvasiloi vvasiloi commented Jul 12, 2026

Copy link
Copy Markdown
Contributor

MediaEntityMetadataWarmer calls getAllMetadata(), which loads every class metadata into the shared factory. DoctrineBundle's own DoctrineMetadataCacheWarmer is optional — it runs later, and only in non-debug kernels — and throws DoctrineMetadataCacheWarmer must load metadata first, check priority of your warmers when the factory already holds metadata. Result: every production cache:clear crashes in apps using the bundle with Doctrine ORM (dev/CI never see it, so it surfaces on deploy).

Fix: reset the entity managers after warming, so later warmers get a clean metadata factory.

The added test boots the test application, runs the bundle warmer, and then runs a real DoctrineMetadataCacheWarmer — red on current main, green with the fix.

@vvasiloi
vvasiloi force-pushed the fix/metadata-warmer-doctrine-conflict branch 2 times, most recently from b5a3317 to 9c0b84c Compare July 12, 2026 17:39
@vvasiloi
vvasiloi marked this pull request as ready for review July 12, 2026 17:45
@lyrixx
lyrixx requested a review from Copilot July 17, 2026 07:59

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR prevents production cache:clear crashes in apps using this bundle with Doctrine ORM by ensuring the Doctrine entity manager metadata factory is left in a clean state after MediaEntityMetadataWarmer runs, allowing DoctrineBundle’s metadata cache warmer to run afterwards without hitting a LogicException.

Changes:

  • Reset all Doctrine managers at the end of MediaEntityMetadataWarmer::warmUp() to avoid leaking loaded metadata into subsequent cache warmers.
  • Add an integration-style kernel test that runs the bundle’s warmer and then executes a real DoctrineMetadataCacheWarmer, asserting it successfully writes its output file.

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.

File Description
src/Cache/MediaEntityMetadataWarmer.php Resets the Doctrine managers after warming to avoid impacting later warmers that assume a clean metadata factory.
tests/src/Cache/MediaEntityMetadataWarmerTest.php Adds coverage to verify DoctrineBundle’s metadata cache warmer can run after this bundle’s warmer without failing.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment thread src/Cache/MediaEntityMetadataWarmer.php Outdated
@loic425

loic425 commented Jul 17, 2026

Copy link
Copy Markdown
Contributor

@vvasiloi Thx for your proposal! I've fixed the PHPUnit test in the main branch. Could you rebase and apply Copilot suggestions please?

@vvasiloi

Copy link
Copy Markdown
Contributor Author

@vvasiloi Thx for your proposal! I've fixed the PHPUnit test in the main branch. Could you rebase and apply Copilot suggestions please?

Will do!

vvasiloi and others added 2 commits July 17, 2026 13:13
getAllMetadata() loads every class metadata into the shared factory;
DoctrineBundle's own metadata warmer is optional (runs after this one,
in non-debug kernels) and throws 'DoctrineMetadataCacheWarmer must load
metadata first' on a factory that already holds metadata — breaking
every production cache:clear in apps using the bundle with Doctrine.
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
@xavierlacot
xavierlacot force-pushed the fix/metadata-warmer-doctrine-conflict branch from c32caee to 638f113 Compare July 17, 2026 11:13
@vvasiloi

Copy link
Copy Markdown
Contributor Author

I see @xavierlacot already did that! Thanks! 🥇

@xavierlacot

Copy link
Copy Markdown
Member

Nice, thanks @vvasiloi 🙏

@xavierlacot
xavierlacot merged commit 64be9cc into jolicode:main Jul 17, 2026
6 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants