[EDMT-458] 포인트 히스토리 관리 테이블 추가 #79
Merged
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
…ns with history records
…prove query performance with indexes
Walkthrough포인트 기능을 Changes
Sequence Diagram(s)sequenceDiagram
actor Client
participant AIFacade as StudentRecordAIFacade
participant MemberSvc as MemberService
participant PointSvc as PointService
participant PointHistRepo as PointHistoryRepository
participant DB as Database
Client->>AIFacade: createTaskId(recordId, memberId)
rect rgb(240,240,255)
note over AIFacade,MemberSvc: 멤버 조회 (동시성 고려)
AIFacade->>MemberSvc: getMemberById(memberId)
MemberSvc-->>AIFacade: Member
end
rect rgb(240,255,240)
note over AIFacade: AI 태스크 생성
AIFacade->>AIFacade: createAITask() -> taskId
end
rect rgb(255,245,230)
note over AIFacade,PointSvc: 포인트 차감 (신규 흐름)
AIFacade->>PointSvc: deductPoints(memberId, points, taskId)
PointSvc->>PointHistRepo: save(PointHistory(DEDUCT, amount, taskId))
PointHistRepo->>DB: INSERT point_history
DB-->>PointHistRepo: OK
PointSvc-->>AIFacade: void
end
AIFacade-->>Client: taskId
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 분 특별 주의 필요 영역:
Possibly related PRs
Suggested reviewers
Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✨ Finishing touches🧪 Generate unit tests (beta)
📜 Recent review detailsConfiguration used: CodeRabbit UI Review profile: CHILL Plan: Pro 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
Comment |
kgy1008
changed the title
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Nitpick comments (8)
edukit-core/src/main/java/com/edukit/core/point/db/repository/PointHistoryRepository.java (1)
1-7: 기본 CRUD 시작은 OK. 조회 패턴에 맞춘 파생 메서드와 페이지네이션을 미리 노출하는 것을 권장합니다.AI 과금/보상 흐름을 고려하면 다음 조회가 자주 필요합니다:
- taskId+transactionType로 최근 1건 조회(보상 중복 방지)
- memberId 기준 최신 이력 페이징 정렬(createdAt desc)
예시: findTopByTaskIdAndTransactionTypeOrderByCreatedAtDesc(...), Page findByMemberIdOrderByCreatedAtDesc(...). 서비스 레이어에서 Pageable 기반 API를 노출하면 추후 목록 화면/관리자 조회에 재사용성이 높습니다. 마이그레이션에서 인덱스(task_id, member_id, created_at)를 이미 추가했다면 위 쿼리와 시너지가 납니다.
장기적 데이터 증가에 대비해 보관기간/아카이빙 전략(예: 파티셔닝 또는 백필+압축 테이블)도 미리 고려해 주세요.
edukit-core/src/main/java/com/edukit/core/studentrecord/db/enums/AITaskStatus.java (1)
3-8: 상태 분기 중복을 줄이기 위해 헬퍼 메서드 추가를 권장합니다.상태 체크가 여러 곳에 생길 수 있어 isTerminal()/isInProgressLike() 등을 제공하면 가독성과 안정성이 좋아집니다.
아래처럼 간단히 확장 가능합니다:
public enum AITaskStatus { PENDING, // 생성됨 IN_PROGRESS, // SQS 전송됨, Lambda 처리 중 COMPLETED, // 성공 - FAILED // 실패 (보상 트랜잭션 대상) + FAILED; // 실패 (보상 트랜잭션 대상) + + public boolean isTerminal() { + return this == COMPLETED || this == FAILED; + } + + public boolean isInProgressLike() { + return this == PENDING || this == IN_PROGRESS; + } }edukit-core/src/main/java/com/edukit/core/point/exception/PointException.java (1)
5-9: 얇은 래퍼로 충분하지만, 원인 전파용 생성자 추가를 고려해 주세요.운영 중 원인 추적을 위해 cause/추가 메세지 전달 오버로드가 있으면 유용합니다(전제: BusinessException이 해당 시그니처를 지원).
BusinessException 생성자 시그니처를 확인해 주실 수 있을까요? 지원한다면 아래와 같은 오버로드를 추가하는 것을 권장합니다(예시):
public PointException(final PointErrorCode errorCode, final Throwable cause) { super(errorCode, cause); } public PointException(final PointErrorCode errorCode, final String detail) { super(errorCode, detail); }edukit-core/src/main/java/com/edukit/core/point/db/enums/PointTransactionType.java (1)
3-9: 필수 확인 사항이 이미 완료되었습니다PointHistory.java의 35번 줄에서
@Enumerated(EnumType.STRING)이 이미 적용되어 있으므로 ORDINAL로 인한 데이터 오염 위험이 없습니다. ✅
권장: 잔액 증감을 명확히 하는 enum 메서드 추가
각 트랜잭션 타입이 잔액을 증가/감소시키는지가 명시적으로 정의되지 않아 서비스 로직에서 실수 위험이 있습니다. 다음과 같이 헬퍼 메서드를 추가하면 도메인 규칙을 캡슐화할 수 있습니다:
public enum PointTransactionType { CHARGE, // 충전 (결제 등) DEDUCT, // 차감 (AI 생성 등) REFUND, // 환불 COMPENSATION, // 보상 (실패 시 복구) - ADMIN_ADJUST // 관리자 조정 -} + ADMIN_ADJUST; // 관리자 조정 + + public boolean increasesBalance() { + return this == CHARGE || this == REFUND || this == COMPENSATION; + } + + public boolean decreasesBalance() { + return this == DEDUCT; + } +}edukit-api/src/main/java/com/edukit/studentrecord/facade/StudentRecordAIFacade.java (2)
44-48: 이벤트는 트랜잭션 커밋 이후에 발행하세요.현재 즉시 발행이라 롤백 시 외부 구독자가 롤백 전 상태를 관측할 수 있습니다.
@TransactionalEventListener(phase = AFTER_COMMIT)또는TransactionSynchronizationManager#afterCommit로 전환을 권장합니다.
32-32: 하드코딩된 차감 포인트의 설정화.
DEDUCTED_POINTS = 100을 설정값(예:application.yml)으로 이동해 운영 중 조정 가능하도록 하세요.edukit-core/src/main/java/com/edukit/core/point/db/entity/PointHistory.java (2)
1-24: 매핑 명시화로 네이밍 전략 의존성 제거.스키마(
point_history)와의 1:1 매핑을 명시해 향후 네이밍 전략 변경 리스크를 제거하세요.적용 diff:
import jakarta.persistence.JoinColumn; import jakarta.persistence.ManyToOne; +import jakarta.persistence.Table; @@ -@Entity +@Entity +@Table(name = "point_history") public class PointHistory extends BaseTimeEntity {
35-43: 컬럼명 명시 및 도메인 검증(양수 금지) 추가.
- DB 컬럼명과의 괴리를 방지하려면
transaction_type,task_id를 명시하세요.amount는 양수만 허용하도록 팩토리에서 방어로직을 추가하세요.적용 diff:
- @Enumerated(EnumType.STRING) - @Column(nullable = false) + @Enumerated(EnumType.STRING) + @Column(name = "transaction_type", nullable = false) private PointTransactionType transactionType; @@ - @Column + @Column(name = "task_id") private Long taskId; @@ - public static PointHistory create(final Member member, final PointTransactionType transactionType, - final int amount, final Long taskId) { + public static PointHistory create(final Member member, final PointTransactionType transactionType, + final int amount, final Long taskId) { + if (amount <= 0) { + throw new IllegalArgumentException("amount must be positive"); + } return PointHistory.builder() .member(member) .transactionType(transactionType) .amount(amount) .taskId(taskId) .build(); }Also applies to: 54-61
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (11)
edukit-api/src/main/java/com/edukit/studentrecord/facade/StudentRecordAIFacade.java(2 hunks)edukit-api/src/main/resources/db/migration/V11__Add_point_history_table.sql(1 hunks)edukit-core/src/main/java/com/edukit/core/member/exception/MemberErrorCode.java(1 hunks)edukit-core/src/main/java/com/edukit/core/member/service/PointService.java(0 hunks)edukit-core/src/main/java/com/edukit/core/point/db/entity/PointHistory.java(1 hunks)edukit-core/src/main/java/com/edukit/core/point/db/enums/PointTransactionType.java(1 hunks)edukit-core/src/main/java/com/edukit/core/point/db/repository/PointHistoryRepository.java(1 hunks)edukit-core/src/main/java/com/edukit/core/point/exception/PointErrorCode.java(1 hunks)edukit-core/src/main/java/com/edukit/core/point/exception/PointException.java(1 hunks)edukit-core/src/main/java/com/edukit/core/point/service/PointService.java(1 hunks)edukit-core/src/main/java/com/edukit/core/studentrecord/db/enums/AITaskStatus.java(1 hunks)
💤 Files with no reviewable changes (1)
- edukit-core/src/main/java/com/edukit/core/member/service/PointService.java
🧰 Additional context used
🧬 Code graph analysis (1)
edukit-core/src/main/java/com/edukit/core/point/service/PointService.java (3)
edukit-core/src/main/java/com/edukit/core/member/exception/MemberException.java (1)
MemberException(6-11)edukit-core/src/main/java/com/edukit/core/point/exception/PointException.java (1)
PointException(5-10)edukit-api/src/main/java/com/edukit/studentrecord/facade/StudentRecordAIFacade.java (1)
Service(21-63)
🔇 Additional comments (3)
edukit-core/src/main/java/com/edukit/core/member/exception/MemberErrorCode.java (1)
11-18: 리팩터링이 올바르게 완료되었습니다. 잔존 참조 없음.검증 결과 MemberErrorCode에서 제거된 INSUFFICIENT_POINTS는 PointErrorCode로 깔끔하게 이관되었고, PointService에서 이미 올바른 위치(PointErrorCode.INSUFFICIENT_POINTS)를 참조하고 있습니다. 기존 오류 코드 문자열(M-40009) 또는 레거시 참조도 없어 리팩터링이 완벽하게 완료된 상태입니다.
edukit-core/src/main/java/com/edukit/core/point/exception/PointErrorCode.java (1)
7-14: 문제 없음 (LGTM).
ErrorCode계약 충족, Lombok 사용 일관적입니다. 현재 범위에선 추가 조치 불필요합니다.edukit-core/src/main/java/com/edukit/core/point/service/PointService.java (1)
23-38: 입력 검증은 필요하나, 제시된 멱등성 로직은 재검토 필요.제시된
existsByTransactionTypeAndTaskId()메서드가 PointHistoryRepository에 존재하지 않으며, 제안된 diff에서도 레포지토리 메서드 구현이 포함되지 않았습니다. 또한 PointHistory 스키마에서taskId가 nullable이므로, taskId가 null인 경우 제시된 멱등성 검사가 작동하지 않습니다.
- ✅
pointsToDeduct <= 0입력 검증은 필수입니다(차감이 가산으로 동작하는 문제 방지).⚠️ 멱등성 가드 로직을 다시 검토하세요:
- taskId 필드가 nullable인 점을 고려하여, 중복 차감 방지 전략을 재설계하세요.
- taskId 기반 유일성 보장이 필요하다면, PointHistory 엔티티에 고유 제약 조건 추가 여부와 레포지토리 쿼리 메서드를 함께 정의하세요.
edukit-api/src/main/resources/db/migration/V11__Add_point_history_table.sql
Show resolved
Hide resolved
edukit-api/src/main/resources/db/migration/V11__Add_point_history_table.sql
Outdated
Show resolved
Hide resolved
Comment on lines
+40
to
+52
| @Transactional | ||
| public Member compensatePoints(final Long memberId, final int pointsToCompensate, final Long taskId) { | ||
| Member member = memberRepository.findByIdWithLock(memberId) | ||
| .orElseThrow(() -> new MemberException(MemberErrorCode.MEMBER_NOT_FOUND)); | ||
|
|
||
| member.deductPoints(-pointsToCompensate); // 음수 차감으로 복구 | ||
|
|
||
| // 포인트 히스토리 기록 | ||
| PointHistory history = PointHistory.create(member, PointTransactionType.COMPENSATION, pointsToCompensate, taskId); | ||
| pointHistoryRepository.save(history); | ||
|
|
||
| return member; | ||
| } |
There was a problem hiding this comment.
보상 멱등성/검증 보강 및 API 일관성.
pointsToCompensate <= 0검증이 없어 음수 입력 시 추가 차감될 수 있습니다.- 동일
taskId에 대한 중복 보상 방지 가드 필요(재시도 시 포인트 과지급 위험). - 반환 타입이 차감(void)과 보상(Member)로 불일치합니다(선택) — 일관성 고려.
적용 diff 제안:
- public Member compensatePoints(final Long memberId, final int pointsToCompensate, final Long taskId) {
+ public Member compensatePoints(final long memberId, final int pointsToCompensate, final Long taskId) {
+ if (pointsToCompensate <= 0) {
+ throw new IllegalArgumentException("pointsToCompensate must be positive");
+ }
+ if (taskId != null
+ && pointHistoryRepository.existsByTransactionTypeAndTaskId(PointTransactionType.COMPENSATION, taskId)) {
+ // 멱등 처리: 이미 보상된 작업이면 재보상하지 않음
+ return memberRepository.findByIdWithLock(memberId)
+ .orElseThrow(() -> new MemberException(MemberErrorCode.MEMBER_NOT_FOUND));
+ }
Member member = memberRepository.findByIdWithLock(memberId)
.orElseThrow(() -> new MemberException(MemberErrorCode.MEMBER_NOT_FOUND));
- member.deductPoints(-pointsToCompensate); // 음수 차감으로 복구
+ member.deductPoints(-pointsToCompensate); // NOTE: 가능하면 add/increase 메서드로 대체 권장추가로, 가능하다면 Member.addPoints(int)와 같은 명시적 가산 메서드 도입을 권장합니다.
Committable suggestion skipped: line range outside the PR's diff.
🤖 Prompt for AI Agents
In edukit-core/src/main/java/com/edukit/core/point/service/PointService.java
around lines 40–52, add validation and idempotency checks and replace the
negative-deduction hack: verify pointsToCompensate > 0 and throw a
MemberException for invalid input; check pointHistoryRepository (or repository
method) for an existing COMPENSATION record with the same taskId and memberId
and return the current member or throw a specific exception to prevent duplicate
compensation; replace member.deductPoints(-pointsToCompensate) with a clear
member.addPoints(pointsToCompensate) call (introduce addPoints on Member if
needed) and keep returning Member (or change both related APIs to void for
consistency—prefer returning Member here to match this method) while preserving
@Transactional and saving the new PointHistory only when no duplicate exists.
…nd idempotency - Add composite index (member_id, created_at DESC) for efficient member history queries - Add unique index (task_id, transaction_type) for idempotency enforcement - Prevent duplicate transactions for same AI task (MySQL NULL handling allows CHARGE duplicates) - Remove redundant single-column indexes 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <noreply@anthropic.com>
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
edukit-api/src/main/resources/db/migration/V11__Add_point_history_table.sql (1)
21-22: 선택사항: task_id 단일 인덱스 재고 고려.
UQ_point_history_task_transaction유니크 인덱스(라인 19)가 이미(task_id, transaction_type)복합 인덱스로 존재하므로, 라인 22의IDX_point_history_task_id는 task_id 단일 조회에서 추가 성능 이점이 제한적입니다. 쿼리 패턴 분석 후 불필요하면 제거하거나, task_id만으로 필터링하는 조회가 자주 발생한다면 유지해도 무방합니다.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
edukit-api/src/main/resources/db/migration/V11__Add_point_history_table.sql(1 hunks)
| point_history_id bigint auto_increment primary key, | ||
| member_id bigint not null, | ||
| transaction_type enum ('CHARGE', 'DEDUCT', 'REFUND', 'COMPENSATION', 'ADMIN_ADJUST') not null, | ||
| amount int not null, |
There was a problem hiding this comment.
데이터 무결성 강화: amount 제약과 FK 삭제 정책을 명시하세요.
현재 스키마에 두 가지 중요한 제약이 누락되어 있습니다:
- amount 양수 제약 부재: 서비스 입력 검증 실패 시 음수 기록이 발생할 수 있어 데이터 오염 위험이 있습니다.
- FK ON DELETE 정책 부재: 멤버 삭제 시 히스토리 보존/제약 의도를 명시하지 않아 미정의 동작 초래.
다음 diff를 적용하여 스키마 수준의 보호를 추가하세요:
constraint FK_point_history_member foreign key (member_id) references member (member_id)
+ on delete restrict
);
+
+-- amount 양수 제약 (서비스 검증 실패 시 대비)
+alter table point_history
+ add constraint CHK_point_history_amount_positive check (amount > 0);Also applies to: 11-11
🤖 Prompt for AI Agents
In edukit-api/src/main/resources/db/migration/V11__Add_point_history_table.sql
around line 7, add a schema-level CHECK to enforce amount > 0 and make the
foreign key explicit about delete behavior: change the member FK to use ON
DELETE SET NULL (and alter member_id to be nullable) so history is preserved
when a member is removed, or alternatively use ON DELETE RESTRICT if you want to
prevent member deletion; implement the chosen policy and add the CHECK
constraint for amount to ensure only positive amounts are allowed.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
📣 Jira Ticket
EDMT-458
👩💻 작업 내용
주요 변경사항
구현 세부사항
PointHistory엔티티: Member와 ManyToOne 관계, 불변 데이터로 설계PointTransactionTypeenum: 5가지 트랜잭션 타입 정의PointService개선:deductPoints(): taskId와 함께 차감 히스토리 기록compensatePoints(): 실패 시 보상 히스토리 기록패키지 구조 개선
📝 리뷰 요청 & 논의하고 싶은 내용
Test Plan
✅ Manual Testing Checklist
🧪 Automated Tests
./gradlew test./gradlew build🔍 Code Quality
Deployment Notes
🤖 Generated with Claude Code
Summary by CodeRabbit
새로운 기능
개선사항
주의