Skip to content
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.

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

이미지 S3 업로드 기능 구현 #707

Merged
merged 13 commits into from
Nov 17, 2023
Merged
Show file tree
Hide file tree
Changes from 11 commits
Commits
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
5 changes: 5 additions & 0 deletions backend/ddang/build.gradle
Original file line number Diff line number Diff line change
Expand Up @@ -67,6 +67,11 @@ dependencies {
implementation 'ch.qos.logback.contrib:logback-json-classic:0.1.5'
implementation 'net.logstash.logback:logstash-logback-encoder:6.1'

// aws
implementation platform('software.amazon.awssdk:bom:2.20.56')
implementation 'software.amazon.awssdk:s3'
implementation 'software.amazon.awssdk:cloudfront'

implementation 'org.springframework.boot:spring-boot-starter-validation'
implementation 'org.springframework.boot:spring-boot-starter-web'
testImplementation 'org.springframework.boot:spring-boot-starter-test'
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,25 @@
package com.ddang.ddang.auction.configuration;

import com.ddang.ddang.configuration.ProductProfile;
import com.google.api.client.util.Value;
import org.springframework.context.annotation.Bean;
import org.springframework.context.annotation.Configuration;
import software.amazon.awssdk.auth.credentials.InstanceProfileCredentialsProvider;
import software.amazon.awssdk.regions.Region;
import software.amazon.awssdk.services.s3.S3Client;

@ProductProfile
Copy link
Collaborator

Choose a reason for hiding this comment

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

칭찬

엔초 멋집니다

@Configuration
public class AwsConfiguration {

@Value("${aws.s3.region}")
private String s3Region;

@Bean
public S3Client s3Client() {
return S3Client.builder()
.region(Region.of(s3Region))
.credentialsProvider(InstanceProfileCredentialsProvider.create())
.build();
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,14 @@
package com.ddang.ddang.configuration;

import org.springframework.context.annotation.Profile;

import java.lang.annotation.ElementType;
import java.lang.annotation.Retention;
import java.lang.annotation.RetentionPolicy;
import java.lang.annotation.Target;

@Target(ElementType.TYPE)
@Retention(RetentionPolicy.RUNTIME)
@Profile("!local && !test")
public @interface ProductProfile {
}
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
package com.ddang.ddang.configuration.fcm;

import com.ddang.ddang.configuration.ProductProfile;
import com.ddang.ddang.configuration.fcm.exception.FcmNotFoundException;
import com.google.auth.oauth2.GoogleCredentials;
import com.google.firebase.FirebaseApp;
Expand All @@ -13,10 +14,9 @@
import java.io.IOException;
import java.io.InputStream;
import java.util.List;
import org.springframework.context.annotation.Profile;

@Configuration
@Profile("!test && !local")
@ProductProfile
public class ProdFcmConfiguration {

@Value("${fcm.key.path}")
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,10 @@

public interface StoreImageProcessor {

StoreImageDto storeImageFile(MultipartFile imageFile);
List<String> WHITE_IMAGE_EXTENSION = List.of("jpg", "jpeg", "png");
Copy link
Member

Choose a reason for hiding this comment

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

질문

전에 클라이언트에서 jpg, jpeg만 와서 png는 넣을 지 말지 얘기했던 것 같은데, png도 그냥 하는 걸로 결정 됐었나요?
사소하긴 하지만, 기억이 나지 않아 여쭤봅니다.

String EXTENSION_FILE_CHARACTER = ".";

List<StoreImageDto> storeImageFiles(List<MultipartFile> imageFiles);
StoreImageDto storeImageFile(final MultipartFile imageFile);

List<StoreImageDto> storeImageFiles(final List<MultipartFile> imageFiles);
}
Original file line number Diff line number Diff line change
Expand Up @@ -5,21 +5,21 @@
import com.ddang.ddang.image.infrastructure.local.exception.EmptyImageException;
import com.ddang.ddang.image.infrastructure.local.exception.StoreImageFailureException;
import com.ddang.ddang.image.infrastructure.local.exception.UnsupportedImageFileExtensionException;
import org.springframework.beans.factory.annotation.Value;
import org.springframework.boot.autoconfigure.condition.ConditionalOnProperty;
import org.springframework.stereotype.Component;
import org.springframework.web.multipart.MultipartFile;

import java.io.File;
import java.io.IOException;
import java.util.ArrayList;
import java.util.List;
import java.util.UUID;
import org.springframework.beans.factory.annotation.Value;
import org.springframework.stereotype.Component;
import org.springframework.web.multipart.MultipartFile;

@Component
@ConditionalOnProperty(name = "aws.s3.enabled", havingValue = "false")
Copy link
Collaborator

Choose a reason for hiding this comment

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

질문

해당 로직을 @Profile이 아닌 @ComditionalOnProperty를 통해 사용해주신 이유가 있으실까요?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

테스트와 로컬 환경뿐만 아니라 프로덕션 환경에서도 s3를 사용할지 말지를 제어할 수 있도록 하기 위해 @ComditionalOnProperty를 사용했습니다.

Copy link
Member

Choose a reason for hiding this comment

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

칭찬

기존에 했던 방법보다 해당 방법이 더 좋네요!
기존에는 로컬에서만 가능하고 만약 s3에 문제가 생긴다면 다시 수정을 진행했어야 했는데, 해당 값만 수정하면 되니 더 편하네요.

public class LocalStoreImageProcessor implements StoreImageProcessor {

private static final List<String> WHITE_IMAGE_EXTENSION = List.of("jpg", "jpeg", "png");
private static final String EXTENSION_FILE_CHARACTER = ".";

@Value("${image.store.dir}")
private String imageStoreDir;

Expand All @@ -38,7 +38,8 @@ public List<StoreImageDto> storeImageFiles(final List<MultipartFile> imageFiles)
return storeImageDtos;
}

public StoreImageDto storeImageFile(MultipartFile imageFile) {
@Override
public StoreImageDto storeImageFile(final MultipartFile imageFile) {
try {
final String originalImageFileName = imageFile.getOriginalFilename();
final String storeImageFileName = createStoreImageFileName(originalImageFileName);
Expand All @@ -47,16 +48,16 @@ public StoreImageDto storeImageFile(MultipartFile imageFile) {
imageFile.transferTo(new File(fullPath));

return new StoreImageDto(originalImageFileName, storeImageFileName);
} catch (IOException ex) {
} catch (final IOException ex) {
Copy link
Member

Choose a reason for hiding this comment

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

칭찬

final 붙이기 감사합니다~

throw new StoreImageFailureException("이미지 저장에 실패했습니다.", ex);
}
}

private String findFullPath(String storeImageFileName) {
private String findFullPath(final String storeImageFileName) {
return imageStoreDir + storeImageFileName;
}

private String createStoreImageFileName(String originalFilename) {
private String createStoreImageFileName(final String originalFilename) {
final String extension = extractExtension(originalFilename);

validateImageFileExtension(extension);
Expand All @@ -66,7 +67,7 @@ private String createStoreImageFileName(String originalFilename) {
return uuid + EXTENSION_FILE_CHARACTER + extension;
}

private String extractExtension(String originalFilename) {
private String extractExtension(final String originalFilename) {
int position = originalFilename.lastIndexOf(EXTENSION_FILE_CHARACTER);

return originalFilename.substring(position + 1);
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,103 @@
package com.ddang.ddang.image.infrastructure.s3;

import com.ddang.ddang.configuration.ProductProfile;
import com.ddang.ddang.image.domain.StoreImageProcessor;
import com.ddang.ddang.image.domain.dto.StoreImageDto;
import com.ddang.ddang.image.infrastructure.local.exception.EmptyImageException;
import com.ddang.ddang.image.infrastructure.local.exception.StoreImageFailureException;
import com.ddang.ddang.image.infrastructure.local.exception.UnsupportedImageFileExtensionException;
import lombok.RequiredArgsConstructor;
import org.springframework.beans.factory.annotation.Value;
import org.springframework.boot.autoconfigure.condition.ConditionalOnProperty;
import org.springframework.stereotype.Component;
import org.springframework.web.multipart.MultipartFile;
import software.amazon.awssdk.core.exception.SdkException;
import software.amazon.awssdk.core.sync.RequestBody;
import software.amazon.awssdk.services.s3.S3Client;
import software.amazon.awssdk.services.s3.model.PutObjectRequest;

import java.io.IOException;
import java.util.ArrayList;
import java.util.List;
import java.util.UUID;

@Component
@ProductProfile
@ConditionalOnProperty(name = "aws.s3.enabled", havingValue = "true")
Comment on lines +25 to +26
Copy link
Member

Choose a reason for hiding this comment

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

질문

로컬과 테스트에서는 해당 값이 false로 되어 있을 텐데 @ProductProfile을 해주는 것이 더 명확하고 실수를 방지할 수 있기에 둘 다 해준 것일까요?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

넵 어노테이션을 통해 운영환경에서만 사용하는구나를 알 수 있도록 하기 위함도 있고, 역할이 조금 다르다고 생각하기도 했습니다.
@ProductProfile의 경우 운영, 로컬, 테스트 중 어느 환경인지에 대해 명시하기 위한 어노테이션이고, @ConditionalOnProperty는 S3를 사용하냐 마냐를 명시하기 위한 어노테이션이기 때문입니다.

@RequiredArgsConstructor
public class S3StoreImageProcessor implements StoreImageProcessor {

@Value("${aws.s3.bucket-name}")
private String bucketName;

@Value("${aws.s3.image-path}")
private String path;

private final S3Client s3Client;

@Override
public List<StoreImageDto> storeImageFiles(final List<MultipartFile> imageFiles) {
final List<StoreImageDto> storeImageDtos = new ArrayList<>();

for (final MultipartFile imageFile : imageFiles) {
if (imageFile.isEmpty()) {
throw new EmptyImageException("이미지 파일의 데이터가 비어 있습니다.");
}

storeImageDtos.add(storeImageFile(imageFile));
}

return storeImageDtos;
}

@Override
public StoreImageDto storeImageFile(final MultipartFile imageFile) {
try {
final String originalImageFileName = imageFile.getOriginalFilename();
final String storeImageFileName = createStoreImageFileName(originalImageFileName);
final String fullPath = findFullPath(storeImageFileName);
final PutObjectRequest putObjectRequest = PutObjectRequest.builder()
.key(fullPath)
.bucket(bucketName)
.contentType(imageFile.getContentType())
.build();

s3Client.putObject(
putObjectRequest,
RequestBody.fromInputStream(imageFile.getInputStream(), imageFile.getSize())
);

return new StoreImageDto(originalImageFileName, storeImageFileName);
} catch (final IOException ex) {
throw new StoreImageFailureException("이미지 저장에 실패했습니다.", ex);
} catch (final SdkException ex) {
throw new StoreImageFailureException("AWS 이미지 저장에 실패했습니다.", ex);
}
Comment on lines +73 to +75
Copy link
Collaborator

Choose a reason for hiding this comment

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

칭찬

예외 처리 👍

}

private String findFullPath(final String storeImageFileName) {
return path + storeImageFileName;
}

private String createStoreImageFileName(final String originalFilename) {
final String extension = extractExtension(originalFilename);

validateImageFileExtension(extension);

final String uuid = UUID.randomUUID().toString();

return uuid + EXTENSION_FILE_CHARACTER + extension;
}

private String extractExtension(final String originalFilename) {
int position = originalFilename.lastIndexOf(EXTENSION_FILE_CHARACTER);

return originalFilename.substring(position + 1);
}

private void validateImageFileExtension(final String extension) {
if (!WHITE_IMAGE_EXTENSION.contains(extension)) {
throw new UnsupportedImageFileExtensionException("지원하지 않는 확장자입니다. : " + extension);
}
}
}
7 changes: 7 additions & 0 deletions backend/ddang/src/main/resources/application-local.yml
Original file line number Diff line number Diff line change
Expand Up @@ -57,3 +57,10 @@ fcm:
enabled: false
key:
path: firebase/private-key.json

aws:
s3:
enabled: false
region: region
bucket-name: awsbucketname
image-path: image/path
Original file line number Diff line number Diff line change
@@ -0,0 +1,106 @@
package com.ddang.ddang.image.infrastructure.s3;

import com.ddang.ddang.image.domain.dto.StoreImageDto;
import com.ddang.ddang.image.infrastructure.local.exception.EmptyImageException;
import com.ddang.ddang.image.infrastructure.local.exception.StoreImageFailureException;
import com.ddang.ddang.image.infrastructure.local.exception.UnsupportedImageFileExtensionException;
import com.ddang.ddang.image.infrastructure.s3.fixture.S3StoreImageProcessorFixture;
import org.assertj.core.api.SoftAssertions;
import org.junit.jupiter.api.DisplayNameGeneration;
import org.junit.jupiter.api.DisplayNameGenerator;
import org.junit.jupiter.api.Test;
import org.junit.jupiter.api.extension.ExtendWith;
import org.mockito.InjectMocks;
import org.mockito.Mock;
import org.mockito.junit.jupiter.MockitoExtension;
import software.amazon.awssdk.core.exception.SdkException;
import software.amazon.awssdk.core.sync.RequestBody;
import software.amazon.awssdk.services.s3.S3Client;
import software.amazon.awssdk.services.s3.model.PutObjectRequest;
import software.amazon.awssdk.services.s3.model.PutObjectResponse;

import java.io.ByteArrayInputStream;
import java.io.IOException;
import java.util.List;

import static org.assertj.core.api.Assertions.assertThatThrownBy;
import static org.mockito.ArgumentMatchers.any;
import static org.mockito.BDDMockito.given;

@ExtendWith({MockitoExtension.class})
@DisplayNameGeneration(DisplayNameGenerator.ReplaceUnderscores.class)
@SuppressWarnings("NonAsciiCharacters")
class S3StoreImageProcessorTest extends S3StoreImageProcessorFixture {

@InjectMocks
S3StoreImageProcessor imageProcessor;

@Mock
S3Client s3Client;
Comment on lines +38 to +39
Copy link
Collaborator

Choose a reason for hiding this comment

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

질문

테스트 영역 AwsConfiguration에서 이미 S3Client를 빈으로 등록할 때 mock()을 통해 Mock bean을 등록해주었는데, 여기서 @Mock 어노테이션을 사용한 이유가 있을까요?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

AWS_이미지_저장에_실패한_경우_예외가_발생한다, 유효한_이미지_파일인_경우_이미지_파일을_저장한다 테스트에서 s3client를 모킹했어야했기 때문입니다. 그런데 테스트영역 AwsConfiguration은 굳이 필요가 없었네요. 테스트용 AwsConfiguration을 삭제하도록 하겠습니다.


@Test
void 이미지_파일이_비어_있는_경우_예외가_발생한다() {
// when & then
assertThatThrownBy(() -> imageProcessor.storeImageFiles(List.of(빈_이미지_파일)))
.isInstanceOf(EmptyImageException.class)
.hasMessage("이미지 파일의 데이터가 비어 있습니다.");
}

@Test
void 허용되지_않은_확장자의_이미지_파일인_경우_예외가_발생한다() {
// given
given(이미지_파일.getOriginalFilename()).willReturn(지원하지_않는_확장자를_가진_이미지_파일명);

// when & then
assertThatThrownBy(() -> imageProcessor.storeImageFiles(List.of(이미지_파일)))
.isInstanceOf(UnsupportedImageFileExtensionException.class)
.hasMessageContaining("지원하지 않는 확장자입니다.");
}

@Test
void 이미지_저장에_실패한_경우_예외가_발생한다() throws IOException {
// given
given(이미지_파일.getOriginalFilename()).willReturn(기존_이미지_파일명);
given(이미지_파일.getInputStream()).willThrow(new IOException());

// when & then
assertThatThrownBy(() -> imageProcessor.storeImageFiles(List.of(이미지_파일)))
.isInstanceOf(StoreImageFailureException.class)
.hasMessage("이미지 저장에 실패했습니다.");
}

@Test
void AWS_이미지_저장에_실패한_경우_예외가_발생한다() throws IOException {
// given
final ByteArrayInputStream fakeInputStream = new ByteArrayInputStream("가짜 이미지 데이터".getBytes());
given(이미지_파일.getOriginalFilename()).willReturn(기존_이미지_파일명);
given(이미지_파일.getInputStream()).willReturn(fakeInputStream);
given(s3Client.putObject(any(PutObjectRequest.class), any(RequestBody.class)))
.willThrow(SdkException.class);

// when & then
assertThatThrownBy(() -> imageProcessor.storeImageFiles(List.of(이미지_파일)))
.isInstanceOf(StoreImageFailureException.class)
.hasMessage("AWS 이미지 저장에 실패했습니다.");
}

@Test
void 유효한_이미지_파일인_경우_이미지_파일을_저장한다() throws Exception {
// given
final ByteArrayInputStream fakeInputStream = new ByteArrayInputStream("가짜 이미지 데이터".getBytes());
given(이미지_파일.getOriginalFilename()).willReturn(기존_이미지_파일명);
given(이미지_파일.getInputStream()).willReturn(fakeInputStream);
given(s3Client.putObject(any(PutObjectRequest.class), any(RequestBody.class)))
.willReturn(PutObjectResponse.builder().build());

// when
final List<StoreImageDto> actual = imageProcessor.storeImageFiles(List.of(이미지_파일));

// then
SoftAssertions.assertSoftly(softAssertions -> {
softAssertions.assertThat(actual).hasSize(1);
softAssertions.assertThat(actual.get(0).storeName()).isNotBlank();
softAssertions.assertThat(actual.get(0).uploadName()).isEqualTo(기존_이미지_파일명);
});
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,14 @@
package com.ddang.ddang.image.infrastructure.s3.fixture;

import org.springframework.mock.web.MockMultipartFile;
import org.springframework.web.multipart.MultipartFile;

import static org.mockito.Mockito.mock;

public class S3StoreImageProcessorFixture {

protected MockMultipartFile 빈_이미지_파일 = new MockMultipartFile("image.png", new byte[0]);
protected MultipartFile 이미지_파일 = mock(MultipartFile.class);
protected String 기존_이미지_파일명 = "image.png";
protected String 지원하지_않는_확장자를_가진_이미지_파일명 = "image.gif";
}
7 changes: 7 additions & 0 deletions backend/ddang/src/test/resources/application.yml
Original file line number Diff line number Diff line change
Expand Up @@ -49,3 +49,10 @@ fcm:
enabled: false
key:
path: firebase/private-key.json

aws:
s3:
enabled: false
region: region
bucket-name: awsbucketname
image-path: image/path
Loading