refactor : 트랜잭션 범위 내 외부 서비스 호출 처리 #102
Merged
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Kimminu7
requested review from
NCookies,
chat26666,
minjee2758,
pokerbearkr and
thezz9
Walkthrough
Changes
Sequence Diagram(s)sequenceDiagram
participant Client
participant ProblemService
participant S3
participant DB
Client->>ProblemService: createProblem()
ProblemService->>DB: 문제 엔티티 저장
ProblemService->>S3: uploadImageAfterTransaction()
alt 업로드 성공
ProblemService->>ProblemService: updateProblemWithImage()
ProblemService->>DB: 이미지 URL로 문제 엔티티 업데이트
else 업로드 실패
ProblemService->>ProblemService: S3Exception 발생 및 로깅
end
Possibly related PRs
Suggested labels
Suggested reviewers
Poem
✨ Finishing Touches
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. Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
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: 3
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
src/main/java/org/ezcode/codetest/application/problem/service/ProblemService.java(3 hunks)
⏰ Context from checks skipped due to timeout of 90000ms (1)
- GitHub Check: build
🔇 Additional comments (2)
src/main/java/org/ezcode/codetest/application/problem/service/ProblemService.java (2)
15-16: 필요한 예외 처리 import가 추가되었습니다.S3 업로드 실패 시 적절한 예외 처리를 위한 import가 추가되어 좋습니다.
24-24: 로깅 지원이 추가되었습니다.Lombok의
@Slf4j어노테이션을 통한 로깅 지원 추가가 적절합니다.Also applies to: 28-28
Comment on lines
+105
to
+112
| private String uploadImageAfterTransaction(MultipartFile image, Long problemId) { | ||
| try { | ||
| return s3Uploader.upload(image, S3Directory.PROBLEM.getDir()); | ||
| } catch (Exception e) { | ||
| log.error("Problem {} 이미지 업로드 실패", problemId, e); | ||
| throw new S3Exception(S3ExceptionCode.S3_UPLOAD_FAILED); | ||
| } | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion
S3 업로드 메서드가 여전히 트랜잭션 내에서 호출됩니다.
메서드명이 uploadImageAfterTransaction이지만 실제로는 트랜잭션 내에서 호출되고 있습니다. 또한 업로드 실패 시 예외가 발생하면 전체 트랜잭션이 롤백됩니다.
진정한 트랜잭션 분리를 위해 다음 중 하나를 고려해보세요:
- 비동기 처리: S3 업로드를 별도 스레드에서 처리
- 트랜잭션 커밋 후 처리:
@TransactionalEventListener를 활용한 이벤트 기반 처리 - 별도 서비스 분리: S3 업로드 전용 서비스를 만들어
@Transactional(propagation = Propagation.NOT_SUPPORTED)적용
+@Async
+@TransactionalEventListener(phase = TransactionPhase.AFTER_COMMIT)
+public void handleImageUpload(ProblemImageUploadEvent event) {
+ try {
+ String imageUrl = s3Uploader.upload(event.getImage(), S3Directory.PROBLEM.getDir());
+ updateProblemWithImage(event.getProblemId(), imageUrl);
+ } catch (Exception e) {
+ log.error("Problem {} 이미지 업로드 실패", event.getProblemId(), e);
+ // 업로드 실패 시 별도 처리 로직 (예: 재시도 큐에 추가)
+ }
+}Committable suggestion skipped: line range outside the PR's diff.
🤖 Prompt for AI Agents
In
src/main/java/org/ezcode/codetest/application/problem/service/ProblemService.java
around lines 105 to 112, the uploadImageAfterTransaction method is misleadingly
named because it is still called within a transaction, causing S3 upload
failures to trigger a transaction rollback. To fix this, refactor the code to
ensure the S3 upload happens outside the transaction by either making the upload
asynchronous, using @TransactionalEventListener to handle the upload after
transaction commit, or moving the upload logic to a separate service method
annotated with @Transactional(propagation = Propagation.NOT_SUPPORTED) to
suspend the transaction during upload.
Comment on lines
+99
to
+103
| @Transactional | ||
| public void updateProblemWithImage(Long problemId, String imageUrl) { | ||
| Problem problem = problemDomainService.getProblem(problemId); | ||
| problem.addImage(imageUrl); | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion
트랜잭션 전파 설정이 필요합니다.
현재 메서드는 기존 트랜잭션에 참여하게 되어 별도의 트랜잭션으로 동작하지 않습니다. 진정한 트랜잭션 분리를 위해서는 전파 설정이 필요합니다.
다음과 같이 트랜잭션 전파 설정을 추가하는 것을 권장합니다:
-@Transactional
+@Transactional(propagation = Propagation.REQUIRES_NEW)
public void updateProblemWithImage(Long problemId, String imageUrl) {
Problem problem = problemDomainService.getProblem(problemId);
problem.addImage(imageUrl);
}🤖 Prompt for AI Agents
In
src/main/java/org/ezcode/codetest/application/problem/service/ProblemService.java
around lines 99 to 103, the updateProblemWithImage method is annotated with
@Transactional but lacks explicit propagation settings, causing it to join
existing transactions instead of running in a separate one. To fix this, add a
propagation attribute to the @Transactional annotation, such as
Propagation.REQUIRES_NEW, to ensure the method executes in its own transaction,
providing true transaction isolation.
Comment on lines
+46
to
+47
| String imageUrl = uploadImageAfterTransaction(image, savedProblem.getId()); | ||
| updateProblemWithImage(savedProblem.getId(), imageUrl); |
There was a problem hiding this comment.
트랜잭션 범위 분리가 불완전합니다.
현재 구현에서는 여전히 S3 업로드가 데이터베이스 트랜잭션 내에서 호출되고 있어 PR 목표가 완전히 달성되지 않았습니다. uploadImageAfterTransaction과 updateProblemWithImage 모두 @Transactional 메서드 내에서 호출되므로 여전히 같은 트랜잭션에 참여합니다.
다음과 같은 방식으로 트랜잭션을 완전히 분리하는 것을 권장합니다:
@Transactional
public ProblemDetailResponse createProblem(ProblemCreateRequest requestDto, MultipartFile image, AuthUser authUser) {
User user = userDomainService.getUserById(authUser.getId());
Problem problem = ProblemCreateRequest.toEntity(requestDto, user);
Problem savedProblem = problemDomainService.createProblem(problem);
- // 문제 이미지 있다면?
- if (image != null && !image.isEmpty()) {
- String imageUrl = uploadImageAfterTransaction(image, savedProblem.getId());
- updateProblemWithImage(savedProblem.getId(), imageUrl);
- }
return ProblemDetailResponse.from(savedProblem);
}
+@Transactional
+public ProblemDetailResponse createProblemWithImage(ProblemCreateRequest requestDto, MultipartFile image, AuthUser authUser) {
+ ProblemDetailResponse response = createProblem(requestDto, null, authUser);
+
+ if (image != null && !image.isEmpty()) {
+ String imageUrl = uploadImageAfterTransaction(image, response.id());
+ updateProblemWithImage(response.id(), imageUrl);
+ }
+
+ return response;
+}Committable suggestion skipped: line range outside the PR's diff.
🤖 Prompt for AI Agents
In
src/main/java/org/ezcode/codetest/application/problem/service/ProblemService.java
around lines 46 to 47, the S3 image upload and database update are still
executed within the same transaction because both methods are called inside a
@Transactional method. To fully separate the transaction scope, refactor so that
the image upload happens outside any @Transactional method, and the
updateProblemWithImage method is called in a separate @Transactional context
after the upload completes. This may involve restructuring the service calls or
using a separate service bean to invoke the update method transactionally after
the upload.
NCookies
approved these changes
Jun 25, 2025
pokerbearkr
approved these changes
Jun 25, 2025
This was referenced Jun 28, 2025
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
작업 내용
하지 않으면, 문제점 3가지 발생
변경 사항
트러블 슈팅
해결해야 할 문제
참고 사항
코드 리뷰 전 확인 체크리스트
type :)Summary by CodeRabbit
버그 수정
기타