Skip to content

refactor(backend): collector/エラーコード/worker の保守性改善とテスト補強#316

Merged
yusuke0610 merged 1 commit into
mainfrom
refactor/be-collector-errcode-worker
Jun 9, 2026
Merged

refactor(backend): collector/エラーコード/worker の保守性改善とテスト補強#316
yusuke0610 merged 1 commit into
mainfrom
refactor/be-collector-errcode-worker

Conversation

@yusuke0610

@yusuke0610 yusuke0610 commented Jun 9, 2026

Copy link
Copy Markdown
Owner

概要

バックエンドのリファクタリングレビュー(report/BE_report_20260609_2152.md)の採用項目を実装。重複の集約・認可ガードの共通化・worker ディスパッチの汎用化・エラーコード推定のテスト補強を行う。

変更内容

collector の重複集約(Medium)

  • services/blog/collector.py の zenn/note/qiita 3 fetcher が重複させていた 8 キー記事 dict の構築を _build_article に集約。repositories/blog.py:_apply_article_payload が期待するキー集合との契約を docstring に明記
  • 日付切り出し x[:10] if x else None_truncate_date に集約(3→1)
  • verify_user_exists の 3 連 if を _VERIFY_ENDPOINTS(platform→(URL, params))テーブル + 単一 httpx 呼び出しへ畳み込み(挙動不変)

github_link router の認可ガード共通化(Medium)

  • start/retry に重複していた github_id is None → 403 AUTH_REQUIREDrequire_github_user 依存へ集約

worker ディスパッチの汎用化(Low・承認のうえ実施)

  • execute_taskif task_type == GITHUB_LINK ハードコード分岐を get_handler(task_type).run(...) のデータ駆動ディスパッチへ。種別追加時に execute_task 無変更、ハンドラ未登録なら早期 return するため「分岐書き忘れで黙って completed」事故を構造的に除去
  • dispatch seam の patch 対象を worker._run_github_linkGitHubLinkHandler.run へ移行。guard テスト test_all_task_types_have_dispatch_branch(source grep)を test_all_task_types_have_registered_handler(レジストリ登録チェック)へ強化

テスト補強(Test Add)

  • tests/test_errors.py(新規): infer_error_code の全分岐を parametrize で固定 + messages.json 文言ドリフト結合ガード(GitHub 未検出メッセージが INTERNAL_ERROR に劣化する脆さを検知)+ async ラッパテスト

挙動変更(要周知)

  • require_github_user 依存化により、未連携ユーザーの POST /api/github-link/run/run/retryrate limit より前に 403 を返す(認可→レート制限の自然な順序)。API レスポンス契約(403/コード/メッセージ)は不変

破壊的変更

なし(DTO・API パス・env・依存追加なし)

検証

  • make lint-backend: ✅ pass
  • make test-backend: ✅ 428 passed(rate_limit テスト 1 件が認可順序変更で fail → 正規ヘルパへ修正し再 pass)
  • make dupe-check: 77→77(解消分は jscpd 閾値未満で件数不変)

Follow-ups

  • github_link の dispatch 節(payload+try/except)抽出は 2 箇所のため Rule of Three で保留(3 例目で抽出)
  • _run_github_link シムは execute_task から未使用化(直接呼び出しテスト専用)→ 将来撤去余地あり(任意・別 PR)

🤖 Generated with Claude Code

Summary by CodeRabbit

  • Bug Fixes

    • Strengthened authorization enforcement for GitHub-link operations.
    • Improved error handling and error-code inference across the platform.
  • Refactor

    • Consolidated blog article collection logic for consistent data handling.
    • Centralized task dispatch mechanism for more reliable GitHub operation processing.
  • Tests

    • Added comprehensive error-handling regression test coverage.
    • Updated security and integration tests to reflect improved authorization checks.

BE レビュー(report/BE_report_20260609_2152.md)の採用項目を実装。

- collector: 3 プラットフォーム fetcher の記事 dict 整形を _build_article に集約し、
  日付切り出しと verify_user_exists のエンドポイント分岐を畳み込み(挙動不変)
- github_link router: start/retry の GitHub 認可ガードを require_github_user 依存へ共通化
- worker: execute_task のタスク種別ハードコード分岐をハンドラレジストリ経由の
  データ駆動ディスパッチへ汎用化し「黙って completed」失敗モードを構造的に除去。
  dispatch seam の patch 対象を GitHubLinkHandler.run へ移行、guard テストを
  registry 登録チェックへ書き換え
- test: infer_error_code の全分岐回帰テストと messages.json 文言ドリフト結合ガードを追加

認可順序の変更により未連携ユーザーの /run 系は rate limit より前に 403 を返す
(レスポンス契約は不変)。

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
@coderabbitai

coderabbitai Bot commented Jun 9, 2026

Copy link
Copy Markdown

Review Change Stack

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro Plus

Run ID: bbae3043-e440-4cb0-8fe5-07cc9d758cec

📥 Commits

Reviewing files that changed from the base of the PR and between a093c4b and d9c388e.

📒 Files selected for processing (8)
  • backend/app/routers/github_link.py
  • backend/app/services/blog/collector.py
  • backend/app/services/tasks/worker.py
  • backend/tests/security/test_rate_limit.py
  • backend/tests/test_blog_collector.py
  • backend/tests/test_errors.py
  • backend/tests/test_retry_flow.py
  • backend/tests/test_worker/test_execute_task.py

📝 Walkthrough

Walkthrough

This PR refactors task dispatch and authorization by introducing a shared GitHub-user authorization guard (require_github_user), converting task execution to a handler-registry pattern, refactoring blog-article construction into shared helpers, and adding comprehensive error-code regression tests with updated mocking throughout the test suite.

Changes

Task Dispatch and Authorization Refactoring

Layer / File(s) Summary
Centralized GitHub User Authorization Guard
backend/app/routers/github_link.py
require_github_user dependency validates user.github_id is present; both /api/github-link/run and /api/github-link/run/retry endpoints switch from get_current_user plus inline checks to the shared guard, centralizing AUTH_REQUIRED error response.
Handler Registry-Based Task Dispatch
backend/app/services/tasks/worker.py
execute_task converts from per-task-type shim calls (e.g., _run_github_link) to generic get_handler(task_type) registry dispatch; _run_github_link is retained as a direct test entry point.
Error Code Inference Regression Tests
backend/tests/test_errors.py
New module adds comprehensive tests for infer_error_code, infer_async_error_code, and resolve_async_error_code, with explicit contract locks for GitHub "user not found" substring matching and status-to-code mappings.
Task Dispatch Unit Test Updates
backend/tests/test_worker/test_execute_task.py
Tests switch from mocking _run_github_link to mocking GitHubLinkHandler.run; new test validates all TaskType values are registered in the handler registry; error and success paths align with the new architecture.
Integration and Rate Limit Test Updates
backend/tests/test_retry_flow.py, backend/tests/security/test_rate_limit.py
test_retry_flow.py mocks GitHubLinkHandler.run via patch.object; test_rate_limit.py constructs requests with github_id header instead of database user setup, ensuring authorization does not block rate-limiter validation.

Blog Collector Service Refactoring

Layer / File(s) Summary
Article Building and Verification Helpers
backend/app/services/blog/collector.py
Introduces _VERIFY_ENDPOINTS mapping for platform-specific user-verification templates, _truncate_date for consistent ISO-date truncation, and _build_article shared helper for standardized article dict construction.
Platform-Specific Article Fetching Updates
backend/app/services/blog/collector.py, backend/tests/test_blog_collector.py
fetch_zenn_articles, fetch_note_articles, and fetch_qiita_articles call _build_article with platform-extracted fields; verify_user_exists uses centralized endpoint lookup instead of per-platform request branching; test assertion updated to expect params=None in HTTP GET call.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~25 minutes

Possibly related PRs

  • yusuke0610/devforge#270: Modifies retry_github_link endpoint error message construction, which shares the updated auth dependency guard introduced in this PR.

Poem

A rabbit hops through the refactor maze,
Centralizing guards that guard all days,
Handlers registry keeps tasks in line,
Blog collectors bloom in helpers' design. 🐰✨

🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title clearly describes the main refactoring effort across three backend components (collector/error-code/worker) with a focus on maintainability improvements and test strengthening, directly matching the PR's core objectives.
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.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
📝 Generate docstrings
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch refactor/be-collector-errcode-worker

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 and usage tips.

@yusuke0610
yusuke0610 merged commit 5801b50 into main Jun 9, 2026
18 checks passed
@yusuke0610
yusuke0610 deleted the refactor/be-collector-errcode-worker branch June 10, 2026 05:59
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant