-
Notifications
You must be signed in to change notification settings - Fork 0
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
[YS-330] refactor: 이메일 전송 기능 AWS SES로 마이그레이션 #121
Conversation
Walkthrough이번 변경 사항은 프로젝트에 AWS Simple Email Service(SES)를 통합하기 위한 전반적인 업데이트를 포함합니다. Changes
Suggested labels
Suggested reviewers
Poem
✨ Finishing Touches
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 0
🧹 Nitpick comments (3)
src/main/kotlin/com/dobby/backend/infrastructure/config/SESClientConfig.kt (1)
15-28
: SES Client Bean 생성 로직 검토
SesClient
를 빌더 패턴으로 생성하여 리전과 자격 정보를 명시적으로 설정하는 방식이 명확합니다. 다만, 자격 증명을 코드나 설정 파일에 직접 저장하지 않고 안전한 방법(예: 환경 변수, Parameter Store 등)을 고려해 보시면 보안 측면에서 도움이 될 수 있습니다.src/main/kotlin/com/dobby/backend/infrastructure/config/properties/SESProperties.kt (1)
19-22
: 자격 증명 프로퍼티 주의
Credentials
에 직접 액세스 키와 시크릿 키를 담는 방식은 편리하지만, 코드/설정 저장소 관리 시 노출 위험이 있을 수 있으므로 비밀관리 기능 혹은 환경 변수 이용을 권장드립니다.src/main/kotlin/com/dobby/backend/infrastructure/gateway/email/EmailGatewayImpl.kt (1)
38-38
: SesClient 응답 처리
sesClient.sendEmail(request)
호출 이후 예외처리가try-catch
로 감싸져 있어 안정적입니다. 예외가 발생할 경우 로그를 남기는 것도 고려해 보세요.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
build.gradle.kts
(1 hunks)src/main/kotlin/com/dobby/backend/infrastructure/config/SESClientConfig.kt
(1 hunks)src/main/kotlin/com/dobby/backend/infrastructure/config/properties/SESProperties.kt
(1 hunks)src/main/kotlin/com/dobby/backend/infrastructure/gateway/email/EmailGatewayImpl.kt
(1 hunks)
⏰ Context from checks skipped due to timeout of 90000ms (1)
- GitHub Check: build
🔇 Additional comments (10)
build.gradle.kts (1)
56-56
: AWS SES 의존성 추가 확인
새로운 SES 의존성이 정상적으로 추가되었습니다. 버전 호환성이나 잠재적 취약점 등을 계속 주시해 주시기 바랍니다.src/main/kotlin/com/dobby/backend/infrastructure/config/SESClientConfig.kt (2)
1-10
: 패키지 및 임포트 구조 점검
패키지 및 임포트 구성이 적절하며, SES 연동을 위한 라이브러리가 올바르게 추가되었습니다.
11-14
: Spring 설정 클래스 정의 확인
@Configuration
어노테이션으로 Bean 구성 클래스임을 명확히 드러내며,SESProperties
를 주입받는 구조가 합리적입니다.src/main/kotlin/com/dobby/backend/infrastructure/config/properties/SESProperties.kt (3)
5-10
: SESProperties 클래스 구조
aws.ses
프리픽스를 사용해 프로퍼티를 구성한 방식이 명확하며, 이메일 설정과 지역, 자격 증명을 분리하여 가독성이 좋습니다.
11-13
: 이메일 설정 프로퍼티
sender
주소를 담도록 분리된 점이 직관적입니다.
15-17
: 리전 설정 프로퍼티
Region
데이터 클래스를 통해 리전을 분리한 구조는 설정을 체계적으로 관리하는 데 유용해 보입니다.src/main/kotlin/com/dobby/backend/infrastructure/gateway/email/EmailGatewayImpl.kt (4)
4-4
: AWS SES 관련 임포트 확인
SESProperties
와 SES 관련 SDK 임포트가 올바르게 추가되어 있습니다.Also applies to: 8-10
14-15
: 생성자 의존성 주입 확인
SesClient
와SESProperties
를 주입받아 AWS SES를 통한 이메일 발송 준비를 깔끔하게 마무리하고 있습니다.
21-26
: HTML/Plain 텍스트 분기 처리
isHtml
여부에 따라Body
를 적절히 분기하는 로직이 명확하며, 가독성도 좋습니다.
27-36
: 이메일 요청 빌드 로직 점검
SendEmailRequest
빌더를 통해 제목, 수신자, 본문 등을 명확히 설정하는 구조가 알기 쉽습니다.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 0
🧹 Nitpick comments (1)
src/test/resources/application-test.yml (1)
61-70
: AWS SES 설정 구성 확인새롭게 추가한 AWS SES 설정 블록이 올바르게 포함되어 있습니다.
aws.ses.email.sender
와aws.ses.region.static
및 자격증명(access-key
,secret-key
) 항목들이 테스트 환경에 맞게 설정된 것을 확인했습니다.- 다만, 기존의 AWS 설정은
cloud.aws
아래에 구성되어 있는데, 이번 추가된 설정은 최상위aws
블록 내에 존재하므로, 이후 설정의 일관성 유지를 위해 두 구성 간의 경계가 명확하게 문서화되었는지 확인해 주세요.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
src/test/resources/application-test.yml
(1 hunks)
⏰ Context from checks skipped due to timeout of 90000ms (1)
- GitHub Check: build
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM! 이메일 전송 로직을 우리 서버에서 직접 다루다 보니 관리 부담이 있었는데, SES로 넘기면서 확실히 안정성이 올라간 것 같아요. 마이그레이션 작업하느라 정말 수고 많으셨습니다! 👍
추가로 더 개선할 수 있는 부분이 있어서 코멘트 남겼는데, 확인해주시면 감사해요~
private val sesClient: SesClient, | ||
private val sesProperties: SESProperties | ||
) : EmailGateway { | ||
|
||
override suspend fun sendEmail(to: String, subject: String, content: String, isHtml: Boolean) { | ||
withContext(Dispatchers.IO) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
현재 SesClient
는 동기 API를 제공하기 때문에 withContext(Dispatchers.IO)
를 사용한 것이 적절해 보입니다.
그런데 제가 찾아보니 AWS에서 비동기 클라이언트인 SesAsyncClient
를 제공해서, 이를 활용하면 코루틴 없이도 자연스럽게 비동기 처리가 가능할 거라 기대해요! 이렇게 되면 서비스 전체적으로 코루틴을 줄이고 AWS SDK의 비동기 기능을 최대한 활용할 수 있어 깔끔해진다고 생각하는데 어떻게 생각하시나요? 🤔
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
오 제가 AWS에서 SesAsyncClient
를 제공해준다는 점을 몰랐었습니다!
관련하여 반영하기 위해 더 찾아보니, SesAsyncClient
의 whenComplete
을 사용하면, 코루틴의 launch{}
함수와 같은 효과(비동기 스레드 풀로 관리) AWS SDK의 내부 비동기 실행 모델을 최대한 활용할 수 있다고 하네요!
오히려 해당 경우에서 코루틴의 launch{}
를 사용하면, SES SDK 내부 구조와 호환성이 맞지 않아 예외가 잡히지 않는다고 합니다 👀!
(제가 코루틴의 launch{}
와 whenComplete
방식으로 개선해보니 두 경우 모두 비슷하게 약 800-900ms 정도의 응답시간이 소요되더라구요
지수님의 꼼꼼한 리뷰 덕에 고칠 수 있었어요! 감사합니다! ✨
|
||
mailSender.send(message) | ||
} catch (ex: Exception) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
이제 AWS SES라는 서드파티 서비스를 사용하는 거니 예외를 아래와 같은 형식으로 두 번 잡는 건 어떨까요?
} catch (ex: SesException) {
throw IllegalStateException("AWS SES 오류 발생: ${ex.awsErrorDetails()?.errorMessage()}")
} catch (ex: Exception) {
throw IllegalStateException("이메일 발송 실패: ${ex.message}")
}
whenComplete - 기존 `SesClient`를 `SesAsyncClient`로 리팩터링하여 비동기 전송 방식으로 개선 - `whenComplete`를 활용하여 AWS SDK의 `CompletableFeature` 최적화 - `SesException`과 일반 예외를 구분하여 서드 파티 예외처리를 명확하게 개선
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 0
♻️ Duplicate comments (1)
src/main/kotlin/com/dobby/backend/infrastructure/gateway/email/EmailGatewayImpl.kt (1)
40-45
: 🛠️ Refactor suggestion비동기 처리 및 예외 처리 개선 필요
현재 구현에는 몇 가지 개선이 필요합니다:
- SesAsyncClient를 사용하고 있지만, 비동기 작업의 완료를 기다리지 않고 있습니다.
await()
을 사용하여 작업이 완료될 때까지 기다려야 합니다.- 44번 라인의 중괄호 들여쓰기가 잘못되었습니다.
아래와 같이 수정하는 것을 제안합니다:
- sesAsyncClient.sendEmail(request) - .whenComplete { _, ex -> - when(ex){ - is SesException -> throw IllegalStateException("AWS SES 오류 발생: ${ex.awsErrorDetails()?.errorMessage()}") - is Exception -> throw IllegalStateException("이메일 전송 실패: ${ex.message}") } - } + try { + sesAsyncClient.sendEmail(request).await() + } catch (ex: SesException) { + throw IllegalStateException("AWS SES 오류 발생: ${ex.awsErrorDetails()?.errorMessage()}") + } catch (ex: Exception) { + throw IllegalStateException("이메일 전송 실패: ${ex.message}") + }
🧹 Nitpick comments (3)
src/main/kotlin/com/dobby/backend/infrastructure/config/SESClientConfig.kt (1)
10-10
: 사용되지 않는 임포트 제거 필요
SesClient
는 현재 클래스에서 사용되지 않고 있습니다. 사용하지 않는 임포트는 제거하는 것이 좋습니다.-import software.amazon.awssdk.services.ses.SesClient
src/test/resources/application-test.yml (1)
50-58
: AWS 설정 구조의 일관성 검토 필요현재 AWS 관련 설정이 두 곳에 분리되어 있습니다:
cloud.aws
(40-48라인): S3 관련 설정aws.ses
(50-58라인): SES 관련 설정일관성을 위해 SES 설정도
cloud.aws
아래에 위치시키는 것이 좋을 것 같습니다. 또는 모든 AWS 설정을aws
아래로 통합하는 방법도 고려해 볼 수 있습니다.-aws: - ses: - email: - sender: test - region: - static: test - credentials: - access-key: test - secret-key: test +cloud: + aws: + s3: + bucket: test + region: + static: test + credentials: + access-key: test + secret-key: test + ses: + email: + sender: testsrc/main/kotlin/com/dobby/backend/infrastructure/gateway/email/EmailGatewayImpl.kt (1)
13-13
: 사용되지 않는 임포트 제거 필요
SesClient
는 현재 클래스에서 사용되지 않고 있습니다. 사용하지 않는 임포트는 제거하는 것이 좋습니다.-import software.amazon.awssdk.services.ses.SesClient
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
src/main/kotlin/com/dobby/backend/infrastructure/config/SESClientConfig.kt
(1 hunks)src/main/kotlin/com/dobby/backend/infrastructure/config/properties/EmailProperties.kt
(0 hunks)src/main/kotlin/com/dobby/backend/infrastructure/gateway/email/EmailGatewayImpl.kt
(1 hunks)src/test/resources/application-test.yml
(1 hunks)
💤 Files with no reviewable changes (1)
- src/main/kotlin/com/dobby/backend/infrastructure/config/properties/EmailProperties.kt
⏰ Context from checks skipped due to timeout of 90000ms (1)
- GitHub Check: build
🔇 Additional comments (2)
src/main/kotlin/com/dobby/backend/infrastructure/config/SESClientConfig.kt (1)
12-29
: SES 클라이언트 구성이 잘 설계되었습니다!AWS SES 비동기 클라이언트 구성이 적절하게 잘 구현되었습니다. 빌더 패턴을 활용하여 가독성 있게 작성되었고, 필요한 설정(리전, 인증 정보)이 모두 포함되어 있습니다.
src/main/kotlin/com/dobby/backend/infrastructure/gateway/email/EmailGatewayImpl.kt (1)
22-39
: 이메일 생성 로직이 잘 구현되었습니다!AWS SES를 사용하는 이메일 생성 로직이 깔끔하게 구현되었습니다. HTML과 일반 텍스트를 구분하여 처리하는 방식과 SendEmailRequest를 빌더 패턴으로 구성한 부분이 좋습니다.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM! 리뷰 반영해주시니 더 깔끔하고 안정적인 코드가 된 거 같아요. 👍 수고하셨습니다~
💡 작업 내용
SESProperties
설정 클래스 추가 및 AWS 자격 증명 관리 방식 개선EmailGatewayImpl
에서 AWS SDK를 활용하여 SES 클라이언트 생성 및 이메일 전송 로직 구현✅ 셀프 체크리스트
🙋🏻 확인해주세요
[email protected]
로 발송 도메인 주소를 변경하여 마이그레이션 작업을 했습니다!🔗 Jira 티켓
https://yappsocks.atlassian.net/browse/YS-330
Summary by CodeRabbit
New Features
Bug Fixes
Refactor