-
Notifications
You must be signed in to change notification settings - Fork 0
[Notification][Refactor] 알림 재시도 전략 고도화 #204
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
Merged
Merged
Changes from 3 commits
Commits
Show all changes
5 commits
Select commit
Hold shift + click to select a range
57d3570
[Notification][Refactor] 에러 코드에 따른 재시도 전략 수정 및 서킷브레이커 적용
sunwon12 2eb8131
[Docs] 테스트 규칙 문서 추가
sunwon12 cebc4cb
[Notification][Refactor] 기기별 재시도를 위해 NotificationDeviceLog 추가
sunwon12 be12a6c
[Notification][Remove] 사용하지 않는 dto 삭제
sunwon12 2e07c17
[Notification][Test] 테스트 코드 코드정리
sunwon12 File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -336,14 +336,21 @@ void 책_평점_계산이_올바르게_동작한다() { | |
| - **최소 데이터**: 테스트에 필요한 최소한의 속성만 설정 | ||
|
|
||
| ### 3. 테스트 독립성 | ||
|
|
||
| 테스트는 실행 순서에 의존하지 않아야 하며, 이전 테스트의 상태가 남아있지 않도록 격리해야 합니다. | ||
|
|
||
| ```java | ||
| @BeforeEach | ||
| void setUp() { | ||
| // 각 테스트마다 깨끗한 상태로 시작 | ||
| databaseCleaner.clear(); | ||
|
|
||
| // 1. CircuitBreakerReset: 이전 테스트의 영향(Open/Half-Open) 제거 | ||
| // 핵심: 이전 테스트의 영향도를 삭제하고 '깨끗한 상태'에서 시작 | ||
| circuitBreakerRegistry.circuitBreaker("fcm").transitionToClosedState(); | ||
| } | ||
| ``` | ||
|
|
||
| > **Note**: DB 초기화(`databaseCleaner.clear()`)는 `@IntegrationTest` 및 `@CustomRepositoryTest`에 등록된 `CleanDatabaseBeforeEach` 확장에 의해 **자동으로 수행**되므로 별도로 호출할 필요가 없습니다. 또한, 이때 `BookCategory` 테이블의 **ROOT, NULL** 엔티티와 같은 필수 기초 데이터도 자동으로 복구됩니다. | ||
|
|
||
| ### 4. Fixture를 활용한 의미있는 테스트 데이터 | ||
|
|
||
| Fixture는 4가지 메서드 패턴을 제공하며, 상황에 맞게 선택하여 사용합니다: | ||
|
|
@@ -417,6 +424,36 @@ Member member = new Member("user123", "[email protected]"); | |
| - **도메인 규칙 보장**: 모든 메서드가 유효한 도메인 객체 생성 | ||
| - **테스트 격리**: 각 테스트마다 독립적인 랜덤 데이터 사용 | ||
| - **최소 속성 원칙**: builder 사용 시 테스트에 꼭 필요한 속성만 명시적으로 설정하고, 나머지는 Fixture 기본값 사용 | ||
| 427: | ||
| 428: ### 5. 시간 의존성 테스트 | ||
| 429: | ||
| 430: `LocalDateTime.now()` 등을 내부에서 직접 호출하면 미래/과거 시점 테스트가 어렵습니다. 시간을 파라미터로 받거나 `Clock` Bean을 사용하여 테스트 가능성을 높이세요. | ||
| 431: | ||
| 432: ```java | ||
| 433: // ❌ 테스트하기 어려움 - 내부에서 현재 시간 고정 | ||
| 434: public void processExpired() { | ||
| 435: LocalDateTime now = LocalDateTime.now(); // 테스트에서 제어 불가 | ||
| 436: // ... | ||
| 437: } | ||
| 438: | ||
| 439: // ✅ 테스트하기 쉬움 - 시간을 파라미터로 주입 | ||
| 440: public void processExpired(LocalDateTime currentTime) { | ||
| 441: // currentTime 기준으로 로직 수행 | ||
| 442: } | ||
| 443: | ||
| 444: // ✅ 테스트 코드 예시 | ||
| 445: @Test | ||
| 446: void 만료된_항목을_처리한다() { | ||
| 447: // given: 1시간 뒤 미래 시간을 주입하여 만료 조건 충족 | ||
| 448: LocalDateTime futureTime = LocalDateTime.now().plusHours(1); | ||
| 449: | ||
| 450: // when | ||
| 451: service.processExpired(futureTime); | ||
| 452: | ||
| 453: // then | ||
| 454: // ... | ||
| 455: } | ||
| 456: ``` | ||
|
|
||
| ## 🚫 안티패턴 | ||
|
|
||
|
|
||
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,53 @@ | ||
| package book.book.config; | ||
|
|
||
| import book.book.notification.exception.FcmInvalidTokenException; | ||
| import book.book.notification.exception.FcmRetryableException; | ||
| import io.github.resilience4j.circuitbreaker.CircuitBreakerConfig; | ||
| import io.github.resilience4j.circuitbreaker.CircuitBreakerRegistry; | ||
| import io.github.resilience4j.core.IntervalFunction; | ||
| import io.github.resilience4j.retry.RetryConfig; | ||
| import io.github.resilience4j.retry.RetryRegistry; | ||
| import java.io.IOException; | ||
| import java.time.Duration; | ||
| import java.util.Map; | ||
| import org.springframework.context.annotation.Bean; | ||
| import org.springframework.context.annotation.Configuration; | ||
|
|
||
| @Configuration | ||
| public class Resilience4jConfig { | ||
|
|
||
| @Bean | ||
| public RetryRegistry retryRegistry() { | ||
| RetryConfig fcmRetryConfig = RetryConfig.custom() | ||
| .maxAttempts(4) // 첫 시도 포함 총 4회 (재시도 3회) | ||
| .intervalFunction(IntervalFunction.ofExponentialBackoff( | ||
| Duration.ofSeconds(1), // 초기 1초 | ||
| 2.0 // 1s -> 2s -> 4s | ||
| )) | ||
| .retryExceptions(FcmRetryableException.class, IOException.class) | ||
| .ignoreExceptions(FcmInvalidTokenException.class) | ||
| .build(); | ||
|
|
||
| return RetryRegistry.of(Map.of("fcm", fcmRetryConfig)); | ||
| } | ||
|
|
||
| @Bean | ||
| public CircuitBreakerRegistry circuitBreakerRegistry() { | ||
| CircuitBreakerConfig fcmCbConfig = CircuitBreakerConfig | ||
| .custom() | ||
| .failureRateThreshold(50) // 실패율 50% 이상 시 서킷 오픈 | ||
| .slowCallRateThreshold(100) | ||
| .slowCallDurationThreshold(Duration.ofSeconds(2)) // 2초 이상 걸리면 느린 호출로 간주 | ||
| .permittedNumberOfCallsInHalfOpenState(3) | ||
| .maxWaitDurationInHalfOpenState(Duration.ofSeconds(10)) | ||
| .slidingWindowSize(10) // 최근 10개의 호출을 통계로 사용 | ||
| .minimumNumberOfCalls(5) // 최소 5번은 호출된 후 통계 계산 | ||
| .waitDurationInOpenState(Duration.ofSeconds(30)) // 서킷 오픈 후 30초 대기 | ||
| // 서킷 브레이커가 전파받을 예외 설정 (Retry에서 걸러진 것들이 여기까지 옴) | ||
| .recordExceptions(FcmRetryableException.class, IOException.class) | ||
| .ignoreExceptions(FcmInvalidTokenException.class) | ||
| .build(); | ||
|
|
||
| return CircuitBreakerRegistry.of(Map.of("fcm", fcmCbConfig)); | ||
| } | ||
| } |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
103 changes: 103 additions & 0 deletions
103
src/main/java/book/book/notification/domain/NotificationDeviceLog.java
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,103 @@ | ||
| package book.book.notification.domain; | ||
|
|
||
| import book.book.common.BaseTimeEntity; | ||
| import jakarta.persistence.Column; | ||
| import jakarta.persistence.Entity; | ||
| import jakarta.persistence.EnumType; | ||
| import jakarta.persistence.Enumerated; | ||
| import jakarta.persistence.FetchType; | ||
| import jakarta.persistence.GeneratedValue; | ||
| import jakarta.persistence.GenerationType; | ||
| import jakarta.persistence.Id; | ||
| import jakarta.persistence.JoinColumn; | ||
| import jakarta.persistence.ManyToOne; | ||
| import jakarta.persistence.Table; | ||
| import lombok.AccessLevel; | ||
| import lombok.Builder; | ||
| import lombok.Getter; | ||
| import lombok.NoArgsConstructor; | ||
|
|
||
| @Entity | ||
| @Table(name = "notification_device_log") | ||
| @Getter | ||
| @NoArgsConstructor(access = AccessLevel.PROTECTED) | ||
| public class NotificationDeviceLog extends BaseTimeEntity { | ||
|
|
||
| @Id | ||
| @GeneratedValue(strategy = GenerationType.IDENTITY) | ||
| private Long id; | ||
|
|
||
| @ManyToOne(fetch = FetchType.LAZY) | ||
| @JoinColumn(name = "notification_id", nullable = false) | ||
| private Notification notification; | ||
|
|
||
| /** | ||
| * 알림 발송 대상 FCM 토큰의 ID (토큰 삭제 시 추적을 위해 저장, FK 제약조건은 없을 수 있음) | ||
| */ | ||
| @Column(nullable = false, unique = true) | ||
| private Long fcmTokenId; | ||
|
|
||
| /** | ||
| * 기기 타입 (예: IOS, ANDROID, WEB) | ||
| * 토큰이 삭제되어도 어떤 기기로 보냈는지 알 수 있도록 저장 | ||
| */ | ||
| @Column(nullable = false) | ||
| private String deviceType; | ||
|
|
||
| @Enumerated(EnumType.STRING) | ||
| @Column(nullable = false) | ||
| private FCMSendStatus status; | ||
|
|
||
| @Column(columnDefinition = "TEXT") | ||
| private String errorCode; | ||
|
|
||
| @Column(nullable = false) | ||
| private int retryCount = 0; | ||
|
|
||
| @Builder | ||
| public NotificationDeviceLog(Notification notification, Long fcmTokenId, String deviceType, FCMSendStatus status, | ||
| String errorCode) { | ||
| this.notification = notification; | ||
| this.fcmTokenId = fcmTokenId; | ||
| this.deviceType = deviceType; | ||
| this.status = status; | ||
| this.errorCode = errorCode; | ||
| } | ||
|
|
||
| public void updateStatus(FCMSendStatus status, String errorCode) { | ||
| this.status = status; | ||
| this.errorCode = errorCode; | ||
| } | ||
|
|
||
| public void incrementRetryCount() { | ||
| this.retryCount++; | ||
| } | ||
|
|
||
| public void succeed() { | ||
| this.status = FCMSendStatus.SENT; | ||
| this.errorCode = null; | ||
| } | ||
|
|
||
| public void fail(String errorCode) { | ||
| this.status = FCMSendStatus.FAILED; | ||
| this.errorCode = errorCode; | ||
| } | ||
|
|
||
| public void failNoRetry(String errorCode) { | ||
| this.status = FCMSendStatus.FAILED_NO_RETRY; | ||
| this.errorCode = errorCode; | ||
| } | ||
|
|
||
| public void failRetryLimitExceeded() { | ||
| this.status = FCMSendStatus.RETRY_LIMIT_EXCEEDED; | ||
| } | ||
|
|
||
| public static NotificationDeviceLog of(Notification notification, FCMToken token) { | ||
| return NotificationDeviceLog.builder() | ||
| .notification(notification) | ||
| .fcmTokenId(token.getId()) | ||
| .deviceType(token.getDeviceType().name()) | ||
| .status(FCMSendStatus.PENDING) | ||
| .build(); | ||
| } | ||
| } | ||
14 changes: 14 additions & 0 deletions
14
src/main/java/book/book/notification/dto/FcmSendResult.java
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,14 @@ | ||
| package book.book.notification.dto; | ||
|
|
||
| public record FcmSendResult( | ||
| int successCount, | ||
| int totalCount, | ||
| String errorLog) { | ||
| public boolean isAnySuccess() { | ||
| return successCount > 0; | ||
| } | ||
|
|
||
| public boolean isAllFailed() { | ||
| return totalCount > 0 && successCount == 0; | ||
| } | ||
| } | ||
|
||
46 changes: 46 additions & 0 deletions
46
src/main/java/book/book/notification/dto/NotificationDto.java
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,46 @@ | ||
| package book.book.notification.dto; | ||
|
|
||
| import book.book.notification.domain.NotificationType; | ||
| import lombok.AllArgsConstructor; | ||
| import lombok.Builder; | ||
| import lombok.Data; | ||
| import lombok.NoArgsConstructor; | ||
|
|
||
| /** | ||
| * 가변적이지 않고 중간에 필드값이 추가됨 | ||
| */ | ||
| @Data | ||
| @AllArgsConstructor | ||
| @NoArgsConstructor | ||
| @Builder(toBuilder = true) | ||
| public class NotificationDto { | ||
| private Long notificationId; // DB에 저장된 알림 ID (FCM 전송용) | ||
| private Long receiverId; | ||
| private Long actorId; // 알림을 발생시킨 사용자 ID (시스템 알림의 경우 null) | ||
| private String title; | ||
| private String content; | ||
| private NotificationType notificationType; | ||
|
|
||
| /** | ||
| * 알림 관련 리소스 정보 (프론트엔드 라우팅용) | ||
| * 예시: | ||
| * - 댓글: {"commentId": 456, "diaryId": 123} | ||
| * - 좋아요: {"diaryId": 123} | ||
| * - 퀴즈 완성: {"quizId": 789, "bookId": 321} | ||
| */ | ||
| private String metadata; // JSON String | ||
|
|
||
| // 내부 로직용 필드 (DB 저장 및 전송 시 활용) | ||
| private Long notificationDeviceLogId; // 전송 로그 ID (상태 업데이트용) | ||
| private Long fcmTokenId; // FCM 토큰 ID (전송 대상 조회용) | ||
| private String fcmToken; // FCM 토큰 값 (실제 전송용) | ||
|
|
||
| public NotificationDto withDetails(Long notificationId, Long deviceLogId, Long tokenId, String tokenValue) { | ||
| return this.toBuilder() | ||
| .notificationId(notificationId) | ||
| .notificationDeviceLogId(deviceLogId) | ||
| .fcmTokenId(tokenId) | ||
| .fcmToken(tokenValue) | ||
| .build(); | ||
| } | ||
| } |
Oops, something went wrong.
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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.
fcmTokenId필드에 설정된unique = true제약 조건은 잘못된 것으로 보입니다. 이 제약 조건은 각 FCM 토큰이notification_device_log테이블에 단 하나의 항목만 가질 수 있음을 의미합니다. 하지만, 단일 기기(즉, 단일 FCM 토큰)는 시간이 지남에 따라 여러 알림을 수신할 수 있습니다. 만약 사용자가 두 개의 다른 알림을 받게 되면, 동일한 기기에 대한 두 번째 알림 로그를 저장하려 할 때DataIntegrityViolationException이 발생할 것입니다.고유 제약 조건은 특정 알림 이벤트가 특정 기기에 한 번만 전송되도록 보장하기 위해
notification_id와fcm_token_id의 조합에 대해 설정되어야 합니다. 다음과 같이 복합 고유 제약 조건으로 변경하는 것을 고려해 보세요.그리고
@Column(name = "fcmTokenId")에서unique=true를 제거해야 합니다.