-
Notifications
You must be signed in to change notification settings - Fork 0
[FEAT] 프로필 해금로직 변경 #244
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: develop
Are you sure you want to change the base?
[FEAT] 프로필 해금로직 변경 #244
Conversation
Walkthrough사용자의 프로필 이미지 해금 상태를 영구 저장하기 위한 신규 엔티티, 리포지토리, 퍼시스턴스 어댑터, 매퍼, 도메인 모델, 포트를 추가하고, ProfileImageService와 UserService에서 해금/검증 로직을 해당 포트를 통해 사용하도록 변경했다. 해금되지 않은 프로필 선택 시 예외 메시지를 추가했다. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
actor U as User
participant S as ProfileImageService
participant P as PostPort
participant UPR as UnlockedProfileImagePort
U->>S: getAvailableProfileImages(userId)
S->>P: countZzimByUser(userId)
P-->>S: totalZzim
S->>UPR: findUnlockedLevelsByUserId(userId)
UPR-->>S: unlockedLevels(Set)
rect rgba(200,240,255,0.25)
note right of S: 레벨별 해금 필요 여부 계산
loop 각 ProfileImage.level
alt 미해금이고 조건 충족
S->>UPR: saveUnlockedLevel(userId, level)
UPR-->>S: saved
S-->>S: 결과에 unlocked 표시
else 이미 해금 또는 조건 미충족
S-->>S: 상태 반영
end
end
end
S-->>U: 프로필 이미지 목록(해금 상태 포함)
sequenceDiagram
autonumber
actor U as User
participant S as UserService
participant UPR as UnlockedProfileImagePort
U->>S: updateUserProfile(userId, profileLevel, ...)
S->>UPR: isLevelUnlocked(userId, profileLevel)
alt 해금됨
S-->>U: 프로필 업데이트 진행(성공)
else 미해금
S-->>U: throw PROFILE_IMAGE_NOT_UNLOCKED
end
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Pre-merge checks and finishing touches❌ Failed checks (2 warnings)
✅ Passed checks (3 passed)
✨ 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 review |
✅ Actions performedReview triggered.
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 0
🧹 Nitpick comments (7)
src/main/java/com/spoony/spoony_server/adapter/out/persistence/user/db/UnlockedProfileImageEntity.java (1)
25-45: profileLevel 유효성 검증 고려.
profileLevel필드에 대한 제약 조건이 없습니다. 음수나 비정상적인 값이 저장될 가능성이 있습니다.엔티티 레벨에서 기본적인 유효성 검증을 추가하는 것을 고려하세요:
@Column(name = "profile_level", nullable = false) +@jakarta.validation.constraints.Positive private Integer profileLevel;또는 서비스 레이어에서 검증 로직을 추가할 수도 있습니다.
src/main/java/com/spoony/spoony_server/application/service/user/UserService.java (1)
260-281: 프로필 이미지 잠금해제 검증 로직 확인 완료.잠금해제 검증 로직이 올바르게 구현되었습니다. 한 가지 고려사항:
imageLevel이 유효한 범위 내의 값인지 추가 검증을 고려할 수 있습니다 (예: 1-10 범위).유효한 프로필 레벨 범위에 대한 검증을 추가하는 것을 고려하세요:
// 프로필 이미지 레벨 검증 if (command.getImageLevel() != null) { + int level = command.getImageLevel().intValue(); + if (level < 1 || level > ProfileImage.values().length) { + throw new BusinessException(UserErrorMessage.INVALID_PROFILE_LEVEL); + } boolean isUnlocked = unlockedProfileImagePort.isLevelUnlocked( command.getUserId(), - command.getImageLevel().intValue() + level );src/main/java/com/spoony/spoony_server/application/service/user/ProfileImageService.java (1)
28-67: 영구 잠금해제 로직 구현 확인 완료.잠금해제 로직이 영구 반영 요구사항을 올바르게 구현했습니다. 한 번 잠금해제된 프로필은 찜 수가 감소해도 잠금해제 상태를 유지합니다.
동시성 시나리오에 대한 고려사항: Line 53의
saveUnlockedLevel호출 시 동시 요청으로 인해 유니크 제약조건 위반이 발생할 수 있습니다. 현재 persistence adapter에서 중복 체크를 하지만, 체크와 저장 사이에 race condition이 존재합니다. 더 강력한 동시성 제어가 필요하다면 다음을 고려하세요:
- 트랜잭션 격리 수준 조정 (예:
@Transactional(isolation = Isolation.SERIALIZABLE))- 또는 persistence adapter의
saveUnlockedLevel에서 constraint violation exception을 catch하여 무시현재 구현도 unique constraint가 데이터 무결성을 보장하므로 동작에는 문제없으나, 예외 처리를 개선하면 사용자 경험이 향상될 수 있습니다.
src/main/java/com/spoony/spoony_server/adapter/out/persistence/user/UnlockedProfileImagePersistenceAdapter.java (1)
33-52: 동시성 시나리오에서 제약조건 위반 예외 처리 고려.중복 저장 방지를 위한 체크(Line 37)가 있지만, 체크와 저장 사이에 race condition이 존재합니다. 두 개의 동시 요청이 모두 체크를 통과하면 유니크 제약조건 위반이 발생합니다.
DataIntegrityViolationException을 처리하여 더 우아한 동시성 제어를 구현하세요:+import org.springframework.dao.DataIntegrityViolationException; + @Override @Transactional public void saveUnlockedLevel(Long userId, Integer profileLevel) { // 중복 저장 방지 if (isLevelUnlocked(userId, profileLevel)) { log.debug("Profile level {} already unlocked for user {}", profileLevel, userId); return; } UserEntity user = userRepository.findById(userId) .orElseThrow(() -> new BusinessException(UserErrorMessage.USER_NOT_FOUND)); UnlockedProfileImageEntity entity = UnlockedProfileImageEntity.builder() .user(user) .profileLevel(profileLevel) .build(); - unlockedProfileImageRepository.save(entity); - log.info("Profile level {} unlocked for user {}", profileLevel, userId); + try { + unlockedProfileImageRepository.save(entity); + log.info("Profile level {} unlocked for user {}", profileLevel, userId); + } catch (DataIntegrityViolationException e) { + // 동시 요청으로 인해 이미 저장된 경우 무시 (idempotent) + log.debug("Profile level {} already unlocked by concurrent request for user {}", profileLevel, userId); + } }이렇게 하면 동시 요청 시에도 안전하게 처리되며, 사용자에게 불필요한 에러가 전파되지 않습니다.
src/main/java/com/spoony/spoony_server/adapter/out/persistence/user/mapper/UnlockedProfileImageMapper.java (1)
8-19: UserMapper.toDomain은 null 전달 시 NPE 발생. UnlockedProfileImageEntity.user가 nullable=false로 보장되지만, null 가능성까지 방어하려면User user = entity.getUser() == null ? null : UserMapper.toDomain(entity.getUser());와 같이 처리하는 것을 고려하세요.
src/main/java/com/spoony/spoony_server/adapter/out/persistence/user/db/UnlockedProfileImageRepository.java (2)
26-27: 프로젝션 쿼리에 정렬을 추가하세요.프로필 레벨만 조회하는 효율적인 프로젝션 쿼리입니다. 그러나 결과에 정렬 순서가 없어 레벨이 일관성 없이 반환될 수 있습니다. 사용자에게 보여줄 때 오름차순으로 정렬하는 것이 좋습니다.
다음과 같이
ORDER BY를 추가하세요:- @Query("SELECT u.profileLevel FROM UnlockedProfileImageEntity u WHERE u.user.userId = :userId") + @Query("SELECT u.profileLevel FROM UnlockedProfileImageEntity u WHERE u.user.userId = :userId ORDER BY u.profileLevel ASC") List<Integer> findProfileLevelsByUserId(@Param("userId") Long userId);
16-16: 정렬 순서 추가 고려
findByUser_UserId메서드는 정렬을 명시하지 않아 결과가 임의 순서로 반환될 수 있습니다. 프로필 레벨 순서나 잠금 해제 시간 순으로 정렬을 추가하세요.
예:- List<UnlockedProfileImageEntity> findByUser_UserId(Long userId); + List<UnlockedProfileImageEntity> findByUser_UserIdOrderByProfileLevelAsc(Long userId);
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (9)
src/main/java/com/spoony/spoony_server/adapter/out/persistence/user/UnlockedProfileImagePersistenceAdapter.java(1 hunks)src/main/java/com/spoony/spoony_server/adapter/out/persistence/user/db/UnlockedProfileImageEntity.java(1 hunks)src/main/java/com/spoony/spoony_server/adapter/out/persistence/user/db/UnlockedProfileImageRepository.java(1 hunks)src/main/java/com/spoony/spoony_server/adapter/out/persistence/user/mapper/UnlockedProfileImageMapper.java(1 hunks)src/main/java/com/spoony/spoony_server/application/port/out/user/UnlockedProfileImagePort.java(1 hunks)src/main/java/com/spoony/spoony_server/application/service/user/ProfileImageService.java(1 hunks)src/main/java/com/spoony/spoony_server/application/service/user/UserService.java(9 hunks)src/main/java/com/spoony/spoony_server/domain/user/UnlockedProfileImage.java(1 hunks)src/main/java/com/spoony/spoony_server/global/message/business/UserErrorMessage.java(1 hunks)
🧰 Additional context used
📓 Path-based instructions (1)
**/*.java
⚙️ CodeRabbit configuration file
**/*.java: 다음 규칙은 '일회성 체크'로 위반 패턴만 지적(지속적 강제 X). hackday-conventions-java 기준: - 탭 들여쓰기(indent_style=tab), indent_size=4 - 한 줄 최대 120자 (max_line_length=120) - 파일 끝 개행 (newline-eof) - 주석 전후 공백 (space-around-comment) - 중괄호 K&R 스타일 참고: https://naver.github.io/hackday-conventions-java/
Files:
src/main/java/com/spoony/spoony_server/domain/user/UnlockedProfileImage.javasrc/main/java/com/spoony/spoony_server/adapter/out/persistence/user/mapper/UnlockedProfileImageMapper.javasrc/main/java/com/spoony/spoony_server/application/service/user/UserService.javasrc/main/java/com/spoony/spoony_server/adapter/out/persistence/user/db/UnlockedProfileImageEntity.javasrc/main/java/com/spoony/spoony_server/application/port/out/user/UnlockedProfileImagePort.javasrc/main/java/com/spoony/spoony_server/adapter/out/persistence/user/db/UnlockedProfileImageRepository.javasrc/main/java/com/spoony/spoony_server/application/service/user/ProfileImageService.javasrc/main/java/com/spoony/spoony_server/global/message/business/UserErrorMessage.javasrc/main/java/com/spoony/spoony_server/adapter/out/persistence/user/UnlockedProfileImagePersistenceAdapter.java
🔇 Additional comments (10)
src/main/java/com/spoony/spoony_server/global/message/business/UserErrorMessage.java (1)
12-13: LGTM!프로필 이미지 잠금해제 검증을 위한 에러 메시지가 적절하게 추가되었습니다.
src/main/java/com/spoony/spoony_server/domain/user/UnlockedProfileImage.java (1)
8-15: LGTM!도메인 모델이 간결하고 적절하게 정의되었습니다.
src/main/java/com/spoony/spoony_server/application/port/out/user/UnlockedProfileImagePort.java (1)
5-28: LGTM!포트 인터페이스가 명확하게 정의되었으며, 메서드 시그니처와 Javadoc이 적절합니다.
src/main/java/com/spoony/spoony_server/application/service/user/UserService.java (1)
10-11: LGTM!
UnlockedProfileImagePort의존성이 적절하게 추가되었습니다.Also applies to: 43-43
src/main/java/com/spoony/spoony_server/application/service/user/ProfileImageService.java (1)
3-26: LGTM!의존성 주입과 필요한 import가 적절하게 추가되었습니다.
src/main/java/com/spoony/spoony_server/adapter/out/persistence/user/UnlockedProfileImagePersistenceAdapter.java (2)
19-31: LGTM!Adapter 구조와
findUnlockedLevelsByUserId구현이 적절합니다.
54-58: LGTM!
isLevelUnlocked메서드가 간결하고 적절하게 구현되었습니다.src/main/java/com/spoony/spoony_server/adapter/out/persistence/user/db/UnlockedProfileImageEntity.java (1)
13-23: JPA Auditing 활성화 확인됨
@EnableJpaAuditing이JpaAuditingConfig.java에 선언되어 있어@CreatedDate가 정상 작동합니다.src/main/java/com/spoony/spoony_server/adapter/out/persistence/user/db/UnlockedProfileImageRepository.java (2)
10-11: 리포지토리 인터페이스 구조가 올바릅니다.Spring Data JPA의 표준 패턴을 따르고 있으며,
@Repository애노테이션과 제네릭 타입 선언이 적절합니다.
21-21: 효율적인 존재 여부 확인 메서드입니다.
exists쿼리는 전체 엔티티를 로드하지 않고 존재 여부만 확인하므로 성능상 효율적입니다. PR 설명에 따르면UNIQUE(user_id, profile_level)제약조건이 있어 이 쿼리는 최적화될 것으로 예상됩니다.
| List<Post> postList = postPort.findPostsByUserId(userId); | ||
| long totalZzimCount = postList.stream() | ||
| .mapToLong(Post::getZzimCount) | ||
| .sum(); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
총 찜 개수를 스트림을 사용해서 계산하는 것도 좋지만, 게시글이 많아졌을 때를 고려해서 쿼리문에서 sum을 이용해서 하는 것도 좋을 것 같습니다...!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
해당 사항 좋은 포인트 같습니다! 검토 후 반영해보겠습니다!
| // 새로 잠금해제 조건 달성 시 저장 | ||
| if (!wasUnlocked && canUnlock) { | ||
| unlockedProfileImagePort.saveUnlockedLevel(userId, level); | ||
| result.add(ProfileImageResponseDTO.of(profileImage, true)); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
프로필을 조회할 때 값을 넘겨주는 부분에 있어서는 문제가 없을 것 같은데, 새로운 프로필이 해금 되었을 때 DB에 중복 저장이 될 것 같아요. 예를 들어서 현재 레벨이 2인데 3으로 레벨이 올라가면 DB에는 (userId, 2)가 있는 상태에서 (userId, 3)이 추가로 저장되는 것 같습니다. 혹시 제가 생각하는 로직이 맞다면 기존 레벨은 지우는 것은 어떨까요?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
해당 부분 충분히 이해했습니다!
맞습니다! 현재 로직은 DB에 해금에 대한 내역이 히스토리 형식으로 저장됩니다. 추후 사용자에게 해금에 대해 알림을 보내는 것을 고려하여 이렇게 구현했습니다! 이 부분에 대해서는 일요일 회의에서 한번 논의해봐도 좋을 것 같습니다!
|
LGTM |
|
지금 로직 검토 필요 타인의 동작에 의해 해금이 되는데 데이터베이서 업데이트를 찜 활동에서 하고 있는지 파악할 것 |
🔍️ 이 PR을 통해 해결하려는 문제가 무엇인가요?
문제 상황
기존 프로필 이미지 잠금해제 로직은 매번 동적으로 계산하는 방식
해결 방안
영구 잠금해제 시스템으로 변경:
✨ 이 PR에서 핵심적으로 변경된 사항은 무엇일까요?
새로운 DB 테이블 추가
unlocked_profile_image테이블 생성비즈니스 로직 개선
ProfileImageService:
UserService:
🔖 핵심 변경 사항 외에 추가적으로 변경된 부분이 있나요?
🙏 Reviewer 분들이 이런 부분을 신경써서 봐 주시면 좋겠어요
비즈니스 로직 검증
🩺 이 PR에서 테스트 혹은 검증이 필요한 부분이 있을까요?
📌 PR 진행 시 이러한 점들을 참고해 주세요
📝 Assignee를 위한 CheckList
Summary by CodeRabbit