[BE] 입장 10분 전 푸시 알림 전송 기능 구현 (#483) #484

xxeol2 · 2023-10-02T07:17:53Z

📌 관련 이슈

closed: [BE] 입장 10분 전 입장 알림 푸시알림을 보낸다. #483

✨ PR 세부 내용

즐거운 한가위 보내고 계신가요? 🌕
추석 전에 말씀드린대로 입장 10분 전 알림 전송 기능을 한번 구현해봤어요!!

기존에 얘기나눴던 1분에 한 번씩 cron job 대신, 티켓 생성시 동적으로 Schedule을 등록하는 방법을 택했습니다..!
해당 방법 선택 과정은 PR에 작성하기 너무 길어져서.. 아래 문서를 참고해주세요!!
https://wooteco-ash.notion.site/10-7a56e61a70d94bb48f5b3ff801d66cea?pvs=4

일단 제 최선을 다해 구현을 해봤는데, 추석 끝나고 캠퍼스에서 다함께 논의할 부분들이 많은 것 같아요!

#472 PR이랑 충돌도 많을거고, 안드분들이랑도 얘기할 부분이 많아서 우선 draft 처리해두겠습니다~!

그럼 남은 추석 잘 보내시고 수요일에 캠퍼스에서 뵙시당! 🙇🏻‍♀️

github-actions · 2023-10-02T07:20:10Z

Unit Test Results

  75 files   75 suites 11s ⏱️
203 tests 203 ✔️ 0 💤 0 ❌
206 runs 206 ✔️ 0 💤 0 ❌

Results for commit e45fde3.

♻️ This comment has been updated with latest results.

xxeol2 · 2023-10-02T07:23:03Z

backend/src/main/java/com/festago/entry_alert/application/EntryAlertService.java

+    private void sendMessages(List<String> tokens) {
+        for (int i = 0; i < tokens.size(); i += BATCH_ALERT_SIZE) {
+            List<String> subTokens = tokens.subList(i, Math.min(i + BATCH_ALERT_SIZE, tokens.size()));
+            fcmClient.sendAllAsync(subTokens, FCMChannel.ENTRY_ALERT, FcmPayload.entryAlert());
+        }
+    }


FCM이 한 번에 최대 500건까지 발송가능하더라구요?!

그래서 500건씩 보내줬는데, 이는 Async로 비동기로 진행했어요!

이게 X-lock 범위랑도 연관되어있어서, FCM 로직 전송시간동안 계속 connection을 점유한다면 connection 고갈문제로도 이어질 수 있어 Async로 진행하는게 옳다고 생각했어요!

이 때, 메세지 전송에 실패해도 entryAlertRepository.delete()가 호출된다는 점이 문젠데..
현재는 재시도 로직도 없고, 입장시간 지나서 재시도가 되어도 문제라고 생각했어요
그래서 fcm 메세지 실제 전송 성공 여부와 상관없이, EntryAlert는 제거해주는 방식을 택했습니다!!

해당 부분은 FCM에서의 제약 사항이니 sendMessages() 메서드의 for문이 fcmClient.sendAll()으로 옮겨야 될 것 같네요!

글렌 의견에 동의합니다.

xxeol2 · 2023-10-02T07:28:00Z

backend/src/test/java/com/festago/domain/MemberTicketRepositoryTest.java

+    @Test
+    void 무대와_입장시간으로_멤버아이디리스트_조회() {
+        // given
+        LocalDateTime entryTime = LocalDateTime.now();
+        Stage stage = saveStage();
+        Member member1 = memberRepository.save(MemberFixture.member().build());
+        Member member2 = memberRepository.save(MemberFixture.member().build());
+        MemberTicket memberTicket1 = memberTicketRepository.save(
+            MemberTicketFixture.memberTicket().stage(stage).owner(member1).entryTime(entryTime).build());
+        MemberTicket memberTicket2 = memberTicketRepository.save(
+            MemberTicketFixture.memberTicket().stage(stage).owner(member2).entryTime(entryTime).build());
+
+        // when
+        List<Long> actual = memberTicketRepository.findAllOwnerIdByStageIdAndEntryTime(
+            stage.getId(), entryTime);
+
+        // then
+        assertThat(actual).containsExactlyInAnyOrder(member1.getId(), member2.getId());


로컬에선 잘 실행되는데 CI는 터지네요 🤔
원인 파악해보겠습니다

LocalDateTime.now() 의 타임존이 맞지 않아서 생기는 것 같기도 하네요.

xxeol2 · 2023-10-02T07:45:51Z

backend/src/main/java/com/festago/entry_alert/application/EntryAlertService.java

+        EntryAlert entryAlert = entryAlertRepository.findByIdForUpdate(id)
+            .orElseThrow(() -> new ConflictException(ErrorCode.ALREADY_ALERT));


참고 노션에도 언급되어있듯 동시에 2대의 WAS에서 실행할 때, 하나만 실행되고 나머지 하나는 예외처리가 되도록 했습니다!
그런데 이 때, BadRequest는 아닌 것 같고.. (클라이언트의 요청이 없으니)
InternalServerException은 로깅이 찍히고 그래서
아예 별도의 예외 객체가 필요할 것 같아서 따라서 ConflictException으로 처리했어요!
(사실 서버 내부적으로 동작하는거라 클라이언트에까지 응답이 내려갈 일은 없을 것 같아요)

여기서 delete 하는 방법도 있지만 처리전, 처리후 같은 식의 상태를 변경시키는 방법은 어떨까요?

그렇게한다면 굳이 throw안하고, early return 하는 방법으로도 처리할 수도 있고
제대로 스케쥴링이 처리됐는지, 명시적으로 확인할 수도 있을 것 같아요.
데이터가 쌓인다는 점이 있지만 수백~수천개 정도의 데이터는 큰 문제가 안되고, 나중에 주기적으로 비워줄 수도 있으니까요.

푸시알림 전송 여부를 추적할 수 있다는 점에서 오리가 말씀해주신 방법이 좋아보이네요!
해당 방법으로 수정해보겠습니다!!

seokjin8678

연휴 동안 많은 일을 해주셨군요. 👍👍👍
몇 부분에 대한 최적화가 가능할 것 같아 리뷰 남겼습니다!
고생하셨어요!!

backend/src/main/java/com/festago/entry_alert/application/EntryAlertEventListener.java

seokjin8678 · 2023-10-02T10:27:54Z

backend/src/main/java/com/festago/entry_alert/application/EntryAlertService.java

+        List<String> tokens = memberFCMRepository.findAllByMemberIdIn(memberIds)
+            .stream()
+            .map(MemberFCM::getFcmToken)
+            .toList();


class MemberFCMRepository { ... @Query("SELECT mf.fcmToken FROM MemberFCM mf WHERE mf.memberId in :memberIds") List<String> findAllTokenByMemberIdIn(@Param("memberIds") List<Long> memberIds); ... }

다음과 같이 사용할 수 있을 것 같네요!

반영 완료했습니다 👍

찾아보니 이런 방법도 있네요.
전 글렌 코멘트대로 JPQL로 처리하는게 더 간단하고 좋다고 합니다만, 그래도 알게되서 참고용으로 링크 걸어봅니다.
인덱스 컨디션 등으로 성능 최적화 시킬 부분이 많고 유지 보수가 어려운 특정 프로젝트에서는 관리 포인트를 편하게하기 위해 사용할 수 있을 것 같습니다.

https://stackoverflow.com/questions/22007341/spring-jpa-selecting-specific-columns

저희 프로젝트에서는 만약 적용한다면 이런식으로..(일케 하자는건 아니고 그냥 참고용입니당. 위에서 적은 것처럼 저희 프로젝트에서는 JPQL이 지금은 더 좋다고 생각해요.)

public interface FcmToken { String getFcmToken(); } 혹은 레코드 public record FcmToken(String fcmToken) { }

List<FcmToken> findTokenByMemberIdIn(List<Long> memberIds); .... Hibernate: select m1_0.fcm_token from member_fcm m1_0 where m1_0.member_id in(?,?)

seokjin8678 · 2023-10-02T10:44:33Z

backend/src/main/java/com/festago/entry_alert/application/EntryAlertService.java

+    private void sendMessages(List<String> tokens) {
+        for (int i = 0; i < tokens.size(); i += BATCH_ALERT_SIZE) {
+            List<String> subTokens = tokens.subList(i, Math.min(i + BATCH_ALERT_SIZE, tokens.size()));
+            fcmClient.sendAllAsync(subTokens, FCMChannel.ENTRY_ALERT, FcmPayload.entryAlert());
+        }
+    }


해당 부분은 FCM에서의 제약 사항이니 sendMessages() 메서드의 for문이 fcmClient.sendAll()으로 옮겨야 될 것 같네요!

backend/src/main/java/com/festago/entry_alert/domain/EntryAlert.java

seokjin8678 · 2023-10-02T10:55:45Z

backend/src/main/java/com/festago/fcm/application/FCMNotificationEventListener.java

 public class FCMNotificationEventListener {

-    private static final Logger log = LoggerFactory.getLogger(FCMNotificationEventListener.class);
-
-    private final FirebaseMessaging firebaseMessaging;
+    private final FcmClient fcmClient;
    private final MemberFCMService memberFCMService;

-    public FCMNotificationEventListener(FirebaseMessaging firebaseMessaging, MemberFCMService memberFCMService) {
-        this.firebaseMessaging = firebaseMessaging;
+    public FCMNotificationEventListener(FcmClient fcmClient, MemberFCMService memberFCMService) {
+        this.fcmClient = fcmClient;
        this.memberFCMService = memberFCMService;
    }

    @TransactionalEventListener(phase = TransactionPhase.AFTER_COMMIT)
    @Async
    public void sendFcmNotification(EntryProcessEvent event) {
-        List<Message> messages = createMessages(getMemberFCMToken(event.memberId()), FCMChannel.NOT_DEFINED.name());
-        try {
-            BatchResponse batchResponse = firebaseMessaging.sendAll(messages);
-            checkAllSuccess(batchResponse, event.memberId());
-        } catch (FirebaseMessagingException e) {
-            log.error("fail send FCM message", e);
-            throw new InternalServerException(ErrorCode.FAIL_SEND_FCM_MESSAGE);
-        }
+        List<String> tokens = getMemberFCMToken(event.memberId());
+        fcmClient.sendAll(tokens, FCMChannel.ENTRY_PROCESS, FcmPayload.entryProcess());
    }

    private List<String> getMemberFCMToken(Long memberId) {
-        return memberFCMService.findMemberFCM(memberId).memberFCMs().stream()
+        return memberFCMService.findMemberFCM(memberId)
+            .memberFCMs()
+            .stream()
            .map(MemberFCMResponse::fcmToken)
            .toList();
    }
-
-    private List<Message> createMessages(List<String> tokens, String channelId) {
-        return tokens.stream()
-            .map(token -> createMessage(token, channelId))
-            .toList();
-    }
-
-    private Message createMessage(String token, String channelId) {
-        return Message.builder()
-            .setAndroidConfig(createAndroidConfig(channelId))
-            .setToken(token)
-            .build();
-    }
-
-    private AndroidConfig createAndroidConfig(String channelId) {
-        return AndroidConfig.builder()
-            .setNotification(createAndroidNotification(channelId))
-            .build();
-    }
-
-    private AndroidNotification createAndroidNotification(String channelId) {
-        return AndroidNotification.builder()
-            .setChannelId(channelId)
-            .build();
-    }
-
-    private void checkAllSuccess(BatchResponse batchResponse, Long memberId) {
-        List<SendResponse> failSend = batchResponse.getResponses().stream()
-            .filter(sendResponse -> !sendResponse.isSuccessful())
-            .toList();
-
-        log.warn("member {} 에 대한 다음 요청들이 실패했습니다. {}", memberId, failSend);
-        throw new InternalServerException(ErrorCode.FAIL_SEND_FCM_MESSAGE);
-    }


@Component public class FCMNotificationEventListener { private final FcmClient fcmClient; private final MemberFCMService memberFCMService; public FCMNotificationEventListener(FcmClient fcmClient, MemberFCMService memberFCMService) { this.fcmClient = fcmClient; this.memberFCMService = memberFCMService; } @TransactionalEventListener(phase = TransactionPhase.AFTER_COMMIT) @Async public void sendFcmNotification(EntryProcessEvent event) { List<String> tokens = memberFCMService.findAllMemberFCMToken(event.memberId()); fcmClient.sendAll(tokens, FCMChannel.ENTRY_PROCESS, FcmPayload.entryProcess()); } }

@Service @Transactional public class MemberFCMService { ... @Transactional(readOnly = true) public List<String> findAllMemberFCMToken(Long memberId) { return memberFCMRepository.findAllTokenByMemberId(memberId); } ... }

public interface MemberFCMRepository extends JpaRepository<MemberFCM, Long> { ... @Query("SELECT mf.fcmToken FROM MemberFCM mf WHERE mf.memberId = :memberId") List<String> findAllTokenByMemberId(@Param("memberId") Long memberId); ... }

다음과 같은 방법은 어떠신가요?

반영완료했습니다~

carsago

애쉬 고생하셨습니다. 코멘트 달아봤어요.

backend/src/main/java/com/festago/entry_alert/application/EntryAlertEventListener.java

carsago · 2023-10-03T08:45:05Z

backend/src/main/java/com/festago/entry_alert/application/EntryAlertService.java

+        List<String> tokens = memberFCMRepository.findAllByMemberIdIn(memberIds)
+            .stream()
+            .map(MemberFCM::getFcmToken)
+            .toList();


찾아보니 이런 방법도 있네요.
전 글렌 코멘트대로 JPQL로 처리하는게 더 간단하고 좋다고 합니다만, 그래도 알게되서 참고용으로 링크 걸어봅니다.
인덱스 컨디션 등으로 성능 최적화 시킬 부분이 많고 유지 보수가 어려운 특정 프로젝트에서는 관리 포인트를 편하게하기 위해 사용할 수 있을 것 같습니다.

https://stackoverflow.com/questions/22007341/spring-jpa-selecting-specific-columns

저희 프로젝트에서는 만약 적용한다면 이런식으로..(일케 하자는건 아니고 그냥 참고용입니당. 위에서 적은 것처럼 저희 프로젝트에서는 JPQL이 지금은 더 좋다고 생각해요.)

public interface FcmToken { String getFcmToken(); } 혹은 레코드 public record FcmToken(String fcmToken) { }

List<FcmToken> findTokenByMemberIdIn(List<Long> memberIds); .... Hibernate: select m1_0.fcm_token from member_fcm m1_0 where m1_0.member_id in(?,?)

carsago · 2023-10-03T08:49:11Z

backend/src/main/java/com/festago/entry_alert/application/EntryAlertService.java

+    private void sendMessages(List<String> tokens) {
+        for (int i = 0; i < tokens.size(); i += BATCH_ALERT_SIZE) {
+            List<String> subTokens = tokens.subList(i, Math.min(i + BATCH_ALERT_SIZE, tokens.size()));
+            fcmClient.sendAllAsync(subTokens, FCMChannel.ENTRY_ALERT, FcmPayload.entryAlert());
+        }
+    }


글렌 의견에 동의합니다.

carsago · 2023-10-03T09:04:47Z

backend/src/main/java/com/festago/config/AsyncConfig.java

-    private static final String DEFAULT_EXECUTOR_NAME = "taskExecutor";
-    public static final String FCM_EXECUTOR_NAME = "fcmExecutor";
-
-    @Bean(name = DEFAULT_EXECUTOR_NAME)
-    public Executor defaultExecutor() {
-        ThreadPoolTaskExecutor executor = new ThreadPoolTaskExecutor();
-        executor.setCorePoolSize(8);
-        executor.initialize();
-        return executor;
-    }
-
-    @Bean(name = FCM_EXECUTOR_NAME)
-    public Executor fcmExecutor() {
-        ThreadPoolTaskExecutor executor = new ThreadPoolTaskExecutor();
-        executor.setCorePoolSize(8);
-        executor.setQueueCapacity(10);
-        executor.initialize();
-        return executor;
-    }


이 부분은 어떤 맥락에서 추가됐다가 다시 사라진 걸까요 ?_?

현재 저희 어플리케이션에서 async 태스크들이 제법 많아졌고, 이벤트 기반으로 수정되며 async 태스크들이 앞으로도 많이 추가될 것으로 예상했는데요

FCM 전송이 500건당 스레드를 하나씩 점유하기때문에, 동시에 많은 사람들에게 FCM을 전송할 경우 다른 async 태스크들도 영향을 받을기에 FCM 스레드풀로 별도로 두는게 좋지않을까? 라는 생각에 분리를 했다가

굳이 지금 그럴 필요는 없을 것 같다는 생각에 롤백했습니다 ㅋㅋㅋ

@async 를 썼을 때 기본적으로 사용하는 스레드 풀 이름이 "taskExecutor" 였군요! overflow
기가 막합니다 👍

맞아요! 현재는 롤백되었지만.. 2개 이상의 Executor를 Bean으로 등록할때는 "taskExecutor"가 기본적으로 사용됩니다~!
"taskExecutor" 네이밍의 빈이 없을 때는 별도의 Qualifier를 지정하지 않은 경우엔 SimpleAsyncTaskExecutor를 사용하기때문에 주의해야합니다 ㅎㅎ

출처: 도둑탈을 쓴 애쉬 ㅋㅋ

carsago · 2023-10-03T09:25:28Z

backend/src/main/java/com/festago/entry_alert/application/EntryAlertService.java

+        EntryAlert entryAlert = entryAlertRepository.findByIdForUpdate(id)
+            .orElseThrow(() -> new ConflictException(ErrorCode.ALREADY_ALERT));


여기서 delete 하는 방법도 있지만 처리전, 처리후 같은 식의 상태를 변경시키는 방법은 어떨까요?

그렇게한다면 굳이 throw안하고, early return 하는 방법으로도 처리할 수도 있고
제대로 스케쥴링이 처리됐는지, 명시적으로 확인할 수도 있을 것 같아요.
데이터가 쌓인다는 점이 있지만 수백~수천개 정도의 데이터는 큰 문제가 안되고, 나중에 주기적으로 비워줄 수도 있으니까요.

seokjin8678 · 2023-10-05T05:28:01Z

backend/src/main/java/com/festago/fcm/infrastructure/FcmClientImpl.java

+    private void validateEmptyTokens(List<String> tokens) {
+        if (tokens.isEmpty()) {
+            throw new InternalServerException(ErrorCode.FCM_NOT_FOUND);
+        }
+    }


그냥 갑자기 든 생각인데, 공연에 참가자가 아무도 없는 상황에서 알림을 보내면 emptyList가 보내어 질텐데, 이때 예외를 발생시키는 것이 과연 괜찮을까 생각이 드네요.
그냥 빈 토큰이라면 early return 해버리는 것도 좋을 것 같아요.

사실 해당 메서드가 비동기 기반으로 호출되는 메서드라 예외가 발생해도 사실상 아무 일도 없을거에요
그런데 이 상황은 글렌 말씀처럼 확실히 예외상황이 아닌 것 같네요 ㅎㅎ
early return으로 수정하겠습니다!!

이게 입장알림이 아니라 QR상태변화용 FCM인 경우
사용자 토큰이 없으면 QR 상태를 변화시킬 수 없어서
푸우가 기존 작성하신 코드에 예외로 처리되어있었을거에요!

맞아요 그래서 로그인한 유저는 무조건 fcm이 있어야한다는 고민에 빠졌다가 나중에 이런 기능이 필요하다면 안드로이드 측에서 fcm 이 필수로 필요한 것은 동기적으로 fcm 을 저장하는 api 를 사전에 호출하면 될 것 같습니다🫡

seokjin8678 · 2023-10-05T05:33:21Z

backend/src/main/java/com/festago/entryalert/application/EntryAlertService.java

+        EntryAlert entryAlert = entryAlertRepository.findByIdAndStatusForUpdate(id, AlertStatus.PENDING)
+            .orElseThrow(() -> new ConflictException(ErrorCode.ALREADY_ALERT));
+        List<String> tokens = findFcmTokens(entryAlert);
+        taskExecutor.execute(() -> fcmClient.sendAll(tokens, FCMChannel.ENTRY_ALERT, FcmPayload.entryAlert()));


EntryAlertFcmClient 클래스가 사라졌네요!
TaskExecutor의 execute 메소드를 호출해서 비동기로 실행하는 것보다 다시 EntryAlertFcmClient를 추가해서 비동기로 실행하는 것은 어떤가요?
그렇게 한다면 다음과 같이 실패한 요청을 로그로 남길 수 있을 것 같아요.

@Component @RequiredArgsConstructor @Slf4j public class EntryAlertFcmClient { private final FcmClient fcmClient; @Async public void sendAll(Long entryAlertId, List<String> tokens, FcmPayload payload) { boolean isSuccess = fcmClient.sendAll(tokens, FCMChannel.ENTRY_ALERT, payload); if (!isSuccess) { log.warn("무대 입장 알림 전송이 실패했습니다. entryAlertId: {}", entryAlertId); } } }

sendAll에서 500건 이상이면 내부적으로 Async로 동작해서(500건씩 잘라서 별도의 스레드에서 전송) isSuccess 여부를 알아오기가 쉽지 않아요 ... ㅠㅠ
그걸 알기 위해선 위의 CompletableFuture 도입이 다시 필요해집니다 🥲

EntryAlertFcmClient를 굳이 클래스로 분리하지 않은 이유는
fcmClient.sendAll(tokens, FCMChannel.ENTRY_ALERT, payload); 메서드를 호출하는 역할 뿐이어서
taskExecutor.execute 하는게 더 간단하고 Async로 돌아가는걸 메서드단에서 파악하기 쉬워서 분리하지않았는데
이 부분은 분리하는게 좋을까요?!

아니면 500건 이상일때 비동기로 보내지않고 한 스레드에서 순차적으로 보내는 방법도 존재합니다..!!

500건 이상일때 순차적으로 보내는 것도 좋겠네요. 😂

backend/src/main/java/com/festago/entryalert/application/EntryAlertService.java

backend/src/main/java/com/festago/entryalert/domain/EntryAlert.java

seokjin8678

롬복 적용할 수 있는 것에 커멘트 남겼습니다.
일부에도 적용할 수 있을 것 같은데, 471번 이슈가 머지되면 결국 변경해야 하므로 우선 뒀습니다.

backend/src/main/java/com/festago/entryalert/application/EntryAlertEventListener.java

backend/src/main/java/com/festago/entryalert/domain/EntryAlert.java

carsago · 2023-10-05T06:53:25Z

애쉬 고생하셨습니다. 제가 보기엔 글렌이 말씀해주신 롬복 어노테이션 일부분만 하면 이만 마무리해도 될것같아요.
굳이 하자면 메시지 보내는 FcmPayload인데 애쉬도 생각하셨겠지만 이건 어차피 안드랑 이야기해야하니 내일 이야기해서 커밋 하나정도만 추가되면 되지 않을까 싶습니다.

BGuga

애쉬 너무 수고 많으셨습니다 👍
이야기할 것은 이미 거의 다 나온 것 같네요!!
@Profile 설정 같은 경우는 현재 dev 로 실행하면 duplicate bean이 뜨니 꼭 확인해 주세요 ㅎㅎ

backend/src/main/java/com/festago/fcm/infrastructure/FcmClientImpl.java

BGuga · 2023-10-05T09:49:45Z

backend/src/main/java/com/festago/config/AsyncConfig.java

-    private static final String DEFAULT_EXECUTOR_NAME = "taskExecutor";
-    public static final String FCM_EXECUTOR_NAME = "fcmExecutor";
-
-    @Bean(name = DEFAULT_EXECUTOR_NAME)
-    public Executor defaultExecutor() {
-        ThreadPoolTaskExecutor executor = new ThreadPoolTaskExecutor();
-        executor.setCorePoolSize(8);
-        executor.initialize();
-        return executor;
-    }
-
-    @Bean(name = FCM_EXECUTOR_NAME)
-    public Executor fcmExecutor() {
-        ThreadPoolTaskExecutor executor = new ThreadPoolTaskExecutor();
-        executor.setCorePoolSize(8);
-        executor.setQueueCapacity(10);
-        executor.initialize();
-        return executor;
-    }


@async 를 썼을 때 기본적으로 사용하는 스레드 풀 이름이 "taskExecutor" 였군요! overflow
기가 막합니다 👍

BGuga · 2023-10-05T11:44:56Z

backend/src/main/java/com/festago/fcm/infrastructure/FcmClientImpl.java

+    private void validateEmptyTokens(List<String> tokens) {
+        if (tokens.isEmpty()) {
+            throw new InternalServerException(ErrorCode.FCM_NOT_FOUND);
+        }
+    }


맞아요 그래서 로그인한 유저는 무조건 fcm이 있어야한다는 고민에 빠졌다가 나중에 이런 기능이 필요하다면 안드로이드 측에서 fcm 이 필수로 필요한 것은 동기적으로 fcm 을 저장하는 api 를 사전에 호출하면 될 것 같습니다🫡

backend/src/main/java/com/festago/fcm/infrastructure/FcmClientImpl.java

backend/src/main/java/com/festago/fcm/application/MockFcmClient.java

BGuga · 2023-10-05T12:28:13Z

backend/src/main/java/com/festago/fcm/application/FCMNotificationEventListener.java

-    private static final Logger log = LoggerFactory.getLogger(FCMNotificationEventListener.class);
-
-    private final FirebaseMessaging firebaseMessaging;
+    private final FcmClient fcmClient;


FcmClient 를 추상화해준 이유가 추후에 IosFcmClient, webFcmClient를 고려한 것이 맞을까용?

확장을 위한 추상화보다는 DIP를 위한 추상화입니다!!
FirebaseMessaging은 외부 라이브러리라고 판단해 infrastructure 레이어에 두기 위해서 추상화를 진행했습니다..!

BGuga

앗 하나더 😅

BGuga · 2023-10-05T12:50:00Z

backend/src/main/java/com/festago/fcm/application/FcmClient.java

+
+    void sendAll(List<String> tokens, FCMChannel channel, FcmPayload fcmPayload);


Suggested change

void sendAll(List<String> tokens, FCMChannel channel, FcmPayload fcmPayload);

@Async

void sendAll(List<String> tokens, FCMChannel channel, FcmPayload fcmPayload);

이렇게 달면 안돼나요? overflow

놀랍게도 sendAll은 Async 메서드가 아닙니다..!
그 이유는 QR 상태 변경용 FCM에서는 이미 비동기 메서드에서 해당 메서드를 호출하기때문에
해당 메서드에 @Async 달면 두 번 Async 처리가 되더라구요!
그래도 큰 문제는 없지만, 굳이 두 번 Async를 탈 필요는 없어서 저렇게 진행했습니다

그러면

@Async public void sendEntryAlert(Long id) { EntryAlert entryAlert = entryAlertRepository.findByIdAndStatusForUpdate(id, AlertStatus.PENDING) .orElseThrow(() -> new ConflictException(ErrorCode.ALREADY_ALERT)); List<String> tokens = findFcmTokens(entryAlert); log.info("EntryAlert 전송 시작 / entryAlertId: {} / to {} devices", id, tokens.size()); taskExecutor.execute(() -> fcmClient.sendAll(tokens, FCMChannel.ENTRY_ALERT, FcmPayload.entryAlert())); ✅ entryAlert.request(); }

마찬가지로 요기도

@Async public void sendEntryAlert(Long id) { EntryAlert entryAlert = entryAlertRepository.findByIdAndStatusForUpdate(id, AlertStatus.PENDING) .orElseThrow(() -> new ConflictException(ErrorCode.ALREADY_ALERT)); List<String> tokens = findFcmTokens(entryAlert); log.info("EntryAlert 전송 시작 / entryAlertId: {} / to {} devices", id, tokens.size()); fcmClient.sendAll(tokens, FCMChannel.ENTRY_ALERT, FcmPayload.entryAlert()); ✅ entryAlert.request(); }

이렇게 하면 안되는 것인가용?

그래도 되긴 하는데... entryAlertRepository.findByIdAndStatusForUpdate() 메서드가 락을 걸기도 하고, 트랜잭션 범위안에 있어서 이건 비동기로 재호출을 한 번 더 해야될 것 같네요

아아 x 락 범위가 있었네요 굿굿

seokjin8678

재택이라 그런지 오늘 애쉬 PR만 팬 것 같네요. 😂
많은 작업 하느라 고생 많으셨습니다.
이만 하산하시죠 ⛰

seokjin8678 · 2023-10-09T12:54:08Z

잠깐 생각이 나서 간단하게 푸시 알림 전송 결과를 DB에 업데이트 하는 로직을 생각해봤어요

@Service
@Transactional
@RequiredArgsConstructor
@Slf4j
public class EntryAlertService {
    ...
    private final EntryAlertFcmService entryAlertFcmService;
    ...
    @Async
    public void sendEntryAlert(Long id) {
        entryAlertRepository.findByIdAndStatusForUpdate(id, AlertStatus.PENDING)
            .ifPresent(entryAlert -> {
                List<String> tokens = findFcmTokens(entryAlert);
                log.info("EntryAlert 전송 시작 / entryAlertId: {} / to {} devices", id, tokens.size());
                taskExecutor.execute(() -> entryAlertFcmService.send(id, tokens));
                entryAlert.changeRequested();
            });
    }
    ...
}

@Service
@RequiredArgsConstructor
@Slf4j
public class EntryAlertFcmService {

    private final EntryAlertRepository entryAlertRepository;
    private final TransactionTemplate transactionTemplate;
    private final FcmClient fcmClient;

    public void send(Long id, List<String> tokens) {
        AlertStatus result = sendEntryAlert(id, tokens);
        transactionTemplate.execute(status -> {
            changeEntryAlertStatus(id, result);
            return null;
        });
    }

    private AlertStatus sendEntryAlert(Long id, List<String> tokens) {
        try {
            fcmClient.sendAll(tokens, FCMChannel.ENTRY_ALERT, FcmPayload.entryAlert());
            log.info("EntryAlert 전송 성공 / entryAlertId: {}", id);
            return AlertStatus.SUCCEED;
        } catch (FirebaseMessagingException e) {
            log.warn("EntryAlert 전송 실패 / entryAlertId: {}", id);
            return AlertStatus.FAILED;
        }
    }

    private void changeEntryAlertStatus(Long id, AlertStatus alertStatus) {
        EntryAlert entryAlert = entryAlertRepository.findByIdAndStatus(id, AlertStatus.REQUESTED)
            .orElseThrow(IllegalArgumentException::new);
        switch (alertStatus) {
            case SUCCEED -> entryAlert.changeSucceed();
            case FAILED -> entryAlert.changeFailed();
            default -> throw new IllegalArgumentException();
        }
    }
}

TransactionTemplate을 사용하여 self-invocation 문제를 해결하고 메서드의 원하는 부분만 더티리드를 할 수 있게 했어요.
해당 로직을 호출할 때는 X락이 걸려있어 동시성이 보장되므로 Repository에서 for update를 사용하지 않고 가져왔어요.
변화가 생긴다면 FCMClient의 sendAll() 메서드가 FirebaseMessagingException을 던지게 하고 500건 이상의 send는 비동기로 처리하는게 아닌 동기로 처리해야해요.

@Component
@Profile({"dev", "prod"})
@RequiredArgsConstructor
@Slf4j
public class FcmClientImpl implements FcmClient {

    private static final int BATCH_ALERT_SIZE = 500;

    private final FirebaseMessaging firebaseMessaging;

    @Override
    public void sendAll(List<String> tokens, FCMChannel channel, FcmPayload payload) throws FirebaseMessagingException {
        if (tokens.isEmpty()) {
            return;
        }
        List<Message> messages = createMessages(tokens, channel, payload);

        if (messages.size() <= BATCH_ALERT_SIZE) {
            sendMessages(messages, channel);
            return;
        }

        for (int i = 0; i < messages.size(); i += BATCH_ALERT_SIZE) {
            List<Message> batchMessages = messages.subList(i, Math.min(i + BATCH_ALERT_SIZE, messages.size()));
            sendMessages(batchMessages, channel);
        }
    }

    private void sendMessages(List<Message> messages, FCMChannel channel) throws FirebaseMessagingException {
        try {
            BatchResponse response = firebaseMessaging.sendAll(messages);
            checkAllSuccess(response, channel);
        } catch (FirebaseMessagingException e) {
            log.warn("[FCM: {}] 전송 실패: {}", channel, e.getMessagingErrorCode());
            throw e;
        }
    }
    ...
}

다시 생각해보니 for update로 호출해야겠네요.
비동기로 실행되기 때문에 요청을 보내고 상태를 바꾸려고 할 때 조회가 되지 않는 문제가 발생할 수 있겠네요..!

seokjin8678 · 2023-10-09T12:58:34Z

backend/src/main/resources/application-local.yml

+server:
+  shutdown: graceful
+


graceful shutdown 👍👍
submodule에도 적용을 해야겠네요!

SeongHoonC · 2023-12-11T05:43:46Z

불필요 브랜치를 삭제하다가 삭제해버렸네요. 죄송합니다!

xxeol2 added 12 commits October 2, 2023 04:10

feat: 입장 알림 전송 기능 구현

cbcad44

refactor: FCMClient 인터페이스화

21f86ff

refactor: 스케쥴링 중복 실행시 409 Conflict

4c17ab6

refactor: entryAlert 비관적락 적용

2e4530d

refactor: EntryAlertEventListener에서 EntryAlertRepository 참조 제거

4a77dd7

refactor: FCM Aycnc Config 정의

690f90f

feat: flyway script 작성

323c6d9

feat: MockFcmClient 생성

1651604

refactor: FCMNotificationEventListener Async 빈 지정

3b308b5

feat: SpringBoot 실행시 initSchedule

97daba8

refactor: Async ThreadPool 하나만 활용하도록 수정

aa4e530

test: FcmClientImplTet 작성

44ced57

xxeol2 added BE 백엔드에 관련된 작업 🙋‍♀️ 제안 제안에 관한 작업 ☢️ DB 데이터베이스에 변경이 있는 작업 labels Oct 2, 2023

xxeol2 self-assigned this Oct 2, 2023

xxeol2 linked an issue Oct 2, 2023 that may be closed by this pull request

[BE] 입장 10분 전 입장 알림 푸시알림을 보낸다. #483

Open

xxeol2 commented Oct 2, 2023

View reviewed changes

seokjin8678 requested changes Oct 2, 2023

View reviewed changes

xxeol2 added 3 commits October 2, 2023 22:59

feat: MemberFcmFixture 정의

bd7451b

refactor: findAllTokenByMemberIdIn 메서드 활용

221cac0

style: 패키지 네이밍 수정 (entry_alert -> entryalert)

a083e3c

carsago requested changes Oct 3, 2023

View reviewed changes

xxeol2 added 4 commits October 4, 2023 16:35

refactor: 500건만 전송하도록 하는 로직 FcmClientImpl로 이동

6aebcd5

refactor: fcmToken String으로 추출

da70c4d

refactor: ApplicationReadyEvent Listener 최상단으로 이동

4c20e30

refactor: taskScheduler에 람다식으로 넘겨주도록 리팩토링

03588d1

xxeol2 added 2 commits October 5, 2023 14:12

remove: 불필요한 유틸 클래스 삭제

310cd14

refactor: Test 의존성 추가

ce2adf1

xxeol2 requested review from carsago and seokjin8678 October 5, 2023 05:20

seokjin8678 reviewed Oct 5, 2023

View reviewed changes

xxeol2 added 2 commits October 5, 2023 14:33

refactor: FCmClient로 빈 토큰이 넘어오면 early return

b081b35

feat: EntryAlert 알림 전송 시작 로깅

40f9359

seokjin8678 reviewed Oct 5, 2023

View reviewed changes

backend/src/main/java/com/festago/entryalert/application/EntryAlertService.java Outdated Show resolved Hide resolved

seokjin8678 reviewed Oct 5, 2023

View reviewed changes

backend/src/main/java/com/festago/entryalert/domain/EntryAlert.java Outdated Show resolved Hide resolved

seokjin8678 reviewed Oct 5, 2023

View reviewed changes

backend/src/main/java/com/festago/entryalert/application/EntryAlertEventListener.java Outdated Show resolved Hide resolved

backend/src/main/java/com/festago/entryalert/domain/EntryAlert.java Outdated Show resolved Hide resolved

BGuga reviewed Oct 5, 2023

View reviewed changes

xxeol2 added 6 commits October 5, 2023 21:56

fix: MockFcmClient Profile 수정

8965022

refactor: FcmClientImpl Executor -> TaskExecutor로 수정

e374d66

refactor: 접근제어자 변경 public -> private

d1f1792

refactor: 롬복 적용

6e06a21

refactor: EntryAlert request상태로 변경 메서드 네이밍 수정

94a3d55

refactor: sendEntryAlert에서 예외를 던지지 않는 방향으로 수정

e45fde3

seokjin8678 approved these changes Oct 5, 2023

View reviewed changes

refactor: graceful shutdown 설정

e2b1eaa

seokjin8678 reviewed Oct 9, 2023

View reviewed changes

SeongHoonC closed this Dec 11, 2023

SeongHoonC deleted the feat/#483 branch December 11, 2023 05:28

SeongHoonC restored the feat/#483 branch December 11, 2023 05:43

SeongHoonC reopened this Dec 11, 2023

xxeol2 closed this Dec 20, 2023

		EntryAlert entryAlert = entryAlertRepository.findByIdForUpdate(id)
		.orElseThrow(() -> new ConflictException(ErrorCode.ALREADY_ALERT));


		void sendAll(List<String> tokens, FCMChannel channel, FcmPayload fcmPayload);

[BE] 입장 10분 전 푸시 알림 전송 기능 구현 (#483) #484

[BE] 입장 10분 전 푸시 알림 전송 기능 구현 (#483) #484

Conversation

xxeol2 commented Oct 2, 2023

📌 관련 이슈

✨ PR 세부 내용

github-actions bot commented Oct 2, 2023 • edited Loading

Unit Test Results

Choose a reason for hiding this comment

seokjin8678 Oct 2, 2023 • edited Loading

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

seokjin8678 left a 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

seokjin8678 Oct 2, 2023 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

carsago left a 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

xxeol2 Oct 4, 2023 • edited Loading

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

xxeol2 Oct 5, 2023 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

xxeol2 Oct 5, 2023 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

xxeol2 Oct 5, 2023 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

seokjin8678 left a comment

Choose a reason for hiding this comment

carsago commented Oct 5, 2023

BGuga left a 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

xxeol2 Oct 5, 2023 • edited Loading

Choose a reason for hiding this comment

BGuga left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

xxeol2 Oct 5, 2023 • edited Loading

Choose a reason for hiding this comment

BGuga Oct 5, 2023 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

seokjin8678 left a comment

Choose a reason for hiding this comment

seokjin8678 commented Oct 9, 2023 • edited Loading

Choose a reason for hiding this comment

SeongHoonC commented Dec 11, 2023

github-actions bot commented Oct 2, 2023 •

edited

Loading

seokjin8678 Oct 2, 2023 •

edited

Loading

seokjin8678 Oct 2, 2023 •

edited

Loading

xxeol2 Oct 4, 2023 •

edited

Loading

xxeol2 Oct 5, 2023 •

edited

Loading

xxeol2 Oct 5, 2023 •

edited

Loading

xxeol2 Oct 5, 2023 •

edited

Loading

xxeol2 Oct 5, 2023 •

edited

Loading

xxeol2 Oct 5, 2023 •

edited

Loading

BGuga Oct 5, 2023 •

edited

Loading

seokjin8678 commented Oct 9, 2023 •

edited

Loading