Conversation
Contributor
Walkthrough배지 도메인, 저장소, 서비스, API/컨트롤러, 이벤트 리스너와 DTO·테스트를 포함한 완전한 배지 시스템을 추가하고, 학습 종료 이벤트 기반 배지 지급 및 시즌 종료 시 랭킹 배지 자동 지급을 통합합니다. Changes
Sequence DiagramsequenceDiagram
participant Client
participant StudySessionService
participant EventPublisher
participant StudyBadgeGrantEventListener
participant BadgeService
participant Repositories as "BadgeRepo / UserBadgeRepo / StatsRepo"
Client->>StudySessionService: endStudySession(userId)
StudySessionService->>StudySessionService: 세션 상태 FINISHED 업데이트
StudySessionService->>EventPublisher: publishEvent(StudySessionEndedEvent(userId))
EventPublisher->>StudyBadgeGrantEventListener: handleStudySessionEnded(event) (afterCommit)
StudyBadgeGrantEventListener->>BadgeService: grantStudyAchievementBadges(userId)
BadgeService->>Repositories: 총 학습 시간 / 연속 공부일 등 조회
BadgeService->>Repositories: 배지 조회 및 UserBadge 저장/업데이트
BadgeService->>Client: (신규 배지 발생 시) NewBadgeResponse 반환 흐름
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs
Suggested labels
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 1 | ❌ 2❌ Failed checks (1 warning, 1 inconclusive)
✅ Passed checks (1 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches
🧪 Generate unit tests (beta)
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. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 15
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (4)
src/main/java/com/gpt/geumpumtabackend/user/domain/User.java (1)
16-25:⚠️ Potential issue | 🔴 Critical소프트 삭제 및 배지 삭제 가드 로직 모두 불완전 — 데이터 무결성 및 비즈니스 규칙 위반
두 가지 문제가 발견되었습니다:
@SQLDelete에서
representative_badge_id미초기화: User 소프트 삭제 시representative_badge_id필드가NULL로 설정되지 않습니다. 이로 인해 soft-deleted 사용자가 여전히 배지를 참조하는 상태가 남아 데이터 무결성이 손상됩니다.
BadgeService.deleteBadge()에서representativeBadgeId가드 누락: PR 설명의 "다른 사용자가 대표로 사용 중인 배지는 삭제 불가" 규칙이 구현되지 않았습니다. 현재 코드는UserBadge테이블만 확인하고,User.representativeBadgeId참조는 전혀 검사하지 않습니다.🛡️ 수정 제안
1단계: User.java의
@SQLDelete에representative_badge_id 초기화 추가`@SQLDelete`(sql = """ UPDATE `user` SET deleted_at = NOW(), email = CONCAT('deleted_', email), school_email= CONCAT('deleted_', school_email), nickname = CONCAT('deleted_', nickname), student_id = CONCAT('deleted_', student_id), - fcm_token = NULL + fcm_token = NULL, + representative_badge_id = NULL WHERE id = ? """)2단계: BadgeService.deleteBadge()에 representativeBadgeId 가드 추가
`@Transactional` public void deleteBadge(Long badgeId) { Badge badge = badgeRepository.findById(badgeId) .orElseThrow(() -> new BusinessException(ExceptionType.BADGE_NOT_FOUND)); if (userBadgeRepository.existsByBadgeId(badgeId)) { throw new BusinessException(ExceptionType.BADGE_IN_USE); } + + if (userRepository.existsByRepresentativeBadgeIdAndDeletedAtIsNull(badgeId)) { + throw new BusinessException(ExceptionType.BADGE_IN_USE); + } badgeRepository.delete(badge); }UserRepository에 쿼리 메서드 추가 필요:
boolean existsByRepresentativeBadgeIdAndDeletedAtIsNull(Long badgeId);🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/main/java/com/gpt/geumpumtabackend/user/domain/User.java` around lines 16 - 25, The soft-delete SQL in User.java fails to null out representative_badge_id and BadgeService.deleteBadge() omits the guard that prevents deleting a badge still referenced as a user's representative; update the `@SQLDelete` block in the User entity to set representative_badge_id = NULL during soft delete, and add a check in BadgeService.deleteBadge() that uses a repository method (add boolean existsByRepresentativeBadgeIdAndDeletedAtIsNull(Long badgeId) to UserRepository) to refuse deletion when any non-deleted User.representativeBadgeId equals the target badgeId, returning/throwing the existing domain error path if the guard triggers.src/main/java/com/gpt/geumpumtabackend/study/service/StudySessionService.java (1)
28-31: 🛠️ Refactor suggestion | 🟠 Major클래스 레벨
@Transactional(readOnly = true)누락 — 코딩 가이드라인 위반
**/*Service.java가이드라인에 따라 서비스 클래스에는@Transactional(readOnly = true)를 클래스 레벨에 적용하고, 쓰기 메서드에만@Transactional을 추가해야 합니다. 현재getTodayStudySession등 읽기 전용 메서드에는 트랜잭션이 없습니다.♻️ 제안된 수정
`@Service` `@RequiredArgsConstructor` `@Slf4j` +@Transactional(readOnly = true) public class StudySessionService {As per coding guidelines, "Use class-level
@Transactional(readOnly = true) and add@Transactionalto write methods."🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/main/java/com/gpt/geumpumtabackend/study/service/StudySessionService.java` around lines 28 - 31, Add class-level `@Transactional`(readOnly = true) to StudySessionService so all query methods (e.g., getTodayStudySession) run in read-only transactions, and annotate each write method that mutates state (any method that calls repository.save, delete, or performs updates — e.g., create/update/delete-like methods in StudySessionService) with `@Transactional` (without readOnly) to enable commits; update imports if needed and ensure method-level `@Transactional` overrides the class-level readOnly.src/main/java/com/gpt/geumpumtabackend/user/service/UserService.java (1)
64-78:⚠️ Potential issue | 🟠 Major배지 지급 실패 시 회원가입 트랜잭션 전체 롤백 위험
completeRegistration은@Transactional로 묶여 있고,badgeService.grantWelcomeBadge(userId)는 메서드 레벨@Transactional(전파 수준 미지정 = PROPAGATION_REQUIRED)로 동일 트랜잭션 경계에 참여합니다.grantWelcomeBadge에서 예외가 발생하면(예:BADGE_NOT_FOUND, 데이터베이스 오류)user.completeRegistration(request)및 토큰 발급까지 모두 롤백되어, 사용자가 회원가입 자체를 완료하지 못하게 됩니다.배지 지급은 회원가입의 필수 기능이 아니므로, 배지 서비스 장애로 인해 등록이 실패하는 것은 부적절합니다.
권장 해결책:
try-catch로 배지 지급 실패를 격리하거나,REQUIRES_NEW전파 수준으로 별도 트랜잭션에서 실행하세요.🛡️ 제안된 수정 (try-catch로 배지 실패 격리)
TokenResponse tokenResponse = TokenResponse.to(token); - NewBadgeResponse newBadge = badgeService.grantWelcomeBadge(userId); + NewBadgeResponse newBadge = null; + try { + newBadge = badgeService.grantWelcomeBadge(userId); + } catch (Exception e) { + log.warn("환영 배지 지급 실패 (userId={}): {}", userId, e.getMessage()); + } return CompleteRegistrationResponse.of(tokenResponse, newBadge);🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/main/java/com/gpt/geumpumtabackend/user/service/UserService.java` around lines 64 - 78, completeRegistration is transactional and calls badgeService.grantWelcomeBadge(userId) which runs in the same transaction, so badge failures can roll back the whole registration; isolate badge granting by wrapping the grantWelcomeBadge call in a try-catch (catching BusinessException and runtime exceptions) and log/ignore badge errors so registration proceeds, or change the badge service method or its invocation to execute in a new transaction (PROPAGATION_REQUIRES_NEW) so failures there do not roll back UserService.completeRegistration; update references in UserService.completeRegistration and/or BadgeService.grantWelcomeBadge accordingly.src/test/java/com/gpt/geumpumtabackend/unit/user/service/UserServiceTest.java (1)
41-43: 🛠️ Refactor suggestion | 🟠 Major
BaseUnitTest를 상속하지 않고@ExtendWith(MockitoExtension.class)를 직접 사용합니다.코딩 가이드라인에 따르면 단위 테스트 클래스는
BaseUnitTest를 상속해야 합니다.♻️ 수정 제안
-import org.junit.jupiter.api.extension.ExtendWith; -import org.mockito.junit.jupiter.MockitoExtension; +import com.gpt.geumpumtabackend.BaseUnitTest; -@ExtendWith(MockitoExtension.class) `@DisplayName`("UserService 단위 테스트") -class UserServiceTest { +class UserServiceTest extends BaseUnitTest {Based on learnings: "Write unit tests using JUnit 5 + Mockito + AssertJ, extending
BaseUnitTest"🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/test/java/com/gpt/geumpumtabackend/unit/user/service/UserServiceTest.java` around lines 41 - 43, The test class UserServiceTest currently uses `@ExtendWith`(MockitoExtension.class) directly; change it to extend BaseUnitTest instead (i.e., remove the `@ExtendWith` annotation and make UserServiceTest extend BaseUnitTest) so it follows the project test base pattern; update imports to remove MockitoExtension and add any BaseUnitTest import if missing, and ensure existing annotations like `@DisplayName`("UserService 단위 테스트") and Mockito usages inside UserServiceTest remain valid under BaseUnitTest.
🧹 Nitpick comments (9)
src/main/java/com/gpt/geumpumtabackend/user/domain/User.java (2)
66-67:representativeBadgeId에 DB 수준 FK 제약 없음
Long타입으로 저장하면 JPA 관계 매핑 없이 애플리케이션 레벨에서만 참조 무결성을 보장합니다. DB 레벨의 FK 제약이 없으므로 서비스 레이어의 가드 로직을 우회하면 참조 무결성이 깨질 수 있습니다. 설계상 의도된 loose coupling이라면 주석으로 이유를 명시하는 것을 권장합니다.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/main/java/com/gpt/geumpumtabackend/user/domain/User.java` around lines 66 - 67, The field representativeBadgeId in class User is stored as a raw Long with no DB-level foreign key, risking referential integrity; either convert it to a proper JPA relation (e.g., change representativeBadgeId to a Badge typed field like representativeBadge with `@ManyToOne` and `@JoinColumn` plus an explicit foreignKey name such as fk_user_representative_badge) so the DB enforces FK constraints, or if loose coupling is intentional, add a clear comment above representativeBadgeId documenting the design decision and why DB-level FK was omitted; adjust service/repository code accordingly to use the new relation or to handle the documented loose-coupling semantics.
112-114: 도메인 메서드에 JavaBeanset접두사 사용 — 이름 불일치
setRepresentativeBadge(Long badgeId)는 메서드명은Badge객체를 받는 것처럼 보이지만 실제 파라미터 타입은Long입니다. 또한 도메인 엔티티 메서드에set접두사를 사용하면 Lombok이 생성하는 setter와 혼동될 수 있습니다.assignRepresentativeBadge또는changeRepresentativeBadge같은 도메인 의미를 드러내는 이름을 권장합니다.♻️ 리팩터링 제안
- public void setRepresentativeBadge(Long badgeId) { + public void assignRepresentativeBadge(Long badgeId) { this.representativeBadgeId = badgeId; }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/main/java/com/gpt/geumpumtabackend/user/domain/User.java` around lines 112 - 114, Rename the domain method setRepresentativeBadge(Long badgeId) in class User to a domain-intent name such as assignRepresentativeBadge or changeRepresentativeBadge, keep the parameter type Long and body setting this.representativeBadgeId = badgeId, and update all call sites to use the new method; this avoids confusion with Lombok-generated setters and makes the intent clear for the field representativeBadgeId.src/main/java/com/gpt/geumpumtabackend/badge/dto/response/BadgeCreateResponse.java (1)
6-28:BadgeResponse와 완전히 동일한 구조 — 코드 중복
BadgeCreateResponse와BadgeResponse는 필드 목록(id, code, name, description, iconUrl, badgeType, thresholdValue, rank)과from(Badge)팩토리 메서드까지 완전히 동일합니다. 하나의 공통 레코드를 재사용하거나, 생성 응답에 추가 필드가 없다면BadgeCreateResponse를 제거하고BadgeResponse만 사용하는 것을 권장합니다.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/main/java/com/gpt/geumpumtabackend/badge/dto/response/BadgeCreateResponse.java` around lines 6 - 28, BadgeCreateResponse duplicates BadgeResponse entirely; remove the duplicate record and switch callers to use BadgeResponse.from(Badge) instead. Delete the BadgeCreateResponse record, update all references/imports to refer to BadgeResponse (e.g., replace BadgeCreateResponse.from(badge) with BadgeResponse.from(badge)), and run compilation to fix any remaining import or package issues; if any API contract requires a distinct type, consolidate by keeping only BadgeResponse and adding any extra fields there instead of duplicating BadgeCreateResponse.src/main/java/com/gpt/geumpumtabackend/badge/domain/UserBadge.java (1)
9-16:UserBadge가BaseEntity를 상속하지 않습니다.
Badge엔티티는BaseEntity를 상속하고 있으나UserBadge는 그렇지 않습니다. 코딩 가이드라인에 따르면 모든 엔티티는BaseEntity를 상속해야 합니다.또한
@NoArgsConstructor접근 수준을 JPA 관례에 맞게PROTECTED로 제한하는 것을 권장합니다(Badge엔티티의 패턴과 일관성 유지).♻️ 수정 제안
+import com.gpt.geumpumtabackend.global.BaseEntity; import lombok.AccessLevel; import lombok.Getter; import lombok.NoArgsConstructor; `@Entity` `@Getter` -@NoArgsConstructor +@NoArgsConstructor(access = AccessLevel.PROTECTED) `@Table`( name = "user_badge", uniqueConstraints = `@UniqueConstraint`(name="uk_user_badge", columnNames = {"user_id", "badge_id"}) ) -public class UserBadge { +public class UserBadge extends BaseEntity {Based on learnings: "Implement soft delete for all entities by extending
BaseEntitywith@SQLDeletefor User masking"🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/main/java/com/gpt/geumpumtabackend/badge/domain/UserBadge.java` around lines 9 - 16, UserBadge does not extend BaseEntity and uses a public no-args constructor; update the entity to extend BaseEntity and restrict the no-args constructor to PROTECTED to match Badge's pattern (use `@NoArgsConstructor`(access = AccessLevel.PROTECTED) on class UserBadge), and ensure soft-delete behavior is enabled by applying the same `@SQLDelete` (and related soft-delete/@Where if used in BaseEntity) semantics inherited from BaseEntity so soft-deletes and user masking work consistently; keep the existing `@Table` and uniqueConstraints intact.src/test/java/com/gpt/geumpumtabackend/unit/badge/service/BadgeServiceTest.java (2)
200-225:grantWelcomeBadge멱등성 경로(이미 지급된 경우) 테스트 누락현재 테스트는
existsByUserIdAndBadgeId = false(신규 지급) 경로만 검증합니다. 서비스 코드 Line 131-133에서 이미 배지를 보유한 경우 저장 없이 반환하는 멱등성 로직이 있으나, 이 경로에 대한 테스트가 없습니다. 특히save가 호출되지 않음을 검증해야 합니다.멱등성 케이스 테스트 코드 생성을 원하시면 알려주세요.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/test/java/com/gpt/geumpumtabackend/unit/badge/service/BadgeServiceTest.java` around lines 200 - 225, Add a unit test for the idempotent path of badgeService.grantWelcomeBadge: arrange a userId and WELCOME badge (as in the existing test) but mock userBadgeRepository.existsByUserIdAndBadgeId(userId, badge.getId()) to return true, call badgeService.grantWelcomeBadge(userId), assert the returned NewBadgeResponse has the expected badge code, and verify userBadgeRepository.save(...) is never invoked (verify(..., times(0))). Reference the same Badge creation helper and the badgeService.grantWelcomeBadge, userBadgeRepository.existsByUserIdAndBadgeId, and userBadgeRepository.save symbols to locate and add the test.
536-548:createBadge헬퍼:rank=null이면id=null이 되는 암묵적 결합
ReflectionTestUtils.setField(badge, "id", rank)는rank가null일 때id를null로 설정합니다. 현재 테스트들은 모두 null rank 사용 시 id를 명시적으로 재설정하고 있어 실제 문제는 없지만, 추후 테스트 추가 시badge.getId()가null인 상태에서 NPE가 발생할 위험이 있습니다.♻️ 수정 제안
private Badge createBadge(String code, BadgeType type, Long rank) { Badge badge = Badge.builder() .code(code) .name("badge-name") .description("badge-desc") .iconUrl("http://example.com/badge.png") .badgeType(type) .thresholdValue(10L) .rank(rank) .build(); - ReflectionTestUtils.setField(badge, "id", rank); + // id는 테스트마다 필요에 따라 ReflectionTestUtils.setField(badge, "id", ...)로 명시 설정 return badge; }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/test/java/com/gpt/geumpumtabackend/unit/badge/service/BadgeServiceTest.java` around lines 536 - 548, createBadge 헬퍼가 rank가 null일 때 ReflectionTestUtils.setField(badge, "id", rank)로 id를 명시적으로 null로 만드는 암묵적 결합 문제를 수정하세요; createBadge 메소드 안에서 ReflectionTestUtils.setField을 호출하기 전에 rank != null인지 검사해서 null일 경우 id를 설정하지 않도록 하거나(또는 테스트용 기본/생성 id를 부여하도록) 변경하고, 관련 호출부(예: badge.getId())가 null을 가정하지 않도록 테스트들을 유지하세요.src/main/java/com/gpt/geumpumtabackend/badge/domain/Badge.java (1)
19-34: 핵심 필드에 DB 수준 NOT NULL 제약 누락 (선택적 개선)
code,name,badgeType등 필수 필드에nullable = false가 없어 DB 수준 무결성 보장이 없습니다.@Builder+ 서비스 레이어 검증으로 어느 정도 보호되지만, 스키마 자체에 제약을 명시하는 것이 더 안전합니다.♻️ 수정 제안
- `@Column`(unique = true) + `@Column`(unique = true, nullable = false) private String code; - private String name; + `@Column`(nullable = false) + private String name; private String description; private String iconUrl; - `@Enumerated`(EnumType.STRING) + `@Enumerated`(EnumType.STRING) + `@Column`(nullable = false) private BadgeType badgeType;🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/main/java/com/gpt/geumpumtabackend/badge/domain/Badge.java` around lines 19 - 34, The Badge entity is missing DB-level NOT NULL constraints for required fields; update the field annotations in class Badge so required fields (code, name, badgeType) are annotated with nullable=false (e.g., change `@Column`(unique = true) on code to `@Column`(unique = true, nullable = false), add `@Column`(nullable = false) to name, and add `@Column`(nullable = false) on the badgeType field alongside `@Enumerated`(EnumType.STRING)) to enforce schema-level non-null constraints while leaving optional fields (description, iconUrl, thresholdValue, rank) unchanged.src/main/java/com/gpt/geumpumtabackend/badge/service/BadgeService.java (2)
226-263:grantTotalHoursBadge/grantStreakBadge: 루프 내 개별save호출루프마다
userBadgeRepository.save(...)를 호출합니다 (Lines 241, 259). 배지가 여러 개일 경우 N번의 INSERT가 발생합니다. 새로 지급할UserBadge목록을 모아saveAll한 번으로 처리하면 DB 라운드트립을 줄일 수 있습니다.grantSeasonRankTypeBadges(Line 219)도 동일한 패턴입니다.♻️ 수정 제안 (grantTotalHoursBadge 예시)
private void grantTotalHoursBadge(Long userId, LocalDateTime now, List<NewBadgeResponse> newlyGranted) { Long totalMillis = studySessionRepository.sumTotalStudyMillisByUserId(userId); if (totalMillis == null) totalMillis = 0L; List<Badge> hourBadges = badgeRepository.findAllByBadgeType(BadgeType.TOTAL_HOURS); + List<UserBadge> toSave = new ArrayList<>(); for (Badge badge : hourBadges) { Long thresholdHours = badge.getThresholdValue(); if (thresholdHours == null) continue; long thresholdMillis = thresholdHours * 60L * 60L * 1000L; if (totalMillis >= thresholdMillis && !userBadgeRepository.existsByUserIdAndBadgeId(userId, badge.getId())) { - userBadgeRepository.save(new UserBadge(userId, badge.getId(), now, now)); + toSave.add(new UserBadge(userId, badge.getId(), now, now)); newlyGranted.add(NewBadgeResponse.from(badge)); } } + userBadgeRepository.saveAll(toSave); }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/main/java/com/gpt/geumpumtabackend/badge/service/BadgeService.java` around lines 226 - 263, Both grantTotalHoursBadge and grantStreakBadge (and similarly grantSeasonRankTypeBadges) currently call userBadgeRepository.save(...) inside the loop causing N separate INSERTs; instead, collect the qualifying UserBadge instances into a List<UserBadge> (using the same checks with badge.getThresholdValue() and userBadgeRepository.existsByUserIdAndBadgeId(...) or, even better, pre-fetch existing badge ids), then call userBadgeRepository.saveAll(collectedBadges) once and add NewBadgeResponse.from(badge) to newlyGranted as you collect; update grantTotalHoursBadge, grantStreakBadge, and grantSeasonRankTypeBadges to build and save batches via userBadgeRepository.saveAll(...) and preserve the now/UserBadge constructor usage.
107-121:findByCode/findByBadgeType에Optional미사용 — 비관용적 패턴
badgeRepository.findByCode(...)와badgeRepository.findByBadgeType(...)가Badge를 직접 반환하고, 서비스에서 null 체크를 직접 수행하고 있습니다(라인 109-112, 127-130). Spring Data JPA 관용 패턴(Optional<Badge>반환)을 사용하면 null 체크 누락으로 인한 NPE 위험을 줄이고 의도가 더 명확해집니다.♻️ 수정 제안 (BadgeRepository 기준)
-Badge findByCode(String code); -Badge findByBadgeType(BadgeType badgeType); +Optional<Badge> findByCode(String code); +Optional<Badge> findByBadgeType(BadgeType badgeType);서비스 사용 예:
-Badge badge = badgeRepository.findByCode(request.badgeCode()); -if (badge == null) { - throw new BusinessException(ExceptionType.BADGE_NOT_FOUND); -} +Badge badge = badgeRepository.findByCode(request.badgeCode()) + .orElseThrow(() -> new BusinessException(ExceptionType.BADGE_NOT_FOUND));🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/main/java/com/gpt/geumpumtabackend/badge/service/BadgeService.java` around lines 107 - 121, The repository currently returns Badge directly causing manual null checks; change BadgeRepository methods findByCode(...) and findByBadgeType(...) to return Optional<Badge>, then update callers in BadgeService (e.g., setRepresentativeBadge and any other methods using findByBadgeType) to use Optional APIs like .orElseThrow(() -> new BusinessException(ExceptionType.BADGE_NOT_FOUND)) instead of explicit null checks, and remove the now-unnecessary null branch logic to follow Spring Data JPA idioms.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In
`@src/main/java/com/gpt/geumpumtabackend/badge/controller/BadgeController.java`:
- Around line 68-78: The setRepresentativeBadge endpoint is missing `@Valid` on
its `@RequestBody` parameter (unlike createBadge) and the BadgeApi interface also
omits it; add `@Valid` to the RepresentativeBadgeRequest parameter in
BadgeController.setRepresentativeBadge and the corresponding method declaration
in BadgeApi so Spring validation runs, and optionally add validation annotations
(e.g., `@NotBlank` on badgeCode) to RepresentativeBadgeRequest if you want to
enforce constraints at runtime.
In `@src/main/java/com/gpt/geumpumtabackend/badge/domain/UserBadge.java`:
- Around line 17-25: The fields id, userId, and badgeId in class UserBadge are
missing the private access modifier which breaks encapsulation; update the
declarations of Long id, Long userId, and Long badgeId in the UserBadge class to
be private (matching the existing private awardedAt and notifiedAt) so they are
not package-private and only accessible via the class's getters/setters.
In
`@src/main/java/com/gpt/geumpumtabackend/badge/dto/request/RepresentativeBadgeRequest.java`:
- Around line 3-6: The RepresentativeBadgeRequest record's badgeCode component
lacks validation; annotate the record component with `@NotBlank` and add the
corresponding import (javax.validation.constraints.NotBlank) so blank or null
values are rejected before reaching the service layer (update the
RepresentativeBadgeRequest declaration to include the `@NotBlank` on badgeCode and
ensure the validation annotation import is present).
In
`@src/main/java/com/gpt/geumpumtabackend/badge/repository/BadgeRepository.java`:
- Around line 10-14: Change the repository signatures in BadgeRepository: make
findByBadgeType(BadgeType) and findByCode(String) return Optional<Badge> instead
of Badge (keep findAllByBadgeType as-is); then update BadgeService usages that
currently do null checks (the blocks that handle badge lookups around the
current null-check locations) to use Optional.orElseThrow() to throw a suitable
exception when not found. Ensure you update import statements and any call sites
that assumed a raw Badge to unwrap Optionals, and review logic where
findByBadgeType coexists with findAllByBadgeType to avoid incorrect result-size
assumptions (use the list query for multi-result types like
TOTAL_HOURS/STREAK_DAYS if needed).
In
`@src/main/java/com/gpt/geumpumtabackend/badge/repository/UserBadgeRepository.java`:
- Around line 10-31: The repository query methods findMyBadges and
findUnnotifiedBadgeResponses use a named parameter :userId but the method
parameters lack `@Param`, so add `@Param`("userId") to the Long userId parameter in
both methods of UserBadgeRepository (i.e., annotate the parameter of
findMyBadges(Long userId) and findUnnotifiedBadgeResponses(Long userId) with
`@Param`("userId")) to ensure correct named-parameter binding during query
execution.
In `@src/main/java/com/gpt/geumpumtabackend/badge/service/BadgeService.java`:
- Around line 75-93: The createBadge method currently checks
badgeRepository.existsByCode(...) but can still hit a unique constraint race
between that check and badgeRepository.save(...); wrap the save call in a
try-catch that catches DataIntegrityViolationException (and/or the underlying
ConstraintViolationException if preferred), and when caught rethrow a
BusinessException with ExceptionType.BADGE_CODE_ALREADY_EXISTS so the
unique-constraint failure is returned consistently; keep the pre-check for early
failure but ensure createBadge (and its `@Transactional` method) converts DB
uniqueness exceptions into the same BusinessException, and add the necessary
import for DataIntegrityViolationException.
- Around line 139-160: getUnnotifiedBadges currently does a DTO projection
(findUnnotifiedBadgeResponses) then separately loads entities
(findByUserIdAndNotifiedAtIsNull) which can miss badges added between the two
queries; fix by loading entities first, marking/saving them, then mapping those
same entities to MyBadgeResponse. Specifically, change getUnnotifiedBadges to
call userBadgeRepository.findByUserIdAndNotifiedAtIsNull(userId), return empty
if none, call markBadgesNotified with that entity list (or overload
markBadgesNotified to accept List<UserBadge>), persist via
userBadgeRepository.saveAll(userBadges), and then map the saved UserBadge
entities to MyBadgeResponse (instead of using findUnnotifiedBadgeResponses)
before returning.
In `@src/main/java/com/gpt/geumpumtabackend/global/exception/ExceptionType.java`:
- Around line 72-76: The BADGE_* entries in the ExceptionType enum have a typo
and a namespace collision: change BADGE_NOT_FOUND's code string from "BOO1" to a
correct badge prefix and numeric id (e.g., "BG001") and rename the other badge
codes BADGE_NOT_OWNED, BADGE_CODE_ALREADY_EXISTS, BADGE_IN_USE from
"B002"/"B003"/"B004" to the new badge-prefixed values ("BG002"/"BG003"/"BG004")
to avoid colliding with BOARD_NOT_FOUND's "B001"; update any related
documentation or prefix list to include the new "BG" prefix and ensure any
code/tests that assert these exact codes (references to
ExceptionType.BADGE_NOT_FOUND, BADGE_NOT_OWNED, BADGE_CODE_ALREADY_EXISTS,
BADGE_IN_USE) are updated accordingly.
In
`@src/main/java/com/gpt/geumpumtabackend/rank/scheduler/SeasonTransitionScheduler.java`:
- Line 18: Remove the extra spaces in the class declaration for
SeasonTransitionScheduler: change the declaration token `public class
SeasonTransitionScheduler` to use a single space between `class` and
`SeasonTransitionScheduler` (i.e., `public class SeasonTransitionScheduler`) so
the class header is correctly formatted.
- Around line 50-53: The code can leave the system in a partial state if
createSeasonSnapshot succeeds but grantSeasonRankingBadges throws; update
SeasonTransitionScheduler so the call to
badgeService.grantSeasonRankingBadges(endedSeasonId) is wrapped in its own
try-catch: on exception log the error and persist a retryable record (or enqueue
a recovery job) referencing endedSeasonId (e.g., via a new
SeasonBadgeRecoveryService.recordFailedGrant(endedSeasonId, error)) so the
missing badges can be retried later; also ensure grantSeasonRankingBadges is
made idempotent so retries are safe and consider exposing an admin retry API.
In
`@src/main/java/com/gpt/geumpumtabackend/statistics/repository/StatisticsRepository.java`:
- Around line 268-313: The recursive CTE repeatedly runs a correlated subquery
per recursion, causing O(N) subqueries and hitting MySQL's
cte_max_recursion_depth; refactor the native query used by
countCurrentConsecutiveStudyDays to first compute per-day totals in a single
non-recursive CTE (e.g., per_day with DATE ranges and SUM of session durations
per user for a bounded window like last 365 days), then apply a window function
(LAG/ROW_NUMBER or differences of row_number over ordered dates) to detect
consecutive days where day_millis >= :thresholdMillis and finally COUNT the
consecutive days ending at :today; this removes the correlated subquery inside
the recursive CTE, bounds recursion (or eliminates it), and dramatically
improves performance and correctness for long streaks.
In
`@src/main/java/com/gpt/geumpumtabackend/study/service/StudySessionService.java`:
- Around line 24-26: StudySessionService.java 현재 import 블록에 java.util.List가 중복
선언되어 있습니다; StudySessionService 클래스의 import 섹션에서 중복된 import java.util.List 한 줄을
제거하고 import 집합을 정리하여 중복을 없애세요 (참조 대상: 클래스 StudySessionService, import 구문에서
java.util.List 및 java.util.ArrayList).
In
`@src/test/java/com/gpt/geumpumtabackend/integration/statistics/StatisticsControllerIntegrationTest.java`:
- Line 126: 테스트에서 반환 타입이 Long인 WeeklyStatistics.getTotalWeekMillis()와
MonthlyStatistics.getTotalMonthMillis()에 대해 jsonPath 어서션이 정수(int)로 비교되면서 타입 불일치가
발생하므로, StatisticsControllerIntegrationTest의 관련 assertions에서 ONE_HOUR_MILLIS * 2
및 14_400_000L 값을 int로 명시적으로 캐스팅하여 일간 통계 테스트와 일관되게 바꿔 주세요; 예를 들어
jsonPath("$.data.weeklyStatistics.totalWeekMillis").value((int)(ONE_HOUR_MILLIS
* 2)) 및
jsonPath("$.data.monthlyStatistics.totalMonthMillis").value((int)14_400_000L) 처럼
수정하면 됩니다.
In
`@src/test/java/com/gpt/geumpumtabackend/unit/badge/service/BadgeServiceTest.java`:
- Around line 45-68: The test class BadgeServiceTest currently uses
`@ExtendWith`(MockitoExtension.class) directly which violates the guideline to
extend BaseUnitTest; change the class to extend BaseUnitTest (remove the
explicit `@ExtendWith`(MockitoExtension.class) annotation) so dependency injection
via `@InjectMocks` and `@Mock` in BadgeServiceTest works under the shared test base;
update the class declaration (BadgeServiceTest) to "class BadgeServiceTest
extends BaseUnitTest" and keep existing fields (badgeRepository,
userBadgeRepository, userRepository, studySessionRepository,
statisticsRepository, seasonRankingSnapshotRepository, badgeService) intact.
---
Outside diff comments:
In
`@src/main/java/com/gpt/geumpumtabackend/study/service/StudySessionService.java`:
- Around line 28-31: Add class-level `@Transactional`(readOnly = true) to
StudySessionService so all query methods (e.g., getTodayStudySession) run in
read-only transactions, and annotate each write method that mutates state (any
method that calls repository.save, delete, or performs updates — e.g.,
create/update/delete-like methods in StudySessionService) with `@Transactional`
(without readOnly) to enable commits; update imports if needed and ensure
method-level `@Transactional` overrides the class-level readOnly.
In `@src/main/java/com/gpt/geumpumtabackend/user/domain/User.java`:
- Around line 16-25: The soft-delete SQL in User.java fails to null out
representative_badge_id and BadgeService.deleteBadge() omits the guard that
prevents deleting a badge still referenced as a user's representative; update
the `@SQLDelete` block in the User entity to set representative_badge_id = NULL
during soft delete, and add a check in BadgeService.deleteBadge() that uses a
repository method (add boolean
existsByRepresentativeBadgeIdAndDeletedAtIsNull(Long badgeId) to UserRepository)
to refuse deletion when any non-deleted User.representativeBadgeId equals the
target badgeId, returning/throwing the existing domain error path if the guard
triggers.
In `@src/main/java/com/gpt/geumpumtabackend/user/service/UserService.java`:
- Around line 64-78: completeRegistration is transactional and calls
badgeService.grantWelcomeBadge(userId) which runs in the same transaction, so
badge failures can roll back the whole registration; isolate badge granting by
wrapping the grantWelcomeBadge call in a try-catch (catching BusinessException
and runtime exceptions) and log/ignore badge errors so registration proceeds, or
change the badge service method or its invocation to execute in a new
transaction (PROPAGATION_REQUIRES_NEW) so failures there do not roll back
UserService.completeRegistration; update references in
UserService.completeRegistration and/or BadgeService.grantWelcomeBadge
accordingly.
In
`@src/test/java/com/gpt/geumpumtabackend/unit/user/service/UserServiceTest.java`:
- Around line 41-43: The test class UserServiceTest currently uses
`@ExtendWith`(MockitoExtension.class) directly; change it to extend BaseUnitTest
instead (i.e., remove the `@ExtendWith` annotation and make UserServiceTest extend
BaseUnitTest) so it follows the project test base pattern; update imports to
remove MockitoExtension and add any BaseUnitTest import if missing, and ensure
existing annotations like `@DisplayName`("UserService 단위 테스트") and Mockito usages
inside UserServiceTest remain valid under BaseUnitTest.
---
Duplicate comments:
In `@src/main/java/com/gpt/geumpumtabackend/badge/api/BadgeApi.java`:
- Around line 151-157: The interface method setRepresentativeBadge is missing
the `@Valid` annotation on the RepresentativeBadgeRequest parameter (like
BadgeController), causing request validation to be skipped; update the method
signature in BadgeApi to annotate the request parameter with `@Valid` for
RepresentativeBadgeRequest (and add the corresponding javax/jakarta.validation
import if needed) so it matches BadgeController's behavior and triggers
validation.
In
`@src/main/java/com/gpt/geumpumtabackend/badge/dto/response/BadgeResponse.java`:
- Around line 6-28: BadgeResponse duplicates the same fields/structure as
BadgeCreateResponse; refactor by extracting a single shared DTO or renaming
BadgeResponse to a common BadgeDto and have BadgeCreateResponse delegate to or
reuse that DTO instead of duplicating fields. Update the factory method
(BadgeResponse.from(Badge)) and any constructors/usages so they return/consume
the shared record/class (e.g., BadgeDto) and remove the duplicated record;
ensure all references to BadgeResponse or BadgeCreateResponse are adjusted to
use the new shared symbol so there is a single source of truth for the badge
response shape.
---
Nitpick comments:
In `@src/main/java/com/gpt/geumpumtabackend/badge/domain/Badge.java`:
- Around line 19-34: The Badge entity is missing DB-level NOT NULL constraints
for required fields; update the field annotations in class Badge so required
fields (code, name, badgeType) are annotated with nullable=false (e.g., change
`@Column`(unique = true) on code to `@Column`(unique = true, nullable = false), add
`@Column`(nullable = false) to name, and add `@Column`(nullable = false) on the
badgeType field alongside `@Enumerated`(EnumType.STRING)) to enforce schema-level
non-null constraints while leaving optional fields (description, iconUrl,
thresholdValue, rank) unchanged.
In `@src/main/java/com/gpt/geumpumtabackend/badge/domain/UserBadge.java`:
- Around line 9-16: UserBadge does not extend BaseEntity and uses a public
no-args constructor; update the entity to extend BaseEntity and restrict the
no-args constructor to PROTECTED to match Badge's pattern (use
`@NoArgsConstructor`(access = AccessLevel.PROTECTED) on class UserBadge), and
ensure soft-delete behavior is enabled by applying the same `@SQLDelete` (and
related soft-delete/@Where if used in BaseEntity) semantics inherited from
BaseEntity so soft-deletes and user masking work consistently; keep the existing
`@Table` and uniqueConstraints intact.
In
`@src/main/java/com/gpt/geumpumtabackend/badge/dto/response/BadgeCreateResponse.java`:
- Around line 6-28: BadgeCreateResponse duplicates BadgeResponse entirely;
remove the duplicate record and switch callers to use BadgeResponse.from(Badge)
instead. Delete the BadgeCreateResponse record, update all references/imports to
refer to BadgeResponse (e.g., replace BadgeCreateResponse.from(badge) with
BadgeResponse.from(badge)), and run compilation to fix any remaining import or
package issues; if any API contract requires a distinct type, consolidate by
keeping only BadgeResponse and adding any extra fields there instead of
duplicating BadgeCreateResponse.
In `@src/main/java/com/gpt/geumpumtabackend/badge/service/BadgeService.java`:
- Around line 226-263: Both grantTotalHoursBadge and grantStreakBadge (and
similarly grantSeasonRankTypeBadges) currently call
userBadgeRepository.save(...) inside the loop causing N separate INSERTs;
instead, collect the qualifying UserBadge instances into a List<UserBadge>
(using the same checks with badge.getThresholdValue() and
userBadgeRepository.existsByUserIdAndBadgeId(...) or, even better, pre-fetch
existing badge ids), then call userBadgeRepository.saveAll(collectedBadges) once
and add NewBadgeResponse.from(badge) to newlyGranted as you collect; update
grantTotalHoursBadge, grantStreakBadge, and grantSeasonRankTypeBadges to build
and save batches via userBadgeRepository.saveAll(...) and preserve the
now/UserBadge constructor usage.
- Around line 107-121: The repository currently returns Badge directly causing
manual null checks; change BadgeRepository methods findByCode(...) and
findByBadgeType(...) to return Optional<Badge>, then update callers in
BadgeService (e.g., setRepresentativeBadge and any other methods using
findByBadgeType) to use Optional APIs like .orElseThrow(() -> new
BusinessException(ExceptionType.BADGE_NOT_FOUND)) instead of explicit null
checks, and remove the now-unnecessary null branch logic to follow Spring Data
JPA idioms.
In `@src/main/java/com/gpt/geumpumtabackend/user/domain/User.java`:
- Around line 66-67: The field representativeBadgeId in class User is stored as
a raw Long with no DB-level foreign key, risking referential integrity; either
convert it to a proper JPA relation (e.g., change representativeBadgeId to a
Badge typed field like representativeBadge with `@ManyToOne` and `@JoinColumn` plus
an explicit foreignKey name such as fk_user_representative_badge) so the DB
enforces FK constraints, or if loose coupling is intentional, add a clear
comment above representativeBadgeId documenting the design decision and why
DB-level FK was omitted; adjust service/repository code accordingly to use the
new relation or to handle the documented loose-coupling semantics.
- Around line 112-114: Rename the domain method setRepresentativeBadge(Long
badgeId) in class User to a domain-intent name such as assignRepresentativeBadge
or changeRepresentativeBadge, keep the parameter type Long and body setting
this.representativeBadgeId = badgeId, and update all call sites to use the new
method; this avoids confusion with Lombok-generated setters and makes the intent
clear for the field representativeBadgeId.
In
`@src/test/java/com/gpt/geumpumtabackend/unit/badge/service/BadgeServiceTest.java`:
- Around line 200-225: Add a unit test for the idempotent path of
badgeService.grantWelcomeBadge: arrange a userId and WELCOME badge (as in the
existing test) but mock userBadgeRepository.existsByUserIdAndBadgeId(userId,
badge.getId()) to return true, call badgeService.grantWelcomeBadge(userId),
assert the returned NewBadgeResponse has the expected badge code, and verify
userBadgeRepository.save(...) is never invoked (verify(..., times(0))).
Reference the same Badge creation helper and the badgeService.grantWelcomeBadge,
userBadgeRepository.existsByUserIdAndBadgeId, and userBadgeRepository.save
symbols to locate and add the test.
- Around line 536-548: createBadge 헬퍼가 rank가 null일 때
ReflectionTestUtils.setField(badge, "id", rank)로 id를 명시적으로 null로 만드는 암묵적 결합 문제를
수정하세요; createBadge 메소드 안에서 ReflectionTestUtils.setField을 호출하기 전에 rank != null인지
검사해서 null일 경우 id를 설정하지 않도록 하거나(또는 테스트용 기본/생성 id를 부여하도록) 변경하고, 관련 호출부(예:
badge.getId())가 null을 가정하지 않도록 테스트들을 유지하세요.
src/main/java/com/gpt/geumpumtabackend/badge/controller/BadgeController.java
Show resolved
Hide resolved
src/main/java/com/gpt/geumpumtabackend/badge/domain/UserBadge.java
Outdated
Show resolved
Hide resolved
src/main/java/com/gpt/geumpumtabackend/badge/dto/request/RepresentativeBadgeRequest.java
Show resolved
Hide resolved
src/main/java/com/gpt/geumpumtabackend/badge/repository/BadgeRepository.java
Outdated
Show resolved
Hide resolved
src/main/java/com/gpt/geumpumtabackend/badge/repository/UserBadgeRepository.java
Show resolved
Hide resolved
src/main/java/com/gpt/geumpumtabackend/statistics/repository/StatisticsRepository.java
Show resolved
Hide resolved
src/main/java/com/gpt/geumpumtabackend/study/service/StudySessionService.java
Outdated
Show resolved
Hide resolved
src/main/java/com/gpt/geumpumtabackend/study/service/StudySessionService.java
Show resolved
Hide resolved
...ava/com/gpt/geumpumtabackend/integration/statistics/StatisticsControllerIntegrationTest.java
Outdated
Show resolved
Hide resolved
src/test/java/com/gpt/geumpumtabackend/unit/badge/service/BadgeServiceTest.java
Show resolved
Hide resolved
Contributor
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Nitpick comments (4)
src/test/java/com/gpt/geumpumtabackend/integration/statistics/StatisticsControllerIntegrationTest.java (3)
128-128:averageDailyMillis매직 넘버를 수식으로 표현하는 것이 바람직함
1_028_571은7_200_000 / 7의 정수 나눗셈 결과입니다. 수식으로 작성하면 계산 근거가 명확해지고, 세션 시간이 변경되어도 기댓값을 쉽게 추적할 수 있습니다.♻️ 수식 기반 표현으로 개선
- .andExpect(jsonPath("$.data.weeklyStatistics.averageDailyMillis").value(1_028_571)); + .andExpect(jsonPath("$.data.weeklyStatistics.averageDailyMillis").value((int) (ONE_HOUR_MILLIS * 2 / 7)));🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/test/java/com/gpt/geumpumtabackend/integration/statistics/StatisticsControllerIntegrationTest.java` at line 128, Replace the magic literal 1_028_571 in the jsonPath assertion inside StatisticsControllerIntegrationTest with a calculated expression so the rationale is explicit; update the .andExpect(jsonPath("$.data.weeklyStatistics.averageDailyMillis").value(...)) call to use the integer division expression (e.g. 7_200_000 / 7) or, preferably, derive the numerator from the existing test constant (such as SESSION_LENGTH_MS or TOTAL_WEEKLY_MILLIS) to compute averageDailyMillis so the expected value updates automatically if session durations change.
145-145:averageDailyMillis매직 넘버 — 수식으로 표현 권장
464_516은14_400_000 / 31(1월 31일)의 정수 나눗셈 결과입니다. 수식으로 명시하면 의도가 드러납니다.♻️ 수식 기반 표현으로 개선
- .andExpect(jsonPath("$.data.monthlyStatistics.averageDailyMillis").value(464_516)) + .andExpect(jsonPath("$.data.monthlyStatistics.averageDailyMillis").value((int) (14_400_000L / 31)))1월 31일 고정이 아니라
BASE_DATE.getMonth().length(BASE_DATE.isLeapYear())같은 방식으로 달의 일수를 동적으로 계산하면BASE_DATE변경 시에도 안전합니다.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/test/java/com/gpt/geumpumtabackend/integration/statistics/StatisticsControllerIntegrationTest.java` at line 145, Replace the magic literal 464_516 in the assertion with an expression that computes the expected average from the known total millis and the actual days in the month so the intent is clear and resilient to BASE_DATE changes; use BASE_DATE (from StatisticsControllerIntegrationTest) and compute days via BASE_DATE.getMonth().length(BASE_DATE.isLeapYear()) (or a precomputed int like daysInMonth) and assert jsonPath("$.data.monthlyStatistics.averageDailyMillis").value(totalMillis / daysInMonth) so the test documents the formula rather than a hardcoded number.
85-164: 예외/경계 케이스 테스트 누락현재 4개 테스트 모두 정상 흐름(happy-path)만 검증합니다. 코딩 가이드라인에는 "정상 케이스, 예외 케이스, 경계값 케이스를 포함" 할 것을 요구합니다. 아래 케이스들이 빠져 있습니다.
누락 케이스 설명 인증 없이 요청 Authorization헤더 생략 →403응답 확인유효하지 않은 토큰 잘못된 JWT 전달 → 401응답 확인데이터 없는 경우 세션 없이 통계 조회 → 빈 슬롯/0값 응답 확인 사용자 데이터 격리 otherUser토큰으로 조회 시testUser데이터가 섞이지 않음을 명시적으로 검증코딩 가이드라인: "Test coverage should include normal cases, exception cases, and boundary value cases per TESTING.md"
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/test/java/com/gpt/geumpumtabackend/integration/statistics/StatisticsControllerIntegrationTest.java` around lines 85 - 164, Add negative and boundary tests alongside the existing happy-path tests (methods 일간_통계를_조회한다, 주간_통계를_조회한다, 월간_통계를_조회한다, 잔디_통계를_조회한다): add cases that call the same endpoints (/api/v1/statistics/day, /week, /month, /grass) without the Authorization header and assert 403, with a malformed/expired JWT and assert 401, with no study sessions created and assert the response contains empty lists / zeroed statistics, and with otherUser's valid token to assert testUser's data is not present (data isolation). Use the same mockMvc.perform pattern and jsonPath assertions to verify the expected status codes and empty/zero response fields.src/test/java/com/gpt/geumpumtabackend/unit/badge/service/BadgeServiceTest.java (1)
536-548:createBadge헬퍼:rank를id로 사용하는 설계 혼용
ReflectionTestUtils.setField(badge, "id", rank)로 인해rank가null이면id도null이 됩니다. 또한TOTAL_HOURS타입 배지에rank=1L을 넘기면 의미 없는rank필드가 설정된 채로id=1L이 되어 필드 의미가 혼용됩니다.thresholdValue를10L로 하드코딩해 테스트마다ReflectionTestUtils.setField로 재설정해야 하는 점도 불편합니다.♻️ 수정 제안 — id와 rank를 별도 파라미터로 분리
-private Badge createBadge(String code, BadgeType type, Long rank) { +private Badge createBadge(String code, BadgeType type, Long rank, Long thresholdValue) { Badge badge = Badge.builder() .code(code) .name("badge-name") .description("badge-desc") .iconUrl("http://example.com/badge.png") .badgeType(type) - .thresholdValue(10L) + .thresholdValue(thresholdValue) .rank(rank) .build(); - ReflectionTestUtils.setField(badge, "id", rank); + // 호출부에서 ReflectionTestUtils.setField(badge, "id", ...) 로 명시적으로 설정 return badge; }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/test/java/com/gpt/geumpumtabackend/unit/badge/service/BadgeServiceTest.java` around lines 536 - 548, createBadge 헬퍼가 rank를 id로 재사용하고 thresholdValue를 하드코딩해 테스트 혼선이 발생하므로 createBadge(Signature)를 변경해 id와 rank를 별도 파라미터로 분리하고 thresholdValue를 인자로 추가하세요; 구체적으로 Badge.builder() 호출부는 그대로 두되 매개변수로 Long id, Long rank, Long thresholdValue를 받도록 만들고 ReflectionTestUtils.setField(badge, "id", id) 는 id가 non-null일 때만 호출하며 badge.setRank(rank) 또는 builder의 rank에 rank를 non-null일 때만 설정하도록 바꾸고 thresholdValue는 전달된 값으로 설정하게 수정하세요 (함수명: createBadge, 사용된 ReflectionTestUtils.setField, Badge.builder(), thresholdValue 참조).
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@src/main/java/com/gpt/geumpumtabackend/badge/service/BadgeService.java`:
- Around line 122-134: The grantWelcomeBadge method currently returns
NewBadgeResponse even when the user already has the badge (via
userBadgeRepository.existsByUserIdAndBadgeId), causing callers to misinterpret
it as a newly granted badge; change the method signature from NewBadgeResponse
to Optional<NewBadgeResponse>, return Optional.empty() when the badge already
exists, and return Optional.of(NewBadgeResponse.from(badge)) after saving;
update all callers of grantWelcomeBadge to handle Optional (or adapt to the new
contract) and keep the existing logic around badge lookup
(badgeRepository.findByBadgeType) and saving (userBadgeRepository.save) intact.
- Around line 159-170: grantStudyAchievementBadges is annotated with
`@Transactional`(propagation = Propagation.REQUIRES_NEW) and is called from
StudySessionService.endStudySession while the outer transaction has not yet
committed, so aggregate queries like sumTotalStudyMillisByUserId will not see
the just-ended session and badges may be delayed; fix by either 1) emit a domain
event from endStudySession and handle badge awarding in a separate listener
annotated with `@TransactionalEventListener`(phase =
TransactionPhase.AFTER_COMMIT) so the new transaction sees committed data, or 2)
change the API to pass the just-ended session’s totalMillis
(session.getTotalMillis()) into grantStudyAchievementBadges (and incorporate it
into the aggregation logic) so you don’t rely on reading uncommitted data.
In
`@src/test/java/com/gpt/geumpumtabackend/unit/study/service/StudySessionServiceTest.java`:
- Around line 51-55: The test class StudySessionServiceTest currently uses
`@ExtendWith`(MockitoExtension.class) directly which violates the project
guideline; change the class to extend BaseUnitTest instead of using the
annotation (remove `@ExtendWith`(MockitoExtension.class) if present) so mocking
setup and common test config come from BaseUnitTest; update any imports/usages
in StudySessionServiceTest that rely on the manual extension (e.g.,
MockitoExtension) and ensure fields like studyProperties and badgeService remain
annotated with `@Mock` and initialized by the BaseUnitTest setup.
---
Duplicate comments:
In `@src/main/java/com/gpt/geumpumtabackend/badge/service/BadgeService.java`:
- Around line 136-157: The current getUnnotifiedBadges does two separate queries
(userBadgeRepository.findUnnotifiedBadgeResponses and then
findByUserIdAndNotifiedAtIsNull) which can race and drop notifications; change
the flow so you fetch the actual UserBadge entities once, mark them notified,
save them, then map those same entities to MyBadgeResponse to return.
Specifically, replace the dual queries by using a single query that returns
UserBadge entities (or a for-update variant) instead of
findUnnotifiedBadgeResponses, call markNotified(now) on each entity (as in
markBadgesNotified), saveAll via userBadgeRepository, and then convert those
saved entities to responses within getUnnotifiedBadges; keep the `@Transactional`
boundary on getUnnotifiedBadges and reference methods
userBadgeRepository.findUnnotifiedBadgeResponses,
userBadgeRepository.findByUserIdAndNotifiedAtIsNull, getUnnotifiedBadges, and
markBadgesNotified when applying this change.
- Around line 76-94: The current createBadge method performs a separate
existsByCode check before save, which is vulnerable to a TOCTOU race; ensure the
badge code uniqueness is enforced at the database level (unique constraint on
the Badge.code column) and remove reliance on the pre-check by wrapping the
badgeRepository.save(...) call in an exception handler that catches constraint
violations (e.g., DataIntegrityViolationException or the JPA provider's unique
constraint exception) and translates them to
BusinessException(ExceptionType.BADGE_CODE_ALREADY_EXISTS); keep the builder and
save flow in createBadge but delete or make the existsByCode pre-check advisory
only, and add the catch around badgeRepository.save (or saveAndFlush) in
BadgeService.createBadge to convert DB uniqueness errors into the existing
business exception.
In
`@src/main/java/com/gpt/geumpumtabackend/rank/scheduler/SeasonTransitionScheduler.java`:
- Line 18: Remove the extra whitespace in the class declaration for
SeasonTransitionScheduler: fix the declaration "public class
SeasonTransitionScheduler {" to have a single space between 'class' and
'SeasonTransitionScheduler' (i.e., "public class SeasonTransitionScheduler {")
and ensure there are no trailing or duplicate spaces around the class name in
that file.
- Around line 51-58: The try-catch around
badgeService.grantSeasonRankingBadges(endedSeasonId) prevents the scheduler from
failing but leaves badge issuance permanently missed because
SeasonTransitionScheduler's isBefore check skips already-transitioned seasons;
fix by making the failure durable and retryable: on exception from
badgeService.grantSeasonRankingBadges (in SeasonTransitionScheduler) persist the
failed seasonId (e.g., via a new badgeRetry table or by calling a new
badgeService.recordPendingBadgeForSeason(endedSeasonId)) or roll back/unmark the
season transition so the next run will attempt issuance again; ensure
grantSeasonRankingBadges and the scheduler cooperate (add
recordPendingBadgeForSeason or revertSeasonTransition methods) so retries are
processed by subsequent scheduler runs or a dedicated retry job.
In
`@src/test/java/com/gpt/geumpumtabackend/unit/badge/service/BadgeServiceTest.java`:
- Around line 45-47: The test class BadgeServiceTest currently does not extend
BaseUnitTest; update the class declaration so BadgeServiceTest extends
BaseUnitTest to inherit common test setup/fixtures and mocks used across unit
tests (locate class BadgeServiceTest and change its declaration to extend
BaseUnitTest), ensuring any required constructors or setup methods from
BaseUnitTest are accommodated and imports adjusted accordingly.
---
Nitpick comments:
In
`@src/test/java/com/gpt/geumpumtabackend/integration/statistics/StatisticsControllerIntegrationTest.java`:
- Line 128: Replace the magic literal 1_028_571 in the jsonPath assertion inside
StatisticsControllerIntegrationTest with a calculated expression so the
rationale is explicit; update the
.andExpect(jsonPath("$.data.weeklyStatistics.averageDailyMillis").value(...))
call to use the integer division expression (e.g. 7_200_000 / 7) or, preferably,
derive the numerator from the existing test constant (such as SESSION_LENGTH_MS
or TOTAL_WEEKLY_MILLIS) to compute averageDailyMillis so the expected value
updates automatically if session durations change.
- Line 145: Replace the magic literal 464_516 in the assertion with an
expression that computes the expected average from the known total millis and
the actual days in the month so the intent is clear and resilient to BASE_DATE
changes; use BASE_DATE (from StatisticsControllerIntegrationTest) and compute
days via BASE_DATE.getMonth().length(BASE_DATE.isLeapYear()) (or a precomputed
int like daysInMonth) and assert
jsonPath("$.data.monthlyStatistics.averageDailyMillis").value(totalMillis /
daysInMonth) so the test documents the formula rather than a hardcoded number.
- Around line 85-164: Add negative and boundary tests alongside the existing
happy-path tests (methods 일간_통계를_조회한다, 주간_통계를_조회한다, 월간_통계를_조회한다, 잔디_통계를_조회한다):
add cases that call the same endpoints (/api/v1/statistics/day, /week, /month,
/grass) without the Authorization header and assert 403, with a
malformed/expired JWT and assert 401, with no study sessions created and assert
the response contains empty lists / zeroed statistics, and with otherUser's
valid token to assert testUser's data is not present (data isolation). Use the
same mockMvc.perform pattern and jsonPath assertions to verify the expected
status codes and empty/zero response fields.
In
`@src/test/java/com/gpt/geumpumtabackend/unit/badge/service/BadgeServiceTest.java`:
- Around line 536-548: createBadge 헬퍼가 rank를 id로 재사용하고 thresholdValue를 하드코딩해 테스트
혼선이 발생하므로 createBadge(Signature)를 변경해 id와 rank를 별도 파라미터로 분리하고 thresholdValue를
인자로 추가하세요; 구체적으로 Badge.builder() 호출부는 그대로 두되 매개변수로 Long id, Long rank, Long
thresholdValue를 받도록 만들고 ReflectionTestUtils.setField(badge, "id", id) 는 id가
non-null일 때만 호출하며 badge.setRank(rank) 또는 builder의 rank에 rank를 non-null일 때만 설정하도록
바꾸고 thresholdValue는 전달된 값으로 설정하게 수정하세요 (함수명: createBadge, 사용된
ReflectionTestUtils.setField, Badge.builder(), thresholdValue 참조).
src/test/java/com/gpt/geumpumtabackend/unit/study/service/StudySessionServiceTest.java
Outdated
Show resolved
Hide resolved
Contributor
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (2)
src/main/java/com/gpt/geumpumtabackend/study/event/StudySessionEndedEvent.java (1)
3-4:userId에@NonNull추가 권장
Long userId를 박싱 타입으로 선언하면null이 허용됩니다. 이벤트 발행 시 항상 유효한 userId가 전달될 것으로 예상되지만, 혹시라도null이 전달될 경우 리스너의catch블록에 의해 조용히 삭제됩니다.long기본 타입 또는@NonNull어노테이션으로 컴파일 타임에 이를 방지할 수 있습니다.💡 개선 제안
-public record StudySessionEndedEvent(Long userId) { +public record StudySessionEndedEvent(long userId) {🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/main/java/com/gpt/geumpumtabackend/study/event/StudySessionEndedEvent.java` around lines 3 - 4, StudySessionEndedEvent currently declares the userId component as boxed Long which allows nulls; change the record to use the primitive type or a non-null annotation to prevent null being passed (e.g., update declaration to "public record StudySessionEndedEvent(long userId) {}" or add an appropriate `@NonNull` annotation on the userId component and import the matching annotation), and adjust any call sites if needed to use primitive long or ensure non-null values.src/main/java/com/gpt/geumpumtabackend/study/event/StudyBadgeGrantEventListener.java (1)
3-15: study 도메인 패키지에서 badge 도메인 서비스를 직접 의존 — 도메인 경계 개선 권장
StudyBadgeGrantEventListener가com.gpt.geumpumtabackend.study.event패키지에 위치하면서badge.service.BadgeService를 직접 임포트하면, 이벤트 기반 설계를 도입한 목적(도메인 간 결합 제거)이 희석됩니다.더 명확한 모듈 경계를 위해 이 리스너를 badge 도메인(예:
com.gpt.geumpumtabackend.badge.event)으로 이동하는 것을 권장합니다. study 도메인은StudySessionEndedEvent만 발행하고, badge 도메인이 해당 이벤트를 수신하여 처리합니다.♻️ 권장 구조
study.event.StudySessionEndedEvent ← study 도메인 (이벤트 발행만) badge.event.StudyBadgeGrantEventListener ← badge 도메인 (이벤트 수신 및 처리)-package com.gpt.geumpumtabackend.study.event; +package com.gpt.geumpumtabackend.badge.event;🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/main/java/com/gpt/geumpumtabackend/study/event/StudyBadgeGrantEventListener.java` around lines 3 - 15, StudyBadgeGrantEventListener currently lives in the study.event package while directly depending on BadgeService; move the listener class to the badge domain (e.g., package com.gpt.geumpumtabackend.badge.event) so it consumes StudySessionEndedEvent and owns BadgeService; update the class package declaration and imports, adjust any Component/RequiredArgsConstructor/Slf4j annotations as needed, and ensure Spring component scanning still picks up the moved StudyBadgeGrantEventListener so the study domain only publishes StudySessionEndedEvent and badge domain handles badge logic.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In
`@src/main/java/com/gpt/geumpumtabackend/study/event/StudyBadgeGrantEventListener.java`:
- Around line 17-24: The grantStudyAchievementBadges method currently performs
DB writes via userBadgeRepository.save but is called from
StudyBadgeGrantEventListener.handleStudySessionEnded during AFTER_COMMIT when no
transaction is active; annotate grantStudyAchievementBadges with
`@Transactional`(propagation = Propagation.REQUIRES_NEW) (importing Propagation)
so it opens a new transaction for its DB work, and ensure the method resides in
a spring-managed bean (badgeService) so the annotation is applied; also consider
letting exceptions bubble (or rethrow) instead of being silently swallowed by
the try/catch in handleStudySessionEnded so failures are visible in logs.
- Around line 17-24: The listener handleStudySessionEnded in
StudyBadgeGrantEventListener is executed synchronously; annotate the method with
`@Async` to run badgeService.grantStudyAchievementBadges(event.userId())
asynchronously after commit, and ensure async support is enabled by adding
`@EnableAsync` to your main application configuration (e.g.,
GeumpumtaBackendApplication). Make sure the class is a Spring-managed bean
(e.g., `@Component/`@Service) so `@Async` is picked up and handle any required
executor configuration if you need a custom thread pool.
---
Nitpick comments:
In
`@src/main/java/com/gpt/geumpumtabackend/study/event/StudyBadgeGrantEventListener.java`:
- Around line 3-15: StudyBadgeGrantEventListener currently lives in the
study.event package while directly depending on BadgeService; move the listener
class to the badge domain (e.g., package com.gpt.geumpumtabackend.badge.event)
so it consumes StudySessionEndedEvent and owns BadgeService; update the class
package declaration and imports, adjust any
Component/RequiredArgsConstructor/Slf4j annotations as needed, and ensure Spring
component scanning still picks up the moved StudyBadgeGrantEventListener so the
study domain only publishes StudySessionEndedEvent and badge domain handles
badge logic.
In
`@src/main/java/com/gpt/geumpumtabackend/study/event/StudySessionEndedEvent.java`:
- Around line 3-4: StudySessionEndedEvent currently declares the userId
component as boxed Long which allows nulls; change the record to use the
primitive type or a non-null annotation to prevent null being passed (e.g.,
update declaration to "public record StudySessionEndedEvent(long userId) {}" or
add an appropriate `@NonNull` annotation on the userId component and import the
matching annotation), and adjust any call sites if needed to use primitive long
or ensure non-null values.
src/main/java/com/gpt/geumpumtabackend/study/event/StudyBadgeGrantEventListener.java
Show resolved
Hide resolved
Juhye0k
approved these changes
Feb 21, 2026
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
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
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.
🚀 1. 개요
📝 2. 주요 변경 사항
getUnnotifiedBadges 메서드의 역할
/api/v1/badge/unnotified로 요청을 보내 이미 지급된 배지이지만, 앱 내 알림을 통해 지급 여부를 전달받지 못한 배지들의 정보를 받습니다. 공부 세션 종료 후에도 동일한 작업을 수행합니다.📸 3. 스크린샷 (API 테스트 결과)
Summary by CodeRabbit
새로운 기능
개선 사항
테스트