[Feat] McpAccessToken 엔티티 및 Flyway 마이그레이션 추가 - #8
Conversation
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
📝 WalkthroughWalkthroughmcp_access_tokens 테이블을 생성하는 새 데이터베이스 마이그레이션(V26)과, 이 테이블에 매핑되는 McpAccessToken JPA 엔티티가 추가되었습니다. 엔티티는 사용자 참조, 토큰 해시, 생성/사용/폐기 시각 필드와 토큰 사용 기록, 폐기 처리, 폐기 여부 확인 메서드를 제공합니다. ChangesMCP 액세스 토큰 관리
Estimated code review effort: 2 (Simple) | ~10 minutes 버그 관점에서 보면 recordUsage/revoke 메서드가 파라미터로 시각을 그대로 받아 저장하는 구조라 호출 측에서 잘못된 시각(예: 과거 시각)을 넘겨도 검증 로직이 없어 보이는데, 엔티티 내부에서 now() 자체를 사용하도록 캡슐화하면 클린코드 관점에서 더 안전할 듯 합니다. 그 외 스키마와 엔티티는 단순 매핑이라 SOLID 위반이나 성능 이슈는 눈에 띄지 않네요. 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Comment |
There was a problem hiding this comment.
🧹 Nitpick comments (2)
src/main/resources/db/migration/V26__create_mcp_access_tokens.sql (1)
5-11: 🚀 Performance & Scalability | 🔵 Trivial | ⚡ Quick win
token_hash조회 성능 및 무결성 확보 필요토큰 검증 로직(후속 작업)에서는
token_hash로 레코드를 조회할 가능성이 높습니다. 현재user_id에만 인덱스가 있고token_hash에는 인덱스나 유니크 제약이 없어, 토큰 검증 시 풀스캔이 발생하고 해시 충돌 시 중복 저장을 막을 방법이 없습니다.♻️ 제안: 유니크 인덱스 추가
CREATE INDEX idx_mcp_access_tokens_user_id ON mcp_access_tokens (user_id); +CREATE UNIQUE INDEX uk_mcp_access_tokens_token_hash ON mcp_access_tokens (token_hash);🤖 Prompt for 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. In `@src/main/resources/db/migration/V26__create_mcp_access_tokens.sql` around lines 5 - 11, The mcp_access_tokens table definition currently indexes only user_id, so token_hash lookups may be slow and duplicate hashes are not prevented. Update the V26__create_mcp_access_tokens.sql migration to add a unique index or unique constraint on token_hash alongside the existing mcp_access_tokens table definition, keeping the current user_id index intact, so the later token validation flow can query by token_hash efficiently and enforce hash uniqueness.src/main/java/com/opensource/docgrid/domain/user/entity/McpAccessToken.java (1)
52-65: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low value시각 처리 방식 일관성 개선 제안
생성자는 내부적으로
LocalDateTime.now()를 사용하는데,recordUsage/revoke는 호출자가 시각을 직접 넘기도록 되어 있습니다. 호출자가 잘못된 시각을 전달하면created_at보다 이른last_used_at/revoked_at이 저장될 수 있어 일관성이 떨어집니다.♻️ 제안: 엔티티 내부에서 시각 결정
- public void recordUsage(LocalDateTime usedAt) { - this.lastUsedAt = usedAt; + public void recordUsage() { + this.lastUsedAt = LocalDateTime.now(); } - public void revoke(LocalDateTime revokedAt) { - this.revokedAt = revokedAt; + public void revoke() { + this.revokedAt = LocalDateTime.now(); }테스트 등에서 특정 시각 주입이 필요하다면 현재 방식이 의도된 설계일 수도 있으니, 그 경우라면 무시해도 됩니다.
🤖 Prompt for 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. In `@src/main/java/com/opensource/docgrid/domain/user/entity/McpAccessToken.java` around lines 52 - 65, McpAccessToken has inconsistent time handling because the constructor sets createdAt internally with LocalDateTime.now() while recordUsage and revoke accept arbitrary timestamps from callers. Update the McpAccessToken methods so the entity controls its own timestamps consistently, either by setting lastUsedAt/revokedAt inside recordUsage/revoke using the current time or by aligning the constructor and lifecycle methods to a single time-source approach. Use the McpAccessToken constructor, recordUsage, and revoke as the reference points when making the change.
🤖 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.
Nitpick comments:
In `@src/main/java/com/opensource/docgrid/domain/user/entity/McpAccessToken.java`:
- Around line 52-65: McpAccessToken has inconsistent time handling because the
constructor sets createdAt internally with LocalDateTime.now() while recordUsage
and revoke accept arbitrary timestamps from callers. Update the McpAccessToken
methods so the entity controls its own timestamps consistently, either by
setting lastUsedAt/revokedAt inside recordUsage/revoke using the current time or
by aligning the constructor and lifecycle methods to a single time-source
approach. Use the McpAccessToken constructor, recordUsage, and revoke as the
reference points when making the change.
In `@src/main/resources/db/migration/V26__create_mcp_access_tokens.sql`:
- Around line 5-11: The mcp_access_tokens table definition currently indexes
only user_id, so token_hash lookups may be slow and duplicate hashes are not
prevented. Update the V26__create_mcp_access_tokens.sql migration to add a
unique index or unique constraint on token_hash alongside the existing
mcp_access_tokens table definition, keeping the current user_id index intact, so
the later token validation flow can query by token_hash efficiently and enforce
hash uniqueness.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro Plus
Run ID: fb1762c6-c4e6-40da-baf5-5dd81c26a8dd
📒 Files selected for processing (2)
src/main/java/com/opensource/docgrid/domain/user/entity/McpAccessToken.javasrc/main/resources/db/migration/V26__create_mcp_access_tokens.sql
🔍 작업 내용
✨ 상세 설명
🛠 추후 리팩토링 및 고도화 계획
📸 스크린샷 (선택)
💬 리뷰 요구사항
Summary by CodeRabbit