Conversation
chaiminwoo0223
left a comment
There was a problem hiding this comment.
고생하셨습니다. 몇가지 궁금한 내용이 있어서 코드 리뷰 남겼습니다. 확인 부탁드립니다!
src/main/java/com/ject/studytrip/image/infra/s3/error/S3ExceptionTranslator.java
Show resolved
Hide resolved
| @RequiredArgsConstructor | ||
| public enum TikaErrorCode implements ErrorCode { | ||
| TIKA_SERVER_ERROR(HttpStatus.INTERNAL_SERVER_ERROR, "Tika 서버 에러가 발생했습니다."), | ||
| ; |
There was a problem hiding this comment.
TikaErrorCode를 domain 계층이 아닌 infra 계층에 두신 이유가 있을까요?
There was a problem hiding this comment.
TikaErrorCode를 domain 계층이 아닌 infra 계층에 두신 이유가 있을까요?
S3와 Tika는 외부 기술이고 여기서 발생하는 에러나 예외는 비즈니스 로직 흐름에서 도메인 정책을 위반해 발생하는게 아니라 외부 시스템 내부에서 발생하는 기술적인 예외로 판단해 infra 계층에 두었습니다.
기술 자체 에러, 예외를 domain 계층에서 관리하는 방법이 더 자연스러울까요?
There was a problem hiding this comment.
TikaErrorCode를 domain 계층이 아닌 infra 계층에 두신 이유가 있을까요?
S3와Tika는 외부 기술이고 여기서 발생하는 에러나 예외는 비즈니스 로직 흐름에서 도메인 정책을 위반해 발생하는게 아니라 외부 시스템 내부에서 발생하는 기술적인 예외로 판단해 infra 계층에 두었습니다.기술 자체 에러, 예외를 domain 계층에서 관리하는 방법이 더 자연스러울까요?
외부 시스템 내부에서 발생하는 기술적인 예외이기 때문에, 지금처럼 infra 계층에 두는 것이 좋을 것 같아요.
저는 항상 에러 코드를 domain 계층에서만 관리했는데, infra 계층에 외부 시스템 관련 에러 코드를 별도로 정의하는 방식이 인상적이었습니다. 각 계층에서 발생하는 예외를 명확하게 처리할 수 있어서 책임 소재가 더 분명해지는 것 같습니다.
src/test/java/com/ject/studytrip/image/application/service/ImageServiceTest.java
Outdated
Show resolved
Hide resolved
a90a1c9 to
b485b86
Compare
코드 리뷰에 답변 남겼습니다 |
hisonghy
left a comment
There was a problem hiding this comment.
코드 리뷰에 대한 답변 남겼습니다
확인부탁드려요
src/main/java/com/ject/studytrip/image/infra/s3/error/S3ExceptionTranslator.java
Show resolved
Hide resolved
| @RequiredArgsConstructor | ||
| public enum TikaErrorCode implements ErrorCode { | ||
| TIKA_SERVER_ERROR(HttpStatus.INTERNAL_SERVER_ERROR, "Tika 서버 에러가 발생했습니다."), | ||
| ; |
There was a problem hiding this comment.
TikaErrorCode를 domain 계층이 아닌 infra 계층에 두신 이유가 있을까요?
S3와 Tika는 외부 기술이고 여기서 발생하는 에러나 예외는 비즈니스 로직 흐름에서 도메인 정책을 위반해 발생하는게 아니라 외부 시스템 내부에서 발생하는 기술적인 예외로 판단해 infra 계층에 두었습니다.
기술 자체 에러, 예외를 domain 계층에서 관리하는 방법이 더 자연스러울까요?
src/test/java/com/ject/studytrip/image/application/service/ImageServiceTest.java
Outdated
Show resolved
Hide resolved
chaiminwoo0223
left a comment
There was a problem hiding this comment.
고생하셨습니다. 커밋 내용 병합 후 머지 부탁드립니다!
* chore: AWS SDK v2 dependency, application-storage.yml, S3Config 설정 * chore: Tika dependency, TikaConfig 설정 * chore: application.yml CDN 설정 * feat: presigned url 발급 로직 구현 * feat: 업로드된 이미지를 검증하는 confirm 로직 구현 * feat: 이미지 업로드를 취소하는 로직 구현 (다중 이미지 업로드에서 예외 발생 시) * feat: S3, Tika client, provider 클래스 구현 * feat: ImageConstants, ImageErrorCode, ImageKeyFactory, ImagePolicy 도메인 객체 추가 * feat: FilenameUtil 클래스 추가 * test: ImageService 단위 테스트 추가
📌 작업 내용 및 특이사항
✅ AWS S3 기반 이미지 스토리지 설정
application-storage.yml파일을 구성해 S3 이미지 스토리지 설정을 추가했습니다.S3Config,S3Properties클래스를 구성했습니다.[ S3 버킷 정책 ]
Presigned URL을 이용해S3에 직접PUT요청(이미지 업로드)을 수행하며,업로드된 객체 조회(GET)는
CloudFront를 통해서만 접근 가능합니다.GET/DELETE권한을 가지며,검증된 IAM User 또는 IAM Role을 통해서만 가능합니다.
(Role은 EC2 권한에 직접 연결해주었으며, 로컬 환경용 IAM 사용자는 노션에 공유해두었습니다.)
[ 객체 수명 주기 ]
✅ Tika 라이브러리 추가
Apache Tika라이브러리를 추가했습니다.Tika로 판별해 MIME 데이터를 추출하도록 설계했습니다.✅ CloudFront CDN 적용
CloudFront CDN도메인으로만 GET 할 수 있도록 설정했습니다.OAC(Origin Access Control)로CloudFront만 접근 하도록 보안을 강화했습니다.✅ 이미지 기능 구현
presign url 발급과직접 객체 조회,삭제,복사역할을 수행하고, 클라이언트는업로드및CDN 이미지 조회역할을 담당하도록 했습니다.클라이언트는 임시 경로에
PUT으로 이미지를 업로드하고, 서버에서confirm이후 최종 경로에 복사하도록 설계했습니다.image패키지를 만들어 공통된presign,confirm,cancel기능을 구현했습니다.회원 프로필,학습 로그,여행 리포트도메인에서 이미지가 필요한데, 각 도메인 별 이미지 관련 로직과 infra 모듈을 구현하지 않고image도메인 로직을 사용할 수 있도록 구현했습니다.추후 도메인 별
presign url 발급 API,업로드된 이미지 confirm API,업로드 과정에서 이슈 발생 시 취소 API는 구현해야합니다.confime과정에서 S3 자체 에러가 아닌 이미지 도메인 정책에 어긋날 경우 업로드된 임시 파일을 삭제하도록 설계했습니다.✅ ImageService 단위 테스트 추가
presign,confirm,cancel로직 단위 테스트 추가했습니다.🌱 관련 이슈
🔍 참고사항(선택)
jpg,jpeg,png,webp이미지 파일의 확장자 및 MIME만 허용하도록 정책을 설계했습니다.CDN 도메인 주소 + ImageKey형태로 저장되도록 리팩토링이 필요합니다.S3_BUCKET_NAME,AWS_REGION,CDN_DOMAIN정보는 노션에 공유하겠습니다.📚 기타(선택)