Skip to content

refactor: 메뉴 이미지 업로드 로직 리팩토링 #301

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
Jjiggu merged 1 commit into from
Sep 2, 2025
Merged

refactor: 메뉴 이미지 업로드 로직 리팩토링 #301

Show file tree
Hide file tree
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
9 changes: 9 additions & 0 deletions ...pp-admin-api/src/main/java/com/nowait/applicationadmin/menu/service/MenuImageService.java
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,5 +1,7 @@
package com.nowait.applicationadmin.menu.service;

import java.util.Optional;

import org.springframework.stereotype.Service;
import org.springframework.transaction.annotation.Transactional;
import org.springframework.web.multipart.MultipartFile;
Expand Down Expand Up @@ -33,6 +35,13 @@ public MenuImageUploadResponse save(Long menuId, MultipartFile file) {
Menu menu = menuRepository.findById(menuId)
.orElseThrow(MenuNotFoundException::new);

Optional<MenuImage> existingMenuImage = menuImageRepository.findByMenuId(menuId);

existingMenuImage.ifPresent(menuImage -> {
s3Service.delete(menuImage.getFileKey());
menuImageRepository.delete(menuImage);
});

Comment on lines +38 to +44
Copy link

@coderabbitai coderabbitai bot Sep 2, 2025

Choose a reason for hiding this comment

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

🛠️ 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) 방식”이 가장 단순하고 경합에도 강합니다.

S3Service.S3UploadResult uploadResult = s3Service.upload(type, menuId, file).join();

// MenuImage 엔티티 생성 및 저장
Expand Down
2 changes: 2 additions & 0 deletions ...-core-rdb/src/main/java/com/nowait/domaincorerdb/menu/repository/MenuImageRepository.java
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
package com.nowait.domaincorerdb.menu.repository;

import java.util.List;
import java.util.Optional;

import org.springframework.data.jpa.repository.JpaRepository;
import org.springframework.stereotype.Repository;
Expand All @@ -11,4 +12,5 @@
@Repository
public interface MenuImageRepository extends JpaRepository<MenuImage, Long> {
List<MenuImage> findByMenu(Menu menu);
Optional<MenuImage> findByMenuId(Long menuId);
Copy link

@coderabbitai coderabbitai bot Sep 2, 2025

Choose a reason for hiding this comment

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

💡 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 || true

Length 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).

}