feat: 앨범 생성 및 권한 관리 구현 #17
Conversation
Walkthrough앨범 생성 API와 DTO/응답을 추가하고 도메인(ThemeEmoji, isTermsAgreement)과 참여자 모델을 UserAlbum→AlbumParticipant로 리팩터링했으며 검증·에러 코드·레포지토리·매퍼·컨트롤러·벤치마크·픽스처·빌드 설정을 함께 변경했습니다. Changes
Sequence Diagram(s)sequenceDiagram
participant User as 사용자
participant Controller as AlbumController
participant Service as AlbumService
participant Validator as AlbumValidator
participant AlbumRepo as AlbumRepository
participant Mapper as AlbumMapper
participant ParticipantRepo as AlbumParticipantRepository
User->>Controller: POST /albums (AlbumCreationRequest)
Controller->>Service: createAlbum(user, request)
rect rgb(235,245,255)
Service->>AlbumRepo: countByUserAndCreatedAtBetween(userId, weekStart, weekEnd)
AlbumRepo-->>Service: createdThisWeek
Service->>Validator: validateAlbumCreation(createdThisWeek, request)
Validator-->>Service: validation 성공/예외
end
rect rgb(235,255,235)
Service->>Service: generate time-ordered UUID
Service->>Mapper: toEntity(...)
Mapper-->>Service: Album entity
Service->>AlbumRepo: save(album)
AlbumRepo-->>Service: saved Album
Service->>ParticipantRepo: save(AlbumParticipant(hostId, albumId, isBlacklisted=false))
ParticipantRepo-->>Service: saved Participant
end
rect rgb(255,250,230)
Service->>Mapper: toCreationResponse(album)
Mapper-->>Service: AlbumCreationResponse
Service-->>Controller: AlbumCreationResponse
Controller-->>User: 200 OK (CommonResponse)
end
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related issues
Possibly related PRs
Suggested reviewers
Poem
Pre-merge checks and finishing touches✅ 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[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 7
🧹 Nitpick comments (12)
src/main/java/com/cheeeese/album/infrastructure/persistence/AlbumRepository.java (1)
19-24: 파라미터명과 쿼리 필드 간 명명 불일치 개선 필요메서드 파라미터는
userId로 명명되어 있지만, 실제 JPQL 쿼리에서는a.hostId를 참조하고 있습니다. 이는 코드 가독성을 저해하고 혼란을 야기할 수 있습니다.다음 중 하나를 선택하여 일관성을 개선하세요:
방안 1 (권장): 파라미터명을 hostId로 변경
- @Query("SELECT COUNT(a) FROM Album a WHERE a.hostId = :userId AND a.createdAt BETWEEN :start AND :end") + @Query("SELECT COUNT(a) FROM Album a WHERE a.hostId = :hostId AND a.createdAt BETWEEN :start AND :end") long countByUserAndCreatedAtBetween( - @Param("userId") Long userId, + @Param("hostId") Long hostId, @Param("start") LocalDateTime start, @Param("end") LocalDateTime end );방안 2: 메서드명을 hostId 기반으로 변경
- @Query("SELECT COUNT(a) FROM Album a WHERE a.hostId = :userId AND a.createdAt BETWEEN :start AND :end") - long countByUserAndCreatedAtBetween( - @Param("userId") Long userId, + @Query("SELECT COUNT(a) FROM Album a WHERE a.hostId = :hostId AND a.createdAt BETWEEN :start AND :end") + long countByHostAndCreatedAtBetween( + @Param("hostId") Long hostId, @Param("start") LocalDateTime start, @Param("end") LocalDateTime end );src/main/java/com/cheeeese/global/common/code/SuccessCode.java (1)
26-26: 성공 메시지 언어 일관성 개선 권장기존 앨범 관련 성공 코드들(Line 24-25)은 한글 메시지를 사용하는 반면, 새로 추가된
ALBUM_CREATE_SUCCESS는 영문 메시지를 사용하고 있습니다. 일관성을 위해 한글 메시지 사용을 권장합니다.- ALBUM_CREATE_SUCCESS(HttpStatus.OK, "Album Create Success"), + ALBUM_CREATE_SUCCESS(HttpStatus.OK, "앨범 생성이 성공적으로 완료되었습니다."),src/main/java/com/cheeeese/album/infrastructure/mapper/AlbumMapper.java (1)
21-47: toEntity 메서드 파라미터 개수 고려현재
toEntity메서드가 10개의 파라미터를 받고 있습니다. 파라미터가 많아 메서드 호출 시 실수 가능성이 있습니다.더 나은 가독성을 위해 파라미터를 그룹화한 DTO나 빌더 패턴 사용을 고려하세요:
// 옵션 1: 생성 파라미터를 위한 DTO public static Album toEntity(AlbumCreationParams params) { return Album.builder() .hostId(params.hostId()) .title(params.title()) // ... .build(); } // 옵션 2: 현재 구조 유지 (현재도 충분히 명확함)다만 현재 구조도 충분히 명확하므로 선택적으로 적용하세요.
src/main/java/com/cheeeese/album/presentation/swagger/AlbumSwagger.java (2)
34-43: 생성 성공 시 201 Created + Location 헤더로 표준화 권장리소스 생성은 200보다 201이 적합합니다. Location 헤더(
/albums/{code}등)도 함께 문서화해주세요. 컨트롤러에는@ResponseStatus(HttpStatus.CREATED)를, 스웨거에는 201 응답을 반영하면 좋습니다.적용 예(스웨거 부분만 발췌):
- @ApiResponse( - responseCode = "200", - description = "앨범 생성이 성공적으로 실행되었습니다." - ) + @ApiResponse( + responseCode = "201", + description = "앨범 생성이 성공적으로 실행되었습니다. Location 헤더에 새 앨범 URL 포함" + )
22-33: RequestBody 상세는 DTO @Schema에 위임하여 중복 제거
AlbumCreationRequest에 필드별 @Schema가 이미 있으므로 여기 설명은 요약 위주로 두고 상세는 DTO로 위임하면 문서 드리프트를 줄일 수 있습니다. 또한@Valid가 효과를 내도록 DTO에 Bean Validation을 추가(아래 DTO 코멘트 참고)하세요.src/main/java/com/cheeeese/album/application/validator/AlbumValidator.java (1)
13-20: 테스트 가능성과 타임존 견고성: Clock 주입 권장
LocalDate.now()대신Clock을 주입해 사용하면(예:LocalDate.now(clock)) 테스트가 안정되고 서버 타임존/서머타임 이슈를 줄일 수 있습니다.적용 예:
-import java.time.LocalDate; +import java.time.Clock; +import java.time.LocalDate; @RequiredArgsConstructor public class AlbumValidator { private final AlbumRepository albumRepository; private final AlbumParticipantRepository albumParticipantRepository; + private final Clock clock; - if (request.eventDate().isAfter(LocalDate.now())) { + if (request.eventDate().isAfter(LocalDate.now(clock))) { throw new AlbumException(AlbumErrorCode.ALBUM_EVENT_DATE_INVALID); }Also applies to: 35-37
src/main/java/com/cheeeese/album/infrastructure/persistence/AlbumParticipantRepository.java (2)
9-15: 조회 패턴에 맞춘 인덱스/유니크 제약 추가 권장다음 쿼리를 자주 쓰므로 인덱스와 유니크 제약을 권장합니다. 엔티티(@table(indexes=…)/@UniqueConstraint) 또는 마이그레이션으로 추가해주세요.
- (album_id, user_id) UNIQUE — 중복 참가 방지
- (album_id, user_id, is_blacklisted) INDEX — 블랙리스트 조회 가속
- (album_id) INDEX — 앨범별 전체 참가자 조회 가속
예시(DDL):
Postgres
CREATE UNIQUE INDEX ux_album_participant ON album_participant(album_id, user_id);CREATE INDEX ix_album_participant_blacklist ON album_participant(album_id, user_id, is_blacklisted);MySQL
ALTER TABLE album_participant ADD UNIQUE KEY ux_album_participant (album_id, user_id);ALTER TABLE album_participant ADD KEY ix_album_participant_blacklist (album_id, user_id, is_blacklisted);
10-14: 메서드 시그니처의 파라미터 순서 통일(가독성)
findByUserIdAndAlbumId(...)와findByAlbumIdAndUserIdAndIsBlacklistedTrue(...)의 파라미터 순서가 다릅니다. 혼동 방지를 위해 통일을 권장합니다(예: 모두 albumId, userId 순서). 호환성 유지를 위해 신규 메서드 추가 후 점진 교체를 추천합니다.src/test/java/com/cheeeese/album/benchmark/AlbumServiceBenchmarkTest.java (1)
30-53: 벤치마크는 CI에서 기본 제외되도록 보장 필요데이터 4만 건 삽입은 CI를 지연시킬 수 있습니다.
@Tag("benchmark")는 좋지만, 빌드 설정에서 태그 제외를 명시하세요.Gradle 예:
test { useJUnitPlatform { excludeTags 'benchmark' } }Maven(Surefire) 예:
<configuration> <properties> <configurationParameters>junit.jupiter.tags=!benchmark</configurationParameters> </properties> </configuration>src/test/java/com/cheeeese/fixture/FixtureFactory.java (1)
14-56: 픽스처 구성 명확 — 벤치마크/단위테스트에 유용합니다v4/v7 구분과 코드 생성이 명확합니다. 유지보수를 위해 공통 상수(이벤트일, 만료일 offset 등)를 상단에 모아두면 중복을 줄일 수 있습니다.
src/main/java/com/cheeeese/album/dto/request/AlbumCreationRequest.java (1)
8-25: Bean Validation 추가로 컨트롤러 단에서 즉시 400 응답 처리현재 DTO에 제약이 없어
@Valid가 효과가 없습니다. 필수/범위를 DTO에 선언해 조기 검증하세요. 서비스 Validator는 비즈니스 규칙만 담당하도록 분리하면 깔끔합니다.적용 예:
package com.cheeeese.album.dto.request; import io.swagger.v3.oas.annotations.media.Schema; import lombok.Builder; import java.time.LocalDate; +import jakarta.validation.constraints.*; @Builder @Schema(description = "앨범 생성 API") public record AlbumCreationRequest( @Schema(description = "썸네일 이미지", example = "https://theme.jpg") - String themeImageUrl, + @NotBlank(message = "썸네일 이미지는 필수입니다.") + String themeImageUrl, @Schema(description = "앨범 이름", example = "졸업식") - String title, + @NotBlank(message = "앨범 이름은 필수입니다.") + @Size(max = 100, message = "앨범 이름은 100자를 넘을 수 없습니다.") + String title, @Schema(description = "참여자 수", example = "64") - int participant, + @Min(value = 1, message = "참여자 수는 1명 이상이어야 합니다.") + @Max(value = 64, message = "참여자 수는 64명을 초과할 수 없습니다.") + int participant, @Schema(description = "행사 날짜", example = "2025-02-01") - LocalDate eventDate, + @NotNull(message = "행사 날짜는 필수입니다.") + @PastOrPresent(message = "행사 날짜는 오늘 또는 과거여야 합니다.") + LocalDate eventDate, @Schema(description = "앨범 생성 필수 약관 동의", example = "true") - boolean isTermsAgreement + @AssertTrue(message = "필수 약관에 동의해야 합니다.") + boolean isTermsAgreement ) { }Based on learnings
src/main/java/com/cheeeese/album/application/AlbumService.java (1)
166-176: 메서드 레퍼런스 사용 고려현재 구현은 올바르게 작동하지만, 람다 표현식을 메서드 레퍼런스로 간소화할 수 있습니다.
private List<AlbumParticipantResponse> getParticipantResponses(Long albumId) { List<Long> participantUserIds = albumParticipantRepository.findAllByAlbumId(albumId).stream() - .map(AlbumParticipant::getUserId) + .map(AlbumParticipant::getUserId) .collect(Collectors.toList()); List<User> participants = userRepository.findAllById(participantUserIds); return participants.stream() .map(AlbumMapper::toParticipantResponse) .collect(Collectors.toList()); }참고: Line 168은 이미 메서드 레퍼런스를 사용하고 있으므로 추가 변경은 불필요합니다. Line 174도 마찬가지로 이미 최적화되어 있습니다.
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (20)
.gitignore(1 hunks)build.gradle(1 hunks)src/main/java/com/cheeeese/album/application/AlbumService.java(4 hunks)src/main/java/com/cheeeese/album/application/validator/AlbumValidator.java(2 hunks)src/main/java/com/cheeeese/album/domain/Album.java(3 hunks)src/main/java/com/cheeeese/album/domain/AlbumParticipant.java(2 hunks)src/main/java/com/cheeeese/album/domain/UserAlbumRole.java(0 hunks)src/main/java/com/cheeeese/album/dto/request/AlbumCreationRequest.java(1 hunks)src/main/java/com/cheeeese/album/dto/response/AlbumCreationResponse.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(2 hunks)src/main/java/com/cheeeese/album/infrastructure/persistence/AlbumParticipantRepository.java(1 hunks)src/main/java/com/cheeeese/album/infrastructure/persistence/AlbumRepository.java(2 hunks)src/main/java/com/cheeeese/album/infrastructure/persistence/UserAlbumRepository.java(0 hunks)src/main/java/com/cheeeese/album/presentation/AlbumController.java(2 hunks)src/main/java/com/cheeeese/album/presentation/swagger/AlbumSwagger.java(2 hunks)src/main/java/com/cheeeese/global/common/code/SuccessCode.java(1 hunks)src/main/java/com/cheeeese/global/config/SwaggerConfig.java(1 hunks)src/test/java/com/cheeeese/album/benchmark/AlbumServiceBenchmarkTest.java(1 hunks)src/test/java/com/cheeeese/fixture/FixtureFactory.java(1 hunks)
💤 Files with no reviewable changes (2)
- src/main/java/com/cheeeese/album/infrastructure/persistence/UserAlbumRepository.java
- src/main/java/com/cheeeese/album/domain/UserAlbumRole.java
🧰 Additional context used
🧬 Code graph analysis (3)
src/test/java/com/cheeeese/album/benchmark/AlbumServiceBenchmarkTest.java (1)
src/test/java/com/cheeeese/fixture/FixtureFactory.java (1)
FixtureFactory(12-57)
src/test/java/com/cheeeese/fixture/FixtureFactory.java (1)
src/main/java/com/cheeeese/album/infrastructure/mapper/AlbumMapper.java (1)
AlbumMapper(16-131)
src/main/java/com/cheeeese/album/application/AlbumService.java (2)
src/main/java/com/cheeeese/album/infrastructure/mapper/AlbumMapper.java (1)
AlbumMapper(16-131)src/main/java/com/cheeeese/photo/application/PhotoService.java (1)
Service(12-29)
🔇 Additional comments (16)
.gitignore (1)
12-12: 테스트 설정 파일 무시 규칙 추가 적절함.
application-test.yml을.gitignore에 추가한 것은 환경별 구성 파일을 버전 관리에서 제외하는 표준 관행입니다. 기존 환경 설정 파일들과 일관성 있게 배치되어 있습니다.build.gradle (2)
54-55: UUID v7 지원 확인됨, 버전 업그레이드 고려 권장.uuid-creator 5.2.0은 UUID v7 구현을 포함하고 있으며, 알범 코드 생성에 적합합니다. 다만 최신 버전은 6.1.1이며, 현재 버전은 2022년 10월 출시본으로 약 3년이 경과했습니다. 최신 버전으로의 업그레이드 가능 여부와 6.x 대의 주요 변경사항을 검토할 것을 권장합니다.
버전 5.2.0 대신 최신 버전(6.1.1)으로 업그레이드할 수 있는지, 그리고 마이그레이션 시 주의사항이 있는지 확인해주세요.
59-61: 벤치마크 테스트 필터링 적절히 설정됨.JUnit Platform의
excludeTags설정으로 벤치마크 테스트를 정상 테스트 실행 시 제외하는 것은 좋은 관행입니다. 다만 벤치마크 테스트가@Tag("benchmark")애너테이션으로 적절히 표시되어 있는지 확인해주세요.src/main/java/com/cheeeese/global/config/SwaggerConfig.java (1)
20-20: 시맨틱 버저닝 적용이 좋습니다.API 버전을 "v1"에서 "v1.0.0"으로 변경하여 시맨틱 버저닝을 따르는 것은 좋은 개선입니다.
src/main/java/com/cheeeese/album/exception/code/AlbumErrorCode.java (1)
17-22: LGTM!앨범 생성 흐름에 필요한 검증 에러 코드들이 명확하고 적절하게 정의되었습니다. HttpStatus 매핑과 한글 에러 메시지 모두 적절합니다.
src/main/java/com/cheeeese/album/presentation/AlbumController.java (1)
26-33: LGTM!앨범 생성 엔드포인트가 적절하게 구현되었습니다:
@Valid를 통한 요청 검증@CurrentUser를 통한 사용자 인증- 서비스 레이어로의 적절한 위임
src/main/java/com/cheeeese/album/domain/Album.java (2)
61-62: LGTM!필수 약관 동의 상태를 추적하기 위한
isTermsAgreement필드가 적절하게 추가되었습니다. 컬럼 매핑과 nullable 제약조건이 올바르게 설정되어 있습니다.
85-86: LGTM!빌더 패턴에
isTermsAgreement파라미터가 올바르게 추가되고 할당되었습니다.Also applies to: 100-100
src/main/java/com/cheeeese/album/domain/AlbumParticipant.java (2)
14-14: 클래스명과 테이블명 불일치 확인 필요클래스명이
AlbumParticipant로 변경되었지만 테이블명은 여전히user_album으로 유지되고 있습니다(Line 12). 이는 다음을 확인해야 합니다:
- 기존 데이터베이스 스키마와의 호환성 유지를 위한 의도적 선택인지
- 향후 테이블명도 변경할 계획인지
의도된 설계인지 확인 부탁드립니다. 만약 데이터베이스 마이그레이션이 계획되어 있다면, 이를 추적할 수 있도록 TODO 주석을 추가하거나 별도 이슈를 생성하는 것을 권장합니다.
29-30: LGTM!
roleenum을isBlacklistedboolean으로 변경한 것은 적절한 개선입니다:
- 블랙리스트 여부를 명확하게 표현
- 단순한 boolean 타입으로 복잡도 감소
- 현재 요구사항에 더 적합한 모델링
Also applies to: 33-33, 36-36
src/main/java/com/cheeeese/album/infrastructure/mapper/AlbumMapper.java (3)
40-40: maxPhotoCount 하드코딩 정책 확인 필요
maxPhotoCount가 2000으로 하드코딩되어 있습니다. PR 설명에서 작성자가 "전사적으로 업로드 가능 사진 수가 2000장으로 고정된 점을 고려할 때 maxPhotoCount 컬럼이 필요한지" 질문하셨습니다.다음 중 하나를 선택하는 것이 좋습니다:
값이 정말 불변이라면: DB 컬럼을 제거하고 상수로 관리
public static final int DEFAULT_MAX_PHOTO_COUNT = 2000;향후 변경 가능성이 있다면: 현재 구조 유지하되 설정으로 외부화
// application.yml album: max-photo-count: 2000앨범별로 다를 수 있다면: 현재처럼 DB 컬럼 유지
비즈니스 요구사항 확인이 필요합니다. 앨범별로 사진 수 제한이 다를 가능성이 있는지, 아니면 정말 시스템 전체에서 고정 값인지 확인 부탁드립니다.
52-60: LGTM!Album 엔티티를 AlbumCreationResponse로 변환하는 로직이 명확하고 적절합니다.
114-120: LGTM!
AlbumParticipant로의 전환이 올바르게 적용되었으며,isBlacklisted를 false로 기본 설정하는 것이 적절합니다.src/main/java/com/cheeeese/album/application/validator/AlbumValidator.java (1)
35-37: 미래 날짜 금지 규칙 확인 필요현재
eventDate.isAfter(LocalDate.now())면 오류를 던집니다. 행사 전 미리 앨범 생성이 불가한 비즈니스가 맞는지 확인 부탁드립니다(요구사항에 따라@PastOrPresent가 아니라@FutureOrPresent일 수도 있습니다).src/main/java/com/cheeeese/album/application/AlbumService.java (2)
3-33: LGTM!필요한 의존성 임포트가 적절하게 추가되었습니다. AlbumParticipant 도메인 모델로의 리팩터링과 앨범 생성 기능을 위한 임포트가 잘 구성되어 있습니다.
104-132: LGTM! 동시성 처리가 잘 구현되었습니다.앨범 참가 로직의 동시성 제어가 매우 잘 구현되어 있습니다:
@Transactional컨텍스트 내에서 실행되어 예외 발생 시 롤백이 보장됩니다DataIntegrityViolationException처리로 동시 참가 시도를 감지합니다- 원자적 증가 로직(
incrementParticipantCountAtomically)으로 정원 초과를 방지합니다- 업데이트 후 앨범을 다시 조회하여 최신 상태를 반환합니다
src/main/java/com/cheeeese/album/application/validator/AlbumValidator.java
Show resolved
Hide resolved
src/main/java/com/cheeeese/album/dto/response/AlbumCreationResponse.java
Show resolved
Hide resolved
src/test/java/com/cheeeese/album/benchmark/AlbumServiceBenchmarkTest.java
Show resolved
Hide resolved
src/test/java/com/cheeeese/album/benchmark/AlbumServiceBenchmarkTest.java
Show resolved
Hide resolved
dahyun24
left a comment
dahyun24
left a comment
There was a problem hiding this comment.
maxPhotoCount 필드를 만든 것은 기획쪽에서 비즈니스 모델로 최대 제한 사진 개수를 조정할 수 있다고 들어서 만들었습니다~
uuid 버전에 따른 테스트 결과 차이가 엄청나네요!
수고하셨습니다~
src/test/java/com/cheeeese/album/benchmark/AlbumServiceBenchmarkTest.java
Show resolved
Hide resolved
src/main/java/com/cheeeese/album/dto/response/AlbumCreationResponse.java
Outdated
Show resolved
Hide resolved
아하 감사합니다!!!🧌 |
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (4)
src/main/java/com/cheeeese/album/application/AlbumService.java (3)
64-64: 만료 기간 상수화 권장이전 리뷰에서 제안된 만료 기간 상수화가 아직 적용되지 않았습니다. 매직 넘버
7을 상수로 추출하면 유지보수성이 향상됩니다.다음과 같이 상수를 정의하세요:
+ private static final int ALBUM_EXPIRATION_DAYS = 7; + @Transactional public AlbumCreationResponse createAlbum(User user, AlbumCreationRequest request) { // ... - LocalDateTime.now().plusDays(7), + LocalDateTime.now().plusDays(ALBUM_EXPIRATION_DAYS),
50-50: UUID 버전 불일치 여전히 미수정 (v6 → v7)이전 리뷰에서 지적된 UUID 버전 이슈가 아직 수정되지 않았습니다. 현재 코드는
getTimeOrdered()를 사용하여 UUID v6을 생성하지만, PR 설명과 벤치마크 테스트 결과에 따르면 UUID v7을 사용하기로 결정했습니다.다음과 같이 수정하세요:
- String code = UuidCreator.getTimeOrdered().toString(); + String code = UuidCreator.getTimeOrderedEpoch().toString();
97-103: 주간 앨범 생성 횟수 계산 로직 오류 (Critical)이 메서드에는 두 가지 심각한 문제가 있습니다:
101번 라인 -
LocalTime.now()사용 오류:
- 종료 시점을
LocalTime.now()로 계산하면 메서드 호출 시각에 따라 주간 범위가 달라집니다- 예: 오전 9시에 호출하면 다음 주 월요일 09:00까지, 오후 3시에 호출하면 15:00까지
- 의도한 범위는 "이번 주 월요일 00:00:00 ~ 다음 주 월요일 00:00:00 (미포함)"이므로
LocalTime.MIN을 사용해야 합니다BETWEEN 연산자 경계 문제 (이전 리뷰에서 미수정):
- SQL
BETWEEN은 양쪽 경계를 모두 포함합니다- 다음 주 월요일 정확히 00:00:00에 생성된 앨범이 현재 주에 포함될 수 있습니다
- Repository 메서드를
>= start AND < end조건으로 변경해야 합니다다음과 같이 수정하세요:
1단계: 101번 라인 수정
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().with(DayOfWeek.MONDAY).plusWeeks(1).atTime(LocalTime.MIN) );2단계: AlbumRepository에 exclusive upper bound 메서드 추가
AlbumRepository에 다음 메서드를 추가하세요:@Query("SELECT COUNT(a) FROM Album a WHERE a.hostId = :hostId AND a.createdAt >= :start AND a.createdAt < :end") long countByHostIdAndCreatedAtBetweenExclusive(@Param("hostId") Long hostId, @Param("start") LocalDateTime start, @Param("end") LocalDateTime end);그리고 이 메서드를 사용하도록 변경:
return albumRepository.countByHostIdAndCreatedAtBetweenExclusive( user.getId(), LocalDate.now().with(DayOfWeek.MONDAY).atStartOfDay(), LocalDate.now().with(DayOfWeek.MONDAY).plusWeeks(1).atStartOfDay() );src/test/java/com/cheeeese/album/benchmark/AlbumServiceBenchmarkTest.java (1)
83-92: 평균 계산이 누락되어 있습니다100회 반복 조회의 총 시간을 밀리초로 변환했지만, 100으로 나눠 평균을 계산하지 않았습니다. 레이블을 "전체 조회 시간"으로 변경하셨지만, 벤치마크에서는 일반적으로 평균 시간을 표시하는 것이 더 의미 있습니다.
수정 예시:
- System.out.printf("[%s] 전체 조회 시간: %.2f ms%n", label, total / 1000000.0); + double avgMs = total / 100.0 / 1_000_000.0; // 100회 평균을 ms로 변환 + System.out.printf("[%s] 평균 조회 시간: %.2f ms%n", label, avgMs);
🧹 Nitpick comments (2)
src/test/java/com/cheeeese/fixture/FixtureFactory.java (1)
30-56: 선택 사항: v4/v7 메서드 간 코드 중복 제거 고려
createAlbumV4와createAlbumV7메서드가 UUID 생성 로직을 제외하고 동일합니다.리팩터링 예시:
+ private static Album createAlbumWithCode(int i, String prefix, String code) { + return AlbumMapper.toEntity( + 1L, + prefix + "-" + i, + code, + "테스트 이미지", + 4, + LocalDate.of(2025, 1, 1), + true, + LocalDateTime.now().plusDays(7), + true + ); + } + public static Album createAlbumV4(int i) { - return AlbumMapper.toEntity( - 1L, - "v4-" + i, - UUID.randomUUID().toString(), - "테스트 이미지", - 4, - LocalDate.of(2025, 1, 1), - true, - LocalDateTime.now().plusDays(7), - true - ); + return createAlbumWithCode(i, "v4", UUID.randomUUID().toString()); } public static Album createAlbumV7(int i) { - return AlbumMapper.toEntity( - 1L, - "v7-" + i, - UuidCreator.getTimeOrderedEpoch().toString(), - "테스트 이미지", - 4, - LocalDate.of(2025, 1, 1), - true, - LocalDateTime.now().plusDays(7), - true - ); + return createAlbumWithCode(i, "v7", UuidCreator.getTimeOrderedEpoch().toString()); }src/test/java/com/cheeeese/album/benchmark/AlbumServiceBenchmarkTest.java (1)
62-72: 선택 사항: 메모리 효율성 개선전체 앨범(40,000개)을 메모리에 로드한 후 Java 스트림으로 필터링하는 것은 비효율적입니다.
리포지토리 메서드 추가를 고려하세요:
List<Album> findAllByTitleStartingWith(String prefix);사용 예시:
- List<Album> allAlbums = albumRepository.findAll(); - - List<String> v4Codes = allAlbums.stream() - .filter(a -> a.getTitle().startsWith("v4")) - .map(Album::getCode) - .toList(); - - List<String> v7Codes = allAlbums.stream() - .filter(a -> a.getTitle().startsWith("v7")) - .map(Album::getCode) - .toList(); + List<String> v4Codes = albumRepository.findAllByTitleStartingWith("v4") + .stream() + .map(Album::getCode) + .toList(); + + List<String> v7Codes = albumRepository.findAllByTitleStartingWith("v7") + .stream() + .map(Album::getCode) + .toList();
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (6)
src/main/java/com/cheeeese/album/application/AlbumService.java(4 hunks)src/main/java/com/cheeeese/album/application/validator/AlbumValidator.java(2 hunks)src/main/java/com/cheeeese/album/exception/code/AlbumErrorCode.java(1 hunks)src/main/java/com/cheeeese/album/infrastructure/persistence/AlbumRepository.java(2 hunks)src/test/java/com/cheeeese/album/benchmark/AlbumServiceBenchmarkTest.java(1 hunks)src/test/java/com/cheeeese/fixture/FixtureFactory.java(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (2)
- src/main/java/com/cheeeese/album/infrastructure/persistence/AlbumRepository.java
- src/main/java/com/cheeeese/album/exception/code/AlbumErrorCode.java
🧰 Additional context used
🧬 Code graph analysis (3)
src/main/java/com/cheeeese/album/application/AlbumService.java (2)
src/main/java/com/cheeeese/album/infrastructure/mapper/AlbumMapper.java (1)
AlbumMapper(16-131)src/main/java/com/cheeeese/photo/application/PhotoService.java (1)
Service(12-29)
src/test/java/com/cheeeese/album/benchmark/AlbumServiceBenchmarkTest.java (1)
src/test/java/com/cheeeese/fixture/FixtureFactory.java (1)
FixtureFactory(12-57)
src/test/java/com/cheeeese/fixture/FixtureFactory.java (1)
src/main/java/com/cheeeese/album/infrastructure/mapper/AlbumMapper.java (1)
AlbumMapper(16-131)
🔇 Additional comments (3)
src/main/java/com/cheeeese/album/application/validator/AlbumValidator.java (1)
22-50: 제목 검증 추가 확인 완료!이전 리뷰 코멘트에서 지적된 제목(title) 검증이 정상적으로 추가되었습니다 (31-33번 라인). 모든 검증 로직이 명확하고 올바르게 구현되어 있습니다.
src/test/java/com/cheeeese/album/benchmark/AlbumServiceBenchmarkTest.java (2)
94-106: 정렬 벤치마크 수정 확인 - 잘 해결되었습니다!이전 리뷰에서 지적된 정렬 벤치마크 문제가 잘 수정되었습니다. 이제
WHERE a.title LIKE :prefix조건으로 각 라벨별 데이터만 필터링하여 정렬하므로, v4와 v7의 정렬 성능을 정확하게 비교할 수 있습니다.
59-81: 참고: 검증 없는 관찰형 벤치마크테스트 메서드에 assertion이 없어 성능 저하 시 실패하지 않습니다. PR 설명에 따르면 이 벤치마크 결과가 UUID v7 채택 결정에 활용되었으므로 의도된 것으로 보입니다.
향후 성능 회귀를 자동으로 감지하려면 기준 임계값과 assertion 추가를 고려하세요.
src/test/java/com/cheeeese/album/benchmark/AlbumServiceBenchmarkTest.java
Show resolved
Hide resolved
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/main/java/com/cheeeese/album/application/AlbumService.java (1)
128-133: 원자적 업데이트 후 엔티티 재조회 시 1차 캐시 오염 가능성JPA 1차 캐시에 기존 Album가 남아있으면 findById가 갱신 전 스냅샷을 반환할 수 있습니다. 아래 중 하나로 일관성을 보장하세요.
옵션 A(권장): 업데이트 쿼리에 clearAutomatically=true 지정
@Modifying(clearAutomatically = true, flushAutomatically = true) @Query("update Album a set a.currentParticipant = a.currentParticipant + 1 " + "where a.id = :albumId and a.currentParticipant < a.participant") int incrementParticipantCountAtomically(@Param("albumId") Long albumId);옵션 B: 서비스에서 refresh 사용
- return albumRepository.findById(album.getId()) + entityManager.refresh(album); + return albumRepository.findById(album.getId()) .orElseThrow( ... );(옵션 B는 EntityManager 주입이 필요)
♻️ Duplicate comments (4)
src/main/java/com/cheeeese/album/application/validator/AlbumValidator.java (1)
31-33: 제목 검증 추가 확인 — 반영 감사합니다이전 피드백대로 title null/blank 검증이 추가되었습니다. 이 부분은 그대로 가면 됩니다.
src/main/java/com/cheeeese/album/application/AlbumService.java (3)
64-64: 만료일 7일 매직 넘버 상수화상수로 추출하면 유지보수성이 좋아집니다.
- LocalDateTime.now().plusDays(7), + LocalDateTime.now().plusDays(ALBUM_EXPIRATION_DAYS),클래스 상단에:
+ private static final int ALBUM_EXPIRATION_DAYS = 7;
50-50: UUID v7 의도 불일치 — v6 생성 메서드 사용 중UuidCreator.getTimeOrdered()는 v6입니다. v7을 쓰려면 getTimeOrderedEpoch()로 교체하세요.
- String code = UuidCreator.getTimeOrdered().toString(); + String code = UuidCreator.getTimeOrderedEpoch().toString();
97-103: 주간 카운트 경계 버그(치명적) — 상한 시각 오류검증 결과, 리뷰 댓글의 지적이 정확합니다.
AlbumRepository의 쿼리는 올바릅니다(
a.createdAt >= :start AND a.createdAt < :end), 하지만 AlbumService의countUserAlbumsCreatedThisWeek()메서드(97-101줄)에서 상한 시각을 잘못 전달하고 있습니다:
현재 코드:
LocalDate.now().with(DayOfWeek.MONDAY).plusWeeks(1).atTime(LocalTime.now())
- 다음 주 월요일의 현재 시각까지 포함
- 예: 수요일 14:30 호출 시 → 다음 주 월요일 14:30까지 카운트
정정 필요:
LocalDate.now().with(DayOfWeek.MONDAY).plusWeeks(1).atStartOfDay()
- 다음 주 월요일 00:00:00부터 제외
- 정확한 주간 경계: [월요일 00:00:00, 다음 주 월요일 00:00:00)
권장 수정:
private long countUserAlbumsCreatedThisWeek(User user) { LocalDate today = LocalDate.now(); LocalDateTime start = today.with(DayOfWeek.MONDAY).atStartOfDay(); - LocalDateTime endExclusive = today.with(DayOfWeek.MONDAY).plusWeeks(1).atTime(LocalTime.now()); + LocalDateTime endExclusive = today.with(DayOfWeek.MONDAY).plusWeeks(1).atStartOfDay(); return albumRepository.countByUserAndCreatedAtBetween( user.getId(), start, endExclusive); }
🧹 Nitpick comments (9)
src/main/java/com/cheeeese/album/application/validator/AlbumValidator.java (3)
39-41: 시간 의존성 주입으로 테스트 안정성 강화LocalDate.now() 직접 호출 대신 Clock 주입을 권장합니다. 타임존/자정 경계에서 플래키해질 수 있습니다.
@@ - if (request.eventDate().isAfter(LocalDate.now())) { + if (request.eventDate().isAfter(LocalDate.now(clock))) { throw new AlbumException(AlbumErrorCode.ALBUM_EVENT_DATE_INVALID); }그리고 필드 주입:
@RequiredArgsConstructor public class AlbumValidator { @@ - private final AlbumParticipantRepository albumParticipantRepository; + private final AlbumParticipantRepository albumParticipantRepository; + private final java.time.Clock clock;
43-49: 매직 넘버 상수화(정원 64, 주간 생성 3)가독성과 일관성을 위해 상수로 추출해 주세요.
@Component @RequiredArgsConstructor public class AlbumValidator { + private static final int MAX_PARTICIPANT = 64; + private static final int MAX_CREATE_PER_WEEK = 3; @@ - if (request.participant() < 1 || request.participant() > 64) { + if (request.participant() < 1 || request.participant() > MAX_PARTICIPANT) { throw new AlbumException(AlbumErrorCode.ALBUM_INVALID_CAPACITY); } @@ - if (createdThisWeek >= 3) { + if (createdThisWeek >= MAX_CREATE_PER_WEEK) { throw new AlbumException(AlbumErrorCode.ALBUM_CREATION_LIMIT_EXCEEDED); }
75-77: 불필요한 엔티티 로딩 없이 존재 여부만 확인Optional 조회 후 ifPresent로 예외를 던지는 대신 existsBy… 쿼리로 변경하면 I/O를 줄일 수 있습니다.
- albumParticipantRepository.findByAlbumIdAndUserIdAndIsBlacklistedTrue(album.getId(), user.getId()) - .ifPresent(blacklisted -> { - throw new AlbumException(AlbumErrorCode.USER_IS_BLACKLISTED); - }); + boolean blacklisted = albumParticipantRepository + .existsByAlbumIdAndUserIdAndIsBlacklistedTrue(album.getId(), user.getId()); + if (blacklisted) { + throw new AlbumException(AlbumErrorCode.USER_IS_BLACKLISTED); + }src/main/java/com/cheeeese/album/domain/Album.java (2)
33-35: DB 제약과 Validator 일치시키기 — emoji 컬럼 NOT NULL 권장Validator에서 이모지 미선택을 금지하므로 DB 스키마도 일치시키는 것이 안전합니다.
- @Column(name = "emoji") + @Column(name = "emoji", nullable = false) private String emoji;
68-70: 현재 시각 의존성 분리 — 만료 판정 테스트 안정화엔티티 내부에서 LocalDateTime.now()를 직접 호출하면 테스트/타임존 이슈가 생깁니다. now를 인자로 받는 메서드를 추가하고 기존 메서드는 이를 위임하도록 개선을 제안합니다.
- public boolean isExpired() { - return this.expiredAt.isBefore(LocalDateTime.now()) || this.status == AlbumStatus.EXPIRED; - } + public boolean isExpired() { + return isExpiredAt(LocalDateTime.now()); + } + + public boolean isExpiredAt(LocalDateTime now) { + return this.expiredAt.isBefore(now) || this.status == AlbumStatus.EXPIRED; + }src/main/java/com/cheeeese/album/dto/request/AlbumCreationRequest.java (1)
8-25: 입력단 Bean Validation 추가로 1차 방어선 강화컨트롤러에서 @Valid로 빠르게 걸러내도록 제약 annotation을 추가하면 UX/성능 측면에서 이점이 큽니다.
+import jakarta.validation.constraints.*; @@ public record AlbumCreationRequest( - @Schema(description = "썸네일 이모지", example = "U+1F9C0") - String emoji, + @Schema(description = "썸네일 이모지", example = "U+1F9C0") + @NotBlank + String emoji, @@ - @Schema(description = "앨범 이름", example = "졸업식") - String title, + @Schema(description = "앨범 이름", example = "졸업식") + @NotBlank + String title, @@ - @Schema(description = "참여자 수", example = "64") - int participant, + @Schema(description = "참여자 수", example = "64") + @Min(1) @Max(64) + int participant, @@ - @Schema(description = "행사 날짜", example = "2025-02-01") - LocalDate eventDate, + @Schema(description = "행사 날짜", example = "2025-02-01") + @NotNull @PastOrPresent + LocalDate eventDate, @@ - @Schema(description = "앨범 생성 필수 약관 동의", example = "true") - boolean isTermsAgreement + @Schema(description = "앨범 생성 필수 약관 동의", example = "true") + @AssertTrue + boolean isTermsAgreement ) { }src/main/java/com/cheeeese/album/exception/code/AlbumErrorCode.java (1)
17-23: HTTP 상태 코드 재검토(선택)주간 생성 한도 초과는 409도 가능하지만, 빈도 제어 관점에서는 429(Too Many Requests)를 고려할 수 있습니다. 팀 API 규약에 맞춰 결정하면 좋겠습니다.
src/main/java/com/cheeeese/album/infrastructure/mapper/AlbumMapper.java (2)
40-40: maxPhotoCount를 파라미터로 받는 것을 고려해보세요.현재
maxPhotoCount가 2000으로 하드코딩되어 있습니다. PR 설명에서 언급하신 것처럼 기획팀에서 앨범별로 최대 사진 개수를 조정할 가능성이 있다면, 이를 메서드 파라미터로 받아 유연하게 대응할 수 있도록 하는 것이 좋습니다.다음과 같이 수정을 고려해보세요:
public static Album toEntity( Long hostId, String title, String code, String emoji, int participant, LocalDate eventDate, boolean isInfoAvailable, LocalDateTime expiredAt, - boolean isTermsAgreement + boolean isTermsAgreement, + int maxPhotoCount ) { return Album.builder() .hostId(hostId) .title(title) .code(code) .emoji(emoji) .participant(participant) .currentParticipant(1) .eventDate(eventDate) - .maxPhotoCount(2000) + .maxPhotoCount(maxPhotoCount) .currentPhotoCount(0) .isInfoAvailable(isInfoAvailable) .expiredAt(expiredAt) .status(Album.AlbumStatus.ACTIVE) .isTermsAgreement(isTermsAgreement) .build(); }
112-112: 주석을 명확히 수정해주세요."isBlacklisted = false 역할의"라는 표현이 부정확합니다.
isBlacklisted는 역할(role)이 아니라 불리언 플래그입니다. 다음과 같이 수정하는 것이 좋겠습니다:- * User와 Album 엔티티를 기반으로 isBlacklisted = false 역할의 AlbumParticipant 엔티티를 생성합니다. + * User와 Album 엔티티를 기반으로 isBlacklisted = false로 설정된 AlbumParticipant 엔티티를 생성합니다.
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (7)
src/main/java/com/cheeeese/album/application/AlbumService.java(4 hunks)src/main/java/com/cheeeese/album/application/validator/AlbumValidator.java(2 hunks)src/main/java/com/cheeeese/album/domain/Album.java(4 hunks)src/main/java/com/cheeeese/album/dto/request/AlbumCreationRequest.java(1 hunks)src/main/java/com/cheeeese/album/dto/response/AlbumCreationResponse.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(3 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- src/main/java/com/cheeeese/album/dto/response/AlbumCreationResponse.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(16-131)src/main/java/com/cheeeese/photo/application/PhotoService.java (1)
Service(12-29)
🔇 Additional comments (4)
src/main/java/com/cheeeese/album/application/AlbumService.java (1)
106-118: 동시성 방어 검토 메모이미 참여한 사용자 선체크 + unique(userId, albumId) 제약 + DataIntegrityViolationException 매핑 흐름은 합리적입니다. 트랜잭션 롤백 정책이 RuntimeException(AlbumException 포함)에 대해 활성화되어 있는지 한 번만 확인해 주세요.
src/main/java/com/cheeeese/album/infrastructure/mapper/AlbumMapper.java (3)
49-60: LGTM!앨범 생성 응답 DTO 매핑 로직이 명확하고 적절합니다.
115-119: 역할 구분 로직이 의도적으로 설계되었습니다.검증 결과,
AlbumParticipant는 역할을 저장하는 필드가 없습니다. 대신 호스트/게스트 구분은Album.hostId와AlbumParticipant.userId를 비교하여 도출됩니다:
- 호스트:
userId == album.hostId인 경우- 게스트: 그 외
AlbumParticipant참가자
toGuestUserAlbum()메서드 이름과 주석이 이 설계를 명확히 보여줍니다. 역할을 별도 필드로 저장하지 않고 관계로 도출하는 것은 정상적인 설계 패턴입니다.
38-38:currentParticipant초기화 값이 호스트 중복 계산을 야기할 가능성 검증 필요코드 분석 결과, 다음 흐름이 확인되었습니다:
- 앨범 생성 시:
currentParticipant = 1로 초기화 (호스트 사전 계산)handleAlbumParticipation()실행 시:
- 호스트의
AlbumParticipant레코드 저장 (line 117)currentParticipant원자적으로 증가 (line 119)결과적으로 실제
AlbumParticipant레코드는 1개(호스트)이지만,currentParticipant는 2로 계산되어 불일치가 발생할 수 있습니다. 이 로직이 의도된 설계(별도의 카운터 역할)인지, 아니면 호스트 중복 계산 버그인지 확인하여 데이터 일관성을 보장해주세요.
src/main/java/com/cheeeese/album/infrastructure/mapper/AlbumMapper.java
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (5)
src/main/java/com/cheeeese/album/dto/response/AlbumCreationResponse.java (1)
8-8: record에서 @builder 제거 권장Java record는 이미 모든 필드를 받는 표준 생성자를 제공하므로, Lombok의
@Builder어노테이션이 불필요합니다. 일부 Lombok 버전에서는 record와 함께 사용 시 올바르게 작동하지 않을 수 있습니다.-@Builder @Schema(description = "앨범 생성 API") public record AlbumCreationResponse(src/main/java/com/cheeeese/album/dto/request/AlbumCreationRequest.java (1)
8-8: record에서 @builder 제거 권장AlbumCreationResponse와 동일하게, Java record는 표준 생성자를 제공하므로
@Builder가 불필요합니다.-@Builder @Schema(description = "앨범 생성 API") public record AlbumCreationRequest(src/main/java/com/cheeeese/album/application/AlbumService.java (3)
64-64: 만료 기간 상수화 권장하드코딩된 만료 기간
7을 상수로 추출하면 유지보수성이 향상됩니다.+private static final int ALBUM_EXPIRATION_DAYS = 7; -LocalDateTime.now().plusDays(7), +LocalDateTime.now().plusDays(ALBUM_EXPIRATION_DAYS),
50-50: UUID 버전 불일치 - v7 대신 v6 생성 중PR 목표에서 UUID v7 채택을 명시했으나,
UuidCreator.getTimeOrdered()는 UUID v6을 반환합니다. UUID v7을 생성하려면getTimeOrderedEpoch()를 사용해야 합니다.-String code = UuidCreator.getTimeOrdered().toString(); +String code = UuidCreator.getTimeOrderedEpoch().toString();
97-103: 주간 범위 계산 오류 - 동적 시간 사용으로 인한 부정확한 집계Line 101의
atTime(LocalTime.now())는 현재 시각을 사용하여 종료 시점을 동적으로 설정하므로, 호출 시각에 따라 집계 범위가 달라지는 문제가 있습니다.예를 들어:
- 오전 10시 호출 시: 다음 월요일 10:00까지 집계
- 오후 3시 호출 시: 다음 월요일 15:00까지 집계
의도한 범위는 [이번 주 월요일 00:00, 다음 주 월요일 00:00)의 반개방 구간이어야 합니다.
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().with(DayOfWeek.MONDAY).plusWeeks(1).atStartOfDay() ); }추가로, SQL의
BETWEEN연산자는 양쪽 경계를 포함하므로, 저장소 메서드를>=와<를 사용하는 방식으로 변경하는 것을 권장합니다.
🧹 Nitpick comments (1)
src/main/java/com/cheeeese/album/application/validator/AlbumValidator.java (1)
22-50: 검증 로직이 잘 구현되었으나 타임존 고려 필요전반적인 검증 로직은 올바르게 구현되었습니다. 다만 Line 39의
LocalDate.now()는 서버의 시스템 타임존을 사용하므로, 사용자와 서버가 다른 타임존에 있을 경우 "미래 날짜" 검증이 의도와 다르게 동작할 수 있습니다.선택적 개선 사항:
- 타임존 명시화:
-if (request.eventDate().isAfter(LocalDate.now())) { +if (request.eventDate().isAfter(LocalDate.now(ZoneId.of("Asia/Seoul")))) {
- 매직 넘버 상수화:
+private static final int MAX_PARTICIPANT_COUNT = 64; +private static final int WEEKLY_ALBUM_CREATION_LIMIT = 3; -if (request.participant() < 1 || request.participant() > 64) { +if (request.participant() < 1 || request.participant() > MAX_PARTICIPANT_COUNT) { throw new AlbumException(AlbumErrorCode.ALBUM_INVALID_CAPACITY); } -if (createdThisWeek >= 3) { +if (createdThisWeek >= WEEKLY_ALBUM_CREATION_LIMIT) { throw new AlbumException(AlbumErrorCode.ALBUM_CREATION_LIMIT_EXCEEDED); }
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (7)
src/main/java/com/cheeeese/album/application/AlbumService.java(4 hunks)src/main/java/com/cheeeese/album/application/validator/AlbumValidator.java(2 hunks)src/main/java/com/cheeeese/album/domain/Album.java(4 hunks)src/main/java/com/cheeeese/album/dto/request/AlbumCreationRequest.java(1 hunks)src/main/java/com/cheeeese/album/dto/response/AlbumCreationResponse.java(1 hunks)src/main/java/com/cheeeese/album/dto/response/AlbumInvitationResponse.java(1 hunks)src/main/java/com/cheeeese/album/infrastructure/mapper/AlbumMapper.java(3 hunks)
🧰 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(16-131)src/main/java/com/cheeeese/photo/application/PhotoService.java (1)
Service(12-29)
🔇 Additional comments (5)
src/main/java/com/cheeeese/album/dto/response/AlbumInvitationResponse.java (1)
14-15: 필드명 변경이 올바르게 적용되었습니다.
themeImageUrl에서themeEmoji로의 필드명 변경이 도메인 모델의 변경사항과 일치하며, Swagger 문서도 적절하게 업데이트되었습니다.src/main/java/com/cheeeese/album/domain/Album.java (1)
33-34: 도메인 모델 변경이 올바르게 적용되었습니다.
themeImageUrl에서themeEmoji로의 필드명 변경과isTermsAgreement필드 추가가 적절하게 구현되었으며, 컬럼 매핑도 정확합니다.Also applies to: 61-62
src/main/java/com/cheeeese/album/application/AlbumService.java (1)
105-133: 참가자 등록 로직이 잘 구현되었습니다.동시성 처리를 위한 원자적 증가 및
DataIntegrityViolationException처리가 적절하게 구현되었으며, 중복 등록 방지 로직도 올바릅니다.src/main/java/com/cheeeese/album/infrastructure/mapper/AlbumMapper.java (2)
21-47: 앨범 엔티티 생성 로직 확인
toEntity메서드가 잘 구현되었습니다. Line 40의maxPhotoCount = 2000하드코딩은 PR 코멘트에서 언급된 대로, 향후 앨범별로 조정 가능성을 고려한 설계입니다.
52-60: 생성 응답 매핑 로직이 올바르게 구현되었습니다.
toCreationResponse메서드가 앨범 정보를 적절하게 매핑하고 있습니다. (단,AlbumCreationResponse의@Builder어노테이션 제거는 별도 코멘트 참고)
src/main/java/com/cheeeese/album/infrastructure/mapper/AlbumMapper.java
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
Actionable comments posted: 1
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
src/main/java/com/cheeeese/album/dto/response/AlbumEnterResponse.java(1 hunks)src/main/java/com/cheeeese/album/infrastructure/mapper/AlbumMapper.java(3 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- src/main/java/com/cheeeese/album/infrastructure/mapper/AlbumMapper.java
🔗 연관된 이슈
🚀 변경 유형
📝 작업 내용
📸 스크린샷
-> 생각보다 insert에서 속도가 크게 차이가 나서 v7으로 코드 생성 로직 구현했습니다!
💬 리뷰 요구사항
maxPhotoCount는 앨범 당 최대 업로드 가능한 사진 수 의미하는 게 맞을까요?! 올릴 수 있는 사진은 모든 앨범마다 2000장으로 고정되어 있는 걸로 알고 있는데 그럼 해당 컬럼이 필요할까 싶어서 여쭤봅니당Summary by CodeRabbit
새로운 기능
개선 사항
테스트
기타