refactor: 이미지 저장 중 에러 발생 시 실행되는 롤백 코드 제거

#848
woowacourse-teams · Dec 9, 2023 · 8a3865a · 8a3865a
1 parent 17ed8f6
commit 8a3865a
Show file tree

Hide file tree

Showing 5 changed files with 9 additions and 61 deletions.
diff --git a/backend/emm-sale/src/main/java/com/emmsale/image/application/ImageCommandService.java b/backend/emm-sale/src/main/java/com/emmsale/image/application/ImageCommandService.java
@@ -37,14 +37,7 @@ public List<Image> saveImages(final ImageType imageType, final Long contentId,
     validateImageCount(imageType, multipartFiles);
 
     final List<String> imageNames = s3Client.uploadImages(multipartFiles);
-
-    try {
-      return saveImagesToDb(imageType, contentId, imageNames);
-    } catch (Exception exception) {
-      s3Client.deleteImages(imageNames);
-      // TODO: 2023/09/15 이 동작이 실패했을 경우에 대한 처리(추후 고도화)
-      throw new ImageException(ImageExceptionType.FAIL_DB_UPLOAD_IMAGE);
-    }
+    return saveImagesToDb(imageType, contentId, imageNames);
   }
 
   private void validateContentExist(final ImageType imageType, final Long contentId) {
@@ -96,6 +89,5 @@ public void deleteImages(final ImageType imageType, final Long contentId) {
         .collect(Collectors.toList());
     imageRepository.deleteAllByIdInBatch(imageIds);
     s3Client.deleteImages(imageNames);
-    // TODO: 2023/09/15 S3의 이미지가 '일부만' 삭제되는 경우에 대한 처리(추후 고도화)
   }
 }
diff --git a/backend/emm-sale/src/main/java/com/emmsale/image/application/S3Client.java b/backend/emm-sale/src/main/java/com/emmsale/image/application/S3Client.java
@@ -12,7 +12,6 @@
 import java.util.List;
 import java.util.Objects;
 import java.util.UUID;
-import java.util.concurrent.atomic.AtomicBoolean;
 import java.util.stream.Collectors;
 import org.springframework.beans.factory.annotation.Value;
 import org.springframework.stereotype.Component;
@@ -41,21 +40,14 @@ public S3Client(
   }
 
   public List<String> uploadImages(final List<MultipartFile> multipartFiles) {
-    final AtomicBoolean catchException = new AtomicBoolean(false);
-
-    final List<String> uploadedImageNames = multipartFiles.stream()
+    return multipartFiles.stream()
         .parallel()
-        .map(file -> uploadImage(file, catchException))
+        .map(this::uploadImage)
         .filter(Objects::nonNull)
         .collect(Collectors.toList());
-    if (catchException.get()) {
-      deleteImages(uploadedImageNames);
-      throw new ImageException(ImageExceptionType.FAIL_S3_UPLOAD_IMAGE);
-    }
-    return uploadedImageNames;
   }
 
-  public String uploadImage(final MultipartFile file, final AtomicBoolean catchException) {
+  public String uploadImage(final MultipartFile file) {
     final String fileExtension = extractFileExtension(file);
     final String newFileName = UUID.randomUUID().toString().concat(fileExtension);
     final ObjectMetadata objectMetadata = configureObjectMetadata(file);
@@ -64,8 +56,7 @@ public String uploadImage(final MultipartFile file, final AtomicBoolean catchExc
       amazonS3.putObject(new PutObjectRequest(bucket, newFileName, inputStream, objectMetadata));
       return newFileName;
     } catch (final IOException | SdkClientException exception) {
-      catchException.set(true);
-      return null;
+      throw new ImageException(ImageExceptionType.FAIL_S3_UPLOAD_IMAGE);
     }
   }
 

diff --git a/backend/emm-sale/src/main/java/com/emmsale/member/application/MemberCommandService.java b/backend/emm-sale/src/main/java/com/emmsale/member/application/MemberCommandService.java
@@ -8,7 +8,6 @@
 import com.emmsale.member.exception.MemberException;
 import com.emmsale.member.exception.MemberExceptionType;
 import java.util.List;
-import java.util.concurrent.atomic.AtomicBoolean;
 import javax.transaction.Transactional;
 import lombok.RequiredArgsConstructor;
 import org.springframework.stereotype.Service;
@@ -52,7 +51,7 @@ public MemberImageResponse updateMemberProfile(
       s3Client.deleteImages(List.of(imageName));
     }
 
-    final String imageName = s3Client.uploadImage(image, new AtomicBoolean(false));
+    final String imageName = s3Client.uploadImage(image);
     final String imageUrl = s3Client.convertImageUrl(imageName);
     member.updateProfile(imageUrl);
 

diff --git a/backend/emm-sale/src/test/java/com/emmsale/image/application/ImageCommandServiceTest.java b/backend/emm-sale/src/test/java/com/emmsale/image/application/ImageCommandServiceTest.java
@@ -164,38 +164,6 @@ void saveImages_fail_over_max_image_count() {
       assertThatThrownBy(actual).isInstanceOf(ImageException.class)
           .hasMessage(ImageExceptionType.OVER_MAX_IMAGE_COUNT.errorMessage());
     }
-
-    @Test
-    @DisplayName("이미지를 DB에 저장하는 작업이 실패하면 S3에 저장된 이미지를 삭제하고 예외를 던진다.")
-    void saveImages_fail_and_rollback() {
-      //givend
-      final Event event = eventRepository.save(인프콘_2023());
-      final List<String> imageNames = List.of("테스트테스트.png", "테스트테스트2.png");
-      final List<MultipartFile> files = List.of(
-          new MockMultipartFile("test", "test.png", "", new byte[]{}),
-          new MockMultipartFile("test", "test.png", "", new byte[]{}));
-
-      BDDMockito.given(s3Client.uploadImages(any()))
-          .willReturn(imageNames);
-      BDDMockito.willDoNothing().given(s3Client).deleteImages(any());
-      BDDMockito.given(mockImageRepository.save(any(Image.class)))
-          .willThrow(new IllegalArgumentException());
-
-      //when
-      final ThrowingCallable actual = () -> imageCommandServiceWithMockImageRepository.saveImages(
-          ImageType.EVENT, event.getId(), files);
-
-      //then
-      assertThatThrownBy(actual)
-          .isInstanceOf(ImageException.class)
-          .hasMessage(ImageExceptionType.FAIL_DB_UPLOAD_IMAGE.errorMessage());
-      assertAll(
-          () -> verify(s3Client, times(1))
-              .uploadImages(any()),
-          () -> verify(s3Client, times(1))
-              .deleteImages(any())
-      );
-    }
   }
 
   @Nested

diff --git a/backend/emm-sale/src/test/java/com/emmsale/member/application/MemberCommandServiceTest.java b/backend/emm-sale/src/test/java/com/emmsale/member/application/MemberCommandServiceTest.java
@@ -5,10 +5,8 @@
 import static org.assertj.core.api.Assertions.assertThat;
 import static org.assertj.core.api.Assertions.assertThatThrownBy;
 import static org.junit.jupiter.api.Assertions.assertAll;
-import static org.mockito.ArgumentMatchers.any;
 import static org.mockito.ArgumentMatchers.anyList;
 import static org.mockito.ArgumentMatchers.anyString;
-import static org.mockito.ArgumentMatchers.eq;
 import static org.mockito.Mockito.times;
 import static org.mockito.Mockito.verify;
 import static org.mockito.Mockito.when;
@@ -148,7 +146,7 @@ class UpdateProfile {
     @BeforeEach
     void setUp() {
       ReflectionTestUtils.setField(s3Client, "bucket", bucket);
-      when(s3Client.uploadImage(eq(image), any())).thenReturn(imageName);
+      when(s3Client.uploadImage(image)).thenReturn(imageName);
       when(s3Client.convertImageUrl(anyString())).thenCallRealMethod();
       when(s3Client.convertImageName(anyString())).thenCallRealMethod();
     }
@@ -168,7 +166,7 @@ void pastGithubUrl() {
           () -> assertThat(member.getImageUrl())
               .isEqualTo(s3Client.convertImageUrl(imageName)),
           () -> verify(s3Client, times(1))
-              .uploadImage(eq(image), any())
+              .uploadImage(image)
       );
     }
 
@@ -188,7 +186,7 @@ void pastCustomImage() {
           () -> assertThat(member.getImageUrl())
               .isEqualTo(s3Client.convertImageUrl(imageName)),
           () -> verify(s3Client, times(1))
-              .uploadImage(eq(image), any()),
+              .uploadImage(image),
           () -> verify(s3Client, times(1))
               .deleteImages(anyList())
       );