Skip to content

docs(plan): PDO移行(DBAPI改修)のExecPlanを作成 - #453

Open
yama wants to merge 6 commits into
mainfrom
feature/pdo-migration-execplan
Open

docs(plan): PDO移行(DBAPI改修)のExecPlanを作成#453
yama wants to merge 6 commits into
mainfrom
feature/pdo-migration-execplan

Conversation

@yama

@yama yama commented Jul 8, 2026

Copy link
Copy Markdown
Member

Summary

  • ロードマップの「PDO移行(最高優先)」タスクに対応するExecPlanを新規作成
  • manager/includes/extenders/dbapi/mysqli.inc.php(DBAPIクラス)をmysqli直叩きからPDOベースのオブジェクト指向実装へ移行する方針を策定
  • フェーズ1(内部PDO化・API完全互換)のみを対象とし、プレースホルダ移行(フェーズ2)とPostgreSQL/SQLite対応(フェーズ3)は別タスクとして分離
  • .agent/roadmap.md の対応タスクの ExecPlan: 欄を反映
  • レビュー指摘を受け、インストーラ(install/connection.servertest.php/install/connection.databasetest.php)がDBAPIの公開メソッドを迂回して内部の生接続オブジェクト($conn)へ直接アクセスしている問題を発見し、Concrete StepsにStep 3(既存公開メソッドへの書き換え)として追加

設計方針(Decision Log抜粋)

  • 手続き型 mysqli_* ラッパー案は不採用。DBAPI内部をPDOベースで作り直す
  • 結果セットは Result ラッパークラスで吸収し、PDOStatement を呼び出し側へ直接渡さない
  • escape() は既存の mysqli::escape_string() 互換(クォートなし文字列)を維持
  • SQL方言差分は DriverInterface 越しに扱えるよう抽象化するが、実装は PdoMysqlDriver のみ(マルチDB対応は将来フェーズ)
  • 新設する DBAPI は公開プロパティ $conn を廃止する(内部保持は private $driver のみ)。インストーラの直接アクセス箇所は既存公開メソッド(isConnected()/disconnect()/query())経由に書き換える

レビュー反映事項

  • 2026-07-08、Copilot/Codex/CodeRabbit指摘への対応、コミット fe0f68360
    • フェーズ1から bindValue() によるプレースホルダ化を除外(既存の事前エスケープ規約との二重エスケープを回避するため。プレースホルダ移行はフェーズ2に集約)
    • PDO接続確立時は ATTR_ERRMODE の設定に関わらず失敗時に PDOException を送出するため、try/catch で捕捉し既存の false 返却フローへ変換する設計を明記
    • dataSeek()(公開API、外部呼び出し0件だが互換対象)を行バッファ方式で互換実装する設計に変更
    • escape() の契約をDriver(単一値のみ)とDBAPIファサード(null/配列/未接続時の分岐を維持)に分離して明記
    • クエリ失敗時のerrno判定は PDOStatement::errorInfo() ではなく接続オブジェクト側の PDO::errorInfo() を参照する設計に修正
    • CLIコマンド名の誤り(health-checkhealth:checkdb-backupdb:backup)を修正
    • Concrete Steps の観測結果欠如・Markdown引用崩れを修正
  • 2026-07-12、Copilot追加指摘への対応、コミット fb6b25051
    • grep\b / \s 解釈差で誤判定しうる確認コマンドを rg ベースへ修正し、ExecPlan の完了判定手順の再現性を改善

Test plan

  • ExecPlanの内容が .agent/PLANS.md の必須12セクション・非交渉要件を満たしているか確認
  • 影響範囲の棚卸し(呼び出し箇所約1,700件、$conn直接アクセス3箇所、mysqli直接依存4ファイル)に漏れがないか確認
  • Concrete Stepsのファイル構成・移行手順(インストーラ対応Step 3を含む)が実装可能な粒度になっているか確認
  • 確認コマンドが grep 実装差に依存せず、そのまま再実行できることを確認
  • 本PRはドキュメント(ExecPlan)のみで実装は含まないため、コード実行による検証は対象外

🤖 Generated with Claude Code

Summary by CodeRabbit

  • Documentation
    • DBアクセス層のPDO移行に向けた実行計画(フェーズ1)を追加し、互換性方針、切り戻し前提、確認観点(接続・CRUD・画面表示など)を整理しました。
    • ロードマップの「PDO移行(最高優先)」項目を更新し、段階移行(フェーズ1〜3)と今回の対応範囲を明確化しました。

yama added 2 commits July 8, 2026 09:02
mysqli直叩きからPDOベースのオブジェクト指向実装へ段階移行するための
ExecPlanを新設し、ロードマップのExecPlan欄を反映する。フェーズ1(内部PDO化)
のみを対象とし、プレースホルダ移行・マルチDB対応は別フェーズとして分離した。
architecture.mdのmysqli.inc.php参照が実装完了後に不整合となるため、
Concrete StepsとProgressに更新手順を追記した。
Copilot AI review requested due to automatic review settings July 8, 2026 00:06
@coderabbitai

coderabbitai Bot commented Jul 8, 2026

Copy link
Copy Markdown

Review Change Stack

Warning

Review limit reached

@yama, you've reached your PR review limit, so we couldn't start this review.

Next review available in: 42 minutes

Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available.
You're only billed for reviews past your plan's rate limits ($0.25/file).

How can I continue?

After more reviews become available, a review can be triggered using the @coderabbitai review command as a PR comment. Alternatively, push new commits to this PR.

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 configuration

Configuration used: Path: .coderabbit.yaml

Review profile: ASSERTIVE

Plan: Pro Plus

Run ID: e490d8f7-c6cf-4b45-aff9-35176edffd94

📥 Commits

Reviewing files that changed from the base of the PR and between fe0f683 and 4a75813.

📒 Files selected for processing (1)
  • .agent/plans/2026-07-08-pdo-migration-dbapi-refactor.md
📝 Walkthrough

Walkthrough

DBAPIのPDO移行に向けたExecPlanを新規作成し、フェーズ1の設計、実装手順、検証条件、復旧前提を整理した。あわせてロードマップのPDO移行項目を更新した。

Changes

PDO移行計画ドキュメント

Layer / File(s) Summary
計画の目的・意思決定ログ・対象範囲
.agent/plans/2026-07-08-pdo-migration-dbapi-refactor.md
ExecPlanの目的、進捗チェックリスト、意思決定ログ、対象ファイルと用語定義、既存呼び出し規模を整理した。
Driver/Result設計とプレースホルダ方針
.agent/plans/2026-07-08-pdo-migration-dbapi-refactor.md
DriverInterface設計案、connect/execのエラー抑制互換、Resultラッパー仕様、DBAPIファサード方針、フェーズ1でのプレースホルダ移行範囲を定義した。
実装手順と受け入れ条件
.agent/plans/2026-07-08-pdo-migration-dbapi-refactor.md
新規ファイル作成、DBAPI置換、インストーラの直接アクセス解消、読み込み先変更、旧実装削除順序、文言更新、architecture.md更新手順を追加した。
検証・冪等性・成果物・依存関係
.agent/plans/2026-07-08-pdo-migration-dbapi-refactor.md
受け入れ条件、途中中断時の復旧手順、関連ファイル一覧、依存関係と将来のマルチDB拡張方針を記載した。
ロードマップ更新
.agent/roadmap.md
PDO移行項目のExecPlan参照を追加し、3フェーズ分割方針と今回の対象範囲を追記した。

Estimated code review effort: 1 (Trivial) | ~5 minutes

Related Issues: 提供された情報からは特定できません。
Related PRs: 提供された情報からは特定できません。
Suggested labels: documentation, planning
Suggested reviewers: 提供された情報からは特定できません。

🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
Title check ✅ Passed タイトルはDBAPI移行用ExecPlan作成という主変更を簡潔に示しており、内容と整合しています。
Description check ✅ Passed 概要・変更内容・確認手順・備考に相当する情報が揃っており、テンプレート要件を概ね満たしています。
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch feature/pdo-migration-execplan

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.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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を新規追加

Comment thread .agent/plans/2026-07-08-pdo-migration-dbapi-refactor.md Outdated
Comment thread .agent/plans/2026-07-08-pdo-migration-dbapi-refactor.md Outdated
Comment thread .agent/plans/2026-07-08-pdo-migration-dbapi-refactor.md Outdated
Comment thread .agent/plans/2026-07-08-pdo-migration-dbapi-refactor.md Outdated
Comment thread .agent/plans/2026-07-08-pdo-migration-dbapi-refactor.md Outdated

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 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".

Comment thread .agent/plans/2026-07-08-pdo-migration-dbapi-refactor.md Outdated
Comment thread .agent/plans/2026-07-08-pdo-migration-dbapi-refactor.md Outdated
Comment thread .agent/plans/2026-07-08-pdo-migration-dbapi-refactor.md Outdated

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

📥 Commits

Reviewing files that changed from the base of the PR and between 595213c and cd404ca.

📒 Files selected for processing (2)
  • .agent/plans/2026-07-08-pdo-migration-dbapi-refactor.md
  • .agent/roadmap.md

Comment thread .agent/plans/2026-07-08-pdo-migration-dbapi-refactor.md
Comment thread .agent/plans/2026-07-08-pdo-migration-dbapi-refactor.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を追加
@yama

yama commented Jul 8, 2026

Copy link
Copy Markdown
Member Author

レビュー指摘(インラインスレッド10件、review本文3件: @copilot-pull-request-reviewer review_id:4649866972 / @chatgpt-codex-connector review_id:4649868935 / @coderabbitai review_id:4649874703)はすべて妥当な指摘と判断し、fe0f68360 で対応済みです。各インラインスレッドは resolved 化済みです。

対応内容の要約:

  • フェーズ1から bindValue() プレースホルダ化を削除(既存の事前エスケープ規約との二重エスケープを回避)
  • PDO接続失敗時の PDOException 捕捉を設計に追加
  • dataSeek() を行バッファ方式で互換実装する設計に変更
  • escape() の契約をDriver/Facade間で分離明記
  • errno判定は PDO::errorInfo()(接続オブジェクト側)を参照する設計に修正
  • CLIコマンド名の誤り(health-checkhealth:checkdb-backupdb:backup)を修正
  • Concrete Steps の観測結果欠如・Markdown崩れを修正

@coderabbitai

coderabbitai Bot commented Jul 8, 2026

Copy link
Copy Markdown

Tip

For best results, initiate chat on the files or code changes.

@yama ご対応ありがとうございます。修正内容を確認しました。いずれも設計上の重要な変更点(二重エスケープ回避、PDOException捕捉、dataSeek()の互換実装、escape()契約の明確化、errno判定の修正、CLIコマンド名修正)であり、妥当な対応と判断します。commit fe0f68360 の内容で問題ありません。

(=^・ω・^=)ノ


If you found this review helpful, would you consider giving us a shout-out on X?

Thank you for using CodeRabbit!

@chatgpt-codex-connector

Copy link
Copy Markdown

Summary

Testing

  • git diff --check
  • python3 - <<'PY' from pathlib import Path p=Path('.agent/plans/2026-07-08-pdo-migration-dbapi-refactor.md') text=p.read_text() required=['Purpose / Big Picture','Progress','Surprises & Discoveries','Decision Log','Outcomes & Retrospective','Context and Orientation','Plan of Work','Concrete Steps','Validation and Acceptance','Idempotence and Recovery','Artifacts and Notes','Interfaces and Dependencies'] missing=[h for h in required if f'## {h}' not in text] if missing: raise SystemExit('missing sections: '+', '.join(missing)) for step in range(1,8): idx=text.find(f'### Step {step}:') if idx < 0: raise SystemExit(f'missing step {step}') next_idx=text.find(f'### Step {step+1}:', idx+1) if step<7 else text.find('## Validation and Acceptance', idx+1) chunk=text[idx: next_idx if next_idx>=0 else len(text)] if '期待される観測結果' not in chunk: raise SystemExit(f'missing observation for step {step}') print('ExecPlan required sections and Concrete Steps observations: OK') PY
  • rg -n "health-check|db-backup|php evo health-check|php evo db-backup|PDOStatement::errorInfo\\(\\).*失敗|bindValue\\(\\) 化しない|期待される観測結果" .agent/plans/2026-07-08-pdo-migration-dbapi-refactor.md

View task →

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

📥 Commits

Reviewing files that changed from the base of the PR and between cd404ca and fe0f683.

📒 Files selected for processing (1)
  • .agent/plans/2026-07-08-pdo-migration-dbapi-refactor.md

Comment thread .agent/plans/2026-07-08-pdo-migration-dbapi-refactor.md Outdated

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 2 out of 2 changed files in this pull request and generated 5 comments.

Comment thread .agent/plans/2026-07-08-pdo-migration-dbapi-refactor.md Outdated
Comment thread .agent/plans/2026-07-08-pdo-migration-dbapi-refactor.md Outdated
Comment thread .agent/plans/2026-07-08-pdo-migration-dbapi-refactor.md Outdated
Comment thread .agent/plans/2026-07-08-pdo-migration-dbapi-refactor.md
Comment thread .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に観測結果を追記
@yama

yama commented Jul 8, 2026

Copy link
Copy Markdown
Member Author

2回目のレビュー指摘(インラインスレッド6件、review本文2件: @coderabbitai review_id:4650240352 / @copilot-pull-request-reviewer review_id:4650273427)はすべて妥当な指摘と判断し、18e48647a で対応済みです。各インラインスレッドは resolved 化済みです。

対応内容の要約:

  • dataSeek() の行バッファ設計を修正(コンストラクタ時点で FETCH_BOTH により結果セット全体をバッファし、絶対位置シークと fetchRow()/fetchBoth()/fetchObject() の数値添字欠落問題を解消)
  • ファイルパス表記をフルパスに統一
  • DSN冪等 の専門用語に日本語説明を追加
  • Step 6・Step 7 に観測結果を追記

@coderabbitai

coderabbitai Bot commented Jul 8, 2026

Copy link
Copy Markdown

Tip

For best results, initiate chat on the files or code changes.

@yama ご対応ありがとうございます。修正内容を確認しました。特に dataSeek() の行バッファ設計変更(コンストラクタ時点での FETCH_BOTH による全件バッファリング)は、絶対位置シークと各種 fetch*() メソッドの数値添字欠落問題を根本的に解消する適切な対応と判断します。ファイルパスの統一、専門用語への日本語補足、Step 6・Step 7 の観測結果追記も含め、commit 18e48647a の内容で問題ありません。

(=^・ω・^=)ノ

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 2 out of 2 changed files in this pull request and generated 3 comments.

Comment thread .agent/plans/2026-07-08-pdo-migration-dbapi-refactor.md Outdated
Comment thread .agent/plans/2026-07-08-pdo-migration-dbapi-refactor.md Outdated
Comment thread .agent/plans/2026-07-08-pdo-migration-dbapi-refactor.md Outdated

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 2 out of 2 changed files in this pull request and generated 2 comments.

Comment thread .agent/plans/2026-07-08-pdo-migration-dbapi-refactor.md Outdated
Comment thread .agent/plans/2026-07-08-pdo-migration-dbapi-refactor.md Outdated
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants