[feature] React Native 알림 기능 구현 API#792
Conversation
Post - 구독중인 클럽 id를 업데이트합니다 Get - 구독중인 클럽 id를 가져옵니다
구독 요청이 한건당 평균 250ms 처리됨 따라서 동아리 7개만되더라도 한요청에 1.8ms 걸리기에 비동기 처리를함 아래 결과를 통해 비동기로의 변경 필요성을 확인할 수 있다. 변경 결과 [동기 처리] 3개 816ms 1개 287ms 7개 1800ms avg : 263ms [비동기 처리] 11개 -> 77ms 9개 -> 91ms avg : 81ms
ApiFuture.get()은 블로킹 메서드. 그렇기에 메인 쓰레드 대신 async 쓰레드에서 fcm subscription 오류 관리
여러번 요청시 write conflict 오류 발생. 데이터 정합성을 위해 Retry 로직 도입 디비 쓰기작업이 30ms 정도로 관측됐기에 오류 발생시 빠른 재시도(100ms) 실행
해당 json format을 시크릿에 넣을 시 malformed 예외 발생 따라서 base64 인코딩 후 디코딩
async를 동기로 변경하는 빈이름을 덮어씌울수 있게 변경
There was a problem hiding this comment.
Actionable comments posted: 4
🧹 Nitpick comments (3)
backend/src/main/java/moadong/global/config/AsyncConfig.java (1)
15-25: Executor 종료 처리 설정 추가를 권장합니다.현재 ThreadPoolTaskExecutor가 애플리케이션 종료 시 진행 중인 작업을 기다리지 않습니다. FCM 구독 작업이 중단되면 사용자의 알림 구독 상태가 불일치할 수 있습니다.
다음 설정을 추가하여 graceful shutdown을 보장하세요:
@Bean(name = "fcmAsync") public TaskExecutor taskExecutor() { ThreadPoolTaskExecutor executor = new ThreadPoolTaskExecutor(); executor.setCorePoolSize(4); executor.setMaxPoolSize(8); executor.setQueueCapacity(100); executor.setThreadNamePrefix("moadong-async-"); executor.setRejectedExecutionHandler(new ThreadPoolExecutor.CallerRunsPolicy()); + executor.setWaitForTasksToCompleteOnShutdown(true); + executor.setAwaitTerminationSeconds(60); executor.initialize(); return executor; }backend/src/main/java/moadong/fcm/service/FcmAsyncService.java (2)
76-82: 비동기 메서드의 예외 처리 방식을 개선하세요.
RuntimeException을 던지면@Async메서드에서 예외가 제대로 전파되지 않을 수 있습니다.CompletableFuture.failedFuture()를 사용하거나 예외를 적절히 래핑하는 것이 더 명확합니다.다음과 같이 개선할 수 있습니다:
} catch (ExecutionException | TimeoutException e) { log.error("error: {}", e.getMessage()); - throw new RuntimeException(e); + return CompletableFuture.failedFuture(e); } catch (InterruptedException e) { Thread.currentThread().interrupt(); - throw new RuntimeException(e); + return CompletableFuture.failedFuture(e); }
41-52: 구독/구독 해제 로직의 중복을 제거할 수 있습니다.구독과 구독 해제의 반복문 구조가 동일합니다. 헬퍼 메서드로 추출하면 가독성과 유지보수성이 향상됩니다.
다음과 같이 리팩터링할 수 있습니다:
private void addSubscriptionFutures(List<ApiFuture<TopicManagementResponse>> futures, String token, Set<String> clubIds, boolean subscribe) { if (clubIds.isEmpty()) return; for (String clubId : clubIds) { ApiFuture<TopicManagementResponse> future = subscribe ? firebaseMessaging.subscribeToTopicAsync(Collections.singletonList(token), clubId) : firebaseMessaging.unsubscribeFromTopicAsync(Collections.singletonList(token), clubId); futures.add(future); } }그리고 호출부를 다음과 같이 단순화:
addSubscriptionFutures(futures, token, clubsToSubscribe, true); addSubscriptionFutures(futures, token, clubsToUnsubscribe, false);
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
Disabled knowledge base sources:
- Jira integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (3)
backend/src/main/java/moadong/fcm/service/FcmAsyncService.java(1 hunks)backend/src/main/java/moadong/global/config/AsyncConfig.java(1 hunks)backend/src/test/java/moadong/fcm/service/FcmServiceTest.java(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- backend/src/test/java/moadong/fcm/service/FcmServiceTest.java
🧰 Additional context used
🧬 Code graph analysis (1)
backend/src/main/java/moadong/fcm/service/FcmAsyncService.java (1)
backend/src/main/java/moadong/fcm/service/FcmService.java (1)
Slf4j(15-72)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: test
🔇 Additional comments (1)
backend/src/main/java/moadong/fcm/service/FcmAsyncService.java (1)
37-85: FcmService의 검증 로직이 실제로 오류입니다. 조건을 수정해야 합니다.분석 결과:
- Line 46-47:
clubsToSubscribe= 새로 추가할 동아리들- Line 49-50:
clubsToUnsubscribe= 제거할 동아리들- Line 52-58: 현재 검증은
clubsToUnsubscribe가 비어있지 않을 때만 실행됨 → 이는 역순입니다문제점:
- 구독만 추가하는 경우(
clubsToUnsubscribe가 비어있음) → 검증이 건너뜀 ❌- 구독 해제할 때(
clubsToUnsubscribe가 비어있지 않음) →clubsToSubscribe검증 실행 ❌수정 필요:
- if (!clubsToUnsubscribe.isEmpty()) { + if (!clubsToSubscribe.isEmpty()) { Long countClub = clubRepository.countByIdIn(clubsToSubscribe.stream().toList()); if (countClub != clubsToSubscribe.size()) { throw new RestApiException(ErrorCode.CLUB_NOT_FOUND); } }새 동아리 추가 전에 반드시 검증이 실행되어야 합니다.
⛔ Skipped due to learnings
Learnt from: lepitaaar PR: Moadong/moadong#406 File: backend/src/main/java/moadong/club/service/ClubApplyService.java:34-38 Timestamp: 2025-05-19T05:45:52.957Z Learning: The code duplication between createClubApplication and editClubApplication methods in ClubApplyService.java is acknowledged but will be addressed in a future refactoring, as per the developer's plan.
구독할 목록이 존재할때 클럽 목록을 검증해야하는데 현재 구독 해제할 목록을 검증했음
There was a problem hiding this comment.
Actionable comments posted: 4
🧹 Nitpick comments (3)
backend/src/main/java/moadong/fcm/service/FcmTxService.java (1)
19-23: 클래스 레벨 재시도 정책을 재검토하세요.클래스 전체에
@Retryable을 적용하면 모든 메서드가DataAccessException발생 시 재시도합니다. 현재는 두 메서드 모두 DB 작업을 수행하므로 적절하지만, 향후 재시도가 불필요한 메서드가 추가될 경우 문제가 될 수 있습니다.필요시 메서드별로 재시도 정책을 개별 적용하는 것을 고려하세요.
backend/src/main/java/moadong/fcm/service/FcmAsyncService.java (2)
59-72: 응답 처리 로직이 개선되었으나 에러 로그 개선을 권장합니다.이전 리뷰에서 지적된
null반환 문제가 수정되었습니다(Line 66).다만 Line 69의 에러 로그에 실패 원인이 더 명확히 드러나도록 개선할 수 있습니다.
-log.error("FCM topic sub failed for {}. errors={}", token, response.getErrors()); +log.error("FCM topic subscription/unsubscription failed for token {}. Failure count: {}, errors: {}", + token, response.getFailureCount(), response.getErrors());
74-85: 예외 처리가 개선되었으나 로그 메시지 구체화를 권장합니다.이전 리뷰에서 지적된
null반환 문제가 수정되었습니다(Line 84).Line 77의 에러 로그 메시지가 너무 일반적입니다. 어떤 작업이 실패했는지 명시하면 디버깅이 용이합니다.
-log.error("error: {}", e.getMessage()); +log.error("FCM topic subscription update failed for token {}: {}", token, e.getMessage());
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
Disabled knowledge base sources:
- Jira integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (9)
backend/src/main/java/moadong/club/entity/Club.java(3 hunks)backend/src/main/java/moadong/club/util/RecruitmentStateCalculator.java(1 hunks)backend/src/main/java/moadong/fcm/entity/FcmToken.java(1 hunks)backend/src/main/java/moadong/fcm/service/FcmAsyncService.java(1 hunks)backend/src/main/java/moadong/fcm/service/FcmService.java(1 hunks)backend/src/main/java/moadong/fcm/service/FcmTxService.java(1 hunks)backend/src/main/java/moadong/global/util/FcmInitializer.java(1 hunks)backend/src/test/java/moadong/fcm/service/FcmServiceTest.java(1 hunks)backend/src/test/java/moadong/fcm/service/FcmTxServiceTest.java(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (4)
- backend/src/test/java/moadong/fcm/service/FcmServiceTest.java
- backend/src/main/java/moadong/global/util/FcmInitializer.java
- backend/src/main/java/moadong/fcm/entity/FcmToken.java
- backend/src/main/java/moadong/club/entity/Club.java
🧰 Additional context used
🧠 Learnings (3)
📚 Learning: 2025-05-19T05:45:52.957Z
Learnt from: lepitaaar
Repo: Moadong/moadong PR: 406
File: backend/src/main/java/moadong/club/service/ClubApplyService.java:34-38
Timestamp: 2025-05-19T05:45:52.957Z
Learning: The code duplication between createClubApplication and editClubApplication methods in ClubApplyService.java is acknowledged but will be addressed in a future refactoring, as per the developer's plan.
Applied to files:
backend/src/main/java/moadong/fcm/service/FcmService.java
📚 Learning: 2025-09-30T05:26:41.788Z
Learnt from: alsdddk
Repo: Moadong/moadong PR: 765
File: backend/src/main/java/moadong/club/service/ClubApplyService.java:431-435
Timestamp: 2025-09-30T05:26:41.788Z
Learning: In the Moadong codebase's club application feature (backend/src/main/java/moadong/club/), multiple ClubApplicationForm entities can have ACTIVE status for the same clubId, semesterYear, and semesterTerm simultaneously. There is no uniqueness constraint requiring only one ACTIVE form per semester.
Applied to files:
backend/src/main/java/moadong/fcm/service/FcmService.java
📚 Learning: 2025-08-25T14:43:52.320Z
Learnt from: lepitaaar
Repo: Moadong/moadong PR: 703
File: backend/src/main/java/moadong/club/controller/ClubApplyController.java:84-84
Timestamp: 2025-08-25T14:43:52.320Z
Learning: In the Moadong codebase, questionId and clubId are equivalent identifiers that represent the same entity. The ClubApplicationRepository.findAllByIdInAndQuestionId method correctly uses clubId as the questionId parameter for filtering club applications.
Applied to files:
backend/src/main/java/moadong/fcm/service/FcmService.java
🧬 Code graph analysis (2)
backend/src/main/java/moadong/fcm/service/FcmService.java (1)
backend/src/main/java/moadong/fcm/service/FcmAsyncService.java (1)
Slf4j(24-86)
backend/src/main/java/moadong/fcm/service/FcmAsyncService.java (1)
backend/src/main/java/moadong/fcm/service/FcmService.java (1)
Slf4j(15-72)
🪛 GitHub Actions: PR Test
backend/src/main/java/moadong/club/util/RecruitmentStateCalculator.java
[error] 74-74: Firebase Message.Builder requires exactly one of token, topic or condition. (IllegalArgumentException) at RecruitmentStateCalculator.buildRecruitmentMessage
🔇 Additional comments (10)
backend/src/main/java/moadong/club/util/RecruitmentStateCalculator.java (2)
18-24: 리팩터링이 잘 되었습니다!관심사 분리가 명확하고 각 단계(상태 계산, 업데이트, 메시지 빌드, 전송)가 잘 구분되어 있습니다.
52-66: 상태별 메시지 구성 로직이 잘 작성되었습니다.한국어 날짜 포맷팅과 상태별 메시지 내용이 명확하고 사용자 친화적입니다. switch 표현식 사용도 적절합니다.
backend/src/test/java/moadong/fcm/service/FcmTxServiceTest.java (1)
26-32: 테스트 설정이 적절합니다.토큰을 생성하고 저장하는 로직이 명확합니다.
backend/src/main/java/moadong/fcm/service/FcmTxService.java (2)
29-35: 이전 리뷰 이슈가 수정되었습니다.토큰이 존재할 때만 삭제 로그를 남기도록 수정되어 더 이상 오해의 소지가 없습니다.
37-43: 토큰 업데이트 로직이 적절합니다.토큰 조회, clubIds 업데이트, 저장 흐름이 명확하고 예외 처리도 적절합니다.
backend/src/main/java/moadong/fcm/service/FcmAsyncService.java (2)
36-53: 비동기 구독 업데이트 설정이 적절합니다.구독/구독 해제 대상을 futures 리스트로 수집하는 로직이 명확합니다.
54-55: 이전 리뷰 이슈가 수정되었습니다.빈 futures 리스트일 때
null대신CompletableFuture.completedFuture(null)을 반환하여 NullPointerException 위험이 제거되었습니다.backend/src/main/java/moadong/fcm/service/FcmService.java (3)
23-37: 토큰 저장 로직이 적절합니다.신규 토큰 생성과 기존 토큰 타임스탬프 업데이트를 명확하게 처리합니다.
39-58: 이전 리뷰 이슈가 수정되었습니다.구독 대상 동아리 존재 여부 검증 조건이 올바르게 수정되었습니다(Line 52:
clubsToSubscribe확인).
67-71: 구독 목록 조회 로직이 적절합니다.토큰 조회 및 응답 생성이 명확합니다.
|
수고하셨습니다!! 모집 끝나기 하루전이나 몇시간전에 모집 마감 직전이라고 한번 더 메시지 보내는 것도 좋을것같습니당 |
좋습니다 나중에 다같이 얘기해보죠 |
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
backend/src/test/java/moadong/club/service/RecruitmentStateCheckerTest.java (1)
50-128: 선택사항: 반복되는 모킹 설정을 고려해보세요.네 개의 테스트에서 동일한
when(club.getId()).thenReturn("1")스텁이 반복됩니다. 필요하다면@BeforeEach메서드에서 공통 설정을 중앙화할 수 있습니다. 다만 현재의 명시적 접근 방식도 각 테스트가 독립적이고 이해하기 쉽다는 장점이 있습니다.예시 (선택사항):
+ private Club club; + private ClubRecruitmentInformation info; + + @BeforeEach + void setUp() { + club = mock(Club.class); + info = mock(ClubRecruitmentInformation.class); + when(club.getId()).thenReturn("1"); + when(club.getClubRecruitmentInformation()).thenReturn(info); + }현재 구조도 충분히 명확하므로 변경하지 않아도 무방합니다.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
Disabled knowledge base sources:
- Jira integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (1)
backend/src/test/java/moadong/club/service/RecruitmentStateCheckerTest.java(4 hunks)
🧰 Additional context used
🧠 Learnings (3)
📚 Learning: 2025-08-25T14:43:52.320Z
Learnt from: lepitaaar
Repo: Moadong/moadong PR: 703
File: backend/src/main/java/moadong/club/controller/ClubApplyController.java:84-84
Timestamp: 2025-08-25T14:43:52.320Z
Learning: In the Moadong codebase, questionId and clubId are equivalent identifiers that represent the same entity. The ClubApplicationRepository.findAllByIdInAndQuestionId method correctly uses clubId as the questionId parameter for filtering club applications.
Applied to files:
backend/src/test/java/moadong/club/service/RecruitmentStateCheckerTest.java
📚 Learning: 2025-05-19T05:45:52.957Z
Learnt from: lepitaaar
Repo: Moadong/moadong PR: 406
File: backend/src/main/java/moadong/club/service/ClubApplyService.java:34-38
Timestamp: 2025-05-19T05:45:52.957Z
Learning: The code duplication between createClubApplication and editClubApplication methods in ClubApplyService.java is acknowledged but will be addressed in a future refactoring, as per the developer's plan.
Applied to files:
backend/src/test/java/moadong/club/service/RecruitmentStateCheckerTest.java
📚 Learning: 2025-09-30T05:26:41.788Z
Learnt from: alsdddk
Repo: Moadong/moadong PR: 765
File: backend/src/main/java/moadong/club/service/ClubApplyService.java:431-435
Timestamp: 2025-09-30T05:26:41.788Z
Learning: In the Moadong codebase's club application feature (backend/src/main/java/moadong/club/), multiple ClubApplicationForm entities can have ACTIVE status for the same clubId, semesterYear, and semesterTerm simultaneously. There is no uniqueness constraint requiring only one ACTIVE form per semester.
Applied to files:
backend/src/test/java/moadong/club/service/RecruitmentStateCheckerTest.java
🔇 Additional comments (1)
backend/src/test/java/moadong/club/service/RecruitmentStateCheckerTest.java (1)
57-57: 테스트 모킹 설정이 올바르고 필요한 것으로 확인되었습니다.
club.getId()스텁이 추가된 이유는 모집 상태 변경 시RecruitmentStateCalculator.buildRecruitmentMessage()가 푸시 메시지의 토픽(topic)으로 사용하기 때문입니다. 상태 변경 시club.getId()는 필수적으로 호출되므로, 해당 테스트들에 스텁이 올바르게 추가되었습니다.상태 변경이 발생하지 않는 첫 번째 테스트(
모집상태_ALWAYS_이면_변경되지_않음)에서는updateRecruitmentStatus가 호출되지 않으므로getId()스텁이 불필요한 것도 적절합니다.
#️⃣연관된 이슈
📝작업 내용
Firebase Cloud Messaging 연동을 위한 api 입니다.
resource/firebase.json에 firebase와 연동이필요한 키를 추가하셔야 합니다.POST /api/fcm앱에 처음 발급된 FCM Token을 timestamp와 저장합니다.
PUT /api/fcm/subscribeclubIds에 들어있는 동아리를 알람 topic에 구독합니다.
기존의 구독중인 목록은 clubIds의 배열로 대체됩니다.
GET /api/fcm/subscribe?fcmToken=~~구독중인 동아리 목록을 모두 가져옵니다.
중점적으로 리뷰받고 싶은 부분
현재
Service레이어에 비지니스 로직을 전부 처리하는 방식이지만,OOP 관점에서
Club에 메시지 전송 책임을 가져야할 것으로 판단되어 해당 비지니스 로직을Club로 옮겼습니다.현재 코드스타일을 해치기 때문에 의견을 내주시면 감사하겠습니다.
비동기 도입
비동기 커밋 : 1209811
fcm topic을 구독할때 clubIds 개수가 많아질수록 한개의 http 요청이 느려지고 블록킹 방식이라 전체 서비스가 느려짐.
따라서 기존
subscribeToTopic,unSubscribeToTopic동기 메서드를subscribeToTopicAsync,unSubscribeToTopicAsync비동기 메서드로 변경1건 평균 전송 속도 85.5% 감소 -> 단일 http 요청 측정입니다.
비동기 서비스 분리
현재 Fcm 메시지 서비스 레이어는 3개로 나뉘어져있습니다.
FcmService-> 일반적인 비즈니스 로직 서비스FcmAsyncService-> FCM서버와 통신하는 Network I/O 비동기 서비스FcmTxService-> db 접근만 관리하는 서비스왜
FcmTxService가 필요하나요?기존엔
FcmAsyncService에 db에 접근했지만,Network 작업과 DB 작업이 강하게 묶여있어 fcm 에러만 발생해도 전체로직을 재시도했어야함.
따라서 서비스를 나누어 db 트랜잭션을 개별적으로 주입하고, write conflict 발생시 재시도 하게 변경. 쓰기 충돌 오류 최소화
@async 스레드 풀 관리
core: 4, max: 8, max-queue: 100
현재 배포 서버 스펙
4 core 고려해 최대 8로 잡음
cpu 부하는 따로 측정해봐야함..
write conflict핸들여러 요청 보낼시 쓰기 충돌 발생
현재 retryable 사용해 최대 2번 100ms 간격으로 재시도
논의하고 싶은 부분
현재 알림에 보내는 메시지를 다같이 생각해보면 좋겠습니다~
🫡 참고사항
Summary by CodeRabbit
새로운 기능
개선 사항
테스트
잡무