[Refactor] s3 이미지 업로드 방식을 Presigned URL로 수정#288
Conversation
Walkthrough멀티파트 기반 이미지 업로드를 제거하고, Presigned URL 기반으로 전환했다. 피드 생성은 JSON 바디로 이미지 URL 목록을 받도록 변경됐다. Presigned URL 발급 엔드포인트와 DTO를 추가했고, S3 어댑터/포트와 기존 업로드/삭제 메서드를 삭제했다. 클라우드프론트 기반 URL 검증 서비스와 에러 코드가 추가됐다. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
actor User
participant Client
participant Server as FeedCommandController
participant S3 as S3Service
participant Validator as ImageUrlValidationService
participant App as FeedCreateService
rect rgba(198,235,255,0.3)
note over Client,S3: Presigned URL 발급 단계 (신규)
User->>Client: 이미지 업로드 준비
Client->>Server: POST /feeds/images/presigned-url [filename, extension, size]*
Server->>S3: getPresignedUrl(list, userId)
S3-->>Server: [presignedUrl, fileUrl]*
Server-->>Client: presignedUrls 응답
Client->>S3: PUT 이미지 업로드 (presignedUrl) [클라이언트에서]
end
rect rgba(206,255,206,0.3)
note over Client,App: 피드 생성 단계 (변경: URL 기반)
Client->>Server: POST /feeds { ..., imageUrls }
Server->>App: createFeed(command(imageUrls))
App->>Validator: validateUrlDomainAndUser(imageUrls, userId)
Validator-->>App: OK
App-->>Server: feedId
Server-->>Client: feedId 응답
end
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Assessment against linked issues
Assessment against linked issues: Out-of-scope changes
Possibly related PRs
Suggested reviewers
Poem
📜 Recent review detailsConfiguration used: CodeRabbit UI Review profile: CHILL Plan: Pro 💡 Knowledge Base configuration:
You can enable these sources in your CodeRabbit configuration. 📒 Files selected for processing (3)
✅ Files skipped from review due to trivial changes (1)
🚧 Files skipped from review as they are similar to previous changes (2)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
✨ Finishing Touches
🧪 Generate unit tests
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. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. CodeRabbit Commands (Invoked using PR/Issue comments)Type Other keywords and placeholders
CodeRabbit Configuration File (
|
Test Results451 tests 451 ✅ 40s ⏱️ Results for commit 88f92d7. ♻️ This comment has been updated with latest results. |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 16
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
src/main/java/konkuk/thip/room/adapter/out/persistence/repository/RoomQueryRepositoryImpl.java (2)
409-416: 추천용 서브쿼리에서 비활성(탈퇴) 참여까지 ‘참여’로 간주됨userJoinedRoom에 participant.status 조건이 없어, 과거에 참여했다가 비활성화된 방도 제외될 수 있습니다. 추천 품질 저하 가능성이 높습니다.
적용 예시:
private BooleanExpression userJoinedRoom(Long userId) { return JPAExpressions .selectOne() .from(participant) - .where(participant.userJpaEntity.userId.eq(userId) - .and(participant.roomJpaEntity.roomId.eq(room.roomId))) + .where(participant.userJpaEntity.userId.eq(userId) + .and(participant.roomJpaEntity.roomId.eq(room.roomId)) + .and(participant.status.eq(ACTIVE))) .exists(); }Also applies to: 420-427
300-317: Cursor 생성 시 priority 누락으로 페이징 경계에서 레코드 스킵 위험
RoomQueryPersistenceAdapter.findPlayingAndRecruitingRoomsUserParticipated에서 Cursor 생성부(List.of(dto.endDate, dto.roomId))에 priority를 첫 번째 요소로 포함하고, findRoomsByDeadlineCursor(cursor…) 로직을 priority→date→roomId 복합 비교로 수정하세요.
🧹 Nitpick comments (64)
src/test/java/konkuk/thip/book/adapter/in/web/BookMostSearchedBooksApiTest.java (5)
75-78: deleteAllInBatch 전환은 적절. Redis 테스트 키도 함께 정리해주세요.
- in-batch 삭제는 성능 상 유리하나, FK 제약/연관 엔티티가 늘어날 경우 무결성 문제 가능성은 한 번 확인 바랍니다.
- 본 테스트는 Redis에 어제자 랭킹 키를 추가하므로, 테스트 간 간섭을 막기 위해 tearDown에서 해당 키 패턴을 함께 삭제하는 것을 권장합니다.
적용 예시(해당 블록 내에서 추가):
void tearDown() { - userJpaRepository.deleteAllInBatch(); + userJpaRepository.deleteAllInBatch(); + // Redis 테스트 키 정리 + String pattern = searchRankDetailPrefix + "*"; + java.util.Set<String> keys = redisTemplate.keys(pattern); + if (keys != null && !keys.isEmpty()) { + redisTemplate.delete(keys); + } }
66-70: 중복된 nicknameUpdatedAt 세팅연속된 두 설정 중 뒤의 값이 앞의 값을 덮어씁니다. 의도 확인 후 하나만 남겨주세요.
- .nicknameUpdatedAt(LocalDate.now().minusMonths(7)) - .nicknameUpdatedAt(LocalDate.now().minusMonths(1)) + .nicknameUpdatedAt(LocalDate.now().minusMonths(7)) // 또는 (1) 중 하나만 유지
84-86: LocalDate.now() 의존으로 자정 경계 플래키 가능세팅 시점과 API 호출 시점 사이에 날짜가 바뀌면(자정 경계) 테스트가 간헐적으로 실패할 수 있습니다. 서비스 쪽에 Clock 주입(LocalDate.now(clock))으로 전환하고, 테스트에서는 Fixed Clock을 주입하는 방식을 권장합니다.
Also applies to: 139-141
81-82: DisplayName과 실제 검증 데이터 불일치(Top 5 vs 3개)표현만 정리하면 가독성이 좋아집니다.
- @DisplayName("어제 랭킹 Top 5를 정상적으로 조회한다") + @DisplayName("어제 랭킹 Top 3를 정상적으로 조회한다")
110-111: 불필요한 조회 제거: setUp에서 저장한 userId를 필드로 재사용 권장매 테스트마다 findAll().get(0) 호출 대신, setUp에서 저장한 사용자의 ID를 필드로 보관해 재사용하면 단순하고 안전합니다.
예시:
@@ - private ObjectMapper objectMapper; + private ObjectMapper objectMapper; + private Long userId; @@ - UserJpaEntity user = userJpaRepository.save(UserJpaEntity.builder() + UserJpaEntity user = userJpaRepository.save(UserJpaEntity.builder() .oauth2Id("kakao_432708231") .nickname("User1") .nicknameUpdatedAt(LocalDate.now().minusMonths(7)) .role(UserRole.USER) .alias(alias) .build()); + this.userId = user.getUserId(); @@ - Long userId = userJpaRepository.findAll().get(0).getUserId(); + Long userId = this.userId; @@ - Long userId = userJpaRepository.findAll().get(0).getUserId(); + Long userId = this.userId;Also applies to: 143-144
src/main/java/konkuk/thip/common/util/Cursor.java (1)
78-80: Double 커서 도입 시 정밀도·정렬 안정성 주의
getDouble(0)사용 지점
- RoomQueryPersistenceAdapter.java (검색 커서 기준)
- BookQueryPersistenceAdapter.java (검색 커서 기준)
부동소수점 표현 오차로 인해 커서 정렬·중복·누락이 발생할 수 있습니다.
BigDecimal 또는 고정 소수점(long) 방식 전환을 검토하거나, 최소한 인코딩/디코딩 시 Locale 고정 및 소수점 자릿수 고정 로직을 적용하세요.build.gradle (1)
82-84: ICU4J 추가 OK. 용량/버전/보안 영향 확인 권장icu4j는 JAR 용량이 큰 편이라(수 MB대) 컨테이너 이미지와 cold start에 영향이 있을 수 있습니다. 현재 기능이 한글 → 라틴 표기 변환(Transliterator) 위주라면, 실제 호출 빈도/경로를 점검하고 필요 시 lazy-load 또는 대체 라이브러리 검토를 권장합니다. 또한 최신 보안 패치 버전 여부를 확인해 주세요.
다음 점검 권장:
- icu4j 최신 안정 버전/보안 공지 확인
- 런타임 의존 경로에서 실제 사용 지점 수/빈도 확인(
rg -nP --type=java 'Transliterator|com\.ibm\.icu')src/main/java/konkuk/thip/room/application/port/in/dto/RoomGetHomeJoinedListQuery.java (1)
8-8:cursorStr네이밍 및 검증
- 컴포넌트명을
cursor로 단순화 제안- 최초 호출에 null/빈 문자열 허용, 이후 호출에 공백 불가 등 입력 제약 추가(@nullable String cursor, @SiZe(min=1) 등)
컨트롤러/서비스/레포지토리 전반에서 페이지 기반 → 커서 기반 파이프라인 전환이 올바르게 반영되었는지 직접 확인해주세요.
src/main/java/konkuk/thip/feed/application/service/FeedUpdateService.java (1)
22-22: 이미지 정리 전략 부재로 S3 orphan 객체 누적 위험직접 삭제 플로우를 제거하면서 남는 이미지 정리 전략이 비어 있습니다. AFTER_COMMIT 이벤트 기반 삭제 또는 주기적 배치/라이프사이클 정책을 즉시 도입해 주세요.
권장 옵션(택1+보완):
- 트랜잭션 커밋 후 이벤트(@TransactionalEventListener(AFTER_COMMIT))로 사용하지 않는 키 삭제.
- presigned 발급 시 태그(temp=true, ttl=24h) 부여 → 배치로 DB 미참조 + ttl 경과 객체 일괄 삭제.
- S3 Lifecycle Rule로 temp-prefix/tag 기준 자동 만료 설정.
관측성: 삭제 실패율/재시도 메트릭, DLQ(SQS) 확보.src/main/java/konkuk/thip/book/application/port/in/dto/BookPinResult.java (1)
3-9: 필수값 null 방지용 compact constructor 추가 제안API 응답 신뢰성을 위해 필수 필드에 대한 null 방지를 권장합니다.
다음과 같이 간단 검증을 추가해 주세요:
package konkuk.thip.book.application.port.in.dto; +import java.util.Objects; + public record BookPinResult( String bookTitle, String authorName, String bookImageUrl, String isbn ) { + public BookPinResult { + Objects.requireNonNull(bookTitle, "bookTitle must not be null"); + Objects.requireNonNull(authorName, "authorName must not be null"); + Objects.requireNonNull(bookImageUrl, "bookImageUrl must not be null"); + Objects.requireNonNull(isbn, "isbn must not be null"); + } }src/main/java/konkuk/thip/room/adapter/in/web/response/RoomGetHomeJoinedListResponse.java (2)
8-9: 불리언 컴포넌트 네이밍 정합성 검토(isLast → last)Record 컴포넌트명이 그대로 JSON 필드가 됩니다. 불리언에 접두어 is를 붙이면 클라이언트 측에서 중복(isLast)으로 보일 수 있어 last 권장합니다. 이미 공개된 API면 호환성 고려가 필요합니다.
- String nextCursor, - boolean isLast + String nextCursor, + boolean last
18-21: 정적 팩토리 of 포매팅 및 제네릭 간소화가독성 향상과 불필요한 정규화 타입 제거를 제안합니다.
- public static RoomGetHomeJoinedListResponse of(List<RoomGetHomeJoinedListResponse.JoinedRoomInfo> roomList, - String nickname, String nextCursor, boolean isLast){ - return new RoomGetHomeJoinedListResponse(roomList, nickname, nextCursor, isLast);} + public static RoomGetHomeJoinedListResponse of( + List<JoinedRoomInfo> roomList, + String nickname, + String nextCursor, + boolean isLast + ) { + return new RoomGetHomeJoinedListResponse(roomList, nickname, nextCursor, isLast); + }src/main/java/konkuk/thip/roompost/adapter/in/web/RoomPostQueryController.java (1)
83-87: Builder 체이닝 포매팅 정리체이닝 시 줄 끝의 점(.)은 허용되지만 일관된 스타일로 정리하면 가독성이 좋아집니다.
- return BaseResponse.ok(RecordPinResponse.of(recordPinUseCase.pinRecord( - RecordPinQuery.builder() - .roomId(roomId). - recordId(recordId) - .userId(userId).build()))); + return BaseResponse.ok(RecordPinResponse.of( + recordPinUseCase.pinRecord( + RecordPinQuery.builder() + .roomId(roomId) + .recordId(recordId) + .userId(userId) + .build() + ) + ));src/main/java/konkuk/thip/feed/application/service/FeedShowSavedListService.java (1)
39-41: 빈 입력 최적화: 좋아요 조회 건너뛰기페이지에 피드가 없을 때 불필요한 질의를 생략하면 좋습니다.
- Set<Long> likedFeedIdsByUser = postLikeQueryPort.findPostIdsLikedByUser(feedIds, userId); + Set<Long> likedFeedIdsByUser = feedIds.isEmpty() + ? Set.of() + : postLikeQueryPort.findPostIdsLikedByUser(feedIds, userId);src/main/java/konkuk/thip/feed/application/port/in/FeedCreateUseCase.java (3)
4-6: 불필요한 import 정리
MultipartFile,List는 더 이상 사용되지 않습니다. 정적 분석/빌드 실패를 유발할 수 있으니 제거 권장.import konkuk.thip.feed.application.port.in.dto.FeedCreateCommand; -import org.springframework.web.multipart.MultipartFile; - -import java.util.List;
8-10: 계약 명시 강화: Presigned URL 플로우 요구사항을 Javadoc으로 고정클라이언트가 DB에 저장할 것은 CloudFront 퍼블릭 URL이며 Presigned PUT URL은 저장 대상이 아님을 인터페이스 수준에서 명확히 하면 호출부 혼선을 줄일 수 있습니다.
public interface FeedCreateUseCase { - Long createFeed(FeedCreateCommand command); + /** + * Presigned URL 업로드 플로우 전제: + * - command.imageUrls: CloudFront 퍼블릭 접근 URL만 포함(예: https://cdn.example.com/<key>) + * - Presigned PUT URL은 저장/노출 금지 + */ + Long createFeed(FeedCreateCommand command); }
9-10: FeedCreateUseCase에서 불필요한 MultipartFile import 제거
컨트롤러·서비스·구현체는 모두createFeed(FeedCreateCommand)시그니처로 정상 이행되었으며, 2-인자 호출 잔존도 없습니다. 다만src/main/java/konkuk/thip/feed/application/port/in/FeedCreateUseCase.java의import org.springframework.web.multipart.MultipartFile;가 더 이상 사용되지 않으니 삭제해 주세요.src/main/java/konkuk/thip/roompost/adapter/in/web/response/RecordPinResponse.java (1)
11-11: 수정자 순서 컨벤션자바 컨벤션상
public static순서를 권장합니다.- static public RecordPinResponse of(BookPinResult book) { + public static RecordPinResponse of(BookPinResult book) {src/main/java/konkuk/thip/roompost/application/service/RecordPinService.java (2)
35-37: 사소한 스타일 정리쉼표 뒤 공백 추가로 일관성 유지.
- record.validatePin(query.userId(),query.roomId()); + record.validatePin(query.userId(), query.roomId());
38-41: 도메인 정합성 확인: Room 기준 조회 vs Record 기준 조회현재 책 조회가
roomId기준입니다. 하나의 방에 여러 책/레코드가 존재할 수 있다면, “핀할 레코드와 연관된 책”을 조회해야 합니다. 레코드가 책을 참조한다면 recordId 기반 조회가 더 안전합니다.- Book book = bookCommandPort.findBookByRoomId(query.roomId()); + // 레코드-책 연관을 따르는 조회가 가능하다면 다음과 같이 전환 검토 + Book book = bookCommandPort.findBookByRecordId(query.recordId()); return bookQueryMapper.toBookPinResult(book);추가로, 조회 실패 시 null 반환이 아니라 예외를 던지는 계약인지 확인해 주세요(일관된 에러 처리).
src/main/java/konkuk/thip/book/application/port/in/BookShowSavedListUseCase.java (1)
6-6: UseCase 계층의 cursor 인자를 Cursor VO로 일관되게 변경하세요.
UserGetFollowUsecase, RoomShowMineUseCase, BookShowSavedListUseCase, BookSelectableListUseCase, BookRecruitingRoomsUseCase, FeedSavedListUseCase 등에서
String cursor→konkuk.thip.common.util.Cursor cursor로 수정 필요.src/main/java/konkuk/thip/book/application/port/in/BookSelectableListUseCase.java (1)
7-7: cursor 표현은 값 객체 사용을 권장합니다.String 대신 공용 Cursor를 사용하면 인코딩 포맷 변경에 대한 영향 범위를 줄일 수 있습니다.
적용 예(선택):
- BookSelectableListResponse getSelectableBookList(BookSelectableType bookSelectableType, Long userId, String cursor); + BookSelectableListResponse getSelectableBookList(BookSelectableType bookSelectableType, Long userId, konkuk.thip.common.util.Cursor cursor);src/main/java/konkuk/thip/feed/adapter/out/persistence/FeedQueryPersistenceAdapter.java (1)
110-115: 쿼리가savedFeed.createdAt(저장 시각)을 사용하므로 동작에는 문제가 없습니다.
- 가독성 제고를 위해 JPA Repository 메서드명을
findSavedFeedsBySavedAt으로 변경하는 방안을 고려하세요.src/main/java/konkuk/thip/room/application/mapper/RoomParticipantQueryMapper.java (1)
12-16: 불필요한 imports 옵션 제거 권장
imports = DateUtil.class가 사용되지 않습니다. 혼동을 줄이기 위해 제거하세요.@Mapper( componentModel = "spring", - imports = DateUtil.class, unmappedTargetPolicy = ReportingPolicy.IGNORE // 명시적으로 매핑하지 않은 필드를 무시하도록 설정 )src/main/java/konkuk/thip/feed/application/service/FeedCreateService.java (2)
32-32: 이미지 수 검증: 불필요한 객체 생성 제거
ContentList.of(...).size()는 생성 시 이미 검증이 수행됩니다. 단순 크기 검증이면 직접 사이즈만 확인하세요.- ContentList.validateImageCount(ContentList.of(command.imageUrls()).size()); + ContentList.validateImageCount(command.imageUrls() == null ? 0 : command.imageUrls().size());
37-46: Presigned URL 전환에 따른 서버측 안전장치 보완 제안클라이언트가 임의 URL을 보내는 경우를 방지하기 위해:
- CDN 도메인/경로 화이트리스트 검증(예: CloudFront 도메인, 키 prefix).
- 생성해준 presigned 키 목록과의 소유권 매칭(토큰/세션에 발급 키 저장 후 소비).
- 저장 전 S3 HEAD로 존재 여부 확인(선택, 트래픽 고려).
- URL이 null일 수 있는 경우 도메인에서 안전하게 처리(빈 리스트로 정규화).
원하시면
ContentList에 URL 패턴 검증을 추가하는 패치 제안 드리겠습니다.src/main/java/konkuk/thip/room/adapter/in/web/RoomQueryController.java (1)
88-94: 커서 파라미터 전환 LGTM + 스웨거 예시/빈 문자열 처리 제안기능 전환 자체는 적절합니다. 스웨거 예시를 추가하고, 빈 문자열이 들어온 경우 null로 정규화하면 방어적입니다.
적용 예시:
- @Parameter(description = "커서 (첫번째 요청시 : null, 다음 요청시 : 이전 요청에서 반환받은 nextCursor 값)") - @RequestParam(value = "cursor", required = false) final String cursor) { + @Parameter( + description = "커서 (첫 요청: null, 다음 요청: 이전 응답의 nextCursor)", + example = "eyJwZXJjZW50YWdlIjo1MC4wLCJzdGFydERhdGUiOiIyMDI1LTA4LTMwIiwicm9vbUlkIjoxMjN9" + ) + @RequestParam(value = "cursor", required = false) final String cursor) { return BaseResponse.ok(roomGetHomeJoinedListUseCase.getHomeJoinedRoomList( RoomGetHomeJoinedListQuery.builder() .userId(userId) - .cursorStr(cursor).build())); + .cursorStr((cursor == null || cursor.isBlank()) ? null : cursor) + .build()));src/main/java/konkuk/thip/feed/adapter/in/web/request/FeedCreateRequest.java (2)
21-23: 사용자 노출 문구 수정(“방” → “피드”)피드 생성 DTO인데 “방 공개 설정”으로 표기되어 혼동 여지가 있습니다.
- @Schema(description = "방 공개 설정 여부 (true: 공개, false: 비공개)", example = "true") - @NotNull(message = "방 공개 설정 여부는 필수입니다.") + @Schema(description = "피드 공개 설정 여부 (true: 공개, false: 비공개)", example = "true") + @NotNull(message = "피드 공개 설정 여부는 필수입니다.")
28-29: 이미지 URL 제약 명시(최대 3개, CDN 도메인만)요구사항 상 최대 3장 제한이 있고, Presigned 업로드 이후 CloudFront URL만 허용됨을 스웨거에 명시하면 좋습니다.
- @Schema(description = "피드에 추가할 이미지들", example = "[\"https://img.domain.com/1.jpg\", \"https://img.domain.com/2.jpg\"]") + @Schema( + description = "피드에 추가할 이미지들 (최대 3개, 서버가 발급한 CloudFront 도메인 URL만 허용)", + example = "[\"https://dxxxxx.cloudfront.net/images/feeds/123/uuid1.jpg\", \"https://dxxxxx.cloudfront.net/images/feeds/123/uuid2.jpg\"]" + )서버 측 검증 권장(보안/무결성):
- CloudFront 도메인 화이트리스트 검증(프로덕션/스테이징 모두).
- 객체 키 prefix(예: images/feeds/{userId}/) 검증.
- Presigned 정책에 content-length-range, content-type(image/*) 조건 포함.
- 피드 생성 시 S3 HEAD로 존재/크기 확인(선택).
- 업로드 후 미참조 오브젝트 정리(수명 주기/배치).
src/main/java/konkuk/thip/room/adapter/out/persistence/repository/RoomQueryRepository.java (1)
24-24: 홈 참여방 커서 메서드에 정렬 기준·타이브레이크 Javadoc 추가 (선택 사항)
메서드 시그니처 위에 cursor 필드(userPercentage, startDate, roomId)와 실제 구현(orderBy: userPercentage.desc(), startDate.asc(), roomId.asc()) 기준·방향을 명시해주세요.--- 기존 - List<RoomParticipantQueryDto> findHomeJoinedRoomsByUserPercentage(Long userId, Double userPercentageCursor, LocalDate startDateCursor, Long roomIdCursor, int pageSize); +++ 제안 + /** + * [홈 참여방] 커서 기반 조회 + * cursor 순서 및 방향: + * 1) userPercentage(desc) + * 2) startDate(asc) + * 3) roomId(asc) + * null 커서(첫 페이지)도 정상 작동해야 합니다. + */ + List<RoomParticipantQueryDto> findHomeJoinedRoomsByUserPercentage( + Long userId, + Double userPercentageCursor, + LocalDate startDateCursor, + Long roomIdCursor, + int pageSize + );src/test/java/konkuk/thip/feed/adapter/in/web/FeedCreateControllerTest.java (1)
45-47: 불필요한 필드(category) 제거 제안
FeedCreateRequest에는category필드가 없으므로 혼란을 줄이기 위해 테스트 요청 본문에서도 제거하는 편이 명확합니다(환경에 따라 unknown property 처리 차이).- request.put("category", "문학");src/main/java/konkuk/thip/book/adapter/in/web/response/BookSelectableListResponse.java (2)
6-8: nextCursor/isLast 불변식 보장 필요isLast=true일 때 nextCursor를 null로 정규화하고, isLast=false일 때 nextCursor 공백 불가를 보장하면 API 일관성이 좋아집니다. null bookList 방지도 함께 권장합니다.
- public static BookSelectableListResponse of(List<BookSelectableDto> bookList, String nextCursor, boolean isLast) { - return new BookSelectableListResponse(bookList, nextCursor, isLast); - } + public static BookSelectableListResponse of(List<BookSelectableDto> bookList, String nextCursor, boolean isLast) { + if (!isLast && (nextCursor == null || nextCursor.isBlank())) { + throw new IllegalArgumentException("nextCursor must be provided when isLast is false"); + } + String normalizedCursor = isLast ? null : nextCursor; + return new BookSelectableListResponse( + bookList == null ? List.of() : bookList, + normalizedCursor, + isLast + ); + }Also applies to: 19-20
10-17: 스웨거/직렬화 메타데이터 추가 제안응답 스키마가 외부에 노출되는 만큼 @Schema, @JsonInclude(Include.NON_NULL) 추가를 권장합니다. nextCursor가 null일 때 필드 생략되면 클라이언트 파싱이 단순해집니다.
추가 변경(파일 상단 import와 타입 어노테이션):
+import com.fasterxml.jackson.annotation.JsonInclude; +import io.swagger.v3.oas.annotations.media.Schema; @@ - public record BookSelectableListResponse( +@JsonInclude(JsonInclude.Include.NON_NULL) +@Schema(name = "BookSelectableListResponse") +public record BookSelectableListResponse( @@ - public record BookSelectableDto( + @Schema(name = "BookSelectableDto") + public record BookSelectableDto(src/main/java/konkuk/thip/book/application/service/BookSavedListService.java (1)
18-18: 페이지 크기 상수의 중복 정의 방지여러 서비스에서 10을 반복 정의 중이면 공통 상수(예: Cursor.DEFAULT_PAGE_SIZE 또는 별도 PaginationProperties)로 모으는 것을 권장합니다.
src/main/java/konkuk/thip/room/application/port/out/dto/RoomParticipantQueryDto.java (2)
22-22: 에러 메시지 오타 수정필드명이 roomTitle인데 메시지는 roomName입니다. 정정 권장.
- Assert.notNull(roomTitle, "roomName must not be null"); + Assert.notNull(roomTitle, "roomTitle must not be null");
15-16: 수치 타입 선택 재검토(정밀도/널 허용성)
- memberCount는 원시 int로, userPercentage는 BigDecimal(스케일 고정) 사용을 고려해 주세요. Double은 비교/정렬 시 부동소수 오차가 누적될 수 있습니다.
src/test/java/konkuk/thip/feed/adapter/in/web/FeedCreateApiTest.java (4)
78-81: 테스트용 이미지 URL 도메인 상수화 및 CDN 도메인 사용운영 계획이 CloudFront 도메인 저장이라면 테스트도 동일 도메인(예: ${cdn.base-url})을 사용하도록 상수화/주입하면 구성 불일치를 줄일 수 있습니다.
- request.put("imageUrls", List.of( - "https://mock-s3-bucket/fake-image-url1.jpg", - "https://mock-s3-bucket/fake-image-url2.jpg" - )); + final String cdn = "https://mock-cloudfront-domain"; // TODO: @Value("${cdn.base-url}") 주입 또는 TestPropertySource 사용 + request.put("imageUrls", List.of( + cdn + "/fake-image-url1.jpg", + cdn + "/fake-image-url2.jpg" + ));
84-87: 중복 요청 구성 헬퍼 메서드로 추출post("/feeds") + 공통 헤더/속성 설정이 반복됩니다. private ResultActions createFeed(Map<String,Object>) 헬퍼로 추출하면 테스트 가독성이 좋아집니다.
165-167: 위와 동일 — CDN 도메인 사용 및 헬퍼 추출이미지 URL 도메인/요청 생성 중복을 위 코멘트와 동일하게 정리해 주세요.
Also applies to: 170-174
121-124: 생성 응답 코드 201 및 Location 헤더 검토생성 API라면 200 대신 201 Created와 Location: /feeds/{id} 반환이 REST 친화적입니다. 컨트롤러 변경 시 테스트도 함께 갱신을 고려해 주세요.
Also applies to: 214-218, 247-251
src/main/java/konkuk/thip/book/adapter/in/web/response/BookShowSavedListResponse.java (2)
3-22: OpenAPI 문서 보강: 스키마 주석 추가 권장클라이언트 스펙 합의가 중요한 응답 DTO라 필드 레벨 @Schema를 붙여 주면 가독성과 유지보수성이 좋아집니다. nextCursor의 null 가능 여부도 명시해 주세요.
package konkuk.thip.book.adapter.in.web.response; import java.util.List; +import io.swagger.v3.oas.annotations.media.Schema; -public record BookShowSavedListResponse( - List<BookShowSavedDto> bookList, - String nextCursor, - boolean isLast -) { - public record BookShowSavedDto( - Long bookId, - String bookTitle, - String authorName, - String publisher, - String bookImageUrl, - String isbn, - boolean isSaved - ) { - } +@Schema(description = "저장한 도서 목록 응답") +public record BookShowSavedListResponse( + @Schema(description = "저장한 도서 목록") + List<BookShowSavedDto> bookList, + @Schema(description = "다음 페이지 커서", nullable = true) + String nextCursor, + @Schema(description = "마지막 페이지 여부") + boolean isLast +) { + @Schema(description = "저장 도서 요약 정보") + public record BookShowSavedDto( + @Schema(description = "도서 ID") Long bookId, + @Schema(description = "도서 제목") String bookTitle, + @Schema(description = "저자명") String authorName, + @Schema(description = "출판사") String publisher, + @Schema(description = "도서 이미지 URL") String bookImageUrl, + @Schema(description = "ISBN-13") String isbn, + @Schema(description = "저장 여부(저장 목록의 경우 항상 true)") boolean isSaved + ) { } public static BookShowSavedListResponse of(List<BookShowSavedDto> bookList, String nextCursor, boolean isLast) { return new BookShowSavedListResponse(bookList, nextCursor, isLast); } }
17-17: isSaved 필드 필요성 재확인저장 목록 응답에서는 항상 true일 가능성이 높습니다. 중복 정보라면 제거하거나, 추후 재사용(예: 혼합 리스트) 계획이 있다면 주석으로 의도를 남겨 주세요.
src/test/java/konkuk/thip/room/adapter/in/web/RoomGetHomeJoinedRoomsApiTest.java (5)
29-33: 숫자 비교의 타입 불일치로 인한 테스트 취약성JSON 숫자 직렬화가 80 vs 80.0으로 달라질 수 있어 is(정수) 비교가 깨질 수 있습니다. closeTo를 사용해 부동소수 비교로 안전하게 바꾸는 것을 권장합니다.
import static org.hamcrest.Matchers.hasSize; -import static org.hamcrest.Matchers.is; +import static org.hamcrest.Matchers.is; +import static org.hamcrest.Matchers.closeTo;
148-149: userPercentage 비교를 부동소수 허용으로 변경직렬화 타입 변화에도 안정적인 어설션으로 교체합니다.
- .andExpect(jsonPath("$.data.roomList[0].userPercentage", is(80))) - .andExpect(jsonPath("$.data.roomList[1].userPercentage", is(60))) + .andExpect(jsonPath("$.data.roomList[0].userPercentage", closeTo(80.0, 0.001))) + .andExpect(jsonPath("$.data.roomList[1].userPercentage", closeTo(60.0, 0.001))) @@ - .andExpect(jsonPath("$.data.roomList[0].userPercentage", is(70))) - .andExpect(jsonPath("$.data.roomList[1].userPercentage", is(70))) - .andExpect(jsonPath("$.data.roomList[2].userPercentage", is(70))) + .andExpect(jsonPath("$.data.roomList[0].userPercentage", closeTo(70.0, 0.001))) + .andExpect(jsonPath("$.data.roomList[1].userPercentage", closeTo(70.0, 0.001))) + .andExpect(jsonPath("$.data.roomList[2].userPercentage", closeTo(70.0, 0.001))) @@ - .andExpect(jsonPath("$.data.roomList[0].userPercentage", is(50))); + .andExpect(jsonPath("$.data.roomList[0].userPercentage", closeTo(50.0, 0.001)));Also applies to: 192-194, 232-232
158-173: LocalDate.now() 직접 사용으로 인한 경계 시각 취약성자정 경계에서 테스트가 비결정적으로 실패할 수 있습니다. 테스트 내에서 today를 한 번만 계산해 재사용하거나 Clock 주입을 고려해 주세요.
- // 방1: 시작일 오늘-2 - RoomJpaEntity room1 = roomJpaRepository.save( - TestEntityFactory.createCustomRoom(book, category, LocalDate.now().minusDays(2), LocalDate.now().plusDays(10)) - ); + LocalDate today = LocalDate.now(); + // 방1: 시작일 오늘-2 + RoomJpaEntity room1 = roomJpaRepository.save( + TestEntityFactory.createCustomRoom(book, category, today.minusDays(2), today.plusDays(10)) + ); // 방2: 시작일 오늘-1 RoomJpaEntity room2 = roomJpaRepository.save( - TestEntityFactory.createCustomRoom(book, category, LocalDate.now().minusDays(1), LocalDate.now().plusDays(8)) + TestEntityFactory.createCustomRoom(book, category, today.minusDays(1), today.plusDays(8)) ); // 방3: 시작일 오늘 RoomJpaEntity room3 = roomJpaRepository.save( - TestEntityFactory.createCustomRoom(book, category, LocalDate.now(), LocalDate.now().plusDays(9)) + TestEntityFactory.createCustomRoom(book, category, today, today.plusDays(9)) );
116-123: 테스트 데이터의 HOST 중복 위험saveSingleUserToRoom에서 항상 HOST로 저장하면 방당 HOST 유니크 제약이 있을 때 실패할 수 있습니다. 멤버로 저장하거나 역할을 파라미터로 받도록 바꾸는 편이 안전합니다.
- private void saveSingleUserToRoom(RoomJpaEntity roomJpaEntity, UserJpaEntity userJpaEntity, Double userPercentage) { + private void saveSingleUserToRoom(RoomJpaEntity roomJpaEntity, UserJpaEntity userJpaEntity, Double userPercentage) { RoomParticipantJpaEntity roomParticipantJpaEntity = RoomParticipantJpaEntity.builder() .userJpaEntity(userJpaEntity) .roomJpaEntity(roomJpaEntity) - .roomParticipantRole(RoomParticipantRole.HOST) + .roomParticipantRole(RoomParticipantRole.MEMBER) .userPercentage(userPercentage) .build();
278-285: 기본 페이지 사이즈 의존 최소화첫 페이지도 size=10을 명시하면 구성 변경(기본값 변경)에 덜 민감해집니다. 선택 사항입니다.
- ResultActions firstPage = mockMvc.perform(get("/rooms/home/joined") - .requestAttr("userId", user.getUserId())); + ResultActions firstPage = mockMvc.perform(get("/rooms/home/joined") + .requestAttr("userId", user.getUserId()) + .param("size", "10"));src/main/java/konkuk/thip/feed/adapter/in/web/response/FeedUploadImagePresignedUrlResponse.java (2)
7-22: OpenAPI 문서 강화 및 명명 일관성
- 상위 레코드와 presignedUrls 필드에도 @Schema를 추가하면 스웨거 문서가 명확해집니다.
- 내부 필드 preSignedUrl → presignedUrl로 케이스를 통일하는 것을 권장합니다(외부 호환성 영향 검토 필요).
package konkuk.thip.feed.adapter.in.web.response; import io.swagger.v3.oas.annotations.media.Schema; import java.util.List; +@Schema(description = "여러 이미지 업로드를 위한 Presigned URL 목록 응답") public record FeedUploadImagePresignedUrlResponse( - List<PresignedUrlInfo> presignedUrls + @Schema(description = "업로드용 Presigned URL과 업로드 후 접근 URL 쌍 목록") + List<PresignedUrlInfo> presignedUrls ) { public static FeedUploadImagePresignedUrlResponse of(List<PresignedUrlInfo> urls) { return new FeedUploadImagePresignedUrlResponse(urls); } @Schema(description = "Presigned URL 및 파일 접근 URL 정보") public record PresignedUrlInfo( - @Schema(description = "PUT 요청을 보내기 위한 파일 업로드용 Presigned URL") - String preSignedUrl, + @Schema(description = "PUT 요청을 보내기 위한 파일 업로드용 Presigned URL") + String presignedUrl, @Schema(description = "업로드 후 접근 가능한 파일 URL") String fileUrl ) {} }
7-22: Presigned URL 운영 보안 체크리스트서버 구현에서 아래를 강제하고 있는지 확인 부탁드립니다.
- URL 만료 짧게(예: 1–5분)
- Content-Length-Range, Content-Type 조건 제한
- ACL 미포함(버킷 private), SSE-S3 또는 KMS 적용
- 오브젝트 키에 경로 접근 제어(사용자 격리, 디렉터리 트래버설 방지)
- CloudFront Origin Access Control 사용 및 GET만 허용
- 필요한 업로드 헤더를 응답에 명시(예: Content-Type)해 클라이언트 서명 불일치 방지
src/main/java/konkuk/thip/book/application/service/BookSelectableListService.java (2)
30-33: switch 기본 분기 추가로 방어적 코딩향후 enum 값 증가나 예외 상황에서 NPE/ISE를 명확히 처리하기 위해 default 분기를 추가하는 것을 권장합니다.
- CursorBasedList<BookQueryDto> result = switch(bookSelectableType) { + CursorBasedList<BookQueryDto> result = switch (bookSelectableType) { case SAVED -> bookQueryPort.findSavedBooksBySavedAt(userId, cursor); case JOINING -> bookQueryPort.findJoiningRoomsBooksByRoomPercentage(userId, cursor); + default -> throw new IllegalArgumentException("Unsupported BookSelectableType: " + bookSelectableType); };
19-19: 페이지 사이즈 설정의 외부화 고려DEFAULT_PAGE_SIZE(10)을 환경설정(예: application.yml)로 외부화하면 운영 중 튜닝이 쉬워집니다.
src/main/java/konkuk/thip/book/application/port/out/dto/BookQueryDto.java (1)
5-6: @nullable 패키지 일관성/의존성 확인javax.annotation.Nullable은 JDK 17에 기본 포함되지 않습니다. 프로젝트 전반에서 사용하는 표준(nullability) 어노테이션으로 통일하는 것을 권장합니다(예: org.springframework.lang.Nullable 또는 jakarta.annotation.Nullable). 빌드 의존성 유무를 확인해 주세요.
-import javax.annotation.Nullable; +import org.springframework.lang.Nullable;src/test/java/konkuk/thip/book/adapter/in/web/BookShowSavedListApiTest.java (1)
108-181: 동일 created_at 충돌 케이스 추가 테스트를 권장합니다.현재 테스트는 타임스탬프가 모두 달라 중복/누락을 탐지하지 못합니다. 동일 created_at에서 경계 중복이 없는지 검증 테스트를 추가하세요.
예시(요지):
- 두 saved_books의 created_at을 동일하게 맞춘 뒤 페이지 경계에 걸치도록 데이터 배치.
- 1페이지/2페이지 합집합이 정확히 N건이고 교집합이 0건임을 검증.
원하시면 테스트 코드 스니펫까지 제공하겠습니다.
src/main/java/konkuk/thip/book/adapter/in/web/BookQueryController.java (1)
97-103: type은 String 대신 Enum 바인딩으로 직접 받고, 유효성 에러를 조기 표면화하세요.
BookSelectableType.from(type)대신 파라미터를BookSelectableType type으로 받아 Spring이 검증하도록 하면 400으로 명확히 떨어집니다.- public BaseResponse<BookSelectableListResponse> showSelectableBookList( - @RequestParam final String type, + public BaseResponse<BookSelectableListResponse> showSelectableBookList( + @RequestParam final BookSelectableType type, @Parameter(hidden = true) @UserId final Long userId, @RequestParam(required = false) final String cursor) { return BaseResponse.ok( - bookSelectableListUseCase.getSelectableBookList(BookSelectableType.from(type), userId, cursor) + bookSelectableListUseCase.getSelectableBookList(type, userId, cursor) ); }src/main/java/konkuk/thip/book/adapter/out/persistence/repository/BookQueryRepositoryImpl.java (1)
73-75: 집계 표현은 템플릿보다 타입 안정적인 max() API 사용을 권장합니다.
numberTemplate("max({0})", …)대신room.roomPercentage.max()사용이 더 안전합니다.- NumberExpression<Double> maxRoomPercentage = Expressions.numberTemplate(Double.class, "max({0})", room.roomPercentage); + NumberExpression<Double> maxRoomPercentage = room.roomPercentage.max();src/main/java/konkuk/thip/room/adapter/out/persistence/RoomQueryPersistenceAdapter.java (1)
72-90: 정렬 및 커서 조건 일치 검증 완료
ORDER BY(userPercentage DESC, startDate ASC, roomId ASC)와 cursor where 절(A OR B OR C)이 1:1로 매핑되어 중복/누락 이슈 없음. 부동소수점 오차 방지를 위해 DECIMAL 고정소수점 또는 정수 스케일 적용 검토 추천.src/main/java/konkuk/thip/feed/adapter/out/s3/S3Service.java (3)
73-75: CloudFront URL 조합 시 슬래시 누락 가능성base URL 설정에 따라 슬래시가 중복/누락될 수 있습니다. 안전 조합을 권장합니다.
-String fileUrl = cloudFrontBaseUrl + key; +String fileUrl = (cloudFrontBaseUrl.endsWith("/") ? cloudFrontBaseUrl : cloudFrontBaseUrl + "/") + key;
63-66: 주석과 상수 불일치(‘1시간’ vs 5분)URL_EXPIRED_TIME은 5분인데 주석은 1시간으로 표기되어 있습니다. 주석 정정 부탁드립니다.
-// url 유효기간 설정하기(1시간) +// url 유효기간 설정하기(5분)Also applies to: 39-39
40-56: 입력값 방어적 검증 추가 제안images null/빈 리스트, filename null/empty, size <= 0 케이스에 대한 방어적 검증이 있으면 좋습니다(클라이언트 오입력 시 빠른 4xx 응답).
원하시면 Controller/DTO 레벨 Bean Validation(@notblank, @positive, @SiZe(min=1))도 함께 제안 드릴게요.
src/main/java/konkuk/thip/room/application/service/RoomGetHomeJoinedListService.java (1)
30-41: 커서 기반 전환 LGTM + 페이지 크기 외부화 제안서비스 레이어에서 커서 생성/매핑이 명확합니다. page size(10)를 설정으로 외부화하면 운영 중 조정이 수월합니다. 또한 리포지토리가 “홈 참여 중”을 현재 진행 중 방으로 제한하는 조건을 포함하는지 한번만 확인 부탁드립니다.
src/main/java/konkuk/thip/room/adapter/out/persistence/repository/RoomQueryRepositoryImpl.java (4)
208-217: LocalDate.now() 중복 호출 제거 제안동일 메서드 내에서 now를 여러 번 호출할 필요가 없습니다. 가독성과 미세 성능 측면에서 캐싱을 권장합니다.
적용 예시:
- BooleanBuilder where = new BooleanBuilder(); - where.and(participant.userJpaEntity.userId.eq(userId)); - where.and(participant.status.eq(ACTIVE)); - where.and(room.status.eq(ACTIVE)); - where.and(room.startDate.loe(LocalDate.now())); - where.and(room.endDate.goe(LocalDate.now())); + LocalDate today = LocalDate.now(); + BooleanBuilder where = new BooleanBuilder() + .and(participant.userJpaEntity.userId.eq(userId)) + .and(participant.status.eq(ACTIVE)) + .and(room.status.eq(ACTIVE)) + .and(room.startDate.loe(today)) + .and(room.endDate.goe(today));
219-229: 커서 복합조건 괄호를 명시적으로 재구성해 가독성/오해 여지 제거현재 표현도 논리상 맞으나, and/or 중첩은 한 번 더 괄호화해 주는 편이 안전합니다.
- if (userPercentageCursor != null && startDateCursor != null && roomIdCursor != null) { - where.and(participant.userPercentage.lt(userPercentageCursor) - .or(participant.userPercentage.eq(userPercentageCursor) - .and(room.startDate.gt(startDateCursor)) - .or(participant.userPercentage.eq(userPercentageCursor) - .and(room.startDate.eq(startDateCursor)) - .and(room.roomId.gt(roomIdCursor)) - ) - ) - ); - } + if (userPercentageCursor != null && startDateCursor != null && roomIdCursor != null) { + BooleanExpression cursorPredicate = + participant.userPercentage.lt(userPercentageCursor) + .or(participant.userPercentage.eq(userPercentageCursor) + .and( + room.startDate.gt(startDateCursor) + .or(room.startDate.eq(startDateCursor) + .and(room.roomId.gt(roomIdCursor)) + ) + )); + where.and(cursorPredicate); + }
211-217: 쿼리 성능 인덱스 제안다음 조합 인덱스를 검토해 주세요(실제 통계/실행계획 기반으로 결정 권장).
- room: (status, startDate), (status, endDate)
- participant: (user_id, status), (room_id, status), 필요 시 (user_id, status, user_percentage)
Also applies to: 245-248
430-431: 주석 최신화 제안현재는 “2단계 조회(IDs → entities)”가 아니라 DTO 직접 프로젝션입니다. 주석을 코드와 일치시키는 편이 좋습니다.
예: “공통 커서 처리 + DTO 직접 조회”
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (60)
build.gradle(1 hunks)src/main/java/konkuk/thip/book/adapter/in/web/BookQueryController.java(2 hunks)src/main/java/konkuk/thip/book/adapter/in/web/response/BookSelectableListResponse.java(1 hunks)src/main/java/konkuk/thip/book/adapter/in/web/response/BookShowSavedListResponse.java(1 hunks)src/main/java/konkuk/thip/book/adapter/out/persistence/BookQueryPersistenceAdapter.java(2 hunks)src/main/java/konkuk/thip/book/adapter/out/persistence/repository/BookJpaRepository.java(1 hunks)src/main/java/konkuk/thip/book/adapter/out/persistence/repository/BookQueryRepository.java(1 hunks)src/main/java/konkuk/thip/book/adapter/out/persistence/repository/BookQueryRepositoryImpl.java(1 hunks)src/main/java/konkuk/thip/book/application/mapper/BookQueryMapper.java(2 hunks)src/main/java/konkuk/thip/book/application/port/in/BookSelectableListUseCase.java(1 hunks)src/main/java/konkuk/thip/book/application/port/in/BookShowSavedListUseCase.java(1 hunks)src/main/java/konkuk/thip/book/application/port/in/dto/BookPinResult.java(1 hunks)src/main/java/konkuk/thip/book/application/port/in/dto/DummyCommand.java(0 hunks)src/main/java/konkuk/thip/book/application/port/in/dto/DummyQuery.java(0 hunks)src/main/java/konkuk/thip/book/application/port/out/BookQueryPort.java(2 hunks)src/main/java/konkuk/thip/book/application/port/out/dto/BookQueryDto.java(1 hunks)src/main/java/konkuk/thip/book/application/service/BookSavedListService.java(1 hunks)src/main/java/konkuk/thip/book/application/service/BookSelectableListService.java(1 hunks)src/main/java/konkuk/thip/common/exception/code/ErrorCode.java(1 hunks)src/main/java/konkuk/thip/common/swagger/SwaggerResponseDescription.java(1 hunks)src/main/java/konkuk/thip/common/util/Cursor.java(1 hunks)src/main/java/konkuk/thip/feed/adapter/in/web/FeedCommandController.java(2 hunks)src/main/java/konkuk/thip/feed/adapter/in/web/request/FeedCreateRequest.java(1 hunks)src/main/java/konkuk/thip/feed/adapter/in/web/request/FeedUploadImagePresignedUrlRequest.java(1 hunks)src/main/java/konkuk/thip/feed/adapter/in/web/response/FeedUploadImagePresignedUrlResponse.java(1 hunks)src/main/java/konkuk/thip/feed/adapter/out/persistence/FeedQueryPersistenceAdapter.java(1 hunks)src/main/java/konkuk/thip/feed/adapter/out/persistence/S3CommandPersistenceAdapter.java(0 hunks)src/main/java/konkuk/thip/feed/adapter/out/s3/S3Service.java(1 hunks)src/main/java/konkuk/thip/feed/application/port/in/FeedCreateUseCase.java(1 hunks)src/main/java/konkuk/thip/feed/application/port/in/dto/FeedCreateCommand.java(1 hunks)src/main/java/konkuk/thip/feed/application/port/out/FeedQueryPort.java(1 hunks)src/main/java/konkuk/thip/feed/application/port/out/S3CommandPort.java(0 hunks)src/main/java/konkuk/thip/feed/application/service/FeedCreateService.java(1 hunks)src/main/java/konkuk/thip/feed/application/service/FeedShowSavedListService.java(1 hunks)src/main/java/konkuk/thip/feed/application/service/FeedUpdateService.java(1 hunks)src/main/java/konkuk/thip/room/adapter/in/web/RoomQueryController.java(1 hunks)src/main/java/konkuk/thip/room/adapter/in/web/response/RoomGetHomeJoinedListResponse.java(1 hunks)src/main/java/konkuk/thip/room/adapter/out/persistence/RoomQueryPersistenceAdapter.java(2 hunks)src/main/java/konkuk/thip/room/adapter/out/persistence/repository/RoomQueryRepository.java(2 hunks)src/main/java/konkuk/thip/room/adapter/out/persistence/repository/RoomQueryRepositoryImpl.java(9 hunks)src/main/java/konkuk/thip/room/application/mapper/RoomParticipantQueryMapper.java(1 hunks)src/main/java/konkuk/thip/room/application/port/in/dto/RoomGetHomeJoinedListQuery.java(1 hunks)src/main/java/konkuk/thip/room/application/port/out/RoomQueryPort.java(2 hunks)src/main/java/konkuk/thip/room/application/port/out/dto/RoomParticipantQueryDto.java(1 hunks)src/main/java/konkuk/thip/room/application/service/RoomGetHomeJoinedListService.java(2 hunks)src/main/java/konkuk/thip/roompost/adapter/in/web/RoomPostQueryController.java(1 hunks)src/main/java/konkuk/thip/roompost/adapter/in/web/response/RecordPinResponse.java(1 hunks)src/main/java/konkuk/thip/roompost/application/port/in/RecordPinUseCase.java(1 hunks)src/main/java/konkuk/thip/roompost/application/port/in/dto/record/RecordPinQuery.java(1 hunks)src/main/java/konkuk/thip/roompost/application/service/RecordPinService.java(3 hunks)src/test/java/konkuk/thip/book/adapter/in/web/BookGetSelectableListApiTest.java(2 hunks)src/test/java/konkuk/thip/book/adapter/in/web/BookMostSearchedBooksApiTest.java(1 hunks)src/test/java/konkuk/thip/book/adapter/in/web/BookShowSavedListApiTest.java(2 hunks)src/test/java/konkuk/thip/config/TestS3MockConfig.java(0 hunks)src/test/java/konkuk/thip/feed/adapter/in/web/FeedCreateApiTest.java(8 hunks)src/test/java/konkuk/thip/feed/adapter/in/web/FeedCreateControllerTest.java(3 hunks)src/test/java/konkuk/thip/feed/adapter/in/web/FeedShowWriteInfoApiTest.java(0 hunks)src/test/java/konkuk/thip/feed/adapter/in/web/FeedUpdateApiTest.java(0 hunks)src/test/java/konkuk/thip/room/adapter/in/web/RoomGetHomeJoinedRoomsApiTest.java(6 hunks)src/test/java/konkuk/thip/room/adapter/in/web/RoomGetHomeJoinedRoomsControllerTest.java(0 hunks)
💤 Files with no reviewable changes (8)
- src/test/java/konkuk/thip/feed/adapter/in/web/FeedShowWriteInfoApiTest.java
- src/test/java/konkuk/thip/feed/adapter/in/web/FeedUpdateApiTest.java
- src/test/java/konkuk/thip/room/adapter/in/web/RoomGetHomeJoinedRoomsControllerTest.java
- src/test/java/konkuk/thip/config/TestS3MockConfig.java
- src/main/java/konkuk/thip/book/application/port/in/dto/DummyQuery.java
- src/main/java/konkuk/thip/feed/adapter/out/persistence/S3CommandPersistenceAdapter.java
- src/main/java/konkuk/thip/feed/application/port/out/S3CommandPort.java
- src/main/java/konkuk/thip/book/application/port/in/dto/DummyCommand.java
🧰 Additional context used
🧠 Learnings (2)
📚 Learning: 2025-07-03T03:05:05.031Z
Learnt from: seongjunnoh
PR: THIP-TextHip/THIP-Server#43
File: src/main/java/konkuk/thip/book/application/port/out/BookCommandPort.java:0-0
Timestamp: 2025-07-03T03:05:05.031Z
Learning: THIP 프로젝트에서는 CQRS Port 분리 시 다음 컨벤션을 따름: CommandPort에는 findByXXX를 통해 도메인 엔티티를 찾아오는 메서드를 추가하고, QueryPort에는 조회 API의 response에 해당하는 데이터들을 DB로부터 조회하는 메서드를 추가함.
Applied to files:
src/main/java/konkuk/thip/book/application/port/out/BookQueryPort.java
📚 Learning: 2025-07-16T11:01:16.732Z
Learnt from: hd0rable
PR: THIP-TextHip/THIP-Server#85
File: src/main/java/konkuk/thip/feed/adapter/in/web/FeedCommandController.java:65-73
Timestamp: 2025-07-16T11:01:16.732Z
Learning: S3Service.deleteImageFromS3() 메서드는 이미 비동기(Async) 함수로 구현되어 있어서 호출하는 쪽에서 별도의 비동기 처리가 필요하지 않습니다.
Applied to files:
src/main/java/konkuk/thip/feed/adapter/out/s3/S3Service.java
🧬 Code graph analysis (8)
src/main/java/konkuk/thip/room/application/mapper/RoomParticipantQueryMapper.java (1)
src/main/java/konkuk/thip/common/util/DateUtil.java (1)
DateUtil(12-85)
src/main/java/konkuk/thip/book/application/service/BookSavedListService.java (1)
src/main/java/konkuk/thip/book/application/service/BookSelectableListService.java (1)
Service(15-40)
src/main/java/konkuk/thip/feed/application/service/FeedCreateService.java (3)
src/main/java/konkuk/thip/feed/domain/value/TagList.java (1)
TagList(11-105)src/main/java/konkuk/thip/feed/adapter/in/web/FeedCommandController.java (1)
Tag(25-115)src/main/java/konkuk/thip/feed/domain/value/ContentList.java (1)
ContentList(13-146)
src/test/java/konkuk/thip/book/adapter/in/web/BookGetSelectableListApiTest.java (2)
src/test/java/konkuk/thip/common/util/TestEntityFactory.java (1)
TestEntityFactory(30-394)src/test/java/konkuk/thip/book/adapter/in/web/BookShowSavedListApiTest.java (1)
SpringBootTest(34-184)
src/test/java/konkuk/thip/room/adapter/in/web/RoomGetHomeJoinedRoomsApiTest.java (1)
src/test/java/konkuk/thip/common/util/TestEntityFactory.java (1)
TestEntityFactory(30-394)
src/main/java/konkuk/thip/book/application/service/BookSelectableListService.java (1)
src/main/java/konkuk/thip/book/application/service/BookSavedListService.java (1)
Service(14-35)
src/test/java/konkuk/thip/book/adapter/in/web/BookShowSavedListApiTest.java (1)
src/test/java/konkuk/thip/common/util/TestEntityFactory.java (1)
TestEntityFactory(30-394)
src/main/java/konkuk/thip/room/adapter/out/persistence/repository/RoomQueryRepositoryImpl.java (1)
src/main/java/konkuk/thip/room/adapter/out/persistence/RoomQueryPersistenceAdapter.java (1)
Repository(21-168)
🔇 Additional comments (39)
src/main/java/konkuk/thip/book/application/port/in/dto/BookPinResult.java (1)
3-8: BookPinResult 매퍼/응답 DTO에서 bookId·publisher 제거 정상 반영됨
BookQueryMapper.toBookPinResult와 RecordPinResponse.of에서 bookId, publisher 접근 없이 bookTitle·authorName·bookImageUrl·isbn만 매핑/반환하도록 구현되어 있어 추가 참조는 없습니다.src/main/java/konkuk/thip/roompost/adapter/in/web/RoomPostQueryController.java (1)
83-87: RecordPinQuery에 @builder 적용 확인, 추가 조치 불필요 RecordPinQuery 레코드에 @builder 어노테이션이 선언되어 있어 빌더 사용에 문제 없으며, pinRecord → BookPinResult → RecordPinResponse.of 매핑도 올바르게 동작합니다.src/main/java/konkuk/thip/roompost/application/port/in/RecordPinUseCase.java (1)
3-8: Swagger 스키마 BookPinResult 반영 여부 확인 필요
서비스 구현체(RecordPinService)와 컨트롤러(RoomPostQueryController) 호출부는 모두 BookPinResult로 정상 변경된 것이 확인되었습니다. OpenAPI/Swagger 스키마(응답 타입 정의 등)도 BookPinResult로 업데이트되었는지 검증해주세요.src/main/java/konkuk/thip/feed/application/service/FeedShowSavedListService.java (1)
33-34: 정렬 기준 변경(createdAt → savedAt) 반영 LGTM저장한 피드 기준으로 커서 페이징하는 방향과 맞습니다.
src/main/java/konkuk/thip/roompost/application/port/in/dto/record/RecordPinQuery.java (2)
3-6: 레코드에 @builder 적용 적절불변 입력 모델에 빌더 추가로 호출 가독성이 좋아졌습니다.
5-12: Lombok 버전(최소 1.18.20) 호환성 확인 필요
Record + @builder 조합은 Lombok 1.18.20 이상에서 지원됩니다. 현재 프로젝트에 설정된 lombok 버전이 이 요구사항을 만족하는지 확인해주세요.src/main/java/konkuk/thip/roompost/adapter/in/web/response/RecordPinResponse.java (1)
11-17: 매핑 일관성 OK
BookPinResult로의 전환과 필드 매핑이 자연스럽습니다.src/main/java/konkuk/thip/roompost/application/service/RecordPinService.java (1)
26-31: 반환 타입 전환 적절읽기 전용 트랜잭션에서 DTO
BookPinResult로 직접 반환하는 방향 전환이 간결합니다.src/main/java/konkuk/thip/book/application/port/in/BookShowSavedListUseCase.java (1)
3-3: 해당 포트-인은 조회 API여서 application 계층이 adapter.in.web.response DTO를 직접 참조하는 CQRS 팀 컨벤션에 부합합니다. 변경 불필요합니다.Likely an incorrect or invalid review comment.
src/main/java/konkuk/thip/book/application/port/in/BookSelectableListUseCase.java (1)
3-3: 포트 레이어가 Web 응답 DTO에 의존합니다.BookSelectableListResponse를 애플리케이션 레이어로 승격하거나, 포트 전용 DTO로 교체해 주세요.
예시:
-import konkuk.thip.book.adapter.in.web.response.BookSelectableListResponse; +import konkuk.thip.book.application.port.in.response.BookSelectableListResponse;애플리케이션 DTO 스켈레톤(새 파일):
package konkuk.thip.book.application.port.in.response; import java.util.List; public record BookSelectableListResponse( List<BookSelectableDto> contents, String nextCursor, boolean hasNext ){ public record BookSelectableDto(Long bookId, String bookTitle, String authorName, String publisher, String bookImageUrl, String isbn){} }⛔ Skipped due to learnings
Learnt from: buzz0331 PR: THIP-TextHip/THIP-Server#78 File: src/main/java/konkuk/thip/user/application/port/out/FollowingQueryPort.java:3-3 Timestamp: 2025-07-14T18:22:56.538Z Learning: THIP 프로젝트에서는 Query API(조회 API)에 한해서는 application 계층에서 adapter.in.web.response 패키지의 response DTO를 직접 참조하는 것을 허용함. 이는 CQRS 아키텍처에서 읽기 전용 작업의 효율성을 위한 팀 컨벤션임.src/main/java/konkuk/thip/book/adapter/out/persistence/repository/BookQueryRepository.java (1)
11-12: LGTM — JOINING 목록은 tie-breaker를 포함합니다.roomPercentage 동일 시 안정적인 순서를 보장하기 위해 lastBookId를 받는 설계가 적절합니다.
src/main/java/konkuk/thip/room/application/mapper/RoomParticipantQueryMapper.java (1)
19-20: 리스트 매핑 시그니처 유지 👍단건 매핑을 기반으로 리스트 변환을 노출하는 것은 명확합니다. 그대로 가셔도 됩니다.
src/main/java/konkuk/thip/common/swagger/SwaggerResponseDescription.java (1)
196-201: FEED 이미지 업로드 에러코드 분리 및 컨트롤러 사용 확인 완료
FEED_CREATE에서 업로드 로직이 FEED_IMAGE_UPLOAD로 분리되었고, 해당 Enum이 presigned-url 엔드포인트에 적용되었으며 INVALID_FILE_EXTENSION 및 FILE_SIZE_OVERFLOW 에러코드도 정의되어 있습니다.src/main/java/konkuk/thip/book/application/port/out/BookQueryPort.java (1)
3-5: 불필요한 변경 없음DTO/커서 타입 임포트 정리 OK.
src/main/java/konkuk/thip/book/adapter/out/persistence/repository/BookJpaRepository.java (1)
10-10: 포맷팅: 쉼표 뒤 공백 추가-public interface BookJpaRepository extends JpaRepository<BookJpaEntity, Long>,BookQueryRepository { +public interface BookJpaRepository extends JpaRepository<BookJpaEntity, Long>, BookQueryRepository {BookQueryRepositoryImpl 구현체도 동일 패키지에서 정상 연결됨을 확인했습니다.
src/main/java/konkuk/thip/book/application/service/BookSavedListService.java (2)
27-33: 커서 일관성/정렬 안정성 점검 요청Cursor 기반 페이징의 정확성을 위해 저장목록 조회 쿼리가 (savedAt DESC, bookId/postId DESC)와 같은 결정적 2차 정렬을 사용하고, nextCursor가 마지막 요소의 키(s)로 생성되는지 확인해 주세요. 중복 timestamp에서 누락/중복 반환이 발생할 수 있습니다.
원하면 저장 쿼리/리포지토리 경로를 점검하는 스크립트를 드리겠습니다.
31-33: LGTM — 매핑/래핑 구조 적절contents → DTO 매핑 후 nextCursor/isLast를 그대로 전달하는 구조가 명확합니다.
src/main/java/konkuk/thip/common/exception/code/ErrorCode.java (1)
175-176: 이미지 오류 코드 재할당의 호환성 확인170001/170002 의미 변경으로 외부/모바일 클라이언트가 구 코드(170003 등)에 의존 중이면 회귀 위험이 있습니다. 변경 공지 및 하위호환(구 코드 ⇒ 신 코드 매핑) 필요 여부 확인을 부탁드립니다.
src/main/java/konkuk/thip/room/application/port/out/dto/RoomParticipantQueryDto.java (1)
9-19: QueryDSL/Lombok 설정 호환성 확인record의 canonical ctor에 @QueryProjection, 동시에 @builder 사용 시 apt/플러그인 버전 의존성이 있습니다. 현재 빌드가 QueryDSL 5.x(+ querydsl-apt), Lombok 1.18.24+를 사용 중인지 확인 부탁드립니다.
원하면 빌드 스크립트에서 버전을 확인하는 점검 스크립트를 제공하겠습니다.
src/test/java/konkuk/thip/feed/adapter/in/web/FeedCreateApiTest.java (1)
192-197: LGTM — 다중 이미지 Content 매핑 검증 적절정확히 2개의 URL이 contentList로 저장되는 것을 검증하고 있어 회귀 방지에 유효합니다.
src/main/java/konkuk/thip/book/application/port/out/dto/BookQueryDto.java (1)
20-45: QueryDSL APT 설정 및 Q 타입 생성 확인 완료
build.gradle에com.querydsl:querydsl-apt:5.0.0:jakarta가 annotationProcessor로 설정되어 있으며,QBookQueryDto가 정상 생성되어 사용되고 있음을 확인했습니다.src/main/java/konkuk/thip/book/adapter/out/persistence/BookQueryPersistenceAdapter.java (1)
7-9: 필요한 의존성 추가는 적절합니다.Cursor 기반 페이지네이션 전환과 DTO 반환을 위한 import 추가, OK.
Also applies to: 14-14
src/test/java/konkuk/thip/book/adapter/in/web/BookShowSavedListApiTest.java (2)
48-57: 테스트 격리(teardown) 순서 적절, OK.FK 제약 고려한 삭제 순서가 안전합니다.
60-106: 정렬·필드 검증 시나리오 충실, OK.DB 타임스탬프를 고정해 최신순을 검증하는 접근이 현실적입니다.
src/main/java/konkuk/thip/book/adapter/in/web/BookQueryController.java (1)
111-116: 커서 파라미터 도입 및 위임 흐름, OK.응답을 UseCase 결과로 감싸는 컨트롤러 책임이 명확합니다.
src/main/java/konkuk/thip/book/adapter/out/persistence/repository/BookQueryRepositoryImpl.java (1)
112-114: pageSize+1 페치 전략 사용, OK.hasNext 계산을 위한 초과 조회 패턴이 적절합니다.
src/main/java/konkuk/thip/feed/adapter/in/web/FeedCommandController.java (1)
45-49: 멀티파트 의존 제거 및 JSON 전환, OK.이미지 업로드를 presigned URL 플로우로 분리한 설계가 명확합니다.
src/main/java/konkuk/thip/feed/adapter/out/s3/S3Service.java (1)
23-26: (회귀 위험 점검) 삭제 로직 호출 위치 미발견 — 수작업 검증 필요
deleteImageFromS3및deleteObject호출이 검색되지 않습니다. 피드/이미지 삭제 플로우에서 S3 객체 정리 로직 누락 여부 직접 점검해주세요.src/main/java/konkuk/thip/book/application/mapper/BookQueryMapper.java (2)
36-37: BookPinResult 매핑 변경 LGTM필요 필드만 매핑하여 단순화한 점 좋습니다.
38-46: DTO 필드 명 일치 여부 확인(자동 매핑 의존 구간)toBookShowSavedDto/SelectableDto는 대부분 자동 매핑에 의존합니다. 대상 DTO에 publisher, bookId 등 속성명이 BookQueryDto와 정확히 일치하는지 확인해 주세요. 일치하지 않으면 null이 나옵니다.
원하시면 누락 필드만 @mapping으로 명시하도록 패치 제안 드리겠습니다.
src/test/java/konkuk/thip/book/adapter/in/web/BookGetSelectableListApiTest.java (3)
55-62: 테스트 데이터 정리 순서 LGTM연관관계 역순 정리 및 InBatch 사용으로 테스트 격리가 잘 보장됩니다.
168-245: 동일 시나리오에 두 엔드포인트 혼용(selectable-list vs saved)본 클래스에서 SAVED 시나리오가 /books/selectable-list?type=SAVED 와 /books/saved 두 방식으로 테스트됩니다. 의도된 이중 지원이면 OK이나, 하나만 유지한다면 테스트를 통일해 주세요(중복 유지 비용 절감).
원하시면 하나로 정리하는 변경(diff)도 제안 가능합니다.
340-396: JOINING 커서 페이징/정렬 검증 케이스 풍부해진 점 좋습니다진행도 DESC, ISBN 기반 tie-break, nextCursor 플로우까지 잘 커버합니다.
src/main/java/konkuk/thip/room/adapter/out/persistence/repository/RoomQueryRepositoryImpl.java (6)
17-20: QRoomParticipantQueryDto/RoomParticipantQueryDto 도입 적절홈 참여방 조회 전용 DTO/프로젝션 추가가 쿼리 비용과 응답 스키마를 잘 맞춥니다. 다운스트림 매퍼/서비스와의 시그니처 정합성도 좋아 보입니다.
28-29: ACTIVE 정적 임포트로 상태 필터 일관화상태값 비교를 도메인 전반에 통일하여 가독성과 오류 가능성을 줄였습니다.
Also applies to: 43-43
201-207: 홈-참여 방 커서 메서드 추가 LGTM정렬 키(userPercentage DESC, startDate ASC, roomId ASC)와 파라미터 커서 구성이 서비스 레이어(Cursor 생성)와 일치합니다.
246-250: 정렬 키와 커서 조건의 일치 확인정렬과 커서 비교 부등호가 정확히 매칭됩니다(DESC → <, ASC → >). 다음 페이지 판별을 위한 limit(pageSize + 1)도 적절합니다.
260-262: 모집중 참여방: ACTIVE 필터 보강 OK참여/방 상태를 모두 ACTIVE로 제한해 불필요한 결과를 배제합니다.
278-282: 진행중 참여방: ACTIVE 필터 보강 OK동일한 상태 제약 반영으로 일관성 좋습니다.
src/main/java/konkuk/thip/book/adapter/out/persistence/BookQueryPersistenceAdapter.java
Show resolved
Hide resolved
src/main/java/konkuk/thip/book/adapter/out/persistence/BookQueryPersistenceAdapter.java
Show resolved
Hide resolved
src/main/java/konkuk/thip/book/adapter/out/persistence/repository/BookQueryRepository.java
Show resolved
Hide resolved
src/main/java/konkuk/thip/book/adapter/out/persistence/repository/BookQueryRepositoryImpl.java
Show resolved
Hide resolved
src/main/java/konkuk/thip/book/adapter/out/persistence/repository/BookQueryRepositoryImpl.java
Show resolved
Hide resolved
src/main/java/konkuk/thip/room/application/mapper/RoomParticipantQueryMapper.java
Show resolved
Hide resolved
| import static konkuk.thip.feed.domain.value.Tag.*; | ||
| import static org.assertj.core.api.Assertions.assertThat; | ||
| import static org.springframework.test.web.servlet.request.MockMvcRequestBuilders.multipart; | ||
| import static org.springframework.test.web.servlet.request.MockMvcRequestBuilders.post; |
There was a problem hiding this comment.
🛠️ Refactor suggestion
음수 케이스 추가 권장: 허용 도메인 외 URL 거부
클라이언트가 임의 외부 URL을 보낼 수 없도록 서버에서 CDN 도메인/키 패턴을 검증해야 합니다. 이를 보장하는 통합 테스트(400 + IMAGE_URL_DOMAIN_NOT_ALLOWED 등)를 추가해 주세요.
원하시면 테스트 메서드 골격을 작성해 드립니다.
🤖 Prompt for AI Agents
In src/test/java/konkuk/thip/feed/adapter/in/web/FeedCreateApiTest.java around
line 31, add a negative integration test that submits a feed creation request
containing an image URL from a disallowed/external domain and assert the
response is HTTP 400 with the application error code
IMAGE_URL_DOMAIN_NOT_ALLOWED; implement the test using MockMvc POST to the same
endpoint as existing tests, construct a request body with an image URL outside
the allowed CDN domain/pattern, perform the request, and assert
status().isBadRequest() and that the JSON error code equals
"IMAGE_URL_DOMAIN_NOT_ALLOWED"; ensure necessary test fixtures and any
authorization headers used by other tests are included so the test runs in the
same context.
There was a problem hiding this comment.
@buzz0331 네, 허용되지 않는 도메인의 이미지 URL을 사용한 음수 케이스 테스트 골격을 작성해 드리겠습니다:
@Test
@DisplayName("허용되지 않는 도메인의 이미지 URL로 피드 생성 시, IMAGE_URL_DOMAIN_NOT_ALLOWED 에러가 발생한다.")
void createFeedWithDisallowedDomainImageUrl_shouldReturnImageUrlDomainNotAllowedError() throws Exception {
// given
bookJpaRepository.save(TestEntityFactory.createBookWithISBN("9788954682152"));
Map<String, Object> request = new HashMap<>();
request.put("isbn", "9788954682152");
request.put("contentBody", "외부 도메인 이미지 URL 테스트");
request.put("isPublic", true);
request.put("tagList", List.of(KOREAN_NOVEL.getValue()));
request.put("imageUrls", List.of(
"https://external-malicious-site.com/image1.jpg",
"https://another-external-domain.net/image2.png"
));
// when
ResultActions result = mockMvc.perform(post("/feeds")
.contentType(MediaType.APPLICATION_JSON)
.requestAttr("userId", user.getUserId())
.content(objectMapper.writeValueAsString(request)));
// then
result.andExpect(status().isBadRequest())
.andExpected(jsonPath("$.errorCode").value("IMAGE_URL_DOMAIN_NOT_ALLOWED"))
.andExpected(jsonPath("$.message").exists());
}이 테스트는 허용되지 않은 외부 도메인의 이미지 URL을 포함한 요청을 보내고, 서버가 적절히 거부하는지 검증합니다. 실제 에러 코드명이나 JSON 응답 구조는 프로젝트의 에러 응답 포맷에 맞게 조정해 주세요.
src/test/java/konkuk/thip/feed/adapter/in/web/FeedCreateControllerTest.java
Show resolved
Hide resolved
…resigned-url # Conflicts: # src/main/java/konkuk/thip/book/adapter/out/persistence/BookQueryPersistenceAdapter.java
seongjunnoh
left a comment
seongjunnoh
left a comment
There was a problem hiding this comment.
고생하셨습니다!! 노션에 문서화도 자세히 작성해주셔서 처음보는 기술을 좀 더 수월하게 이해할 수 있었습니다!
리뷰 조금 달아보았는데 확인부탁드립니다!!
| // 업로드 성공시에 S3 파일의 public URL 반환 | ||
| return amazonS3.getUrl(bucket, s3FileName).toString(); | ||
| } | ||
| // url 유효기간 설정하기(1시간) |
There was a problem hiding this comment.
p3 : 이거 구현된 코드에 의하면 5분인데 오타일까요?
There was a problem hiding this comment.
엇 넵 오타맞습니다 첨엔 1시간으로했다가 넘긴거같아서 줄였습니다 ㅎ
| // 3. Feed 생성 및 저장 | ||
| Feed feed = Feed.withoutId( | ||
| command.contentBody(), | ||
| command.userId(), | ||
| command.isPublic(), | ||
| targetBookId, | ||
| command.tagList(), | ||
| command.imageUrls() | ||
| ); | ||
| return feedCommandPort.save(feed); |
There was a problem hiding this comment.
request로 url 들을 받아오니 피드 생성 프로덕션 코드도 훨씬 간단해지네요 굳굳
|
|
||
| @Override | ||
| @Transactional | ||
| //TODO 추후 이벤트 기반으로 트랜잭션 커밋후 S3 삭제하도록 리펙토링 or 사용하지 않는 이미지 배치 삭제방식 논의 |
There was a problem hiding this comment.
너무 좋은 리펙 포인트인 것 같습니다!! 추후에 성능 개선을 위해 동기 -> 이벤트 드리븐으로 리펙할 포인트를 한번 쫙 정리해보면 좋을 것 같습니다!
| @Schema(description = "업로드 후 접근 가능한 파일 URL") | ||
| String fileUrl |
There was a problem hiding this comment.
p3 : 응답에 업로드 이후 접근 가능한 파일 url을 포함하신 이유가 있을까요??
FE에게 fileUrl 을 응답한 후, 다시 피드 생성 요청시에 fileUrl을 request body로 받는 것 보다는, FE에게 presignedUrl과 key(= presignedUrl을 생성할 때 사용한 key) 를 응답하고, 피드 생성 요청시에 key를 request body로 받아 내부적으로 key에 해당하는 fileUrl을 DB에 저장하는 플로우는 어떤가 싶습니다!!
현재 구현된 코드에서는 피드 생성 request 에 포함된 fileUrl 을 검증하는 부분이 누락되어 있는데, 위 방식으로 수정되면 내부에서 request 에 포함된 key에 해당하는 fileUrl 을 획득하는 과정에서 key 값의 검증과정이 포함되므로 더 안전할 것 같습니다!!
제가 생각한 피드 생성 플로우를 간략하게 추가해보았습니다!
[1] FE → BE: presign 요청(filename, size, type)
[2] BE → FE: { key, presignedPutUrl }
[3] FE → S3: PUT 업로드
[4] FE → BE: 피드 생성(content, imageKeys=[key1, key2, ...])
[5] BE: 각 key가 이 사용자 prefix인지 검증 + (선택) S3 HEAD로 존재/사이즈 확인
[6] BE: DB에는 s3Key 저장 (전체 cdnUrl을 저장하는게 아니라)
[7] 피드 조회 시 BE가 cdnUrl = cdnBase + key 로 조립해서 내려줌 -> 이러면 추후에 cdnBase 값이 수정되더라도 유연하게 대처 가능
There was a problem hiding this comment.
굿굿 깔끔하게 정리해주셨네요! 성준님 말대로 접근 가능한 파일 url이 노출될 필요는 없어보이네요!
그리고 앞 리뷰에서 key를 구조화 시키면 성준님이 작성하신 것 중 5번에 해당하는 사용자 prefix(userId) 검증도 가능할 것 같습니다!
| // UUID 앞 10자리 + 정리된 파일명으로 객체 key 생성 | ||
| String key = UUID.randomUUID().toString().substring(0, 10) + sanitizedFilename; |
There was a problem hiding this comment.
p3 : filename = key 값을 UUID + '한글->영문 으로 변환한 파일명' 으로 설정한 이유가 있으실까요??
좀 찾아보니 보통 실무에서는 presigned url을 생성하는 key 값에 파일명과 같은 인코딩 에러 위험성이 있는 문자열을 포함하기 보다는 "/uploads/{userId}/{yyyyMM}/{UUID}.{ext} 와 같이 운영 친화적인 구조 + UUID 의 형식으로 key를 구성하는 것 같습니다.
위와 같이 key의 prefix 로 userId/yyyyMM 등을 붙이면 추후에
- 사용자별로 업로드한 파일을 정리 가능
- s3 관련 비용/과금 모니터링 가능
- 접근 정책 제어(ex. 특정 userId의 s3 업로드 제한) 가능
과 같은 이점이 있다고 합니다!
유저가 업로드한 파일명을 저장해야하는 이슈가 있다면, 현재 피드와 매핑되어 있는 attribute converter 인 ContentList 가 내부에 List 으로 첨부파일의 url 값만을 가지는게 아니라, "List + Content 내부에 첨부파일의 메타데이터와 관련된 필드 추가" 하는 방식으로 리펙토링을 진행하는게 더 좋지 않나 라고 생각합니다!!
@hd0rable @buzz0331 어떻게 생각하시나요?
There was a problem hiding this comment.
오 저도 코드보고 key를 디렉토리화 시키면 좋지 않을까 해서 리뷰를 달러 왔는데 성준님이 한발 빠르셨네요 하핳
저도 key를 구조화시키자는 성준님 의견에 적극 동의합니다. 추후에 악성 사용자가 이상한 이미지를 업로드할때 추적에도 용이할 것 같다는 생각입니다.
추가적으로, ContentList를 List에 대한 일급 컬렉션으로 리팩토링 하는 것에 대해서도 동의합니다. 원래 저희가 converter를 적용했을 때 이야기가 나왔던 얘기인데 메타데이터가 필요하다면 그렇게 추가되는게 맞을 것 같아요.
There was a problem hiding this comment.
앗 저도 key 구조화에 대해서 생각해봤는데 이미 저장된 s3버킷에 대해서 같이 처리를해야하다보니 분기문처리가 필요할것같아서 일단은 기존 key생성방식대로 key를 생성하긴합니다.
기존 버킷의 키를 바꿔도 버킷의 퍼블릭 url이 변경되지않을까요..?
이미지 업로드 방식이 수정되면서 db에 이미 저장되어있는 url도 바뀌어야할텐데 그렇다면 실제 배포중인 서비스를 몇시간정도 중지시켜놓고 한번에 작업해야겠네요.
근데 혹시 Content 내부에 첨부파일의 메타데이터는 왜 필요한걸까요??
There was a problem hiding this comment.
앗 저도 key 구조화에 대해서 생각해봤는데 이미 저장된 s3버킷에 대해서 같이 처리를해야하다보니 분기문처리가 필요할것같아서 일단은 기존 key생성방식대로 key를 생성하긴합니다. 기존 버킷의 키를 바꿔도 버킷의 퍼블릭 url이 변경되지않을까요..? 이미지 업로드 방식이 수정되면서 db에 이미 저장되어있는 url도 바뀌어야할텐데 그렇다면 실제 배포중인 서비스를 몇시간정도 중지시켜놓고 한번에 작업해야겠네요.
답글달아주신 내용처럼 key 를 디렉토리 구조화하여도 변경이 발생하면 기존 s3에 올라가있는 파일들의 key 값들의 수정이 필요하긴 하겠네요!
저는 파일들의 관리측면에서 key 의 구조화가 더 유용하지 않을까라고 생각해서 말씀드린 거긴 합니다!!
근데 혹시 Content 내부에 첨부파일의 메타데이터는 왜 필요한걸까요??
key값에 파일명을 넣기보다는, 파일명을 기록해야하는 경우가 있으면 key값에 넣기보다는 Content 내부에 첨부파일의 메타데이터를 넣는 방식이 더 좋지않나? 라는 생각에 코멘트 달아보았습니다!
buzz0331
left a comment
buzz0331
left a comment
There was a problem hiding this comment.
전반적으로 복잡한 작업이였는데 역시 고수 ㄷㄷ
문서까지 잘 정리해주셔서 리뷰하기 수월했습니다~~ 👍🏻
| return BaseResponse.ok(FeedIdResponse.of(feedCreateUseCase.createFeed(request.toCommand(userId)))); | ||
| } | ||
|
|
||
| @Operation( | ||
| summary = "피드 생성시 이미지 업로드용 presigned url 발급 요청", | ||
| description = "S3에 프론트엔드가 직접 이미지를 업로드하기위한 presigned url를 발급 받습니다." | ||
| ) | ||
| @ExceptionDescription(FEED_IMAGE_UPLOAD) | ||
| @PostMapping("/feeds/images/presigned-url") | ||
| public BaseResponse<FeedUploadImagePresignedUrlResponse> getPresignedUrls(@RequestBody List<FeedUploadImagePresignedUrlRequest> request) { | ||
| return BaseResponse.ok(s3Service.getPresignedUrl(request)); | ||
| } |
There was a problem hiding this comment.
비즈니스 로직이 들어가는게 맞을까 조금 조심스럽긴 하지만, 여기 presigned url을 발급하는 api에서부터 이미지 개수 제한을 검증하는 것 어떨까요? api 응답 속도와 고아 파일 방지 등의 효과가 있을 것 같아서 제안드려봅니다! @seongjunnoh @hd0rable 두 분 의견 궁금합니다!
There was a problem hiding this comment.
괜찮은것 같습니다!! 이미지 개수 제한을 여기서 해야 프엔이 4개 초과의 이미지를 업로드하지않을것같네요
| // UUID 앞 10자리 + 정리된 파일명으로 객체 key 생성 | ||
| String key = UUID.randomUUID().toString().substring(0, 10) + sanitizedFilename; |
There was a problem hiding this comment.
오 저도 코드보고 key를 디렉토리화 시키면 좋지 않을까 해서 리뷰를 달러 왔는데 성준님이 한발 빠르셨네요 하핳
저도 key를 구조화시키자는 성준님 의견에 적극 동의합니다. 추후에 악성 사용자가 이상한 이미지를 업로드할때 추적에도 용이할 것 같다는 생각입니다.
추가적으로, ContentList를 List에 대한 일급 컬렉션으로 리팩토링 하는 것에 대해서도 동의합니다. 원래 저희가 converter를 적용했을 때 이야기가 나왔던 얘기인데 메타데이터가 필요하다면 그렇게 추가되는게 맞을 것 같아요.
| TagList.validateTags(Tag.fromList(command.tagList())); | ||
| // todo 나중에 presignedURL로 바꾸면 ContentList.of로 변경 | ||
| ContentList.validateImageCount(images != null ? images.size() : 0); | ||
| ContentList.validateImageCount(ContentList.of(command.imageUrls()).size()); |
There was a problem hiding this comment.
이거 저번에 성준님이랑 얘기 나눴었던 것 같은데, 일급 컬렉션이니만큼 이렇게 정적 메서드를 뚫어서 유효성 검증을 하는게 아닌 객체가 생성될 때, 유효성 검증을 하는 것이 조금더 객체 지향적이라고 생각하는데 어떻게 생각하시나요? @seongjunnoh @hd0rable @coderabbitai
다만, 여기서 정적 메서드로 먼저 유효성 검증을 한 이유는 기존 코드에서는 이미지 업로드가 뒤쪽에서 이루어졌기 때문에 유효성 검증을 앞쪽에서 해줘야 고아 파일이 생기는 것을 방지할 수 있었기 때문인데, 이제는 이미지가 업로드가 되어서 피드 생성 요청이 들어오는 것이니 다시 리팩토링을 진행해도 될 것 같습니다!
There was a problem hiding this comment.
🧩 Analysis chain
@buzz0331 완전히 동의합니다! 현재 코드가 일급 컬렉션의 장점을 제대로 활용하지 못하고 있네요.
현재 방식의 문제점:
ContentList.validateImageCount(ContentList.of(command.imageUrls()).size());이 코드는 객체를 생성하고 → 데이터를 꺼내서 → 외부에서 검증하는 구조로, 캡슐화를 깨뜨리고 있습니다.
개선 방향으로는:
-
생성자/팩토리 메서드에서 검증:
// FeedCreateService에서는 단순히 ContentList contentList = ContentList.of(command.imageUrls()); // ContentList 내부에서 검증 수행 public static ContentList of(List<String> imageUrls) { validateImageCount(imageUrls.size()); return new ContentList(imageUrls); }
-
Tell, Don't Ask 원칙 준수: 객체에게 검증을 요청하는 것이 아니라, 객체가 스스로 유효성을 보장하도록 하는 것이 좋겠습니다.
-
도메인 로직 응집도 향상: 이미지 개수 제한이라는 비즈니스 룰이 ContentList 안에 캡슐화되어 더 응집도 높은 설계가 됩니다.
말씀하신 대로 이제는 presigned URL로 인해 고아 파일 문제가 해결되었으니, 이런 리팩토링을 진행하기 좋은 타이밍인 것 같습니다!
🏁 Script executed:
#!/bin/bash
# ContentList 클래스의 현재 구현을 확인해보겠습니다
find . -name "ContentList.java" -type f | head -5 | xargs cat
Length of output: 3812
@buzz0331 ContentList 구현을 확인해보니, 말씀하신 문제가 더욱 명확해지네요!
현재 상황 분석:
ContentList는 이미 생성자에서 validate() 메서드를 통해 내부적으로 검증하고 있습니다:
private ContentList(List<String> values, int maxSize) {
this.maxSize = maxSize;
this.contents = new ArrayList<>(values == null ? List.of() : values);
validate(); // 이미 여기서 검증!
}
private void validate() {
validateImageCount(contents.size()); // 내부적으로 같은 검증 수행
}문제점:
현재 FeedCreateService에서는 중복 검증이 일어나고 있습니다:
// 1. ContentList.of()에서 내부적으로 검증 수행
// 2. 다시 외부에서 static 메서드로 검증 수행 (불필요!)
ContentList.validateImageCount(ContentList.of(command.imageUrls()).size());개선 방안:
-
즉시 적용 가능한 개선:
// FeedCreateService에서 단순히 ContentList contentList = ContentList.of(command.imageUrls()); // 이미 생성자에서 검증이 완료되므로 추가 검증 불필요
-
더 나은 설계:
public static void validateImageCount(int size)메서드를 제거- 모든 검증 로직을 ContentList 내부에서만 관리
- 외부에서는 객체 생성만 하고, 실패시 예외가 발생하는 것을 처리
이렇게 하면 진정한 일급 컬렉션의 장점을 살릴 수 있고, Tell Don't Ask 원칙도 준수할 수 있겠네요! 🎯
There was a problem hiding this comment.
Seems like the humans are having a chat. I'll hop back into my burrow for now. If you need me again, just tag @coderabbitai in a new comment, and I'll come hopping out!
There was a problem hiding this comment.
아 이 부분은 전에 말씀주신것처럼 book 검증 전에 ContentList 관련 validate 를 수행하려고 createFeed 메서드 내부에서 유효성 검증 -> Feed 생성 이런식으로 메서드를 호출하는 구조를 유지하셨군요!! 확인했습니다!
|
|
||
| @Override | ||
| @Transactional | ||
| //TODO 추후 이벤트 기반으로 트랜잭션 커밋후 S3 삭제하도록 리펙토링 or 사용하지 않는 이미지 배치 삭제방식 논의 |
| import static konkuk.thip.feed.domain.value.Tag.*; | ||
| import static org.assertj.core.api.Assertions.assertThat; | ||
| import static org.springframework.test.web.servlet.request.MockMvcRequestBuilders.multipart; | ||
| import static org.springframework.test.web.servlet.request.MockMvcRequestBuilders.post; |
| public record PresignedUrlInfo( | ||
|
|
||
| @Schema(description = "PUT 요청을 보내기 위한 파일 업로드용 Presigned URL") | ||
| String preSignedUrl, |
There was a problem hiding this comment.
p3: aws 공식 문서에서는 PresignedUrl이 공식적인 카멜 케이스라고 하네요~
There was a problem hiding this comment.
카멜케이스인데첫번째글짜가 대문자인가여?? 신기하네여 수정할게ㅔ욧
There was a problem hiding this comment.
잉? 아 presignedUrl을 말씀드린겁니다! 맨 처음 대문자는 공식 용어라는 얘기 헣,, 헷갈리게 해서 죄송합니다
| @Schema(description = "업로드 후 접근 가능한 파일 URL") | ||
| String fileUrl |
There was a problem hiding this comment.
굿굿 깔끔하게 정리해주셨네요! 성준님 말대로 접근 가능한 파일 url이 노출될 필요는 없어보이네요!
그리고 앞 리뷰에서 key를 구조화 시키면 성준님이 작성하신 것 중 5번에 해당하는 사용자 prefix(userId) 검증도 가능할 것 같습니다!
…resigned-url # Conflicts: # build.gradle
There was a problem hiding this comment.
Actionable comments posted: 2
♻️ Duplicate comments (2)
src/main/java/konkuk/thip/common/exception/code/ErrorCode.java (1)
175-178: Presigned URL 전용 에러 코드 추가 제안(재언급)운영 이슈 분류를 위해 발급 실패/만료/메타데이터 불일치 등 전용 코드를 두면 관측과 디버깅이 수월합니다. 이전 코멘트와 동일 제안입니다.
예시 추가안:
URL_USER_ID_MISMATCH(HttpStatus.BAD_REQUEST, 170004, "URL의 사용자 ID가 요청 사용자와 일치하지 않습니다."), + PRESIGNED_URL_GENERATION_FAILED(HttpStatus.INTERNAL_SERVER_ERROR, 170005, "Presigned URL 발급에 실패했습니다."), + PRESIGNED_URL_EXPIRED(HttpStatus.BAD_REQUEST, 170006, "Presigned URL이 만료되었습니다."), + INVALID_IMAGE_UPLOAD_METADATA(HttpStatus.BAD_REQUEST, 170007, "업로드 메타데이터(Content-Type/Content-MD5)가 유효하지 않습니다."),원하시면 해당 코드 사용 위치(서비스/컨트롤러)까지 포함한 변경 PR 초안도 만들어드리겠습니다.
src/main/java/konkuk/thip/feed/adapter/in/web/FeedCommandController.java (1)
45-48: JSON 본문 + 사용자 컨텍스트 전달 전환 LGTM멀티파트 제거하고
@RequestBody @Valid+@UserId로 전환한 부분 좋습니다. 이전 코멘트에서 제안한 방향이 반영되었습니다.
🧹 Nitpick comments (10)
src/main/java/konkuk/thip/common/swagger/SwaggerResponseDescription.java (1)
199-203: FEED_IMAGE_UPLOAD에 환경설정 누락 에러도 포함 권장CDN(CloudFront) 베이스 URL/버킷 베이스 URL 미설정 시 Presigned URL 발급 응답 구성 자체가 불가할 수 있으므로 문서화 차원에서 FEED_IMAGE_UPLOAD 섹션에 포함을 권장합니다.
적용 예시:
- FEED_IMAGE_UPLOAD(new LinkedHashSet<>(Set.of( - INVALID_FILE_EXTENSION, - FILE_SIZE_OVERFLOW, - CONTENT_LIST_SIZE_OVERFLOW - ))), + FEED_IMAGE_UPLOAD(new LinkedHashSet<>(Set.of( + INVALID_FILE_EXTENSION, + FILE_SIZE_OVERFLOW, + CONTENT_LIST_SIZE_OVERFLOW, + AWS_BUCKET_BASE_URL_NOT_CONFIGURED + ))),src/main/java/konkuk/thip/common/s3/service/ImageUrlValidationService.java (1)
13-18: @requiredargsconstructor 불필요 및 설정 주입 방식 개선 제안현재 final 필드가 없고 @value 필드만 존재하여 @requiredargsconstructor는 효과가 없습니다. 또한 URL/도메인 설정은 @ConfigurationProperties로 묶어 타입 세이프하게 주입하는 편이 유지보수에 유리합니다.
예시(요약):
- cloud.aws.s3.cloud-front-base-url를 S3Properties로 바인딩
- 본 서비스는 S3Properties를 생성자 주입
src/test/java/konkuk/thip/feed/adapter/in/web/FeedCreateControllerTest.java (1)
140-148: 체이닝으로 간결화 가능(선택)
ResultActions result없이 바로 체이닝하면 가독성이 조금 좋아집니다.- ResultActions result = mockMvc.perform(post("/feeds") - .contentType(MediaType.APPLICATION_JSON) - .content(objectMapper.writeValueAsBytes(req)) - .requestAttr("userId", 1L) - ); - - result.andExpect(status().isBadRequest()) + mockMvc.perform(post("/feeds") + .contentType(MediaType.APPLICATION_JSON) + .content(objectMapper.writeValueAsBytes(req)) + .requestAttr("userId", 1L)) + .andExpect(status().isBadRequest()) .andExpect(jsonPath("$.code").value(CONTENT_LIST_SIZE_OVERFLOW.getCode())) .andExpect(jsonPath("$.message",containsString(CONTENT_LIST_SIZE_OVERFLOW.getMessage())));src/main/java/konkuk/thip/feed/application/service/FeedCreateService.java (4)
35-37: 중복 검증 제거 + NPE 방지: ContentList 한 번만 생성해 검증·값 재사용하세요현재
ContentList.of(...)생성 시 내부에서 이미 개수 검증이 수행되는데, 직후에validateImageCount(...)를 다시 호출해 중복 검증이 발생합니다. 또한command.imageUrls()가 null일 경우imageUrlValidationService에서 NPE가 날 수 있습니다. 아래처럼 한 번만 생성·검증하고, 동일 값을 아래 Feed 생성에도 재사용하면 깔끔합니다.- ContentList.validateImageCount(ContentList.of(command.imageUrls()).size()); - // 1-1. 서명된 url 검증 - imageUrlValidationService.validateUrlDomainAndUser(command.imageUrls(),command.userId()); + // 이미지 URL 검증 및 정규화(개수 검증 포함) + final var imageUrls = ContentList.of(command.imageUrls()).toUnmodifiableList(); + // 1-1. 서명된 url 검증 + imageUrlValidationService.validateUrlDomainAndUser(imageUrls, command.userId()); @@ - Feed feed = Feed.withoutId( + Feed feed = Feed.withoutId( command.contentBody(), command.userId(), command.isPublic(), targetBookId, command.tagList(), - command.imageUrls() + imageUrls );추가로, 위 변경 시
import java.util.List;가 필요하면 상단 import에 추가해주세요.Also applies to: 43-51
34-34: 태그 검증도 도메인 일급 컬렉션 생성 시 수행하도록 일관화 제안
TagList.validateTags(...)대신TagList.of(...)생성에 검증을 위임하면 캡슐화와 일관성이 좋아집니다(이미지와 동일 패턴). 도메인에서 최종 검증이 이뤄지도록 정리해보세요.
30-31: 외부 API 호출을 트랜잭션 경계 밖으로 이동 고려
createFeed내에서(같은 트랜잭션에서) Naver API 호출이 발생하면 트랜잭션 길이가 불필요하게 늘어납니다. 사전 조회/캐시 후 트랜잭션 시작, 혹은 트랜잭션 분리(REQUIRES_NEW/이벤트)로 IO 지연이 DB 락에 영향을 주지 않도록 하는 것을 권장합니다.
66-66: 메서드명 오타(nit):saveNewBookWithFromExternalApi→saveNewBookFromExternalApi가독성을 위해 메서드명을 정리해 주세요.
src/main/java/konkuk/thip/feed/adapter/in/web/FeedCommandController.java (3)
56-60: 컬렉션 요소 검증 확실화 및 업로드 개수 제한 선제 검증 권장
- Bean Validation에서 리스트 요소까지 확실히 검증하도록 컨테이너 요소에
@Valid부여를 권장합니다.- 프리사인 발급 단계에서 요청 개수(및 총 업로드 예정 개수)가 도메인 상한(예: 3장)을 넘지 않도록 선제 검증하면 고아 업로드를 줄일 수 있습니다.
- public BaseResponse<FeedUploadImagePresignedUrlResponse> getPresignedUrls( - @RequestBody @Valid final List<FeedUploadImagePresignedUrlRequest> request, + public BaseResponse<FeedUploadImagePresignedUrlResponse> getPresignedUrls( + @RequestBody final List<@Valid FeedUploadImagePresignedUrlRequest> request, @Parameter(hidden = true) @UserId final Long userId) { return BaseResponse.ok(s3Service.getPresignedUrl(request,userId)); }가능하면
s3Service.getPresignedUrl(...)내부에서 이미지 총량(개수/용량)도 제한해 주세요.
36-37: 레이어드 아키텍처 정합성: Controller → Adapter(out) 직접 의존 회피
adapter.in(컨트롤러)에서adapter.out구현체(S3Service)에 직접 의존하면 의존성 역전이 깨집니다.PresignedUrlUseCase같은 애플리케이션 포트를 신설하고, 어댑터에서 구현하도록 분리하는 것을 권장합니다.
51-53: Swagger 문구 타이포 및 띄어쓰기 수정 제안표기 통일과 맞춤법 개선입니다.
- summary = "피드 생성시 이미지 업로드용 presigned url 발급 요청", - description = "S3에 프론트엔드가 직접 이미지를 업로드하기위한 presigned url를 발급 받습니다." + summary = "피드 생성 시 이미지 업로드용 Presigned URL 발급 요청", + description = "S3에 클라이언트가 직접 이미지를 업로드하기 위한 Presigned URL을 발급받습니다."
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (12)
build.gradle(1 hunks)src/main/java/konkuk/thip/common/exception/code/ErrorCode.java(1 hunks)src/main/java/konkuk/thip/common/s3/service/ImageUrlValidationService.java(1 hunks)src/main/java/konkuk/thip/common/swagger/SwaggerResponseDescription.java(1 hunks)src/main/java/konkuk/thip/feed/adapter/in/web/FeedCommandController.java(2 hunks)src/main/java/konkuk/thip/feed/adapter/in/web/request/FeedUploadImagePresignedUrlRequest.java(1 hunks)src/main/java/konkuk/thip/feed/adapter/in/web/response/FeedUploadImagePresignedUrlResponse.java(1 hunks)src/main/java/konkuk/thip/feed/adapter/out/s3/S3Service.java(1 hunks)src/main/java/konkuk/thip/feed/application/service/FeedCreateService.java(1 hunks)src/main/java/konkuk/thip/feed/domain/Feed.java(0 hunks)src/test/java/konkuk/thip/feed/adapter/in/web/FeedCreateApiTest.java(9 hunks)src/test/java/konkuk/thip/feed/adapter/in/web/FeedCreateControllerTest.java(5 hunks)
💤 Files with no reviewable changes (1)
- src/main/java/konkuk/thip/feed/domain/Feed.java
🚧 Files skipped from review as they are similar to previous changes (4)
- src/main/java/konkuk/thip/feed/adapter/in/web/request/FeedUploadImagePresignedUrlRequest.java
- src/test/java/konkuk/thip/feed/adapter/in/web/FeedCreateApiTest.java
- src/main/java/konkuk/thip/feed/adapter/out/s3/S3Service.java
- src/main/java/konkuk/thip/feed/adapter/in/web/response/FeedUploadImagePresignedUrlResponse.java
🧰 Additional context used
🧬 Code graph analysis (1)
src/main/java/konkuk/thip/feed/application/service/FeedCreateService.java (4)
src/main/java/konkuk/thip/feed/adapter/in/web/FeedCommandController.java (1)
Tag(25-117)src/main/java/konkuk/thip/feed/domain/value/TagList.java (1)
TagList(11-105)src/main/java/konkuk/thip/feed/domain/value/ContentList.java (1)
ContentList(13-146)src/main/java/konkuk/thip/feed/application/service/FeedUpdateService.java (1)
Service(14-54)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: build
🔇 Additional comments (3)
build.gradle (1)
85-85: 형식 변경만 존재 — 기능 영향 없음빈 줄 추가 외에 동작 변화가 없습니다. 그대로 진행해도 됩니다.
src/main/java/konkuk/thip/common/swagger/SwaggerResponseDescription.java (1)
196-198: FEED_CREATE에 URL 기반 검증 에러 추가 — 적절합니다Presigned URL 플로우로 전환된 현재 설계와 일치합니다.
src/main/java/konkuk/thip/common/exception/code/ErrorCode.java (1)
175-178: 이미지 URL/확장자/사이즈 에러 코드 재정렬 — OK메시지와 코드 범위가 현 플로우(직접 업로드 → URL 전달)에 맞게 정돈되었습니다.
| for (String url : imageUrls) { | ||
| // 1. 도메인 시작 주소 확인 | ||
| if (!url.startsWith(cloudFrontBaseUrl)) { | ||
| throw new BusinessException(URL_INVALID_DOMAIN); | ||
| } | ||
| // 2. 도메인 이후의 key 추출 | ||
| String key = url.substring(cloudFrontBaseUrl.length()); | ||
| // 3. key를 '/'로 분리 | ||
| String[] parts = key.split("/"); | ||
|
|
||
| // 4. 경로 구조 확인 | ||
| // feed/{userId}/{date}/{uuid + filename} | ||
| if (parts.length != 4) { | ||
| throw new BusinessException(URL_INVALID_DOMAIN); | ||
| } | ||
|
|
||
| // 5. userId와 비교 | ||
| String userIdInUrl = parts[1]; | ||
| Long userIdFromUrl = Long.parseLong(userIdInUrl); | ||
| if (!userIdFromUrl.equals(currentUserId)) { | ||
| throw new BusinessException(URL_USER_ID_MISMATCH); | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion
도메인 검증 취약점 및 예외 누락: 문자열 prefix 비교(+ NumberFormatException 누락)
- startsWith로 전체 URL 문자열을 검사하면
https://{허용도메인}.evil.com/...같은 케이스가 통과할 수 있습니다. 반드시 URI 파싱 후 scheme/host 동치 비교가 필요합니다. Long.parseLong(parts[1])의 NumberFormatException 미처리로 500이 발생할 수 있습니다.- 경로 스키마(
feed/{userId}/{date}/...) 주석은 있으나parts[0] == "feed"검증이 없습니다. - cloudFrontBaseUrl의 슬래시 유무에 따라
substring/split 결과가 흔들릴 수 있습니다.
보안/안정성 영향이 크므로 아래처럼 URI 기반 검증으로 교체를 권장합니다.
@@
-import java.util.List;
+import java.util.List;
+import java.net.URI;
+import java.util.Objects;
@@
- for (String url : imageUrls) {
- // 1. 도메인 시작 주소 확인
- if (!url.startsWith(cloudFrontBaseUrl)) {
- throw new BusinessException(URL_INVALID_DOMAIN);
- }
- // 2. 도메인 이후의 key 추출
- String key = url.substring(cloudFrontBaseUrl.length());
- // 3. key를 '/'로 분리
- String[] parts = key.split("/");
-
- // 4. 경로 구조 확인
- // feed/{userId}/{date}/{uuid + filename}
- if (parts.length != 4) {
- throw new BusinessException(URL_INVALID_DOMAIN);
- }
-
- // 5. userId와 비교
- String userIdInUrl = parts[1];
- Long userIdFromUrl = Long.parseLong(userIdInUrl);
- if (!userIdFromUrl.equals(currentUserId)) {
- throw new BusinessException(URL_USER_ID_MISMATCH);
- }
- }
+ final URI base = URI.create(ensureTrailingSlash(cloudFrontBaseUrl));
+ final String baseHost = base.getHost();
+ final String baseScheme = base.getScheme();
+ final String basePath = base.getPath(); // 보통 "/" 또는 "/cdn-prefix/"
+
+ for (String url : imageUrls) {
+ final URI u;
+ try {
+ u = URI.create(url);
+ } catch (IllegalArgumentException e) {
+ throw new BusinessException(URL_INVALID_DOMAIN);
+ }
+ if (!"https".equalsIgnoreCase(baseScheme) || !"https".equalsIgnoreCase(u.getScheme())) {
+ throw new BusinessException(URL_INVALID_DOMAIN);
+ }
+ if (!Objects.equals(baseHost, u.getHost())) {
+ throw new BusinessException(URL_INVALID_DOMAIN);
+ }
+
+ // basePath를 제거한 뒤 경로 분해
+ String path = u.getPath(); // 예: "/feed/1/250901/uuid-file.jpg"
+ if (basePath != null && basePath.length() > 1 && path.startsWith(basePath)) {
+ path = path.substring(basePath.length());
+ } else if (path.startsWith("/")) {
+ path = path.substring(1);
+ }
+
+ String[] parts = path.split("/");
+ // feed/{userId}/{date}/{uuid+filename}
+ if (parts.length != 4 || !"feed".equals(parts[0])) {
+ throw new BusinessException(URL_INVALID_DOMAIN);
+ }
+ final Long userIdFromUrl;
+ try {
+ userIdFromUrl = Long.parseLong(parts[1]);
+ } catch (NumberFormatException ex) {
+ throw new BusinessException(URL_INVALID_DOMAIN);
+ }
+ if (!userIdFromUrl.equals(currentUserId)) {
+ throw new BusinessException(URL_USER_ID_MISMATCH);
+ }
+ }
@@
}
+
+// 내부 유틸
+private static String ensureTrailingSlash(String s) {
+ return s.endsWith("/") ? s : s + "/";
+}테스트 보강 제안(선택):
- 허용 도메인 + 서브도메인 스푸핑(
{allowed}.evil.com)이 400을 유발하는지 - http 스킴이 거부되는지
- cloudFrontBaseUrl에 슬래시 유무가 달라도 동일하게 동작하는지
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| for (String url : imageUrls) { | |
| // 1. 도메인 시작 주소 확인 | |
| if (!url.startsWith(cloudFrontBaseUrl)) { | |
| throw new BusinessException(URL_INVALID_DOMAIN); | |
| } | |
| // 2. 도메인 이후의 key 추출 | |
| String key = url.substring(cloudFrontBaseUrl.length()); | |
| // 3. key를 '/'로 분리 | |
| String[] parts = key.split("/"); | |
| // 4. 경로 구조 확인 | |
| // feed/{userId}/{date}/{uuid + filename} | |
| if (parts.length != 4) { | |
| throw new BusinessException(URL_INVALID_DOMAIN); | |
| } | |
| // 5. userId와 비교 | |
| String userIdInUrl = parts[1]; | |
| Long userIdFromUrl = Long.parseLong(userIdInUrl); | |
| if (!userIdFromUrl.equals(currentUserId)) { | |
| throw new BusinessException(URL_USER_ID_MISMATCH); | |
| } | |
| // -------------------------------------------------- | |
| // At top of file: add new imports alongside the existing ones | |
| import java.util.List; | |
| import java.net.URI; | |
| import java.util.Objects; | |
| // -------------------------------------------------- | |
| // … other class and member declarations … | |
| public class ImageUrlValidationService { | |
| // … any fields … | |
| public void validateImageUrls(List<String> imageUrls, Long currentUserId) { | |
| // Parse and normalize the base URL once | |
| final URI base = URI.create(ensureTrailingSlash(cloudFrontBaseUrl)); | |
| final String baseHost = base.getHost(); | |
| final String baseScheme = base.getScheme(); | |
| final String basePath = base.getPath(); // e.g. "/" or "/cdn-prefix/" | |
| for (String url : imageUrls) { | |
| final URI u; | |
| try { | |
| u = URI.create(url); | |
| } catch (IllegalArgumentException e) { | |
| throw new BusinessException(URL_INVALID_DOMAIN); | |
| } | |
| // 1) Both base and target must use HTTPS | |
| if (!"https".equalsIgnoreCase(baseScheme) | |
| || !"https".equalsIgnoreCase(u.getScheme())) { | |
| throw new BusinessException(URL_INVALID_DOMAIN); | |
| } | |
| // 2) Host must match exactly (no “allowed.evil.com” spoof) | |
| if (!Objects.equals(baseHost, u.getHost())) { | |
| throw new BusinessException(URL_INVALID_DOMAIN); | |
| } | |
| // 3) Strip off any basePath prefix, then split | |
| String path = u.getPath(); // e.g. "/feed/1/250901/uuid-file.jpg" | |
| if (basePath != null && basePath.length() > 1 && path.startsWith(basePath)) { | |
| path = path.substring(basePath.length()); | |
| } else if (path.startsWith("/")) { | |
| path = path.substring(1); | |
| } | |
| String[] parts = path.split("/"); | |
| // Expect exactly: feed/{userId}/{date}/{uuid+filename} | |
| if (parts.length != 4 || !"feed".equals(parts[0])) { | |
| throw new BusinessException(URL_INVALID_DOMAIN); | |
| } | |
| // 4) Safely parse userId | |
| final Long userIdFromUrl; | |
| try { | |
| userIdFromUrl = Long.parseLong(parts[1]); | |
| } catch (NumberFormatException ex) { | |
| throw new BusinessException(URL_INVALID_DOMAIN); | |
| } | |
| if (!userIdFromUrl.equals(currentUserId)) { | |
| throw new BusinessException(URL_USER_ID_MISMATCH); | |
| } | |
| } | |
| } | |
| // ---------------------------------------------------------------- | |
| // Utility to ensure the base URL always ends with a slash | |
| private static String ensureTrailingSlash(String s) { | |
| return s.endsWith("/") ? s : s + "/"; | |
| } | |
| // ---------------------------------------------------------------- | |
| // … rest of class … | |
| } |
src/test/java/konkuk/thip/feed/adapter/in/web/FeedCreateControllerTest.java
Show resolved
Hide resolved
There was a problem hiding this comment.
Actionable comments posted: 3
♻️ Duplicate comments (2)
src/main/java/konkuk/thip/feed/adapter/out/s3/S3Service.java (2)
62-66: S3 키에서 사용자 filename 제거 및 서버 확정 확장자 사용현재 키에 원본 filename 전체가 포함되어 경로/공백/제어문자/이중 확장자 등 리스크가 있습니다. UUID+서버 확정 ext 기반으로 단순/안전하게 구성해 주세요. (이 사안은 이전 리뷰 지적과 동일 맥락입니다.)
- String key = "feed/" + userId + "/" + datePath + "/" + UUID.randomUUID() + image.filename(); + String uuid = UUID.randomUUID().toString().replace("-", ""); + String key = "feed/" + userId + "/" + datePath + "/" + uuid + "." + ext;대안(운영 제약으로 filename 일부가 필요하다면): filename은 basename만 취하고, 길이/문자셋을 제한해 slug 처리 후
uuid + "-" + slug + "." + ext형태로 제한적으로 포함하세요.
71-73: Presigned URL의 Content-Type 결정 근거 일치지금은 fileName에서 확장자를 재파싱해 Content-Type을 고정합니다. 상단에서 이미 검증된
ext로 Content-Type을 계산해 서명에 포함시키는 것이 일관되고 안전합니다.- GeneratePresignedUrlRequest generatePresignedUrlRequest = - getPutGeneratePresignedUrlRequest(key, expiration); + String contentType = contentTypeFromExtension(ext); + GeneratePresignedUrlRequest generatePresignedUrlRequest = + getPutGeneratePresignedUrlRequest(key, expiration, contentType);- private GeneratePresignedUrlRequest getPutGeneratePresignedUrlRequest(String fileName, Date expiration) { - return new GeneratePresignedUrlRequest(bucket, fileName) + private GeneratePresignedUrlRequest getPutGeneratePresignedUrlRequest(String key, Date expiration, String contentType) { + return new GeneratePresignedUrlRequest(bucket, key) .withMethod(HttpMethod.PUT) - .withKey(fileName) + .withKey(key) .withExpiration(expiration) - .withContentType(determineMimeTypeFromExtension(fileName)); + .withContentType(contentType); }- private String determineMimeTypeFromExtension(String fileName) { - String extension = fileName.substring(fileName.lastIndexOf('.') + 1).toLowerCase(); - return switch (extension) { + private String contentTypeFromExtension(String extension) { + extension = extension.toLowerCase(); + return switch (extension) { case "png" -> "image/png"; case "jpg", "jpeg" -> "image/jpeg"; case "gif" -> "image/gif"; default -> throw new BusinessException(INVALID_FILE_EXTENSION); }; }Also applies to: 85-92, 102-109
🧹 Nitpick comments (8)
src/main/java/konkuk/thip/config/properties/AwsS3Properties.java (1)
11-14: CloudFront 기본 URL은 HTTPS로 강제하는 것이 안전합니다.혼합콘텐츠·중간자 공격 방지 차원에서 스킴을 https로 제한하는 검증을 추가하는 것을 권장합니다.
- @URL(message = "cloud-front-base-url 의 형식이 올바르지 않습니다.") + @URL(protocol = "https", message = "cloud-front-base-url 은 반드시 HTTPS 스킴이어야 합니다.") String cloudFrontBaseUrlsrc/main/java/konkuk/thip/config/AwsS3ImageUrlInitializer.java (1)
19-24: 중복 검증보다는 정규화 + 에러코드 명칭 정합성이 더 중요합니다.
- NotBlank/@url로 이미 부트 바인딩 단계에서 실패하므로 null/empty 체크는 사실상 중복입니다. 유지할 경우 hasText 사용을 권장합니다.
- 에러코드가 “BUCKET”을 가리키고 있어 의미가 모호합니다. CloudFront로 변경된 맥락에 맞게 상수명을 교정하는 것이 좋습니다.
- void bindCloudFrontBaseUrl() { - String baseUrl = awsS3Properties.cloudFrontBaseUrl(); - if (baseUrl == null || baseUrl.isEmpty()) { - throw new BusinessException(AWS_BUCKET_BASE_URL_NOT_CONFIGURED); - } + void bindCloudFrontBaseUrl() { + String baseUrl = awsS3Properties.cloudFrontBaseUrl(); + if (!org.springframework.util.StringUtils.hasText(baseUrl)) { + // TODO: ErrorCode를 AWS_CLOUDFRONT_BASE_URL_NOT_CONFIGURED(가칭) 등으로 교체 + throw new BusinessException(AWS_BUCKET_BASE_URL_NOT_CONFIGURED); + }추가 변경(파일 상단 import 보완):
import org.springframework.util.StringUtils;src/main/java/konkuk/thip/feed/adapter/out/s3/S3Service.java (6)
76-79: CloudFront URL 조립 시 중복 슬래시 방지base URL에 이미 슬래시가 있는 경우
//가 생깁니다.- String fileUrl = cloudFrontBaseUrl + "/" + key; + String base = cloudFrontBaseUrl.endsWith("/") ? cloudFrontBaseUrl.substring(0, cloudFrontBaseUrl.length() - 1) : cloudFrontBaseUrl; + String fileUrl = base + "/" + key;
38-39: 허용 확장자 컬렉션은 Set이 적합멤버십 체크는 Set이语의도에 맞고 미미하지만 더 빠릅니다.
- private static final List<String> ALLOWED_EXTENSIONS = List.of("jpg", "jpeg", "png", "gif"); + private static final Set<String> ALLOWED_EXTENSIONS = Set.of("jpg", "jpeg", "png", "gif");추가:
import java.util.Set;필요.
94-100: 만료시간 계산 간결화Instant/Duration을 쓰면 가독성이 좋아집니다.
- private static Date getExpiration() { - Date expiration = new Date(); - long expTimeMillis = expiration.getTime(); - expTimeMillis += URL_EXPIRED_TIME; - expiration.setTime(expTimeMillis); - return expiration; - } + private static Date getExpiration() { + return Date.from(java.time.Instant.now().plusMillis(URL_EXPIRED_TIME)); + }
62-64: 폴더 날짜 기준 타임존 명시(선택)서버 TZ가 UTC이면 경계 시점에 날짜 파티셔닝이 어긋날 수 있습니다. KST 고정이 필요하면 TZ를 명시하세요.
- String datePath = LocalDate.now().format(DateTimeFormatter.ofPattern("yyMMdd")); + String datePath = LocalDate.now(java.time.ZoneId.of("Asia/Seoul")) + .format(DateTimeFormatter.ofPattern("yyMMdd"));
87-90: withKey 중복 설정 제거 가능(미세 최적화)
new GeneratePresignedUrlRequest(bucket, key)가 이미 키를 보유하므로withKey(key)는 중복입니다. 유지해도 무방하나 제거하면 더 깔끔합니다.- .withKey(key)
25-37: AWS SDK v1 → v2 전환 고려(후속 과제)장기적으로는
S3Presigner(SDK v2)가 종속성/스레드 안전성/성능에서 이점이 있습니다. 이번 PR 범위를 넘지만 로드맵에 포함해 두면 좋습니다.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (3)
src/main/java/konkuk/thip/config/AwsS3ImageUrlInitializer.java(1 hunks)src/main/java/konkuk/thip/config/properties/AwsS3Properties.java(1 hunks)src/main/java/konkuk/thip/feed/adapter/out/s3/S3Service.java(1 hunks)
🧰 Additional context used
🧠 Learnings (1)
📚 Learning: 2025-07-16T11:01:16.732Z
Learnt from: hd0rable
PR: THIP-TextHip/THIP-Server#85
File: src/main/java/konkuk/thip/feed/adapter/in/web/FeedCommandController.java:65-73
Timestamp: 2025-07-16T11:01:16.732Z
Learning: S3Service.deleteImageFromS3() 메서드는 이미 비동기(Async) 함수로 구현되어 있어서 호출하는 쪽에서 별도의 비동기 처리가 필요하지 않습니다.
Applied to files:
src/main/java/konkuk/thip/feed/adapter/out/s3/S3Service.java
🧬 Code graph analysis (1)
src/main/java/konkuk/thip/feed/adapter/out/s3/S3Service.java (1)
src/main/java/konkuk/thip/feed/domain/value/ContentList.java (1)
ContentList(13-146)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: build
🔇 Additional comments (1)
src/main/java/konkuk/thip/feed/adapter/out/s3/S3Service.java (1)
42-46: 유효성 보장 확인 필요
컨트롤러(@RequestBody @Valid List<FeedUploadImagePresignedUrlRequest>)와 DTO(@NotBlank)가 null 혹은 빈 리스트, 그리고userIdnull 케이스를 완전히 방어하는지 검증하세요. 그렇지 않다면 NPE나 잘못된 S3 키 생성 위험이 있어 메서드 시작부에 최소한의 null/빈 검사 로직을 추가해야 합니다.
| Alias.registerBaseUrlSupplier(awsS3Properties::cloudFrontBaseUrl); | ||
| Category.registerBaseUrlSupplier(awsS3Properties::cloudFrontBaseUrl); |
There was a problem hiding this comment.
🛠️ Refactor suggestion
트레일링 슬래시를 정규화하지 않으면 '//' 이중 슬래시 URL이 생성될 수 있습니다.
등록 전 한 번 정규화해서 두 도메인 객체에 동일하게 주입하세요.
- Alias.registerBaseUrlSupplier(awsS3Properties::cloudFrontBaseUrl);
- Category.registerBaseUrlSupplier(awsS3Properties::cloudFrontBaseUrl);
+ String normalizedBaseUrl = baseUrl.endsWith("/") ? baseUrl.substring(0, baseUrl.length() - 1) : baseUrl;
+ Alias.registerBaseUrlSupplier(() -> normalizedBaseUrl);
+ Category.registerBaseUrlSupplier(() -> normalizedBaseUrl);Committable suggestion skipped: line range outside the PR's diff.
🤖 Prompt for AI Agents
In src/main/java/konkuk/thip/config/AwsS3ImageUrlInitializer.java around lines
26-27, normalize the cloudFrontBaseUrl once before registering so neither Alias
nor Category receive a URL with a trailing slash (which can cause '//' when
concatenated); compute a single normalizedBaseUrl by trimming any trailing '/'
(and handling null/empty safely), then register suppliers that return this
normalizedBaseUrl for both Alias and Category so both use the exact same
sanitized value.
There was a problem hiding this comment.
코드래빗이 말하는 것처럼 현재 cloud-front-base-url의 환경변수 마지막이 슬래쉬(/)로 끝나고, Alias에서 사진 url의 시작이 슬래쉬(/)로 시작하는데 이중 슬래쉬 이슈는 없을까요??
There was a problem hiding this comment.
🧩 Analysis chain
@buzz0331 좋은 지적입니다! 이중 슬래시 문제를 확인해보겠습니다.
네, 말씀하신 우려가 정확합니다!
검색 결과를 보면 Alias와 Category 클래스에서 이미지 URL을 구성할 때 다음과 같은 패턴으로 사용하고 있습니다:
// Alias 클래스에서
return baseUrlSupplier.get() + "/user/" + userId + "/alias/" + value + ".jpg";
// Category 클래스에서
return baseUrlSupplier.get() + "/room/" + value + ".jpg";만약 cloud-front-base-url 환경변수가 https://cdn.example.com/처럼 슬래시로 끝나면:
https://cdn.example.com/+/user/123/alias/image.jpg=https://cdn.example.com//user/123/alias/image.jpg
이렇게 이중 슬래시(//)가 발생합니다.
CodeRabbit의 제안이 정확합니다. 다음과 같이 수정하는 것이 좋겠습니다:
@PostConstruct
void bindCloudFrontBaseUrl() {
String baseUrl = awsS3Properties.cloudFrontBaseUrl();
if (baseUrl == null || baseUrl.isEmpty()) {
throw new BusinessException(AWS_BUCKET_BASE_URL_NOT_CONFIGURED);
}
// 트레일링 슬래시 정규화
String normalizedBaseUrl = baseUrl.endsWith("/") ? baseUrl.substring(0, baseUrl.length() - 1) : baseUrl;
Alias.registerBaseUrlSupplier(() -> normalizedBaseUrl);
Category.registerBaseUrlSupplier(() -> normalizedBaseUrl);
}이렇게 하면 환경변수 설정과 관계없이 항상 올바른 URL이 생성됩니다.
(\ /)
( ._. )
o_(")(")
*생각하는 토끼*
🏁 Script executed:
#!/bin/bash
# Alias 클래스에서 URL 구성 방식 확인
ast-grep --pattern $'class Alias {
$$$
}'
Length of output: 44
🏁 Script executed:
#!/bin/bash
# Alias와 Category 클래스에서 URL 구성 방식을 더 자세히 확인
rg -nP --type=java -C5 "registerBaseUrlSupplier|baseUrlSupplier"
Length of output: 4903
🏁 Script executed:
#!/bin/bash
# baseUrl 사용하는 부분과 URL 구성 패턴 확인
rg -nP --type=java -C3 "baseUrl.*/"
Length of output: 37
🏁 Script executed:
#!/bin/bash
# Alias 도메인 클래스에서 이미지 URL 생성 로직 확인
fd "Alias.java" --exec cat {}
Length of output: 1297
🏁 Script executed:
#!/bin/bash
# Category 도메인 클래스에서 이미지 URL 생성 로직 확인
fd "Category.java" --exec cat {}
Length of output: 1649
@buzz0331 네, 정확히 맞습니다! 코드를 확인해보니 이중 슬래시 문제가 실제로 발생할 수 있습니다.
문제 상황 분석
Alias 클래스 (Line 40):
public String getImageUrl() {
return baseUrlSupplier.get() + imageUrl;
}Category 클래스 (Line 49):
public String getImageUrl() {
return baseUrlSupplier.get() + imageUrl;
}두 클래스 모두에서 imageUrl 값들이 슬래시로 시작합니다:
- Alias:
"/profile_literature.png","/profile_science.png"등 - Category:
"/group_science.png","/group_literature.png"등
만약 환경변수 cloud-front-base-url이 https://cdn.example.com/처럼 슬래시로 끝나면:
"https://cdn.example.com/" + "/profile_literature.png"
= "https://cdn.example.com//profile_literature.png" // 이중 슬래시!
해결 방안
CodeRabbit의 제안대로 트레일링 슬래시를 정규화하는 것이 정확한 해결책입니다:
@PostConstruct
void bindCloudFrontBaseUrl() {
String baseUrl = awsS3Properties.cloudFrontBaseUrl();
if (baseUrl == null || baseUrl.isEmpty()) {
throw new BusinessException(AWS_BUCKET_BASE_URL_NOT_CONFIGURED);
}
// 트레일링 슬래시 정규화 - 이중 슬래시 방지
String normalizedBaseUrl = baseUrl.endsWith("/") ? baseUrl.substring(0, baseUrl.length() - 1) : baseUrl;
Alias.registerBaseUrlSupplier(() -> normalizedBaseUrl);
Category.registerBaseUrlSupplier(() -> normalizedBaseUrl);
}이렇게 수정하면 환경변수 설정과 관계없이 항상 올바른 URL이 생성됩니다:
https://cdn.example.com+/profile_literature.png=https://cdn.example.com/profile_literature.png✅
좋은 지적이었습니다! 🎯
(\ /)
( ^.^ )
o_(")(")
*문제를 찾은 토끼*
There was a problem hiding this comment.
Seems like the humans are having a chat. I'll hop back into my burrow for now. If you need me again, just tag @coderabbitai in a new comment, and I'll come hopping out!
| // 스프링 컨텍스트 시작 시점에 cloud-front-base-url의 값 유효성 검사 실행 | ||
| @NotBlank(message = "cloud-front-base-url 은 비어있을 수 없습니다.") | ||
| @URL(message = "cloud-front-base-url 의 형식이 올바르지 않습니다.") | ||
| String cloudFrontBaseUrl |
There was a problem hiding this comment.
💡 Verification agent
🧩 Analysis chain
설정 키 변경에 따른 하위 호환 및 문서/환경 변수 갱신 확인 필요.
기존 bucket-base-url → cloud-front-base-url로 바뀌었으므로 application.yml/운영 환경 변수/문서에서 누락된 레거시 키가 없는지 점검해 주세요. 필요 시 마이그레이션 가이드를 PR 설명에 명시해주세요.
아래 스크립트로 레거시 용례를 빠르게 확인할 수 있습니다.
🏁 Script executed:
#!/bin/bash
# 레거시 키/식별자 흔적 찾기
rg -n -C2 -S 'bucket-base-url|bucketBaseUrl|AWS_BUCKET_BASE_URL_NOT_CONFIGURED'
# 설정 파일에서 현재 키 사용 여부 확인
fd -a 'application*.yml' | xargs -I{} rg -n 'cloud\.aws\.s3\.(cloud-front-base-url|bucket-base-url)' {}Length of output: 1786
애플리케이션 설정·문서·예외코드 레거시 키 갱신 필요
application*.yml(예: src/main/resources/application.yml)에서 기존
cloud.aws.s3.bucket-base-url제거 후
cloud.aws.s3.cloud-front-base-url추가 → 운영 환경 변수(CLOUD_AWS_S3_CLOUD_FRONT_BASE_URL)도 동기화- PR 설명에 “기존
bucket-base-url→cloud-front-base-url매핑” 마이그레이션 가이드 명시 ErrorCode.java의
AWS_BUCKET_BASE_URL_NOT_CONFIGURED("aws s3 bucket base url 설정이 누락되었습니다.")
상수명·메시지를
AWS_CLOUD_FRONT_BASE_URL_NOT_CONFIGURED("aws s3 cloud-front base url 설정이 누락되었습니다.")
등으로 업데이트해주세요.
🤖 Prompt for AI Agents
In src/main/java/konkuk/thip/config/properties/AwsS3Properties.java around lines
11-14, update project configuration keys and error constants to reflect the
rename from bucket-base-url to cloud-front-base-url: remove the old
application.yml key cloud.aws.s3.bucket-base-url and add
cloud.aws.s3.cloud-front-base-url (ensure environment variable
CLOUD_AWS_S3_CLOUD_FRONT_BASE_URL is set in deployment), update the PR
description to include a migration guide mapping the old key to the new key, and
rename the ErrorCode constant AWS_BUCKET_BASE_URL_NOT_CONFIGURED to
AWS_CLOUD_FRONT_BASE_URL_NOT_CONFIGURED (and update its message text
accordingly) plus refactor all usages/tests/config references to the new
constant and key names.
| String ext = image.extension() == null ? "" : image.extension().toLowerCase(); | ||
| if (!ALLOWED_EXTENSIONS.contains(ext)) { | ||
| throw new BusinessException(INVALID_FILE_EXTENSION); | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion
확장자 정규화('.' 제거) + 허용 목록 체크 보완
요청이 ".jpg" 형태로 올 때 허용 목록 미스매치가 납니다. 먼저 선행 정규화 후 검증해 주세요.
- String ext = image.extension() == null ? "" : image.extension().toLowerCase();
- if (!ALLOWED_EXTENSIONS.contains(ext)) {
+ String ext = image.extension() == null ? "" : image.extension().toLowerCase();
+ if (ext.startsWith(".")) ext = ext.substring(1);
+ if (!ALLOWED_EXTENSIONS.contains(ext)) {
throw new BusinessException(INVALID_FILE_EXTENSION);
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| String ext = image.extension() == null ? "" : image.extension().toLowerCase(); | |
| if (!ALLOWED_EXTENSIONS.contains(ext)) { | |
| throw new BusinessException(INVALID_FILE_EXTENSION); | |
| } | |
| String ext = image.extension() == null ? "" : image.extension().toLowerCase(); | |
| if (ext.startsWith(".")) ext = ext.substring(1); | |
| if (!ALLOWED_EXTENSIONS.contains(ext)) { | |
| throw new BusinessException(INVALID_FILE_EXTENSION); | |
| } |
🤖 Prompt for AI Agents
In src/main/java/konkuk/thip/feed/adapter/out/s3/S3Service.java around lines 52
to 55, the code compares the extension directly against ALLOWED_EXTENSIONS which
fails when the input includes a leading dot (e.g., ".jpg"); normalize the
extension first by handling null, trimming whitespace, removing a leading '.' if
present, and converting to lowercase, then validate against ALLOWED_EXTENSIONS
and throw BusinessException(INVALID_FILE_EXTENSION) if it is not allowed.
There was a problem hiding this comment.
Actionable comments posted: 0
♻️ Duplicate comments (1)
src/test/java/konkuk/thip/feed/adapter/in/web/FeedCreateControllerTest.java (1)
23-23: multipart → post 전환 반영 완료컨트롤러의 @RequestBody(JSON)와 일치하게 post 빌더를 사용한 점 적절합니다.
🧹 Nitpick comments (6)
src/test/java/konkuk/thip/feed/adapter/in/web/FeedCreateControllerTest.java (6)
36-38: 프로퍼티 기본값 및 슬래시 정규화로 플래키 방지
- test 프로파일에서
cloud.aws.s3.cloud-front-base-url미정의 시 컨텍스트 로딩 실패 가능. 기본값을 지정해 안정화하세요.- 또한 베이스 URL의 말미 슬래시 유무에 따라 URL 결합이 깨질 수 있으므로 테스트 내에서 한 번 정규화해 쓰는 편이 안전합니다.
적용 예시(diff – 기본값 추가):
- @Value("${cloud.aws.s3.cloud-front-base-url}") + @Value("${cloud.aws.s3.cloud-front-base-url:https://cdn.test/}") private String cloudFrontBaseUrl;베이스 URL 정규화(파일 내 임의 위치에 메서드 추가):
private String baseUrl() { return cloudFrontBaseUrl.endsWith("/") ? cloudFrontBaseUrl : cloudFrontBaseUrl + "/"; }
133-138: URL 결합 안정화 및 하드코딩된 날짜 제거 권장
cloudFrontBaseUrl뒤 슬래시 의존(현재는 경로 앞에/미포함)으로 환경별 플래키 가능. 위에서 제안한baseUrl()사용으로 방지하세요.- 날짜
250901하드코딩은 도메인 로직이 날짜를 검증할 경우 취약합니다. 실행 시점 기준으로 생성하는 편이 안전합니다.적용 예시(diff – baseUrl 사용 및 datePart 변수 사용):
- req.put("imageUrls", List.of( - cloudFrontBaseUrl + "feed/1/250901/uuid-file1.jpg", - cloudFrontBaseUrl + "feed/1/250901/uuid-file2.jpg", - cloudFrontBaseUrl + "feed/1/250901/uuid-file3.jpg", - cloudFrontBaseUrl + "feed/1/250901/uuid-file4.jpg" - )); + req.put("imageUrls", List.of( + baseUrl() + "feed/1/" + datePart + "/uuid-file1.jpg", + baseUrl() + "feed/1/" + datePart + "/uuid-file2.jpg", + baseUrl() + "feed/1/" + datePart + "/uuid-file3.jpg", + baseUrl() + "feed/1/" + datePart + "/uuid-file4.jpg" + ));메서드 상단에 날짜 변수 선언 추가:
String datePart = java.time.LocalDate.now().format(java.time.format.DateTimeFormatter.ofPattern("yyMMdd"));백엔드가 URL 내 날짜 값을 실제 업로드일과 일치하도록 검증하는지 확인 부탁드립니다. 검증한다면 위 동적 날짜가 필요합니다.
140-148: 중복 제거: 공용 헬퍼로 치환 가능동일한 검증 패턴이므로 헬퍼를 사용하면 테스트 간 일관성이 올라갑니다.
적용 예시(diff):
- ResultActions result = mockMvc.perform(post("/feeds") - .contentType(MediaType.APPLICATION_JSON) - .content(objectMapper.writeValueAsBytes(req)) - .requestAttr("userId", 1L) - ); - - result.andExpect(status().isBadRequest()) - .andExpect(jsonPath("$.code").value(CONTENT_LIST_SIZE_OVERFLOW.getCode())) - .andExpect(jsonPath("$.message",containsString(CONTENT_LIST_SIZE_OVERFLOW.getMessage()))); + assertBadRequest_InvalidFeedCreate( + req, + CONTENT_LIST_SIZE_OVERFLOW.getMessage(), + CONTENT_LIST_SIZE_OVERFLOW.getCode() + );
152-167: 도메인 불일치 케이스도 공용 헬퍼 사용 가능검증 패턴 동일. 헬퍼로 간결화하세요.
적용 예시(diff):
- mockMvc.perform(post("/feeds") - .contentType(MediaType.APPLICATION_JSON) - .content(objectMapper.writeValueAsBytes(req)) - .requestAttr("userId", 1L)) - .andExpect(status().isBadRequest()) - .andExpect(jsonPath("$.code").value(URL_INVALID_DOMAIN.getCode())) - .andExpect(jsonPath("$.message", containsString(URL_INVALID_DOMAIN.getMessage()))); + assertBadRequest_InvalidFeedCreate( + req, + URL_INVALID_DOMAIN.getMessage(), + URL_INVALID_DOMAIN.getCode() + );
171-185: 에러 코드 단정치 재검토: 도메인은 유효, 경로가 잘못된 케이스현재 케이스는 베이스 도메인은 올바르고 경로 형식만 잘못(feed/ 누락)입니다. 그런데 기대 코드를
URL_INVALID_DOMAIN으로 두면 명세/구현과 불일치할 가능성이 큽니다.
- 권장: 경로 형식 전용 에러코드가 있다면 그 코드(예:
URL_INVALID_PATH또는URL_INVALID_FORMAT)로 검증.- 없다면 포괄 코드
API_INVALID_PARAM으로 기대값을 수정.적용 예시(diff – 포괄 코드로 수정):
- .andExpect(jsonPath("$.code").value(URL_INVALID_DOMAIN.getCode())) - .andExpect(jsonPath("$.message", containsString(URL_INVALID_DOMAIN.getMessage()))); + .andExpect(jsonPath("$.code").value(API_INVALID_PARAM.getCode())) + .andExpect(jsonPath("$.message", containsString("URL 형식")));또는 공용 헬퍼 사용:
assertBadRequest_InvalidParam(req, "URL 형식");추가로, 여기서도
baseUrl()과datePart사용을 권장합니다.- req.put("imageUrls", List.of( - cloudFrontBaseUrl + "1/250901/uuid-file.jpg" // url형식이 알맞지 않음 (feed/ 생략) - )); + req.put("imageUrls", List.of( + baseUrl() + "1/" + datePart + "/uuid-file.jpg" // url 형식 불일치 (feed/ 생략) + ));
187-202: userId 불일치 테스트도 URL 결합 안정화 + 헬퍼 사용 권장
baseUrl()사용으로 슬래시 이슈 제거- 공용 헬퍼로 중복 제거
적용 예시(diff):
- req.put("imageUrls", List.of( - cloudFrontBaseUrl + "feed/999/250901/uuid-file.jpg" // userId 999는 요청 userId 1과 불일치 - )); + req.put("imageUrls", List.of( + baseUrl() + "feed/999/" + datePart + "/uuid-file.jpg" // userId 999는 요청 userId 1과 불일치 + ));- mockMvc.perform(post("/feeds") - .contentType(MediaType.APPLICATION_JSON) - .content(objectMapper.writeValueAsBytes(req)) - .requestAttr("userId", 1L)) - .andExpect(status().isBadRequest()) - .andExpect(jsonPath("$.code").value(URL_USER_ID_MISMATCH.getCode())) - .andExpect(jsonPath("$.message", containsString(URL_USER_ID_MISMATCH.getMessage()))); + assertBadRequest_InvalidFeedCreate( + req, + URL_USER_ID_MISMATCH.getMessage(), + URL_USER_ID_MISMATCH.getCode() + );
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (1)
src/test/java/konkuk/thip/feed/adapter/in/web/FeedCreateControllerTest.java(5 hunks)
🔇 Additional comments (3)
src/test/java/konkuk/thip/feed/adapter/in/web/FeedCreateControllerTest.java (3)
9-9: @value 도입 적절테스트에서 CloudFront 베이스 URL을 주입받도록 한 선택 좋습니다.
52-56: 공용 요청 헬퍼 구성 적절JSON POST + userId requestAttr 고정 패턴을 캡슐화해 재사용한 점 좋습니다.
62-66: 파라미터 오류용 헬퍼도 OK에러코드(API_INVALID_PARAM) 경로와 메시지 검증을 공통화해 중복을 줄였습니다.
| String userIdInUrl = parts[1]; | ||
| Long userIdFromUrl = Long.parseLong(userIdInUrl); | ||
| if (!userIdFromUrl.equals(currentUserId)) { | ||
| throw new BusinessException(URL_USER_ID_MISMATCH); | ||
| } | ||
| } |
There was a problem hiding this comment.
parseLong에서 파싱에러 발생시에 NumberFormatException이 발생하는데 이 부분은 저희가 직접 try-catch로 잡아줘야 될 것 같아욥. 아니면 500 에러가 발생할 것 같습니다!
| Alias.registerBaseUrlSupplier(awsS3Properties::cloudFrontBaseUrl); | ||
| Category.registerBaseUrlSupplier(awsS3Properties::cloudFrontBaseUrl); |
There was a problem hiding this comment.
코드래빗이 말하는 것처럼 현재 cloud-front-base-url의 환경변수 마지막이 슬래쉬(/)로 끝나고, Alias에서 사진 url의 시작이 슬래쉬(/)로 시작하는데 이중 슬래쉬 이슈는 없을까요??
| @Schema(description = "파일 이름 (확장자명 포함)", example = "image1.png") | ||
| @NotBlank(message = "파일 이름은 필수입니다.") | ||
| String filename, | ||
|
|
||
| @Schema(description = "파일 확장자 [허용 확장자 jpg, jpeg, png, gif]", example = "png") | ||
| @NotBlank(message = "파일 확장자는 필수입니다.") | ||
| String extension, | ||
|
|
||
| @Schema(description = "파일 크기 (바이트) [허용 파일 최대 크기 5MB]", example = "102400") | ||
| @Positive(message = "파일 크기는 양수여야 합니다.") |
|
|
||
| @Schema(description = "PUT 요청을 보내기 위한 파일 업로드용 Presigned URL") | ||
| String preSignedUrl, | ||
| String presignedUrl, |
seongjunnoh
left a comment
seongjunnoh
left a comment
There was a problem hiding this comment.
고생하셨습니다!! 수정하신 부분 확인했습니다!!
| // 이미지 업로드 개수 검증 | ||
| ContentList.validateImageCount(images.size()); |
| // 현재 날짜를 yyMMdd 형식으로 포맷팅 | ||
| String datePath = LocalDate.now().format(DateTimeFormatter.ofPattern("yyMMdd")); | ||
| // 객체 key 생성 (일단 피드 생성시에만 이미지를 업로드하기때문에 나중에 더 추가되면 분기처리) | ||
| String key = "feed/" + userId + "/" + datePath + "/" + UUID.randomUUID() + image.filename(); |
There was a problem hiding this comment.
ContentLIst 에 파일의 메타데이터를 추가하지는 않고, key 값에 file name을 추가하는 방식으로 구현하신 것 확인했습니다!! 추후에 다른 메타데이터가 필요한 요구사항이 생기면 그때가서 수정해도 문제없을 것 같습니다!!
| // 업로드 후 접근 가능한 파일 URL | ||
| String fileUrl = cloudFrontBaseUrl + "/" + key; | ||
|
|
||
| result.add(new FeedUploadImagePresignedUrlResponse.PresignedUrlInfo(url.toExternalForm(), fileUrl)); |
There was a problem hiding this comment.
cdn get url 응답하도록 구현하신것 확인했습니다!!
| TagList.validateTags(Tag.fromList(command.tagList())); | ||
| // todo 나중에 presignedURL로 바꾸면 ContentList.of로 변경 | ||
| ContentList.validateImageCount(images != null ? images.size() : 0); | ||
| ContentList.validateImageCount(ContentList.of(command.imageUrls()).size()); |
There was a problem hiding this comment.
아 이 부분은 전에 말씀주신것처럼 book 검증 전에 ContentList 관련 validate 를 수행하려고 createFeed 메서드 내부에서 유효성 검증 -> Feed 생성 이런식으로 메서드를 호출하는 구조를 유지하셨군요!! 확인했습니다!
| @HelperService | ||
| @RequiredArgsConstructor | ||
| public class ImageUrlValidationService { |
3 participants
#️⃣ 연관된 이슈
📝 작업 내용
📸 스크린샷
💬 리뷰 요구사항
📌 PR 진행 시 이러한 점들을 참고해 주세요
Summary by CodeRabbit
New Features
Refactor
Documentation