fix: プラグイン/Customize 直下のバンドルでも Entity の redeclare fatal を防ぐ (auto_mapping の二重登録をコンパイル時に除去) - #6982
Conversation
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 EC-CUBE#6979, EC-CUBE#6963, EC-CUBE#6895, EC-CUBE#6891, EC-CUBE#5844 Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
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 の auto-mapping 用 AttributeDriver から明示登録済み Entity パスを除外するコンパイラパスを追加し、Kernel に登録した。パス除外、ドライバ構成、Kernel 起動後のメタデータ解決を検証する回帰テストも追加した。 ChangesEntity パスマッピング制御
Estimated code review effort: 3 (Moderate) | ~25 minutes Possibly related issues
Possibly related PRs
Suggested labels: Suggested reviewers: Sequence Diagram(s)sequenceDiagram
participant Kernel
participant ContainerBuilder
participant StripAutoMappedEntityPathsPass
participant MappingDriverChain
Kernel->>ContainerBuilder: 明示 Entity パスとコンパイラパスを登録
ContainerBuilder->>StripAutoMappedEntityPathsPass: process()
StripAutoMappedEntityPathsPass->>MappingDriverChain: auto-mapping ドライバのパスを更新
StripAutoMappedEntityPathsPass->>MappingDriverChain: 空のドライバ参照を除去
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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
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 `@src/Eccube/DependencyInjection/Compiler/StripAutoMappedEntityPathsPass.php`:
- Around line 86-89: 型宣言のない無名関数を更新してください。array_filter のコールバックである無名関数の $path
引数に適切な型を付け、戻り値を bool と明示してください。また、同じファイルの 135-145
行付近にある無名関数にも、各引数と戻り値の型宣言を追加してください。
🪄 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: 4f0295be-e311-48d2-b503-61f1148e1e5c
📒 Files selected for processing (4)
src/Eccube/DependencyInjection/Compiler/StripAutoMappedEntityPathsPass.phpsrc/Eccube/Kernel.phptests/Eccube/Tests/DependencyInjection/Compiler/StripAutoMappedEntityPathsPassTest.phptests/Eccube/Tests/Doctrine/ORM/Mapping/EccubeEntityMetadataDriverTest.php
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## 4.4 #6982 +/- ##
==========================================
- Coverage 77.25% 77.25% -0.01%
==========================================
Files 547 548 +1
Lines 27163 27225 +62
==========================================
+ Hits 20985 21032 +47
- Misses 6178 6193 +15
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 の報告者です。迅速な対応ありがとうございます。方式の変更まで含めて対応いただき助かりました。 手元の再現環境（#6979 で使ったもの）に本 PR（ 検証結果PHP 8.2.27 / SQLite / doctrine-bundle 2.18.2 / doctrine/orm 3.6.2
追加された 7 について一番壊れやすいと思ったので、同一プラグイン内に除去対象と非対象を同居させて試しました。 共有された 1 つの 8 について明示登録側は 気になった点いずれも本 PR で対応が必要とは考えていません。記録として残しておきます。 1. 第三者バンドルが自前の
|
issue EC-CUBE#6979 の再現構成を実際に配置して Kernel を boot し、メタデータ解決まで 到達することを検証する。再現に必要な 3 要素 (対照実験 A/B で確定済み) を fixture で組む: 1. Entity を持つ第三者バンドル (prefix が明示登録の対象外なので素の AttributeDriver が MappingDriverChain に残る) 2. app/Customize 直下に置かれた Bundle (auto_mapping が app/Customize/Entity を 素のドライバの paths に入れる) 3. 同一 FQCN の Proxy (Kernel::loadEntityProxies が先にロードする) 既存の StripAutoMappedEntityPathsPassTest は ContainerBuilder を組み立てるコンパイル時の 単体テスト、EccubeEntityMetadataDriverTest は素の構成でのドライバ種別の検証であり、 いずれも再現構成そのものは組み立てていなかった。 StripAutoMappedEntityPathsPass を一時的に無効化して "Cannot redeclare class Customize\Entity\StripAutoMappedTarget" の fatal を再現し、 パスが有効な状態では green になることを両方向で確認済み。 Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (1)
tests/Eccube/Tests/Doctrine/ORM/Mapping/AutoMappedEntityPathsBootTest.php (1)
106-108: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winメタデータ変換コールバックに型宣言を追加してください。
$metadata引数と戻り値が無型です。Doctrine ORMの実際の要素型を確認したうえで、引数型とstring戻り値を宣言してください。- static fn ($metadata) => $metadata->getName(), + static fn (ClassMetadata $metadata): string => $metadata->getName(),As per coding guidelines: PHPの引数と戻り値には型宣言を付けます。
🤖 Prompt for 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. In `@tests/Eccube/Tests/Doctrine/ORM/Mapping/AutoMappedEntityPathsBootTest.php` around lines 106 - 108, メタデータ変換コールバックにDoctrine ORMの実際のメタデータ要素型を引数型として指定し、戻り値をstringに宣言してください。対象はgetAllMetadata()をmapするstaticクロージャで、既存のgetName()による変換結果と処理フローは維持してください。Source: Coding guidelines
🤖 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/AutoMappedEntityPathsBootTest.php`:
- Around line 67-89:
テスト用ファイルが実プロジェクトを上書き・削除しないよう、setUpのmirror()/copy()とtearDown()を隔離された一時プロジェクトまたはサブプロセス上で実行する構成に変更してください。既存のapp/Customize、app/proxy/entity配下を使う場合は、CustomizeRootBundle.php、StripAutoMappedTarget.php、Lib、bundles.php、proxyFile()の既存内容を事前に退避し、正常終了時だけでなくfatal終了後も親プロセスで復元できるようにしてください。
In `@tests/Fixtures/CustomizeRootBundle/CustomizeRootBundle.php`:
- Around line 1-3: tests/Fixtures/CustomizeRootBundle/CustomizeRootBundle.php
(1-3), tests/Fixtures/CustomizeRootBundle/Entity/StripAutoMappedTarget.php
(1-3), tests/Fixtures/CustomizeRootBundle/Lib/CustomizeLibBundle.php (1-3),
tests/Fixtures/CustomizeRootBundle/Lib/Entity/StripAutoMappedExtra.php (1-3),
and tests/Fixtures/CustomizeRootBundle/Resource/config/bundles.php (1-3) should
each place declare(strict_types=1); immediately after the PHP opening tag and
before the license comment, preserving the existing file contents otherwise.
---
Nitpick comments:
In `@tests/Eccube/Tests/Doctrine/ORM/Mapping/AutoMappedEntityPathsBootTest.php`:
- Around line 106-108: メタデータ変換コールバックにDoctrine
ORMの実際のメタデータ要素型を引数型として指定し、戻り値をstringに宣言してください。対象はgetAllMetadata()をmapするstaticクロージャで、既存のgetName()による変換結果と処理フローは維持してください。
🪄 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: 38fecdad-bdf0-4090-a829-ce5b12fd0983
📒 Files selected for processing (6)
tests/Eccube/Tests/Doctrine/ORM/Mapping/AutoMappedEntityPathsBootTest.phptests/Fixtures/CustomizeRootBundle/CustomizeRootBundle.phptests/Fixtures/CustomizeRootBundle/Entity/StripAutoMappedTarget.phptests/Fixtures/CustomizeRootBundle/Lib/CustomizeLibBundle.phptests/Fixtures/CustomizeRootBundle/Lib/Entity/StripAutoMappedExtra.phptests/Fixtures/CustomizeRootBundle/Resource/config/bundles.php
- AutoMappedEntityPathsBootTest: 文字列 FQCN を ::class に変更 (StringClassNameToClassConstantRector)。fixture は app/Customize へ配置して初めて 実体を持つが、::class は静的解決のためロードは発生しない - fixture の Entity 2 本: #[ORM\Column(type: 'integer')] を Types::INTEGER に変更 (AttributeKeyToClassConstFetchRector) Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
CodeRabbit の指摘 2 件に対応する。
1. テスト用ファイルを実プロジェクトから隔離 (Major)
- 検証を bin/console doctrine:mapping:info のサブプロセス実行に変更した。
回帰時の redeclare は PHP の fatal のため、同一プロセスで起動すると tearDown が
実行されず fixture が残っていた。子プロセスに閉じ込めることで、終了コードによる
通常のアサーション失敗として報告され、後始末も確実に実行される
- 実プロジェクトに同名のファイル・ディレクトリがある場合に備え、setUp で退避し
tearDown で復元する
- Proxy の配置で作成したディレクトリは、空になった場合のみ取り除く
(実運用で生成された Proxy を巻き込まないため)
2. fixture 5 ファイルに declare(strict_types=1) を追加 (Minor)
Pass を一時的に無効化した状態で、子プロセスの fatal が
"Failed asserting that 255 is identical to 0." として報告され、
fixture の後始末も走ることを確認済み。
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
前コミットで導入したサブプロセス検証が、コンテナを再生成させるために var/cache/test を削除していた。PHPUnit は 1 プロセスで全テストを実行するため、 既にコンテナを起動済みの他テストが遅延ロードするサービス定義ファイル (var/cache/test/Container*/get*.php) まで消え、unit-test が 865 errors で失敗していた。 サブプロセスを専用の APP_ENV (test_auto_mapped) で実行し、 var/cache/test_auto_mapped だけを作成・削除するよう変更する。 Kernel::configureContainer は packages/<env> を is_dir で判定するため、 専用環境名でも共通設定だけで起動できる (doctrine:mapping:info で動作確認済み)。 Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
4 participants
概要(Overview・Refs Issue)
Entity の
if (!class_exists())ガードを全廃した #6895 以降、Proxy 生成後に全リクエスト・全コンソールコマンドが redeclare fatal で失敗する問題を修正します。fixes #6979 / #6963 を置き換えます（#6963 の方式では塞げない経路が #6979 で報告されたため、方式を変更して出し直したものです。#6963 はクローズします）
refs #6979, #6963, #6895, #6891, #5844
原因
doctrine.orm.auto_mappingが、EC-CUBE が明示登録しているのと同じ Entity ディレクトリを素のAttributeDriverにも登録することが原因です。Kernel::addEntityExtensionPass()はsrc/Eccube/Entity/app/Customize/Entity/app/Plugin/<Code>/EntityをTraitProxyAttributeDriverで登録する。こちらは refactor: Entity の if(!class_exists()) ガードを全廃 (refs #6891, #5844) #6895 の追随修正（77defa74ca）で「宣言済みの Entity は再require_onceしない」ようになっている。/Entityを auto_mapping の対象として検出する（DoctrineExtension::detectMetadataDriver()/getMappingDriverBundleConfigDefaults()）。registerMappingDrivers()は同じドライバ型のバンドルを 1 つの Definition に集約する。ColocatedMappingDriver::getAllClassNames()で Entity ソースを無条件にrequire_onceする。Kernel::loadEntityProxies()が先にapp/proxy/entityの Proxy を読み込んでいるため、二重宣言になる。ガード全廃前は各 Entity のif (!class_exists())がこれを吸収していた。MappingDriverChainは名前空間ごとに 1 ドライバしか保持しないため、EC-CUBE の明示登録で上書きされたように見えます。しかしgetAllClassNames()はドライバインスタンス単位で走査するため、素のドライバが別の名前空間（第三者バンドル）でチェーンに残っていると、そのタイミングで自身の全パスをrequire_onceして同じ fatal になります。実際のスタックトレース（PostgreSQL /
APP_ENV=prod）:方針(Policy)
Kernel::addEntityExtensionPass()が明示登録した Entity ディレクトリを、コンパイル時に auto_mapping 側のドライバのpathsから取り除きます（StripAutoMappedEntityPathsPass）。doctrine.orm.<em>_attribute_metadata_driver（および.inner）のみ。第三者バンドルのパスはそのまま残るため、プラグインが依存するバンドル（league/oauth2-server-bundle等）の Entity は従来どおりマッピングされます。addEntityExtensionPass()が登録したものをそのまま pass に渡すため、明示登録とパス一覧が二重管理になりません。pathsが空のままgetAllClassNames()を呼ばれるとMappingException::pathRequiredForDriverになるため、MappingDriverChainのaddDriver()呼び出しからも外します。バンドル名を列挙する方式を採らなかった理由
当初 #6963 では
doctrine.orm.mappings: { EccubeBundle: false }でコア分だけ無効化していましたが、#6979 でapp/Plugin/<Code>/やapp/Customize/の直下にバンドルクラスを置いた構成では同じ fatal が残ることが報告されました（@kurozumi さんによる詳細な調査・再現手順に感謝します）。サードパーティ製プラグインが持ち込むバンドル名は事前に知ることができないため、列挙方式ではこの経路を塞げません。バンドル名に依存せず、EC-CUBE が明示登録しているディレクトリという確実な情報でパスを取り除く方式に変更しました。
なお #6979 で言及されているとおり、
TraitProxyAttributeDriver側で吸収する案は成立しません。実際に走査してrequire_onceしているのは doctrine-bundle が生成する素のAttributeDriverであり、受け側では届かないためです。実装に関する補足(Appendix)
app/config/eccube/packages/doctrine.yamlへの変更は不要になりました。EccubeBundle: falseでは塞がらない Entity 二重登録の経路がある（app/Plugin/* / app/Customize 直下に Bundle を置いた場合） #6979 の対照実験（検証 A / B）と一致します。eccube-api4はBundle/ApiBundle.php配置のため対象外）。サードパーティ製プラグインがapp/Plugin/Foo/FooBundle.phpという配置を選んだ場合に踏みます。テスト（Test)
ローカル（PHP 8.5 / SQLite /
APP_ENV=prodおよびdev）で、#6979 の再現手順どおりの構成を作って確認しました。再現用の構成:
app/Plugin/ExtraBundle/Lib/ExtraLibBundle.php+Lib/Entity/ExtraThing.php（prefix がPlugin\ExtraBundle\Lib\Entityになり明示登録の対象外＝チェーンに残る）app/Plugin/Foo/FooBundle.php+Entity/Bar.phpapp/Customize/CustomizeBundle.php+Entity/MyThing.phpapp/proxy/entity/配下に同一 FQCN を配置（eccube:generate:proxies相当）Cannot redeclare class Eccube\Entity\Customerapp/Plugin/Foo/FooBundle.php+ ProxyCannot redeclare class Plugin\Foo\Entity\Bar（#6963 適用時）app/Customize/CustomizeBundle.php+ ProxyCannot redeclare class Customize\Entity\MyThing（#6963 適用時）bin/console cache:warmupが prod / dev とも成功、bin/console eccube:generate:proxiesも正常終了。bin/console doctrine:mapping:info:Plugin\ExtraBundle\Lib\Entity\ExtraThing（auto_mapping 経由）・Plugin\Foo\Entity\Bar・Customize\Entity\MyThingがいずれも[OK]tests/Eccube/Tests/DependencyInjection/Compiler/StripAutoMappedEntityPathsPassTest.php（新規・DB 非依存）— doctrine-bundle が生成するコンテナ構造を模し、明示登録済みパスだけが除去されること / 第三者バンドルのパスが残ること / 全除去時にチェーンから外れること / パラメータ表記の解決 / 無関係なドライバ定義を変更しないこと を検証tests/Eccube/Tests/Doctrine/ORM/Mapping/EccubeEntityMetadataDriverTest.php（新規）— コンパイル済みコンテナ上で、明示登録した全ディレクトリ（コア / Customize / 各プラグイン）を担当するドライバがTraitProxyAttributeDriverだけであることを固定する回帰テストphpstan analyse src= 本 PR 由来のエラーなし（既存 15 件から増減なし）、php-cs-fixer --dry-run= 0 件、rector --dry-run= 差分なし（指摘は適用済み）、phpunit tests/Eccube/Tests/Doctrine/ORM/Mapping tests/Eccube/Tests/DependencyInjection/Compiler/StripAutoMappedEntityPathsPassTest.php= OK（7 tests）相談（Discussion）
eccube:plugin:generateのスケルトンや開発ドキュメントで推奨配置を示す価値はあるかもしれません。マイナーバージョン互換性保持のための制限事項チェックリスト
レビュワー確認項目
🤖 Generated with Claude Code
Summary by CodeRabbit