Skip to content

Conversation

@lvalentine6
Copy link
Member

@lvalentine6 lvalentine6 commented Dec 23, 2025

✨ 개요

  • 응원 등록 이미지 트랜잭션 분리 및 보상 트랜잭션을 Prod 적용합니다.

🧾 관련 이슈

#213 #216

🔍 참고 사항 (선택)

Summary by CodeRabbit

릴리스 노트

  • 새로운 기능

    • 응원 정보를 찾을 수 없을 때의 오류 응답이 추가되었습니다.
  • 개선 사항

    • 파일 작업 중 오류 발생 시 자동 롤백 기능이 추가되었습니다.
    • 응원 등록 시 이미지 처리 프로세스가 개선되었습니다.

✏️ Tip: You can customize this high-level summary in your review settings.

@coderabbitai
Copy link

coderabbitai bot commented Dec 23, 2025

Walkthrough

응원 등록 프로세스를 팩사드 패턴으로 리팩토링하여 서비스와 파일 처리 계층을 분리합니다. 파일 클라이언트의 이미지 이동 흐름을 동기화하고 에러 처리 및 롤백 메커니즘을 강화하며, 응원 생성과 이미지 저장을 독립적인 단계로 분리합니다.

Changes

Cohort / File(s) 요약
파일 스토리지 개선
src/main/java/eatda/client/file/FileClient.java
비동기 루프를 동기 처리로 변경, 성공한 키 누적, S3 에러 처리 및 롤백 추가, 새 deleteFiles(List<String>) 메서드 추가, 상수 PATH_DELIMITER 도입, 로깅 기능 추가
응원 서비스 계층 리팩토링
src/main/java/eatda/service/cheer/CheerService.java
registerCheer(..., ImageDomain)createCheer(...) 변경 (반환 타입 CheerCreationResult), 이미지 처리 로직 제거, 새 메서드 deleteCheer(Long), getCheerResponse(Long), saveCheerImages(...) 추가, 정렬 상수 도입
응원 팩사드 신규
src/main/java/eatda/facade/CheerRegisterFacade.java
새 팩사드 클래스로 응원 생성과 이미지 처리 오케스트레이션, 실패 시 롤백 및 정리 로직 포함
응원 생성 결과 타입
src/main/java/eatda/facade/CheerCreationResult.java
새 레코드: 응원과 스토어 캡슐화
응원 컨트롤러 및 응답
src/main/java/eatda/controller/cheer/CheerController.java, src/main/java/eatda/controller/cheer/CheerResponse.java
CheerController에 CheerRegisterFacade 의존성 추가, CheerResponse 생성자에서 Store 파라미터 제거 (Cheer에서 파생)
예외 처리 확장
src/main/java/eatda/exception/BusinessErrorCode.java
새 에러 코드 CHEER_NOT_FOUND 추가
테스트 기초 클래스
src/test/java/eatda/document/BaseDocumentTest.java, src/test/java/eatda/facade/BaseFacadeTest.java
테스트 의존성 추가 (CheerRegisterFacade, 클라이언트 모의 객체)
테스트 스위트
src/test/java/eatda/document/cheer/CheerDocumentTest.java, src/test/java/eatda/facade/cheer/CheerRegisterFacadeTest.java, src/test/java/eatda/service/cheer/CheerServiceTest.java
모의 대상 및 테스트 흐름 업데이트, 새 팩사드 테스트 추가, 서비스 테스트 API 변경에 맞춰 리팩토링

Sequence Diagram(s)

sequenceDiagram
    participant Client
    participant CheerRegisterFacade
    participant CheerService
    participant FileClient
    participant Database as DB / Repository
    participant S3

    Client->>CheerRegisterFacade: registerCheer(request, storeResult, memberId, imageDomain)
    
    CheerRegisterFacade->>CheerService: createCheer(request, storeResult, memberId)
    CheerService->>Database: save(Cheer)
    Database-->>CheerService: saved Cheer
    CheerService-->>CheerRegisterFacade: CheerCreationResult(cheer, store)
    
    alt Images Provided
        CheerRegisterFacade->>CheerRegisterFacade: sortImages(uploadedImages)
        CheerRegisterFacade->>FileClient: moveTempFilesToPermanent(imageDomain, cheerId, sortedImages)
        
        alt Move Success
            FileClient->>S3: copy temp → permanent
            S3-->>FileClient: success
            FileClient-->>CheerRegisterFacade: permanentKeys
            
            CheerRegisterFacade->>CheerService: saveCheerImages(cheerId, sortedImages, permanentKeys)
            CheerService->>Database: save(CheerImages)
            Database-->>CheerService: success
            CheerService-->>CheerRegisterFacade: success
            
            CheerRegisterFacade->>CheerService: getCheerResponse(cheerId)
            CheerService-->>CheerRegisterFacade: CheerResponse
            CheerRegisterFacade-->>Client: CheerResponse
        else Move/Save Failure
            FileClient->>Client: throw SdkException
            CheerRegisterFacade->>FileClient: deleteFiles(permanentKeys)
            FileClient->>S3: delete
            CheerRegisterFacade->>CheerService: deleteCheer(cheerId)
            CheerService->>Database: delete(Cheer)
            CheerRegisterFacade->>Client: throw exception
        end
    else No Images
        CheerRegisterFacade->>CheerService: getCheerResponse(cheerId)
        CheerService-->>CheerRegisterFacade: CheerResponse
        CheerRegisterFacade-->>Client: CheerResponse
    end
Loading

Estimated code review effort

🎯 4 (Complex) | ⏱️ ~60 minutes

Possibly related PRs

Suggested labels

refactor

Suggested reviewers

  • leegwichan

Poem

🐰 팩사드의 마법으로 응원이 날개를 펼쳤네,
서비스와 파일이 손을 잡고 춤을 춘다네!
롤백의 그물로 안전함을 담아내고,
이미지는 순서대로 제자리를 찾아가네. ✨

Pre-merge checks and finishing touches

❌ Failed checks (1 warning)
Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. You can run @coderabbitai generate docstrings to improve docstring coverage.
✅ Passed checks (2 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed PR 제목은 이미지 트랜잭션 분리와 보상 트랜잭션 적용이라는 핵심 변경사항을 정확하게 반영합니다.
✨ Finishing touches
  • 📝 Generate docstrings
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment
  • Commit unit tests in branch develop

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.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

@sonarqubecloud
Copy link

Copy link

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 3

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/eatda/service/cheer/CheerService.java (1)

55-56: 동시성 요청 시 상점 조회/저장에서 고유 제약 조건 위반 발생

kakaoId에 unique 제약 조건이 존재하지만, findByKakaoIdorElseGet으로 저장하는 패턴은 동시성을 처리하지 못합니다. 동일한 kakaoId로 동시 요청이 들어올 때, 두 요청 모두 조회 결과가 없어 저장을 시도하면 두 번째 요청이 DataIntegrityViolationException으로 실패합니다.

해결 방안:

  • DataIntegrityViolationException 캐치 후 재조회
  • @Lock(LockModeType.PESSIMISTIC_WRITE) 사용
  • 분산 락(Redis 등) 활용
🧹 Nitpick comments (5)
src/main/java/eatda/client/file/FileClient.java (1)

74-79: deleteFiles에서 개별 삭제 실패 시 나머지 파일 삭제가 중단됨

keys.forEach(this::deleteObject)는 첫 번째 삭제 실패 시 예외가 전파되어 나머지 파일 삭제가 실행되지 않습니다. 롤백 시나리오에서는 가능한 한 많은 파일을 삭제하는 것이 바람직합니다.

🔎 Best-effort 삭제 구현
     public void deleteFiles(List<String> keys) {
         if (keys.isEmpty()) {
             return;
         }
-        keys.forEach(this::deleteObject);
+        for (String key : keys) {
+            try {
+                deleteObject(key);
+            } catch (SdkException e) {
+                log.warn("파일 삭제 실패. key={}", key, e);
+            }
+        }
     }
src/main/java/eatda/controller/cheer/CheerResponse.java (1)

17-28: cheer.getStore() lazy loading 접근 안전성 검토

Store 필드는 @ManyToOne(fetch = FetchType.LAZY) 로 설정되어 있으며, 현재 CheerResponse(Cheer, String) 생성자는 CheerService.getCheerResponse()에서만 호출되는데 이 메서드가 @Transactional(readOnly = true) 로 보호되어 있으므로 트랜잭션 컨텍스트 내에서 lazy loading이 안전하게 작동합니다. 다만, 향후 이 생성자가 다른 곳에서 트랜잭션 외부에서 호출될 경우 LazyInitializationException이 발생할 수 있는 설계상 취약점이 있습니다. 생성자의 트랜잭션 의존성을 명시적으로 문서화하거나, Store 의 fetch 타입을 EAGER로 변경하는 것을 검토해보세요.

src/test/java/eatda/facade/cheer/CheerRegisterFacadeTest.java (2)

84-132: 테스트명과 실제 동작이 일치하지 않습니다.

이미지_이동_중_실패하면_응원을_삭제한다 테스트는 실제로 응원이 "삭제"되는 것이 아니라, 롤백으로 인해 응원이 "저장되지 않음"을 검증합니다. Facade 코드를 보면 createCheer가 먼저 호출되고, 이미지 이동 실패 시 deleteCheer가 호출되므로 테스트명은 정확합니다.

다만, 테스트에서 응원이 실제로 생성된 후 삭제되었는지 더 명확하게 검증하려면 deleteCheer 호출 여부도 확인하는 것이 좋습니다.

🔎 deleteCheer 호출 검증 추가 제안
             assertThat(cheerRepository.count()).isZero();
+
+            // 롤백 시 deleteCheer가 호출되었는지 검증 (선택적)
+            // verify(cheerService).deleteCheer(anyLong());
         }

257-295: 헬퍼 메서드 중복을 줄일 수 있습니다.

getRegisterRequest()getCheerRegisterRequest()는 이미지 개수만 다르고 거의 동일합니다. 파라미터화된 단일 헬퍼 메서드로 통합할 수 있습니다.

🔎 통합된 헬퍼 메서드 제안
@NonNull
private CheerRegisterRequest createRegisterRequest(int imageCount) {
    List<CheerRegisterRequest.UploadedImageDetail> images = IntStream.rangeClosed(1, imageCount)
            .mapToObj(i -> new CheerRegisterRequest.UploadedImageDetail(
                    "temp/key" + i + ".jpg", (long) i, "image/jpeg", 1000L))
            .toList();

    return new CheerRegisterRequest(
            "kakao-1",
            "농민백암순대",
            "맛있어요",
            images,
            List.of(CheerTagName.GOOD_FOR_DATING)
    );
}
src/main/java/eatda/service/cheer/CheerService.java (1)

71-91: sortedImagespermanentKeys 크기 불일치 시 예외 발생 가능성이 있습니다.

permanentKeys.get(i) 호출 시 두 리스트의 크기가 다르면 IndexOutOfBoundsException이 발생합니다. Facade에서 올바르게 호출하면 문제없지만, 방어적으로 크기 검증을 추가하는 것이 안전합니다.

🔎 크기 검증 추가 제안
     @Transactional
     public void saveCheerImages(Long cheerId,
                                 List<CheerRegisterRequest.UploadedImageDetail> sortedImages,
                                 List<String> permanentKeys) {

+        if (sortedImages.size() != permanentKeys.size()) {
+            throw new IllegalArgumentException(
+                    "sortedImages와 permanentKeys의 크기가 일치해야 합니다: " +
+                    sortedImages.size() + " vs " + permanentKeys.size());
+        }
+
         Cheer cheer = cheerRepository.findById(cheerId)
                 .orElseThrow(() -> new BusinessException(BusinessErrorCode.CHEER_NOT_FOUND));
📜 Review details

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between e66d104 and 3d9dbe2.

📒 Files selected for processing (12)
  • src/main/java/eatda/client/file/FileClient.java
  • src/main/java/eatda/controller/cheer/CheerController.java
  • src/main/java/eatda/controller/cheer/CheerResponse.java
  • src/main/java/eatda/exception/BusinessErrorCode.java
  • src/main/java/eatda/facade/CheerCreationResult.java
  • src/main/java/eatda/facade/CheerRegisterFacade.java
  • src/main/java/eatda/service/cheer/CheerService.java
  • src/test/java/eatda/document/BaseDocumentTest.java
  • src/test/java/eatda/document/cheer/CheerDocumentTest.java
  • src/test/java/eatda/facade/BaseFacadeTest.java
  • src/test/java/eatda/facade/cheer/CheerRegisterFacadeTest.java
  • src/test/java/eatda/service/cheer/CheerServiceTest.java
🧰 Additional context used
🧬 Code graph analysis (1)
src/test/java/eatda/facade/BaseFacadeTest.java (2)
src/test/java/eatda/DatabaseCleaner.java (1)
  • DatabaseCleaner (11-47)
src/test/java/eatda/EatdaApplicationTests.java (1)
  • SpringBootTest (6-12)
🪛 GitHub Actions: Notify Discord on `/noti` Comment
src/test/java/eatda/document/BaseDocumentTest.java

[error] 93-93: No such file or directory

src/main/java/eatda/facade/CheerCreationResult.java

[error] 93-93: No such file or directory

src/test/java/eatda/document/cheer/CheerDocumentTest.java

[error] 93-93: No such file or directory

src/main/java/eatda/client/file/FileClient.java

[error] 93-93: No such file or directory

src/main/java/eatda/facade/CheerRegisterFacade.java

[error] 93-93: No such file or directory

src/test/java/eatda/facade/cheer/CheerRegisterFacadeTest.java

[error] 93-93: No such file or directory

src/main/java/eatda/controller/cheer/CheerResponse.java

[error] 93-93: No such file or directory

src/main/java/eatda/service/cheer/CheerService.java

[error] 93-93: No such file or directory

src/main/java/eatda/exception/BusinessErrorCode.java

[error] 93-93: No such file or directory

src/test/java/eatda/facade/BaseFacadeTest.java

[error] 93-93: No such file or directory

src/test/java/eatda/service/cheer/CheerServiceTest.java

[error] 93-93: No such file or directory

src/main/java/eatda/controller/cheer/CheerController.java

[error] 93-93: No such file or directory

⏰ 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: test
🔇 Additional comments (17)
src/main/java/eatda/exception/BusinessErrorCode.java (1)

31-31: LGTM!

새로운 에러 코드가 기존 패턴(CHE00X)과 일관되게 추가되었고, HttpStatus.NOT_FOUND가 적절하게 적용되었습니다.

src/test/java/eatda/document/cheer/CheerDocumentTest.java (2)

102-102: LGTM!

CheerRegisterFacade 도입에 따라 mock 대상이 올바르게 변경되었습니다.


139-140: LGTM!

실패 케이스도 facade를 통해 테스트되도록 일관성 있게 수정되었습니다.

src/main/java/eatda/facade/CheerCreationResult.java (1)

1-7: LGTM!

응원 생성 결과를 캡슐화하기 위한 간결한 record 정의입니다. Cheer와 Store를 함께 반환해야 하는 요구사항에 적합합니다.

src/test/java/eatda/document/BaseDocumentTest.java (1)

70-72: LGTM!

CheerRegisterFacade에 대한 mock bean이 기존 패턴과 일관되게 추가되었습니다.

src/main/java/eatda/controller/cheer/CheerController.java (2)

33-33: LGTM!

Facade 패턴을 도입하여 응원 등록 workflow를 분리한 것이 좋습니다. Controller는 HTTP 처리에 집중하고, Facade가 비즈니스 orchestration을 담당합니다.


40-40: LGTM!

응원 등록 로직이 CheerRegisterFacade로 위임되었습니다. ImageDomain.CHEER를 명시적으로 전달하여 도메인 구분이 명확합니다.

src/test/java/eatda/facade/BaseFacadeTest.java (1)

15-36: LGTM!

Facade 테스트를 위한 기반 클래스가 잘 구성되었습니다:

  • DatabaseCleaner로 테스트 격리 보장
  • 외부 클라이언트(OAuth, Map, File) mock 처리
  • 테스트 fixture 및 repository autowiring
src/main/java/eatda/client/file/FileClient.java (2)

53-66: 이동 중간에 실패 시 원본 임시 파일 일부가 이미 삭제된 상태가 됨

현재 로직은 copyObjectdeleteObject 순서로 각 파일을 처리합니다. N번째 파일에서 copyObject 성공 후 deleteObject도 성공하고, N+1번째 파일의 copyObject에서 실패하면:

  • 1~N번 임시 파일은 이미 삭제됨
  • 롤백으로 1~N번 permanent 파일만 삭제됨

원본 임시 파일 복구가 불가능한 상태가 됩니다. 이 동작이 의도된 것인지 확인이 필요합니다.


23-23: LGTM!

PATH_DELIMITER 상수 도입으로 매직 스트링 사용을 제거하고 일관성을 확보했습니다.

src/test/java/eatda/facade/cheer/CheerRegisterFacadeTest.java (1)

175-216: DB 저장 실패 시 롤백 검증이 잘 구현되었습니다.

이미지 이동 성공 후 DB 저장 실패 시 파일 삭제(deleteFiles) 호출을 검증하는 테스트입니다. 보상 트랜잭션 패턴이 잘 테스트되고 있습니다.

src/main/java/eatda/facade/CheerRegisterFacade.java (2)

64-75: moveImages의 빈 리스트 검사는 방어적이지만 중복됩니다.

Line 33에서 이미 request.images().isEmpty() 검사 후 early return이 있으므로, sortedImages가 비어있을 수 없습니다. 하지만 방어적 프로그래밍 관점에서 유지해도 무방합니다.


25-55: 트랜잭션 분리 설계가 적절합니다.

Facade 메서드에 @Transactional이 없고, 각 서비스 호출(createCheer, saveCheerImages, deleteCheer)이 개별 트랜잭션으로 실행됩니다. 이는 S3 작업과 DB 작업을 분리하여 부분 실패 시 보상 트랜잭션을 수행하려는 PR 목적에 부합합니다.

src/test/java/eatda/service/cheer/CheerServiceTest.java (2)

101-118: 새로운 CheerCreationResult 반환 타입에 맞게 테스트가 잘 업데이트되었습니다.

createCheer 호출 후 CheerCreationResult에서 cheer()store()를 추출하여 검증하는 방식이 새로운 API 설계에 적합합니다.


296-309: 테스트 헬퍼 메서드 추가로 코드 중복이 줄었습니다.

storeSearchResult(String kakaoId) 헬퍼 메서드가 테스트 전반에서 일관된 StoreSearchResult 생성을 담당하여 가독성과 유지보수성이 향상되었습니다.

src/main/java/eatda/service/cheer/CheerService.java (2)

129-132: deleteCheer가 존재하지 않는 응원 삭제 시 예외를 던지지 않습니다.

deleteById는 ID가 존재하지 않아도 예외를 던지지 않습니다. Facade의 롤백 시나리오에서는 이것이 문제가 되지 않지만, 다른 컨텍스트에서 호출될 경우 의도치 않은 동작이 발생할 수 있습니다. 현재 사용 패턴에서는 문제없으나 명시적 검증을 고려해볼 수 있습니다.


134-140: getCheerResponse 메서드가 잘 구현되었습니다.

Facade에서 응원 등록 완료 후 응답을 생성하는 데 사용됩니다. 예외 처리가 명확하고 역할이 분명합니다.

@lvalentine6 lvalentine6 merged commit 32c6387 into main Dec 23, 2025
14 of 15 checks passed
@github-actions
Copy link

🎉 This PR is included in version 1.9.4 🎉

The release is available on GitHub release

Your semantic-release bot 📦🚀

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants