feat: 사진 생성 제한 및 최대 사진 수 수정 #92
Conversation
WalkthroughAlbumMapper의 toEntity 메서드에서 앨범의 기본 최대 사진 개수(maxPhotoCount)를 100에서 300으로 변경했습니다. 메서드 서명이나 제어 흐름에는 변화가 없습니다. Changes
Estimated code review effort🎯 1 (Trivial) | ⏱️ ~2 minutes Possibly related PRs
Suggested labels
Suggested reviewers
Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
📜 Recent review detailsConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
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}
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (3)
src/main/java/com/cheeeese/album/infrastructure/mapper/AlbumMapper.java (1)
37-37: 기본maxPhotoCount2000 상향은 적절해 보입니다만, 매직 넘버 상수화(P4) 제안현재 변경 자체(기본 최대 사진 수를 100 → 2000으로 확대)는 요구사항에 잘 맞는 것 같습니다. 다만
2000이 도메인 상 의미 있는 값이라면, 추후 다른 곳에서도 재사용될 수 있으니Album쪽 도메인 상수나 설정 값(예:Album.DEFAULT_MAX_PHOTO_COUNT)으로 빼 두면 유지보수와 정책 변경에 더 유리할 것 같습니다.src/main/java/com/cheeeese/album/application/AlbumService.java (1)
71-74: 주간 앨범 생성 횟수 계산 로직의 기간/타임존 정의를 조금 더 명시적으로 정리하면 좋겠습니다 (P3)
- Service에서
createdThisWeek를 계산해AlbumValidator.validateAlbumCreation(createdThisWeek, request)로 넘기는 구조는 책임 분리가 잘 되어 있어 보입니다.- 다만
countUserAlbumsCreatedThisWeek구현은 의도가 코드에 바로 드러나지 않아, 이후 유지보수 시 혼동 여지가 있습니다.현재 구현:
private long countUserAlbumsCreatedThisWeek(User user) { return albumRepository.countByUserAndCreatedAtBetween( user.getId(), LocalDate.now().with(DayOfWeek.MONDAY).atTime(LocalTime.MIN), LocalDate.now().with(DayOfWeek.MONDAY).plusWeeks(1).atTime(LocalTime.now()) ); }
LocalDate.now()를 두 번 호출하고 있어, 이론상 자정 경계 등 극단적 상황에서 서로 다른 날짜를 참조할 가능성이 있습니다(실제 문제로 이어질 확률은 낮지만 가독성/명확성 측면에서 좋지 않습니다).- 상한값이
이번 주 월요일 + 1주일 at LocalTime.now()로 설정되어 있어, 코드를 처음 보는 사람이 “이번 주 월요일 00시부터 다음 주 월요일 같은 시각까지”로 해석하기 쉽습니다. 실제로는 쿼리 시점의 DB 스냅샷만 보게 되므로 기능상 큰 문제는 없겠지만, “이번 주 월요일 00시부터 지금까지” 또는 “이번 주(월~일) 전체” 중 어떤 의도인지 명시해 두는 편이 좋겠습니다.AlbumValidator에서는LocalDate.now(KST_ZONE)를 사용해 날짜를 판단하고 있는데, 여기서는 시스템 기본 타임존의LocalDate.now()를 사용 중입니다. 서버가 KST가 아니라는 전제까지 고려한다면, “이번 주” 기준 타임존이 무엇인지 한 번 맞춰 두는 것이 안전합니다.예시 리팩터 (의도: “이번 주 월요일 0시(KST)부터 지금까지 생성한 앨범 수”):
- private long countUserAlbumsCreatedThisWeek(User user) { - return albumRepository.countByUserAndCreatedAtBetween( - user.getId(), - LocalDate.now().with(DayOfWeek.MONDAY).atTime(LocalTime.MIN), - LocalDate.now().with(DayOfWeek.MONDAY).plusWeeks(1).atTime(LocalTime.now()) - ); - } + private long countUserAlbumsCreatedThisWeek(User user) { + // TODO: 서비스 전역 타임존 정책에 맞게 ZoneId를 명시적으로 사용하는 것도 고려해 주세요. + LocalDate today = LocalDate.now(); + LocalDate weekStart = today.with(DayOfWeek.MONDAY); + + return albumRepository.countByUserAndCreatedAtBetween( + user.getId(), + weekStart.atStartOfDay(), + LocalDateTime.now() + ); + }비즈니스적으로 “주당 3개” 제한이 캘린더 주(월~일) 기준인지, 마지막 7일 롤링 윈도우 기준인지도 확실히 정해 두고, 그에 맞춰 기간을 계산하면 이후 정책 변경 때도 덜 헷갈릴 것 같습니다.
Also applies to: 254-259
src/main/java/com/cheeeese/album/application/validator/AlbumValidator.java (1)
29-52: 앨범 주간 생성 제한(3개) 로직은 잘 붙었고, 상수/네이밍 정리만 조금 더 하면 좋겠습니다 (P3)
validateAlbumCreation(long createdThisWeek, AlbumCreationRequest request)로 시그니처를 바꾸고, Service에서 계산된 값을 주입받는 구조는 의존성을 깔끔하게 정리한 것 같아서 좋습니다.createdThisWeek >= 3일 때ALBUM_CREATION_LIMIT_EXCEEDED를 던지는 로직도 요구사항(주간 생성 제한)과 자연스럽게 맞아 보입니다.다만 아래 두 가지 정도는 한 번 고려해 볼 만합니다.
정책 값 상수화
3이라는 숫자는 도메인 정책(“사용자는 주당 N개의 앨범만 생성 가능”)을 나타내는 값이므로, Validator 클래스 상단이나 별도 도메인 상수로 추출해 두면 의미가 더 분명해집니다.private static final int WEEKLY_ALBUM_CREATION_LIMIT = 3; ... if (createdThisWeek >= WEEKLY_ALBUM_CREATION_LIMIT) { throw new AlbumException(AlbumErrorCode.ALBUM_CREATION_LIMIT_EXCEEDED); }파라미터 네이밍/주석으로 의미 명시
createdThisWeek가 “이번 주(월~일) 동안 이미 생성된 앨범 수”인지, “현재 요청까지 포함한 시도 횟수”인지 등은 Service 구현을 보면 유추 가능하지만, Validator 메서드 시그니처만 봐서는 바로 드러나지 않습니다. 짧은 주석으로 의미를 적어 두면 이후 사용하는 쪽에서 실수할 가능성이 줄어듭니다.현재 구현 자체는 문제 없어 보이고, 위 내용은 도메인 정책 가독성을 조금 더 높이기 위한 제안 정도입니다.
📜 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/application/validator/AlbumValidator.java(2 hunks)src/main/java/com/cheeeese/album/infrastructure/mapper/AlbumMapper.java(1 hunks)
🔗 연관된 이슈
🚀 변경 유형
📝 작업 내용
💬 리뷰 요구사항
📜 리뷰 규칙
Reviewer는 아래 P5 Rule을 참고하여 리뷰를 진행합니다.
P5 Rule을 통해 Reviewer는 Reviewee에게 리뷰의 의도를 보다 정확히 전달할 수 있습니다.
Summary by CodeRabbit
변경 사항
✏️ Tip: You can customize this high-level summary in your review settings.