[FIX] 비동기로 인해, ChatMessage가 두 번 저장하려하면서 락 장애 발생하는 오류 수정#263
[FIX] 비동기로 인해, ChatMessage가 두 번 저장하려하면서 락 장애 발생하는 오류 수정#263yooooonshine merged 1 commit intodevelopfrom
Conversation
WalkthroughAI 채팅 요청 처리 흐름을 ID 기반으로 단순화. AiChatMessageService에서 단일 이미지 ID 소유권 검증으로 변경하고, AiServerService의 processAiRequest 메서드 시그니처를 엔터티 목록 대신 messageId와 aiChatImageId를 받도록 수정. AiServerService 내부에서 필요한 엔터티를 직접 조회하는 방식으로 변경. Changes
Sequence DiagramsequenceDiagram
participant Client
participant AiChatMessageService as AiChatMessageService
participant AiServerService as AiServerService
participant DB as Database
Note over AiChatMessageService,AiServerService: 기존 방식 (Before)
Client->>AiChatMessageService: AI 요청 처리
AiChatMessageService->>DB: 여러 이미지 엔터티 조회
AiChatMessageService->>AiChatMessageService: 여러 이미지 소유권 검증
AiChatMessageService->>AiServerService: processAiRequest(userId, message, imageList)
Note over AiChatMessageService,AiServerService: 새로운 방식 (After)
Client->>AiChatMessageService: AI 요청 처리
AiChatMessageService->>DB: 단일 이미지 ID로 소유권 검증
AiChatMessageService->>AiServerService: processAiRequest(userId, messageId, aiChatImageId)
AiServerService->>DB: messageId로 메시지 엔터티 조회
AiServerService->>DB: aiChatImageId로 이미지 엔터티 조회 (선택적)
AiServerService->>AiServerService: 후속 처리 (권한 검증, 분류 등)
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes
Possibly related PRs
Poem
Pre-merge checks and finishing touches✅ Passed checks (5 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/main/java/hanium/modic/backend/domain/ai/aiServer/service/AiServerService.java (1)
279-289: 잠재적 NoSuchElementException 위험Line 280의
.get()호출은 이미지가 존재하지 않을 경우NoSuchElementException을 발생시킬 수 있습니다.시나리오:
AiChatMessageEntity의aiChatImageId는 null이 아님 (hasImage()통과)- 하지만 실제
AiChatImageEntity는 삭제되었거나 존재하지 않음- Line 280에서 예외 발생
다음과 같이 안전하게 처리하세요:
// 이미지 컨텐츠 추가 (이미지가 있는 경우) if (entity.hasImage()) { - AiChatImageEntity aiChatImage = aiChatImageRepository.findById(entity.getAiChatImageId()).get(); + AiChatImageEntity aiChatImage = aiChatImageRepository.findById(entity.getAiChatImageId()) + .orElseThrow(() -> new AppException(ErrorCode.IMAGE_NOT_FOUND_EXCEPTION)); contents.add( AiImageRequestMessageDto.ChatContent.image(
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
src/main/java/hanium/modic/backend/domain/ai/aiChat/service/AiChatMessageService.java(3 hunks)src/main/java/hanium/modic/backend/domain/ai/aiServer/service/AiServerService.java(1 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 (5)
src/main/java/hanium/modic/backend/domain/ai/aiServer/service/AiServerService.java (2)
103-108: 에러 처리 로직 확인됨비동기 메서드 내에서 엔티티를 재조회한 후 상태를 업데이트하고 명시적으로 저장하는 패턴이 올바르게 적용되었습니다. 이는 PR 목표인 중복 저장 문제를 해결합니다.
참고:
@Transactional컨텍스트에서 명시적인save()호출은 변경 감지(dirty checking)로 자동 처리될 수 있지만, 비동기 환경에서는 명시적 저장이 의도를 명확히 하고 더 안전합니다.
172-174: 상태 업데이트 및 저장 로직이 올바르게 적용됨요청 메시지의 상태를
REQUEST로 업데이트하고 명시적으로 저장하는 로직이 에러 처리 패턴과 일관되게 적용되었습니다. 이는 Issue #262의 목표인 "요청 메시지 status를 REQUEST로 변경하여 즉시 반영"을 충족합니다.src/main/java/hanium/modic/backend/domain/ai/aiChat/service/AiChatMessageService.java (3)
54-92: 비동기 처리 리팩토링이 올바르게 적용됨엔티티 대신 ID를 비동기 메서드에 전달하도록 변경한 것은 PR의 핵심 목표인 "중복 저장으로 인한 락 장애" 문제를 해결합니다.
개선 사항:
- Line 59: 단일 이미지 소유권 검증으로 단순화
- Line 65: 비동기 호출 전 이미지 존재 여부 검증
- Line 88: 엔티티 대신 ID 전달하여 비동기 메서드 내부에서 재조회 가능
이는 분리된 엔티티(detached entity) 문제와 중복 저장 시도를 방지하는 올바른 접근 방식입니다.
127-135: 소유권 검증 로직이 효율적으로 개선됨단일 이미지 검증으로 단순화하고
existsByIdAndUserId를 사용하는 것은 좋은 개선입니다.장점:
- 엔티티 전체를 조회하지 않고 존재 여부만 확인하여 성능 최적화
- null 처리가 명확함
- 메서드명이 단수형으로 변경되어 의도가 명확함
137-143: 이미지 존재 검증 로직이 올바르게 작동함비동기 처리 전에 이미지 존재 여부를 검증하는 것은 좋은 방어 코딩입니다.
참고: 검증 시점과 비동기 메서드에서 이미지를 실제로 사용하는 시점 사이에 이미지가 삭제될 수 있는 경쟁 조건(race condition)이 존재합니다.
AiServerService의 Lines 80-82에서 이를 처리하는 방식이 주석과 일치하는지 확인이 필요합니다 (해당 파일의 별도 코멘트 참고).
| final Long messageId, | ||
| final Long aiChatImageId // 없으면 null, null이 아니면 해당 이미지는 반드시 존재해야 함 | ||
| ) { |
There was a problem hiding this comment.
메서드 계약과 구현 간의 불일치 확인 필요
Line 71의 주석은 aiChatImageId가 null이 아니면 해당 이미지가 반드시 존재해야 한다고 명시하고 있습니다. 하지만 Lines 80-82의 구현은 .orElse(List.of())를 사용하여 이미지가 없을 경우 조용히 빈 리스트를 반환합니다.
이는 다음과 같은 문제를 야기할 수 있습니다:
AiChatMessageService에서 이미지 존재 여부를 검증(Line 65)한 후, 비동기 메서드가 실행되기 전에 이미지가 삭제되면 조용히 무시됨- 디버깅이 어려워지고 예상치 못한 동작 발생 가능
다음 중 하나를 선택하여 일관성을 확보하세요:
옵션 1 (권장): 이미지가 없으면 예외 발생
// 이미지가 있으면 해당 이미지 조회
List<AiChatImageEntity> aiChatImages = List.of();
if (aiChatImageId != null) {
- aiChatImages = aiChatImageRepository.findById(aiChatImageId)
- .map(List::of)
- .orElse(List.of());
+ AiChatImageEntity aiChatImage = aiChatImageRepository.findById(aiChatImageId)
+ .orElseThrow(() -> new AppException(ErrorCode.IMAGE_NOT_FOUND_EXCEPTION));
+ aiChatImages = List.of(aiChatImage);
}옵션 2: 주석을 구현에 맞게 수정
- final Long aiChatImageId // 없으면 null, null이 아니면 해당 이미지는 반드시 존재해야 함
+ final Long aiChatImageId // 없으면 null, null이 아니거나 이미지가 삭제된 경우 빈 리스트로 처리📝 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.
| final Long messageId, | |
| final Long aiChatImageId // 없으면 null, null이 아니면 해당 이미지는 반드시 존재해야 함 | |
| ) { | |
| final Long messageId, | |
| final Long aiChatImageId // 없으면 null, null이 아니거나 이미지가 삭제된 경우 빈 리스트로 처리 | |
| ) { |
개요
작업사항
이를 비동기 메서드에 엔티티를 직접적으로 넘기지 않고, 다시 조회하게끔하여 해결했다.
Summary by CodeRabbit
릴리스 노트
개선 사항
버그 수정