refactor: 메뉴 이미지 업로드 로직 리팩토링 #301
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
Walkthrough기존 이미지 업로드 흐름에 기존 이미지 교체 로직을 추가했습니다. 저장 시 menuId로 기존 MenuImage를 조회해 존재하면 S3에서 파일 삭제 후 레코드를 제거하고, 새 이미지를 업로드·저장합니다. 이를 위해 Repository에 Changes
Sequence Diagram(s)sequenceDiagram
autonumber
actor Admin as Admin Client
participant Svc as MenuImageService
participant Repo as MenuImageRepository
participant S3 as S3Service
Admin->>Svc: save(menuId, file)
Svc->>Repo: findByMenuId(menuId)
alt 기존 이미지 존재
Repo-->>Svc: Optional.of(MenuImage)
Svc->>S3: delete(existingImagePath)
S3-->>Svc: deleted
Svc->>Repo: delete(existingMenuImage)
Repo-->>Svc: deleted
else 없음
Repo-->>Svc: Optional.empty
end
Svc->>S3: upload(file)
S3-->>Svc: UploadResult(url, path, ...)
Svc->>Repo: save(new MenuImage)
Repo-->>Svc: saved(MenuImage)
Svc-->>Admin: MenuImageUploadResponse
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related PRs
Suggested reviewers
✨ 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: 2
🧹 Nitpick comments (2)
nowait-domain/domain-core-rdb/src/main/java/com/nowait/domaincorerdb/menu/repository/MenuImageRepository.java (1)
14-15: API 중복 축소 제안
findByMenu(Menu)와findByMenuId/Id가 공존하면 사용처가 분산되고 일관성이 떨어집니다. 가능하다면findByMenu_Id(Long)만 노출하고 나머지는 단계적으로 제거하는 것을 권장합니다.nowait-app-admin-api/src/main/java/com/nowait/applicationadmin/menu/service/MenuImageService.java (1)
45-45: CompletableFuture.join() 무한 대기 가능성 — 타임아웃 처리 권장
upload이CompletableFuture<S3UploadResult>를 반환하므로,.join()대신 타임아웃을 지정하세요.
예:S3UploadResult result = s3Service.upload(type, menuId, file) .orTimeout(30, TimeUnit.SECONDS) .join();또는 체크드 예외 처리를 위해:
S3UploadResult result = s3Service.upload(type, menuId, file) .get(30, TimeUnit.SECONDS);delete는 동기
void반환이므로 별도 처리 불필요합니다.
📜 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 (2)
nowait-app-admin-api/src/main/java/com/nowait/applicationadmin/menu/service/MenuImageService.java(2 hunks)nowait-domain/domain-core-rdb/src/main/java/com/nowait/domaincorerdb/menu/repository/MenuImageRepository.java(2 hunks)
🔇 Additional comments (2)
nowait-domain/domain-core-rdb/src/main/java/com/nowait/domaincorerdb/menu/repository/MenuImageRepository.java (1)
4-4: Optional 도입 적절단일 조회 시 부재 표현에 맞습니다. 그대로 진행해도 됩니다.
nowait-app-admin-api/src/main/java/com/nowait/applicationadmin/menu/service/MenuImageService.java (1)
3-4: Optional 도입 적절NPE 없이 존재 여부 분기에 잘 맞습니다.
Comment on lines
+38
to
+44
| Optional<MenuImage> existingMenuImage = menuImageRepository.findByMenuId(menuId); | ||
|
|
||
| existingMenuImage.ifPresent(menuImage -> { | ||
| s3Service.delete(menuImage.getFileKey()); | ||
| menuImageRepository.delete(menuImage); | ||
| }); | ||
|
|
There was a problem hiding this comment.
🛠️ Refactor suggestion
삭제→업로드 순서는 실패 시 이미지 유실 위험. 업로드→DB 갱신→커밋 후 S3 삭제로 전환 권장
- 현재는 기존 S3를 먼저 삭제합니다. 업로드/DB 저장이 실패하면 사용자는 이미지가 완전히 사라집니다.
- 안전한 순서: (1) S3 업로드 성공 → (2) DB에 기존 엔티티 업데이트(또는 삭제 후 생성) → (3) 트랜잭션 커밋 후 기존 S3 삭제(보상).
- 동시에, 리포지토리 메서드는
findByMenu_Id(또는 잠금 적용)로 일관화하세요.
최소 변경 예(해당 블록 수정):
- Optional<MenuImage> existingMenuImage = menuImageRepository.findByMenuId(menuId);
-
- existingMenuImage.ifPresent(menuImage -> {
- s3Service.delete(menuImage.getFileKey());
- menuImageRepository.delete(menuImage);
- });
+ Optional<MenuImage> existingMenuImage = menuImageRepository.findByMenu_Id(menuId);
+ String oldFileKey = existingMenuImage.map(MenuImage::getFileKey).orElse(null);
+ existingMenuImage.ifPresent(menuImageRepository::delete);커밋 후 기존 S3 삭제(메서드 하단, save 이후에 추가):
import org.springframework.transaction.support.TransactionSynchronization;
import org.springframework.transaction.support.TransactionSynchronizationManager;
if (oldFileKey != null) {
TransactionSynchronizationManager.registerSynchronization(new TransactionSynchronization() {
@Override public void afterCommit() { s3Service.delete(oldFileKey); }
});
}참고: 가능하면 “기존 엔티티를 업데이트(setImageUrl,setFileKey) 방식”이 가장 단순하고 경합에도 강합니다.
| @Repository | ||
| public interface MenuImageRepository extends JpaRepository<MenuImage, Long> { | ||
| List<MenuImage> findByMenu(Menu menu); | ||
| Optional<MenuImage> findByMenuId(Long menuId); |
There was a problem hiding this comment.
💡 Verification agent
🧩 Analysis chain
메뉴당 단일 이미지 보장 + 경합 방지(유니크 + 잠금 + 메서드 명시화) 필요
- 동시 업로드 시 둘 다 새 레코드를 만들 수 있습니다. 메뉴당 1개 이미지를 전제로 한다면 DB 레벨에서 menu_id 유니크 제약을 반드시 추가하세요.
- 삭제/생성 사이 경합을 줄이려면 조회에 비관적 잠금(PESSIMISTIC_WRITE)을 걸거나, 기존 엔티티를 update 방식으로 교체하세요.
- 프로퍼티 경로를 명시적으로 표기해 혼동을 줄이기 위해 메서드명을
findByMenu_Id로 권장합니다.
적용 예(리포지토리 메서드):
- Optional<MenuImage> findByMenuId(Long menuId);
+ @Lock(LockModeType.PESSIMISTIC_WRITE)
+ Optional<MenuImage> findByMenu_Id(Long menuId);추가 import:
import javax.persistence.LockModeType;
import org.springframework.data.jpa.repository.Lock;마이그레이션(예시):
ALTER TABLE menu_image ADD CONSTRAINT uk_menu_image_menu UNIQUE (menu_id);다음 스크립트로 엔티티 매핑과 유니크 제약 존재 여부를 확인해 주세요.
🏁 Script executed:
#!/bin/bash
# MenuImage 엔티티 매핑 확인
fd -a "MenuImage.java"
rg -nC3 -S "class MenuImage|@Table|@Index|@ManyToOne|@OneToOne|menu" $(fd -a "MenuImage.java")
# SQL 마이그레이션 내 유니크 제약 검색
rg -n -S "menu_image|unique|constraint|menu_id" -- **/*.sql **/db/** **/migration/** 2>/dev/null || trueLength of output: 1074
메뉴당 단일 이미지 보장 및 경합 방지 조치 적용 필요
MenuImage 엔티티·DB에 menu_id 유니크 제약이 없어 복수 레코드 생성이 가능하며, 동시 조회·수정 시 경합도 방지되지 않습니다.
- DB 마이그레이션으로
menu_images(menu_id)에 유니크 제약 추가 - Repository 메서드를
findByMenu_Id로 변경하고@Lock(PESSIMISTIC_WRITE)적용 - 관련 import 추가
// src/main/java/com/nowait/domaincorerdb/menu/repository/MenuImageRepository.java
- Optional<MenuImage> findByMenuId(Long menuId);
+ @Lock(LockModeType.PESSIMISTIC_WRITE)
+ Optional<MenuImage> findByMenu_Id(Long menuId);// 추가 import
import javax.persistence.LockModeType;
import org.springframework.data.jpa.repository.Lock;-- 예시 마이그레이션: db/migration/Vxxx__add_menu_image_unique.sql
ALTER TABLE menu_images
ADD CONSTRAINT uk_menu_images_menu_id UNIQUE (menu_id);🤖 Prompt for AI Agents
In
nowait-domain/domain-core-rdb/src/main/java/com/nowait/domaincorerdb/menu/repository/MenuImageRepository.java
around line 15, the repository method and entity/DB allow multiple images per
menu and lack race protection; add a DB migration to add a unique constraint on
menu_images(menu_id) (e.g., db/migration/Vxxx__add_menu_image_unique.sql ALTER
TABLE ... ADD CONSTRAINT ... UNIQUE (menu_id)), rename the repository method to
findByMenu_Id and annotate it with @Lock(LockModeType.PESSIMISTIC_WRITE) to
prevent concurrent races, and add the necessary imports
(javax.persistence.LockModeType and
org.springframework.data.jpa.repository.Lock).
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.
작업 요약
Issue Link
#300
문제점 및 어려움
해결 방안
Reference
Summary by CodeRabbit