-
Notifications
You must be signed in to change notification settings - Fork 0
feat: 앨범 입장 로직 구현 #13
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
Conversation
Walkthrough앨범 초대 조회와 입장 기능이 추가됨. Album·UserAlbum·Photo 도메인과 리포지토리(원자적 참여자 증가 포함), DTO·매퍼·검증기·예외·에러코드·성공코드, AlbumService·PhotoService, 컨트롤러·스웨거, 보안 화이트리스트가 도입됨. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
actor User
participant Controller as AlbumController
participant Service as AlbumService
participant Validator as AlbumValidator
participant AlbumRepo as AlbumRepository
participant UserAlbumRepo as UserAlbumRepository
participant PhotoService as PhotoService
participant Mapper as AlbumMapper
rect rgb(245,248,255)
note over User,Controller: 초대 정보 조회 (공개)
User->>Controller: GET /v1/album/{code}/invitation
Controller->>Service: getInvitationInfo(code)
Service->>Validator: validateAlbumCode(code)
Validator->>AlbumRepo: findByCode(code)
AlbumRepo-->>Validator: Album
Service->>Mapper: toInvitationResponse(album, host)
Mapper-->>Service: AlbumInvitationResponse
Service-->>Controller: AlbumInvitationResponse
Controller-->>User: 200 OK
end
rect rgb(245,255,245)
note over User,Controller: 앨범 입장 (인증)
User->>Controller: POST /v1/album/{code}/enter (CurrentUser)
Controller->>Service: enterAlbum(code, currentUser)
Service->>Validator: validateAlbumCode(code)
Validator->>AlbumRepo: findByCode(code)
AlbumRepo-->>Validator: Album
Service->>Validator: validateAlbumEntry(album, currentUser)
Service->>UserAlbumRepo: findByUserIdAndAlbumId(userId, albumId)
alt 비참가자
Service->>Mapper: toGuestUserAlbum(user, album)
Mapper-->>Service: UserAlbum
Service->>UserAlbumRepo: save(UserAlbum)
Service->>AlbumRepo: incrementParticipantCountAtomically(albumId)
AlbumRepo-->>Service: updatedCount (int)
alt 증가 실패
Service-->>User: throw ALBUM_MAX_PARTICIPANT_REACHED
end
else 이미 참가자
UserAlbumRepo-->>Service: existing UserAlbum
end
Service->>PhotoService: countTotalPhotos(albumId)
PhotoService-->>Service: totalPhotoCount
Service->>PhotoService: getRecentPhotoUrls(albumId)
PhotoService-->>Service: recentPhotoUrls
Service->>UserAlbumRepo: findAllByAlbumId(albumId)
UserAlbumRepo-->>Service: participants
Service->>Mapper: toEnterResponse(...)
Mapper-->>Service: AlbumEnterResponse
Service-->>Controller: AlbumEnterResponse
Controller-->>User: 200 OK
end
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related issues
Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
Comment |
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: 5
🧹 Nitpick comments (10)
src/main/java/com/cheeeese/album/domain/UserAlbumRole.java (1)
3-7: 선택사항: enum 값에 대한 문서화를 고려해보세요.
BLACK역할의 의미(블랙리스트된 사용자)가 명확하지 않을 수 있습니다. JavaDoc 주석을 추가하면 코드 가독성이 향상됩니다.+/** + * 사용자의 앨범 내 역할을 정의합니다. + */ public enum UserAlbumRole { + /** 앨범 호스트 (생성자) */ HOST, + /** 일반 게스트 */ GUEST, + /** 블랙리스트된 사용자 */ BLACK }src/main/java/com/cheeeese/album/dto/response/AlbumInvitationResponse.java (1)
26-27: 선택사항: null 허용 필드에 대한 스키마 명시를 고려하세요.호스트가 프로필 이미지를 설정하지 않은 경우
hostProfileImage가 null일 수 있습니다. API 문서 명확성을 위해@Schema에nullable = true를 추가하는 것을 고려하세요.- @Schema(description = "호스트 프로필 이미지 URL", example = "http://example.com/host_profile.png") + @Schema(description = "호스트 프로필 이미지 URL", example = "http://example.com/host_profile.png", nullable = true) String hostProfileImagesrc/main/java/com/cheeeese/album/presentation/swagger/AlbumSwagger.java (1)
109-135: 400 응답 예시 메시지를 실제 오류 메시지와 맞춰 주세요.
AlbumErrorCode에는ALBUM_EXPIRED(“만료된 앨범입니다.”)와ALBUM_MAX_PARTICIPANT_REACHED(“앨범의 최대 참가 인원수를 초과했습니다.”)가 각각 정의되어 있는데, 현재 예시 JSON은 “앨범이 만료되었거나 최대 인원을 초과했습니다.”로 통합 표기되어 있습니다. 실제 반환 메시지와 일치하도록 예시를 분리하거나 문구를 정리해 주시면 API 소비자가 혼동하지 않을 것 같습니다.src/main/java/com/cheeeese/album/dto/response/AlbumEnterResponse.java (1)
42-43: 최근 사진 개수 제한을 명확히 문서화하세요.주석에 "최대 9개 등"이라고 되어 있는데, 정확한 개수를 명시하는 것이 좋습니다.
PhotoService.getRecentPhotoUrls에서 실제로 최대 9개를 반환하므로 주석을 "최근 사진 미리보기 URL 목록 (최대 9개)"로 수정하는 것을 권장합니다.- @Schema(description = "최근 사진 미리보기 URL 목록 (최대 9개 등)") + @Schema(description = "최근 사진 미리보기 URL 목록 (최대 9개)") List<String> recentPhotoUrlssrc/main/java/com/cheeeese/album/domain/Album.java (1)
50-51: 불필요한 기본값 할당을 제거하세요.필드 선언에
= 0을 지정하고 빌더 생성자에도currentPhotoCount파라미터를 받고 있어 중복됩니다. 빌더 패턴을 사용할 때는 필드 레벨 기본값보다는 빌더에서 기본값을 설정하는 것이 명확합니다.@Column(name = "current_photo_count", nullable = false) - private int currentPhotoCount = 0; + private int currentPhotoCount;또는 빌더에서 기본값 처리:
@Builder public Album(...) { ... this.currentPhotoCount = currentPhotoCount != null ? currentPhotoCount : 0; ... }src/main/java/com/cheeeese/album/application/AlbumService.java (3)
82-83: TODO를 이슈로 추적하세요.Soft delete 논의가 필요하다는 TODO가 있습니다. PR 설명에도 언급되어 있으니 별도 이슈로 등록하여 추적하는 것이 좋겠습니다.
이슈 생성을 도와드릴까요? 다음 내용으로 이슈를 만들 수 있습니다:
- 제목: "앨범 사진 개수 집계 시 soft delete 처리 방식 논의"
- 내용: Photo 테이블의 soft delete된 레코드를 총 사진 개수에 포함할지 여부 결정 필요
89-90: TODO를 이슈로 추적하세요.썸네일 이미지 생성 후 제공하도록 변경 예정이라는 TODO가 있습니다. PR 설명의 "추후 제공 예정" 내용과 일치하며, 별도 이슈로 등록하여 추적하세요.
이슈 생성을 도와드릴까요?
106-109: 스트림 연산을 단순화하세요.메서드 참조를 사용하면 더 간결하고 읽기 쉬운 코드가 됩니다.
List<Long> participantUserIds = userAlbumRepository.findAllByAlbumId(albumId).stream() - .map(UserAlbum::getUserId) - .collect(Collectors.toList()); + .map(UserAlbum::getUserId) + .toList();참고: Java 16+에서는
.collect(Collectors.toList())대신.toList()를 사용할 수 있습니다.src/main/java/com/cheeeese/album/infrastructure/mapper/AlbumMapper.java (2)
14-14: 유틸리티 클래스에 private 생성자를 추가하세요.정적 메서드만 포함하는 유틸리티 클래스는 인스턴스화를 방지하기 위해 private 생성자를 추가하는 것이 모범 사례입니다.
public class AlbumMapper { + + private AlbumMapper() { + throw new UnsupportedOperationException("Utility class"); + } /** * Album 엔티티와 Host User 정보를 초대장 응답 DTO로 변환합니다.
14-85: 장기적으로 MapStruct 도입을 검토하세요.현재 수동 매핑 방식도 잘 작동하지만, 프로젝트가 성장하면서 매퍼가 많아질 경우 MapStruct와 같은 컴파일 타임 매핑 라이브러리 사용을 고려해보세요. 이는 다음과 같은 이점을 제공합니다:
- 타입 안정성 향상
- 보일러플레이트 코드 감소
- 컴파일 타임 검증
- 일관된 매핑 패턴
현재 코드는 문제없으며, 이는 향후 개선 사항으로 고려할 수 있는 아키텍처 제안입니다.
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (19)
src/main/java/com/cheeeese/album/application/AlbumService.java(1 hunks)src/main/java/com/cheeeese/album/application/validator/AlbumValidator.java(1 hunks)src/main/java/com/cheeeese/album/domain/Album.java(1 hunks)src/main/java/com/cheeeese/album/domain/UserAlbum.java(1 hunks)src/main/java/com/cheeeese/album/domain/UserAlbumRole.java(1 hunks)src/main/java/com/cheeeese/album/dto/response/AlbumEnterResponse.java(1 hunks)src/main/java/com/cheeeese/album/dto/response/AlbumInvitationResponse.java(1 hunks)src/main/java/com/cheeeese/album/exception/AlbumException.java(1 hunks)src/main/java/com/cheeeese/album/exception/code/AlbumErrorCode.java(1 hunks)src/main/java/com/cheeeese/album/infrastructure/mapper/AlbumMapper.java(1 hunks)src/main/java/com/cheeeese/album/infrastructure/persistence/AlbumRepository.java(1 hunks)src/main/java/com/cheeeese/album/infrastructure/persistence/UserAlbumRepository.java(1 hunks)src/main/java/com/cheeeese/album/presentation/AlbumController.java(1 hunks)src/main/java/com/cheeeese/album/presentation/swagger/AlbumSwagger.java(1 hunks)src/main/java/com/cheeeese/global/common/code/SuccessCode.java(1 hunks)src/main/java/com/cheeeese/global/config/SecurityConfig.java(1 hunks)src/main/java/com/cheeeese/photo/application/PhotoService.java(1 hunks)src/main/java/com/cheeeese/photo/domain/Photo.java(1 hunks)src/main/java/com/cheeeese/photo/infrastructure/persistence/PhotoRepository.java(1 hunks)
🧰 Additional context used
🧬 Code graph analysis (3)
src/main/java/com/cheeeese/album/exception/code/AlbumErrorCode.java (1)
src/main/java/com/cheeeese/album/exception/AlbumException.java (1)
Getter(7-12)
src/main/java/com/cheeeese/photo/application/PhotoService.java (1)
src/main/java/com/cheeeese/album/application/AlbumService.java (1)
Service(24-117)
src/main/java/com/cheeeese/album/application/AlbumService.java (2)
src/main/java/com/cheeeese/album/infrastructure/mapper/AlbumMapper.java (1)
AlbumMapper(14-85)src/main/java/com/cheeeese/photo/application/PhotoService.java (1)
Service(12-29)
🔇 Additional comments (7)
src/main/java/com/cheeeese/global/config/SecurityConfig.java (1)
38-39: 앨범 초대장 조회 엔드포인트가 화이트리스트에 적절히 추가되었습니다.비로그인 사용자의 초대장 조회를 위한 엔드포인트가 올바르게 허용되었습니다. 와일드카드 패턴(
/v1/album/*/invitation)이 앨범 코드 기반 라우팅에 적합하게 설정되었습니다.src/main/java/com/cheeeese/global/common/code/SuccessCode.java (1)
23-25: LGTM!앨범 관련 성공 코드가 기존 패턴에 맞게 적절히 추가되었습니다.
src/main/java/com/cheeeese/album/exception/AlbumException.java (1)
7-12: LGTM!예외 클래스가 기존 패턴을 따라 적절히 구현되었습니다.
src/main/java/com/cheeeese/photo/domain/Photo.java (1)
43-56: 빌더에서 불변 필드 초기화가 적절합니다.
likesCnt와isDeleted의 기본값 설정이 올바르게 구현되었습니다. 새 사진은 좋아요 수 0, 삭제되지 않은 상태로 시작합니다.src/main/java/com/cheeeese/album/infrastructure/persistence/AlbumRepository.java (1)
8-10: 고유 제약조건 확인: Album 엔티티의code필드에@Column(unique = true)가 설정돼 있어 JPA DDL 생성 시 고유 인덱스가 자동 적용됩니다. 별도 마이그레이션 스크립트를 사용 중이라면, 해당 스크립트에code컬럼 인덱스 생성 로직이 포함됐는지 확인하세요.src/main/java/com/cheeeese/album/presentation/AlbumController.java (1)
20-41: LGTM! 컨트롤러 구현이 깔끔합니다.REST API 구조가 명확하고, 책임 분리가 잘 되어 있습니다.
/invitation엔드포인트는 인증 없이 접근 가능하고,/enter엔드포인트는@CurrentUser를 통해 인증된 사용자만 접근하도록 설계되어 있어 PR 목표와 일치합니다.src/main/java/com/cheeeese/album/application/validator/AlbumValidator.java (1)
20-40: 검증 로직이 잘 구조화되어 있습니다.검증 메서드들이 명확하게 분리되어 있고, 각각의 책임이 잘 정의되어 있습니다. 만료, 블랙리스트, 정원 초과 검증이 순차적으로 수행되는 구조가 적절합니다.
다만, 검증(
validateAlbumEntry)과 실제 저장(handleAlbumParticipation) 사이에 시간차가 있어 TOCTOU(Time-of-check to time-of-use) 문제가 발생할 수 있습니다. 이는AlbumService.java의 동시성 이슈와 연관되어 있으니 함께 검토하시기 바랍니다.
| @Column(name = "is_deleted", nullable = false) | ||
| private boolean isDeleted; |
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.
소프트 딜리트 컬럼에 인덱스 추가를 고려하세요.
isDeleted 필드는 소프트 딜리트 패턴에 사용되며, 대부분의 쿼리가 WHERE is_deleted = false 조건으로 필터링할 것입니다. 사진 데이터가 증가하면 이 컬럼에 인덱스가 없을 경우 쿼리 성능이 저하될 수 있습니다.
엔티티에 인덱스 정의를 추가하거나 데이터베이스 마이그레이션에서 인덱스를 생성하세요:
@Entity
@Getter
-@Table(name = "photo")
+@Table(name = "photo", indexes = {
+ @Index(name = "idx_photo_is_deleted", columnList = "is_deleted"),
+ @Index(name = "idx_photo_album_active", columnList = "album_id, is_deleted, capture_time")
+})
@NoArgsConstructor(access = AccessLevel.PROTECTED)
public class Photo extends BaseEntity {복합 인덱스 (album_id, is_deleted, capture_time)는 앨범별 최근 활성 사진 조회 쿼리를 최적화합니다.
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| @Column(name = "is_deleted", nullable = false) | |
| private boolean isDeleted; | |
| @Entity | |
| @Getter | |
| @Table(name = "photo", indexes = { | |
| @Index(name = "idx_photo_is_deleted", columnList = "is_deleted"), | |
| @Index(name = "idx_photo_album_active", columnList = "album_id, is_deleted, capture_time") | |
| }) | |
| @NoArgsConstructor(access = AccessLevel.PROTECTED) | |
| public class Photo extends BaseEntity { | |
| // existing fields and methods... |
🤖 Prompt for AI Agents
In src/main/java/com/cheeeese/photo/domain/Photo.java around lines 40-41, the
soft-delete boolean field isDeleted is not indexed which will slow queries
filtering WHERE is_deleted = false as photo volume grows; add an index for
is_deleted (and consider a composite index on album_id, is_deleted,
capture_time) by either adding a JPA @Table(indexes = ...) definition on the
entity or creating a database migration that creates the index(es); update
schema/migration files, run the migration, and ensure any existing query plans
use the new index.
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.
추후 업로드, 다운로드 로직을 구현할때 고려할 예정
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.
@dahyun24, 알겠습니다! 업로드, 다운로드 로직 구현 시 고려하시는 것이 합리적인 접근입니다. 해당 기능 구현 단계에서 쿼리 패턴과 함께 인덱스 전략을 검토하시면 됩니다. 👍
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: 2
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
src/main/java/com/cheeeese/album/application/AlbumService.java(1 hunks)src/main/java/com/cheeeese/album/domain/Album.java(1 hunks)src/main/java/com/cheeeese/album/domain/UserAlbum.java(1 hunks)src/main/java/com/cheeeese/album/exception/code/AlbumErrorCode.java(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (2)
- src/main/java/com/cheeeese/album/domain/UserAlbum.java
- src/main/java/com/cheeeese/album/domain/Album.java
🧰 Additional context used
🧬 Code graph analysis (2)
src/main/java/com/cheeeese/album/exception/code/AlbumErrorCode.java (1)
src/main/java/com/cheeeese/album/exception/AlbumException.java (1)
Getter(7-12)
src/main/java/com/cheeeese/album/application/AlbumService.java (2)
src/main/java/com/cheeeese/album/infrastructure/mapper/AlbumMapper.java (1)
AlbumMapper(14-85)src/main/java/com/cheeeese/photo/application/PhotoService.java (1)
Service(12-29)
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: 3
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
src/main/java/com/cheeeese/album/application/AlbumService.java(1 hunks)src/main/java/com/cheeeese/album/application/validator/AlbumValidator.java(1 hunks)src/main/java/com/cheeeese/album/domain/Album.java(1 hunks)src/main/java/com/cheeeese/album/infrastructure/persistence/AlbumRepository.java(1 hunks)
🧰 Additional context used
🧬 Code graph analysis (2)
src/main/java/com/cheeeese/album/domain/Album.java (1)
src/main/java/com/cheeeese/album/domain/UserAlbum.java (1)
Entity(10-39)
src/main/java/com/cheeeese/album/application/AlbumService.java (2)
src/main/java/com/cheeeese/album/infrastructure/mapper/AlbumMapper.java (1)
AlbumMapper(14-85)src/main/java/com/cheeeese/photo/application/PhotoService.java (1)
Service(12-29)
src/main/java/com/cheeeese/album/infrastructure/persistence/AlbumRepository.java
Outdated
Show resolved
Hide resolved
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
♻️ Duplicate comments (1)
src/main/java/com/cheeeese/album/application/AlbumService.java (1)
66-74: 사전 조회(find-then-save) 제거 권장 — 레이스와 불필요한 IO.
findByUserIdAndAlbumId로 미리 확인하는 패턴은 동시에 빈 Optional을 받는 레이스를 막지 못하고 DB 왕복만 추가합니다. 유니크 제약 + 예외 처리에 맡기고 이 블록은 제거하는 편이 명확합니다.적용 예시:
- boolean isAlreadyParticipant = userAlbumRepository.findByUserIdAndAlbumId(currentUser.getId(), album.getId()).isPresent(); - - if (isAlreadyParticipant) { - log.info("User {} is already a participant of album {}. Skipping registration.", - currentUser.getId(), album.getId()); - return album; - }
🧹 Nitpick comments (3)
src/main/java/com/cheeeese/album/application/AlbumService.java (3)
75-87: 중복 입장 멱등 처리 필요 시 REQUIRES_NEW로 분리하세요.현재는 유니크 제약 위반 시 예외를 던져 전체 트랜잭션을 롤백시킵니다. 멱등 성공으로 처리하려면 INSERT를 별도 트랜잭션에서 시도해 제약 위반을 안전하게 흡수해야 합니다. 단순 try-catch로 계속 진행하면 트랜잭션이 rollback-only로 표시되어 커밋 시
UnexpectedRollbackException이 발생할 수 있습니다.적용 가이드:
- 별도 메서드로 분리:
@Transactional(propagation = REQUIRES_NEW)로userAlbumRepository.save(...)수행, 성공 여부(boolean) 반환.- 메인 트랜잭션에서는 성공 시에만
incrementParticipantCountAtomically호출, 실패(이미 참가) 시에는 증가 없이 현재 앨범만 조회해 반환.예시 코드(새 메서드 추가):
@Transactional(propagation = Propagation.REQUIRES_NEW) public boolean tryCreateGuestRelation(User user, Album album) { try { userAlbumRepository.save(AlbumMapper.toGuestUserAlbum(user, album)); return true; } catch (DataIntegrityViolationException e) { return false; // 이미 참가 } }메인 흐름:
if (tryCreateGuestRelation(currentUser, album)) { int updated = albumRepository.incrementParticipantCountAtomically(album.getId()); if (updated == 0) throw new AlbumException(AlbumErrorCode.ALBUM_MAX_PARTICIPANT_REACHED); } // 여기서는 항상 응답 생성으로 진행 (멱등)
103-113: 소프트 삭제/썸네일 TODO는 이슈로 트래킹하세요.
- 총 사진 수: 소프트 삭제 정책 확정 후 쿼리 일관성 재검토 필요.
- 썸네일: 생성/보관 전략과 URL 제공 시점을 정의해 API 계약을 안정화하세요(프로필 이미지에도 동일 정책 적용 고려).
원하시면 작업 항목을 이슈 템플릿으로 정리해 드립니다.
128-138: 참가자 조회 최적화/정렬은 선택사항입니다.현재는 ID 리스트 조회 후
findAllById두 번 왕복합니다. 규모가 커지면 JOIN 기반 단일 쿼리나 정렬(예: 참가일 최신순) 추가를 고려해도 좋습니다.
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
src/main/java/com/cheeeese/album/application/AlbumService.java(1 hunks)src/main/java/com/cheeeese/album/exception/code/AlbumErrorCode.java(1 hunks)src/main/java/com/cheeeese/album/infrastructure/persistence/AlbumRepository.java(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- src/main/java/com/cheeeese/album/exception/code/AlbumErrorCode.java
🧰 Additional context used
🧬 Code graph analysis (1)
src/main/java/com/cheeeese/album/application/AlbumService.java (2)
src/main/java/com/cheeeese/album/infrastructure/mapper/AlbumMapper.java (1)
AlbumMapper(14-85)src/main/java/com/cheeeese/photo/application/PhotoService.java (1)
Service(12-29)
🔇 Additional comments (3)
src/main/java/com/cheeeese/album/infrastructure/persistence/AlbumRepository.java (1)
14-16: JPQL 벌크 업데이트 후 1차 캐시 정리/플러시 보장 좋습니다.
clearAutomatically,flushAutomatically설정으로 일관성과 동시성 안정성이 확보되었습니다. LGTM.src/main/java/com/cheeeese/album/application/AlbumService.java (2)
51-64: 중복 입장 처리 방침 확인 필요 (에러 vs. 멱등 성공).현재 중복 입장은
AlbumException(USER_ALREADY_JOINED_CONCURRENTLY)로 에러 처리됩니다. 제품 요구가 “이미 참가”를 성공(멱등)으로 간주해야 한다면, 트랜잭션 경계/전파 전략을 조정해야 합니다. 결정만 확인 부탁드립니다.
89-94: 벌크 업데이트 후 재조회로 최신 상태 사용 — 적절합니다.
@Modifying(clearAutomatically = true)이후findById로 최신 앨범을 사용하도록 한 점이 데이터 일관성 측면에서 올바릅니다.
zyovn
left a comment
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.
수고하셧습니다 🩻🧀🫕
src/main/java/com/cheeeese/album/presentation/AlbumController.java
Outdated
Show resolved
Hide resolved
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 (6)
src/main/java/com/cheeeese/album/domain/UserAlbum.java (2)
21-27: 외래키 제약조건 추가를 고려하세요.현재
userId와albumId가 단순 Long 타입으로 정의되어 있어 JPA 레벨에서의 참조 무결성 검증이 없습니다. TODO 주석에 명시된 대로 향후@ManyToOne관계를 추가할 계획이라면, 데이터베이스 마이그레이션에 외래키 제약조건을 추가하여 참조 무결성을 보장하는 것을 권장합니다.예시 (Flyway/Liquibase 마이그레이션):
ALTER TABLE user_album ADD CONSTRAINT fk_user_album_user FOREIGN KEY (user_id) REFERENCES users(user_id); ALTER TABLE user_album ADD CONSTRAINT fk_user_album_album FOREIGN KEY (album_id) REFERENCES album(album_id);
33-38: 빌더에 null 검증을 추가하는 것을 고려하세요.현재 빌더 생성자는 매개변수에 대한 null 검증을 수행하지 않습니다. 데이터베이스 제약조건이 null을 거부하지만, 애플리케이션 레벨에서 조기에 검증하면 더 명확한 오류 메시지를 제공할 수 있습니다.
적용 예시:
@Builder private UserAlbum(Long userId, Long albumId, UserAlbumRole role) { + if (userId == null || albumId == null || role == null) { + throw new IllegalArgumentException("userId, albumId, role must not be null"); + } this.userId = userId; this.albumId = albumId; this.role = role; }또는 Spring의
@NonNull어노테이션을 활용할 수도 있습니다:@Builder -private UserAlbum(Long userId, Long albumId, UserAlbumRole role) { +private UserAlbum(@NonNull Long userId, @NonNull Long albumId, @NonNull UserAlbumRole role) { this.userId = userId; this.albumId = albumId; this.role = role; }src/main/java/com/cheeeese/album/presentation/AlbumController.java (1)
26-28: 경로 변수에 대한 유효성 검증을 추가하세요.
code파라미터에 대한 형식 검증이 없습니다. 잘못된 형식의 코드가 서비스 레이어까지 전달되면 불필요한 DB 조회가 발생할 수 있습니다.다음과 같이 Bean Validation을 추가하는 것을 고려하세요:
+import jakarta.validation.constraints.NotBlank; +import jakarta.validation.constraints.Pattern; +import org.springframework.validation.annotation.Validated; + +@Validated @RestController @RequiredArgsConstructor @RequestMapping("/v1/album") public class AlbumController implements AlbumSwagger { @Override @GetMapping("/{code}/invitation") - public CommonResponse<AlbumInvitationResponse> getInvitationInfo(@PathVariable String code) { + public CommonResponse<AlbumInvitationResponse> getInvitationInfo( + @PathVariable @NotBlank @Pattern(regexp = "^[A-Za-z0-9-_]+$") String code + ) { return CommonResponse.success(ALBUM_INVITATION_FETCH_SUCCESS, albumService.getInvitationInfo(code)); }src/main/java/com/cheeeese/album/domain/Album.java (3)
69-96: 빌더에 불변성 검증을 추가하는 것을 고려하세요.빌더 생성자에서 도메인 규칙을 검증하지 않아, 잘못된 상태의 엔티티가 생성될 수 있습니다.
다음과 같은 검증을 추가하는 것을 고려하세요:
@Builder private Album( Long hostId, String title, String code, String themeImageUrl, int participant, int currentParticipant, LocalDate eventDate, int maxPhotoCount, int currentPhotoCount, boolean isInfoAvailable, LocalDateTime expiredAt, AlbumStatus status ) { + if (participant <= 0) { + throw new IllegalArgumentException("참여 인원은 0보다 커야 합니다"); + } + if (currentParticipant < 0 || currentParticipant > participant) { + throw new IllegalArgumentException("현재 참여 인원이 유효하지 않습니다"); + } + if (maxPhotoCount <= 0) { + throw new IllegalArgumentException("최대 사진 개수는 0보다 커야 합니다"); + } + if (currentPhotoCount < 0 || currentPhotoCount > maxPhotoCount) { + throw new IllegalArgumentException("현재 사진 개수가 유효하지 않습니다"); + } this.hostId = hostId; this.title = title; // ... rest of assignments }
17-67: 앨범 정원 체크를 위한 헬퍼 메서드 추가를 고려하세요.입장 검증 로직에서 사용할 수 있도록
isFull()메서드를 추가하면 비즈니스 로직이 더 명확해집니다.다음과 같은 메서드를 추가할 수 있습니다:
public boolean isFull() { return this.currentParticipant >= this.participant; }
36-49: 필드명 수정 및 사진 개수 처리 문서화
participant는 최대 수용 인원을 나타내므로maxParticipant(또는capacity)로 리네이밍 검토currentPhotoCount는PhotoRepository.countByAlbumIdAndIsDeletedFalse로 소프트삭제된 사진을 제외해 계산되므로, 이 방침을 주석 또는 개발 문서에 명시- (권장) 저장된
currentPhotoCount대신 조회 로직을 일원화하는 방안 검토
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
src/main/java/com/cheeeese/album/domain/Album.java(1 hunks)src/main/java/com/cheeeese/album/domain/UserAlbum.java(1 hunks)src/main/java/com/cheeeese/album/presentation/AlbumController.java(1 hunks)src/main/java/com/cheeeese/photo/domain/Photo.java(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- src/main/java/com/cheeeese/photo/domain/Photo.java
🧰 Additional context used
🧬 Code graph analysis (2)
src/main/java/com/cheeeese/album/domain/Album.java (2)
src/main/java/com/cheeeese/album/domain/UserAlbum.java (1)
Entity(10-39)src/main/java/com/cheeeese/photo/domain/Photo.java (1)
Entity(12-57)
src/main/java/com/cheeeese/album/domain/UserAlbum.java (2)
src/main/java/com/cheeeese/album/domain/Album.java (1)
Entity(13-97)src/main/java/com/cheeeese/photo/domain/Photo.java (1)
Entity(12-57)
🔇 Additional comments (6)
src/main/java/com/cheeeese/album/domain/UserAlbum.java (3)
10-14: 이전 리뷰 지적사항이 반영되었습니다!
(user_id, album_id)조합에 대한 유니크 제약조건이 올바르게 추가되어 동일한 사용자가 같은 앨범에 중복 참여하는 것을 방지합니다.
16-19: 기본키 정의가 올바릅니다.IDENTITY 전략을 사용한 표준적인 JPA 기본키 설정입니다.
29-31: Enum 타입 설정이 적절합니다.
EnumType.STRING을 사용하여 enum의 순서 변경에도 안전하게 대응할 수 있습니다.src/main/java/com/cheeeese/album/presentation/AlbumController.java (2)
32-38: LGTM! 앨범 입장 엔드포인트가 적절하게 구현되었습니다.POST 메서드를 사용하여 상태 변경을 처리하고,
@CurrentUser로 인증된 사용자를 받아 입장 로직을 수행하는 구조가 올바릅니다.
24-28: 확인:/v1/album/*/invitation엔드포인트가 SecurityConfig의 permitAll에 포함되어 있습니다.src/main/java/com/cheeeese/album/domain/Album.java (1)
65-67: 만료 체크 로직이 잘 구현되었습니다.
expiredAt시간과status상태를 모두 확인하는 방어적 프로그래밍이 적절합니다.
🔗 연관된 이슈
🚀 변경 유형
📝 작업 내용
📸 스크린샷
💬 리뷰 요구사항
Summary by CodeRabbit
신규 기능
버그/검증
문서화