-
Notifications
You must be signed in to change notification settings - Fork 693
[POC]symfony7対応 #6458
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: 4.3-symfony7
Are you sure you want to change the base?
[POC]symfony7対応 #6458
Conversation
32ec612
to
b073ff9
Compare
(メモ) 試したこと;
手順を再現した: |
一旦、バージョンアップを優先します。 残タスク
|
Codecov Report❌ Patch coverage is Additional details and impacted files@@ Coverage Diff @@
## 4.3-symfony7 #6458 +/- ##
================================================
- Coverage 82.17% 82.15% -0.02%
================================================
Files 481 481
Lines 27010 27071 +61
================================================
+ Hits 22196 22241 +45
- Misses 4814 4830 +16
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull Request Overview
This pull request migrates EC-CUBE from Doctrine annotations to PHP attributes, upgrading to Symfony 7 compatibility. The main changes include updating annotations across entities, value objects, and test entities, along with adjusting Doctrine drivers and dependencies.
Key changes:
- Migration from Doctrine annotations (
@ORM\*
) to PHP attributes (#[ORM\*]
) across all entities and related classes - Update of custom Doctrine drivers from AnnotationDriver to AttributeDriver variants
- Dependency updates including rector and doctrine/dbal versions
Reviewed Changes
Copilot reviewed 104 out of 106 changed files in this pull request and generated 5 comments.
Show a summary per file
File | Description |
---|---|
Entity classes | Converted ORM annotations to PHP attributes |
Doctrine drivers | Renamed and updated from annotation-based to attribute-based drivers |
Kernel.php | Updated entity extension configuration to use attribute drivers |
Tests | Migrated test entity annotations to attributes |
Dependencies | Updated rector, doctrine/dbal, and other related packages |
Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.
protected $type; | ||
|
||
public function __construct($OrderItems, $type = null) | ||
public function __construct($OrderItems = [], $type = null) |
Copilot
AI
Sep 24, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Adding a default value of empty array to the first parameter changes the method signature. This could be a breaking change if code elsewhere relies on the original signature where the parameter was required.
public function __construct($OrderItems = [], $type = null) | |
public function __construct($OrderItems, $type = null) |
Copilot uses AI. Check for mistakes.
protected $type; | ||
|
||
public function __construct($Items, $type = null) | ||
public function __construct($Items = [], $type = null) |
Copilot
AI
Sep 24, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Adding a default value of empty array to the first parameter changes the method signature. This could be a breaking change if code elsewhere relies on the original signature where the parameter was required.
public function __construct($Items = [], $type = null) | |
public function __construct($Items, $type = null) |
Copilot uses AI. Check for mistakes.
/** | ||
* Blocknn | ||
* Block |
Copilot
AI
Sep 24, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There's a typo fix from 'Blocknn' to 'Block' in the comment, which is good, but ensure this is intentional and not part of a larger pattern that needs addressing.
Copilot uses AI. Check for mistakes.
* @var string|null | ||
*/ | ||
#[ORM\Column(name: 'test_update_schema_command', type: 'text', nullable: true)] |
Copilot
AI
Sep 24, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The column type changed from 'string' to 'text' which could affect database schema and data handling. Ensure this change is intentional and properly tested.
Copilot uses AI. Check for mistakes.
#[ORM\HasLifecycleCallbacks] | ||
#[ORM\Entity(repositoryClass: AuthorityRepository::class)] | ||
#[ORM\Cache(usage: 'NONSTRICT_READ_WRITE')] | ||
#[ORM\InheritanceType('SINGLE_TABLE')] | ||
#[ORM\DiscriminatorColumn(name: 'discriminator_type', type: 'string', length: 255)] |
Copilot
AI
Sep 24, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
[nitpick] The order of ORM attributes has been changed from the original annotation order. While this doesn't affect functionality, it's inconsistent with the pattern used in other entity files where InheritanceType and DiscriminatorColumn appear before HasLifecycleCallbacks and Cache attributes.
#[ORM\HasLifecycleCallbacks] | |
#[ORM\Entity(repositoryClass: AuthorityRepository::class)] | |
#[ORM\Cache(usage: 'NONSTRICT_READ_WRITE')] | |
#[ORM\InheritanceType('SINGLE_TABLE')] | |
#[ORM\DiscriminatorColumn(name: 'discriminator_type', type: 'string', length: 255)] | |
#[ORM\InheritanceType('SINGLE_TABLE')] | |
#[ORM\DiscriminatorColumn(name: 'discriminator_type', type: 'string', length: 255)] | |
#[ORM\HasLifecycleCallbacks] | |
#[ORM\Entity(repositoryClass: AuthorityRepository::class)] | |
#[ORM\Cache(usage: 'NONSTRICT_READ_WRITE')] |
Copilot uses AI. Check for mistakes.
1cafbc1
to
88de017
Compare
メモ
ただ、元は namespaceごとにDriverをセットしていたため、AttributeDriverで同様のことができないか調べる必要がある。 |
UpdateSchemaDoctrineCommandTest系が落ちています。 gen -> /var/folders/gk/ck15prcj11l7dgf8m5tnrfrw0000gq/T/proxy_6oyTaSUNVyoP/src/Eccube/Entity/Customer.php --no-proxy is do not use proxy |
プラグインのインストールをしたところ、テーブルが生成されていませんでした。 余談ですが、 こちらは修正されました。 |
ログインがhttpsじゃないと出来ないため、テストは一旦スキップします。 |
- LogType: ログディレクトリが存在しない場合に自動作成する機能を追加 - Monolog StreamHandlerと同様にmkdir($dirs, 0777, true)を使用 - テスト環境でのLogTypeTest、LogControllerTestのエラーを解消 - EmailValidatorTest: VALIDATION_MODE_LOOSEをVALIDATION_MODE_HTML5に変更 - Symfony 6.2以降でVALIDATION_MODE_LOOSEが非推奨となったため - EmailValidator実装は既存のNoRFCEmailValidatorを継続使用 修正により9件のPHPUnitエラーが解決: - EmailValidatorTest: 5件 - LogTypeTest: 3件 - LogControllerTest: 6件 (ディレクトリ作成により連動解決) 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <[email protected]>
- RemoveNullTagValueNodeRector getOrderStatusは常にNullを返すが、OrderStatus|nullとする。
## 問題 Symfony 7アップグレード後、TopControllerTestで以下のエラーが発生: 1. Remember Me認証で "A non-empty secret is required." エラー 2. PHPUnit 11で "Test code or tested code did not remove its own exception handlers" 警告 ## 原因 1. テスト環境でframework.secretが未設定 2. SymfonyカーネルがHTTPリクエスト処理時に例外ハンドラーを設定するが、 PHPUnit 11はテスト終了時に例外ハンドラーが残っていることを検知して riskyテストとして警告を出す ## 対応 1. テスト環境用framework.yamlにsecretを追加 2. EccubeTestCase::tearDown()で例外ハンドラーをクリーンアップ - Symfonyが設定した例外ハンドラーを全て削除 - PHPUnitの例外ハンドラー検知を回避 3. phpunit.xml.distから不要な設定を削除 - beStrictAboutChangesToGlobalState="false" - failOnRisky="false" (例外ハンドラーのクリーンアップで警告が解消されたため不要) 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <[email protected]> (cherry picked from commit a398c1d)
- LogType: ログディレクトリが存在しない場合に自動作成する機能を追加 - Monolog StreamHandlerと同様にmkdir($dirs, 0777, true)を使用 - テスト環境でのLogTypeTest、LogControllerTestのエラーを解消 - EmailValidatorTest: VALIDATION_MODE_LOOSEをVALIDATION_MODE_HTML5に変更 - Symfony 6.2以降でVALIDATION_MODE_LOOSEが非推奨となったため - EmailValidator実装は既存のNoRFCEmailValidatorを継続使用 修正により9件のPHPUnitエラーが解決: - EmailValidatorTest: 5件 - LogTypeTest: 3件 - LogControllerTest: 6件 (ディレクトリ作成により連動解決) 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <[email protected]> (cherry picked from commit 46cc833)
## 問題の背景 ### 環境変数取得の問題 - phpunit.xml.dist で `<server name="APP_ENV" value="test" force="true" />` を設定 - PHPUnitの`<server>`タグは`$_SERVER`スーパーグローバルに値を設定する - しかし、既存のenv()関数は`getenv()`のみを使用していた - Symfony Dotenvはスレッドセーフのため、デフォルトで`putenv()`を使用しない - 環境変数は`$_ENV`と`$_SERVER`にのみ設定される - `getenv()`では取得できない状態になっていた - 結果として、テスト実行時に`APP_ENV=test`が認識されず、dev環境として動作 - IgnoreTwigSandboxErrorExtensionTestで10件のテストが失敗 ## 修正内容 ### 1. env() 関数の修正 (src/Eccube/Resource/functions/env.php) - `$_ENV`と`$_SERVER`を優先的に確認するように変更 - `getenv()`はフォールバックとして保持(古い環境との互換性) - これによりPHPUnitの`<server>`タグ設定が正しく認識されるようになった ### 2. IgnoreTwigSandboxErrorExtension の改善 - deprecated な `\twig_include()` から `CoreExtension::include()` に移行 - `SecurityError`を直接catchすることでコードを簡素化 - 不要なuse文を削除し、`CoreExtension`を追加 ## テスト結果 - 修正前: 44テスト中34成功、10失敗(Symfony 7移行による問題) - 修正後: 44テスト中43成功、1失敗 - 残り1件の失敗はSymfony 6でも存在する既存の問題(今回の移行とは無関係) 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <[email protected]>
Symfony 7 / Doctrine ORM 3.x では、entityManager->lock() の実行に 明示的なトランザクションが必須となったため、ShoppingController::checkout() メソッドにトランザクション管理を追加。 変更内容: - executeApply() 実行前に明示的にトランザクション開始 - 成功時 (executeApply/executeCheckout/正常終了) にコミット - 例外発生時にロールバック処理を追加 - すべてのトランザクション操作前に isTransactionActive() でチェック 根本原因: - dama/doctrine-test-bundle 8.0 がテスト環境でトランザクション管理を担当 - TransactionListener はテスト環境で無効化される (isEnabled = false) - Doctrine ORM 3.x では PESSIMISTIC_WRITE ロックにトランザクションが必須 (StockReduceProcessor::prepare() で使用) この修正により、本番環境とテスト環境の両方で正常に動作するようになった。 関連テスト: - ShoppingControllerWithMultipleTest::testAddMultiShippingThreeItemsOfOneProduct - ShoppingControllerWithMultipleNonmemberTest::testAddMultiShippingThreeItemsOfOneProduct 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <[email protected]>
CsvLoaderCommandTest から重複する setUpBeforeClass() を削除。 既に setUp() で markTestIncomplete() を呼んでいるため、 setUpBeforeClass() での呼び出しは不要。 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <[email protected]>
- ディレクトリ存在チェックを明示的に追加 - @mkdir を適切なエラーハンドリング付き mkdir に変更 - ファイル/ディレクトリ作成後に clearstatcache を追加してファイルシステムキャッシュをクリア これによりテストの信頼性が向上し、ファイルシステムキャッシュによる テスト失敗を防止できます。 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <[email protected]>
PHPUnit 11の警告とエラーを解消するため以下を修正: - EventArgsTest.php: クラス名のタイポを修正(EventArgsrTest → EventArgsTest) - PaymentChargeProcessorTest.php: ファイル名とクラス名の不一致を修正 - phpunit.xml.dist: カバレッジ設定をPHPUnit 11対応(coverage → source) - CustomizeBundleTest.php: 例外ハンドラー復元処理を追加してrisky警告を解消 - GitHub Actions: --exclude-group のカンマ区切りを個別オプションに変更(PHPUnit 12非推奨対応) これにより exit status 1 が解消され、テストが正常に完了するようになります。 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <[email protected]>
AbstractCommandTest を CommandTestCase にリネームして、PHPUnit 11の警告を解消しました。PHPUnit 11では抽象テストクラスは *TestCase という命名規則に従う必要があります。 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <[email protected]>
dama/doctrine-test-bundle 8.0 との組み合わせで、MySQLでのみ testSortWithWrapQueriesTrue が失敗する問題を修正しました。 変更内容: - CREATE TABLE → CREATE TEMPORARY TABLE に変更 - dropTable メソッドを DB プラットフォームごとに分岐 - MySQL: DROP TEMPORARY TABLE(暗黙的コミット回避) - SQLite/PostgreSQL: DROP TABLE(SQLiteは DROP TEMPORARY TABLE 非対応) - setUp の不要なトランザクション管理処理を削除 - tearDown で DB プラットフォームを判定して適切な DROP 文を実行 根本原因: - MySQL では DDL 操作(CREATE TABLE, DROP TABLE)が暗黙的コミットを引き起こす - dama/doctrine-test-bundle 8.0 が使用するセーブポイントが失われていた - TEMPORARY テーブルの CREATE/DROP は暗黙的コミットを引き起こさない データベース別の対応: - MySQL: DROP TEMPORARY TABLE で暗黙的コミットを回避 - SQLite: DROP TABLE を使用(TEMPORARY 句は構文エラー) - PostgreSQL: DROP TABLE を使用(どちらでも動作) 効果: - MySQL, PostgreSQL, SQLite 全てで正常に動作 - コードがシンプルで保守しやすくなった - dama/doctrine-test-bundle のトランザクション管理と競合しない 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <[email protected]>
- Application::addCommand() を Application::add() に変更 Symfony\Bundle\FrameworkBundle\Console\Application では add() メソッドを使用 - tearDown() で restore_exception_handler() を追加して risky test 警告を解消 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <[email protected]>
GitHub Actionsのunit-testワークフローを修正し、テストの実行条件を適切に分割: - cache-clear/cache-clear-installテストは全DBプラットフォーム(MySQL, PostgreSQL, SQLite3)で実行 - update-schema-doctrine関連テストはPostgreSQLのみで実行(if: matrix.db == 'pgsql') これにより、PostgreSQL以外のプラットフォームでのテストスキップ時に発生していた PHPUnit警告(exit code 1)によるCI失敗を解消。 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <[email protected]>
- DoctrineのNOT NULL制約違反エラー解決 - Symfony Validator 7.3の非推奨警告 - Doctrine ORM 3.0の非推奨警告 - Twig 3.10の非推奨警告 - Fakerの非推奨警告 - PHPUnit設定の非推奨警告
- DoctrineのNOT NULL制約違反エラー解決 - Symfony Validator 7.3の非推奨警告 - Doctrine ORM 3.0の非推奨警告 - Twig 3.10の非推奨警告 - Fakerの非推奨警告 - PHPUnit設定の非推奨警告
phpunit.xml.distの除外設定を削除し、代わりにテスト内でmarkTestIncomplete()を使用: - phpunit.xml.distから<exclude>設定を削除(PluginManagerTest、PluginServiceTest、GenerateDummyDataCommandTest) - PluginManagerTestとPluginServiceTestのsetUp()にmarkTestIncomplete()を追加 - GitHub Actionsのplugin-serviceテスト実行をコメントアウトし、警告メッセージに変更 これにより、GitHub Actionsで"No tests executed!"エラーが発生せず、 Symfony 7.4アップグレード後に対応予定である旨を明示的に通知。 関連リビジョン: e762747 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <[email protected]>
Symfony 7.4 アップグレードに伴い、framework.secret に必要な ECCUBE_AUTH_MAGIC 環境変数が不足していた問題を修正。 変更内容: - 各ジョブで openssl rand -hex 32 を使用してランダムな64文字の 16進数文字列を生成 - doctrine:schema:create 実行時の SignatureHasher エラーを解消 - 以下のワークフローファイルを修正: - unit-test.yml (1箇所) - plugin-test.yml (8箇所) - e2e-test-throttling.yml (2箇所) - coverage.yml (2箇所) - e2e-test.yml (3箇所) 関連: job ID 52884246343 のエラー解析結果
PHPUnit 11.x への対応として codeception/module-webdriver を 3.2.0 から 4.0.3 にアップグレード。 変更内容: - codeception/module-webdriver: 3.2.0 → 4.0.3 - Constraint::fail() メソッドの戻り値型が void から never に変更され、PHPUnit 11 の要件に適合 - PHP 8.3 での致命的エラー (method signature incompatibility) を解決 影響: - Codeception E2Eテスト (WebDriver) が PHPUnit 11.5.42 で正常に動作 - Symfony 7.4 アップグレードブランチでの CI/CD テストが正常実行可能 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <[email protected]>
…題を解決 - PHPビルトインサーバー(HTTP)ではcookie_secure=falseが必要 - SameSite=Noneの場合はSecureフラグが必須のため、HTTP環境ではcookie_samesite=laxに変更 - この設定はAPP_ENV=codeception環境でのみ適用され、本番環境には影響しない 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <[email protected]>
- RenameAttributeRectorのルールがRector 2.2.3では存在しないため除外
- NestedAnnotationToAttributeRector - 既に移行済み - RemoveReflectionSetAccessibleCallsRector - 8.1以降ならデフォルトでtrue
- @paramの型を修正 - RecastingRemovalRector - NullToStrictStringFuncCallArgRector - RemoveReflectionSetAccessibleCallsRector
概要(Overview・Refs Issue)
主な変更
既知の懸念/TODO
残タスク
方針(Policy)
実装に関する補足(Appendix)
テスト(Test)
相談(Discussion)
マイナーバージョン互換性保持のための制限事項チェックリスト
レビュワー確認項目