이미지 S3 업로드 기능 구현 #707
이미지 S3 업로드 기능 구현 #707
Conversation
|
|
||
| @Component | ||
| @ConditionalOnProperty(name = "aws.s3.enabled", havingValue = "false") |
There was a problem hiding this comment.
질문
해당 로직을 @Profile이 아닌 @ComditionalOnProperty를 통해 사용해주신 이유가 있으실까요?
There was a problem hiding this comment.
테스트와 로컬 환경뿐만 아니라 프로덕션 환경에서도 s3를 사용할지 말지를 제어할 수 있도록 하기 위해 @ComditionalOnProperty를 사용했습니다.
There was a problem hiding this comment.
칭찬
기존에 했던 방법보다 해당 방법이 더 좋네요!
기존에는 로컬에서만 가능하고 만약 s3에 문제가 생긴다면 다시 수정을 진행했어야 했는데, 해당 값만 수정하면 되니 더 편하네요.
| import java.util.UUID; | ||
|
|
||
| @Component | ||
| @Profile("!local && !test") |
There was a problem hiding this comment.
선택
이 @Profile("!local && !test")이 계속 반복되는 것 같은데 좀 의미있는 커스텀 어노테이션으로 변경할만할 것 같습니다
뭐 대충 @ProductProfile이나 뭐 그런 느낌으로?
물론 선택입니다
There was a problem hiding this comment.
좋습니다! 이 생각은 못했네요
| } catch (final SdkException ex) { | ||
| throw new StoreImageFailureException("AWS 이미지 저장에 실패했습니다.", ex); | ||
| } |
| @Mock | ||
| S3Client s3Client; |
There was a problem hiding this comment.
질문
테스트 영역 AwsConfiguration에서 이미 S3Client를 빈으로 등록할 때 mock()을 통해 Mock bean을 등록해주었는데, 여기서 @Mock 어노테이션을 사용한 이유가 있을까요?
There was a problem hiding this comment.
AWS_이미지_저장에_실패한_경우_예외가_발생한다, 유효한_이미지_파일인_경우_이미지_파일을_저장한다 테스트에서 s3client를 모킹했어야했기 때문입니다. 그런데 테스트영역 AwsConfiguration은 굳이 필요가 없었네요. 테스트용 AwsConfiguration을 삭제하도록 하겠습니다.
| @@ -7,7 +7,10 @@ | |||
|
|
|||
| public interface StoreImageProcessor { | |||
|
|
|||
| StoreImageDto storeImageFile(MultipartFile imageFile); | |||
| List<String> WHITE_IMAGE_EXTENSION = List.of("jpg", "jpeg", "png"); | |||
There was a problem hiding this comment.
질문
전에 클라이언트에서 jpg, jpeg만 와서 png는 넣을 지 말지 얘기했던 것 같은데, png도 그냥 하는 걸로 결정 됐었나요?
사소하긴 하지만, 기억이 나지 않아 여쭤봅니다.
|
|
||
| @Component | ||
| @ConditionalOnProperty(name = "aws.s3.enabled", havingValue = "false") |
There was a problem hiding this comment.
칭찬
기존에 했던 방법보다 해당 방법이 더 좋네요!
기존에는 로컬에서만 가능하고 만약 s3에 문제가 생긴다면 다시 수정을 진행했어야 했는데, 해당 값만 수정하면 되니 더 편하네요.
| @@ -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) { | |||
| @ProductProfile | ||
| @ConditionalOnProperty(name = "aws.s3.enabled", havingValue = "true") |
There was a problem hiding this comment.
질문
로컬과 테스트에서는 해당 값이 false로 되어 있을 텐데 @ProductProfile을 해주는 것이 더 명확하고 실수를 방지할 수 있기에 둘 다 해준 것일까요?
There was a problem hiding this comment.
넵 어노테이션을 통해 운영환경에서만 사용하는구나를 알 수 있도록 하기 위함도 있고, 역할이 조금 다르다고 생각하기도 했습니다.
@ProductProfile의 경우 운영, 로컬, 테스트 중 어느 환경인지에 대해 명시하기 위한 어노테이션이고, @ConditionalOnProperty는 S3를 사용하냐 마냐를 명시하기 위한 어노테이션이기 때문입니다.
|
|
||
| @Component | ||
| @Profile("local || test") |
There was a problem hiding this comment.
storeImageFile() 메서드 내부에 final이 누락된 것을 발견했습니다
물론 엔초의 작업은 아니겠지만.....
final을 붙여주시면 아주 감사하겠습니다 👍🏻
| import java.util.UUID; | ||
|
|
||
| @Component | ||
| @Profile("!local && !test") |
| import software.amazon.awssdk.regions.Region; | ||
| import software.amazon.awssdk.services.s3.S3Client; | ||
|
|
||
| @ProductProfile |
📄 작업 내용 요약
이미지 S3 업로드 기능 구현
🙋🏻 리뷰 시 주의 깊게 확인해야 하는 코드
@Profile이랑@ConditionalOnProperty붙은 부분 뭔가 덕지덕지 붙은 느낌인데 혹시나 중복된 설정인지(없어도 되는 설정이 있는지) 확인 부탁드립니다. 서브모듈은 제가 업데이트해 놓겠습니다!📎 Issue 번호