Skip to content

Feature/photo s3#46

Open
khg9900 wants to merge 11 commits intodevfrom
feature/photo-s3
Open

Feature/photo s3#46
khg9900 wants to merge 11 commits intodevfrom
feature/photo-s3

Conversation

@khg9900
Copy link
Copy Markdown
Owner

@khg9900 khg9900 commented Dec 25, 2025

📝 PR 요약 (Summary)

  • S3 업로드 연동을 위한 인프라 설정 추가
  • Photo 도메인 업로드/조회/이동/삭제 기능 구현
  • Swagger 문서 정리 및 테스트/seed 데이터 보완

🔗 관련 이슈 (Related Issues)


🧩 변경 유형 (Change Type)

이 PR의 변경 사항에 해당하는 항목을 선택하세요.

  • 🚀 기능 추가 (Feature)
  • 🐛 버그 수정 (Bug Fix)
  • 🧰 코드 리팩토링 (Refactor)
  • 🧱 구조 개선 / 폴더 정리 (Structure)
  • 🧪 테스트 코드 추가 / 수정 (Test)
  • 📄 문서 수정 (Docs)
  • ♻️ 기타 (기타 작업)

🧾 주요 변경 내용 (What was changed)

  1. S3 업로드 인프라 구성
  • S3 연동 설정 추가
  • S3Config, S3Service, S3KeyGenerator 구현
  • application-dev.yml 설정 반영
  • AWS SDK 의존성 추가 (build.gradle)
  1. Photo 도메인 기능 구현
  • Photo API 구현
    • 업로드 URL 생성
    • 사진 메타데이터 등록
    • 앨범별 사진 조회
    • 미지정(앨범 없는) 사진 조회
    • 사진 단건 조회
    • 사진 이동(앨범 변경)
    • 사진 삭제
  • Trip 기반 권한 체크
    • TripPermissionService를 활용하여 업로드/이동/삭제 시 편집 권한 검증
  1. 문서 및 테스트 정리
  • Photo 및 연관 도메인 Swagger 문서 정리
  • PhotoServiceTest 추가
  • API 테스트용 HTTP 시나리오(photo.http) 작성
  • ScheduleServiceTest 파라미터 오타 수정
  • 개발 환경 기준 seed 데이터 정리

🧪 테스트 및 검증 (Test)

테스트하거나 검증한 항목을 체크해주세요.

  • 로컬 환경에서 정상 작동 확인
  • 유닛 테스트 통과
  • 주요 시나리오 수동 검증 완료
  • 테스트 코드 추가 / 수정 완료

결과 요약:

업로드 URL 생성 API 정상 동작
사진 등록/조회/이동/삭제 시나리오 정상 수행
권한 없는 사용자 요청 시 예외 정상 처리


✅ PR 체크리스트 (Checklist)

PR 제출 전 다음 항목을 확인하세요.

  • 커밋 메시지와 PR 제목이 명확함
  • 불필요한 주석 / 콘솔 로그 제거
  • 코드 포맷팅 완료 (Ctrl + Alt + L or Prettier)
  • 빌드 및 테스트 통과
  • 변경 사항이 팀 코딩 컨벤션을 준수함

💬 추가 참고사항 (Notes)

  • S3 업로드는 presigned URL 방식이며, 로컬 실행 시 AWS 자격 증명 설정이 필요합니다.

Summary by CodeRabbit

  • 새로운 기능

    • 앨범 관리 추가: 앨범 생성/조회/수정/삭제 UI·API 제공
    • 사진 관리 추가: 업로드 URL 생성, 사진 등록·조회·이동·삭제
    • AWS S3 통합: 파일 업로드·다운로드용 사전 서명 URL 지원
  • 문서화

    • API 문서(스웨거) 정리 및 개선
  • 테스트

    • 앨범·사진 서비스 단위 테스트 추가

✏️ Tip: You can customize this high-level summary in your review settings.

@coderabbitai
Copy link
Copy Markdown

coderabbitai bot commented Dec 25, 2025

Walkthrough

Photo 및 Album 도메인을 추가·확장하고 AWS S3 통합을 도입합니다. 컨트롤러·서비스·리포지토리·DTO·Swagger 문서와 테스트, S3 구성(S3Client/S3Presigner, S3Service, S3KeyGenerator) 및 DB 시드가 함께 추가/수정되었습니다.

Changes

Cohort / File(s) 변경 사항
빌드 / S3 의존성
build.gradle
AWS SDK BOM 및 software.amazon.awssdk:s3 추가; Spring Security 의존성 두 줄을 주석 처리(비활성화).
S3 인프라
src/main/java/.../global/config/S3Config.java, src/main/java/.../global/s3/S3Service.java, src/main/java/.../global/s3/S3KeyGenerator.java, src/main/resources/application-dev.yml
S3Client·S3Presigner 빈 등록, 업로드/다운로드 presigned URL 생성, 파일 삭제, 키 생성(확장자 검증) 및 dev용 aws.s3 설정 추가.
Album API 및 도메인
src/main/java/.../domain/album/controller/AlbumController.java, .../entity/Album.java, .../service/AlbumService.java, .../service/AlbumServiceImpl.java, .../repository/AlbumRepository.java, .../repository/AlbumRepositoryCustom.java, .../repository/AlbumRepositoryCustomImpl.java
Album CRUD 컨트롤러·서비스·리포지토리 및 QueryDSL 기반 커스텀 조회 추가. Album.photos 매핑에서 cascade/orphanRemoval 제거, updateTitle 메서드 추가.
Album DTO·Docs
src/main/java/.../domain/album/dto/*, src/main/java/.../domain/album/docs/AlbumSwaggerDocs.java, src/main/java/.../domain/album/docs/*Docs.java
요청/응답/쿼리 DTO 추가(toEntity/from 등), Swagger 상수 및 CRUD용 문서 어노테이션(재사용형) 추가.
Photo API 및 도메인
src/main/java/.../domain/photo/controller/PhotoController.java, .../entity/Photo.java, .../service/PhotoService.java, .../service/PhotoServiceImpl.java, .../repository/PhotoRepository.java
Photo 엔티티 재구성(Trip·User 연관, s3Key 등 메타데이터 추가, url/caption 제거), 컨트롤러·서비스에 업로드 URL 생성·생성/조회/이동/삭제 로직 구현, 리포지토리 조회 메서드 확장.
Photo DTO·Docs
src/main/java/.../domain/photo/dto/request/*, src/main/java/.../domain/photo/dto/response/*, src/main/java/.../domain/photo/docs/PhotoSwaggerDocs.java, src/main/java/.../domain/photo/docs/*Docs.java
UploadUrlRequestDto, PhotoRequestDto, MovePhotoRequestDto 등 요청 DTO와 UploadUrlResponseDto, PhotoResponseDto, PhotoDetailResponseDto 등 응답 DTO 추가 및 Swagger 문서 상수/어노테이션 추가.
권한 서비스 및 Trip 변경
src/main/java/.../domain/trip/service/TripPermissionService.java, src/main/java/.../domain/trip/service/TripService.java
TripPermissionService 신규 추가로 편집/조회 권한 중앙화; TripService에서 중복 메서드 제거(시그니처 정리).
Swagger 어노테이션 일괄 정리
src/main/java/.../domain/*/docs/*Docs.java (folder/schedule/trip/tripFolder 등)
기존 @ApiResponses 컨테이너를 개별 @ApiResponse로 변경(임포트 정리 포함)하여 문서 어노테이션 표현 방식 통일.
테스트·시드·HTTP 시나리오
src/test/java/.../album/service/AlbumServiceTest.java, src/test/java/.../photo/service/PhotoServiceTest.java, src/test/java/.../schedule/service/ScheduleServiceTest.java, src/test/http/album.http, src/test/http/photo.http, src/main/resources/db/migration/dev/V1__insert_seed_data.sql
Album/Photo 서비스 단위 테스트 추가, HTTP 예제 스크립트 추가, DB 시드 사진 데이터 스키마와 값 확장(파일 메타데이터·s3key 포함), ScheduleUpdateDto 호출 순서 변화에 따른 테스트 수정.
문서 삭제(정리)
여러 README.md 파일들 under album/photo/repository/service/controller/...
Album/Photo 관련 패키지 내 다수 README 파일 삭제(문서 정리).

Sequence Diagram(s)

sequenceDiagram
    actor User
    participant PC as PhotoController
    participant PS as PhotoServiceImpl
    participant TPS as TripPermissionService
    participant S3 as S3Service
    participant PR as PhotoRepository
    participant DB as Database

    User->>PC: POST /trips/{tripId}/photos/upload-urls (requestDtos)
    PC->>PS: generateUploadUrls(userId, tripId, requestDtos)
    PS->>TPS: getViewableTrip(userId, tripId)
    TPS->>DB: load User/Trip
    DB-->>TPS: entities
    TPS-->>PS: Trip
    PS->>S3: createUploadUrl(key)
    S3-->>PS: presignedUrl
    PS-->>PC: List<UploadUrlResponseDto>
    PC-->>User: 200 OK (upload urls)

    User->>PC: POST /trips/{tripId}/photos (PhotoRequestDto[])
    PC->>PS: createPhotos(userId, tripId, albumId, requestDtos)
    PS->>TPS: getEditableTrip(userId, tripId)
    TPS->>DB: load User/Trip
    DB-->>TPS: entities
    TPS-->>PS: Trip
    PS->>PR: save(Photo entity)
    PR->>DB: INSERT
    DB-->>PR: persisted
    PS->>S3: createDownloadUrl(s3Key)
    S3-->>PS: downloadUrl
    PS-->>PC: List<PhotoResponseDto>
    PC-->>User: 201 Created (photos)
Loading
sequenceDiagram
    actor User
    participant AC as AlbumController
    participant AS as AlbumServiceImpl
    participant TPS as TripPermissionService
    participant AR as AlbumRepository
    participant DB as Database

    User->>AC: POST /trips/{tripId}/albums (AlbumRequestDto)
    AC->>AS: createAlbum(userId, tripId, requestDto)
    AS->>TPS: getEditableTrip(userId, tripId)
    TPS->>DB: load User/Trip
    DB-->>TPS: entities
    TPS-->>AS: Trip
    AS->>AR: save(Album)
    AR->>DB: INSERT album
    DB-->>AR: Album
    AS-->>AC: AlbumResponseDto (photoCount=0)
    AC-->>User: 201 Created (album)
Loading

Estimated code review effort

🎯 4 (Complex) | ⏱️ ~45 minutes

Possibly related PRs

  • Feature/docs structure readme #2: 동일 영역(Album/Photo, S3 통합, 테스트) 대규모 변경을 포함—엔티티·서비스·API 구현이 겹침.
  • Feature/domain structure #6: Album/Photo 엔티티 초기 구조 관련 PR로, 본 PR의 엔티티 확장·수정과 직접적인 연관.
  • Feature/schedule #20: ScheduleUpdateDto 변경 및 스케줄 도메인 관련 수정사항이 본 PR의 테스트/DTO 변경과 연관됨.

Suggested labels

enhancement

Poem

🐇 새 앨범이 폴짝 폴짝 왔네,
S3 구름에 파일들 반짝이네.
업로드 URL 톡, 사진은 쏙,
여행 기억들 포근히 모아놨네 📸✨

Pre-merge checks and finishing touches

❌ Failed checks (1 warning)
Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. You can run @coderabbitai generate docstrings to improve docstring coverage.
✅ Passed checks (4 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed 제목 'Feature/photo s3'는 변경의 핵심인 Photo 도메인 및 S3 업로드 기능 구현을 명확하게 요약하고 있습니다.
Linked Issues check ✅ Passed PR의 모든 주요 구현 사항이 #44 요구사항과 일치합니다: Photo API 전체 구현, S3 통합, TripPermissionService 기반 권한 체크, Swagger 문서, 테스트 및 시드 데이터 업데이트가 모두 포함되어 있습니다.
Out of Scope Changes check ✅ Passed Folder, Trip, Schedule 도메인의 Swagger 주석 정리(ApiResponses를 개별 ApiResponse로 변경)는 기존 코드 품질 개선이며, ScheduleServiceTest의 매개변수 순서 수정은 DTO 변경에 따른 필수 수정입니다. 모두 PR 목표와 관련성이 있거나 필수적인 변경입니다.
✨ Finishing touches
  • 📝 Generate docstrings
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment
  • Commit unit tests in branch feature/photo-s3

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.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

@khg9900 khg9900 mentioned this pull request Dec 25, 2025
16 tasks
Copy link
Copy Markdown

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 13

🧹 Nitpick comments (14)
src/main/java/com/example/pventure/domain/schedule/docs/GetScheduleDocs.java (1)

15-16: 404 응답 설명에 일정 누락 케이스 추가 고려

API 응답 문서화가 전반적으로 적절하나, 단일 일정 조회 API의 404 응답 설명에 일정이 존재하지 않는 경우도 포함하는 것을 고려해보세요. UpdateScheduleDocs와 ReorderScheduleDocs는 "여행 또는 사용자 또는 일정 없음"을 사용하고 있습니다.

🔎 제안하는 개선안
-@ApiResponse(responseCode = "404", description = "여행 또는 사용자 없음")
+@ApiResponse(responseCode = "404", description = "여행 또는 사용자 또는 일정 없음")
src/main/resources/application-dev.yml (1)

29-33: 환경 변수 문서화 권장

AWS S3 설정이 올바르게 구현되었습니다. 환경 변수를 사용하여 민감한 정보를 외부화했으며, presigned URL의 만료 시간(10분)도 적절합니다.

다만 필수 환경 변수(S3_BUCKET, S3_REGION)를 README 또는 .env.example 파일에 문서화하여 개발자가 쉽게 설정할 수 있도록 하는 것을 권장합니다.

📝 환경 변수 문서화 예시

.env.example 파일을 추가하여 필수 환경 변수를 명시할 수 있습니다:

# AWS S3 Configuration
S3_BUCKET=your-bucket-name
S3_REGION=ap-northeast-2

# Database Configuration
DB_URL=jdbc:mysql://localhost:3306/pventure
DB_USERNAME=root
DB_PASSWORD=password
src/main/java/com/example/pventure/domain/photo/dto/request/PhotoRequestDto.java (1)

38-48: toEntity 메서드에 매개변수 검증 추가를 고려하세요.

toEntity 메서드가 trip, album, user 매개변수의 null 여부를 검증하지 않습니다. 서비스 레이어에서 검증이 이루어진다고 가정하지만, 방어적 코딩을 위해 Objects.requireNonNull이나 명시적 검증을 추가하면 더 안전합니다.

🔎 제안된 개선
+import java.util.Objects;
+
 public Photo toEntity(Trip trip, Album album, User user) {
+    Objects.requireNonNull(trip, "trip must not be null");
+    Objects.requireNonNull(user, "user must not be null");
+    // album은 nullable이므로 검증하지 않음
     return Photo.builder()
         .trip(trip)
         .album(album)
         .uploader(user)
         .s3Key(s3Key)
         .originalFileName(originalFileName)
         .contentType(contentType)
         .fileSize(fileSize)
         .build();
 }
src/main/java/com/example/pventure/global/config/S3Config.java (1)

32-39: S3Presigner 리소스 정리가 필요합니다.

S3PresignerSdkAutoCloseable을 구현하므로 애플리케이션 종료 시 적절히 close해야 합니다. Bean 정의에 destroyMethod를 추가하거나 별도로 정리 로직을 구현하는 것을 권장합니다.

🔎 제안된 수정
     @Bean
-    public S3Presigner s3Presigner(AwsCredentialsProvider provider) {
+    public S3Presigner s3Presigner(AwsCredentialsProvider provider) {
 
         return S3Presigner.builder()
             .region(Region.of(region))
             .credentialsProvider(provider)
             .build();
     }

S3ClientS3Presigner 모두 SdkAutoCloseable을 구현합니다. Spring이 자동으로 close()를 호출하도록 명시적으로 설정하거나, 수동 정리를 위해 @PreDestroy 메서드를 추가할 수 있습니다:

@Bean(destroyMethod = "close")
public S3Presigner s3Presigner(AwsCredentialsProvider provider) {
    // ...
}

@Bean(destroyMethod = "close")
public S3Client s3Client(AwsCredentialsProvider provider) {
    // ...
}
src/main/java/com/example/pventure/domain/photo/repository/PhotoRepository.java (2)

24-25: Spring Data JPA 파생 쿼리 메서드 사용을 고려하세요.

countByAlbum 메서드는 @Query 없이도 Spring Data JPA의 파생 쿼리 메서드로 동일한 결과를 얻을 수 있습니다.

🔎 제안된 수정
-    @Query("SELECT COUNT(*) FROM Photo p WHERE p.album = :album")
-    Long countByAlbum(@Param("album") Album album);
+    Long countByAlbum(Album album);

또는 현재 JPQL을 유지한다면, COUNT(*)보다 COUNT(p)가 JPA에서 더 일반적인 표현입니다:

@Query("SELECT COUNT(p) FROM Photo p WHERE p.album = :album")

18-20: 파라미터 명명 일관성을 고려하세요.

findAllByIdInfindAllByIdInAndTrip 메서드의 파라미터 이름이 id로 되어 있지만, 실제로는 여러 ID를 나타내므로 ids가 더 명확합니다.

🔎 제안된 수정
-    List<Photo> findAllByIdIn(List<Long> id);
+    List<Photo> findAllByIdIn(List<Long> ids);

-    List<Photo> findAllByIdInAndTrip(List<Long> id, Trip trip);
+    List<Photo> findAllByIdInAndTrip(List<Long> ids, Trip trip);
src/main/java/com/example/pventure/global/s3/S3Service.java (1)

28-41: 업로드 URL 생성 시 Content-Type 제한을 고려하세요.

현재 PutObjectRequest에 Content-Type 제한이 없어 클라이언트가 어떤 파일 유형이든 업로드할 수 있습니다. 사진 업로드 용도라면 이미지 MIME 타입으로 제한하는 것이 보안상 좋습니다.

🔎 Content-Type 제한 예시

업로드 URL 생성 시 Content-Type을 파라미터로 받아 제한할 수 있습니다:

public String createUploadUrl(String key, String contentType) {
    PutObjectRequest objectRequest = PutObjectRequest.builder()
        .bucket(bucket)
        .key(key)
        .contentType(contentType)
        .build();
    // ...
}

또는 서비스 레이어에서 허용된 MIME 타입을 검증하는 방식도 가능합니다.

src/test/java/com/example/pventure/domain/album/service/AlbumServiceTest.java (1)

70-74: 잠재적 위험: 리플렉션을 통한 ID 설정 시 예외 처리 부족

setId 메서드는 리플렉션을 사용하여 엔티티의 ID를 설정하지만, NoSuchFieldException 또는 IllegalAccessException 발생 시 단순히 throws로 전파됩니다. 상위 클래스에 "id" 필드가 없거나 접근 불가능한 경우 테스트가 실패할 수 있습니다.

🔎 더 명확한 에러 메시지를 위한 개선 제안
 private void setId(Object entity, Long id) throws Exception {
-    Field idField = entity.getClass().getSuperclass().getDeclaredField("id");
-    idField.setAccessible(true);
-    idField.set(entity, id);
+    try {
+        Field idField = entity.getClass().getSuperclass().getDeclaredField("id");
+        idField.setAccessible(true);
+        idField.set(entity, id);
+    } catch (NoSuchFieldException | IllegalAccessException e) {
+        throw new RuntimeException("Failed to set id for entity: " + entity.getClass().getSimpleName(), e);
+    }
 }
src/main/java/com/example/pventure/domain/photo/dto/request/UploadUrlRequestDto.java (2)

24-26: 검증 강화 권장: fileSize 범위 검증

fileSize 필드에 @NotNull만 적용되어 있어 음수나 0 값이 허용됩니다. 유효한 파일 크기 범위를 제한하는 것이 좋습니다.

🔎 개선 제안
+    import jakarta.validation.constraints.Min;
+    import jakarta.validation.constraints.Max;
+
     @Schema(description = "파일 크기(Byte)", example = "345678")
-    @NotNull(message = "파일 크기를 입력해주세요.")
+    @NotNull(message = "파일 크기를 입력해주세요.")
+    @Min(value = 1, message = "파일 크기는 1바이트 이상이어야 합니다.")
+    @Max(value = 104857600, message = "파일 크기는 100MB를 초과할 수 없습니다.") // 100MB 제한 예시
     private Long fileSize;

20-22: 검증 강화 권장: contentType 형식 검증

contentType 필드가 유효한 MIME 타입인지 확인하는 검증이 없습니다. 잘못된 형식이 입력될 수 있습니다.

🔎 개선 제안
+    import jakarta.validation.constraints.Pattern;
+
     @Schema(description = "파일 유형", example = "image/jpeg")
     @NotBlank(message = "파일 유형을 입력해주세요.")
+    @Pattern(regexp = "^image/(jpeg|jpg|png|gif|webp|bmp)$", 
+             message = "지원되는 이미지 형식이 아닙니다.")
     private String contentType;
src/main/java/com/example/pventure/domain/photo/service/PhotoServiceImpl.java (2)

60-86: 로더 메서드 일관성을 위해 TripPermissionService 사용 권장

createPhotos 메서드에서 사용자와 여행을 개별적으로 로드한 후 권한을 확인하는 반면, 다른 메서드들(generateUploadUrls, movePhotos, deletePhotos)은 tripPermissionService.getEditableTrip을 직접 사용합니다.

일관성을 위해 다음과 같이 리팩토링하는 것을 권장합니다:

🔎 제안하는 리팩토링
 @Override
 public List<PhotoResponseDto> createPhotos(Long userId, Long tripId, Long albumId, List<PhotoRequestDto> requestDtos) {

-    User user = loadUser(userId);
-    Trip trip = loadTrip(tripId);
-
-    tripPermissionService.checkEditableTrip(user, trip);
+    Trip trip = tripPermissionService.getEditableTrip(userId, tripId);
+    User user = loadUser(userId);

     Album album;

     if (albumId != null) {
         album = loadAlbum(albumId, trip);
     } else {
         album = null;
     }

     return requestDtos.stream()
         .map(requestDto -> {

             Photo photo = requestDto.toEntity(trip, album, user);
             Photo savedPhoto = photoRepository.save(photo);

             String downloadUrl = s3Service.createDownloadUrl(savedPhoto.getS3Key());

             return PhotoResponseDto.from(savedPhoto, downloadUrl);
         })
         .toList();
 }

177-179: S3 삭제 실패 시 DB 정합성 문제 발생 가능

S3에서 파일 삭제가 실패하면 예외가 발생하고 트랜잭션이 롤백되어 DB 레코드는 유지되지만, 일부 S3 파일은 이미 삭제되었을 수 있습니다. 이는 DB와 S3 간의 정합성 문제를 야기할 수 있습니다.

다음 중 하나를 고려하세요:

  1. DB를 먼저 삭제하고 S3 삭제 실패 시 로깅만 수행 (DB 우선 정합성)
  2. S3 삭제를 배치 작업으로 분리하여 재시도 로직 구현
  3. 보상 트랜잭션 패턴 적용
src/main/java/com/example/pventure/domain/album/service/AlbumServiceImpl.java (1)

72-83: 사진 수 조회 방식의 일관성 개선 권장

getAlbumsgetAlbumalbumRepository의 프로젝션 쿼리를 사용하여 사진 수를 가져오는 반면, updateAlbumphotoService.countPhotos를 사용합니다.

일관성을 위해 모든 메서드에서 동일한 방식을 사용하는 것을 권장합니다:

  • 옵션 1: albumRepository.findByIdAndTripWithPhotoCount를 사용 (선호)
  • 옵션 2: 모든 곳에서 photoService.countPhotos 사용
🔎 옵션 1 - 리포지토리 프로젝션 사용 (권장)
 @Override
 public AlbumResponseDto updateAlbum(Long userId, Long tripId, Long albumId, AlbumRequestDto requestDto) {

     Trip trip = tripPermissionService.getEditableTrip(userId, tripId);

     Album album = albumRepository.findByIdAndTrip(albumId, trip)
         .orElseThrow(() -> new ApiException(ErrorCode.NOT_FOUND_ALBUM));

     album.updateTitle(requestDto.getTitle());

-    return AlbumResponseDto.from(album, photoService.countPhotos(album));
+    AlbumWithPhotoCountQDto updatedAlbum = albumRepository.findByIdAndTripWithPhotoCount(
+        albumId, trip.getId());
+    return AlbumResponseDto.from(updatedAlbum);
 }
src/main/java/com/example/pventure/domain/photo/controller/PhotoController.java (1)

104-112: 대량 삭제 시 URL 길이 제한 고려

@RequestParam List<Long> photoIds를 사용하는 경우, 많은 수의 사진을 삭제할 때 URL 길이 제한에 도달할 수 있습니다.

RESTful 관점에서 DELETE 요청에 body를 사용하는 것은 논쟁의 여지가 있지만, 대량 삭제의 경우 실용적인 선택입니다. 다음을 고려하세요:

  1. 현재 구조 유지 (간단한 경우에 적합)
  2. Request body 사용으로 변경 (대량 작업 지원)

현재 사용 사례와 예상되는 사진 수에 따라 결정하시면 됩니다.

🔎 Request body를 사용하는 대안

DTO 생성:

public record DeletePhotosRequestDto(List<Long> photoIds) {}

컨트롤러 수정:

 @DeletePhotosDocs
 @DeleteMapping("/photos")
 public ResponseEntity<Void> deletePhotos(
     @RequestParam Long userId,
     @PathVariable Long tripId,
-    @RequestParam List<Long> photoIds
+    @Valid @RequestBody DeletePhotosRequestDto requestDto
 ) {
-    photoService.deletePhotos(userId, tripId, photoIds);
+    photoService.deletePhotos(userId, tripId, requestDto.photoIds());
     return CustomResponseHelper.noContent();
 }
📜 Review details

Configuration used: Repository UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between db61727 and 9dea065.

📒 Files selected for processing (75)
  • build.gradle
  • src/main/java/com/example/pventure/domain/album/controller/AlbumController.java
  • src/main/java/com/example/pventure/domain/album/controller/README.md
  • src/main/java/com/example/pventure/domain/album/docs/AlbumSwaggerDocs.java
  • src/main/java/com/example/pventure/domain/album/docs/CreateAlbumDocs.java
  • src/main/java/com/example/pventure/domain/album/docs/DeleteAlbumDocs.java
  • src/main/java/com/example/pventure/domain/album/docs/GetAlbumDocs.java
  • src/main/java/com/example/pventure/domain/album/docs/GetAlbumsDocs.java
  • src/main/java/com/example/pventure/domain/album/docs/UpdateAlbumDocs.java
  • src/main/java/com/example/pventure/domain/album/dto/query/AlbumWithPhotoCountQDto.java
  • src/main/java/com/example/pventure/domain/album/dto/request/AlbumRequestDto.java
  • src/main/java/com/example/pventure/domain/album/dto/request/README.md
  • src/main/java/com/example/pventure/domain/album/dto/response/AlbumResponseDto.java
  • src/main/java/com/example/pventure/domain/album/dto/response/README.md
  • src/main/java/com/example/pventure/domain/album/entity/Album.java
  • src/main/java/com/example/pventure/domain/album/repository/AlbumRepository.java
  • src/main/java/com/example/pventure/domain/album/repository/AlbumRepositoryCustom.java
  • src/main/java/com/example/pventure/domain/album/repository/AlbumRepositoryCustomImpl.java
  • src/main/java/com/example/pventure/domain/album/repository/README.md
  • src/main/java/com/example/pventure/domain/album/service/AlbumService.java
  • src/main/java/com/example/pventure/domain/album/service/AlbumServiceImpl.java
  • src/main/java/com/example/pventure/domain/album/service/README.md
  • src/main/java/com/example/pventure/domain/folder/docs/CreateFolderDocs.java
  • src/main/java/com/example/pventure/domain/folder/docs/DeleteFolderDocs.java
  • src/main/java/com/example/pventure/domain/folder/docs/GetFolderDocs.java
  • src/main/java/com/example/pventure/domain/folder/docs/GetFoldersDocs.java
  • src/main/java/com/example/pventure/domain/folder/docs/UpdateFolderDocs.java
  • src/main/java/com/example/pventure/domain/photo/controller/PhotoController.java
  • src/main/java/com/example/pventure/domain/photo/controller/README.md
  • src/main/java/com/example/pventure/domain/photo/docs/CreatePhotosDocs.java
  • src/main/java/com/example/pventure/domain/photo/docs/DeletePhotosDocs.java
  • src/main/java/com/example/pventure/domain/photo/docs/GenerateUploadUrlsDocs.java
  • src/main/java/com/example/pventure/domain/photo/docs/GetPhotoDocs.java
  • src/main/java/com/example/pventure/domain/photo/docs/GetPhotosByAlbumDocs.java
  • src/main/java/com/example/pventure/domain/photo/docs/GetUnassignedPhotosDocs.java
  • src/main/java/com/example/pventure/domain/photo/docs/MovePhotosDocs.java
  • src/main/java/com/example/pventure/domain/photo/docs/PhotoSwaggerDocs.java
  • src/main/java/com/example/pventure/domain/photo/dto/request/MovePhotoRequestDto.java
  • src/main/java/com/example/pventure/domain/photo/dto/request/PhotoRequestDto.java
  • src/main/java/com/example/pventure/domain/photo/dto/request/README.md
  • src/main/java/com/example/pventure/domain/photo/dto/request/UploadUrlRequestDto.java
  • src/main/java/com/example/pventure/domain/photo/dto/response/PhotoDetailResponseDto.java
  • src/main/java/com/example/pventure/domain/photo/dto/response/PhotoResponseDto.java
  • src/main/java/com/example/pventure/domain/photo/dto/response/README.md
  • src/main/java/com/example/pventure/domain/photo/dto/response/UploadUrlResponseDto.java
  • src/main/java/com/example/pventure/domain/photo/entity/Photo.java
  • src/main/java/com/example/pventure/domain/photo/repository/PhotoRepository.java
  • src/main/java/com/example/pventure/domain/photo/repository/README.md
  • src/main/java/com/example/pventure/domain/photo/service/PhotoService.java
  • src/main/java/com/example/pventure/domain/photo/service/PhotoServiceImpl.java
  • src/main/java/com/example/pventure/domain/photo/service/README.md
  • src/main/java/com/example/pventure/domain/schedule/docs/GetScheduleDocs.java
  • src/main/java/com/example/pventure/domain/schedule/docs/GetSchedulesDocs.java
  • src/main/java/com/example/pventure/domain/schedule/docs/ReorderScheduleDocs.java
  • src/main/java/com/example/pventure/domain/schedule/docs/UpdateScheduleDocs.java
  • src/main/java/com/example/pventure/domain/trip/docs/CreateTripDocs.java
  • src/main/java/com/example/pventure/domain/trip/docs/DeleteTripDocs.java
  • src/main/java/com/example/pventure/domain/trip/docs/GetTripDocs.java
  • src/main/java/com/example/pventure/domain/trip/docs/GetTripsDocs.java
  • src/main/java/com/example/pventure/domain/trip/docs/UpdateTripDocs.java
  • src/main/java/com/example/pventure/domain/trip/service/TripPermissionService.java
  • src/main/java/com/example/pventure/domain/trip/service/TripService.java
  • src/main/java/com/example/pventure/domain/tripFolder/docs/AddTripToFolderDocs.java
  • src/main/java/com/example/pventure/domain/tripFolder/docs/DeleteTripInFolderDocs.java
  • src/main/java/com/example/pventure/domain/tripFolder/docs/GetTripsInFolderDocs.java
  • src/main/java/com/example/pventure/global/config/S3Config.java
  • src/main/java/com/example/pventure/global/s3/S3KeyGenerator.java
  • src/main/java/com/example/pventure/global/s3/S3Service.java
  • src/main/resources/application-dev.yml
  • src/main/resources/db/migration/dev/V1__insert_seed_data.sql
  • src/test/http/album.http
  • src/test/http/photo.http
  • src/test/java/com/example/pventure/domain/album/service/AlbumServiceTest.java
  • src/test/java/com/example/pventure/domain/photo/service/PhotoServiceTest.java
  • src/test/java/com/example/pventure/domain/schedule/service/ScheduleServiceTest.java
💤 Files with no reviewable changes (11)
  • src/main/java/com/example/pventure/domain/album/repository/README.md
  • src/main/java/com/example/pventure/domain/photo/repository/README.md
  • src/main/java/com/example/pventure/domain/album/dto/response/README.md
  • src/main/java/com/example/pventure/domain/photo/dto/response/README.md
  • src/main/java/com/example/pventure/domain/photo/controller/README.md
  • src/main/java/com/example/pventure/domain/photo/service/README.md
  • src/main/java/com/example/pventure/domain/album/controller/README.md
  • src/main/java/com/example/pventure/domain/album/dto/request/README.md
  • src/main/java/com/example/pventure/domain/trip/service/TripService.java
  • src/main/java/com/example/pventure/domain/photo/dto/request/README.md
  • src/main/java/com/example/pventure/domain/album/service/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/AlbumRepositoryCustom.java
  • src/main/java/com/example/pventure/domain/album/repository/AlbumRepositoryCustomImpl.java
  • src/main/java/com/example/pventure/domain/trip/service/TripPermissionService.java
  • src/main/java/com/example/pventure/domain/photo/repository/PhotoRepository.java
  • src/main/java/com/example/pventure/domain/album/repository/AlbumRepository.java
  • src/main/java/com/example/pventure/domain/photo/entity/Photo.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/docs/UpdateTripDocs.java
  • src/main/java/com/example/pventure/domain/trip/docs/DeleteTripDocs.java
  • src/main/java/com/example/pventure/domain/trip/service/TripPermissionService.java
  • src/main/java/com/example/pventure/domain/trip/docs/CreateTripDocs.java
  • src/main/java/com/example/pventure/domain/tripFolder/docs/DeleteTripInFolderDocs.java
  • src/main/java/com/example/pventure/domain/photo/entity/Photo.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/AlbumRepositoryCustomImpl.java
  • src/main/java/com/example/pventure/domain/trip/service/TripPermissionService.java
  • src/main/java/com/example/pventure/domain/photo/repository/PhotoRepository.java
  • src/main/java/com/example/pventure/domain/album/repository/AlbumRepository.java
  • src/main/java/com/example/pventure/domain/tripFolder/docs/GetTripsInFolderDocs.java
  • src/main/java/com/example/pventure/domain/photo/entity/Photo.java
🧬 Code graph analysis (36)
src/main/java/com/example/pventure/domain/photo/docs/DeletePhotosDocs.java (2)
src/main/java/com/example/pventure/domain/schedule/docs/DeleteScheduleDocs.java (1)
  • Documented (8-17)
src/main/java/com/example/pventure/domain/schedule/docs/CreateScheduleDocs.java (1)
  • Documented (8-17)
src/main/java/com/example/pventure/domain/album/controller/AlbumController.java (3)
src/main/java/com/example/pventure/global/response/CustomResponseHelper.java (1)
  • CustomResponseHelper (7-31)
src/main/java/com/example/pventure/domain/folder/controller/FolderController.java (1)
  • Tag (18-66)
src/main/java/com/example/pventure/domain/trip/controller/TripController.java (3)
  • UpdateTripDocs (60-70)
  • GetTripDocs (49-58)
  • Tag (19-81)
src/main/java/com/example/pventure/domain/schedule/docs/ReorderScheduleDocs.java (3)
src/main/java/com/example/pventure/domain/schedule/docs/CreateScheduleDocs.java (1)
  • Documented (8-17)
src/main/java/com/example/pventure/domain/schedule/docs/DeleteScheduleDocs.java (1)
  • Documented (8-17)
src/main/java/com/example/pventure/domain/schedule/docs/ScheduleSwaggerDocs.java (1)
  • NoArgsConstructor (6-39)
src/main/java/com/example/pventure/domain/photo/dto/response/PhotoResponseDto.java (3)
src/main/java/com/example/pventure/domain/photo/dto/response/PhotoDetailResponseDto.java (1)
  • Getter (11-53)
src/main/java/com/example/pventure/domain/photo/dto/response/UploadUrlResponseDto.java (1)
  • Getter (9-30)
src/main/java/com/example/pventure/domain/member/dto/response/MemberSummaryDto.java (1)
  • Getter (9-31)
src/main/java/com/example/pventure/domain/photo/docs/GetPhotosByAlbumDocs.java (1)
src/main/java/com/example/pventure/domain/schedule/docs/CreateScheduleDocs.java (1)
  • Documented (8-17)
src/main/java/com/example/pventure/domain/album/docs/GetAlbumsDocs.java (1)
src/main/java/com/example/pventure/domain/schedule/docs/CreateScheduleDocs.java (1)
  • Documented (8-17)
src/main/java/com/example/pventure/domain/schedule/docs/UpdateScheduleDocs.java (3)
src/main/java/com/example/pventure/domain/schedule/docs/CreateScheduleDocs.java (1)
  • Documented (8-17)
src/main/java/com/example/pventure/domain/schedule/docs/DeleteScheduleDocs.java (1)
  • Documented (8-17)
src/main/java/com/example/pventure/domain/schedule/docs/ScheduleSwaggerDocs.java (1)
  • NoArgsConstructor (6-39)
src/main/java/com/example/pventure/domain/photo/docs/GenerateUploadUrlsDocs.java (1)
src/main/java/com/example/pventure/domain/schedule/docs/CreateScheduleDocs.java (1)
  • Documented (8-17)
src/main/java/com/example/pventure/domain/photo/docs/GetPhotoDocs.java (1)
src/main/java/com/example/pventure/domain/schedule/docs/CreateScheduleDocs.java (1)
  • Documented (8-17)
src/test/java/com/example/pventure/domain/album/service/AlbumServiceTest.java (2)
src/test/java/com/example/pventure/domain/trip/service/TripServiceTest.java (3)
  • ExtendWith (39-246)
  • Test (133-156)
  • Test (115-131)
src/test/java/com/example/pventure/domain/folder/service/FolderServiceTest.java (1)
  • ExtendWith (27-121)
src/main/java/com/example/pventure/domain/photo/service/PhotoService.java (2)
src/main/java/com/example/pventure/domain/trip/service/TripService.java (1)
  • TripService (10-30)
src/main/java/com/example/pventure/domain/folder/service/FolderService.java (1)
  • FolderService (11-24)
src/main/java/com/example/pventure/domain/schedule/docs/GetSchedulesDocs.java (2)
src/main/java/com/example/pventure/domain/schedule/docs/CreateScheduleDocs.java (1)
  • Documented (8-17)
src/main/java/com/example/pventure/domain/schedule/docs/DeleteScheduleDocs.java (1)
  • Documented (8-17)
src/main/java/com/example/pventure/domain/album/docs/CreateAlbumDocs.java (2)
src/main/java/com/example/pventure/domain/schedule/docs/CreateScheduleDocs.java (1)
  • Documented (8-17)
src/main/java/com/example/pventure/domain/schedule/docs/DeleteScheduleDocs.java (1)
  • Documented (8-17)
src/main/java/com/example/pventure/domain/photo/docs/CreatePhotosDocs.java (2)
src/main/java/com/example/pventure/domain/schedule/docs/CreateScheduleDocs.java (1)
  • Documented (8-17)
src/main/java/com/example/pventure/domain/schedule/docs/DeleteScheduleDocs.java (1)
  • Documented (8-17)
src/main/java/com/example/pventure/domain/trip/docs/DeleteTripDocs.java (1)
src/main/java/com/example/pventure/domain/schedule/docs/DeleteScheduleDocs.java (1)
  • Documented (8-17)
src/main/java/com/example/pventure/domain/trip/service/TripPermissionService.java (2)
src/main/java/com/example/pventure/domain/trip/util/TripFinder.java (1)
  • RequiredArgsConstructor (15-58)
src/main/java/com/example/pventure/domain/member/service/MemberService.java (1)
  • MemberService (10-25)
src/main/java/com/example/pventure/domain/folder/docs/UpdateFolderDocs.java (2)
src/main/java/com/example/pventure/domain/folder/docs/FolderSwaggerDocs.java (1)
  • FolderSwaggerDocs (3-26)
src/main/java/com/example/pventure/domain/folder/controller/FolderController.java (1)
  • UpdateFolderDocs (49-56)
src/main/java/com/example/pventure/domain/album/dto/request/AlbumRequestDto.java (3)
src/main/java/com/example/pventure/domain/album/dto/query/AlbumWithPhotoCountQDto.java (1)
  • Getter (9-18)
src/main/java/com/example/pventure/domain/album/dto/response/AlbumResponseDto.java (1)
  • Getter (11-42)
src/main/java/com/example/pventure/domain/album/docs/AlbumSwaggerDocs.java (1)
  • NoArgsConstructor (6-28)
src/main/java/com/example/pventure/domain/folder/docs/GetFolderDocs.java (2)
src/main/java/com/example/pventure/domain/folder/docs/FolderSwaggerDocs.java (1)
  • FolderSwaggerDocs (3-26)
src/main/java/com/example/pventure/domain/schedule/docs/DeleteScheduleDocs.java (1)
  • Documented (8-17)
src/main/java/com/example/pventure/domain/photo/dto/response/UploadUrlResponseDto.java (3)
src/main/java/com/example/pventure/domain/photo/dto/request/UploadUrlRequestDto.java (1)
  • Getter (10-28)
src/main/java/com/example/pventure/domain/photo/dto/response/PhotoDetailResponseDto.java (1)
  • Getter (11-53)
src/main/java/com/example/pventure/domain/photo/dto/response/PhotoResponseDto.java (1)
  • Getter (10-32)
src/main/java/com/example/pventure/domain/album/docs/DeleteAlbumDocs.java (2)
src/main/java/com/example/pventure/domain/schedule/docs/DeleteScheduleDocs.java (1)
  • Documented (8-17)
src/main/java/com/example/pventure/domain/schedule/docs/CreateScheduleDocs.java (1)
  • Documented (8-17)
src/main/java/com/example/pventure/domain/trip/docs/CreateTripDocs.java (2)
src/main/java/com/example/pventure/domain/schedule/docs/CreateScheduleDocs.java (1)
  • Documented (8-17)
src/main/java/com/example/pventure/domain/trip/docs/TripSwaggerDocs.java (1)
  • NoArgsConstructor (6-39)
src/main/java/com/example/pventure/domain/album/docs/UpdateAlbumDocs.java (2)
src/main/java/com/example/pventure/domain/schedule/docs/CreateScheduleDocs.java (1)
  • Documented (8-17)
src/main/java/com/example/pventure/domain/schedule/docs/DeleteScheduleDocs.java (1)
  • Documented (8-17)
src/main/java/com/example/pventure/domain/schedule/docs/GetScheduleDocs.java (3)
src/main/java/com/example/pventure/domain/schedule/docs/CreateScheduleDocs.java (1)
  • Documented (8-17)
src/main/java/com/example/pventure/domain/schedule/docs/DeleteScheduleDocs.java (1)
  • Documented (8-17)
src/main/java/com/example/pventure/domain/schedule/docs/ScheduleSwaggerDocs.java (1)
  • NoArgsConstructor (6-39)
src/main/java/com/example/pventure/domain/photo/repository/PhotoRepository.java (2)
src/main/java/com/example/pventure/domain/trip/repository/TripRepository.java (5)
  • TripRepository (12-72)
  • Query (30-37)
  • Query (55-62)
  • Query (39-45)
  • Query (64-71)
src/main/java/com/example/pventure/domain/trip/repository/TripRepositoryCustomImpl.java (1)
  • Repository (16-61)
src/main/java/com/example/pventure/domain/photo/docs/MovePhotosDocs.java (2)
src/main/java/com/example/pventure/domain/schedule/docs/CreateScheduleDocs.java (1)
  • Documented (8-17)
src/main/java/com/example/pventure/domain/schedule/docs/DeleteScheduleDocs.java (1)
  • Documented (8-17)
src/main/java/com/example/pventure/domain/tripFolder/docs/AddTripToFolderDocs.java (1)
src/main/java/com/example/pventure/domain/tripFolder/docs/TripFolderSwaggerDocs.java (1)
  • NoArgsConstructor (6-23)
src/main/java/com/example/pventure/domain/album/docs/GetAlbumDocs.java (1)
src/main/java/com/example/pventure/domain/schedule/docs/CreateScheduleDocs.java (1)
  • Documented (8-17)
src/main/java/com/example/pventure/domain/tripFolder/docs/DeleteTripInFolderDocs.java (2)
src/main/java/com/example/pventure/domain/tripFolder/controller/FolderTripController.java (1)
  • DeleteTripInFolderDocs (43-51)
src/main/java/com/example/pventure/domain/schedule/docs/DeleteScheduleDocs.java (1)
  • Documented (8-17)
src/main/java/com/example/pventure/domain/folder/docs/GetFoldersDocs.java (1)
src/main/java/com/example/pventure/domain/folder/docs/FolderSwaggerDocs.java (1)
  • FolderSwaggerDocs (3-26)
src/main/java/com/example/pventure/domain/folder/docs/DeleteFolderDocs.java (3)
src/main/java/com/example/pventure/domain/schedule/docs/DeleteScheduleDocs.java (1)
  • Documented (8-17)
src/main/java/com/example/pventure/domain/folder/docs/FolderSwaggerDocs.java (1)
  • FolderSwaggerDocs (3-26)
src/main/java/com/example/pventure/domain/folder/controller/FolderController.java (1)
  • DeleteFolderDocs (58-65)
src/main/java/com/example/pventure/domain/album/dto/query/AlbumWithPhotoCountQDto.java (1)
src/main/java/com/example/pventure/domain/album/dto/response/AlbumResponseDto.java (1)
  • Getter (11-42)
src/main/java/com/example/pventure/domain/album/service/AlbumServiceImpl.java (3)
src/main/java/com/example/pventure/domain/trip/util/TripFinder.java (1)
  • RequiredArgsConstructor (15-58)
src/main/java/com/example/pventure/domain/tripFolder/service/TripFolderServiceImpl.java (2)
  • Service (24-128)
  • trip (67-67)
src/main/java/com/example/pventure/domain/trip/service/TripServiceImpl.java (1)
  • Service (28-159)
src/main/java/com/example/pventure/domain/photo/dto/request/UploadUrlRequestDto.java (2)
src/main/java/com/example/pventure/domain/photo/dto/request/PhotoRequestDto.java (1)
  • Getter (15-49)
src/main/java/com/example/pventure/domain/photo/dto/response/UploadUrlResponseDto.java (1)
  • Getter (9-30)
src/main/java/com/example/pventure/domain/photo/dto/request/MovePhotoRequestDto.java (4)
src/main/java/com/example/pventure/domain/photo/dto/request/PhotoRequestDto.java (1)
  • Getter (15-49)
src/main/java/com/example/pventure/domain/photo/dto/request/UploadUrlRequestDto.java (1)
  • Getter (10-28)
src/main/java/com/example/pventure/domain/photo/dto/response/PhotoResponseDto.java (1)
  • Getter (10-32)
src/main/java/com/example/pventure/domain/photo/dto/response/UploadUrlResponseDto.java (1)
  • Getter (9-30)
src/main/java/com/example/pventure/domain/photo/controller/PhotoController.java (1)
src/main/java/com/example/pventure/global/response/CustomResponseHelper.java (1)
  • CustomResponseHelper (7-31)
🔇 Additional comments (60)
src/main/java/com/example/pventure/domain/folder/docs/GetFoldersDocs.java (1)

15-16: LGTM!

@ApiResponses 래퍼를 개별 @ApiResponse 어노테이션으로 변경하여 코드가 더 간결해졌습니다. 이 패턴은 DeleteScheduleDocs.java 등 다른 도메인에서 이미 사용 중인 방식과 일치합니다.

src/main/java/com/example/pventure/domain/folder/docs/DeleteFolderDocs.java (1)

15-16: LGTM!

DELETE 작업에 적합한 응답 코드(204, 404)를 사용하고 있으며, DeleteScheduleDocs.java와 동일한 패턴으로 일관성 있게 구현되었습니다.

src/main/java/com/example/pventure/domain/folder/docs/CreateFolderDocs.java (1)

15-18: LGTM!

생성 작업에 적합한 응답 코드들(201 생성 성공, 400 잘못된 요청, 409 기본 폴더 중복)이 잘 정의되어 있습니다. useReturnTypeSchema = true 설정으로 응답 스키마가 자동 생성됩니다.

src/main/java/com/example/pventure/domain/folder/docs/UpdateFolderDocs.java (1)

15-16: LGTM!

수정 작업에 적합한 응답 코드(200, 404)가 정의되어 있으며, 다른 폴더 문서 어노테이션들과 일관된 패턴으로 구현되었습니다.

src/main/java/com/example/pventure/domain/folder/docs/GetFolderDocs.java (1)

15-16: LGTM!

단일 리소스 조회에 적합한 응답 코드(200, 404)가 정의되어 있습니다. 모든 폴더 도메인 Swagger 문서 어노테이션이 일관된 패턴으로 통일되었습니다.

src/main/java/com/example/pventure/domain/trip/docs/UpdateTripDocs.java (1)

16-18: LGTM! 개별 @ApiResponse 어노테이션으로 개선되었습니다.

@ApiResponses 래퍼를 개별 @ApiResponse 어노테이션으로 교체하여 코드가 더 명확해졌습니다. 응답 코드(200, 400, 404)와 설명이 UPDATE 작업에 적합하며, Schedule 도메인의 문서화 스타일과 일관성을 유지합니다.

src/main/java/com/example/pventure/domain/trip/docs/DeleteTripDocs.java (1)

15-16: LGTM! DELETE 작업에 적합한 응답 코드입니다.

개별 @ApiResponse 어노테이션으로 리팩터링이 잘 되었습니다. DELETE 작업에 204(성공)와 404(리소스 없음) 응답 코드가 적절하며, DeleteScheduleDocs와 동일한 패턴을 따릅니다.

src/main/java/com/example/pventure/domain/trip/docs/CreateTripDocs.java (1)

16-17: LGTM! CREATE 작업에 적합한 응답 코드입니다.

개별 @ApiResponse 어노테이션으로 개선되었습니다. 201(생성 성공)과 400(잘못된 요청)은 POST 작업에 적절한 응답 코드이며, useReturnTypeSchema 사용이 올바릅니다.

src/main/java/com/example/pventure/domain/trip/docs/GetTripsDocs.java (1)

16-17: LGTM! 목록 조회 작업에 적합한 응답 코드입니다.

@ApiResponses 래퍼를 개별 @ApiResponse 어노테이션으로 교체하여 가독성이 향상되었습니다. 200(조회 성공)과 400(잘못된 요청)은 목록 조회 API에 적절합니다.

src/main/java/com/example/pventure/domain/trip/docs/GetTripDocs.java (1)

16-17: LGTM! 단일 조회 작업에 적합한 응답 코드입니다.

개별 @ApiResponse 어노테이션으로 리팩터링이 잘 되었습니다. 200(조회 성공)과 404(여행 없음)은 ID 기반 조회 API의 표준 응답 코드이며, 프로젝트 전반의 문서화 스타일과 일관성을 유지합니다.

src/main/java/com/example/pventure/domain/schedule/docs/UpdateScheduleDocs.java (2)

5-5: LGTM!

ApiResponse 임포트가 올바르게 추가되었습니다.


15-16: LGTM!

API 응답 문서화가 적절합니다. 200과 404 응답 코드 및 설명이 일정 수정 API에 적합합니다.

src/main/java/com/example/pventure/domain/schedule/docs/ReorderScheduleDocs.java (2)

5-5: LGTM!

ApiResponse 임포트가 올바르게 추가되었습니다.


15-16: LGTM!

API 응답 문서화가 적절합니다. 일정 재정렬 API에 대한 200과 404 응답이 올바르게 정의되었습니다.

src/main/java/com/example/pventure/domain/schedule/docs/GetScheduleDocs.java (1)

5-5: LGTM!

ApiResponse 임포트가 올바르게 추가되었습니다.

src/main/java/com/example/pventure/domain/schedule/docs/GetSchedulesDocs.java (2)

5-5: LGTM!

ApiResponse 임포트가 올바르게 추가되었습니다.


15-16: LGTM!

API 응답 문서화가 적절합니다. 일정 목록 조회 API에서 404는 여행이나 사용자가 없을 때 발생하므로, 현재 설명이 정확합니다.

src/test/java/com/example/pventure/domain/schedule/service/ScheduleServiceTest.java (1)

124-126: ScheduleUpdateDto 생성자 및 접근자 메서드 변경이 올바르게 반영되었습니다.

DTO 생성자 호출(ScheduleUpdateDto("new memo", true))이 현재 DTO 정의(String memo, Boolean isCompleted)와 일치하며, 이 테스트 파일의 모든 사용처(124줄, 189줄)에 일관되게 적용되었습니다. 또한 response.getIsCompleted() 호출도 Lombok 생성 접근자와 일치합니다.

코드베이스 전체에서 ScheduleUpdateDto는 이 테스트 파일에서만 사용되므로 추가 검증은 필요하지 않습니다.

src/main/java/com/example/pventure/domain/tripFolder/docs/AddTripToFolderDocs.java (1)

15-16: Swagger 문서 리팩토링 적용 확인

@ApiResponses 래퍼를 개별 @ApiResponse 어노테이션으로 변경한 것은 코드를 더 간결하고 읽기 쉽게 만듭니다. 응답 코드(200, 404)와 설명이 폴더에 여행을 추가하는 작업에 적합합니다.

src/main/java/com/example/pventure/domain/tripFolder/docs/DeleteTripInFolderDocs.java (1)

15-16: DELETE 작업에 적합한 응답 코드 사용

개별 @ApiResponse 어노테이션으로의 리팩토링이 올바르게 적용되었습니다. 204(삭제 성공)는 DELETE 작업의 표준 응답 코드로 적절하며, 404 설명도 누락될 수 있는 모든 엔티티를 포함하고 있습니다.

src/main/java/com/example/pventure/domain/tripFolder/docs/GetTripsInFolderDocs.java (1)

15-16: [Rewritten review comment]
[Classification tag]

src/main/java/com/example/pventure/domain/album/repository/AlbumRepositoryCustom.java (1)

6-11: LGTM!

Custom repository 인터페이스 구조가 올바르게 작성되었습니다. QueryDSL 기반 projection을 위한 명확한 메서드 시그니처를 제공하고 있으며, AlbumWithPhotoCountQDto를 사용한 DTO projection 패턴이 적절합니다.

src/main/java/com/example/pventure/domain/photo/dto/response/PhotoResponseDto.java (1)

10-32: LGTM!

사진 목록 조회용 DTO가 올바르게 구현되었습니다. PhotoDetailResponseDto와 일관된 구조를 유지하면서 목록 조회에 필요한 최소한의 필드(id, downloadUrl)만 포함하여 응답 크기를 최적화했습니다. Factory 메서드 패턴도 적절하게 적용되었습니다.

src/main/java/com/example/pventure/domain/album/repository/AlbumRepository.java (1)

8-10: LGTM!

Repository 확장이 적절하게 구현되었습니다. AlbumRepositoryCustom 인터페이스 확장과 findByIdAndTrip 메서드 추가로 앨범-여행 복합 조회 기능을 제공하며, Spring Data JPA의 query derivation 규칙을 올바르게 따르고 있습니다.

src/main/java/com/example/pventure/domain/photo/docs/DeletePhotosDocs.java (1)

11-21: LGTM!

Swagger 문서화 annotation이 올바르게 구현되었습니다. 기존 패턴(DeleteScheduleDocs 등)과 일관성을 유지하며, 사진 삭제 작업의 다양한 응답 시나리오(204 성공, 403 권한 없음, 404 리소스 없음)를 적절히 문서화하고 있습니다.

src/test/http/album.http (1)

1-24: LGTM!

앨범 CRUD 작업을 위한 HTTP 테스트 시나리오가 잘 구성되었습니다. 생성, 조회, 수정, 삭제의 전체 흐름을 커버하며, 일관된 형식과 파라미터(userId, tripId, albumId)를 사용하여 수동 테스트 및 API 검증에 유용합니다.

src/main/java/com/example/pventure/domain/photo/docs/GetPhotoDocs.java (1)

11-21: LGTM!

사진 조회 API를 위한 Swagger 문서화 annotation이 올바르게 구현되었습니다. PhotoSwaggerDocs의 상수를 참조하여 일관된 문서를 제공하며, GET 작업에 적합한 응답 코드(200 성공, 403 권한 없음, 404 리소스 없음)를 명시하고 있습니다. 다른 photo docs annotation들과 구조적 일관성도 유지하고 있습니다.

src/main/java/com/example/pventure/domain/album/entity/Album.java (2)

30-31: 변경 사항 승인

앨범 제목 수정을 위한 updateTitle 메서드가 적절하게 추가되었습니다.


27-27: 앨범 삭제 로직이 연결된 사진을 명시적으로 처리합니다. AlbumServiceImpl.deleteAlbum()에서 모든 연결된 사진에 대해 removeAlbum()을 호출하여 앨범 참조를 제거하므로 고아 Photo 레코드는 발생하지 않습니다.

src/main/java/com/example/pventure/domain/album/dto/request/AlbumRequestDto.java (1)

1-29: 구현 승인

AlbumRequestDto가 표준 패턴을 따라 적절하게 구현되었습니다:

  • Lombok을 활용한 보일러플레이트 코드 제거
  • 제목 필드에 대한 적절한 유효성 검증
  • Swagger 문서화 메타데이터 포함
  • toEntity 메서드를 통한 명확한 엔티티 변환
src/main/java/com/example/pventure/domain/album/docs/GetAlbumDocs.java (1)

1-21: 문서화 어노테이션 구현 승인

GetAlbumDocs가 기존 문서화 패턴(예: CreateScheduleDocs)을 일관되게 따르고 있습니다. OpenAPI 메타데이터가 적절하게 구성되었으며, 응답 코드도 GET 작업에 적합합니다.

src/main/java/com/example/pventure/domain/album/docs/DeleteAlbumDocs.java (1)

1-21: 문서화 어노테이션 구현 승인

DeleteAlbumDocs가 기존 패턴을 따르고 있으며, DELETE 작업에 적합한 응답 코드(204 성공, 403 권한 없음, 404 없음)를 정의하고 있습니다.

src/main/java/com/example/pventure/domain/album/docs/UpdateAlbumDocs.java (1)

1-22: 문서화 어노테이션 구현 승인

UpdateAlbumDocs가 적절하게 구현되었습니다. UPDATE 작업에 필요한 모든 응답 코드(200 성공, 400 잘못된 요청, 403 권한 없음, 404 없음)를 포함하고 있습니다.

src/main/java/com/example/pventure/domain/photo/docs/GenerateUploadUrlsDocs.java (1)

1-22: 문서화 어노테이션 구현 승인

GenerateUploadUrlsDocs가 프로젝트의 문서화 패턴을 일관되게 따르고 있으며, 업로드 URL 생성 엔드포인트에 적합한 응답 코드를 정의하고 있습니다.

src/main/java/com/example/pventure/domain/album/docs/GetAlbumsDocs.java (1)

1-21: 문서화 어노테이션 구현 승인

GetAlbumsDocs가 앨범 목록 조회를 위한 문서화를 적절하게 제공하고 있으며, 기존 패턴과 일관성을 유지하고 있습니다.

src/main/java/com/example/pventure/domain/photo/docs/GetPhotosByAlbumDocs.java (1)

11-21: LGTM!

Swagger 문서화 어노테이션이 프로젝트의 기존 패턴과 일관성 있게 구현되어 있습니다. 응답 코드와 설명도 적절합니다.

src/main/java/com/example/pventure/domain/photo/dto/response/UploadUrlResponseDto.java (1)

9-30: LGTM!

DTO 구조가 프로젝트의 다른 response DTO들과 일관성 있게 구현되어 있으며, Swagger 문서화도 적절합니다.

src/main/java/com/example/pventure/domain/photo/docs/MovePhotosDocs.java (1)

11-22: LGTM!

Swagger 문서화 어노테이션이 기존 패턴을 따르며, 사진 이동 작업에 필요한 모든 응답 코드(200, 400, 403, 404)가 적절하게 문서화되어 있습니다.

src/main/java/com/example/pventure/domain/photo/dto/request/MovePhotoRequestDto.java (1)

10-22: LGTM!

DTO 구조가 명확하며, photoIds에 대한 @NotEmpty 검증과 앨범 미지정 기능을 위한 targetAlbumId nullable 설정이 적절합니다.

src/main/java/com/example/pventure/global/config/S3Config.java (1)

18-21: LGTM!

DefaultCredentialsProvider 사용은 로컬 개발(~/.aws/credentials)과 프로덕션 환경(IAM Role, 환경 변수 등) 모두에서 유연하게 동작하는 적절한 선택입니다.

src/main/java/com/example/pventure/domain/photo/docs/CreatePhotosDocs.java (1)

11-22: LGTM!

Swagger 문서화 어노테이션이 기존 프로젝트 패턴(예: CreateScheduleDocs, DeleteScheduleDocs)과 일관성 있게 구현되었습니다. 권한 검증을 위한 403 응답 코드가 적절히 포함되어 있습니다.

src/main/java/com/example/pventure/domain/album/dto/response/AlbumResponseDto.java (1)

27-41: LGTM!

정적 팩토리 메서드 패턴을 사용하여 Album 엔티티와 AlbumWithPhotoCountQDto로부터 DTO를 생성하는 구조가 깔끔합니다. 두 from 메서드가 오버로딩되어 다양한 소스에서 일관된 방식으로 객체를 생성할 수 있습니다.

src/main/java/com/example/pventure/domain/album/docs/CreateAlbumDocs.java (1)

7-18: LGTM!

앨범 생성 API에 대한 Swagger 문서화 어노테이션이 프로젝트의 기존 패턴과 일관되게 구현되었습니다.

src/main/java/com/example/pventure/domain/photo/repository/PhotoRepository.java (1)

14-16: LGTM!

앨범별 사진 조회와 미분류 사진(album이 null인) 조회 메서드가 적절하게 구현되었습니다.

src/main/java/com/example/pventure/domain/album/repository/AlbumRepositoryCustomImpl.java (2)

42-57: LGTM!

QueryDSL을 사용한 사진 개수 포함 앨범 조회 구현이 잘 되어 있습니다. queryAlbumWithPhotoCount 헬퍼 메서드를 통한 재사용 가능한 쿼리 구조가 좋습니다. leftJoin을 사용하여 사진이 없는 앨범도 0으로 카운트되도록 처리되었습니다.


29-40: fetchOne() 사용 시 고려사항

fetchOne()은 결과가 2개 이상일 경우 NonUniqueResultException을 발생시킵니다. 현재 로직에서는 album.id로 필터링하고 그룹핑하므로 항상 고유한 결과가 보장되지만, 결과가 없을 경우 null을 반환합니다. 호출하는 서비스 레이어에서 null 처리가 적절히 되어 있는지 확인해주세요.

src/main/java/com/example/pventure/domain/album/service/AlbumService.java (1)

7-19: LGTM!

앨범 서비스 인터페이스가 깔끔하게 정의되었습니다. 모든 메서드에 userIdtripId가 포함되어 권한 검증 컨텍스트가 일관되게 전달됩니다.

src/main/java/com/example/pventure/global/s3/S3Service.java (2)

58-64: S3 삭제 작업에 대한 에러 처리를 고려하세요.

deleteFile 메서드에서 S3 삭제 작업 실패 시 SdkException이 발생할 수 있습니다. 현재는 예외가 그대로 전파되는데, 호출하는 서비스에서 적절히 처리되고 있는지 확인이 필요합니다. 또한 삭제 작업에 대한 로깅을 추가하면 운영 시 디버깅에 도움이 됩니다.


43-56: LGTM!

다운로드 URL 생성 로직이 올바르게 구현되었습니다. GetObjectPresignRequest를 사용한 presigned URL 생성이 적절합니다.

src/test/java/com/example/pventure/domain/album/service/AlbumServiceTest.java (1)

297-314: 테스트 로직 검증 완료

앨범 삭제 시 관련 사진들의 앨범 참조를 제거한 후 앨범을 삭제하는 로직이 올바르게 테스트되었습니다. Mock Photo 객체의 removeAlbum() 호출과 albumRepository.delete() 순서가 정확하게 검증되었습니다.

src/main/java/com/example/pventure/domain/photo/service/PhotoService.java (1)

13-27: 인터페이스 설계 승인

위 시그니처 이슈를 제외하면, 인터페이스가 잘 설계되었습니다:

  • 명확한 메서드 명명
  • 일관된 파라미터 순서 (userId, tripId 우선)
  • 적절한 반환 타입 사용
src/main/java/com/example/pventure/domain/trip/service/TripPermissionService.java (1)

23-43: 권한 체크 로직 설계 승인

getEditableTripgetViewableTrip이 명확하게 분리되어 있고, 적절한 예외 처리가 구현되어 있습니다. 위의 fetch join 이슈만 해결하면 안전한 구현입니다.

src/main/java/com/example/pventure/domain/album/docs/AlbumSwaggerDocs.java (1)

6-28: 문서화 상수 클래스 구현 승인

Swagger 문서화를 위한 상수들이 깔끔하게 정리되어 있습니다:

  • 유틸리티 클래스 패턴 올바르게 적용 (private 생성자)
  • 명확하고 일관된 네이밍 컨벤션
  • 한글 설명이 명확함
src/test/java/com/example/pventure/domain/photo/service/PhotoServiceTest.java (1)

105-465: 포괄적인 테스트 커버리지 확인

PhotoServiceImpl에 대한 테스트가 잘 작성되었습니다:

  • 성공 및 실패 시나리오 모두 커버
  • 권한 체크 검증
  • Repository 및 S3Service 상호작용 확인
  • 에러 코드 검증 포함
  • Mock 검증이 철저함

위의 DisplayName 오타만 수정하면 훌륭한 테스트입니다.

src/main/java/com/example/pventure/domain/photo/docs/GetUnassignedPhotosDocs.java (1)

18-20: API 응답 문서가 정확합니다.

검증 결과, 선언된 API 응답 코드가 실제 구현과 완벽하게 일치합니다:

  • 403: "보기 권한 없음" - UNAUTHORIZED_MEMBER_ACCESS 에러 (사용자가 여행 멤버가 아님)
  • 404: "사용자 또는 여행 없음" - NOT_FOUND_USER 또는 NOT_FOUND_TRIP 에러

문서는 정확하므로 수정이 필요하지 않습니다.

src/main/java/com/example/pventure/domain/album/service/AlbumServiceImpl.java (1)

93-97: 앨범 삭제 시 사진 처리 로직 확인

앨범 삭제 시 연결된 사진들의 앨범 참조를 null로 설정하여 미분류 상태로 전환하는 것은 적절한 접근입니다. 사진은 삭제되지 않고 보존됩니다.

src/main/java/com/example/pventure/domain/album/controller/AlbumController.java (1)

1-89: REST 컨트롤러 구현이 올바릅니다

앨범 CRUD 엔드포인트가 프로젝트의 기존 패턴(TripController, FolderController)을 잘 따르고 있습니다:

  • 일관된 응답 래핑 (CustomResponseHelper)
  • 적절한 유효성 검증 (@Valid)
  • 올바른 HTTP 상태 코드 (201 Created, 200 OK, 204 No Content)
  • Swagger 문서화 통합
src/main/java/com/example/pventure/domain/photo/docs/PhotoSwaggerDocs.java (1)

1-40: Swagger 문서화 상수 클래스가 적절합니다

문서화 텍스트를 중앙화하는 접근 방식이 좋으며, private 생성자로 인스턴스화를 방지하는 것도 올바릅니다.

src/main/java/com/example/pventure/domain/photo/entity/Photo.java (1)

17-47: 엔티티 구조 변경이 요구사항을 잘 지원합니다

Photo 엔티티의 변경사항:

  • Trip 관계 추가로 사진이 여행에 명확히 속함
  • Album을 nullable로 변경하여 미분류 사진 지원
  • User uploader 추가로 업로드 추적 가능
  • S3 메타데이터 필드 추가로 파일 관리 개선
  • updateAlbum/removeAlbum 메서드로 앨범 관리 간소화

모든 변경사항이 PR 목표와 일치하며 잘 설계되었습니다.

src/main/java/com/example/pventure/domain/photo/controller/PhotoController.java (1)

42-50: 업로드 URL 생성 엔드포인트 설계가 적절합니다

Presigned URL 생성은 DB에 영구 리소스를 생성하지 않으므로 200 OK를 반환하는 것이 201 Created보다 의미론적으로 더 적절합니다.

Copy link
Copy Markdown

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 1

🧹 Nitpick comments (3)
src/main/java/com/example/pventure/domain/photo/dto/request/UploadUrlRequestDto.java (2)

22-25: 비표준 MIME 타입 image/jpg 포함 확인 필요

contentType 패턴에 image/jpg가 포함되어 있지만, 표준 MIME 타입은 image/jpeg입니다. image/jpg는 비공식적으로 사용되기도 하지만, 표준을 따르려면 제거하거나 클라이언트에서 image/jpeg로 정규화하는 것이 좋습니다.

🔎 표준 MIME 타입만 허용하도록 수정
-    @Pattern(regexp = "^image/(jpeg|jpg|png|webp)$", message = "지원되는 이미지 형식이 아닙니다.")
+    @Pattern(regexp = "^image/(jpeg|png|webp)$", message = "지원되는 이미지 형식이 아닙니다.")

27-30: 대용량 파일 업로드 방지를 위한 최대 크기 제한 고려

fileSize@Min(1) 검증만 있고 최대 크기 제한이 없습니다. S3 presigned URL 생성 전에 과도하게 큰 파일 요청을 차단하려면 @Max 제약을 추가하는 것을 권장합니다.

🔎 최대 파일 크기 제한 추가 예시
     @Schema(description = "파일 크기(Byte)", example = "345678")
     @NotNull(message = "파일 크기를 입력해주세요.")
     @Min(value = 1, message = "파일 크기는 1바이트 이상이어야 합니다.")
+    @Max(value = 10_485_760, message = "파일 크기는 10MB를 초과할 수 없습니다.")
     private Long fileSize;
src/main/java/com/example/pventure/domain/photo/service/PhotoServiceImpl.java (1)

77-88: 사진 일괄 저장 시 배치 처리 권장

현재 stream().map()으로 각 사진을 개별 저장하고 있습니다. saveAll()을 사용하면 배치 insert가 가능하여 성능이 향상될 수 있습니다.

🔎 배치 저장으로 리팩토링
-        return requestDtos.stream()
-            .map(requestDto -> {
-
-                Photo photo = requestDto.toEntity(trip, album, user);
-                Photo savedPhoto = photoRepository.save(photo);
-
-                String downloadUrl = s3Service.createDownloadUrl(savedPhoto.getS3Key());
-
-                return PhotoResponseDto.from(savedPhoto, downloadUrl);
-            })
-            .toList();
+        List<Photo> photos = requestDtos.stream()
+            .map(requestDto -> requestDto.toEntity(trip, album, user))
+            .toList();
+
+        List<Photo> savedPhotos = photoRepository.saveAll(photos);
+
+        return savedPhotos.stream()
+            .map(photo -> {
+                String downloadUrl = s3Service.createDownloadUrl(photo.getS3Key());
+                return PhotoResponseDto.from(photo, downloadUrl);
+            })
+            .toList();
📜 Review details

Configuration used: Repository UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 9dea065 and b3a2d44.

📒 Files selected for processing (13)
  • build.gradle
  • src/main/java/com/example/pventure/domain/album/dto/query/AlbumWithPhotoCountQDto.java
  • src/main/java/com/example/pventure/domain/album/repository/AlbumRepositoryCustomImpl.java
  • src/main/java/com/example/pventure/domain/photo/dto/request/PhotoRequestDto.java
  • src/main/java/com/example/pventure/domain/photo/dto/request/UploadUrlRequestDto.java
  • src/main/java/com/example/pventure/domain/photo/dto/response/PhotoDetailResponseDto.java
  • src/main/java/com/example/pventure/domain/photo/repository/PhotoRepository.java
  • src/main/java/com/example/pventure/domain/photo/service/PhotoService.java
  • src/main/java/com/example/pventure/domain/photo/service/PhotoServiceImpl.java
  • src/main/java/com/example/pventure/global/s3/S3KeyGenerator.java
  • src/main/resources/db/migration/dev/V1__insert_seed_data.sql
  • src/test/http/photo.http
  • src/test/java/com/example/pventure/domain/photo/service/PhotoServiceTest.java
🚧 Files skipped from review as they are similar to previous changes (7)
  • src/test/http/photo.http
  • src/main/java/com/example/pventure/domain/photo/repository/PhotoRepository.java
  • src/main/java/com/example/pventure/domain/photo/service/PhotoService.java
  • src/main/java/com/example/pventure/domain/photo/dto/response/PhotoDetailResponseDto.java
  • src/main/java/com/example/pventure/domain/album/dto/query/AlbumWithPhotoCountQDto.java
  • src/test/java/com/example/pventure/domain/photo/service/PhotoServiceTest.java
  • src/main/java/com/example/pventure/domain/photo/dto/request/PhotoRequestDto.java
🧰 Additional context used
🧠 Learnings (4)
📚 Learning: 2025-12-25T05:23:14.671Z
Learnt from: khg9900
Repo: khg9900/Pventure_BE PR: 46
File: build.gradle:32-33
Timestamp: 2025-12-25T05:23:14.671Z
Learning: Spring Security 스타터는 현재 주석 처리되어 있지만, Auth 도메인 구현을 위해 곧 사용될 예정이므로 spring-security-test 테스트 의존성은 유지됩니다.

Applied to files:

  • build.gradle
📚 Learning: 2025-12-25T04:23:02.024Z
Learnt from: khg9900
Repo: khg9900/Pventure_BE PR: 45
File: src/main/java/com/example/pventure/domain/album/controller/AlbumController.java:42-42
Timestamp: 2025-12-25T04:23:02.024Z
Learning: Album API의 userId는 현재 RequestParam으로 전달되고 있으며, Auth 도메인 구현 후 Spring Security의 인증 컨텍스트에서 추출하도록 수정될 예정입니다.

Applied to files:

  • build.gradle
📚 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/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/AlbumRepositoryCustomImpl.java
🧬 Code graph analysis (2)
src/main/java/com/example/pventure/domain/photo/service/PhotoServiceImpl.java (1)
src/main/java/com/example/pventure/domain/trip/util/TripFinder.java (1)
  • RequiredArgsConstructor (15-58)
src/main/java/com/example/pventure/domain/photo/dto/request/UploadUrlRequestDto.java (6)
src/main/java/com/example/pventure/domain/photo/dto/request/PhotoRequestDto.java (1)
  • Getter (16-51)
src/main/java/com/example/pventure/domain/photo/dto/response/PhotoDetailResponseDto.java (1)
  • Getter (13-57)
src/main/java/com/example/pventure/domain/photo/dto/request/MovePhotoRequestDto.java (1)
  • Getter (10-22)
src/main/java/com/example/pventure/domain/photo/dto/response/PhotoResponseDto.java (1)
  • Getter (10-32)
src/main/java/com/example/pventure/domain/photo/dto/response/UploadUrlResponseDto.java (1)
  • Getter (9-30)
src/main/java/com/example/pventure/domain/photo/docs/PhotoSwaggerDocs.java (1)
  • NoArgsConstructor (6-40)
🔇 Additional comments (7)
build.gradle (1)

78-83: S3 의존성 추가가 올바르게 구성되었습니다.

AWS SDK BOM 패턴을 사용하여 S3 의존성을 선언한 것은 모범 사례입니다. 이를 통해 모든 AWS SDK 모듈 간 버전 일관성이 보장되며, 이전 리뷰에서 확인된 2.40.11 버전은 보안 취약점이 해결된 최신 버전입니다.

src/main/resources/db/migration/dev/V1__insert_seed_data.sql (1)

111-128: 사진 시드 데이터가 새 스키마에 맞게 잘 업데이트되었습니다.

content_types3key 확장자가 일치하도록 수정되었고, 새로운 Photo 엔티티 필드(trip_id, uploader_id, original_file_name, content_type, file_size, s3key)가 올바르게 반영되었습니다.

src/main/java/com/example/pventure/global/s3/S3KeyGenerator.java (1)

1-48: 파일명 검증 및 확장자 유효성 검사가 잘 구현되었습니다.

이전 리뷰에서 지적된 보안 관련 사항들이 모두 반영되었습니다:

  • null/blank 파일명 검증
  • 확장자 존재 여부 확인
  • 허용된 확장자만 통과 (jpg, jpeg, png, webp)
src/main/java/com/example/pventure/domain/album/repository/AlbumRepositoryCustomImpl.java (2)

39-50: QueryDSL 쿼리 구조가 올바르게 구현되었습니다.

leftJoin을 사용하여 사진이 없는 앨범도 조회되도록 하고, groupBy로 앨범별 사진 수를 집계하는 구현이 적절합니다.


29-37: 서비스 레이어에서 null 처리가 이미 구현되어 있어 추가 조치가 필요하지 않습니다. AlbumServiceImpl.getAlbum() 메서드에서 findByIdAndTripWithPhotoCount() 호출 후 null 체크를 수행하여 ApiException(ErrorCode.NOT_FOUND_ALBUM)을 발생시키고 있습니다.

src/main/java/com/example/pventure/domain/photo/service/PhotoServiceImpl.java (2)

194-202: 중복 ID 처리 로직이 잘 구현되었습니다.

이전 리뷰에서 지적된 중복 ID 문제가 HashSet을 사용하여 올바르게 해결되었습니다. 고유 ID 개수와 조회된 사진 개수를 비교하여 누락된 사진을 정확히 감지합니다.


41-59: 업로드 URL 생성 로직이 적절합니다.

@Transactional(readOnly = true)를 사용하고, TripPermissionService를 통한 권한 검사 후 presigned URL을 생성하는 흐름이 명확합니다.

Comment on lines +159 to +172
@Override
public void deletePhotos(Long userId, Long tripId, List<Long> photoIds) {

if (photoIds == null || photoIds.isEmpty()) {
return;
}

Trip trip = tripPermissionService.getEditableTrip(userId, tripId);

List<Photo> photos = loadPhotos(photoIds, trip);
photos.forEach(photo -> s3Service.deleteFile(photo.getS3Key()));

photoRepository.deleteAll(photos);
}
Copy link
Copy Markdown

@coderabbitai coderabbitai bot Dec 25, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟠 Major

S3 삭제와 DB 삭제 순서로 인한 데이터 불일치 위험

현재 S3 파일을 먼저 삭제한 후 DB 레코드를 삭제합니다. photoRepository.deleteAll(photos) 실행 중 예외가 발생하면 S3 파일은 이미 삭제되었지만 DB 레코드는 남아있는 불일치 상태가 발생할 수 있습니다.

🔎 순서 변경 및 보상 트랜잭션 패턴 제안
     @Override
     public void deletePhotos(Long userId, Long tripId, List<Long> photoIds) {

         if (photoIds == null || photoIds.isEmpty()) {
             return;
         }

         Trip trip = tripPermissionService.getEditableTrip(userId, tripId);

         List<Photo> photos = loadPhotos(photoIds, trip);
-        photos.forEach(photo -> s3Service.deleteFile(photo.getS3Key()));
 
+        List<String> s3Keys = photos.stream()
+            .map(Photo::getS3Key)
+            .toList();
+
         photoRepository.deleteAll(photos);
+
+        // DB 삭제 성공 후 S3 삭제 (실패해도 orphan 파일만 남음)
+        s3Keys.forEach(s3Service::deleteFile);
     }
🤖 Prompt for AI Agents
In src/main/java/com/example/pventure/domain/photo/service/PhotoServiceImpl.java
around lines 159-172, the current flow deletes files from S3 before removing DB
records creating a risk of S3/DB inconsistency if the DB delete fails; change to
remove DB records inside a transaction first (or mark them as "pending deletion"
and commit), then perform S3 deletions afterwards, and on any S3 failure
record/log the failure and enqueue a retry/job to reconcile rather than leaving
S3 already-deleted with DB intact; ensure the DB delete is transactional and
that S3 deletions are performed asynchronously or with compensating retry logic
so failures don’t produce permanent inconsistency.

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

여기도 확인 부탁드립니다.

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@rabitis99, 네, 해당 이슈를 재확인했습니다. 현재 코드의 삭제 순서로 인한 데이터 불일치 위험이 실제로 존재합니다.

현재 문제점:

photos.forEach(photo -> s3Service.deleteFile(photo.getS3Key())); // 1. S3 삭제
photoRepository.deleteAll(photos);                                 // 2. DB 삭제

이 순서에서는 S3 삭제가 성공한 후 DB 삭제 중 예외가 발생하면, DB에는 이미 삭제된 S3 파일을 가리키는 레코드가 남게 됩니다. 사용자가 해당 사진을 조회하려 할 때 S3에서 404 에러가 발생할 수 있습니다.

권장 수정 방안:

  1. 순서 변경: DB를 먼저 삭제하고 S3는 나중에 삭제

    • DB 삭제 실패 시 트랜잭션 롤백으로 아무 변경 없음
    • DB 삭제 성공 + S3 삭제 실패 시 고아 파일만 남음 (상대적으로 덜 심각)
  2. S3 삭제 실패 처리:

    • S3 삭제 실패 시 로깅하고 계속 진행 (try-catch)
    • 또는 비동기 재시도 큐에 등록하여 나중에 정리
@Override
public void deletePhotos(Long userId, Long tripId, List<Long> photoIds) {
    if (photoIds == null || photoIds.isEmpty()) {
        return;
    }

    Trip trip = tripPermissionService.getEditableTrip(userId, tripId);
    List<Photo> photos = loadPhotos(photoIds, trip);
    
    List<String> s3Keys = photos.stream()
        .map(Photo::getS3Key)
        .toList();

    // 1. DB 먼저 삭제 (트랜잭션 내)
    photoRepository.deleteAll(photos);

    // 2. S3 삭제 (실패해도 고아 파일만 남음)
    s3Keys.forEach(key -> {
        try {
            s3Service.deleteFile(key);
        } catch (Exception e) {
            // 로깅 및 재시도 큐 등록
            log.warn("Failed to delete S3 file: {}", key, e);
        }
    });
}

이 방식으로 변경하시면 데이터 일관성이 훨씬 개선됩니다.

🐰

@khg9900 khg9900 closed this Dec 27, 2025
@khg9900 khg9900 reopened this Dec 27, 2025
Copy link
Copy Markdown
Collaborator

@rabitis99 rabitis99 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

전체적으로 고생하셨습니다. 어제 이야기 나눈대로 Dto단 validattion은 실제 프론트와 연동함으로 해결하는 쪽으로 가시죠.

summary = PhotoSwaggerDocs.GENERATE_UPLOAD_URLS_SUMMARY,
description = PhotoSwaggerDocs.GENERATE_UPLOAD_URLS_DESCRIPTION
)
@ApiResponse(responseCode = "200", description = "업로드 URL 생성 성공", useReturnTypeSchema = true)
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

201짜리가 아닌 200으로 하신 이유가 있으실 까요?

Comment on lines +159 to +172
@Override
public void deletePhotos(Long userId, Long tripId, List<Long> photoIds) {

if (photoIds == null || photoIds.isEmpty()) {
return;
}

Trip trip = tripPermissionService.getEditableTrip(userId, tripId);

List<Photo> photos = loadPhotos(photoIds, trip);
photos.forEach(photo -> s3Service.deleteFile(photo.getS3Key()));

photoRepository.deleteAll(photos);
}
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

여기도 확인 부탁드립니다.


int index = originalFilename.lastIndexOf(".");
if (index == -1 || index == originalFilename.length() - 1) {
throw new IllegalArgumentException("파일 확장자가 존재하지 않습니다.");
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

여기도 에러코드로 관리하는 편이 좋을것 같습니다.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[FEAT] Photo 도메인 기능 구현 및 S3 업로드 연동

2 participants