feat: 채팅 이미지 전송 구현 #475
feat: 채팅 이미지 전송 구현 #475
Conversation
- topic/{roomId} -> topic/chat/{roomId}
- 의미적 명확성을 위해
- 이에 컨트롤러 인자로 siteUserId를 받도록 하고, DTO에 senderId를 삭제한다.
- 검증 로직이 핸들러에서 사용됨에 따라 발생하는 순환 참조를 막기 위해 Lazy 어노테이션을 사용한 생성자를 직접 작성
- 정규표현식을 사용하여 채팅방 ID 추출 - DTO 검증 추가 - 구체화 클래스가 아닌 인터페이스 사용하도록 (DIP) - senderId가 siteUserId가 아니라 chatParticipantId로 설정되도록 변경
…rver into feat/428-chat-with-socket-image
Walkthrough
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Suggested reviewers
Tip 🔌 Remote MCP (Model Context Protocol) integration is now available!Pro plan users can now connect to remote MCP servers from the Integrations page. Connect with popular remote MCPs such as Notion and Linear to add more context to your reviews and chats. ✨ 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 (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 4
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/main/java/com/example/solidconnection/s3/service/S3Service.java (1)
38-56: 리팩터링 및 업로드 안전성 강화 요청
상수명 변경
MAX_FILE_SIZE_MB가 바이트가 아닌 메가바이트로 오해될 수 있으므로,MAX_FILE_SIZE_BYTES로 교체해주세요.비동기 업로드 예외 로깅 강화
asyncExecutor.submit내부에try/catch를 추가해 예외가 로그로 남고 모니터링될 수 있도록 보완해주세요.
참고로AsyncConfig에서 현재corePoolSize=5,maxPoolSize=10,queueCapacity=50로 설정되어 있으니, 대량 업로드 시 병목이나 작업 거절 여부를 검토 후 필요 시 값 조정을 권장드립니다.ImgType별 확장자 엄격 분리
CHAT 전용 업로드에는IMAGE_EXTS = List.of("jpg","jpeg","png","webp")만, 그 외 용도에는 별도의DOC_EXTS = List.of("pdf","doc","docx")등을 사용해 확장자 허용 목록을 분리해주세요.콘텐츠 스푸핑 방지 강화
현재 확장자 검사만으로는 파일 위조를 완벽히 차단하기 어려우므로,fileUploadService레이어에서 HTTPContent-Type과 파일 헤더(매직 넘버) 검증을 추가해주세요.확장자 명칭 수정
“word”는 실제 확장자가 아니므로, 문서 포맷 중doc이 맞다면 “word” 대신doc으로 교정해주세요.
🧹 Nitpick comments (6)
src/main/java/com/example/solidconnection/chat/dto/ChatImageSendRequest.java (1)
8-13: 필드 네이밍 및 서버 측 검증 보강 제안
네이밍 변경
필드명과 메시지를imageUrls에서imageKeys로 변경하여 실제 S3 키를 표현하도록 명확성을 높여 주세요.패턴 검증 추가
@Pattern(regexp="^chat/.*", message="이미지 키는 'chat/'로 시작해야 합니다")를 필드에 적용해 서버 측에서 키 형식을 방어적으로 검증하세요.JDK 17 지원 확인 완료
build.gradle에 이미sourceCompatibility = '17'가 정의되어 있어record사용이 안전합니다.src/main/java/com/example/solidconnection/chat/controller/ChatMessageController.java (1)
34-44: 인증 객체 캐스팅 중복을 제거하고 안전하게 추출하는 헬퍼로 정리해 주세요.
- 현재 Principal → TokenAuthentication 다운캐스팅이 메서드마다 반복됩니다.
- 공통 헬퍼로 사용자 ID만 안전하게 뽑으면 NPE/CCE 여지를 줄이고 중복을 제거할 수 있습니다.
아래처럼 변경을 제안드립니다.
@MessageMapping("/chat/{roomId}/image") public void sendChatImage( @DestinationVariable Long roomId, @Valid @Payload ChatImageSendRequest chatImageSendRequest, Principal principal ) { - TokenAuthentication tokenAuthentication = (TokenAuthentication) principal; - SiteUserDetails siteUserDetails = (SiteUserDetails) tokenAuthentication.getPrincipal(); - - chatService.sendChatImage(chatImageSendRequest, siteUserDetails.getSiteUser().getId(), roomId); + long siteUserId = extractSiteUserId(principal); + chatService.sendChatImage(chatImageSendRequest, siteUserId, roomId); }추가로 파일 하단 등 공용 위치에 다음 헬퍼를 두면 sendChatMessage에서도 재사용할 수 있습니다.
private long extractSiteUserId(Principal principal) { TokenAuthentication tokenAuthentication = (TokenAuthentication) principal; SiteUserDetails siteUserDetails = (SiteUserDetails) tokenAuthentication.getPrincipal(); return siteUserDetails.getSiteUser().getId(); }src/test/java/com/example/solidconnection/chat/service/ChatServiceTest.java (2)
465-494: 멀티 이미지 전송 시나리오가 핵심 경로를 잘 검증합니다. 다만 썸네일 전부에 대한 보강 검증을 한 줄 추가해 주세요.
- 현재 1번 테스트는 attachments 개수/순서, messageType, content 빈 값까지 커버합니다.
- 모든 첨부에 대해 thumbnailUrl이 비어있지 않음을 함께 확인하면 회귀에 강해집니다.
예시:
assertThat(response.attachments()) .allSatisfy(att -> assertThat(att.thumbnailUrl()).isNotBlank());
533-556: 썸네일 URL 생성 테스트 훌륭합니다. 엣지 케이스 2가지만 추가 권장합니다.
- 확장자가 없는 파일명(example) 입력 시 원본 반환(or 예외) 동작.
- 서명 쿼리 파라미터가 붙은 URL(example.jpg?X-Amz-...) 입력 시 정상 생성/보존 여부.
원하시면 위 2개 케이스에 대한 테스트 메서드를 제가 추가해드릴게요.
src/main/java/com/example/solidconnection/chat/service/ChatService.java (2)
193-202: 썸네일 경로를 문자열 치환으로 “추론”해 저장하는 현재 방식은 취약합니다. S3 키 중심 보존으로 변경을 제안합니다.
- URL 스킴/호스트/CDN 변경 시 문자열 치환은 깨집니다.
- “원본 키”와 “썸네일 키”를 DB에 함께 저장(람다에서 반환)하면, 조회 시 안정적으로 URL을 구성할 수 있습니다.
- 키 기반 접근은 서명 URL, 리전, 버킷, CDN 프리픽스 변경에도 견고합니다.
간단 대안:
- 람다: 썸네일 생성 후 {originalKey, thumbnailKey}를 메시징/메타데이터로 회신.
- 서버: 그 키를 그대로 DB에 보관하고, 응답 시 URL 빌더(버킷/리전/CDN 프리픽스 주입)를 통해 최종 URL 생성.
223-240: thumbnail URL 생성 로직을 패턴 검증/확장자/쿼리스트링을 고려해 견고하게 바꿔 주세요.
- /chat/images/ 패턴이 없을 때도 파일명만 바뀌는 부자연스러운 결과가 나옵니다.
- 확장자 없는 파일명, 서명 쿼리스트링이 있는 URL을 안전하게 처리해야 합니다.
아래처럼 방어적으로 개선을 제안드립니다.
- private String generateThumbnailUrl(String originalUrl) { - try { - String fileName = originalUrl.substring(originalUrl.lastIndexOf('/') + 1); - - String nameWithoutExt = fileName.substring(0, fileName.lastIndexOf('.')); - String extension = fileName.substring(fileName.lastIndexOf('.')); - - String thumbnailFileName = nameWithoutExt + "_thumb" + extension; - - String thumbnailUrl = originalUrl.replace("chat/images/", "chat/thumbnails/") - .replace(fileName, thumbnailFileName); - - return thumbnailUrl; - - } catch (Exception e) { - return originalUrl; - } - } + private String generateThumbnailUrl(String originalUrl) { + if (originalUrl == null || originalUrl.isBlank()) return originalUrl; + // 예상 경로가 아니면 원본을 그대로 반환 + final String imagesMarker = "/chat/images/"; + int markerIdx = originalUrl.indexOf(imagesMarker); + if (markerIdx < 0) return originalUrl; + + int lastSlash = originalUrl.lastIndexOf('/'); + if (lastSlash < 0 || lastSlash + 1 >= originalUrl.length()) return originalUrl; + + String fileNameAndQuery = originalUrl.substring(lastSlash + 1); + int qIdx = fileNameAndQuery.indexOf('?'); + String fileName = (qIdx >= 0) ? fileNameAndQuery.substring(0, qIdx) : fileNameAndQuery; + + int dot = fileName.lastIndexOf('.'); + if (dot < 0) return originalUrl; // 확장자 없으면 변경하지 않음 + + String nameWithoutExt = fileName.substring(0, dot); + String extension = fileName.substring(dot); + String thumbFileName = nameWithoutExt + "_thumb" + extension; + + String prefix = originalUrl.substring(0, markerIdx); + return prefix + "/chat/thumbnails/" + thumbFileName; + }
📜 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 (10)
src/main/java/com/example/solidconnection/chat/controller/ChatMessageController.java(2 hunks)src/main/java/com/example/solidconnection/chat/domain/MessageType.java(1 hunks)src/main/java/com/example/solidconnection/chat/dto/ChatImageSendRequest.java(1 hunks)src/main/java/com/example/solidconnection/chat/dto/ChatMessageSendResponse.java(1 hunks)src/main/java/com/example/solidconnection/chat/service/ChatService.java(3 hunks)src/main/java/com/example/solidconnection/s3/controller/S3Controller.java(2 hunks)src/main/java/com/example/solidconnection/s3/domain/ImgType.java(1 hunks)src/main/java/com/example/solidconnection/s3/service/S3Service.java(2 hunks)src/main/resources/secret(1 hunks)src/test/java/com/example/solidconnection/chat/service/ChatServiceTest.java(3 hunks)
⏰ 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 (8)
src/main/java/com/example/solidconnection/chat/domain/MessageType.java (1)
3-6: 메시지 타입 enum 도입이 간결하고 적절합니다.
- TEXT/IMAGE로 현재 스코프를 정확히 커버합니다.
- 직렬화 시 문자열 값이 API 계약과 일치하는지만 테스트로 보강하면 좋겠습니다.
src/main/java/com/example/solidconnection/s3/domain/ImgType.java (1)
7-7: 📌 CHAT 타입 분기 자동 적용 및 추가 점검 안내
ImgType 분기 처리 로직
–switch/if구문이 없더라도,ImgTypeenum 생성자에 정의된"chat"prefix가S3Service구현에서 자동으로 적용됩니다.
– 별도CHAT케이스 추가 불필요하며, 코드상 누락된 분기 없이 정상 동작합니다.Lambda 트리거 설정 점검
– S3 이벤트 필터에서chat/경로가 Lambda 함수의 트리거 목록에 포함되었는지 확인해주세요.CloudFront 캐시 및 권한 설정
–chat/경로에 대한 캐시 동작(Behavior)과 권한 정책(Origin Access Identity, IAM Role 등)이 올바르게 구성되었는지 함께 검토하면 안전합니다.src/main/java/com/example/solidconnection/chat/controller/ChatMessageController.java (1)
3-3: 새 DTO 도입 적합합니다.
문맥에 맞는 ChatImageSendRequest import 추가로 가독성과 의도가 분명해졌습니다.src/test/java/com/example/solidconnection/chat/service/ChatServiceTest.java (2)
14-16: 테스트에 필요한 import 보강 좋습니다.
MessageType/ChatImageSendRequest/List 추가로 새 시나리오들이 명확해졌습니다.Also applies to: 33-33
520-531: 비참여자 차단 플로우 검증 👍
예외 타입/메시지까지 고정해 회귀 방어에 적합합니다.src/main/java/com/example/solidconnection/chat/service/ChatService.java (1)
8-8: 단순 import 변경은 OK.
의존 추가가 현재 변경과 일관됩니다.src/main/java/com/example/solidconnection/chat/dto/ChatMessageSendResponse.java (2)
10-12: 응답 스키마 확장(발신자/타입/첨부) 방향 좋습니다.
클라이언트가 메시지 표현을 결정하는 데 필요한 최소 정보로 보입니다.
20-28: 첨부 매핑 로직은 명확하고 일관됩니다.
생성 시각 포함으로 디버깅/정렬에도 유용합니다.
| MessageType messageType = chatMessage.getChatAttachments().isEmpty() | ||
| ? MessageType.TEXT | ||
| : MessageType.IMAGE; | ||
|
|
There was a problem hiding this comment.
🛠️ Refactor suggestion
메시지 타입 산정 시 ‘첨부 존재 여부’ 대신 ‘이미지 첨부 포함 여부’를 사용해 주세요.
- 파일(비이미지) 첨부도 있는 경우, 현재 로직은 IMAGE로 잘못 분류될 수 있습니다.
- isImage 플래그를 활용해 타입을 판정해야 확장 시에도 안전합니다.
아래처럼 변경을 제안드립니다(추가 import 없이 람다 사용).
- MessageType messageType = chatMessage.getChatAttachments().isEmpty()
- ? MessageType.TEXT
- : MessageType.IMAGE;
+ MessageType messageType = chatMessage.getChatAttachments().stream()
+ .anyMatch(att -> Boolean.TRUE.equals(att.getIsImage()))
+ ? MessageType.IMAGE
+ : MessageType.TEXT;📝 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.
| MessageType messageType = chatMessage.getChatAttachments().isEmpty() | |
| ? MessageType.TEXT | |
| : MessageType.IMAGE; | |
| MessageType messageType = chatMessage.getChatAttachments().stream() | |
| .anyMatch(att -> Boolean.TRUE.equals(att.getIsImage())) | |
| ? MessageType.IMAGE | |
| : MessageType.TEXT; |
🤖 Prompt for AI Agents
In
src/main/java/com/example/solidconnection/chat/dto/ChatMessageSendResponse.java
around lines 16 to 19, the code currently sets MessageType based only on whether
any attachments exist, which misclassifies messages that have non-image
attachments; update the logic to check if any attachment has the isImage flag
true (e.g., use a stream anyMatch on chatMessage.getChatAttachments() to detect
image attachments) and set MessageType.IMAGE only when an image attachment is
present, otherwise fallback to MessageType.TEXT; keep imports unchanged by using
a lambda.
There was a problem hiding this comment.
기억이 가물가물한데 현재 저희는 일단 이미지만 작업하는 거 맞나요 ? 그렇다면 이건 추후에 반영해도 될 거 같습니다
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Check if there are existing non-image attachment implementations
echo "=== Checking ChatAttachment entity structure ==="
fd -e java -x grep -l "ChatAttachment" {} \; | head -5 | xargs cat
echo -e "\n=== Checking for existing file upload types beyond images ==="
rg -n "isImage" --type java -C 3
echo -e "\n=== Checking ImgType enum for file type support ==="
cat src/main/java/com/example/solidconnection/s3/domain/ImgType.java
echo -e "\n=== Checking if there are existing file/attachment upload endpoints ==="
rg -n "upload.*file" --type java -C 2
Length of output: 21605
@whqtker 맞습니다! 현재 작업 스코프는 이미지 전송에 집중되어 있네요.
코드를 확인해보니:
- 현재 PR 스코프:
ChatImageSendRequest를 통한 이미지 전송만 구현 - 인프라는 준비됨:
ChatAttachment에isImage필드가 있어서 이미지/비이미지 구분 가능 - 현재 구현:
sendChatImage에서isImage=true인 첨부만 생성
PR 설명에서도 "이미지를 제외한 기타 첨부파일 관련 로직은 정상 동작을 확인했고, 추가 작업 예정"이라고 하셨으니, 지금은 이미지만 다루고 있는 게 맞는 것 같습니다.
따라서 제안드린 개선사항은 추후에 비이미지 첨부파일이 실제로 추가될 때 함께 반영하는 게 더 실용적일 것 같아요. 현재로서는 단순히 isEmpty() 체크만으로도 충분하겠네요!
ᕦ(◕‿◕)ᕤ
| @Transactional | ||
| public void sendChatImage(ChatImageSendRequest chatImageSendRequest, long siteUserId, long roomId) { | ||
| long senderId = chatParticipantRepository.findByChatRoomIdAndSiteUserId(roomId, siteUserId) | ||
| .orElseThrow(() -> new CustomException(CHAT_PARTICIPANT_NOT_FOUND)) | ||
| .getId(); | ||
|
|
||
| ChatRoom chatRoom = chatRoomRepository.findById(roomId) | ||
| .orElseThrow(() -> new CustomException(INVALID_CHAT_ROOM_STATE)); | ||
|
|
||
| ChatMessage chatMessage = new ChatMessage( | ||
| "", | ||
| senderId, | ||
| chatRoom | ||
| ); | ||
|
|
||
| for (String imageUrl : chatImageSendRequest.imageUrls()) { | ||
| String thumbnailUrl = generateThumbnailUrl(imageUrl); | ||
|
|
||
| new ChatAttachment( | ||
| true, | ||
| imageUrl, | ||
| thumbnailUrl, | ||
| chatMessage | ||
| ); | ||
| } | ||
|
|
||
| chatMessageRepository.save(chatMessage); | ||
|
|
||
| ChatMessageSendResponse chatMessageResponse = ChatMessageSendResponse.from(chatMessage); | ||
| simpMessageSendingOperations.convertAndSend("/topic/chat/" + roomId, chatMessageResponse); | ||
| } | ||
|
|
There was a problem hiding this comment.
💡 Verification agent
🧩 Analysis chain
첨부 생성 후 ChatMessage와의 연관관계가 보장되지 않습니다. 컬렉션 연동 없이 new만 하면 DB/응답에서 누락될 수 있어요.
- 현재는 new ChatAttachment(...)만 호출되고, chatMessage.getChatAttachments() 컬렉션에 추가하는 코드가 없습니다.
- JPA에서 부모→자식 cascade를 타려면 보통 부모 컬렉션에 자식을 추가하는 양방향 연동이 필요합니다.
- 현 상태에선 DB 저장 누락 또는 실시간 응답에서 attachments 비어 있음이 발생할 수 있습니다.
아래처럼 부모 컬렉션에 추가하도록 수정해 주세요.
ChatMessage chatMessage = new ChatMessage(
"",
senderId,
chatRoom
);
for (String imageUrl : chatImageSendRequest.imageUrls()) {
String thumbnailUrl = generateThumbnailUrl(imageUrl);
- new ChatAttachment(
- true,
- imageUrl,
- thumbnailUrl,
- chatMessage
- );
+ ChatAttachment attachment = new ChatAttachment(
+ true,
+ imageUrl,
+ thumbnailUrl,
+ chatMessage
+ );
+ // 양방향 연관관계 편의 메서드 사용을 권장합니다.
+ chatMessage.addAttachment(attachment);
}
chatMessageRepository.save(chatMessage);추가로, ChatMessage 엔티티에 아래 편의 메서드와 cascade 설정이 있는지 확인해 주세요.
@OneToMany(mappedBy = "chatMessage", cascade = CascadeType.ALL, orphanRemoval = true)
private List<ChatAttachment> chatAttachments = new ArrayList<>();
public void addAttachment(ChatAttachment attachment) {
this.chatAttachments.add(attachment);
attachment.setChatMessage(this);
}아래 스크립트로 addAttachment 존재 여부와 cascade 설정을 빠르게 점검할 수 있습니다.
🏁 Script executed:
#!/bin/bash
# ChatMessage ↔ ChatAttachment 연관관계 및 편의 메서드 점검
rg -n -C3 'class\s+ChatMessage\b' src/main/java
rg -n -C3 '@OneToMany' src/main/java | rg -n -C1 'ChatAttachment|cascade|orphanRemoval'
rg -n -C2 'addAttachment\s*\(' src/main/java
rg -n -C2 'class\s+ChatAttachment\b' src/main/java
rg -n -C3 '@ManyToOne' src/main/java | rg -n -C1 'ChatMessage'Length of output: 6277
🚨 중요: ChatAttachment가 ChatMessage와 양방향 연관관계로 등록되지 않아 저장 누락 문제가 발생합니다
-
addAttachment 편의 메서드 추가
ChatMessage 엔티티에 child 엔티티를 컬렉션에 등록하고 양방향 연관관계를 설정해 주는 메서드를 반드시 구현해야 합니다.
예시 구현:
java @OneToMany(mappedBy = "chatMessage", cascade = CascadeType.ALL, orphanRemoval = true) private final List<ChatAttachment> chatAttachments = new ArrayList<>(); public void addAttachment(ChatAttachment attachment) { this.chatAttachments.add(attachment); attachment.setChatMessage(this); } -
@onetomany 설정에 orphanRemoval=true 추가
현재cascade = CascadeType.ALL만 지정되어 있어, 컬렉션에서 제거 시 자식 엔티티가 자동으로 삭제되지 않습니다.
orphanRemoval = true를 추가해 주세요:
diff - @OneToMany(mappedBy = "chatMessage", cascade = CascadeType.ALL) + @OneToMany(mappedBy = "chatMessage", cascade = CascadeType.ALL, orphanRemoval = true) private final List<ChatAttachment> chatAttachments = new ArrayList<>(); -
ChatService.sendChatImage에서 addAttachment 호출
이미지마다new ChatAttachment(...)만 생성하면 JPA가 자식을 인식하지 못해 DB에 저장되지 않습니다.
생성 후에 반드시chatMessage.addAttachment(attachment)를 호출하도록 수정해 주세요.
diff for (String imageUrl : chatImageSendRequest.imageUrls()) { String thumbnailUrl = generateThumbnailUrl(imageUrl); - new ChatAttachment( - true, - imageUrl, - thumbnailUrl, - chatMessage - ); + ChatAttachment attachment = new ChatAttachment( + true, + imageUrl, + thumbnailUrl, + chatMessage + ); + chatMessage.addAttachment(attachment); }
이 세 가지 수정을 적용하면 ChatAttachment가 ChatMessage에 올바르게 연관 등록되어 DB 저장 및 실시간 응답에서 누락되지 않습니다.
🤖 Prompt for AI Agents
In src/main/java/com/example/solidconnection/chat/service/ChatService.java
around lines 178-209: ChatAttachment instances are being constructed but not
linked to the ChatMessage bidirectional relationship so they are not persisted;
to fix, add an addAttachment(…) helper to ChatMessage that adds the attachment
to its chatAttachments list and sets attachment.setChatMessage(this), update the
ChatMessage @OneToMany mapping to include orphanRemoval = true (cascade =
CascadeType.ALL, orphanRemoval = true), and modify sendChatImage to call
chatMessage.addAttachment(attachment) for each newly created ChatAttachment so
JPA sees and cascades the child entities to the DB.
There was a problem hiding this comment.
이건 적용해야 할 것 같습니다 ChatMessage 객체 생성 후 addAttachment 메서드 구현하여 추가하는 게 좋을 거 같습니다 orphanRemoval 도요 !
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!
| @PostMapping("/chat") | ||
| public ResponseEntity<List<UploadedFileUrlResponse>> uploadChatImage( | ||
| @RequestParam("files") List<MultipartFile> imageFiles | ||
| ) { | ||
| List<UploadedFileUrlResponse> chatImageUrls = s3Service.uploadFiles(imageFiles, ImgType.CHAT); | ||
| return ResponseEntity.ok(chatImageUrls); | ||
| } | ||
|
|
There was a problem hiding this comment.
💡 Verification agent
🧩 Analysis chain
채팅 이미지 다중 업로드 엔드포인트에 인증·밸리데이션을 추가해 주세요.
- 인증: 기존 /file/* 엔드포인트의 보안 규칙을 재사용하더라도, 채팅 기능 특성상 업로더 식별이 중요합니다. @Authorizeduser long siteUserId 파라미터를 추가해 호출 주체를 명시적으로 확보하는 것을 권장합니다.
- 밸리데이션: 프런트 제약과 맞춰 서버에서도 파일 개수(1~10) 제약을 선언적으로 검증(@SiZe)하고, 비어 있거나 null인 경우 400을 반환해 주세요.
- 미디어 타입: consumes = multipart/form-data를 명시하면 클라이언트/게이트웨이 간 협업 오류를 줄일 수 있습니다.
선택 범위 내에서 적용 가능한 최소 diff입니다(추가 import: org.springframework.http.MediaType, jakarta.validation.constraints.Size, 클래스/메서드에 @validated 필요).
- @PostMapping("/chat")
- public ResponseEntity<List<UploadedFileUrlResponse>> uploadChatImage(
- @RequestParam("files") List<MultipartFile> imageFiles
- ) {
- List<UploadedFileUrlResponse> chatImageUrls = s3Service.uploadFiles(imageFiles, ImgType.CHAT);
+ @PostMapping(value = "/chat", consumes = org.springframework.http.MediaType.MULTIPART_FORM_DATA_VALUE)
+ public ResponseEntity<List<UploadedFileUrlResponse>> uploadChatImage(
+ @AuthorizedUser long siteUserId,
+ @RequestParam("files") @jakarta.validation.constraints.Size(min = 1, max = 10) List<MultipartFile> imageFiles
+ ) {
+ // siteUserId는 감사/추적용으로 로깅하거나 업로드 정책에 활용 가능
+ List<UploadedFileUrlResponse> chatImageUrls = s3Service.uploadFiles(imageFiles, ImgType.CHAT);
return ResponseEntity.ok(chatImageUrls);
}아래 스크립트로 보안 설정에서 /file/** 경로의 접근 정책을 확인해 주세요(무권한 업로드 방지).
🏁 Script executed:
#!/bin/bash
# SecurityFilterChain 내 /file/** 접근 정책 점검
rg -n -C3 -P '(SecurityFilterChain|WebSecurityConfigurerAdapter|@EnableWebSecurity)' --glob '!**/build/**' --glob '!**/out/**'
rg -n -C2 -P '(/file/\*\*|/file/|S3Controller)' --glob '!**/build/**' --glob '!**/out/**'
rg -n -C2 -P '(permitAll|hasRole|authenticated)\(' --glob '!**/build/**' --glob '!**/out/**'Length of output: 4590
인증·밸리데이션 및 보안 설정 강화 제안
아래 4가지 변경 사항을 적용해 주세요:
- 인증(Authenticated User)
1.1 메서드 시그니처에@AuthorizedUser long siteUserId파라미터를 추가해 호출 주체를 명시적으로 확보합니다.
1.2SecurityConfiguration내 SecurityFilterChain 설정에서/file/**경로에도.authenticated()를 적용해, 현재.anyRequest().permitAll()로 노출된 업로드 엔드포인트를 보호합니다. - 요청 검증(Validation)
2.1 컨트롤러 클래스 또는 메서드에@Validated를 선언해 밸리데이션을 활성화합니다.
2.2@RequestParam("files")파라미터에@Size(min = 1, max = 10)(및 필요 시@NotEmpty)를 붙여, 1~10개 파일만 허용하도록 선언적으로 검증합니다. - 미디어 타입 명시(Media Type)
3.1@PostMapping에consumes = MediaType.MULTIPART_FORM_DATA_VALUE를 지정해 클라이언트·게이트웨이 간 오해를 줄입니다. - 최소 적용 예시(Diff)
아래 스니펫을 참고해 보세요(추가 import:org.springframework.http.MediaType,jakarta.validation.constraints.Size, 클래스에@Validated필요).
- @PostMapping("/chat")
- public ResponseEntity<List<UploadedFileUrlResponse>> uploadChatImage(
- @RequestParam("files") List<MultipartFile> imageFiles
- ) {
- List<UploadedFileUrlResponse> chatImageUrls = s3Service.uploadFiles(imageFiles, ImgType.CHAT);
+ @PostMapping(
+ value = "/chat",
+ consumes = org.springframework.http.MediaType.MULTIPART_FORM_DATA_VALUE
+ )
+ public ResponseEntity<List<UploadedFileUrlResponse>> uploadChatImage(
+ @AuthorizedUser long siteUserId,
+ @RequestParam("files")
+ @jakarta.validation.constraints.Size(min = 1, max = 10)
+ List<MultipartFile> imageFiles
+ ) {
+ // siteUserId는 감사/추적 및 업로드 정책에 활용할 수 있습니다.
+ List<UploadedFileUrlResponse> chatImageUrls = s3Service.uploadFiles(imageFiles, ImgType.CHAT);
return ResponseEntity.ok(chatImageUrls);
}—
스크립트 결과: SecurityFilterChain 설정이
.requestMatchers("/connect/**").authenticated()
.requestMatchers("/admin/**").hasRole(ADMIN.name())
.anyRequest().permitAll()
형태로 /file/** 경로는 인증 없이 허용되고 있습니다. 따라서 메서드 레벨 인증과 보안 필터체인 수정을 함께 적용해 주세요.
📝 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.
| @PostMapping("/chat") | |
| public ResponseEntity<List<UploadedFileUrlResponse>> uploadChatImage( | |
| @RequestParam("files") List<MultipartFile> imageFiles | |
| ) { | |
| List<UploadedFileUrlResponse> chatImageUrls = s3Service.uploadFiles(imageFiles, ImgType.CHAT); | |
| return ResponseEntity.ok(chatImageUrls); | |
| } | |
| @PostMapping( | |
| value = "/chat", | |
| consumes = org.springframework.http.MediaType.MULTIPART_FORM_DATA_VALUE | |
| ) | |
| public ResponseEntity<List<UploadedFileUrlResponse>> uploadChatImage( | |
| @AuthorizedUser long siteUserId, | |
| @RequestParam("files") | |
| @jakarta.validation.constraints.Size(min = 1, max = 10) | |
| List<MultipartFile> imageFiles | |
| ) { | |
| // siteUserId는 감사/추적 및 업로드 정책에 활용할 수 있습니다. | |
| List<UploadedFileUrlResponse> chatImageUrls = s3Service.uploadFiles(imageFiles, ImgType.CHAT); | |
| return ResponseEntity.ok(chatImageUrls); | |
| } |
🤖 Prompt for AI Agents
In src/main/java/com/example/solidconnection/s3/controller/S3Controller.java
around lines 72-79, the uploadChatImage endpoint is exposed without validation
or authenticated user context; update the controller to require an authenticated
caller and validate inputs by adding an @AuthorizedUser long siteUserId
parameter to the method signature, annotate the controller (or method) with
@Validated, add @Size(min = 1, max = 10) (and @NotEmpty if desired) to the
@RequestParam("files") List<MultipartFile> imageFiles parameter, and specify
consumes = MediaType.MULTIPART_FORM_DATA_VALUE on the @PostMapping;
additionally, modify your SecurityConfiguration SecurityFilterChain to require
authentication for the upload path (e.g., add
.requestMatchers("/file/**").authenticated() and remove/adjust the
.anyRequest().permitAll() exposure); also add necessary imports for MediaType
and validation annotations.
There was a problem hiding this comment.
이건 비단 chat 엔드포인트에만 해당하는 건 아니네요 ... 추후 구현하는 것이 좋을 것 같습니다
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!
| public enum MessageType { | ||
| TEXT, | ||
| IMAGE, | ||
| } |
There was a problem hiding this comment.
이게 추가된 히스토리가 어떤거였나요? 제가 기억을 못하는 것일 수도 있습니다..
There was a problem hiding this comment.
디코에 올려놓겠습니다 ! 회의할 때 나왔던 내용인데, 수연 님과 DM으로 결정했었습니다
| new ChatAttachment( | ||
| true, | ||
| imageUrl, | ||
| thumbnailUrl, | ||
| chatMessage | ||
| ); | ||
| } |
| private String generateThumbnailUrl(String originalUrl) { | ||
| try { | ||
| String fileName = originalUrl.substring(originalUrl.lastIndexOf('/') + 1); | ||
|
|
||
| String nameWithoutExt = fileName.substring(0, fileName.lastIndexOf('.')); | ||
| String extension = fileName.substring(fileName.lastIndexOf('.')); | ||
|
|
||
| String thumbnailFileName = nameWithoutExt + "_thumb" + extension; | ||
|
|
||
| String thumbnailUrl = originalUrl.replace("chat/images/", "chat/thumbnails/") | ||
| .replace(fileName, thumbnailFileName); | ||
|
|
||
| return thumbnailUrl; | ||
|
|
||
| } catch (Exception e) { | ||
| return originalUrl; | ||
| } | ||
| } |
| String fileName = originalUrl.substring(originalUrl.lastIndexOf('/') + 1); | ||
|
|
There was a problem hiding this comment.
확장자 검증이 따로 없는 거 같은데 상관없나요? 사진만 받기로 했던 거 같아서요
There was a problem hiding this comment.
확장자 검증은 S3업로드 함수에서 합니다! 이후 인증로직 추가되면 URL자체를 검증하기 때문에 따로 확장자 검증은 필요없을 것 같습니당
| @Test | ||
| void 채팅_참여자가_아니면_이미지_메시지를_전송할_수_없다() { |
| assertAll( | ||
| () -> assertThat(response.attachments().get(0).url()).isEqualTo(imageUrls.get(0)), | ||
| () -> assertThat(response.attachments().get(0).thumbnailUrl()).isEqualTo( | ||
| "https://bucket.s3.ap-northeast-2.amazonaws.com/chat/thumbnails/example_thumb.jpg" |
There was a problem hiding this comment.
앞부분은 따로 상수로 빼서 테스트 해도 좋을 거 같네요
whqtker
left a comment
whqtker
left a comment
There was a problem hiding this comment.
확인했습니다 ! 양방향 연관관계 부분은 한 번 확인해주세요 !
| public enum MessageType { | ||
| TEXT, | ||
| IMAGE, | ||
| } |
There was a problem hiding this comment.
디코에 올려놓겠습니다 ! 회의할 때 나왔던 내용인데, 수연 님과 DM으로 결정했었습니다
| public record ChatImageSendRequest( | ||
| @NotNull(message = "이미지 URL 목록은 필수입니다") | ||
| @Size(min = 1, max = 10, message = "이미지는 1~10개까지 가능합니다") | ||
| List<@NotBlank(message = "이미지 URL은 필수입니다") String> imageUrls |
There was a problem hiding this comment.
검증 로직은 곧 추가할 예정입니다!
| MessageType messageType = chatMessage.getChatAttachments().isEmpty() | ||
| ? MessageType.TEXT | ||
| : MessageType.IMAGE; | ||
|
|
There was a problem hiding this comment.
기억이 가물가물한데 현재 저희는 일단 이미지만 작업하는 거 맞나요 ? 그렇다면 이건 추후에 반영해도 될 거 같습니다
|
|
||
| ChatMessageSendResponse chatMessageResponse = ChatMessageSendResponse.from(chatMessage); | ||
| simpMessageSendingOperations.convertAndSend("/topic/chat/" + roomId, chatMessageResponse); | ||
| } |
There was a problem hiding this comment.
메시지 전송 실패해도 DB에 트랜잭션 반영이 되는 거 같은데 이건 추후에 롤백 기능 넣어야겠네요
| @Transactional | ||
| public void sendChatImage(ChatImageSendRequest chatImageSendRequest, long siteUserId, long roomId) { | ||
| long senderId = chatParticipantRepository.findByChatRoomIdAndSiteUserId(roomId, siteUserId) | ||
| .orElseThrow(() -> new CustomException(CHAT_PARTICIPANT_NOT_FOUND)) | ||
| .getId(); | ||
|
|
||
| ChatRoom chatRoom = chatRoomRepository.findById(roomId) | ||
| .orElseThrow(() -> new CustomException(INVALID_CHAT_ROOM_STATE)); | ||
|
|
||
| ChatMessage chatMessage = new ChatMessage( | ||
| "", | ||
| senderId, | ||
| chatRoom | ||
| ); | ||
|
|
||
| for (String imageUrl : chatImageSendRequest.imageUrls()) { | ||
| String thumbnailUrl = generateThumbnailUrl(imageUrl); | ||
|
|
||
| new ChatAttachment( | ||
| true, | ||
| imageUrl, | ||
| thumbnailUrl, | ||
| chatMessage | ||
| ); | ||
| } | ||
|
|
||
| chatMessageRepository.save(chatMessage); | ||
|
|
||
| ChatMessageSendResponse chatMessageResponse = ChatMessageSendResponse.from(chatMessage); | ||
| simpMessageSendingOperations.convertAndSend("/topic/chat/" + roomId, chatMessageResponse); | ||
| } | ||
|
|
There was a problem hiding this comment.
이건 적용해야 할 것 같습니다 ChatMessage 객체 생성 후 addAttachment 메서드 구현하여 추가하는 게 좋을 거 같습니다 orphanRemoval 도요 !
| @PostMapping("/chat") | ||
| public ResponseEntity<List<UploadedFileUrlResponse>> uploadChatImage( | ||
| @RequestParam("files") List<MultipartFile> imageFiles | ||
| ) { | ||
| List<UploadedFileUrlResponse> chatImageUrls = s3Service.uploadFiles(imageFiles, ImgType.CHAT); | ||
| return ResponseEntity.ok(chatImageUrls); | ||
| } | ||
|
|
There was a problem hiding this comment.
이건 비단 chat 엔드포인트에만 해당하는 건 아니네요 ... 추후 구현하는 것이 좋을 것 같습니다
There was a problem hiding this comment.
Actionable comments posted: 2
♻️ Duplicate comments (1)
src/main/java/com/example/solidconnection/chat/dto/ChatImageSendRequest.java (1)
8-12: URL 스킴·호스트 검증을 DTO 수준에서 보강해 주세요.프론트의 URL 교체 방지 계획과 별개로, 서버에서 최소한 https 스킴과 허용 도메인만 받도록 막아두면 안전성이 크게 올라갑니다. 이 코멘트는 이전 리뷰의 “URL 검증” 피드백을 구체화한 연장선입니다.
- https 강제: DTO에 @pattern을 추가해 http·javascript·data 스킴 등을 차단하세요.
- (선택) Hibernate Validator 사용 시: @url(protocol="https", …)을 활용하면 가독성이 좋습니다.
- 도메인 화이트리스트: 환경별 CDN/S3 도메인을 서비스 레이어에서 Set으로 명시적으로 제한하세요.
옵션 A: 표준 Jakarta Validation만으로 적용(@pattern)
@@ -import jakarta.validation.constraints.NotBlank; +import jakarta.validation.constraints.NotBlank; +import jakarta.validation.constraints.Pattern; @@ - List<@NotBlank(message = "이미지 URL은 필수입니다") String> imageUrls + List< + @NotBlank(message = "이미지 URL은 필수입니다") + @Pattern( + regexp = "^https://[A-Za-z0-9.-]+(?::[0-9]+)?/.*$", + message = "이미지 URL은 https 스킴이어야 합니다" + ) + String + > imageUrls옵션 B: Hibernate Validator의 @url 사용(도입되어 있다면 권장)
@@ -import jakarta.validation.constraints.NotBlank; +import jakarta.validation.constraints.NotBlank; +import org.hibernate.validator.constraints.URL; @@ - List<@NotBlank(message = "이미지 URL은 필수입니다") String> imageUrls + List< + @NotBlank(message = "이미지 URL은 필수입니다") + @URL(protocol = "https", message = "이미지 URL은 https만 허용합니다") + String + > imageUrls서비스 레이어 예시(허용 호스트 화이트리스트 확인)
// ChatService 등에서 private static final Set<String> ALLOWED_HOSTS = Set.of( "cdn.example.com", "your-bucket.s3.amazonaws.com" ); private void assertAllowedHosts(List<String> urls) { for (String u : urls) { String host = URI.create(u).getHost(); if (!ALLOWED_HOSTS.contains(host)) { throw new IllegalArgumentException("허용되지 않은 이미지 호스트: " + host); } } }원하시면 허용 호스트/경로(prefix)까지 검증하는 커스텀 Constraint(@AllowedImageUrl)도 빠르게 생성해 드릴게요.
🧹 Nitpick comments (9)
src/main/java/com/example/solidconnection/chat/dto/ChatImageSendRequest.java (3)
11-11: 개별 URL 길이 상한을 두어 비정상 입력을 조기 차단하세요.브라우저·프록시 현실을 감안하면 2KB 내외가 실무적으로 안전합니다. 2048자 상한을 권장합니다.
- List<@NotBlank(message = "이미지 URL은 필수입니다") String> imageUrls + List< + @NotBlank(message = "이미지 URL은 필수입니다") + @Size(max = 2048, message = "이미지 URL은 2048자 이하여야 합니다") + String + > imageUrls
12-14: 레코드라도 내부 List는 가변입니다. 방어적 복사로 불변성을 보장하세요.요청 객체가 다른 계층에서 수정되지 않도록 수신 시점에 불변 리스트로 고정해 두는 편이 안전합니다.
) { - - + public ChatImageSendRequest { + // 외부 가변 리스트 참조를 끊고, 불변 리스트로 고정 + imageUrls = List.copyOf(imageUrls); + } }참고로 중복 URL 제거까지 원하시면 LinkedHashSet 기반으로 정렬 유지·중복 제거하는 변형도 제안드릴 수 있어요.
8-12: API 문서화를 위해 @Schema를 추가해 예시와 제약을 노출하세요.클라이언트·프론트 합의를 빠르게 하기 좋고, 제약을 문서로 자동 반영할 수 있습니다.
+import io.swagger.v3.oas.annotations.media.Schema; @@ -public record ChatImageSendRequest( +public record ChatImageSendRequest( + @Schema(description = "채팅 이미지 URL 목록 (최소 1, 최대 10)", example = "[\"https://cdn.example.com/chat/2025/08/24/abc.jpg\"]") @NotNull(message = "이미지 URL 목록은 필수입니다") @Size(min = 1, max = 10, message = "이미지는 1~10개까지 가능합니다") List<@NotBlank(message = "이미지 URL은 필수입니다") String> imageUrls ) {필요하시다면 OpenAPI 스펙에 “허용 도메인” 설명도 함께 노출해 드리겠습니다.
src/main/java/com/example/solidconnection/chat/domain/ChatAttachment.java (3)
46-48: 2) 연결 책임 단일화: 생성자에서의 양방향 처리 대신, 부모 측(addAttachment)으로만 위임하는 구조로 정리 권장.지금 생성자는 chatMessage 컬렉션에 직접 add까지 수행합니다(라인 41~43). 이와 동시에 ChatMessage.addAttachment도 존재해 중복/불일치 위험이 있습니다. 사이드이펙트가 없는 생성자를 유지하고, 실제 연결은 부모 메서드(또는 setChatMessage) 한 군데로만 일원화하는 편이 안전합니다.
생성자 바깥 변경 제안을 예시로 남깁니다:
public ChatAttachment(boolean isImage, String url, String thumbnailUrl, ChatMessage chatMessage) { this.isImage = isImage; this.url = url; this.thumbnailUrl = thumbnailUrl; if (chatMessage != null) { // 단일 진실의 원천으로 위임 chatMessage.addAttachment(this); } }
46-48: 3) 가시성 축소 검토: protected → package-private로 축소하여 오남용 표면을 줄여보세요.엔티티 상속을 적극적으로 쓰지 않는 구조라면, 동일 패키지 내부(예: ChatMessage)에서만 호출 가능하도록 package-private로 좁히는 것이 안전합니다. 팀 컨벤션에 맞춘 결정이므로 선택 사항입니다.
46-48: 5) 중복 판별 신뢰성: equals/hashCode 구현 고려(Set 사용 시 특히 중요)..contains 체크는 현재 객체 동일성에 의존합니다. 프록시/영속성 컨텍스트 경계를 넘나드는 경우를 대비해 id 기반 equals/hashCode를 도입하거나, 컬렉션을 Set으로 전환하면(아래 ChatMessage 코멘트 참고) 중복 방지에 유리합니다.
src/main/java/com/example/solidconnection/chat/domain/ChatMessage.java (3)
48-51: 2) addAttachment 단순화: 리스트 조작은 setChatMessage(...)에 위임하여 중복 추가를 방지하세요.현재는 리스트에 먼저 add 후 setChatMessage를 호출합니다. setChatMessage가 양쪽 컬렉션을 관리하도록 만들면, addAttachment는 이를 위임만 해도 충분합니다. 가독성과 불변식 유지가 좋아집니다.
아래처럼 치환을 제안드립니다.
- public void addAttachment(ChatAttachment attachment) { - this.chatAttachments.add(attachment); - attachment.setChatMessage(this); - } + public void addAttachment(ChatAttachment attachment) { + if (attachment == null) return; + attachment.setChatMessage(this); + }
48-51: 3) removeAttachment 보강 제안: 고아 삭제(orphanRemoval)와 이동(use-case) 모두를 안전하게 다루기 위함입니다.분리 시 반대편 참조 해제까지 일괄 처리하는 편의 메서드가 있으면 서비스 레이어가 깔끔해집니다.
추가 메서드 예시(참고용):
public void removeAttachment(ChatAttachment attachment) { if (attachment == null) return; if (this.chatAttachments.contains(attachment)) { attachment.setChatMessage(null); // setChatMessage가 컬렉션 정리를 수행 } }
36-36: 4) 중복 방지를 중시한다면 Set 전환을 고려하세요(List 유지 사유가 없다면).표시 순서 유지가 중요하지 않다면,
Set<ChatAttachment>가 자연스럽게 중복을 차단합니다. equals/hashCode(id 기반)를 함께 도입하면 효과가 극대화됩니다. UI 정렬 기준이 존재한다면, 정렬 기준을 명시한 SortedSet도 대안입니다.
📜 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 (4)
src/main/java/com/example/solidconnection/chat/domain/ChatAttachment.java(1 hunks)src/main/java/com/example/solidconnection/chat/domain/ChatMessage.java(2 hunks)src/main/java/com/example/solidconnection/chat/dto/ChatImageSendRequest.java(1 hunks)src/main/java/com/example/solidconnection/chat/service/ChatService.java(2 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- src/main/java/com/example/solidconnection/chat/service/ChatService.java
🔇 Additional comments (2)
src/main/java/com/example/solidconnection/chat/dto/ChatImageSendRequest.java (1)
8-12: 기본 컨테이너·요소 검증 구성이 알차고 일관적입니다.List에 @NotNull·@SiZe, 요소에 @notblank를 컨테이너 요소 제약으로 선언한 점이 좋습니다.
src/main/java/com/example/solidconnection/chat/domain/ChatMessage.java (1)
36-36: 1) orphanRemoval=true 도입은 적절합니다만, 첨부 이동 시 삭제-재연결 타이밍에 유의해주세요.orphanRemoval=true에서는 부모 컬렉션에서 제거되는 순간 고아 삭제가 트리거됩니다. 첨부를 A → B 메시지로 이동할 때, 이전 컬렉션에서 제거와 새 부모로의 연결이 동일 트랜잭션/플러시 흐름에서 안전하게 처리되도록, 반드시 단일 메서드(예: attachment.setChatMessage(...) 또는 addAttachment)로 일괄 수행해 주세요. 앞서 제안한 setChatMessage 보강으로 대부분의 리스크를 해소할 수 있습니다.
| protected void setChatMessage(ChatMessage chatMessage) { | ||
| this.chatMessage = chatMessage; | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion
1) 양방향 연관관계 정합성 보강: setChatMessage(...)가 이전 부모 분리와 중복 방지까지 책임지도록 바꿔주세요.
현재는 단순 대입만 수행하여, 첨부파일이 다른 ChatMessage로 이동하는 경우 이전 부모의 컬렉션에 잔존하거나, 대상 부모 컬렉션에 중복으로 들어갈 수 있습니다. orphanRemoval=true를 안정적으로 활용하려면 이동/분리 시 컬렉션 정리를 함께 수행해야 합니다.
아래처럼 메서드를 치환해 주세요.
- protected void setChatMessage(ChatMessage chatMessage) {
- this.chatMessage = chatMessage;
- }
+ protected void setChatMessage(ChatMessage chatMessage) {
+ if (this.chatMessage == chatMessage) return;
+ // 1) 기존 부모에서 분리
+ if (this.chatMessage != null) {
+ this.chatMessage.getChatAttachments().remove(this);
+ }
+ // 2) 새 부모에 연결
+ this.chatMessage = chatMessage;
+ if (chatMessage != null && !chatMessage.getChatAttachments().contains(this)) {
+ chatMessage.getChatAttachments().add(this);
+ }
+ }📝 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.
| protected void setChatMessage(ChatMessage chatMessage) { | |
| this.chatMessage = chatMessage; | |
| } | |
| protected void setChatMessage(ChatMessage chatMessage) { | |
| if (this.chatMessage == chatMessage) return; | |
| // 1) 기존 부모에서 분리 | |
| if (this.chatMessage != null) { | |
| this.chatMessage.getChatAttachments().remove(this); | |
| } | |
| // 2) 새 부모에 연결 | |
| this.chatMessage = chatMessage; | |
| if (chatMessage != null && !chatMessage.getChatAttachments().contains(this)) { | |
| chatMessage.getChatAttachments().add(this); | |
| } | |
| } |
🤖 Prompt for AI Agents
In src/main/java/com/example/solidconnection/chat/domain/ChatAttachment.java
around lines 46 to 48, replace the simple setter so it maintains bidirectional
consistency: when changing chatMessage remove this attachment from the previous
chatMessage's attachments collection (if non-null), avoid adding duplicates to
the new chatMessage's attachments collection, and handle null to support
orphan/removal; ensure you update the field after removal/addition and preserve
orphanRemoval behavior by not leaving stale references in either parent
collection.
💡 Verification agent
🧩 Analysis chain
4) DB 제약 검토: 첨부가 항상 메시지에 소속된다면 @joincolumn(nullable = false, optional = false)로 일관성 보장 필요.
첨부가 독립적으로 존재하지 않는 도메인이라면, DB 차원에서도 NOT NULL 제약을 두는 것이 안전합니다. 반대로 업로드-임시 단계에서 메시지 없이 생성될 수 있다면 현재 설정(Nullable 허용)이 타당합니다. 코드 전반 사용처를 한번 확인 부탁드립니다.
레포에서 ChatAttachment 생성 패턴을 점검하는 간단 스크립트를 안내드립니다(루트에서 실행):
nullable=false로 전환 시, 아래와 같이 선언을 명시할 수 있습니다(참고용):
@ManyToOne(fetch = FetchType.LAZY, optional = false)
@JoinColumn(name = "chat_message_id", nullable = false)
private ChatMessage chatMessage;🏁 Script executed:
#!/bin/bash
# ChatAttachment 직접 생성 및 null 전달 여부 점검
rg -nP --type=java -C2 '\bnew\s+ChatAttachment\s*\('
# setChatMessage 혹은 addAttachment 사용 편향 확인
rg -nP --type=java -C2 '\bsetChatMessage\s*\('
rg -nP --type=java -C2 '\baddAttachment\s*\('Length of output: 3508
DB 제약조건 강제 설정 안내
-
검증 결과
- 테스트의
ChatAttachmentFixtureBuilder에서는 생성 시 항상chatMessage를 파라미터로 전달하며, ChatService에서는new ChatAttachment(true, imageUrl, thumbnailUrl, null)로 초기화하더라도 즉시addAttachment(…)호출을 통해chatMessage가 설정됨을 확인했습니다.
- 테스트의
-
매핑 수정 제안
-
도메인 불변식을 DB 차원에서 보장하기 위해 아래와 같이
optional=false,nullable=false를 명시하여 NOT NULL 제약을 추가 적용하세요.@ManyToOne(fetch = FetchType.LAZY, optional = false) @JoinColumn(name = "chat_message_id", nullable = false) private ChatMessage chatMessage;
-
-
참고 사항
- 현재 설정에서도 런타임 오류는 발생하지 않으나, 애플리케이션 로직과 DB 스키마 간 일관성을 위해 반드시 적용을 권장드립니다.
🤖 Prompt for AI Agents
In src/main/java/com/example/solidconnection/chat/domain/ChatAttachment.java
around lines 46 to 48, the JPA mapping for chatMessage is currently allowing
nulls at the DB level; update the field mapping to enforce the domain invariant
by marking the ManyToOne relationship as non-optional and the join column as
non-nullable (i.e., set optional=false on @ManyToOne and nullable=false on
@JoinColumn) so the database will have a NOT NULL constraint that matches the
application logic.
There was a problem hiding this comment.
반영해도 좋을 거 같네요~ 추가로 혹시 protected로 하신 이유가 있으신가요?
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.
저도 protected로 하신 이유가 있는지 궁금합니다 !
There was a problem hiding this comment.
혹시 service에서 직접 호출하는 경우가 생길까봐 protected로 했는데... 문제가 될까요??
There was a problem hiding this comment.
아 서비스 계층이 아니라 ChatMessage 에서 해당 메서드를 호출하고 있군요 ... 그렇다면 protected 가 적합한 것 같네요
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (3)
src/main/java/com/example/solidconnection/chat/domain/ChatAttachment.java (3)
47-58: [2] 양방향 연관관계 정합성 메서드(setChatMessage) 구현이 깔끔합니다.
- 동일 객체 재할당 단축(return), 이전 부모 컬렉션 제거, 새 부모 컬렉션 중복 방지까지 한 번에 처리한 점이 좋습니다.
- orphanRemoval=true와 맞물려 객체 이동/분리 시 메모리·DB 양쪽 참조가 깨지지 않도록 안전하게 동작합니다.
- 다만 ChatMessage 쪽 컬렉션 타입이 List라면 중복 삽입 가능성이 구조적으로 열려 있어, Set으로 선언하면 방어가 한층 견고해집니다(동일 인스턴스 중복 방지).
[summary]
1) 현재 구현은 정합성을 잘 지킵니다.
2) 컬렉션 타입이 List라면 중복 삽입 여지가 있어 Set 전환을 권장합니다.
[/summary]
37-45: [3] 생성자에서 양방향 세팅을 직접 하지 말고 setChatMessage로 위임해 중복·누락을 방지하세요.
- 현재 생성자는 필드 직접 대입 후 부모 컬렉션에 add를 수행하여, 이후 addAttachment(setChatMessage 호출 포함)와 조합되면 중복 삽입 위험이 생길 수 있습니다.
- 생성자에서도 setChatMessage(chatMessage)를 호출하도록 일원화하면, 이동/분리/중복 방지 로직이 단일 진입점으로 모여 실수를 줄일 수 있습니다.
- 정리된 진입점을 통해 equals/hashCode 구현 여부와 무관하게 동일 인스턴스 중복을 안정적으로 피할 수 있습니다.
권장 변경(diff):
- this.chatMessage = chatMessage; - if (chatMessage != null) { - chatMessage.getChatAttachments().add(this); - } + this.setChatMessage(chatMessage);[summary]
1) 생성자 내부 로직을 setChatMessage로 위임합니다.
2) 중복 삽입 리스크를 제거합니다.
[/summary]Also applies to: 47-58
25-33: [4] (선택) URL/썸네일 URL 길이 정책 재검토를 제안드립니다.
- length=500은 여유롭지만, 추후 인덱싱(검색/정합성 검사)을 고려하면 255/350 같은 현실적 상한도 검토할 가치가 있습니다.
- 썸네일 URL이 규칙적으로 원본+“_thumb” 패턴이라면, 저장 시 하드코딩 대신 규칙 기반 파생을 서비스/밸류 오브젝트로 캡슐화하면 변경에 견고합니다.
[summary]
1) 인덱싱 가능성을 보면 URL 길이 상한 최적화 여지가 있습니다.
2) “원본→썸네일” 파생 규칙을 캡슐화해 변경 내성을 높이세요.
[/summary]
📜 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/main/java/com/example/solidconnection/chat/domain/ChatAttachment.java(3 hunks)
🔇 Additional comments (1)
src/main/java/com/example/solidconnection/chat/domain/ChatAttachment.java (1)
10-10: 변경 사항 요약
- Mapping에
@JoinColumn(nullable=false)를 명시합니다.
src/main/java/com/example/solidconnection/chat/domain/ChatAttachment.java34–35행의
@ManyToOne(fetch = FetchType.LAZY, optional = false)바로 위에를 추가했습니다.@JoinColumn(name = "chat_message_id", nullable = false)- DB 제약은 이미 마이그레이션에서 포함되어 있습니다.
src/main/resources/db/migration/V24__add_chat_related_tables.sql에서로 생성되므로, 별도 NOT NULL 제약 추가 마이그레이션은 불필요합니다.chat_message_id BIGINT NOT NULL- 서비스 로직 및 테스트 흐름을 점검합니다.
ChatService.sendChatMessage에서new ChatAttachment(..., null)후
addAttachment호출로 부모 참조를 설정하기 때문에, 변경 후에도 기능 및 테스트가
정상 동작하는지 간단히 확인해 주세요.
| protected void setChatMessage(ChatMessage chatMessage) { | ||
| this.chatMessage = chatMessage; | ||
| } |
There was a problem hiding this comment.
저도 protected로 하신 이유가 있는지 궁금합니다 !
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (1)
src/main/java/com/example/solidconnection/chat/service/ChatService.java (1)
215-221: 2) 양방향 연관관계 편의 메서드 적용 확인
- addAttachment 호출로 부모-자식 연동 및 cascade persist 경로가 확보되었습니다. 좋습니다.
- 위 코멘트대로 생성자에서 null 전달만 제거하면 완성입니다.
🧹 Nitpick comments (1)
src/main/java/com/example/solidconnection/chat/service/ChatService.java (1)
102-108: 4) 채팅방 목록 미리보기 공백 이슈(이미지 메시지 대응)
- latestMessage.getContent()가 빈 문자열이면 목록 미리보기가 공백이 됩니다. 이미지 메시지일 때 “[이미지]” 등 플레이스홀더를 노출하는 편이 UX에 유리합니다.
- messageType 또는 attachments 존재 여부로 분기해 주세요.
예시:
- latestMessage != null ? latestMessage.getContent() : "", + (latestMessage == null) + ? "" + : (latestMessage.hasImage() ? "[이미지]" : latestMessage.getContent()),
- hasImage()가 없다면 attachments.isEmpty() 체크 또는 messageType 기준으로 구현하세요.
📜 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/main/java/com/example/solidconnection/chat/service/ChatService.java(2 hunks)
⏰ 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)
src/main/java/com/example/solidconnection/chat/service/ChatService.java (3)
8-8: 임포트 추가 확인
- ChatAttachment 사용을 위한 임포트 추가 건 문제 없습니다.
13-13: 임포트 추가 확인
- ChatImageSendRequest 임포트 추가 건 문제 없습니다.
228-245: generateThumbnailUrl 메서드 견고화 및 하드코딩 축소 제안
- 예외 대신 안전한 분기 처리
-lastIndexOf('.')실패 시 예외를 던지지 않고 명시적 if 문으로 원본 URL 반환- URI 기반 경로 조작
-java.net.URI로 URL 파싱 후 path만 추출해 "/chat/images/" → "/chat/thumbnails/" 변환- 상수 또는 설정 분리
-"chat/images/","chat/thumbnails/","_thumb"등의 하드코딩 값을 상수 또는 설정 값으로 분리- 테스트 케이스 추가
- 확장자 없음, 다중 점 파일명(a.b.c.png), 쿼리스트링 포함 URL, 다른 경로(URL에 chat/images/ 미포함) 케이스 보강
| @Transactional | ||
| public void sendChatImage(ChatImageSendRequest chatImageSendRequest, long siteUserId, long roomId) { | ||
| long senderId = chatParticipantRepository.findByChatRoomIdAndSiteUserId(roomId, siteUserId) | ||
| .orElseThrow(() -> new CustomException(CHAT_PARTICIPANT_NOT_FOUND)) | ||
| .getId(); | ||
|
|
||
| ChatRoom chatRoom = chatRoomRepository.findById(roomId) | ||
| .orElseThrow(() -> new CustomException(INVALID_CHAT_ROOM_STATE)); | ||
|
|
||
| ChatMessage chatMessage = new ChatMessage( | ||
| "", | ||
| senderId, | ||
| chatRoom | ||
| ); | ||
|
|
||
| for (String imageUrl : chatImageSendRequest.imageUrls()) { | ||
| String thumbnailUrl = generateThumbnailUrl(imageUrl); | ||
|
|
||
| ChatAttachment attachment = new ChatAttachment(true, imageUrl, thumbnailUrl, null); | ||
| chatMessage.addAttachment(attachment); | ||
| } | ||
|
|
||
| chatMessageRepository.save(chatMessage); | ||
|
|
||
| ChatMessageSendResponse chatMessageResponse = ChatMessageSendResponse.from(chatMessage); | ||
| simpMessageSendingOperations.convertAndSend("/topic/chat/" + roomId, chatMessageResponse); | ||
| } |
There was a problem hiding this comment.
💡 Verification agent
🧩 Analysis chain
1) 트랜잭션 커밋 이후에만 브로드캐스트하도록 변경 + 첨부 생성 시 null 제거 제안
-
- 메시지 브로드캐스트는 커밋 이후(AFTER_COMMIT)에 발생해야 일관성이 보장됩니다. 지금은 @transactional 내부에서 즉시 전송되어 롤백 시 “유령 메시지”가 나갈 수 있습니다.
-
- ChatAttachment 생성자에 chatMessage를 null로 전달한 뒤 addAttachment로 다시 세팅하고 있습니다. optional=false 매핑일 경우 중간 상태가 불안정합니다. 바로 chatMessage를 넘기세요.
-
- 입력 URL에 대해 도메인/프리픽스(chat/images/) 검증과 개수 제한(DoS 방지)을 권장합니다. DTO Bean Validation으로도 가능합니다.
-
- messageType=IMAGE 설정/응답 생성 로직이 어디서 결정되는지 재확인해 주세요. from(chatMessage)에서 첨부 유무로 추론하는지, 혹은 엔티티 필드가 있는지요.
제안 diff:
@@
for (String imageUrl : chatImageSendRequest.imageUrls()) {
String thumbnailUrl = generateThumbnailUrl(imageUrl);
- ChatAttachment attachment = new ChatAttachment(true, imageUrl, thumbnailUrl, null);
+ ChatAttachment attachment = new ChatAttachment(true, imageUrl, thumbnailUrl, chatMessage);
chatMessage.addAttachment(attachment);
}
chatMessageRepository.save(chatMessage);
- ChatMessageSendResponse chatMessageResponse = ChatMessageSendResponse.from(chatMessage);
- simpMessageSendingOperations.convertAndSend("/topic/chat/" + roomId, chatMessageResponse);
+ final ChatMessageSendResponse chatMessageResponse = ChatMessageSendResponse.from(chatMessage);
+ org.springframework.transaction.support.TransactionSynchronizationManager.registerSynchronization(
+ new org.springframework.transaction.support.TransactionSynchronization() {
+ @Override
+ public void afterCommit() {
+ simpMessageSendingOperations.convertAndSend("/topic/chat/" + roomId, chatMessageResponse);
+ }
+ }
+ );추가 import(파일 상단):
import org.springframework.transaction.support.TransactionSynchronization;
import org.springframework.transaction.support.TransactionSynchronizationManager;🏁 Script executed:
#!/bin/bash
echo "1) ChatAttachment 클래스 위치 및 정의 검색:"
rg -nP --type=java 'class\s+ChatAttachment' -C5
echo "2) ChatAttachment의 @ManyToOne 매핑 확인:"
rg -n '@ManyToOne' -C3 --glob '*ChatAttachment.java'
echo "3) ChatAttachment 생성자 시그니처 검사:"
rg -n 'ChatAttachment\(' -C3 --glob '*ChatAttachment.java'
echo "4) ChatMessageSendResponse 클래스 정의 검색:"
rg -nP --type=java 'class\s+ChatMessageSendResponse' -C5
echo "5) ChatMessageSendResponse.from() 메서드 구현 확인:"
rg -n 'from\s*\(' -C3 --glob '*ChatMessageSendResponse.java'Length of output: 6951
반영해야 할 변경 사항
- 트랜잭션 커밋 이후에만 메시지를 브로드캐스트하도록 TransactionSynchronizationManager.registerSynchronization 또는 @TransactionalEventListener(phase = AFTER_COMMIT)을 사용해 롤백 시 유령 메시지가 전송되지 않도록 보장해주세요.
- ChatAttachment 생성자에 null 대신 chatMessage를 직접 전달해 ChatAttachment(true, imageUrl, thumbnailUrl, chatMessage) 형태로 생성하고, 이후 chatMessage.addAttachment(attachment)로 양방향 연관관계를 설정해 optional=false 매핑의 불안정성을 해소해주세요.
- chatImageSendRequest.imageUrls에 도메인·경로 프리픽스(chat/images/) 검증 및 최대 개수 제한을 DTO Bean Validation으로 추가해 URL 무결성과 DoS 방지를 강화해주세요.
- ChatMessageSendResponse.from에서 첨부 여부로 MessageType을 추론하는 로직이 의도한 방식인지 재확인해 엔티티 필드로 직접 설정해야 하는지 검토해주세요.
3 participants
관련 이슈
작업 내용
특이 사항
리뷰 요구사항 (선택)