Skip to content

refactor : 트랜잭션 범위 내 외부 서비스 호출 처리 #102

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
Kimminu7 merged 1 commit into from
Jun 26, 2025
Merged

refactor : 트랜잭션 범위 내 외부 서비스 호출 처리 #102

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
23 changes: 21 additions & 2 deletions src/main/java/org/ezcode/codetest/application/problem/service/ProblemService.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -12,16 +12,20 @@
import org.ezcode.codetest.domain.user.service.UserDomainService;
import org.ezcode.codetest.infrastructure.s3.S3Directory;
import org.ezcode.codetest.infrastructure.s3.S3Uploader;
import org.ezcode.codetest.infrastructure.s3.exception.S3Exception;
import org.ezcode.codetest.infrastructure.s3.exception.code.S3ExceptionCode;
import org.springframework.data.domain.Page;
import org.springframework.data.domain.Pageable;
import org.springframework.stereotype.Service;
import org.springframework.transaction.annotation.Transactional;
import org.springframework.web.multipart.MultipartFile;

import lombok.RequiredArgsConstructor;
import lombok.extern.slf4j.Slf4j;

@Service
@RequiredArgsConstructor
@Slf4j
public class ProblemService {

private final ProblemDomainService problemDomainService;
Expand All @@ -39,8 +43,8 @@ public ProblemDetailResponse createProblem(ProblemCreateRequest requestDto, Mult

// 문제 이미지 있다면?
if (image != null && !image.isEmpty()) {
String imageUrl = s3Uploader.upload(image, S3Directory.PROBLEM.getDir());
savedProblem.addImage(imageUrl);
String imageUrl = uploadImageAfterTransaction(image, savedProblem.getId());
updateProblemWithImage(savedProblem.getId(), imageUrl);
Comment on lines +46 to +47
Copy link

@coderabbitai coderabbitai bot Jun 25, 2025

Choose a reason for hiding this comment

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

⚠️ Potential issue

트랜잭션 범위 분리가 불완전합니다.

현재 구현에서는 여전히 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.

}

return ProblemDetailResponse.from(savedProblem);
Expand Down Expand Up @@ -91,5 +95,20 @@ public void removeProblem(Long problemId) {

problemDomainService.removeProblem(findProblem);
}

@Transactional
public void updateProblemWithImage(Long problemId, String imageUrl) {
Problem problem = problemDomainService.getProblem(problemId);
problem.addImage(imageUrl);
}
Comment on lines +99 to +103
Copy link

@coderabbitai coderabbitai bot Jun 25, 2025

Choose a reason for hiding this comment

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

🛠️ 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.


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);
}
}
Comment on lines +105 to +112
Copy link

@coderabbitai coderabbitai bot Jun 25, 2025

Choose a reason for hiding this comment

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

🛠️ Refactor suggestion

S3 업로드 메서드가 여전히 트랜잭션 내에서 호출됩니다.

메서드명이 uploadImageAfterTransaction이지만 실제로는 트랜잭션 내에서 호출되고 있습니다. 또한 업로드 실패 시 예외가 발생하면 전체 트랜잭션이 롤백됩니다.

진정한 트랜잭션 분리를 위해 다음 중 하나를 고려해보세요:

  1. 비동기 처리: S3 업로드를 별도 스레드에서 처리
  2. 트랜잭션 커밋 후 처리: @TransactionalEventListener를 활용한 이벤트 기반 처리
  3. 별도 서비스 분리: 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.

}