fix: プラグインで Entity 拡張時に redeclare fatal になる問題を修正 (EccubeBundle の auto_mapping を無効化) - #6963
fix: プラグインで Entity 拡張時に redeclare fatal になる問題を修正 (EccubeBundle の auto_mapping を無効化)#6963nanasess wants to merge 3 commits into
Conversation
Entity の if(!class_exists()) ガード全廃 (EC-CUBE#6895) 後、プラグイン/Customize が コア Entity を trait 拡張して Proxy (app/proxy/entity) が生成されている環境で、 prod 起動時に "Cannot redeclare class Eccube\Entity\Customer" で全リクエストが 500 になっていた。 原因は Doctrine の mapping driver が二重に登録されていること: - Kernel::addEntityExtensionPass が src/Eccube/Entity を TraitProxyAttributeDriver で登録 (Proxy 済みクラスは再 require しない) - doctrine.orm.auto_mapping が EccubeBundle を検出し、同じ src/Eccube/Entity を 素の AttributeDriver でも登録する。こちらは ColocatedMappingDriver::getAllClassNames() で Entity ソースを無条件に require_once するため、Kernel::loadEntityProxies が 先に Proxy を読み込んだ状態で二重宣言になる (旧: 各 Entity の class_exists ガードが吸収) auto_mapping 全体を切るのではなく EccubeBundle の自動マッピングのみ無効化する (他バンドルの auto_mapping は維持)。 検証 (ローカル・SQLite・APP_ENV=prod): - 修正前: samplepayment44 を enable し Proxy が生成された状態で cache:clear / ビルトインサーバへのリクエストが redeclare fatal (CI の失敗と同一スタック) - 修正後: cache:clear / warmup 成功、doctrine:mapping:info は修正前と同じ 82 entities (league/oauth2-server-bundle の 5 entity も維持)、e2e/agent の ACP discovery (7 assertions) と UCP checkout session (10 assertions) が PASS Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
src/Eccube/Entity を担当するドライバが TraitProxyAttributeDriver 以外に存在しないことを 検証する。素の AttributeDriver が同じパスを担当すると、Entity ソースを無条件に require_once するため Proxy ロード済み環境で redeclare fatal になる。 Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
📝 WalkthroughWalkthroughDoctrine の EccubeBundle 自動マッピングを無効化し、コア Entity が TraitProxyAttributeDriver のみでマッピングされることを検証する回帰テストを追加します。 ChangesDoctrine Entity マッピング
Estimated code review effort: 2 (Simple) | ~10 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@tests/Eccube/Tests/Doctrine/ORM/Mapping/EccubeEntityMetadataDriverTest.php`:
- Around line 51-63: Update the driver validation in
EccubeEntityMetadataDriverTest to collect drivers whose getPaths() includes
coreEntityDir, then assert after the loop that at least one matching driver was
found. Preserve the existing TraitProxyAttributeDriver assertion for each match
so the test detects both missing core Entity mappings and incorrect driver
types.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro Plus
Run ID: b47446cc-a876-4684-a4ba-36a9c0dc30d0
📒 Files selected for processing (2)
app/config/eccube/packages/doctrine.yamltests/Eccube/Tests/Doctrine/ORM/Mapping/EccubeEntityMetadataDriverTest.php
対象パスのドライバが 1 件も無い構成 (Kernel::addEntityExtensionPass の明示登録が 失われた場合) でもループが素通りして green になるため、収集した対象ドライバが 空でないことをアサートする。 CodeRabbit のレビュー指摘に対応。 Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## 4.4 #6963 +/- ##
==========================================
+ Coverage 77.14% 77.23% +0.08%
==========================================
Files 546 547 +1
Lines 27133 27163 +30
==========================================
+ Hits 20933 20980 +47
+ Misses 6200 6183 -17
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
|
#6979 で、本 PR の サードパーティ製プラグインが持ち込むバンドル名は事前に知ることができないため、バンドル名を列挙する方式では原理的にこの経路を塞げません。そのため、明示登録済みの Entity ディレクトリをコンパイル時に auto_mapping 側ドライバの paths から取り除く方式へ変更し、#6982 として出し直しました。 #6982 は本 PR が直すケース(コア Entity の redeclare)も解消する上位互換で、 @dotani1111 レビューいただいたのに恐れ入りますが、#6982 で改めてご確認いただけますと幸いです。 レビューいただいた点(テストが 0 件でも素通りしないよう明示登録の存在を検証する)は #6982 のテストにも引き継いでいます。 |
doctrine.orm.auto_mapping は、バンドルクラスが置かれたディレクトリの Entity/ を 検出して素の AttributeDriver に登録する (DoctrineExtension::detectMetadataDriver)。 このとき同じドライバ型のバンドルは 1 つの AttributeDriver インスタンスに集約されるため (DoctrineExtension::registerMappingDrivers)、Kernel::addEntityExtensionPass が TraitProxyAttributeDriver で明示登録しているディレクトリも素のドライバに入り込む。 素のドライバは ColocatedMappingDriver::getAllClassNames() で Entity ソースを 無条件に require_once するため、Kernel::loadEntityProxies() が app/proxy/entity の Proxy を先にロードした状態では "Cannot redeclare class" で fatal になる (Entity の if (!class_exists()) ガード全廃前は、そのガードが吸収していた)。 MappingDriverChain は名前空間ごとに 1 ドライバしか保持しないため、EC-CUBE の 明示登録で上書きされたように見えるが、素のドライバが別の名前空間 (第三者バンドル) でチェーンに残っていると、その getAllClassNames() が自身の全パスを走査して 同じ fatal を引き起こす。 StripAutoMappedEntityPathsPass を追加し、明示登録済みのディレクトリを素の AttributeDriver の paths から取り除く。全パスが取り除かれた場合は paths 空で getAllClassNames() が例外になるため、MappingDriverChain からも外す。 バンドル名を列挙する方式 (doctrine.orm.mappings.<Bundle>: false) では、 サードパーティ製プラグインが持ち込むバンドル名を事前に知ることができず、 app/Plugin/<Code> や app/Customize の直下にバンドルクラスを置いた構成を 塞げないため、パスを動的に取り除く方式とした。 refs #6979, #6963, #6895, #6891, #5844 Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
概要(Overview・Refs Issue)
Entity の
if (!class_exists())ガードを全廃した #6895 以降、コア Entity を trait 拡張するプラグインを導入した環境で、Proxy 生成後に全リクエストが 500 になる問題を修正します。refs #6891, #5844, #6895
原因は
src/Eccube/Entityのマッピングドライバが二重に登録されることです。Kernel::addEntityExtensionPass()がsrc/Eccube/EntityをTraitProxyAttributeDriverで登録する。こちらは refactor: Entity の if(!class_exists()) ガードを全廃 (refs #6891, #5844) #6895 の追随修正(77defa74ca)で「宣言済みの Entity は再require_onceしない」ようになっている。doctrine.orm.auto_mappingはEccubeBundle(bundle path =src/Eccube)を検出し、同じsrc/Eccube/Entityを素のAttributeDriverでも登録する。素のドライバはColocatedMappingDriver::getAllClassNames()で Entity ソースを無条件にrequire_onceする。Kernel::loadEntityProxies()が先にapp/proxy/entityの Proxy を読み込んでいるため、素のドライバが元ソースを require した時点で二重宣言になる。ガード全廃前は各 Entity のif (!class_exists())がこれを吸収していた。実際のスタックトレース(PostgreSQL /
APP_ENV=prod):このとき問題の素の
AttributeDriverが持つ paths は次の通りで、src/Eccube/Entityが第三者バンドルのマッピングと同一ドライバに合流しています。方針(Policy)
auto_mapping全体を無効化するのではなく、EccubeBundleの自動マッピングのみ無効化します(mappings: { EccubeBundle: false })。src/Eccube/Entity)・app/Customize/Entity・各プラグインのEntityは、いずれもKernel::addEntityExtensionPass()がTraitProxyAttributeDriverで明示登録しているため、auto_mapping による重複登録は不要。league/oauth2-server-bundle)の auto_mapping はそのまま維持されるため、プラグインが依存するバンドルの Entity は従来どおりマッピングされる。実装に関する補足(Appendix)
AttributeDriverは doctrine-bundle が auto_mapping 対象バンドルをまとめて 1 つのドライバにするため、EccubeBundle単独ではチェーンに残らず(EC-CUBE 側の明示登録が同一 namespace を上書きするため)、league/oauth2-server-bundleのような別 namespace のバンドルが加わって初めてsrc/Eccube/Entityを含むドライバがチェーンに露出します。src/Eccube/Entityを担当するドライバがTraitProxyAttributeDriver以外に存在しない」という不変条件を固定するものです。上記の理由から、第三者バンドルが無いコアのテスト環境では修正前でも green になります(fatal そのものの再現はプラグイン導入環境が前提)。不変条件を将来にわたって守るためのガードとして追加しています。テスト(Test)
ローカル(SQLite /
APP_ENV=prod/ec-cube/api44(→league/oauth2-server-bundle)とec-cube/samplepayment44(コアCustomer・Orderを trait 拡張)を実際に導入)で確認しました。SamplePayment44を enable してapp/proxy/entity/src/Eccube/Entity/Customer.phpが生成された状態で、bin/console cache:clearおよびビルトインサーバへの全リクエストが上記 redeclare fatal で失敗(Compile Error: Cannot declare class Eccube\Entity\Customer ...)。cache:clear/cache:warmupが成功し、リクエストも 200 を返す。bin/console doctrine:mapping:infoは修正前後とも 82 entities(league/oauth2-server-bundleの 5 entity を含む)で、マッピング対象に増減がないことを確認。vendor/bin/phpunit tests/Eccube/Tests/Doctrine/ORM/Mapping/= OK(追加テスト+既存TraitProxyAttributeDriverTest)。vendor/bin/phpstan analyse src= No errors、php-cs-fixer --dry-run= 0 件、rector --dry-run= 差分なし(指摘は適用済み)。相談(Discussion)
TraitProxyAttributeDriver側で吸収する(素のドライバの登録自体は許容する)案もありますが、素のドライバは doctrine-bundle が生成するためコア側で挙動を制御できず、auto_mappingから外すのが確実と判断しました。マイナーバージョン互換性保持のための制限事項チェックリスト
レビュワー確認項目
🤖 Generated with Claude Code
Summary by CodeRabbit
バグ修正
テスト