Conversation
WalkthroughTrip 하위에 Album 도메인을 추가하고 CRUD REST API, Swagger 문서 어노테이션, DTO/Repository/서비스 구현(권한 검사, QueryDSL 기반 사진 개수 집계 포함) 및 관련 단위/HTTP 테스트를 추가했습니다. Changes
Sequence Diagram(s)sequenceDiagram
participant Client
participant AlbumController as Controller
participant TripPermissionService as PermissionSvc
participant AlbumService as Service
participant AlbumRepository as Repo
participant PhotoRepository as PhotoRepo
rect rgb(240,248,255)
Client->>Controller: POST /trips/{tripId}/albums?userId
Controller->>PermissionSvc: getEditableTrip(userId, tripId)
PermissionSvc-->>Controller: Trip (ok) / throw
Controller->>Service: createAlbum(userId, tripId, dto)
end
rect rgb(245,255,240)
Service->>Repo: save(Album)
Repo-->>Service: saved Album
Service->>PhotoRepo: (none for create)
Service-->>Controller: AlbumResponseDto
Controller-->>Client: 201 CustomResponse<AlbumResponseDto>
end
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related PRs
Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
📜 Recent review detailsConfiguration used: Repository UI Review profile: CHILL Plan: Pro 📒 Files selected for processing (2)
🚧 Files skipped from review as they are similar to previous changes (2)
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 |
There was a problem hiding this comment.
Actionable comments posted: 4
🧹 Nitpick comments (7)
src/test/http/album.http (1)
1-24: 테스트 시나리오가 잘 구성되어 있습니다.모든 CRUD 작업을 적절히 테스트하고 있습니다. 다만 하드코딩된 ID(tripId=1, albumId=1, userId=1)를 사용하고 있으니, 파일 상단에 테스트 데이터 사전 설정이 필요하다는 주석을 추가하는 것을 권장합니다.
🔎 테스트 데이터 준비 안내 주석 추가 제안
+### 테스트 실행 전 필요 데이터: +### - userId=1인 사용자 +### - tripId=1인 여행 (userId=1이 편집 권한 보유) +### ### 앨범 생성 (Create Album) POST http://localhost:8080/api/trips/1/albums?userId=1 Content-Type: application/jsonsrc/main/java/com/example/pventure/domain/album/dto/response/AlbumResponseDto.java (1)
27-33: Factory 메서드에 null 방어 로직 검토 필요
album파라미터가 null인 경우NullPointerException이 발생할 수 있습니다. 서비스 레이어에서 null 체크를 수행한다면 문제없지만, 방어적 코딩 관점에서 검토해 주세요.src/main/java/com/example/pventure/domain/album/repository/AlbumRepositoryCustom.java (1)
10-10: 단건 조회 메서드의 반환 타입으로Optional사용 권장
findByIdAndTripWithPhotoCount메서드가 앨범을 찾지 못할 경우null을 반환합니다. Spring Data JPA 관례에 따라Optional<AlbumWithPhotoCountQDto>를 사용하면 호출자 측에서 null 처리를 명시적으로 강제할 수 있어 NPE 방지에 도움이 됩니다.🔎 Optional 적용 제안
- AlbumWithPhotoCountQDto findByIdAndTripWithPhotoCount(Long albumId, Long tripId); + Optional<AlbumWithPhotoCountQDto> findByIdAndTripWithPhotoCount(Long albumId, Long tripId);구현체(
AlbumRepositoryCustomImpl)에서도Optional.ofNullable()로 감싸서 반환하면 됩니다.src/main/java/com/example/pventure/domain/album/dto/query/AlbumWithPhotoCountQDto.java (1)
3-7: 사용하지 않는 import 제거 필요
Schema,Builder,NoArgsConstructorimport가 선언되어 있지만 실제 코드에서 사용되지 않습니다. 불필요한 import는 제거하는 것이 좋습니다.🔎 수정 제안
package com.example.pventure.domain.album.dto.query; -import io.swagger.v3.oas.annotations.media.Schema; import lombok.AllArgsConstructor; -import lombok.Builder; import lombok.Getter; -import lombok.NoArgsConstructor;src/main/java/com/example/pventure/domain/album/repository/AlbumRepositoryCustomImpl.java (1)
22-26: QAlbum, QPhoto 인스턴스를 클래스 필드로 추출 고려
QAlbum.album과QPhoto.photo가 여러 메서드에서 반복 선언됩니다. 클래스 레벨private static final필드로 추출하면 중복을 줄일 수 있습니다.🔎 리팩토링 제안
@Repository @RequiredArgsConstructor public class AlbumRepositoryCustomImpl implements AlbumRepositoryCustom { + private static final QAlbum album = QAlbum.album; + private static final QPhoto photo = QPhoto.photo; + private final JPAQueryFactory queryFactory; @Override public List<AlbumWithPhotoCountQDto> findAllByTripWithPhotoCount(Long tripId) { - QAlbum album = QAlbum.album; - return queryAlbumWithPhotoCount() .where(album.trip.id.eq(tripId)) .fetch(); }Also applies to: 32-33, 44-45
src/main/java/com/example/pventure/domain/trip/service/TripPermissionService.java (1)
23-43: 기존 TripFinder 재사용 검토현재 구현은 User와 Trip을 직접 로드하고 있지만, 기존
TripFinder.findById메서드가 이미 member fetch join과 예외 처리를 포함하여 동일한 기능을 제공합니다. TripFinder를 재사용하면 중복을 줄이고 일관성을 높일 수 있습니다.🔎 TripFinder를 활용한 리팩토링안
+private final TripFinder tripFinder; public Trip getEditableTrip(Long userId, Long tripId) { User user = loadUser(userId); - Trip trip = loadTrip(tripId); + Trip trip = tripFinder.findById(tripId, false); if (!memberService.canEdit(user, trip)) { throw new ApiException(ErrorCode.UNAUTHORIZED_MEMBER_ACCESS); } return trip; } public Trip getViewableTrip(Long userId, Long tripId) { User user = loadUser(userId); - Trip trip = loadTrip(tripId); + Trip trip = tripFinder.findById(tripId, false); if (!memberService.isMember(user, trip)) { throw new ApiException(ErrorCode.UNAUTHORIZED_MEMBER_ACCESS); } return trip; }이 경우
loadTrip메서드는 제거 가능합니다.src/main/java/com/example/pventure/domain/album/service/AlbumServiceImpl.java (1)
82-82: 앨범 수정 후 사진 개수를 별도 쿼리로 조회하는 비효율앨범 수정 직후
photoService.countPhotos로 별도 쿼리를 실행합니다. 업데이트 트랜잭션 내에서 추가 쿼리가 발생하므로, 가능하면 쿼리 프로젝션이나 캐싱으로 최적화할 수 있습니다.다만, 수정 빈도가 낮다면 현재 구현도 허용 가능합니다.
🔎 쿼리 프로젝션을 활용한 최적화안
업데이트 후
findByIdAndTripWithPhotoCount를 재호출:album.updateTitle(requestDto.getTitle()); -return AlbumResponseDto.from(album, photoService.countPhotos(album)); +AlbumWithPhotoCountQDto updatedAlbum = + albumRepository.findByIdAndTripWithPhotoCount(albumId, trip.getId()); +return AlbumResponseDto.from(updatedAlbum);또는 현재 구현을 유지하되, PhotoService에서 캐싱을 적용하는 방안도 고려 가능합니다.
📜 Review details
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (25)
src/main/java/com/example/pventure/domain/album/controller/AlbumController.javasrc/main/java/com/example/pventure/domain/album/controller/README.mdsrc/main/java/com/example/pventure/domain/album/docs/AlbumSwaggerDocs.javasrc/main/java/com/example/pventure/domain/album/docs/CreateAlbumDocs.javasrc/main/java/com/example/pventure/domain/album/docs/DeleteAlbumDocs.javasrc/main/java/com/example/pventure/domain/album/docs/GetAlbumDocs.javasrc/main/java/com/example/pventure/domain/album/docs/GetAlbumsDocs.javasrc/main/java/com/example/pventure/domain/album/docs/UpdateAlbumDocs.javasrc/main/java/com/example/pventure/domain/album/dto/query/AlbumWithPhotoCountQDto.javasrc/main/java/com/example/pventure/domain/album/dto/request/AlbumRequestDto.javasrc/main/java/com/example/pventure/domain/album/dto/request/README.mdsrc/main/java/com/example/pventure/domain/album/dto/response/AlbumResponseDto.javasrc/main/java/com/example/pventure/domain/album/dto/response/README.mdsrc/main/java/com/example/pventure/domain/album/entity/Album.javasrc/main/java/com/example/pventure/domain/album/repository/AlbumRepository.javasrc/main/java/com/example/pventure/domain/album/repository/AlbumRepositoryCustom.javasrc/main/java/com/example/pventure/domain/album/repository/AlbumRepositoryCustomImpl.javasrc/main/java/com/example/pventure/domain/album/repository/README.mdsrc/main/java/com/example/pventure/domain/album/service/AlbumService.javasrc/main/java/com/example/pventure/domain/album/service/AlbumServiceImpl.javasrc/main/java/com/example/pventure/domain/album/service/README.mdsrc/main/java/com/example/pventure/domain/trip/service/TripPermissionService.javasrc/main/java/com/example/pventure/domain/trip/service/TripService.javasrc/test/http/album.httpsrc/test/java/com/example/pventure/domain/album/service/AlbumServiceTest.java
💤 Files with no reviewable changes (6)
- src/main/java/com/example/pventure/domain/album/repository/README.md
- src/main/java/com/example/pventure/domain/album/dto/response/README.md
- src/main/java/com/example/pventure/domain/trip/service/TripService.java
- src/main/java/com/example/pventure/domain/album/controller/README.md
- src/main/java/com/example/pventure/domain/album/service/README.md
- src/main/java/com/example/pventure/domain/album/dto/request/README.md
🧰 Additional context used
🧠 Learnings (3)
📚 Learning: 2025-11-06T03:40:48.230Z
Learnt from: rabitis99
Repo: khg9900/Pventure_BE PR: 26
File: src/main/java/com/example/pventure/domain/trip/repository/TripRepositoryCustomImpl.java:48-55
Timestamp: 2025-11-06T03:40:48.230Z
Learning: In TripRepositoryCustomImpl.findByUserAndPeriod (src/main/java/com/example/pventure/domain/trip/repository/TripRepositoryCustomImpl.java), trip.members is always fetch joined regardless of the includeMembers flag because member data is required for authorization checks. Only the member.user association is conditionally fetch joined based on the includeMemberUserJoin parameter to optimize loading of user details.
Applied to files:
src/main/java/com/example/pventure/domain/album/repository/AlbumRepository.javasrc/main/java/com/example/pventure/domain/trip/service/TripPermissionService.javasrc/main/java/com/example/pventure/domain/album/repository/AlbumRepositoryCustom.javasrc/main/java/com/example/pventure/domain/album/repository/AlbumRepositoryCustomImpl.java
📚 Learning: 2025-11-06T03:42:12.797Z
Learnt from: rabitis99
Repo: khg9900/Pventure_BE PR: 26
File: src/main/java/com/example/pventure/domain/tripFolder/repository/TripFolderRepository.java:15-20
Timestamp: 2025-11-06T03:42:12.797Z
Learning: In TripFolderRepository.findByFolder (src/main/java/com/example/pventure/domain/tripFolder/repository/TripFolderRepository.java), user information (member.user) must always be eagerly loaded via fetch join because it is required for membership validation and authorization checks when retrieving trips in a folder, regardless of any include parameter.
Applied to files:
src/main/java/com/example/pventure/domain/album/repository/AlbumRepository.javasrc/main/java/com/example/pventure/domain/trip/service/TripPermissionService.javasrc/main/java/com/example/pventure/domain/album/repository/AlbumRepositoryCustom.javasrc/main/java/com/example/pventure/domain/album/repository/AlbumRepositoryCustomImpl.java
📚 Learning: 2025-11-05T13:46:05.156Z
Learnt from: rabitis99
Repo: khg9900/Pventure_BE PR: 24
File: src/main/java/com/example/pventure/domain/trip/entity/Trip.java:57-60
Timestamp: 2025-11-05T13:46:05.156Z
Learning: In the Trip entity (src/main/java/com/example/pventure/domain/trip/entity/Trip.java), the updateDates method intentionally allows null values to overwrite existing startDate and endDate fields, unlike other update methods (updateTitle, updateDestination, updateTripStatus) which preserve existing values when null is passed. This is a deliberate design decision to allow dates to be explicitly cleared.
Applied to files:
src/main/java/com/example/pventure/domain/trip/service/TripPermissionService.java
🧬 Code graph analysis (3)
src/main/java/com/example/pventure/domain/trip/service/TripPermissionService.java (1)
src/main/java/com/example/pventure/domain/trip/util/TripFinder.java (1)
RequiredArgsConstructor(15-58)
src/main/java/com/example/pventure/domain/album/controller/AlbumController.java (2)
src/main/java/com/example/pventure/global/response/CustomResponseHelper.java (1)
CustomResponseHelper(7-31)src/main/java/com/example/pventure/domain/trip/util/TripFinder.java (1)
RequiredArgsConstructor(15-58)
src/main/java/com/example/pventure/domain/album/service/AlbumServiceImpl.java (1)
src/main/java/com/example/pventure/domain/trip/util/TripFinder.java (1)
RequiredArgsConstructor(15-58)
🔇 Additional comments (24)
src/main/java/com/example/pventure/domain/album/docs/DeleteAlbumDocs.java (1)
1-21: 구현이 적절합니다.Swagger 문서화 어노테이션이 표준 패턴을 따르고 있으며, DELETE 작업에 적합한 응답 코드(204, 403, 404)가 정의되어 있습니다.
src/main/java/com/example/pventure/domain/album/entity/Album.java (1)
30-30: 메서드 구현이 적절합니다.제목 업데이트 메서드가 정상적으로 추가되었습니다. 입력 검증은 DTO 레이어에서
@NotBlank로 처리되고 있어 문제없습니다.src/main/java/com/example/pventure/domain/album/dto/request/AlbumRequestDto.java (2)
1-21: DTO 구조가 적절합니다.Lombok 어노테이션과 Bean Validation이 적절히 적용되어 있으며, Swagger 문서화도 포함되어 있습니다.
23-28: 엔티티 변환 로직이 올바릅니다.DTO에서 Album 엔티티로의 변환이 적절하게 구현되어 있으며, Trip 연관관계도 정확히 설정됩니다.
src/main/java/com/example/pventure/domain/album/docs/GetAlbumsDocs.java (1)
1-21: Swagger 문서화가 적절합니다.앨범 목록 조회 API에 대한 문서화 어노테이션이 일관된 패턴으로 구현되어 있으며, 응답 코드도 적절합니다.
src/main/java/com/example/pventure/domain/album/docs/UpdateAlbumDocs.java (1)
1-22: UPDATE 작업 문서화가 적절합니다.수정 작업에 필요한 모든 응답 코드(200, 400, 403, 404)가 정의되어 있으며, 다른 문서화 어노테이션들과 일관된 구조를 따릅니다.
src/main/java/com/example/pventure/domain/album/docs/CreateAlbumDocs.java (1)
1-18: CREATE 작업 문서화가 적절합니다.생성 작업에 적합한 응답 코드(201, 400, 403, 404)가 정의되어 있으며, 표준 패턴을 따르고 있습니다.
src/main/java/com/example/pventure/domain/album/repository/AlbumRepository.java (2)
8-8: 커스텀 Repository 확장이 적절합니다.
AlbumRepositoryCustom확장을 통해 QueryDSL 기반의 복잡한 쿼리(예: 사진 개수 포함 조회)를 지원할 수 있습니다. 표준 패턴을 따르고 있습니다.
10-10: Trip 범위 조회 메서드가 적절합니다.특정 Trip 내에서 Album을 조회하는 메서드로, 권한 검증 및 데이터 격리에 유용합니다. Spring Data JPA 네이밍 규칙을 따르며
Optional반환으로 null-safety도 확보되어 있습니다.src/main/java/com/example/pventure/domain/album/dto/response/AlbumResponseDto.java (1)
35-41: QueryDSL DTO 매핑 팩토리 메서드 구현이 적절합니다.
AlbumWithPhotoCountQDto에서 응답 DTO로의 변환 로직이 명확하고, 두 개의 오버로딩된 팩토리 메서드로 사용 목적에 따른 변환을 깔끔하게 분리했습니다.src/main/java/com/example/pventure/domain/album/dto/query/AlbumWithPhotoCountQDto.java (1)
9-18: 불변 Query DTO 설계가 적절합니다.
final필드와@AllArgsConstructor를 사용하여 QueryDSLProjections.constructor와 호환되는 불변 DTO로 설계되었습니다. 쿼리 결과 매핑에 적합한 구조입니다.src/test/java/com/example/pventure/domain/album/service/AlbumServiceTest.java (4)
70-74: 리플렉션을 통한 ID 설정은 테스트 목적으로 허용됨
setId헬퍼 메서드가 리플렉션을 사용하여BaseEntity의 ID를 설정합니다. 엔티티 ID가 DB에서 생성되는 구조에서 테스트를 위한 일반적인 패턴입니다. 다만, 테스트 유틸리티 클래스로 분리하면 다른 테스트에서도 재사용할 수 있습니다.
76-97: createAlbum 성공 테스트 케이스가 적절합니다.권한 체크, 저장소 호출, 응답 DTO 검증이 모두 포함되어 있습니다.
photoCount가 새 앨범의 경우 0임을 확인하는 것도 좋습니다.
286-314: deleteAlbum 성공 테스트에서 사진 연관 해제 검증이 좋습니다.앨범 삭제 시 연관된 사진들의
removeAlbum()호출을 검증하여, 데이터 정합성이 유지됨을 확인합니다. 사진이 삭제되지 않고 연관만 해제되는 비즈니스 로직을 명확히 테스트하고 있습니다.
215-234: getAlbum 실패 케이스: null 반환 검증현재
findByIdAndTripWithPhotoCount가null을 반환할 때NOT_FOUND_ALBUM예외가 발생함을 테스트합니다. 앞서 제안한 대로Optional을 사용하면 이 테스트도.thenReturn(Optional.empty())로 변경해야 합니다.src/main/java/com/example/pventure/domain/album/docs/GetAlbumDocs.java (1)
11-21: Swagger 문서화 어노테이션 구조가 깔끔합니다.
@Operation과@ApiResponse를 조합하여 API 스펙을 명확하게 정의했습니다. 상수를AlbumSwaggerDocs에서 참조하여 중앙 관리하는 패턴이 일관성 유지에 좋습니다.src/main/java/com/example/pventure/domain/album/service/AlbumService.java (1)
7-19: 서비스 인터페이스 설계가 적절합니다.모든 메서드에
userId를 포함하여 권한 검증 컨텍스트를 명확히 전달합니다. CRUD 연산이 일관된 시그니처로 정의되어 있어 구현체와 컨트롤러 계층에서 사용하기 편리합니다.src/main/java/com/example/pventure/domain/album/repository/AlbumRepositoryCustomImpl.java (2)
42-57: QueryDSL 쿼리 구조가 적절합니다.
leftJoin을 사용하여 사진이 없는 앨범도 포함되고,photo.count()로 사진 개수를 집계합니다. 헬퍼 메서드로 쿼리 로직을 재사용하는 것도 좋습니다.
29-40: 단건 조회 시 결과가 없을 때 null 반환 주의
fetchOne()은 결과가 없으면null을 반환하고, 결과가 2개 이상이면NonUniqueResultException을 던집니다. 앨범 ID와 여행 ID 조합이 유니크하다면 문제없지만,Optional로 감싸면 호출자 측에서 더 안전하게 처리할 수 있습니다.src/main/java/com/example/pventure/domain/album/docs/AlbumSwaggerDocs.java (1)
6-28: Swagger 문서 상수 관리 패턴이 좋습니다.문서화 문자열을 중앙 집중화하여 일관성을 유지하고 유지보수를 용이하게 합니다.
@NoArgsConstructor(access = AccessLevel.PRIVATE)로 인스턴스화를 방지한 것도 적절합니다.src/main/java/com/example/pventure/domain/album/controller/AlbumController.java (1)
31-35: 컨트롤러 구조 및 응답 표준화는 적절함Tag 어노테이션, RequestMapping, 그리고 CustomResponseHelper 활용이 일관되게 적용되어 있습니다.
src/main/java/com/example/pventure/domain/album/service/AlbumServiceImpl.java (3)
31-40: 앨범 생성 로직 및 권한 검증 흐름이 명확함TripPermissionService를 통한 편집 권한 확인, DTO to Entity 변환, 그리고 초기 사진 개수 0으로 응답 생성이 적절하게 구현되어 있습니다.
44-54: QueryDSL 기반 사진 개수 포함 조회 로직이 효율적
findAllByTripWithPhotoCount를 사용하여 N+1 쿼리 없이 앨범 목록과 사진 개수를 한 번에 조회하는 구조가 잘 설계되었습니다.
58-70: 단건 조회 시 null 체크 및 예외 처리가 정확함DTO 기반 쿼리 결과가 null일 경우
NOT_FOUND_ALBUM예외를 발생시키는 로직이 명확하고 안전합니다.
📝 PR 요약 (Summary)
🔗 관련 이슈 (Related Issues)
🧩 변경 유형 (Change Type)
이 PR의 변경 사항에 해당하는 항목을 선택하세요.
🧾 주요 변경 내용 (What was changed)
공통 권한 서비스
TripPermissionService추가Trip편집 권한 기반 접근 제어 로직 분리 및 재사용 가능 구조로 개선Album도메인 구현Album생성 / 수정 / 삭제 / 단건 조회 / 목록 조회 API 구현Custom Repository및QDto적용Swagger 문서 작성
Album관련 API 스펙 명시 및 요청/응답 구조 문서화테스트 코드
AlbumServiceTest추가HTTP 시나리오 테스트 파일(
album.http) 작성🧪 테스트 및 검증 (Test)
테스트하거나 검증한 항목을 체크해주세요.
결과 요약:
✅ PR 체크리스트 (Checklist)
PR 제출 전 다음 항목을 확인하세요.
Ctrl + Alt + LorPrettier)💬 추가 참고사항 (Notes)
Summary by CodeRabbit
릴리스 노트
새로운 기능
문서
테스트
✏️ Tip: You can customize this high-level summary in your review settings.