[COT-264] Feature: 메일 전송 API 구현 #353

gikhoon · 2025-05-01T07:24:07Z

주의

아직 테스트 코드 작성 미완성
HTML 변경 가능

Motivation

메일 전송 시 Aws Ses로 메일 전송 + 로그 작성

Modification

메일 전송 시 NOT_SENT 또는 Failed 상태의 요청들에게 전송
전솓된 메일은 상태 저장
전송 로그 저장

Result

https://youthing.atlassian.net/browse/COT-264

Test (option)

SQL

gikhoon · 2025-05-01T07:26:07Z

아직 테스트 코드 작성 미완성과 HTML 문구 미완성

refactor할 거 많을 것 같아서 미리 올리고 리뷰 받으면서 수정하겠습니다

Youthhing · 2025-05-02T15:47:45Z

src/main/java/org/cotato/csquiz/api/recruitment/controller/RecruitmentController.java

 import org.springframework.web.bind.annotation.PostMapping;
+import org.springframework.web.bind.annotation.PutMapping;


요건 왜 맨날 순서가 바뀌지 .. 둘이 쓰는 코드 포매터가 다른가

Youthhing · 2025-05-02T15:50:06Z

src/main/java/org/cotato/csquiz/common/config/AwsSesConfig.java

+@Configuration
+public class AwsSesConfig {
+
+    @Value("${cloud.aws.credentials.accessKey}")
+    private String accessKey;
+    @Value("${cloud.aws.credentials.secretKey}")
+    private String secretKey;
+    @Value("${cloud.aws.region.static}")
+    private String region;


환경변수에 있는 값들 이렇게 Bean으로 등록해서 관리할 수 있는데 외부에서 주입 필요할땐 다음에 이렇게 쓰면 좋을 것 같아요

@Getter @Setter @ConfigurationProperties(prefix = "cloud.aws.s3") public class S3Properties { private String accessKey; private String secretKey; private String region; private String bucketName; }

Youthhing · 2025-05-02T15:50:37Z

src/main/java/org/cotato/csquiz/common/config/CotatoConfiguration.java

+        taskExecutor.setCorePoolSize(20);
+        taskExecutor.setMaxPoolSize(100);
+        taskExecutor.setQueueCapacity(10000);


요거 각 필드 개수 선정한 기준이 있나여?

Youthhing · 2025-05-02T15:51:08Z

src/main/java/org/cotato/csquiz/common/email/AwsMailSender.java

+    @Value("${cloud.aws.ses.emailAddress}")
+    private String from;


오 딱 이런 부분에서 위에 언급한 것처럼 yaml 파일 상수로 가져와서 쓰면 좋을 것 같아요

Youthhing · 2025-05-02T15:52:33Z

src/main/java/org/cotato/csquiz/domain/recruitment/email/RecruitmentEmailFactory.java

+package org.cotato.csquiz.domain.recruitment.email;
+
+public class RecruitmentEmailFactory {
+    private static final String LINK_URL = "https://www.cotato.kr";


요것도 환경변수로 넣어서 QA랑 main에서 활용을 구분하면 좋을 것 같습니다 !

Youthhing · 2025-05-02T15:53:11Z

src/main/java/org/cotato/csquiz/domain/recruitment/email/RecruitmentEmailFactory.java

+public class RecruitmentEmailFactory {
+    private static final String LINK_URL = "https://www.cotato.kr";
+
+    public static EmailContent createForGeneration(int generation) {


기수를 위해서 만든다? 메서드명이 너무 모호한 것 같아요

모집 안내 전용인가..? 용도에 맞게 구체화되었으면 좋겠습니다 !

Youthhing · 2025-05-02T15:55:27Z

.../cotato/csquiz/domain/recruitment/repository/RecruitmentNotificationRequesterRepository.java

+    @Modifying
+    @Query("update RecruitmentNotificationRequester r set r.sendStatus = :status where r.id in :ids")
+    void updateSendStatusByIds(@Param("status") SendStatus sendStatus, @Param("ids") List<Long> ids);


update 쿼리인데 트랜잭션없어도 괜찮을까여?

Youthhing · 2025-05-02T15:56:05Z

...tato/csquiz/domain/recruitment/repository/RecruitmentNotificationEmailLogJdbcRepository.java

+
+    private final JdbcTemplate jdbcTemplate;
+
+    public void saveAllWithBatch(List<RecruitmentNotificationEmailLog> logs) {


update 를 하는 부분인데 transaction 안걸어도 될까여?

Youthhing · 2025-05-02T16:08:28Z

src/main/java/org/cotato/csquiz/domain/recruitment/service/RecruitmentNotificationService.java

+        //메일 전송 + 로그 작성 비동기 처리
+        List<CompletableFuture<NotificationResult>> notificationTasks = allNotSentOrFailRequester.stream()
+                .map(requester -> recruitmentNotificationSender.sendNotificationAsync(requester,
+                        emailContent.htmlBody(), emailContent.subject()))


EmailContent클래스를 정보 전달용으로 추가하신 것 같은데 파라미터는 또 변수를 하나씩 넘기는 이유가 있나요?

Youthhing · 2025-05-02T16:15:31Z

...va/org/cotato/csquiz/domain/recruitment/service/component/RecruitmentNotificationSender.java

+    @Async("emailSendThreadPoolExecutor")
+    public CompletableFuture<NotificationResult> sendNotificationAsync(final RecruitmentNotificationRequester requester,
+                                                                       final String htmlBody, final String subject
+    ) {
+        boolean success = true;
+        try {
+            awsMailSender.sendRawMessageBody(
+                    requester.getEmail(),
+                    htmlBody,
+                    subject
+            );
+        } catch (Exception e) {
+            success = false;
+            requester.updateSendStatus(SendStatus.FAIL);
+            log.info("메일 전송 실패 email: {} exception: {}", requester.getEmail(), e.getMessage());
+        }
+
+        return CompletableFuture.completedFuture(NotificationResult.of(requester.getId(), success));
+    }


우선은 실패시 requester 객체를 update하는 부분이 어색합니다.

메일 전송이라는 로직에서 결과를 업데이트하는 로직까지 들어가는 부분이 어색합니다
추가로, 성공을 한 경우엔 updateSendStatus(SUCCESS)로 수정할 필요가 없는걸까요?

어색함을 떠나서 해당 로직을 고려해보면 아래와 같은 문제가 있을 것 같아요.

비동기로 처리하면 기존 스레드가 아닌 스레드 풀에 있는 새로운 스레드를 활용하는데 새로운 스레드는 기존 트랜잭션의 영향을 받지 않고 결국 기존 영속성 컨텍스트에서 관리하지 않고 결국 변경 사항은 flush 되지 않아서 .. 해당 부분에서 update를 해도 반영이 안되는 것 아닌가 싶습니다.

Youthhing · 2025-05-02T16:19:35Z

src/main/java/org/cotato/csquiz/domain/recruitment/service/RecruitmentNotificationService.java

+    private void saveEmailLogs(List<RecruitmentNotificationRequester> requesters,
+                               List<NotificationResult> results,
+                               RecruitmentNotification notification) {
+        Map<Long, RecruitmentNotificationRequester> reqesterMap = requesters.stream()


map은 valueByKey와 같이 변수명 써주시면 좋을 것 같아요 !
Long타입으로 있는 키가 어떤 값을 의미하는지 알기 어려워서요

Youthhing · 2025-05-02T16:20:39Z

src/main/java/org/cotato/csquiz/domain/recruitment/service/RecruitmentNotificationService.java

+                .map(r -> RecruitmentNotificationEmailLog.of(
+                        reqesterMap.get(r.requestId()),
+                        notification,
+                        r.success()
+                ))


요기도 내부 r이 어떤 의미인지 조금 더 구체적으로 들어가면 좋겠습니다 !

람다 내부라 축악형이라도 표현이 되면 좋을 것 같아요

Youthhing · 2025-05-02T16:31:02Z

...va/org/cotato/csquiz/domain/recruitment/service/component/RecruitmentNotificationSender.java

+    @Async("emailSendThreadPoolExecutor")
+    public CompletableFuture<NotificationResult> sendNotificationAsync(final RecruitmentNotificationRequester requester,
+                                                                       final String htmlBody, final String subject
+    ) {
+        boolean success = true;
+        try {
+            awsMailSender.sendRawMessageBody(
+                    requester.getEmail(),
+                    htmlBody,
+                    subject
+            );
+        } catch (Exception e) {
+            success = false;
+            requester.updateSendStatus(SendStatus.FAIL);
+            log.info("메일 전송 실패 email: {} exception: {}", requester.getEmail(), e.getMessage());
+        }
+
+        return CompletableFuture.completedFuture(NotificationResult.of(requester.getId(), success));
+    }


요건 살짝 별개인데 CompletableFutrue의 run-async와 exceptionally를 아래와 같이 활용하면 Try-catch 문을 줄일 수 있을 것 같아요

Suggested change

@Async("emailSendThreadPoolExecutor")

public CompletableFuture<NotificationResult> sendNotificationAsync(final RecruitmentNotificationRequester requester,

final String htmlBody, final String subject

) {

boolean success = true;

try {

awsMailSender.sendRawMessageBody(

requester.getEmail(),

htmlBody,

subject

);

} catch (Exception e) {

success = false;

requester.updateSendStatus(SendStatus.FAIL);

log.info("메일 전송 실패 email: {} exception: {}", requester.getEmail(), e.getMessage());

}

return CompletableFuture.completedFuture(NotificationResult.of(requester.getId(), success));

}

@Async("emailSendThreadPoolExecutor")

public CompletableFuture<NotificationResult> sendNotificationAsync(

final RecruitmentNotificationRequester requester,

final String htmlBody, final String subject

) {

return sendEmailAsync(requester, htmlBody, subject)

.thenApply(v -> NotificationResult.of(requester.getId(), true))

.exceptionally(e -> {

log.error("메일 전송 중 예외 발생 email: {}, exception: {}", requester.getEmail(), e.getMessage());

requester.updateSendStatus(SendStatus.FAIL);

return NotificationResult.of(requester.getId(), false);

});

}

private CompletableFuture<Void> sendEmailAsync(RecruitmentNotificationRequester requester, String htmlBody, String subject) {

return CompletableFuture.runAsync(() -> {

awsMailSender.sendRawMessageBody(

requester.getEmail(),

htmlBody,

subject

);

});

}

Youthhing · 2025-05-02T16:32:08Z

src/main/java/org/cotato/csquiz/common/email/AwsMailSender.java

+@Slf4j
+@Component
+@RequiredArgsConstructor
+public class AwsMailSender {


구체적으로 어느 서비스를 활용한 구현체인지 알아야하니까 Ses Email Sender로 가시죠 !!

Youthhing · 2025-05-02T16:32:43Z

src/main/java/org/cotato/csquiz/common/email/AwsMailSender.java

+    @Value("${cloud.aws.ses.emailAddress}")
+    private String from;
+
+    public void sendRawMessageBody(String recipient, String htmlBody, String subject) {


요기 recipient가 Email 형식인지 검증 안해도 괜찮을까여?

gikhoon added 8 commits April 25, 2025 13:52

build: ses 빌드 파일 작성

f42569d

feat: SesConfig 작성

4dadf05

feat: raw html으로 Ses 메일 전송 로직 구현

404a338

feat: 비동기 메일 전송 로직 구현

7a455b6

feat: 비동기 시 결과를 결과 반환하게 변경

1372640

feat: 메일 전송 서비스 로직 구현

8061844

refactor: Batch size에 맞게 나눠서 저장하게 수정

06bfef5

feat: 메일 제목+html바디 return하는 로직 구현

40e41e8

gikhoon added the do not merge 의존성이나 여타 이유로 당장 머지하면 안되는 PR label May 1, 2025

gikhoon added this to the 모집 이메일 전송 milestone May 1, 2025

gikhoon self-assigned this May 1, 2025

gikhoon requested a review from Youthhing May 1, 2025 07:24

Youthhing reviewed May 2, 2025

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[COT-264] Feature: 메일 전송 API 구현 #353

[COT-264] Feature: 메일 전송 API 구현 #353

gikhoon commented May 1, 2025

gikhoon commented May 1, 2025

Youthhing May 2, 2025

Youthhing May 2, 2025

Youthhing May 2, 2025

Youthhing May 2, 2025

Youthhing May 2, 2025

Youthhing May 2, 2025

Youthhing May 2, 2025

Youthhing May 2, 2025

Youthhing May 2, 2025

Youthhing May 2, 2025

Youthhing May 2, 2025

Youthhing May 2, 2025

Youthhing May 2, 2025

Youthhing May 2, 2025

Youthhing May 2, 2025

		import org.springframework.web.bind.annotation.PostMapping;
		import org.springframework.web.bind.annotation.PutMapping;


		private final JdbcTemplate jdbcTemplate;

		public void saveAllWithBatch(List<RecruitmentNotificationEmailLog> logs) {

[COT-264] Feature: 메일 전송 API 구현 #353

Are you sure you want to change the base?

[COT-264] Feature: 메일 전송 API 구현 #353

Conversation

gikhoon commented May 1, 2025

주의

Motivation

Modification

Result

Test (option)

SQL

gikhoon commented May 1, 2025

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment