-
Notifications
You must be signed in to change notification settings - Fork 1
[FEAT] 비활성화 토큰삭제 배치 처리 구현 #298
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
Conversation
Walkthrough비활성 알림 토큰을 주기적으로 정리하는 배치 기능을 추가합니다. Spring Retry 활성화, 주간 스케줄러(매주 토요일 03:00)에서 90일 이상 비활성 토큰 삭제, 실패 시 Slack 알림 전송 및 재시도/복구 로직을 도입합니다. Changes
Sequence DiagramsequenceDiagram
participant Scheduler as PushTokenCleanScheduler
participant Service as NotificationTokenService
participant Repo as MemberPushTokenRepository
participant DB as Database
participant SlackSvc as SlackNotificationService
participant SlackAPI as Slack API
Scheduler->>Service: deleteInactiveTokens() 트리거 (매주 토요일 03:00)
Note over Scheduler: limitDate = now() - 90일
Service->>Repo: deleteByLastActiveAtBefore(limitDate)
Repo->>DB: DELETE FROM ... WHERE last_active_at < limitDate
DB-->>Repo: 삭제된 행 수 반환
Repo-->>Service: 삭제된 수 반환
Service-->>Scheduler: 결과 반환 및 로그
alt 예외 발생
Scheduler-->>Scheduler: Retry (최대 3회, 2초 백오프)
Note over Scheduler: 재시도 모두 실패 시
Scheduler->>Scheduler: recoverDeleteInactiveTokens(Exception)
Scheduler->>SlackSvc: sendSchedulerFailureMessage(schedulerName, truncatedError)
SlackSvc->>SlackAPI: POST Block Kit JSON to channel
SlackAPI-->>SlackSvc: 응답
end
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 3 | ❌ 2❌ Failed checks (2 warnings)
✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
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: 2
🤖 Fix all issues with AI agents
In
`@eeos/src/main/java/com/blackcompany/eeos/notification/application/scheduler/PushTokenCleanScheduler.java`:
- Around line 31-42: The scheduled method deleteInactiveTokens in
PushTokenCleanScheduler mixes `@Retryable` and `@Transactional` which can produce
incorrect proxy ordering; refactor so retry logic and transactional work are on
different beans or methods: keep `@Scheduled` and `@Retryable` on the scheduler
method and move the DB operation (call to
memberPushTokenRepository.deleteByLastActiveAtBefore(limitDate)) into a separate
`@Service` method (e.g.,
PushTokenCleanupService.deleteInactiveTokensWithinTransaction) annotated with
`@Transactional` (or use `@Transactional`(propagation = Propagation.REQUIRES_NEW) on
that service method if you must keep one bean); update the scheduler to call
that service method and add tests to verify each retry creates an independent
transaction.
In
`@eeos/src/main/java/com/blackcompany/eeos/notification/persistence/MemberPushTokenEntity.java`:
- Around line 27-30: Add a CREATE TABLE definition for the "Notification_token"
table to the DB migration file V1.00.0.0__init_tables.sql so the schema matches
the MemberPushTokenEntity declaration (`@Table`(name = "Notification_token")). The
migration should create the table using the same column names, types, primary
key and not-null constraints for all fields declared on MemberPushTokenEntity
and any inherited columns from BaseEntity (e.g., id, created_at/updated_at or
audit columns), plus any foreign key (e.g., member_id) and indexes used by the
entity; ensure the table name matches ENTITY_PREFIX ("Notification_token")
exactly and commit the updated migration so ddl-auto: validate will succeed in
production.
🧹 Nitpick comments (2)
eeos/src/main/resources/application-slack.yml (1)
16-17: YAML 포맷팅 일관성기능적으로는 문제없지만, 콜론 앞에 공백이 있는 것(
notification :,error-report :)은 기존 라인들(slack-message-test:)과 일관성이 없습니다. YAML 표준에서는 콜론 앞에 공백이 없는 것이 일반적입니다.♻️ 포맷팅 수정 제안
- notification : ${BLACK_COMPANY_NOTIFICATION_CHANNEL_ID} - error-report : ${BLACK_COMPANY_ERROR_REPORT_CHANNEL_ID} + notification: ${BLACK_COMPANY_NOTIFICATION_CHANNEL_ID} + error-report: ${BLACK_COMPANY_ERROR_REPORT_CHANNEL_ID}eeos/src/main/java/com/blackcompany/eeos/notification/persistence/JpaMemberPushTokenRepository.java (1)
32-34:@Modifying어노테이션에clearAutomatically = true추가를 권장합니다.벌크 DELETE 쿼리 실행 후 영속성 컨텍스트와 DB 간 불일치가 발생할 수 있습니다. 이 스케줄러에서는 삭제 후 추가 작업이 없어 현재로서는 문제가 없지만, 향후 확장성을 고려하면
clearAutomatically = true를 추가하는 것이 안전합니다.♻️ 제안된 수정
- `@Modifying` + `@Modifying`(clearAutomatically = true) `@Query`("DELETE FROM MemberPushTokenEntity t where t.lastActiveAt < :limitDate") int deleteByLastActiveAtBefore(`@Param`("limitDate") LocalDateTime limitDate);
📜 Review details
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (10)
eeos/build.gradle.ktseeos/gradle/libs.versions.tomleeos/src/main/java/com/blackcompany/eeos/config/RetryConfig.javaeeos/src/main/java/com/blackcompany/eeos/notification/application/repository/MemberPushTokenRepository.javaeeos/src/main/java/com/blackcompany/eeos/notification/application/scheduler/PushTokenCleanScheduler.javaeeos/src/main/java/com/blackcompany/eeos/notification/application/service/SlackNotificationService.javaeeos/src/main/java/com/blackcompany/eeos/notification/persistence/JpaMemberPushTokenRepository.javaeeos/src/main/java/com/blackcompany/eeos/notification/persistence/MemberPushTokenEntity.javaeeos/src/main/java/com/blackcompany/eeos/notification/persistence/MemberPushTokenRepositoryImpl.javaeeos/src/main/resources/application-slack.yml
🧰 Additional context used
🧬 Code graph analysis (1)
eeos/src/main/java/com/blackcompany/eeos/notification/application/scheduler/PushTokenCleanScheduler.java (1)
eeos/src/main/java/com/blackcompany/eeos/notification/application/service/SlackNotificationService.java (1)
Slf4j(14-54)
⏰ 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 (11)
eeos/gradle/libs.versions.toml (2)
68-71: LGTM!Spring Retry와 Spring Aspects 라이브러리 추가가 올바르게 구성되었습니다.
spring-aspects는 버전을 명시하지 않아 Spring Boot BOM에서 관리되므로 적절합니다.
25-27: Spring Retry 2.0.5는 Spring Boot 3.2.2와 호환됩니다spring-retry 2.0.5는 Spring Boot 3.2.2의 공식 관리 버전입니다. 해당 버전 선언은 Spring Boot의 dependency management와 일치하므로 추가 조치가 필요하지 않습니다.
eeos/build.gradle.kts (1)
70-74: LGTM!Spring Retry 기능 활성화를 위한 의존성이 올바르게 추가되었습니다.
spring-retry는@Retryable/@recover 어노테이션 지원에 필요하고,spring-aspects는 AOP 기반 재시도 처리에 필요합니다.eeos/src/main/java/com/blackcompany/eeos/config/RetryConfig.java (1)
1-8: LGTM!Spring Retry 기능 활성화를 위한 설정 클래스가 올바르게 구현되었습니다.
@EnableRetry를 통해 프로젝트 전체에서@Retryable과@Recover어노테이션을 사용할 수 있게 됩니다.eeos/src/main/java/com/blackcompany/eeos/notification/persistence/MemberPushTokenRepositoryImpl.java (1)
68-72: LGTM! 비활성 토큰 삭제를 위한 메서드가 기존 패턴과 일관되게 구현되었습니다.검증 결과:
- JpaMemberPushTokenRepository에
@Modifying및@Query를 통해 올바르게 정의됨- MemberPushTokenRepository 인터페이스 계약 준수
- PushTokenCleanScheduler에서 실제로 활용 중
- 구현부가 JPA Repository로 올바르게 위임
eeos/src/main/java/com/blackcompany/eeos/notification/application/repository/MemberPushTokenRepository.java (1)
26-27: LGTM!새로운 메서드가 기존 인터페이스 패턴과 일관성 있게 추가되었습니다.
int반환 타입으로 삭제된 레코드 수를 확인할 수 있어 로깅에 유용합니다.eeos/src/main/java/com/blackcompany/eeos/notification/persistence/MemberPushTokenEntity.java (1)
47-48: 컬럼명과 필드명 간 철자 불일치 확인 필요.컬럼명은
_last_activate_at이고 필드명은lastActiveAt입니다. "activate" vs "active" 철자가 다른데, 이것이 의도된 것인지 확인해 주세요. 일관성을 위해 둘 다 동일한 단어를 사용하는 것이 좋습니다.eeos/src/main/java/com/blackcompany/eeos/notification/application/service/SlackNotificationService.java (2)
28-39: LGTM!스케줄러 실패 메시지 포맷이 명확하고, 필요한 정보(스케줄러명, 실패 시각, 에러 메시지)를 잘 포함하고 있습니다.
41-53: 알림 실패 시 예외를 삼키는 것은 적절합니다.Slack 알림 전송 실패가 메인 배치 프로세스에 영향을 주지 않도록 예외를 로깅만 하고 전파하지 않는 것은 올바른 설계입니다. 다만, 운영 환경에서 Slack 알림 실패가 반복될 경우 이를 감지할 수 있도록 메트릭 수집을 고려해 볼 수 있습니다.
eeos/src/main/java/com/blackcompany/eeos/notification/application/scheduler/PushTokenCleanScheduler.java (2)
44-54:@Recover메서드 구현이 적절합니다.에러 메시지 null 체크와 길이 제한(300자) 처리가 잘 되어 있습니다. 한 가지 개선 제안: 예외 타입에 따른 분기 처리가 필요하다면
@Recover메서드를 오버로딩할 수 있습니다.
23-24: 상수 정의가 명확합니다.비활성 기준일(90일)과 스케줄러 이름이 상수로 잘 정의되어 있어 유지보수성이 좋습니다. 향후 이 값들을 외부 설정으로 분리하면 재배포 없이 조정할 수 있습니다.
✏️ Tip: You can disable this entire section by setting review_details to false in your review settings.
...n/java/com/blackcompany/eeos/notification/application/scheduler/PushTokenCleanScheduler.java
Show resolved
Hide resolved
eeos/src/main/java/com/blackcompany/eeos/notification/persistence/MemberPushTokenEntity.java
Show resolved
Hide resolved
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.
고생하셨습니다~ Ci 통과하도록 수정도 부탁드립니다
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: 1
🤖 Fix all issues with AI agents
In
`@eeos/src/main/java/com/blackcompany/eeos/notification/application/scheduler/PushTokenCleanScheduler.java`:
- Line 21: PushTokenCleanScheduler contains an unused constant
INACTIVE_DAYS_THRESHOLD — remove the declaration of INACTIVE_DAYS_THRESHOLD from
PushTokenCleanScheduler and rely on the existing deletion logic in
NotificationTokenService.deleteInactiveTokens(); ensure no other code in
PushTokenCleanScheduler references INACTIVE_DAYS_THRESHOLD and remove any
now-unneeded imports or comments related to that constant.
🧹 Nitpick comments (1)
eeos/src/main/java/com/blackcompany/eeos/notification/application/service/NotificationTokenService.java (1)
27-27: 상수 중복 정의 확인 필요
INACTIVE_DAYS_THRESHOLD가PushTokenCleanScheduler에도 동일하게 정의되어 있습니다. 한 곳에서만 정의하고 다른 곳에서 참조하는 방식으로 변경하면 유지보수성이 향상됩니다.
| private final SlackNotificationService slackNotificationService; | ||
| private final NotificationTokenService notificationTokenService; | ||
|
|
||
| private static final int INACTIVE_DAYS_THRESHOLD = 90; |
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.
사용되지 않는 상수 제거
INACTIVE_DAYS_THRESHOLD 상수가 이 클래스에서 사용되지 않습니다. 실제 삭제 로직은 NotificationTokenService.deleteInactiveTokens()에서 자체 상수를 사용하고 있습니다.
🔧 제안된 수정
- private static final int INACTIVE_DAYS_THRESHOLD = 90;
private static final String SCHEDULER_NAME = "비활성화 토큰 삭제 스케줄러";🤖 Prompt for AI Agents
In
`@eeos/src/main/java/com/blackcompany/eeos/notification/application/scheduler/PushTokenCleanScheduler.java`
at line 21, PushTokenCleanScheduler contains an unused constant
INACTIVE_DAYS_THRESHOLD — remove the declaration of INACTIVE_DAYS_THRESHOLD from
PushTokenCleanScheduler and rely on the existing deletion logic in
NotificationTokenService.deleteInactiveTokens(); ensure no other code in
PushTokenCleanScheduler references INACTIVE_DAYS_THRESHOLD and remove any
now-unneeded imports or comments related to that constant.
📌 관련 이슈
closes #297
✒️ 작업 내용
💬 REVIEWER에게 요구사항 💬
비활성화 토큰을 주기적으로 삭제하는 스케줄링 작업에 대한 pr 입니다!
mysql 재시작 관련 docker-compose 파일 수정은 다음 pr(알림전송) 에서 함께 업로드할 예정입니다.
Summary by CodeRabbit
릴리스 노트
새로운 기능
기능 개선
✏️ Tip: You can customize this high-level summary in your review settings.