feat: EC-CUBE 4.4への対応#5
Conversation
ubuntuバージョンを22.04へと修正
📝 WalkthroughWalkthroughDatabackup4 を Databackup44 として EC-CUBE 4.4、PHP 8.2以降、Symfony 7.4、Doctrine DBAL 3 に対応させました。Docker開発環境、CI、静的解析、PHPUnit設定、リリース手順、関連ドキュメントも更新しています。 ChangesDatabackup44 移行
Estimated code review effort: 4 (Complex) | ~45 minutes Possibly related issues
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 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
🧹 Nitpick comments (3)
.github/workflows/release.yml (1)
16-19: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win
Testsディレクトリの削除漏れの可能性があります。各種テスト用や開発用の設定ファイルが除外されていますが、もしリリース版のパッケージにテストコードを含めない運用であれば、
Testsディレクトリも除外リストに加えることを推奨します。💡 修正案
# 開発・テスト用ファイルは配布パッケージに含めない rm -rf $GITHUB_WORKSPACE/docker-compose*.yml $GITHUB_WORKSPACE/dockerbuild + rm -rf $GITHUB_WORKSPACE/Tests rm -f $GITHUB_WORKSPACE/CLAUDE.md $GITHUB_WORKSPACE/phpstan.neon.dist rm -f $GITHUB_WORKSPACE/Resource/rector.php "$GITHUB_WORKSPACE/Resource/.php-cs-fixer.dist.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 @.github/workflows/release.yml around lines 16 - 19, Update the release cleanup commands in the workflow to remove the Tests directory from the package, alongside the existing development and test files. Preserve the current exclusions and use the workspace path consistently.Controller/Admin/Databackup4Controller.php (1)
65-65: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low value未使用の
$eventパラメータの削除PHPMD で警告されている通り、クロージャ内で
$eventが使用されていません。PHP では使用しない引数を省略できるため、削除することでコードがすっきりし、静的解析の警告も解消できます。♻️ 提案する変更
- $eventDispatcher->addListener(KernelEvents::TERMINATE, function (TerminateEvent $event) use ($backupBaseDir, $fs) { + $eventDispatcher->addListener(KernelEvents::TERMINATE, function () use ($backupBaseDir, $fs) {🤖 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 `@Controller/Admin/Databackup4Controller.php` at line 65, Remove the unused $event parameter from the KernelEvents::TERMINATE listener closure registered via addListener, while preserving the existing $backupBaseDir and $fs captures and listener behavior.Source: Linters/SAST tools
phpunit.xml.dist (1)
32-35: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winカバレッジの除外対象に
vendorとnode_modulesを追加することを検討してください。
php-cs-fixerやrectorの設定と同様に、万が一プラグイン配下にvendorやnode_modulesディレクトリが存在した場合、それらがカバレッジ計測の対象となってしまい、実行時間の増大やメモリ不足エラーの原因になるのを防ぐことができます。💡 修正案
<exclude> <directory suffix=".php">./Tests</directory> <directory suffix=".php">./Resource</directory> + <directory suffix=".php">./vendor</directory> + <directory suffix=".php">./node_modules</directory> </exclude>🤖 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 `@phpunit.xml.dist` around lines 32 - 35, phpunit.xml.dist のカバレッジ除外設定に vendor と node_modules ディレクトリを追加してください。既存の Tests と Resource の除外は維持し、プラグイン配下にこれらの依存ディレクトリがある場合もカバレッジ対象外になるよう設定します。
🤖 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 @.github/workflows/ci.yaml:
- Around line 42-51: Update the MySQL and PostgreSQL service database
environment values in the CI workflow to use the fixed database name eccube_db
instead of the undefined matrix.dbname reference, matching the database_url
configuration.
---
Nitpick comments:
In @.github/workflows/release.yml:
- Around line 16-19: Update the release cleanup commands in the workflow to
remove the Tests directory from the package, alongside the existing development
and test files. Preserve the current exclusions and use the workspace path
consistently.
In `@Controller/Admin/Databackup4Controller.php`:
- Line 65: Remove the unused $event parameter from the KernelEvents::TERMINATE
listener closure registered via addListener, while preserving the existing
$backupBaseDir and $fs captures and listener behavior.
In `@phpunit.xml.dist`:
- Around line 32-35: phpunit.xml.dist のカバレッジ除外設定に vendor と node_modules
ディレクトリを追加してください。既存の Tests と Resource
の除外は維持し、プラグイン配下にこれらの依存ディレクトリがある場合もカバレッジ対象外になるよう設定します。
🪄 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
Run ID: 55867bd2-737e-4c42-8140-0f9cace9571b
📒 Files selected for processing (19)
.github/workflows/ci.yaml.github/workflows/release.yml.gitignoreCLAUDE.mdController/Admin/Databackup4Controller.phpREADME.mdResource/.php-cs-fixer.dist.phpResource/rector.phpService/Databackup4Service.phpTests/Web/Databackup4AdminControllerTest.phpcomposer.jsondocker-compose.dev.ymldocker-compose.mysql.ymldocker-compose.pgsql.ymldocker-compose.ymldockerbuild/dev-framework.yamldockerbuild/grant_to_dbuser.sqlphpstan.neon.distphpunit.xml.dist
| MYSQL_DATABASE: ${{ matrix.dbname }} | ||
| ports: | ||
| - 3306:3306 | ||
| - 3308:3306 | ||
| options: --health-cmd="mysqladmin ping" --health-interval=10s --health-timeout=5s --health-retries=3 | ||
| postgres: | ||
| image: postgres:14 | ||
| env: | ||
| POSTGRES_USER: postgres | ||
| POSTGRES_PASSWORD: password | ||
| POSTGRES_DB: ${{ matrix.dbname }} |
There was a problem hiding this comment.
🎯 Functional Correctness | 🔴 Critical | ⚡ Quick win
マトリクス変数 dbname が未定義です。
strategy.matrix や include 内で dbname が定義されていないため、matrix.dbname は空として評価されてしまいます。これにより、データベースが正しく作成されずテストが失敗する原因となります。
database_url 内の指定に合わせて、固定値の eccube_db を指定してください。
🐛 修正案
mysql8:
image: mysql:8
env:
MYSQL_ROOT_PASSWORD: password
- MYSQL_DATABASE: ${{ matrix.dbname }}
+ MYSQL_DATABASE: eccube_db
ports:
- 3308:3306
options: --health-cmd="mysqladmin ping" --health-interval=10s --health-timeout=5s --health-retries=3
postgres:
image: postgres:14
env:
POSTGRES_USER: postgres
POSTGRES_PASSWORD: password
- POSTGRES_DB: ${{ matrix.dbname }}
+ POSTGRES_DB: eccube_db📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| MYSQL_DATABASE: ${{ matrix.dbname }} | |
| ports: | |
| - 3306:3306 | |
| - 3308:3306 | |
| options: --health-cmd="mysqladmin ping" --health-interval=10s --health-timeout=5s --health-retries=3 | |
| postgres: | |
| image: postgres:14 | |
| env: | |
| POSTGRES_USER: postgres | |
| POSTGRES_PASSWORD: password | |
| POSTGRES_DB: ${{ matrix.dbname }} | |
| MYSQL_DATABASE: eccube_db | |
| ports: | |
| - 3308:3306 | |
| options: --health-cmd="mysqladmin ping" --health-interval=10s --health-timeout=5s --health-retries=3 | |
| postgres: | |
| image: postgres:14 | |
| env: | |
| POSTGRES_USER: postgres | |
| POSTGRES_PASSWORD: password | |
| POSTGRES_DB: eccube_db |
🧰 Tools
🪛 actionlint (1.7.12)
[error] 42-42: property "dbname" is not defined in object type {database_charset: string; database_server_version: number; database_url: string; db: string; eccube_version: number; php: number; plugin_code: string}
(expression)
[error] 51-51: property "dbname" is not defined in object type {database_charset: string; database_server_version: number; database_url: string; db: string; eccube_version: number; php: number; plugin_code: string}
(expression)
🤖 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 @.github/workflows/ci.yaml around lines 42 - 51, Update the MySQL and
PostgreSQL service database environment values in the CI workflow to use the
fixed database name eccube_db instead of the undefined matrix.dbname reference,
matching the database_url configuration.
Source: Linters/SAST tools
| working-directory: ../ | ||
| run: | | ||
| rm -rf $GITHUB_WORKSPACE/.github | ||
| rm -rf $GITHUB_WORKSPACE/Tests |
There was a problem hiding this comment.
Testsは除外のままでよいと思いますが、何かパッケージにいれる理由がありますでしょうか。
|
@KenTanaka |
5 participants
概要
データバックアッププラグインを EC-CUBE 4.4（Symfony 7.4 / Doctrine DBAL 3.0 / PHP 8.2+） に対応させ、コードを
Databackup42→Databackup44に改名します。4.3 とは非互換（属性必須・DBAL 3・PHP 8.2+）のため、新規4.4ブランチへの取り込みです。参考: 4.3→4.4 マイグレーション手順 (doc #346) / 参照 Recommend-plugin #67
変更内容
1. EC-CUBE 4.4 対応（コア移行）
@Route/@Template→#[Route]/#[Template]（Sensio依存除去）array<int, string>/list<array<string, mixed>>等）private readonly化getSchemaManager()→createSchemaManager()query()→executeQuery()fetchAll()→fetchAllAssociative()\PDO::FETCH_NUM→fetchNumeric()quoteIdentifier()を適用phpunit.xml.distを<source>/<extensions>形式へcode/version: 4.4.0/ ルート名・テンプレート参照をDatabackup44に統一2. Docker Compose によるテスト環境
docker-compose.yml+dev/mysql/pgsqlオーバーレイ、dockerbuild/ghcr.io/ec-cube/ec-cube-php:8.2-apache-4.4）でプラグインを自動導入・有効化し PHPUnit を実行可能APP_ENV=dev起動＋dev 用 Cookie 設定（cookie_secure:false/cookie_samesite:lax）を上書き3. 静的解析・整形ツール
phpstan.neon.dist（level 6）/Resource/rector.php/Resource/.php-cs-fixer.dist.phpを追加（.php設定は本体のPlugin\:サービス検出で 500 を避けるためResource/配下に配置）CLAUDE.mdに開発・テスト手順、アーキテクチャ、移行・配置の注意を記載4. CI（
.github/workflows/ci.yamlほか）checkout@v4・$GITHUB_OUTPUT化（MySQL 5.7 は除外）--ignore-platform-req=ext-redis: Symfony 7.4 のsymfony/cacheが古いphp-redisと衝突して本体 composer install が失敗するのを回避（redis は未使用）cache:warmup: 有効プラグインのルートはコンテナのコンパイル時に確定するため、phpunit プロセスの遅延コンパイルだとRouteNotFoundExceptionが断続的に発生する問題を解消release.yml: 配布パッケージから開発・テスト用ファイル（docker-compose / dockerbuild / CLAUDE.md / phpstan / rector / php-cs-fixer）を除外テスト
release.yml相当）から開発用ファイルが除外されることを確認Summary by CodeRabbit
新機能
改善
テスト・品質
ドキュメント