Skip to content

feat: #31 DB設計とモデル実装(全6テーブル + ランク計算)#42

Merged
Inlet-back merged 25 commits intodevelopfrom
feature/issue-31-db-models
Feb 19, 2026
Merged

feat: #31 DB設計とモデル実装(全6テーブル + ランク計算)#42
Inlet-back merged 25 commits intodevelopfrom
feature/issue-31-db-models

Conversation

@Reeeid
Copy link
Contributor

@Reeeid Reeeid commented Feb 19, 2026

Issue: #31

実装の概要

issue #31で定義された6テーブルとランク計算機能、および管理用APIを実装しました。

実装内容

  • 6テーブルのモデル・スキーマ・CRUD: Profile, OAuthAccount, Badge, Quest, QuestProgress, SkillTree
  • 暗号化ユーティリティ: Fernet対称暗号によるOAuthトークン暗号化(スレッドセーフ対応済み)
  • ランク計算機能: 経験値から0-9(種子〜世界樹)のランク自動算出、Pydantic検証、管理API
  • Alembicマイグレーション: 6テーブル分(依存順に分離、冗長インデックス削除済み)
  • Pydanticバリデーション: tier (1-3), difficulty (0-9), provider (github), portfolio_url (HttpUrl), rank整合性
  • 管理用API: ランク不整合修正API(X-Admin-Key認証、独立Swagger UI)

🔧 技術的な意思決定とトレードオフ (最重要)

採用したアプローチ

1. TDD(テスト駆動開発)

  • 手法: 実装前にテストコードを作成し、RED→GREEN→REFACTORサイクルで開発
  • メリット: 仕様が明確なため、テスト先行で実装品質を担保。66件のテストがすべてGREEN
  • デメリット/リスク: 初期工数が増加。仕様変更時にテストコードも修正が必要

2. Fernet対称暗号によるトークン暗号化(スレッドセーフ対応)

  • 手法: cryptography.fernet.Fernet + 環境変数 ENCRYPTION_KEYthreading.Lockによるdouble-checked locking
  • メリット:
    • 業界標準ライブラリ使用、独自実装を避けることでセキュリティリスク低減(IPA基準準拠)
    • DB漏洩時の二次被害を防止
    • マルチスレッド環境(uvicorn等)での競合状態を防止
  • デメリット/リスク:
    • 暗号化キーの管理が必須(本番環境では秘密管理ツール必須)
    • 暗号化/復号のオーバーヘッド(数ms)

3. 主キーの冗長なインデックス削除

  • 手法: モデルとAlembicマイグレーションから主キーのindex=Trueを削除
  • メリット: PRIMARY KEYは自動的にインデックスを作成するため、重複を防ぎDB容量削減・パフォーマンス改善
  • デメリット/リスク: なし(ベストプラクティス)

4. SkillTree自動初期化

  • 手法: create_user()内で6カテゴリのSkillTreeを自動生成
  • メリット: ユーザー作成と同時にSkillTreeが用意され、別途初期化処理が不要
  • デメリット/リスク: トランザクション失敗時のロールバック処理が必要

5. 管理用API(独立認証・Swagger UI)

  • 手法: FastAPI sub-applicationとして独立、X-Admin-Key認証、専用Swagger UI (/admin/docs)
  • メリット:
    • 通常APIと管理APIを分離、認証も独立
    • Swagger UIも認証が必要なため、管理APIの仕様が外部に露出しない
  • デメリット/リスク:
    • API Key認証のみ(DBテーブル未定のため)
    • 本番環境では必ず強力なキーを設定する必要がある

6. 本番環境での起動時検証

  • 手法: ENV=productionの場合のみvalidate_encryption_key()を実行
  • メリット: 本番環境で暗号化キー未設定のまま起動するリスクを防止
  • デメリット/リスク: テスト環境では検証をスキップ(conftest.pyでダミーキー設定済み)

却下したアプローチ(代替案)

1. 独自実装の暗号化アルゴリズム

  • 手法: 自作の暗号化ロジック
  • 却下理由: セキュリティの専門知識が不足しており、脆弱性を生むリスクが高い。業界標準ライブラリ(cryptography)が信頼性が高く、IPA基準にも準拠

2. テスト肥大化を許容

  • 手法: スキーマバリデーションテスト10ファイルをそのまま維持
  • 却下理由: テストコードの保守性が低下し、CI実行時間も増加。妥当性を担保しつつ削減可能

3. マイグレーション1本化

  • 手法: 全6テーブルを1つのマイグレーションファイルにまとめる
  • 却下理由: ロールバック時に個別テーブル単位で戻せない。テーブル追加時の依存関係が複雑化

🧪 テスト戦略と範囲

追加したテストケース

正常系

  • CRUD操作: 全6テーブルのCreate/Read/Update操作が正常に動作
  • 暗号化/復号: OAuthトークンの暗号化→復号ラウンドトリップが成功
  • ランク計算: 経験値から0-9のランク算出が正しく動作(境界値含む)
  • バリデーション: tier (1-3), difficulty (0-9), provider (github), portfolio_url (HttpUrl) が正常値で通過
  • ランク整合性: Pydanticバリデーションで経験値とランクの整合性を検証
  • SkillTree自動初期化: ユーザー作成時に6カテゴリ自動生成

異常系

  • バリデーションエラー: 範囲外の値(tier 0/4, difficulty -1/10, provider "twitter", portfolio_url "not-a-url")でエラー
  • 重複制約違反: UNIQUE制約(user_id + provider, user_id + quest_id, user_id + category + tier)違反でエラー
  • ランク不整合検出: expとrankが不一致の場合にPydanticでエラー

テストしていないこと

  • 本番環境での暗号化キー管理(AWS Secrets Manager等の秘密管理ツール連携)
  • 大規模データでのパフォーマンス(10万件以上のクエリ性能)
  • 同時接続数100件以上でのトランザクション競合
  • データベースクラッシュ時のリカバリ手順
  • 管理API認証のブルートフォース攻撃対策

セキュリティに関する自己評価

  • 機密情報のハードコードはないか

    • OAuthトークンは環境変数 ENCRYPTION_KEY による暗号化
    • 管理APIキーは環境変数 ADMIN_API_KEY で管理
    • .env 管理、リポジトリに含めない
    • テスト用キーはTEST_ENCRYPTION_KEYとして明示(conftest.py)
  • 入力値の検証(バリデーション)は行っているか

    • Pydantic field_validator による入力検証
      • Badge.tier: 1-3範囲チェック
      • Quest.difficulty: 0-9範囲チェック
      • OAuthAccount.provider: ホワイトリスト(github)
      • Profile.portfolio_url: HttpUrl型による形式検証
      • User.rank: 経験値との整合性検証(model_validator)
  • 既知の脆弱性パターンへの対策は考慮したか

    • SQLインジェクション: SQLAlchemy ORMによるパラメータバインド(文字列結合なし)
    • 暗号化: 業界標準ライブラリ(cryptography.fernet)使用、独自実装を避ける
    • 入力検証: Pydantic schema層での検証により不正データのDB投入を防止
    • スレッドセーフ: threading.LockによるFernetインスタンス初期化の競合状態防止
    • 認証: 管理APIはX-Admin-Key認証(DB未定のため簡易実装)

レビュワー(人間)への申し送り事項

SwaggerUI確認

053bf4b38a3b58c5b9745e66d7d93249

GitHub Copilot指摘対応済み

  • ✅ Alembicマイグレーションの冗長インデックス削除(0002, 0003, 0004)
  • _get_fernet()のスレッドセーフ対応(threading.Lock + double-checked locking)
  • ✅ 本番環境での暗号化キー検証(main.py)
  • tree_dataのデフォルト値修正(モデル層から削除、Pydanticで設定)
  • ✅ IntegrityError個別処理(全CRUDファイル)

テスト実行結果

66 tests passed, coverage 86%

注意点

  • ENCRYPTION_KEY: 本番環境では必ず .env で設定すること(32バイトのFernet互換base64キー)
  • ADMIN_API_KEY: 本番環境では必ず強力なキーを設定すること
  • SkillTree初期化: ユーザー作成時に6カテゴリ自動生成する機能は実装済み(crud.skill_tree.initialize_skill_trees_for_user
  • Admin Swagger UI: /admin/docsX-Admin-Key認証が必要(通常の/docsとは独立)

補足

  • product-spec準拠(ランク0-9、バッジカテゴリ5種、スキルカテゴリ6種)
  • User.skills(JSON)は全カテゴリの習得済みスキル集約キャッシュとして維持
  • 管理用APIはAPIキーによる認証(DBテーブル未定のため簡易実装)

Closes #31

issue #31で定義された6テーブル(Profile, OAuthAccount, Badge, Quest,
QuestProgress, SkillTree)+暗号化ユーティリティ+ランク計算のテストを先行作成。(TDD RED状態)
OAuthテストではFernet暗号化/復号のラウンドトリップ、refresh_token暗号化、
UNIQUE制約違反を網羅。product-specのランク0-9(種子〜世界樹)に準拠。
OAuthトークン保護のためFernet対称暗号化を導入。
- app/core/encryption.py: encrypt_token/decrypt_token(遅延初期化、キー未設定時にValueError)
- config.py: ENCRYPTION_KEY設定追加(.env必須)
- conftest.py: テスト用暗号化キー設定とキャッシュリセットfixture
独自実装を避け、cryptographyライブラリの標準Fernetを使用(security SKILL準拠)。
…Status)

product-specのバッジ5種(Commit/Days/Builder/Writer/Seeker)、
スキルツリー6カテゴリ(Web/AI/Security/Infrastructure/Design/Game)、
クエストステータス3種(NotStarted/InProgress/Completed)を定義。
QuestCategoryはSkillCategoryと同じ6種に統一。
Profile(1:1), OAuthAccount(1:N), Badge(1:N), Quest(独立),
QuestProgress(User-Quest中間), SkillTree(1:6)のモデルを追加。
- OAuthAccount: encrypted_access_token/encrypted_refresh_tokenで暗号化保存
- Badge: UNIQUE(user_id, category, tier)で重複バッジ防止
- SkillTree: UNIQUE(user_id, category)で6カテゴリ固定
- User: 全テーブルへのrelationship定義
- base.py: Alembic自動検出用の全モデルimport
Create/Update/Response用スキーマを各テーブルに対して定義。
- OAuthAccountCreate: access_tokenを平文で受け取りCRUD層で暗号化
- OAuthAccount(Response): トークン値を含めないセキュアなレスポンス設計
- BadgeCreate/QuestCreate: Enumを型として受け取りバリデーション
- Profile: get_by_user_id, create, update(exclude_unsetで部分更新)
- OAuthAccount: create時にaccess_token/refresh_tokenをFernet暗号化、update_tokens
- Badge: award_badge, get_badges_by_user
- Quest: get, list_by_category, create
- QuestProgress: start_quest, complete_quest, get_quest_progress
- SkillTree: initialize_for_user(6カテゴリ一括生成), update_tree, get_by_user_category
全操作でcommit失敗時のrollback処理を実装。
経験値からランク0-9(種子〜世界樹)を算出するロジックを実装。
閾値は暫定値(0,100,300,600,1000,1500,2500,4000,6000,9000)。
update_user_expで経験値加算+ランク再計算を一括処理。
MVP段階ではこの関数で十分、仕様確定後に閾値を調整する想定。
FK依存順にチェーン:
0002: profiles (User 1:1)
0003: oauth_accounts (User 1:N, UNIQUE user_id+provider)
0004: badges (User 1:N, UNIQUE user_id+category+tier)
0005: quests (独立テーブル)
0006: quest_progress (User-Quest中間, UNIQUE user_id+quest_id)
0007: skill_trees (User 1:6, UNIQUE user_id+category)
個別rollbackが可能な設計。autogenerateの結果を手動分割
- pytest-cov: カバレッジ計測のため追加(spec 12.テスト方針準拠)
coding-standards SKILLに準拠し、Pydanticのfield_validatorで入力値検証を追加:
- Badge.tier: 1-3(Bronze/Silver/Gold)の範囲チェック
- Quest.difficulty: 0-9(ランク: 種子〜世界樹)の範囲チェック
- OAuthAccount.provider: githubのホワイトリスト検証(MVPスコープ)
- Profile.portfolio_url: HttpUrl型でURL形式検証

HttpUrl→str変換をCRUD層で実施(SQLiteバインディング対応)。
スキーマバリデーションテスト10件追加(合計58テスト全GREEN)。
テストケースの肥大化を防ぐため、個別のスキーマバリデーションテスト
ファイル(test_schema_*.py)を単一のtest_validation.pyに統合した。

変更内容:
- 削除: test_schema_badge.py, test_schema_quest.py, test_schema_oauth_account.py, test_schema_profile.py
- 作成: test_validation.py(4つのテスト関数)
- テスト数: 58件 → 51件(妥当性は担保)

検証:
- 全51件のテストがパス
- ruffリントチェック通過
主キー(PRIMARY KEY)には自動的にインデックスが作成されるため、
明示的に `index=True` を指定すると無駄なインデックスが重複作成される。

変更内容:
- User, Profile, OAuthAccount, Badge, Quest, QuestProgress, SkillTree
- `id = Column(Integer, primary_key=True, index=True)`
  → `id = Column(Integer, primary_key=True)`

検証:
- 全51件のテストがパス
- ruffリントチェック通過
Copy link

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

このPRは、Issue #31に基づいて6つの新規テーブル(Profile, OAuthAccount, Badge, Quest, QuestProgress, SkillTree)とランク計算機能、OAuth トークン暗号化機能を実装したものです。TDD(テスト駆動開発)アプローチを採用し、51件のテストが全てGREENの状態で実装が完了しています。

Changes:

  • 6テーブルのモデル・スキーマ・CRUD実装(Profile, OAuthAccount, Badge, Quest, QuestProgress, SkillTree)
  • Fernet対称暗号化によるOAuthトークン保護機能
  • 経験値ベースのランク計算ロジック(0-9の10段階)
  • Alembicマイグレーション6本(依存関係順に分離、ロールバック可能)
  • Pydanticバリデーション(tier 1-3, difficulty 0-9, provider whitelist, HttpUrl)
  • 包括的なテストスイート(51件、正常系・異常系・バリデーションをカバー)

Reviewed changes

Copilot reviewed 41 out of 42 changed files in this pull request and generated 14 comments.

Show a summary per file
File Description
backend/app/models/*.py 6つの新規モデルとenums定義、Userモデルへのリレーションシップ追加
backend/app/schemas/*.py Pydanticスキーマ定義とfield_validatorによる入力検証
backend/app/crud/*.py CRUD操作実装(暗号化処理、トランザクション管理を含む)
backend/app/core/encryption.py Fernet対称暗号化ユーティリティ
backend/app/core/rank.py ランク計算ロジック(経験値ベース)
backend/app/core/config.py 暗号化キーとランク閾値の設定追加
backend/alembic/versions/000*.py 6本のマイグレーションファイル(依存順: profiles→oauth_accounts→badges→quests→quest_progress→skill_trees)
backend/tests/test_*.py 51件のテストケース(暗号化、ランク計算、CRUD、バリデーション)
backend/pyproject.toml cryptography ^44.0.0 と pytest-cov ^7.0.0 の依存関係追加
backend/tests/conftest.py 暗号化キーのテスト環境設定とFernetキャッシュリセット

Comment on lines 34 to 40
op.create_index(op.f("ix_oauth_accounts_id"), "oauth_accounts", ["id"], unique=False)
op.create_index(op.f("ix_oauth_accounts_user_id"), "oauth_accounts", ["user_id"], unique=False)


def downgrade() -> None:
op.drop_index(op.f("ix_oauth_accounts_user_id"), table_name="oauth_accounts")
op.drop_index(op.f("ix_oauth_accounts_id"), table_name="oauth_accounts")
Copy link

Copilot AI Feb 19, 2026

Choose a reason for hiding this comment

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

主キーに対する冗長なインデックスが作成されています。PRIMARY KEY制約は自動的にインデックスを作成するため、ix_oauth_accounts_id インデックスは不要です。DB容量削減のため削除を推奨します。

Suggested change
op.create_index(op.f("ix_oauth_accounts_id"), "oauth_accounts", ["id"], unique=False)
op.create_index(op.f("ix_oauth_accounts_user_id"), "oauth_accounts", ["user_id"], unique=False)
def downgrade() -> None:
op.drop_index(op.f("ix_oauth_accounts_user_id"), table_name="oauth_accounts")
op.drop_index(op.f("ix_oauth_accounts_id"), table_name="oauth_accounts")
op.create_index(op.f("ix_oauth_accounts_user_id"), "oauth_accounts", ["user_id"], unique=False)
def downgrade() -> None:
op.drop_index(op.f("ix_oauth_accounts_user_id"), table_name="oauth_accounts")

Copilot uses AI. Check for mistakes.
Comment on lines 32 to 38
op.create_index(op.f("ix_badges_id"), "badges", ["id"], unique=False)
op.create_index(op.f("ix_badges_user_id"), "badges", ["user_id"], unique=False)


def downgrade() -> None:
op.drop_index(op.f("ix_badges_user_id"), table_name="badges")
op.drop_index(op.f("ix_badges_id"), table_name="badges")
Copy link

Copilot AI Feb 19, 2026

Choose a reason for hiding this comment

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

主キーに対する冗長なインデックスが作成されています。PRIMARY KEY制約は自動的にインデックスを作成するため、ix_badges_id インデックスは不要です。DB容量削減のため削除を推奨します。

Suggested change
op.create_index(op.f("ix_badges_id"), "badges", ["id"], unique=False)
op.create_index(op.f("ix_badges_user_id"), "badges", ["user_id"], unique=False)
def downgrade() -> None:
op.drop_index(op.f("ix_badges_user_id"), table_name="badges")
op.drop_index(op.f("ix_badges_id"), table_name="badges")
op.create_index(op.f("ix_badges_user_id"), "badges", ["user_id"], unique=False)
def downgrade() -> None:
op.drop_index(op.f("ix_badges_user_id"), table_name="badges")

Copilot uses AI. Check for mistakes.
Comment on lines 10 to 24
def _get_fernet() -> Fernet:
"""Fernetインスタンスを遅延初期化して返す。

ENCRYPTION_KEYが未設定の場合はValueErrorを送出する。
"""
global _fernet # noqa: PLW0603
if _fernet is None:
key = settings.ENCRYPTION_KEY
if not key:
raise ValueError(
"ENCRYPTION_KEY is not set. "
"Generate one with: python -c \"from cryptography.fernet import Fernet; print(Fernet.generate_key().decode())\""
)
_fernet = Fernet(key.encode())
return _fernet
Copy link

Copilot AI Feb 19, 2026

Choose a reason for hiding this comment

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

グローバル変数_fernetの初期化とリセットがスレッドセーフではありません。複数のリクエストが同時に_get_fernet()を呼び出した場合、競合状態(race condition)が発生する可能性があります。

マルチスレッド環境(uvicorn等)では、threading.Lockを使用して初期化を保護することを推奨します:

import threading

_fernet: Fernet | None = None
_fernet_lock = threading.Lock()

def _get_fernet() -> Fernet:
    global _fernet
    if _fernet is None:
        with _fernet_lock:
            if _fernet is None:  # Double-checked locking
                key = settings.ENCRYPTION_KEY
                if not key:
                    raise ValueError(...)
                _fernet = Fernet(key.encode())
    return _fernet

Copilot uses AI. Check for mistakes.
Comment on lines 59 to 69
def validate_encryption_key() -> None:
"""ENCRYPTION_KEYが設定されているかを検証する。

未設定の場合はValueErrorを送出する。
テスト環境ではダミーキーが自動設定されるため、本番環境のみチェック。
"""
if not settings.ENCRYPTION_KEY:
raise ValueError(
"ENCRYPTION_KEY is not set. "
"Generate one with: python -c \"from cryptography.fernet import Fernet; print(Fernet.generate_key().decode())\""
)
Copy link

Copilot AI Feb 19, 2026

Choose a reason for hiding this comment

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

validate_encryption_key()関数が定義されていますが、アプリケーション起動時(main.py)やテスト初期化時に呼び出されていません。

本番環境でENCRYPTION_KEYが未設定のまま起動すると、初回のOAuthトークン暗号化時にエラーが発生します。これは実行時エラーとなり、デバッグが困難です。

アプリケーション起動時に検証を実行することを推奨します:

# backend/app/main.py
from app.core.config import settings, validate_encryption_key

# 本番環境のみ検証(テストではconftest.pyでENVが設定される)
import os
if os.getenv("ENV", "development") == "production":
    validate_encryption_key()

Copilot uses AI. Check for mistakes.
id = Column(Integer, primary_key=True)
user_id = Column(Integer, ForeignKey("users.id"), nullable=False, index=True)

category = Column(String, nullable=False)
Copy link

Copilot AI Feb 19, 2026

Choose a reason for hiding this comment

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

モデルのcategoryフィールドがString型として定義されていますが、スキーマではBadgeCategory Enumを使用しています。この不一致により、DB層とアプリケーション層の間で型の整合性が保証されません。

CRUDレイヤーで.valueを使用して文字列に変換していますが(例: badge.py:16)、モデル側でSQLAlchemy のEnum型を使用することで型安全性を向上できます:

from sqlalchemy import Enum as SAEnum
from app.models.enums import BadgeCategory

category = Column(SAEnum(BadgeCategory), nullable=False)

ただし、現在の実装でも動作に問題はないため、オプショナルな改善提案です。

Copilot uses AI. Check for mistakes.
Comment on lines 32 to 36
op.create_index(op.f("ix_quests_id"), "quests", ["id"], unique=False)


def downgrade() -> None:
op.drop_index(op.f("ix_quests_id"), table_name="quests")
Copy link

Copilot AI Feb 19, 2026

Choose a reason for hiding this comment

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

主キーに対する冗長なインデックスが作成されています。PRIMARY KEY制約は自動的にインデックスを作成するため、ix_quests_id インデックスは不要です。DB容量削減のため削除を推奨します。

Suggested change
op.create_index(op.f("ix_quests_id"), "quests", ["id"], unique=False)
def downgrade() -> None:
op.drop_index(op.f("ix_quests_id"), table_name="quests")
def downgrade() -> None:

Copilot uses AI. Check for mistakes.
sa.PrimaryKeyConstraint("id"),
sa.UniqueConstraint("user_id", "quest_id", name="uq_progress_user_quest"),
)
op.create_index(op.f("ix_quest_progress_id"), "quest_progress", ["id"], unique=False)
Copy link

Copilot AI Feb 19, 2026

Choose a reason for hiding this comment

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

主キーに対する冗長なインデックスが作成されています。PRIMARY KEY制約は自動的にインデックスを作成するため、ix_quest_progress_id インデックスは不要です。DB容量削減のため削除を推奨します。

Copilot uses AI. Check for mistakes.
Comment on lines 32 to 38
op.create_index(op.f("ix_skill_trees_id"), "skill_trees", ["id"], unique=False)
op.create_index(op.f("ix_skill_trees_user_id"), "skill_trees", ["user_id"], unique=False)


def downgrade() -> None:
op.drop_index(op.f("ix_skill_trees_user_id"), table_name="skill_trees")
op.drop_index(op.f("ix_skill_trees_id"), table_name="skill_trees")
Copy link

Copilot AI Feb 19, 2026

Choose a reason for hiding this comment

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

主キーに対する冗長なインデックスが作成されています。PRIMARY KEY制約は自動的にインデックスを作成するため、ix_skill_trees_id インデックスは不要です。DB容量削減のため削除を推奨します。

Suggested change
op.create_index(op.f("ix_skill_trees_id"), "skill_trees", ["id"], unique=False)
op.create_index(op.f("ix_skill_trees_user_id"), "skill_trees", ["user_id"], unique=False)
def downgrade() -> None:
op.drop_index(op.f("ix_skill_trees_user_id"), table_name="skill_trees")
op.drop_index(op.f("ix_skill_trees_id"), table_name="skill_trees")
op.create_index(op.f("ix_skill_trees_user_id"), "skill_trees", ["user_id"], unique=False)
def downgrade() -> None:
op.drop_index(op.f("ix_skill_trees_user_id"), table_name="skill_trees")

Copilot uses AI. Check for mistakes.
Comment on lines +29 to +45
def update_profile(db: Session, profile_id: int, profile_in: ProfileUpdate) -> Profile:
db_profile = db.query(Profile).filter(Profile.id == profile_id).first()
if db_profile is None:
raise ValueError(f"Profile with id={profile_id} not found")
update_data = profile_in.model_dump(exclude_unset=True)
# HttpUrlをstrに変換
if "portfolio_url" in update_data and update_data["portfolio_url"]:
update_data["portfolio_url"] = str(update_data["portfolio_url"])
for field, value in update_data.items():
setattr(db_profile, field, value)
try:
db.commit()
except Exception:
db.rollback()
raise
db.refresh(db_profile)
return db_profile
Copy link

Copilot AI Feb 19, 2026

Choose a reason for hiding this comment

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

CRUD操作のValueErrorが発生するケース(存在しないレコードの更新等)のテストが不足しています。

例えば、以下のケースのテストを追加することを推奨します:

  1. update_profile() で存在しないprofile_idを指定した場合
  2. update_skill_tree() で存在しないuser_id/categoryを指定した場合
  3. complete_quest() で存在しないquest_idを指定した場合

テスト例:

def test_update_profile_not_found(db):
    with pytest.raises(ValueError, match="Profile with id=999 not found"):
        update_profile(db, 999, ProfileUpdate(github_username="test"))

現在51件のテストがGREENですが、エラーケースのカバレッジを追加することで、より堅牢なコードになります。

Copilot uses AI. Check for mistakes.
Comment on lines 11 to 29
def test_initialize_skill_trees_for_user(db):
"""ユーザー作成時に6カテゴリ全てのSkillTreeが生成される"""
user = create_user(db, UserCreate(username="skill_tree_user"))
trees = initialize_skill_trees_for_user(db, user.id)

assert len(trees) == 6
categories = {t.category for t in trees}
assert categories == {
SkillCategory.WEB,
SkillCategory.AI,
SkillCategory.SECURITY,
SkillCategory.INFRASTRUCTURE,
SkillCategory.DESIGN,
SkillCategory.GAME,
}
for tree in trees:
assert tree.tree_data == {}
assert tree.generated_at is None

Copy link

Copilot AI Feb 19, 2026

Choose a reason for hiding this comment

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

テストでinitialize_skill_trees_for_userが正しく動作することは確認されていますが、実際のユーザー作成フロー(create_user)でこの関数が自動的に呼び出されることが確認されていません。

Issue #31の要件「SkillTree初期化: ユーザー作成時6カテゴリ自動生成」を完全に満たすには、create_user関数内でinitialize_skill_trees_for_userを自動的に呼び出す実装が必要です。

テストケースの追加を推奨します:

def test_create_user_initializes_skill_trees(db):
    """ユーザー作成時にSkillTreeが自動生成されることを確認"""
    user = create_user(db, UserCreate(username="auto_tree_user"))
    trees = db.query(SkillTree).filter(SkillTree.user_id == user.id).all()
    assert len(trees) == 6

Copilot uses AI. Check for mistakes.
@Reeeid Reeeid force-pushed the feature/issue-31-db-models branch from be74f1c to 2d1096d Compare February 19, 2026 04:17
GitHub Copilotのレビュー指摘に基づき、以下の修正を実施:

1. Alembicマイグレーション - 主キーの冗長インデックス削除
   - 0002, 0003, 0004のマイグレーションから`ix_*_id`削除
   - PRIMARY KEY制約は自動的にインデックスを作成するため不要

2. encryption.py - スレッドセーフ対応
   - threading.Lock追加、double-checked lockingパターン実装
   - マルチスレッド環境での競合状態を防止

3. main.py - 本番環境での暗号化キー検証追加
   - ENV=productionの場合のみvalidate_encryption_key()を実行
   - テスト環境ではconftest.pyでダミーキー設定済み

4. skill_tree.py (model) - tree_dataデフォルト値修正
   - lambda: {}をnullable=Falseに変更
   - Pydanticスキーマ層でデフォルト値を設定

5. skill_tree.py (crud) - IntegrityError個別処理追加
   - initialize_skill_trees_for_user()にIntegrityError処理追加
   - UNIQUE制約違反時に明確なエラーメッセージを提供
Copy link

Copilot AI left a comment

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 49 out of 50 changed files in this pull request and generated 6 comments.

Comments suppressed due to low confidence (1)

backend/alembic/versions/0007_create_skill_trees_table.py:37

  • 0007 の downgrade() が index を落とすだけで skill_trees テーブル自体を drop していません。ロールバック不能になるので、op.drop_table("skill_trees") まで含めて完全に元に戻せるようにしてください。
def downgrade() -> None:
    op.drop_index(op.f("ix_skill_trees_user_id"), table_name="skill_trees")

Comment on lines 27 to 42
def verify_admin_key(x_admin_key: str = Header(...)) -> None:
"""管理APIキーを検証する

リクエストヘッダー `X-Admin-Key` と環境変数 `ADMIN_API_KEY` を照合。
一致しない場合は401 Unauthorizedを返す。
"""
if not settings.ADMIN_API_KEY:
raise HTTPException(
status_code=status.HTTP_500_INTERNAL_SERVER_ERROR,
detail="ADMIN_API_KEY is not configured on server",
)
if x_admin_key != settings.ADMIN_API_KEY:
raise HTTPException(
status_code=status.HTTP_401_UNAUTHORIZED,
detail="Invalid admin API key",
)
Copy link

Copilot AI Feb 19, 2026

Choose a reason for hiding this comment

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

verify_admin_key が Header(...) で必須ヘッダーになっているため、ヘッダー未指定時に 401 ではなく 422 が返ります。認証として扱うなら Header(None) にして未指定/不一致ともに 401 を返すようにし、比較は secrets.compare_digest を使ってタイミング差分も抑える実装にしてください。

Copilot uses AI. Check for mistakes.
from app.core.config import settings, validate_encryption_key
from app.api.admin import admin_app

# 本番環境のみ暗号化キーの検証を実行(テストではconftest.pyでENVが設定される)
Copy link

Copilot AI Feb 19, 2026

Choose a reason for hiding this comment

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

コメントに「テストではconftest.pyでENVが設定される」とありますが、conftest.py で設定しているのは ENCRYPTION_KEY で、ENV は設定されていません。誤解を招くので記述を実態に合わせて修正してください。

Suggested change
# 本番環境のみ暗号化キーの検証を実行(テストではconftest.pyでENVが設定される
# 本番環境のみ暗号化キーの検証を実行(テストでは conftest.py で ENCRYPTION_KEY が設定される

Copilot uses AI. Check for mistakes.
sa.Column("id", sa.Integer(), nullable=False),
sa.Column("user_id", sa.Integer(), nullable=False),
sa.Column("category", sa.String(), nullable=False),
sa.Column("tree_data", sa.JSON(), nullable=True),
Copy link

Copilot AI Feb 19, 2026

Choose a reason for hiding this comment

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

tree_data が nullable=True で作成されていますが、SQLAlchemy モデル側は tree_data を nullable=False にしており前提が不一致です。DB側も NOT NULL(必要ならデフォルト {} を持たせる)に合わせて、アプリとスキーマの整合性を取ってください。

Suggested change
sa.Column("tree_data", sa.JSON(), nullable=True),
sa.Column("tree_data", sa.JSON(), nullable=False, server_default=sa.text("'{}'")),

Copilot uses AI. Check for mistakes.
Comment on lines 20 to 23
db.commit()
db.refresh(db_user)
# ユーザー作成時に6カテゴリのSkillTreeを自動初期化
initialize_skill_trees_for_user(db, db_user.id)
Copy link

Copilot AI Feb 19, 2026

Choose a reason for hiding this comment

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

create_user() が users への commit 後に SkillTree 初期化を別トランザクションで行っており、SkillTree 側で失敗すると「ユーザーだけ作成され SkillTree が欠落した状態」が確定します。User 作成と SkillTree 6件生成は同一トランザクションで原子的に行えるよう、commit を1回にまとめる(flushで user_id を確定してから SkillTree を add し最後に commit する、もしくは db.begin() を使う)構成にしてください。

Suggested change
db.commit()
db.refresh(db_user)
# ユーザー作成時に6カテゴリのSkillTreeを自動初期化
initialize_skill_trees_for_user(db, db_user.id)
# user.id を確定させるが、まだトランザクションはコミットしない
db.flush()
# ユーザー作成時に6カテゴリのSkillTreeを自動初期化
initialize_skill_trees_for_user(db, db_user.id)
# ユーザーと SkillTree 初期化をまとめて 1 回の commit で確定させる
db.commit()
db.refresh(db_user)

Copilot uses AI. Check for mistakes.
Comment on lines 54 to 56
# Admin API Access Control
ADMIN_API_KEY: str = "test" # 本番では必ず.envで設定すること

Copy link

Copilot AI Feb 19, 2026

Choose a reason for hiding this comment

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

ADMIN_API_KEY のデフォルト値が "test" になっているため、環境変数を設定し忘れても管理APIが既知キーで有効化されてしまいます。デフォルトは空文字(または None)にして本番起動時に必須検証する(ENCRYPTION_KEY と同様に validate_* を用意する)形に変更してください。

Copilot uses AI. Check for mistakes.
Comment on lines 61 to 72
# ENCRYPTION_KEYの検証(アプリ起動時にチェック)
def validate_encryption_key() -> None:
"""ENCRYPTION_KEYが設定されているかを検証する。

未設定の場合はValueErrorを送出する。
テスト環境ではダミーキーが自動設定されるため、本番環境のみチェック。
"""
if not settings.ENCRYPTION_KEY:
raise ValueError(
"ENCRYPTION_KEY is not set. "
"Generate one with: python -c \"from cryptography.fernet import Fernet; print(Fernet.generate_key().decode())\""
)
Copy link

Copilot AI Feb 19, 2026

Choose a reason for hiding this comment

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

validate_encryption_key() が「未設定かどうか」しか見ていないため、形式不正のキーでも起動時検知できず、実際の暗号化/復号時に例外になります。ここで Fernet(key.encode()) を試して例外を握りつぶさず明示的に ValueError にする等、キー形式の検証も追加してください。

Copilot uses AI. Check for mistakes.
issue #31実装における主要な技術的意思決定を記録:

1. ADR 005: OAuth トークン暗号化戦略
   - Fernet対称暗号 + スレッドセーフ対応を採用
   - 独自実装、KMS、Vaultなどの代替案を検討・却下
   - IPA基準準拠、DB漏洩時の二次被害防止

2. ADR 006: 管理用API認証設計
   - API Key認証(環境変数)+ 独立FastAPI appを採用
   - DBテーブル、JWT、認証なしなどの代替案を検討・却下
   - ハッカソン期間中の優先度と責務分離を考慮
issue #31実装における技術的意思決定を記録:

- ADR 007: SkillTree初期化タイミング
  - ユーザー作成時に自動初期化を採用
  - 遅延初期化、手動初期化、CRON初期化などの代替案を検討・却下
  - ADR 003(トランザクション管理はCRUD関数内で完結)に準拠
  - LangChain連携時のオーバーヘッド削減、仕様準拠を優先
GitHub Copilotの再レビュー指摘6件に対応:

1. admin.py - 認証エラーを401に統一、secrets.compare_digest使用
   - Header(None)でヘッダー未指定時も401を返すように修正
   - タイミング攻撃対策としてsecrets.compare_digestを使用

2. main.py - コメント修正(ENV→ENCRYPTION_KEY)
   - conftest.pyで設定されるのはENCRYPTION_KEYであることを明記

3. 0007マイグレーション - tree_dataをNOT NULL + デフォルト値
   - nullable=False, server_default='{}'でモデルとの整合性を確保

4. user.py - db.flush()で同一トランザクション化(重要)
   - User作成とSkillTree初期化を1つのトランザクションで実行
   - SkillTree失敗時にUserも作成されない(原子性保証)

5. config.py - ADMIN_API_KEYデフォルト値を空文字に変更
   - デフォルト"test"はセキュリティリスクのため削除

6. config.py - validate_encryption_key()にキー形式検証追加
   - Fernet(key.encode())で形式不正のキーを起動時に検出
issue #31実装における技術的意思決定を記録:

- ADR 008: Alembicマイグレーション粒度
  - テーブル単位でマイグレーションを分離(6本)
  - 一括マイグレーション、機能単位グループ化などの代替案を検討・却下
  - ロールバック粒度の細かさ、デバッグ容易性を優先
Reeeid and others added 2 commits February 19, 2026 21:13
- User.skillsカラム削除(SkillTreeテーブルに移行)
- マイグレーションを7本から1本に統合(issue単位一括)
- ADR 008更新: MVP優先でissue単位一括マイグレーションに変更

理由:
- SkillTreeテーブル実装によりskillsカラムは冗長
- 部分的ロールバックの必要性が薄く、all-or-nothingが適切
- マイグレーションの本質(環境同期)に集中

Co-Authored-By: Claude Sonnet 4.5 <[email protected]>
プロジェクト全体のDB設計方針を明確化するため、ADR 009を作成:

- 正規化の程度: 第3正規形(3NF)を基本とし、パフォーマンスのためのキャッシュカラムは許容
- 型選択基準: JSON vs JSONB(SkillTreeはJSON型 = 全体読み込みのため)、Enum vs 整数、Text vs String
- 外部キー制約: すべてのFK関係に外部キー制約を使用、ON DELETE CASCADEは使用しない
- インデックス戦略: 主キー、外部キー、UNIQUE制約のみ
- SQLite vs PostgreSQL互換性: JSON型使用、ORM中心でDB固有機能を避ける
- NULL許容とデフォルト値、タイムスタンプ管理の方針
@Inlet-back Inlet-back merged commit f227cac into develop Feb 19, 2026
4 checks passed
Neon-straySheep added a commit that referenced this pull request Feb 19, 2026
## 概要
ユーザーが学びたい技術のドキュメントを入力すると、
LLMを使用してランクに見合った実践的なハンズオン演習を自動生成する機能を実装。
概念実証を優先し、まずは動くことを重視。

## 実装内容

### 1. プロンプトテンプレート拡張
- `app/core/prompts.py`: QUEST_GENERATION_TEMPLATEを詳細化
  - ランク別の難易度設定ガイドライン
  - JSON出力形式の厳密な指定
  - ステップ構成の要件定義

### 2. サービス層実装
- `app/services/quest_service.py`: generate_handson_quest関数
  - LLM呼び出し (temperature=0.7で創造性を持たせる)
  - JSONパース+エラーハンドリング
  - デフォルト値フォールバック

### 3. スキーマ定義拡張
- `app/schemas/quest.py`: QuestGenerationRequest/Response追加
  - QuestStep: 演習ステップの詳細定義
  - リクエストバリデーション (document_content: 10KB制限)
  - Swagger UIのExample付き

### 4. APIエンドポイント実装
- `app/api/endpoints/quest.py`: POST /api/v1/quest/generate
  - リクエスト受け取り
  - サービス呼び出し
  - エラーハンドリング

### 5. ルーター登録
- `app/api/api.py`: questルーターを統合

### 6. テスト実装
- `tests/test_services/test_quest_service.py`: サービス層テスト
- `tests/test_api/test_quest.py`: APIエンドポイントテスト
  - LLM呼び出しをモック化してCI環境対応
  - 正常系・エラー系・バリデーションをカバー

## MVP制限(概念実証優先)
- ❌ DB保存なし: 生成された演習はAPIレスポンスのみ
- ❌ 進捗管理なし: 演習の完了状態やスコアは後続Issue
- ⚠️ 生成品質: プロンプト調整は後続Issue

## セキュリティ考慮
- Request Bodyサイズ制限(document_content: 10KB)
- LLMレスポンスのサニタイズ(JSONパースエラー時のフォールバック)
- エラーメッセージの安全な返却

## テスト要件
- モック使用でAPI キー不要(CI環境対応)
- 正常系・エラー系・バリデーションを網羅

## 完了条件(DoD)
- [x] POST /api/v1/quest/generate実装完了
- [x] スキーマ定義とバリデーション実装
- [x] テスト5件以上実装(モック使用)
- [x] エラーハンドリング実装

## 依存
- Issue #32(AI基盤セットアップ)✅
- Issue #36(ランク判定AI)✅

## 次のステップ(後続Issue)
- Issue #41: 演習のDB保存機能
- Issue #42: 演習進捗管理機能
- Issue #43: 演習生成品質向上
- Issue #44: 生成コードのセキュリティチェック
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.

Backend: DB設計とモデル実装 (Rankマスタ/JSON対応)

2 participants

Comments