docs(plan): PDO移行(DBAPI改修)のExecPlanを作成 - #453
Conversation
mysqli直叩きからPDOベースのオブジェクト指向実装へ段階移行するための ExecPlanを新設し、ロードマップのExecPlan欄を反映する。フェーズ1(内部PDO化) のみを対象とし、プレースホルダ移行・マルチDB対応は別フェーズとして分離した。
architecture.mdのmysqli.inc.php参照が実装完了後に不整合となるため、 Concrete StepsとProgressに更新手順を追記した。
|
Warning Review limit reached
Next review available in: 42 minutes Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available. How can I continue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews. How do review limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: ASSERTIVE Plan: Pro Plus Run ID: 📒 Files selected for processing (1)
📝 WalkthroughWalkthroughDBAPIのPDO移行に向けたExecPlanを新規作成し、フェーズ1の設計、実装手順、検証条件、復旧前提を整理した。あわせてロードマップのPDO移行項目を更新した。 ChangesPDO移行計画ドキュメント
Estimated code review effort: 1 (Trivial) | ~5 minutes Related Issues: 提供された情報からは特定できません。 🚥 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.
Pull request overview
PDO移行(DBAPI改修)に向けたフェーズ1(内部PDO化・API互換維持)のExecPlanを新規作成し、ロードマップから参照できるようにする変更。
Changes:
.agent/plans/2026-07-08-pdo-migration-dbapi-refactor.mdを追加し、PDOベース実装への移行方針・段階移行・検証観点を定義.agent/roadmap.mdの「PDO移行(最高優先)」にExecPlanパスを紐付け、フェーズ分割の判断ログを追記
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 5 comments.
| File | Description |
|---|---|
| .agent/roadmap.md | PDO移行タスクにExecPlan参照を追加し、フェーズ分割のメモを追記 |
| .agent/plans/2026-07-08-pdo-migration-dbapi-refactor.md | DBAPIの内部PDO化(フェーズ1)に向けたExecPlanを新規追加 |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: cd404ca43b
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 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 @.agent/plans/2026-07-08-pdo-migration-dbapi-refactor.md:
- Around line 33-35: `DBAPI::escape()` の戻り値契約が `DriverInterface::escape($value):
string` と `PDO::quote()` 前提の実装で崩れているため、公開 API を維持したまま未接続時は false、null は
'NULL'、配列は再帰的に配列を返す既存挙動を残すように見直してください。`DBAPI::escape()`
側で配列処理と特殊値の分岐を保持し、`PdoMysqlDriver::escape()` は単一値の変換だけに限定して、必要なら `DBAPI`
から戻り値型と委譲先の扱いを調整してください。
- Around line 90-93: `PDO::exec()` のエラー判定先が `PDOStatement::errorInfo()`
になっているのを修正してください。`PdoMysqlDriver` の `exec()` 相当処理で `PDO::exec()` の戻り値と
`PDO::errorInfo()[1]` を使って MySQL の errno を判定し、既存の `messageQuit()` 相当の停止条件と
1064/1054/1060/1061/1091 の黙殺ロジックを同じように再現してください。
🪄 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: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: 38eaa74f-4472-4cc8-8b26-1ab628877343
📒 Files selected for processing (2)
.agent/plans/2026-07-08-pdo-migration-dbapi-refactor.md.agent/roadmap.md
- フェーズ1からbindValue()プレースホルダ化を削除(事前エスケープ済み値の二重エスケープを回避) - PDO接続失敗時のPDOException捕捉、dataSeek()の行バッファ方式互換実装を設計に追加 - escape()の契約分離、errno判定はPDO::errorInfo()(接続オブジェクト側)に統一 - CLIコマンド名(health:check/db:backup)の誤りを修正、Concrete Stepsの観測結果欠如・Markdown崩れを修正 - インストーラ(connection.servertest.php/connection.databasetest.php)が$connに直接アクセスしている問題を発見し、既存公開メソッド経由へ書き換えるStepを追加
|
レビュー指摘(インラインスレッド10件、review本文3件: @copilot-pull-request-reviewer review_id:4649866972 / @chatgpt-codex-connector review_id:4649868935 / @coderabbitai review_id:4649874703)はすべて妥当な指摘と判断し、 対応内容の要約:
|
|
Tip For best results, initiate chat on the files or code changes.
(=^・ω・^=)ノ If you found this review helpful, would you consider giving us a shout-out on X? Thank you for using CodeRabbit! |
|
Summary
Testing
|
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 @.agent/plans/2026-07-08-pdo-migration-dbapi-refactor.md:
- Around line 118-123: The dataSeek() compatibility cache in the PDOStatement
wrapper is currently based on FETCH_ASSOC and only caches remaining rows, which
breaks fetchRow(), fetchBoth(), fetchObject(), and absolute seeking. Update the
buffering logic in the
dataSeek()/fetchAssoc()/fetchRow()/fetchObject()/fetchBoth() flow to materialize
the full result set from the start using FETCH_BOTH-like rows so both numeric
and associative keys are preserved and any row index can be sought correctly.
Ensure the internal cursor reads from this full buffer rather than
reconstructing rows on demand.
🪄 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: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: b624e791-1fc6-45ad-948a-5d0a46fead04
📒 Files selected for processing (1)
.agent/plans/2026-07-08-pdo-migration-dbapi-refactor.md
- dataSeek()の行バッファ設計を修正: コンストラクタ時点でFETCH_BOTHにより 結果セット全体をバッファし、絶対位置シークとfetchRow/fetchBoth/fetchObject の数値添字欠落問題を解消 - ファイルパス表記をフルパスに統一(document.parser.subparser.trait.php) - DSN・冪等の専門用語に日本語説明を追加 - Step 6・Step 7に観測結果を追記
|
2回目のレビュー指摘(インラインスレッド6件、review本文2件: @coderabbitai review_id:4650240352 / @copilot-pull-request-reviewer review_id:4650273427)はすべて妥当な指摘と判断し、 対応内容の要約:
|
|
Tip For best results, initiate chat on the files or code changes.
(=^・ω・^=)ノ |
Summary
manager/includes/extenders/dbapi/mysqli.inc.php(DBAPIクラス)をmysqli直叩きからPDOベースのオブジェクト指向実装へ移行する方針を策定.agent/roadmap.mdの対応タスクのExecPlan:欄を反映install/connection.servertest.php/install/connection.databasetest.php)がDBAPIの公開メソッドを迂回して内部の生接続オブジェクト($conn)へ直接アクセスしている問題を発見し、Concrete StepsにStep 3(既存公開メソッドへの書き換え)として追加設計方針(Decision Log抜粋)
Resultラッパークラスで吸収し、PDOStatementを呼び出し側へ直接渡さないescape()は既存のmysqli::escape_string()互換(クォートなし文字列)を維持DriverInterface越しに扱えるよう抽象化するが、実装はPdoMysqlDriverのみ(マルチDB対応は将来フェーズ)DBAPIは公開プロパティ$connを廃止する(内部保持はprivate $driverのみ)。インストーラの直接アクセス箇所は既存公開メソッド(isConnected()/disconnect()/query())経由に書き換えるレビュー反映事項
fe0f68360bindValue()によるプレースホルダ化を除外(既存の事前エスケープ規約との二重エスケープを回避するため。プレースホルダ移行はフェーズ2に集約)ATTR_ERRMODEの設定に関わらず失敗時にPDOExceptionを送出するため、try/catchで捕捉し既存のfalse返却フローへ変換する設計を明記dataSeek()(公開API、外部呼び出し0件だが互換対象)を行バッファ方式で互換実装する設計に変更escape()の契約をDriver(単一値のみ)とDBAPIファサード(null/配列/未接続時の分岐を維持)に分離して明記PDOStatement::errorInfo()ではなく接続オブジェクト側のPDO::errorInfo()を参照する設計に修正health-check→health:check、db-backup→db:backup)を修正fb6b25051grepの\b/\s解釈差で誤判定しうる確認コマンドをrgベースへ修正し、ExecPlan の完了判定手順の再現性を改善Test plan
.agent/PLANS.mdの必須12セクション・非交渉要件を満たしているか確認$conn直接アクセス3箇所、mysqli直接依存4ファイル)に漏れがないか確認grep実装差に依存せず、そのまま再実行できることを確認🤖 Generated with Claude Code
Summary by CodeRabbit