refactor: rename class from ProgramStorage to ProgramSaver#53
refactor: rename class from ProgramStorage to ProgramSaver#53
Conversation
Walkthrough
Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Poem
📜 Recent review detailsConfiguration used: CodeRabbit UI Review profile: CHILL Plan: Pro 💡 Knowledge Base configuration:
You can enable these sources in your CodeRabbit configuration. 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
✨ Finishing Touches
🧪 Generate unit tests
🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. CodeRabbit Commands (Invoked using PR/Issue comments)Type Other keywords and placeholders
CodeRabbit Configuration File (
|
There was a problem hiding this comment.
Actionable comments posted: 0
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/ProgramSaver.php (1)
24-39: today.json の競合や部分書き込みを防ぐため原子的書き込みに変更を。
file_put_contents($path, $contents)は並行実行時に破損の恐れ。テンポラリへ書いてrename()で入れ替える実装へ。- if (file_put_contents($path, $contents) === false) { - throw new \RuntimeException("Failed to save programs to {$path}"); - } + $tmpPath = $path . '.tmp.' . uniqid('', true); + if (file_put_contents($tmpPath, $contents, LOCK_EX) === false) { + @unlink($tmpPath); + throw new \RuntimeException("Failed to write temp file: {$tmpPath}"); + } + if (!@rename($tmpPath, $path)) { + @unlink($tmpPath); + throw new \RuntimeException("Failed to move temp file into place: {$path}"); + }
🧹 Nitpick comments (5)
src/ProgramSaver.php (2)
26-29: JSON エンコードの堅牢化(例外+日本語可読性)。例外で確実に検知し、文字化け回避のため UNESCAPED_UNICODE を付与するのを推奨。
- $contents = json_encode(['programs' => $programs]); - if ($contents === false) { - throw new \RuntimeException("Failed to encode programs to JSON"); - } + try { + $contents = json_encode( + ['programs' => $programs], + JSON_UNESCAPED_UNICODE | JSON_UNESCAPED_SLASHES | JSON_PRESERVE_ZERO_FRACTION | JSON_THROW_ON_ERROR + ); + } catch (\JsonException $e) { + throw new \RuntimeException("Failed to encode programs to JSON", 0, $e); + }
31-34: 作成ディレクトリのパーミッションを適正化。
0777は広すぎます。公開用途でなければ0775か0755を推奨。- if (!is_dir($dir) && !mkdir($dir, 0777, true) && !is_dir($dir)) { + if (!is_dir($dir) && !mkdir($dir, 0775, true) && !is_dir($dir)) {scraper.php (2)
37-39: 変数名もクラス意図に合わせてリネームすると読みやすいです。
$storage→$saverへ。意味の一貫性向上。-$storage = new ProgramSaver(); -$storage->save($programs, "docs/{$version}/" . $date->format('Y') . '/' . $date->format('Ymd') . '.json'); -$storage->save($programs, "docs/{$version}/today.json"); +$saver = new ProgramSaver(); +$saver->save($programs, "docs/{$version}/" . $date->format('Y') . '/' . $date->format('Ymd') . '.json'); +$saver->save($programs, "docs/{$version}/today.json");
30-33: 終了コードの明示を推奨。処理不成立時は
exit(0)かexit(1)を明示。CI での判定が安定します(要件に応じて選択)。- exit; + exit(0);tests/ProgramSaverTest.php (1)
30-32: 一時ディレクトリ名のプリフィックスも新名称へ。整合性のため
program_saver_test_に。- $this->tempDir = sys_get_temp_dir() . '/program_storage_test_' . uniqid(); + $this->tempDir = sys_get_temp_dir() . '/program_saver_test_' . uniqid();
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (3)
scraper.php(2 hunks)src/ProgramSaver.php(1 hunks)tests/ProgramSaverTest.php(2 hunks)
🧰 Additional context used
🧬 Code graph analysis (2)
tests/ProgramSaverTest.php (1)
src/ProgramSaver.php (1)
ProgramSaver(12-40)
scraper.php (1)
src/ProgramSaver.php (1)
ProgramSaver(12-40)
🔇 Additional comments (5)
src/ProgramSaver.php (1)
12-12: 残存参照はありませんでした。マージして構いません。scraper.php (1)
8-8: use 句のリネーム対応OK。tests/ProgramSaverTest.php (3)
7-7: インポートのリネーム対応OK。
13-13: テストクラス名のリネーム対応OK。
62-62: インスタンス化のリネーム対応OK。
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (2)
tests/ProgramSaverTest.php (2)
30-31: 一時ディレクトリ生成の堅牢化(uniqid の衝突回避と mkdir 成否確認)テストのフレーク対策として、
uniqid('', true)でエントロピーを増やし、mkdirの戻り値を検証しましょう。- $this->tempDir = sys_get_temp_dir() . '/program_saver_test_' . uniqid(); - mkdir($this->tempDir, 0777, true); + $this->tempDir = sys_get_temp_dir() . '/program_saver_test_' . uniqid('', true); + if (!mkdir($this->tempDir, 0777, true) && !is_dir($this->tempDir)) { + $this->fail('Failed to create temp dir: ' . $this->tempDir); + }
105-111: JSON 読み書きテストの堅牢化(read 失敗検知と厳密デコード)
file_get_contentsの失敗検知とJSON_THROW_ON_ERRORの利用で、失敗時に原因が即時に分かるようにしましょう。- $content = json_decode(file_get_contents($path), true); + $raw = file_get_contents($path); + $this->assertNotFalse($raw); + $content = json_decode($raw, true, 512, JSON_THROW_ON_ERROR); $this->assertArrayHasKey('programs', $content); $this->assertSame($programs, $content['programs']);追加で、ディレクトリ自動作成の挙動やエラー系も押さえると安心です(任意):
public function testSaveCreatesIntermediateDirectories(): void { $saver = new ProgramSaver(); $path = $this->tempDir . '/nested/a/b/programs.json'; $saver->save([['race_date' => '2025-01-01']], $path); $this->assertFileExists($path); } public function testSaveThrowsOnInvalidJson(): void { $this->expectException(\RuntimeException::class); $saver = new ProgramSaver(); // 不正な UTF-8 を含む文字列でエンコード失敗を誘発 $invalid = "\xC3\x28"; $saver->save([['race_title' => $invalid]], $this->tempDir . '/invalid.json'); }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (1)
tests/ProgramSaverTest.php(4 hunks)
🧰 Additional context used
🧬 Code graph analysis (1)
tests/ProgramSaverTest.php (1)
src/ProgramSaver.php (2)
ProgramSaver(12-40)save(24-39)
🔇 Additional comments (4)
tests/ProgramSaverTest.php (4)
7-7: import のリネーム反映 OK新しいクラス名
ProgramSaverへの置換が適切です。
13-13: テストクラス名のリネーム OK命名の一貫性が保たれています。
62-62: インスタンス化の置換 OK
ProgramSaverへの移行が問題なく行われています。
7-7: リネーム漏れはありません
リポジトリ全体にProgramStorage/program_storage_test_の参照は存在せず、ProgramSaverのクラス宣言とuse構文がすべて整合しています。
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (2)
tests/ProgramSaverTest.php (2)
30-31: 一時ディレクトリの衝突耐性と権限を強化しましょう。uniqid() は予測可能で衝突耐性が弱めです。乱数ベース+最小権限(0700)にし、mkdir 失敗も検知すると頑健です。
- $this->tempDir = sys_get_temp_dir() . '/program_saver_test_' . uniqid(); - mkdir($this->tempDir, 0755, true); + $this->tempDir = sys_get_temp_dir() . '/program_saver_test_' . bin2hex(random_bytes(8)); + if (!mkdir($this->tempDir, 0700, true) && !is_dir($this->tempDir)) { + $this->fail('Failed to create temp dir: ' . $this->tempDir); + }
105-105: カバレッジ拡充: save() 内のディレクトリ自動作成分岐をテストに追加を。setUp() で親ディレクトリを先に作っているため、
ProgramSaver::save()のmkdir(..., 0755, true)分岐が現状未カバーです。ネストされたパスに保存するテストを1件追加してください。json_decodeの結果が配列であることの明示的検証もあると親切です。例(追加する新規テストメソッド):
public function testSaveCreatesNestedDirectory(): void { $saver = new ProgramSaver(); $path = $this->tempDir . '/nested/a/b/programs.json'; $programs = [['race_date' => '2025-01-01']]; $saver->save($programs, $path); $this->assertFileExists($path); }既存テストの堅牢化(参考・任意):
$this->assertIsArray($content, 'JSON decode failed: ' . json_last_error_msg());
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (2)
src/ProgramSaver.php(2 hunks)tests/ProgramSaverTest.php(4 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- src/ProgramSaver.php
🧰 Additional context used
🧬 Code graph analysis (1)
tests/ProgramSaverTest.php (1)
src/ProgramSaver.php (2)
ProgramSaver(12-40)save(24-39)
🔇 Additional comments (3)
tests/ProgramSaverTest.php (3)
13-13: テストクラス名とファイル名の整合性OK。
62-62: 変数名の一貫性($saver)およびクラス名の置換は適切です。
7-7: import 更新と残存参照の検証完了
リネームに伴う import 更新は適切で、旧クラスProgramStorageの残存参照やmkdir(..., 0777)呼び出しは検出されませんでした。
JSON ファイルを保存するだけなので直感的な Saver にリネームしました。また、ディレクトリ作成の権限を 0777 から 0755 に変更しました。さらに、一時ディレクトリの衝突耐性を強化しました。
Summary by CodeRabbit