-
Notifications
You must be signed in to change notification settings - Fork 1
feat: 알림기능을 개발합니다. #123
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
feat: 알림기능을 개발합니다. #123
Conversation
Walkthrough배치 기반 FCM 알림 기능과 사용자 알림 관련 필드·API가 추가됩니다. Firebase 설정·전송 서비스·주기적 알림 스케줄러, 도메인/인프라/리포지토리 확장 및 관련 빌드·설정 변경이 포함됩니다. Changes
시퀀스 다이어그램sequenceDiagram
autonumber
participant Scheduler as 배치 스케줄러
participant Job as FcmNotificationJobConfig
participant UserRepo as UserRepository
participant NotifRepo as NotificationRepository
participant FcmSvc as FcmService
participant Firebase as Firebase Admin
Note over Scheduler,Job: 주기적 실행(60s)
Scheduler->>Job: checkAndSendNotifications()
Job->>UserRepo: findByLastActivityBeforeAndNotificationEnabled(... 7일/30일 ...)
UserRepo-->>Job: 사용자 목록
loop 각 사용자 처리
Job->>NotifRepo: existing = findByUser(user)
alt fcmToken 존재
Job->>FcmSvc: sendMulticastNotification(tokens, title, body)
FcmSvc->>Firebase: 전송 요청
Firebase-->>FcmSvc: 전송 결과(메시지ID들)
FcmSvc-->>Job: 성공 ID 목록
Job->>NotifRepo: save(notification isSent=true, sentAt=now)
else 토큰 없음
Job->>NotifRepo: save(notification isSent=false)
end
end
Note over Job,NotifRepo: 전송 후 활성 사용자 리셋
Job->>NotifRepo: findBySent(true)
loop 발송된 알림 순회
alt user.lastActivity > sentAt
Job->>NotifRepo: update isSent=false, sentAt=null
end
end
예상 코드 리뷰 난이도🎯 4 (Complex) | ⏱️ ~50분 Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
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: 21
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (3)
batch/src/main/resources/application.yml (1)
36-62: 배치 모듈에서 Swagger 설정을 제거하세요.검증 결과, 배치 모듈의 Swagger 설정은 불필요하고 잘못된 구성입니다:
- REST 컨트롤러 없음: 배치 모듈에 REST API가 존재하지 않음 (FCM 배치 작업만 존재)
- springdoc 의존성 없음: batch/build.gradle.kts에 springdoc 라이브러리가 선언되어 있지 않음
- 잘못된 구성: app.module-name이 "apis"로 설정되어 있고, swagger 타이틀이 "YAPP API (Dev)"로 되어 있음 (apis 모듈 구성이 그대로 복사된 상태)
batch/src/main/resources/application.yml의 lines 36-62에서 springdoc 및 swagger 설정을 모두 제거하세요.domain/src/main/kotlin/org/yapp/domain/user/User.kt (2)
71-88: 팩토리 메서드 업데이트가 완료되었으나 시간 처리 개선이 필요합니다.새로운 필드들이 팩토리 메서드에 적절히 추가되었습니다. 하지만 Line 87의
LocalDateTime.now()사용은 앞서 지적한 것과 동일한 문제를 가지고 있습니다.Line 87의
lastActivity = LocalDateTime.now()를 다음과 같이 개선하세요:fun create( email: String, nickname: String, profileImageUrl: String?, providerType: ProviderType, providerId: String, termsAgreed: Boolean = false, fcmToken: String? = null, - notificationEnabled: Boolean = true + notificationEnabled: Boolean = true, + lastActivity: LocalDateTime = LocalDateTime.now() ): User { return User( id = Id.newInstance(UuidGenerator.create()), email = Email.newInstance(email), nickname = nickname, profileImageUrl = profileImageUrl, providerType = providerType, providerId = ProviderId.newInstance(providerId), role = Role.USER, termsAgreed = termsAgreed, appleRefreshToken = null, fcmToken = fcmToken, notificationEnabled = notificationEnabled, - lastActivity = LocalDateTime.now() + lastActivity = lastActivity ) }이렇게 하면 테스트 시 시간을 명시적으로 주입할 수 있으며, 기본값으로는 여전히 현재 시간을 사용합니다.
99-116:createWithRole메서드도 동일한 시간 처리 개선이 필요합니다.
create()메서드와 동일한 패턴이 적용되었으나, Line 115에서도LocalDateTime.now()를 직접 사용하는 문제가 있습니다.
create()메서드와 일관되게 개선하세요:fun createWithRole( email: String, nickname: String, profileImageUrl: String?, providerType: ProviderType, providerId: String, role: Role, termsAgreed: Boolean = false, fcmToken: String? = null, - notificationEnabled: Boolean = true + notificationEnabled: Boolean = true, + lastActivity: LocalDateTime = LocalDateTime.now() ): User { return User( id = Id.newInstance(UuidGenerator.create()), email = Email.newInstance(email), nickname = nickname, profileImageUrl = profileImageUrl, providerType = providerType, providerId = ProviderId.newInstance(providerId), role = role, termsAgreed = termsAgreed, appleRefreshToken = null, fcmToken = fcmToken, notificationEnabled = notificationEnabled, - lastActivity = LocalDateTime.now() + lastActivity = lastActivity ) }
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (39)
.gitignore(1 hunks)apis/src/main/kotlin/org/yapp/apis/book/service/UserBookService.kt(2 hunks)apis/src/main/kotlin/org/yapp/apis/readingrecord/service/ReadingRecordService.kt(4 hunks)apis/src/main/kotlin/org/yapp/apis/readingrecord/usecase/ReadingRecordUseCase.kt(1 hunks)apis/src/main/kotlin/org/yapp/apis/user/controller/UserController.kt(2 hunks)apis/src/main/kotlin/org/yapp/apis/user/controller/UserControllerApi.kt(2 hunks)apis/src/main/kotlin/org/yapp/apis/user/dto/request/FcmTokenRequest.kt(1 hunks)apis/src/main/kotlin/org/yapp/apis/user/dto/request/NotificationSettingsRequest.kt(1 hunks)apis/src/main/kotlin/org/yapp/apis/user/dto/response/UserProfileResponse.kt(2 hunks)apis/src/main/kotlin/org/yapp/apis/user/service/UserService.kt(2 hunks)apis/src/main/kotlin/org/yapp/apis/user/usecase/UserUseCase.kt(2 hunks)batch/build.gradle.kts(1 hunks)batch/src/main/kotlin/org/yapp/batch/BatchApplication.kt(1 hunks)batch/src/main/kotlin/org/yapp/batch/config/FcmConfig.kt(1 hunks)batch/src/main/kotlin/org/yapp/batch/config/InfraConfig.kt(1 hunks)batch/src/main/kotlin/org/yapp/batch/job/fcm/FcmNotificationJobConfig.kt(1 hunks)batch/src/main/kotlin/org/yapp/batch/service/FcmService.kt(1 hunks)batch/src/main/resources/application.yml(3 hunks)buildSrc/src/main/kotlin/Dependencies.kt(1 hunks)domain/src/main/kotlin/org/yapp/domain/notification/Notification.kt(1 hunks)domain/src/main/kotlin/org/yapp/domain/notification/NotificationRepository.kt(1 hunks)domain/src/main/kotlin/org/yapp/domain/notification/NotificationType.kt(1 hunks)domain/src/main/kotlin/org/yapp/domain/readingrecord/ReadingRecordRepository.kt(2 hunks)domain/src/main/kotlin/org/yapp/domain/user/User.kt(7 hunks)domain/src/main/kotlin/org/yapp/domain/user/UserDomainService.kt(2 hunks)domain/src/main/kotlin/org/yapp/domain/user/UserRepository.kt(2 hunks)domain/src/main/kotlin/org/yapp/domain/user/vo/UserProfileVO.kt(2 hunks)domain/src/main/kotlin/org/yapp/domain/userbook/UserBookRepository.kt(1 hunks)infra/src/main/kotlin/org/yapp/infra/notification/entity/NotificationEntity.kt(1 hunks)infra/src/main/kotlin/org/yapp/infra/notification/repository/JpaNotificationRepository.kt(1 hunks)infra/src/main/kotlin/org/yapp/infra/notification/repository/impl/NotificationRepositoryImpl.kt(1 hunks)infra/src/main/kotlin/org/yapp/infra/readingrecord/repository/JpaReadingRecordRepository.kt(2 hunks)infra/src/main/kotlin/org/yapp/infra/readingrecord/repository/impl/ReadingRecordRepositoryImpl.kt(2 hunks)infra/src/main/kotlin/org/yapp/infra/user/entity/UserEntity.kt(4 hunks)infra/src/main/kotlin/org/yapp/infra/user/repository/JpaUserRepository.kt(2 hunks)infra/src/main/kotlin/org/yapp/infra/user/repository/impl/UserRepositoryImpl.kt(2 hunks)infra/src/main/kotlin/org/yapp/infra/userbook/repository/JpaUserBookRepository.kt(2 hunks)infra/src/main/kotlin/org/yapp/infra/userbook/repository/impl/UserBookRepositoryImpl.kt(1 hunks)infra/src/main/resources/application-persistence.yml(1 hunks)
🧰 Additional context used
🪛 detekt (1.23.8)
batch/src/main/kotlin/org/yapp/batch/config/FcmConfig.kt
[warning] 84-84: RuntimeException is a too generic Exception. Prefer throwing specific exceptions that indicate a specific error case.
(detekt.exceptions.TooGenericExceptionThrown)
[warning] 16-17: Usages of lateinit should be avoided.
(detekt.potential-bugs.LateinitUsage)
[warning] 19-20: Usages of lateinit should be avoided.
(detekt.potential-bugs.LateinitUsage)
[warning] 22-23: Usages of lateinit should be avoided.
(detekt.potential-bugs.LateinitUsage)
[warning] 25-26: Usages of lateinit should be avoided.
(detekt.potential-bugs.LateinitUsage)
[warning] 28-29: Usages of lateinit should be avoided.
(detekt.potential-bugs.LateinitUsage)
[warning] 31-32: Usages of lateinit should be avoided.
(detekt.potential-bugs.LateinitUsage)
[warning] 34-35: Usages of lateinit should be avoided.
(detekt.potential-bugs.LateinitUsage)
[warning] 37-38: Usages of lateinit should be avoided.
(detekt.potential-bugs.LateinitUsage)
[warning] 40-41: Usages of lateinit should be avoided.
(detekt.potential-bugs.LateinitUsage)
[warning] 43-44: Usages of lateinit should be avoided.
(detekt.potential-bugs.LateinitUsage)
[warning] 46-47: Usages of lateinit should be avoided.
(detekt.potential-bugs.LateinitUsage)
batch/src/main/kotlin/org/yapp/batch/job/fcm/FcmNotificationJobConfig.kt
[warning] 28-28: The function checkAndSendNotifications appears to be too complex based on Cognitive Complexity (complexity: 34). Defined complexity threshold for methods is set to '25'
(detekt.complexity.CognitiveComplexMethod)
[warning] 218-218: The caught exception is too generic. Prefer catching specific exceptions to the case that is currently handled.
(detekt.exceptions.TooGenericExceptionCaught)
⏰ 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: build-validation
🔇 Additional comments (42)
infra/src/main/resources/application-persistence.yml (1)
32-41: Flyway와 Hibernate의ddl-auto: update동시 활성화에 대한 검증 필요dev 프로파일에서
ddl-auto: update로 변경되었으나, 동일 프로파일에서 Flyway도 활성화되어 있습니다(line 37-41). 이는 두 가지 스키마 관리 메커니즘이 동시에 작동하게 되어 다음과 같은 문제를 야기할 수 있습니다:
- 스키마 불일치: Flyway 마이그레이션 버전 추적과 Hibernate의 자동 스키마 수정이 동기화되지 않을 수 있습니다.
- 검증 오류:
validate-on-migrate: true(기본 프로파일)가 상속될 경우, Flyway는 Hibernate가 생성한 스키마와 마이그레이션 파일을 비교할 때 불일치를 감지할 수 있습니다.모범 사례: 프로덕션과 개발 환경 간 일관성을 위해 Flyway만 사용하고,
ddl-auto: none으로 유지하는 것이 권장됩니다.PR에서 추가된 Flyway 마이그레이션 파일들이 모든 스키마 변경(새로운 엔티티, User 필드 추가 등)을 완전히 포함하는지 확인하고, 다음 중 하나를 선택하시기 바랍니다:
- 권장:
ddl-auto: none으로 유지하고 Flyway만 사용- 대안: 개발 편의성을 위해 현재 설정 유지 시, 로컬 개발 환경에서만 사용하고 CI/테스트 환경에서는 Flyway 검증을 엄격하게 실행
batch/src/main/kotlin/org/yapp/batch/config/InfraConfig.kt (2)
4-5: 필요한 import 추가가 올바르게 적용되었습니다.새로운 인프라 설정 어노테이션을 사용하기 위한 import 문이 정확하게 추가되었습니다.
8-12: 원본 리뷰 의견이 부정확합니다.InfraConfig.kt의 현재 설정은 올바릅니다. 검증 결과 FCM은
@EnableInfraBaseConfig를 통해 구성되지 않으며, 대신batch/src/main/kotlin/org/yapp/batch/config/FcmConfig.kt에서 별도의@Configuration클래스로 독립적으로 처리됩니다.
FcmConfig.kt는 FirebaseApp 빈을 별도로 초기화하는 독립적인 구성 클래스입니다@EnableInfraBaseConfig는InfraBaseConfigGroup항목들만 제어하며, FCM은 이 메커니즘을 사용하지 않습니다InfraBaseConfigGroup.JPA만 활성화하는 것이 올바릅니다따라서 InfraConfig.kt의 코드를 그대로 유지하세요.
Likely an incorrect or invalid review comment.
batch/build.gradle.kts (1)
11-16: 배치 모듈 의존성 추가가 적절합니다.Firebase Admin SDK와 Spring Data JPA 의존성 추가가 알림 기능 요구사항과 잘 맞습니다:
- JPA: 사용자 및 알림 데이터 조회
- Firebase Admin: FCM 푸시 알림 전송
domain/src/main/kotlin/org/yapp/domain/userbook/UserBookRepository.kt (1)
36-43: 특정 시점 이후 등록된 책 조회 메서드가 적절하게 추가되었습니다.메서드 시그니처와 문서화가 명확하며, 30일간 책 등록이 없는 사용자를 식별하는 UNRECORDED 알림 기능의 요구사항과 일치합니다.
apis/src/main/kotlin/org/yapp/apis/user/dto/request/NotificationSettingsRequest.kt (1)
10-10: 추가 검증을 위해 NotificationSettingsRequest 파일의 전체 구조를 확인하겠습니다.검증 완료: 원본 리뷰 의견은 근거 없음
Kotlin의 공식 라이브러리들은 대부분 public이 아닌 생성자만 노출하고 있으며, 팩토리 함수를 통해 객체를 생성합니다. 제시된 스크립트 결과에서 확인된 모든 Request DTO들이 private 생성자를 사용하는 것은 일관된 아키텍처 패턴이며, Kotlin의 companion object를 통해 팩토리 메서드를 정의하고 클래스 수준의 함수와 프로퍼티를 생성하는 방식입니다.
이는 DTOs의 일반적인 관례를 벗어난 것이 아니라, Kotlin 커뮤니티의 권장 패턴이며 코드베이스에서 일관되게 적용된 설계입니다.
Likely an incorrect or invalid review comment.
apis/src/main/kotlin/org/yapp/apis/readingrecord/usecase/ReadingRecordUseCase.kt (1)
88-92: userId 파라미터 전달 및 사용자 활동 추적이 일관되게 구현됨을 확인했습니다.검증 결과:
- ✅
createReadingRecord: 라인 33에서userDomainService.updateLastActivity(userId)호출- ✅
updateReadingRecord: 라인 72에서userDomainService.updateLastActivity(userId)호출- ✅ 책 등록(
UserBookService): 라인 38에서userDomainService.updateLastActivity(userId)호출모든 사용자 활동 지점에서 일관성 있게
updateLastActivity가 호출되고 있으며, userId 파라미터가 컨트롤러부터 서비스까지 적절하게 전달되고 있습니다. PR의 비활성 사용자 알림 기능 목표와도 일치합니다.domain/src/main/kotlin/org/yapp/domain/user/UserRepository.kt (1)
24-31: LGTM!비활성 사용자를 조회하기 위한 메서드가 명확한 문서와 함께 잘 정의되어 있습니다. 메서드 시그니처와 KDoc이 PR 목표와 일치합니다.
batch/src/main/kotlin/org/yapp/batch/BatchApplication.kt (1)
6-13: 컴포넌트 스캔 범위 확장이 적절합니다.배치 모듈이 infra, domain, globalutils의 빈을 사용할 수 있도록 컴포넌트 스캔 범위가 확장되었습니다. 멀티 모듈 Spring Boot 애플리케이션에서 일반적인 패턴이며 알림 기능 구현에 필요합니다.
infra/src/main/kotlin/org/yapp/infra/userbook/repository/impl/UserBookRepositoryImpl.kt (1)
89-91: LGTM!JPA 레포지토리에 위임하고 도메인 객체로 매핑하는 구현이 기존 패턴과 일관성 있게 작성되었습니다. 30일 이내 책 등록 여부 확인 요구사항을 지원합니다.
infra/src/main/kotlin/org/yapp/infra/userbook/repository/JpaUserBookRepository.kt (1)
14-14: LGTM!Spring Data JPA의 derived query method 명명 규칙을 올바르게 따르고 있으며,
userId와createdAt조건으로 정확하게 조회됩니다.domain/src/main/kotlin/org/yapp/domain/readingrecord/ReadingRecordRepository.kt (1)
38-45: LGTM!독서 기록 조회를 위한 메서드가 명확한 문서와 함께 잘 정의되어 있습니다. 여러 userBookId에 대한 IN 절과 생성 시간 필터링을 통해 30일 이내 기록 여부 확인 요구사항을 지원합니다.
infra/src/main/kotlin/org/yapp/infra/readingrecord/repository/JpaReadingRecordRepository.kt (1)
23-23: LGTM!Spring Data JPA의 derived query method에서
In과After키워드를 올바르게 조합하여 사용했습니다. 여러userBookId와createdAt조건으로 정확하게 조회됩니다.infra/src/main/kotlin/org/yapp/infra/readingrecord/repository/impl/ReadingRecordRepositoryImpl.kt (1)
65-68: LGTM!JPA 레포지토리에 위임하고 도메인 객체로 매핑하는 구현이 기존 패턴과 일관성 있게 작성되었습니다. 시간 기반 독서 기록 필터링을 올바르게 지원합니다.
infra/src/main/kotlin/org/yapp/infra/user/repository/impl/UserRepositoryImpl.kt (1)
54-62: LGTM! 소프트 삭제 필터링이 포함되어 있습니다.JPA 레포지토리 메서드에
AndDeletedAtIsNull조건이 포함되어 삭제된 사용자가 결과에서 올바르게 제외됩니다. 도메인 객체 매핑도 기존 패턴과 일관성 있게 구현되었습니다.domain/src/main/kotlin/org/yapp/domain/user/vo/UserProfileVO.kt (1)
11-12: 추가 필드 매핑 LGTMnotificationEnabled 필드 추가/매핑 일관성 좋습니다. VO 생성 제약과도 충돌 없습니다.
Also applies to: 28-29
apis/src/main/kotlin/org/yapp/apis/user/usecase/UserUseCase.kt (1)
26-34: 신규 유스케이스 메서드 LGTM트랜잭션 스코프와 위임 구조가 일관적입니다.
apis/src/main/kotlin/org/yapp/apis/user/service/UserService.kt (1)
42-52: 업데이트 메서드 위임/검증 흐름 LGTM + DTO 전제 확인흐름(존재 검증 → 도메인 위임 → 응답 매핑)은 적절합니다. 단, FcmTokenRequest의 역직렬화/널 처리 문제가 해결되어야 NPE 리스크가 제거됩니다(별도 코멘트 참조).
apis/src/main/kotlin/org/yapp/apis/readingrecord/service/ReadingRecordService.kt (2)
32-34: 트랜잭션 경계는 이미 보장되어 있습니다.코드베이스를 검증한 결과,
ReadingRecordUseCase의createReadingRecord와updateReadingRecord메서드에 이미@Transactional애너테이션이 적용되어 있습니다(UseCase 클래스 26번, 80번 라인). 이 메서드들은ReadingRecordService의 메서드를 호출하므로, Spring의 트랜잭션 전파 메커니즘에 의해readingRecordDomainService작업과userDomainService.updateLastActivity호출이 동일한 트랜잭션 내에서 실행됩니다. 따라서 부분 커밋으로 인한 일관성 문제는 발생하지 않으며, 현재 구조에서는 Service 레이어에 추가@Transactional애너테이션이 불필요합니다.
58-62: 시그니처 변경이 모든 호출부에 올바르게 반영되었습니다.검증 결과, 모든 호출부가 새로운 시그니처(userId, readingRecordId, request)로 올바르게 업데이트되었습니다:
- ReadingRecordUseCase.kt (88줄): readingRecordService.updateReadingRecord 호출 시 3개 파라미터 모두 전달 ✓
- ReadingRecordController.kt (93줄): readingRecordUseCase.updateReadingRecord 호출 시 3개 파라미터 모두 전달 ✓
infra/src/main/kotlin/org/yapp/infra/user/repository/JpaUserRepository.kt (1)
34-37: 조회 쿼리의 NULL 처리 동작 확인 필요현재 메서드는 Spring Data JPA 파생 쿼리(derived query)로,
lastActivity < 기준일을 필터링합니다. SQL 의미론상NULL < X는UNKNOWN으로 평가되어 NULL 값이 제외됩니다.가입 후 한 번도 활동하지 않은 신규 사용자(lastActivity = NULL)의 취급이 의도된 설계인지 확인이 필요합니다:
- 현재 동작: NULL 제외 (신규 사용자는 휴면 알림 대상 아님)
- 대체 방안: 명시적 JPQL로 OR 조건 포함 시 NULL도 대상 포함
또한 쿼리 성능 개선을 위해 다음 인덱스 생성을 검토하세요:
CREATE INDEX IF NOT EXISTS idx_users_inactive_notification ON users (deleted_at, notification_enabled, last_activity);apis/src/main/kotlin/org/yapp/apis/user/dto/request/FcmTokenRequest.kt (1)
10-19: 요청 DTO 주 생성자 private 선언으로 인한 역직렬화 실패 위험(중요)Spring Boot 3.5.0의 jackson-module-kotlin은 기본적으로 접근 가능한(public) 생성자만 지원합니다. 현재
private constructor는 @RequestBody JSON 바인딩을 차단하여 400/500 오류를 유발할 수 있습니다. 추가로 nullable 필드에서!!단언 사용은 불필요한 NPE 위험을 증가시킵니다.수정 사항:
private constructor→public constructor변경fcmToken: String? = null→fcmToken: String변경fcmToken!!.trim()→fcmToken.trim()변경NotificationSettingsRequest, TermsAgreementRequest 등 다른 요청 DTO도 동일 패턴이므로 함께 검토하고 수정해 주세요.
apis/src/main/kotlin/org/yapp/apis/book/service/UserBookService.kt (1)
21-23: 의존성 주입 추가 LGTMUserDomainService 주입으로 사이드이펙트(최근 활동 업데이트) 연결이 명확해졌습니다.
apis/src/main/kotlin/org/yapp/apis/user/controller/UserController.kt (2)
45-52: 리뷰 코멘트가 부정확합니다. 코드는 이미 모든 권장사항을 구현하고 있습니다.현재 구현 상황:
FcmTokenRequest.kt16줄에@field:NotBlank검증 애노테이션이 이미 적용되어 있습니다.FcmTokenRequest.kt19줄의validFcmToken()메서드에서trim()을 통해 토큰 공백이 이미 제거됩니다.UserService.kt50줄에서request.validFcmToken()을 호출하여 정제된 토큰을 도메인 서비스에 전달합니다.이 리뷰 코멘트에 명시된 개선 사항들은 모두 현재 코드에 반영되어 있습니다.
Likely an incorrect or invalid review comment.
36-43: 알림 설정 업데이트 엔드포인트는 보안 구성이 올바르게 설정되어 있습니다.검증 결과,
jwtAuthenticationConverter에서 JWT 토큰의PRINCIPAL_CLAIM을 추출하여UUID.fromString()으로 변환한 후UsernamePasswordAuthenticationToken의 principal로 설정하고 있습니다. 따라서@AuthenticationPrincipal userId: UUID는 Spring Security 표준 동작에 따라Authentication.getPrincipal()에서 UUID 객체를 자동으로 바인딩하므로 의도대로 동작합니다.입력 검증(
@Valid @RequestBody), 보안 주체 주입(@AuthenticationPrincipal), 반환 정책(200 응답) 모두 적절합니다.domain/src/main/kotlin/org/yapp/domain/notification/NotificationRepository.kt (1)
6-12: 일부 지적은 과도하나, findBySent의 성능 리스크는 실제로 존재합니다검증 결과:
findByUser: 호출처가 없으므로 실제 영향도가 불명확합니다. 도메인상 사용자별 다건이 가능한 구조지만, 이 메서드는 사용되지 않고 있습니다.
findByUserId: 저장소 설계가 적절합니다(batch 작업에서 사용자별 복수 알림 조회).
findBySent: batch/FcmNotificationJobConfig.kt:108에서
findBySent(true)반환값을 foreach로 순회하며 처리하고 있으며, 페이징 없이 전체 데이터를 메모리 로드하는 성능 리스크가 실제로 확인됩니다. 다만 이는 저장소 인터페이스 설계 문제보다는 사용처(batch)에서 페이징이나 스트리밍을 적용할 책임으로 보입니다.findAll: 호출처가 없습니다.
저장소 인터페이스 자체는 필요한 데이터만 반환하는 합리적 설계이므로, 현재 시그니처 변경보다는 batch 코드의 사용 패턴을 개선하는 것이 더 적절합니다.
infra/src/main/kotlin/org/yapp/infra/notification/repository/impl/NotificationRepositoryImpl.kt (2)
23-26: UserEntity 재구성 없이 ID 기반 조회로 단순화 필요도메인 User → UserEntity 변환 없이
findByUserId(user.id.value)를 재사용하면 불필요한 엔티티 생성/동등성 비교를 피하고 JPA 쿼리 의도를 명확히 할 수 있습니다.
[ suggest_recommended_refactor ]
아래처럼 위임하도록 간소화하는 것을 제안합니다.- override fun findByUser(user: User): Notification? { - val userEntity = UserEntity.fromDomain(user) - return jpaNotificationRepository.findByUser(userEntity)?.toDomain() - } + override fun findByUser(user: User): Notification? { + return jpaNotificationRepository.findByUserId(user.id.value).firstOrNull()?.toDomain() + }
12-21: 읽기 전용 트랜잭션 지정으로 불필요한 쓰기 플러시/락 방지
save를 제외한 조회 메서드에@Transactional(readOnly = true)를 지정하면 성능 및 락 측면에서 유리합니다.
[ suggest_recommended_refactor ]import org.springframework.stereotype.Repository +import org.springframework.transaction.annotation.Transactional @@ - override fun findByUser(user: User): Notification? { + @Transactional(readOnly = true) + override fun findByUser(user: User): Notification? { @@ - override fun findByUserId(userId: UUID): List<Notification> { + @Transactional(readOnly = true) + override fun findByUserId(userId: UUID): List<Notification> { @@ - override fun findAll(): List<Notification> { + @Transactional(readOnly = true) + override fun findAll(): List<Notification> { @@ - override fun findBySent(isSent: Boolean): List<Notification> { + @Transactional(readOnly = true) + override fun findBySent(isSent: Boolean): List<Notification> {Also applies to: 23-38
domain/src/main/kotlin/org/yapp/domain/user/UserDomainService.kt (3)
13-13: 시간 소스 주입으로 테스트 용이성과 타임존 일관성 확보
LocalDateTime.now()직접 호출 대신Clock주입 후LocalDateTime.now(clock)을 사용하세요. 테스트 안정성과 타임존 문제를 줄일 수 있습니다.
[ suggest_recommended_refactor ][ request_verification ]
146-152: 알림 OFF 시 처리 정의 필요(FCM 토큰 보존 여부)알림을 비활성화할 때
fcmToken을 유지/삭제 중 어떤 정책인지 명확히 해야 합니다. OFF 상태에서 토큰을 남기면 이후 재활성화 시 편리하지만, 운영상 원치 않는 발송 리스크가 있습니다. 결정된 정책에 맞춰 토큰 정리 로직을 추가해 주세요(예: OFF 시 null).
[ request_verification ][ offer_assistance ]
161-167: FCM 토큰 입력값 정리 및 빈 문자열 처리공백/빈 문자열을 그대로 저장하면 하위 로직에서 혼선이 생깁니다. trim 후 빈 문자열은 null로 정규화하세요. 길이 제한(현재 엔티티 1024)도 검증 권장.
[ suggest_essential_refactor ]- fun updateFcmToken(userId: UUID, fcmToken: String): UserProfileVO { + fun updateFcmToken(userId: UUID, fcmToken: String): UserProfileVO { val user = userRepository.findById(userId) ?: throw UserNotFoundException(UserErrorCode.USER_NOT_FOUND) - - val updatedUser = userRepository.save(user.updateFcmToken(fcmToken)) + val normalized = fcmToken.trim().ifEmpty { null } + // 필요 시 길이 검증: require(normalized == null || normalized.length <= 1024) + val updatedUser = userRepository.save(user.updateFcmToken(normalized)) return UserProfileVO.newInstance(updatedUser) }infra/src/main/kotlin/org/yapp/infra/user/entity/UserEntity.kt (1)
86-89: 도메인 매핑은 일관적이나 타임존 명세 확인 권장
lastActivity: LocalDateTime은 DBtimestamp without time zone과 함께 사용 시 서버 타임존에 의존합니다. 시스템 전반(로그/배치/보고서)에서 UTC 일관성이 필요하다면Instant/OffsetDateTime으로 전환하거나 컬럼 정의를 명시하세요.
[ suggest_recommended_refactor ][ request_verification ]Also applies to: 104-108
domain/src/main/kotlin/org/yapp/domain/notification/Notification.kt (1)
28-47: 생성 시 입력 유효성 검증 추가 권장빈 제목/메시지, 과도하게 긴 텍스트는 즉시 차단하는 편이 안전합니다.
[ suggest_recommended_refactor ]fun create( user: User, title: String, message: String, @@ ): Notification { + require(title.isNotBlank()) { "title must not be blank" } + require(message.isNotBlank()) { "message must not be blank" } return Notification(batch/src/main/kotlin/org/yapp/batch/job/fcm/FcmNotificationJobConfig.kt (7)
26-33: 스케줄 빈도 과도 + lastActivity NULL 사용자 누락 가능성
- 1분 주기는 과도합니다. 일 1회 또는 시간 단위 cron으로 낮추세요(예: 새벽 3시).
findByLastActivityBeforeAndNotificationEnabled는last_activity IS NULL사용자를 제외할 수 있습니다. “활동 이력이 전혀 없는 사용자” 포함이 요구사항이라면 쿼리에서 NULL 포함 조건을 확인해 주세요.
[ offer_operational_advice ][ request_verification ]예시:
- 스케줄:
@Scheduled(cron = "0 0 3 * * *")- 리포지토리:
WHERE (last_activity IS NULL OR last_activity < :threshold) AND notification_enabled = trueAlso applies to: 67-69
26-104: 분산 환경 중복 발송 방지(락/아이템포턴시)멀티 인스턴스에서 동시 실행 시 둘 다 미전송 상태로 판단해 중복 발송할 수 있습니다. ShedLock 등 분산 락을 도입하거나 DB 레벨 유니크/상태 전이를 이용한 아이템포턴시를 구현하세요.
[ offer_operational_advice ]옵션:
- ShedLock: 스케줄 메서드에
@SchedulerLock적용.- DB 제약:
(user_id, notification_type, is_sent=true)유니크 + “전송 중” 상태 추가.
106-134: 리셋 루프의 스캔 범위 축소 필요
findBySent(true)는 전량 스캔입니다. 최근 전송분만 대상으로 제한하거나 리포지토리 레벨에서 조건을 추가하세요(예:sentAt > now - INTERVAL '45 days').
[ suggest_recommended_refactor ][ offer_operational_advice ]예시:
notificationRepository.findBySentSince(true, LocalDateTime.now(clock).minusDays(45))및 해당 인덱스:
CREATE INDEX ON notifications (sent_at) WHERE is_sent = true;
56-59: 하드코딩된 한글 메시지는 i18n 자원으로 분리하세요문자열을
MessageSource/리소스 번들로 옮기면 다국어와 문구 변경이 용이합니다.
[ suggest_recommended_refactor ]Also applies to: 90-93
28-104: 메서드 복잡도 초과(detekt 경고) — 로직 분리 권장
checkAndSendNotifications의 인지 복잡도가 높습니다(경고: 34). UNRECORDED/DORMANT 흐름을 공통 함수로 추출해 간결화하세요.
[ suggest_recommended_refactor ]
218-221: 예외 포착이 지나치게 광범위함(detekt 경고)
Exception전체를 포착하면 장애 원인 구분이 어렵습니다. FCM/IO/DB 등 예상 가능한 예외를 구체적으로 분기하고, 나머지는 상위로 전파하거나 재시도/서킷브레이커를 적용하세요.
[ suggest_recommended_refactor ]
31-33: 인덱스 필요(운영 성능)
users(last_activity, notification_enabled)조건 검색은 인덱스 없으면 테이블 스캔이 발생합니다. 부분 인덱스를 추가하세요.
[ offer_operational_advice ]예시:
CREATE INDEX IF NOT EXISTS idx_users_last_activity_enabled ON users (last_activity) WHERE notification_enabled = true;Also applies to: 67-69
domain/src/main/kotlin/org/yapp/domain/user/User.kt (2)
52-56: LGTM!알림 설정 토글 메서드가 올바르게 구현되었습니다. 간결하고 명확합니다.
129-152: LGTM!
reconstruct메서드가 올바르게 업데이트되었습니다. 새로운 필드들이 적절히 추가되었고,lastActivity를 nullable로 처리하여 기존 데이터베이스 레코드와의 하위 호환성을 유지한 것이 좋습니다. 특히 이 메서드에서는LocalDateTime.now()를 사용하지 않고 파라미터로 받은 값을 그대로 사용하는 것이 올바른 접근입니다.
| # secret | ||
| application-*-secret.properties | ||
| secret/ | ||
| **/reed-firebase-adminsdk.json |
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.
🧹 Nitpick | 🔵 Trivial
Firebase Admin SDK 자격증명 파일 무시 패턴 추가 — 보안 모범 사례입니다.
reed-firebase-adminsdk.json 패턴 추가는 Firebase Admin SDK 서비스 계정 키가 실수로 저장소에 커밋되는 것을 방지하는 중요한 보안 조치입니다. **/ 접두사를 사용하여 모든 디렉터리 레벨에서 파일을 감지하는 것도 적절합니다.
권장사항: 팀의 개발 환경 설정 문서에 Firebase Admin SDK 파일을 로컬에서 올바르게 설정하는 방법(예: 환경 변수 또는 설정 파일 위치)을 명시하여, 개발자들이 이 파일을 .gitignore로 인해 놓치지 않도록 하세요.
🤖 Prompt for AI Agents
In .gitignore around line 178, add the ignore pattern
**/reed-firebase-adminsdk.json to prevent Firebase Admin SDK service account
keys from being committed, and then update the project’s developer setup
documentation to explain how to provide those credentials locally (e.g., using a
specific local file path, environment variables, or a secure secrets manager) so
developers know how to configure their environment despite the file being
ignored.
| // Update user's lastActivity when a book is registered | ||
| userDomainService.updateLastActivity(userId) | ||
|
|
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.
lastActivity 업데이트를 본 흐름과 분리(베스트‑에포트)하세요.
upsert 성공 후 lastActivity 업데이트 실패가 전체 요청 실패로 전파될 수 있습니다(네트워크/DB 일시 오류 등). 예외를 삼켜 로그만 남기거나 비동기/도메인이벤트로 전환을 권장합니다.
적용 예시(간단 베스트‑에포트):
- userDomainService.updateLastActivity(userId)
+ try {
+ userDomainService.updateLastActivity(userId)
+ } catch (ex: Exception) {
+ logger.warn("Failed to update lastActivity for user {}", userId, ex)
+ }클래스 상단에 로거가 없다면:
private val logger = org.slf4j.LoggerFactory.getLogger(UserBookService::class.java)더 나은 구조: 도메인 이벤트(or outbox)로 발행 후 비동기 처리.
| @PutMapping("/me/notification-settings") | ||
| fun updateNotificationSettings( | ||
| @Parameter(hidden = true) @AuthenticationPrincipal userId: UUID, | ||
| @Valid @RequestBody @Parameter(description = "알림 설정 요청 객체") request: NotificationSettingsRequest | ||
| ): ResponseEntity<UserProfileResponse> | ||
|
|
||
| @Operation( | ||
| summary = "FCM 토큰 등록", | ||
| description = "사용자의 FCM 토큰을 등록합니다." | ||
| ) | ||
| @ApiResponses( | ||
| value = [ | ||
| ApiResponse( | ||
| responseCode = "200", | ||
| description = "FCM 토큰 등록 성공", | ||
| content = [Content(schema = Schema(implementation = UserProfileResponse::class))] | ||
| ), | ||
| ApiResponse( | ||
| responseCode = "400", | ||
| description = "잘못된 요청 파라미터", | ||
| content = [Content(schema = Schema(implementation = ErrorResponse::class))] | ||
| ), | ||
| ApiResponse( | ||
| responseCode = "401", | ||
| description = "인증되지 않은 사용자", | ||
| content = [Content(schema = Schema(implementation = ErrorResponse::class))] | ||
| ), | ||
| ApiResponse( | ||
| responseCode = "404", | ||
| description = "사용자를 찾을 수 없습니다.", | ||
| content = [Content(schema = Schema(implementation = ErrorResponse::class))] | ||
| ) | ||
| ] | ||
| ) | ||
| @PutMapping("/me/fcm-token") | ||
| fun updateFcmToken( | ||
| @Parameter(hidden = true) @AuthenticationPrincipal userId: UUID, | ||
| @Valid @RequestBody @Parameter(description = "FCM 토큰 요청 객체") request: FcmTokenRequest | ||
| ): ResponseEntity<UserProfileResponse> |
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.
🧹 Nitpick | 🔵 Trivial
OpenAPI에 보안 요구 사항을 명시하면 문서 활용성이 좋아집니다.
각 Operation 또는 인터페이스 레벨에 @securityrequirement(name = "bearerAuth") 추가를 고려해 주세요. 스웨거 UI에서 인증 흐름이 명확해집니다.
예시:
@Operation(
summary = "...",
security = [SecurityRequirement(name = "bearerAuth")]
)🤖 Prompt for AI Agents
In apis/src/main/kotlin/org/yapp/apis/user/controller/UserControllerApi.kt
around lines 118 to 156, the OpenAPI operations (updateNotificationSettings,
updateFcmToken) lack security metadata; add @SecurityRequirement(name =
"bearerAuth") to each @Operation (or place a single @SecurityRequirement at the
controller/class level) so the generated Swagger UI shows the bearer token auth
flow; ensure the annotation is imported from
io.swagger.v3.oas.annotations.security.SecurityRequirement and apply it
consistently to all authenticated endpoints.
| data class NotificationSettingsRequest private constructor( | ||
| @field:Schema( | ||
| description = "알림 설정 여부", | ||
| example = "true", | ||
| required = true | ||
| ) | ||
| @field:NotNull(message = "알림 설정 여부는 필수입니다.") | ||
| val notificationEnabled: Boolean? = null | ||
| ) { | ||
| fun validNotificationEnabled(): Boolean = notificationEnabled!! |
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.
Nullable 타입과 @NotNull 검증의 모순된 사용 및 위험한 !! 연산자 사용
notificationEnabled 필드가 nullable 타입(Boolean?)으로 선언되어 있으면서 @NotNull 검증을 사용하고, validNotificationEnabled() 메서드에서 !! 연산자를 사용하고 있습니다. 이는 다음과 같은 문제가 있습니다:
- 검증이 실패하거나 우회되면 NPE가 발생할 수 있습니다
- nullable과 @NotNull의 조합이 혼란스럽습니다
!!연산자는 Kotlin에서 권장되지 않는 패턴입니다
다음과 같이 non-nullable 타입으로 변경하세요:
@Schema(
name = "NotificationSettingsRequest",
description = "DTO for notification settings update requests"
)
-data class NotificationSettingsRequest private constructor(
+data class NotificationSettingsRequest(
@field:Schema(
description = "알림 설정 여부",
example = "true",
required = true
)
@field:NotNull(message = "알림 설정 여부는 필수입니다.")
- val notificationEnabled: Boolean? = null
-) {
- fun validNotificationEnabled(): Boolean = notificationEnabled!!
-}
+ val notificationEnabled: Boolean
+)또는 nullable을 유지해야 한다면:
- fun validNotificationEnabled(): Boolean = notificationEnabled!!
+ fun validNotificationEnabled(): Boolean =
+ notificationEnabled ?: throw IllegalStateException("알림 설정 여부는 필수입니다.")Committable suggestion skipped: line range outside the PR's diff.
🤖 Prompt for AI Agents
In
apis/src/main/kotlin/org/yapp/apis/user/dto/request/NotificationSettingsRequest.kt
around lines 10-19, the field notificationEnabled is declared as nullable
Boolean? while annotated with @NotNull and the validNotificationEnabled() uses
the unsafe !! operator; change notificationEnabled to a non-nullable Boolean
(remove the ? and the default = null) and update/remove the
validNotificationEnabled() method to return the field directly, or if you must
keep nullable, keep the ? type but eliminate !! by returning a safe default or
throwing a clear IllegalArgumentException after an explicit null check; ensure
annotations and types match so @NotNull isn't applied to a nullable type.
| @field:Schema( | ||
| description = "Whether notifications are enabled for the user", | ||
| example = "true" | ||
| ) | ||
| val notificationEnabled: Boolean |
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.
🧹 Nitpick | 🔵 Trivial
🧩 Analysis chain
신규 응답 필드 추가는 적절해 보입니다. 명명/기본값 문서화 점검 부탁드립니다.
- 클라이언트 영향은 낮지만, Request/VO/Entity 전 구간에서 필드명이 일관적인지 확인해 주세요(예: NotificationSettingsRequest의 필드명).
- Swagger 스키마에 defaultValue를 추가하면 사용성이 좋아집니다.
다음으로 빠르게 확인해 보세요:
🏁 Script executed:
#!/bin/bash
# NotificationSettingsRequest 필드 확인
fd -t f 'NotificationSettingsRequest.kt' | xargs -I{} sh -c 'echo "== {} =="; sed -n "1,200p" {}'
# notificationEnabled 전역 사용처 확인
rg -n --type=kt -S '\bnotificationEnabled\b' -C2
# UserProfileVO에 동일 필드 존재/매핑 확인
fd -t f 'UserProfileVO.kt' | xargs -I{} sh -c 'echo "== {} =="; sed -n "1,200p" {}'Length of output: 1739
---
🏁 Script executed:
```shell
#!/bin/bash
# rg 재시도: notificationEnabled 전역 사용처
rg -n 'notificationEnabled' -C2
# User Entity 파일 찾기
fd -t f 'User\.kt' | head -5
# UserProfileResponse 전체 확인
fd -t f 'UserProfileResponse.kt' | xargs -I{} sh -c 'echo "== {} =="; cat {}'
Length of output: 14295
신규 필드 추가는 적절하고 전체 레이어에서 일관성이 확보되었습니다.
검증 결과 다음 사항들을 확인했습니다:
- ✅ 필드명 일관성: Request, Domain Entity, VO, Response 모두
notificationEnabled사용 - ✅ 매핑 체인: UserProfileVO.newInstance()와 UserProfileResponse.from()에서 정확히 매핑됨
- ✅ 기본값: User와 UserEntity 수준에서
notificationEnabled = true로 설정됨 - ✅ Request 검증:
@NotNullvalidation으로 필수값 강제됨
선택적 개선사항: UserProfileResponse의 @field:Schema에 defaultValue = "true"를 명시하면 Swagger 문서의 가독성이 더 좋아집니다.
@field:Schema(
description = "Whether notifications are enabled for the user",
example = "true",
defaultValue = "true" // 추가 제안
)
val notificationEnabled: Boolean🤖 Prompt for AI Agents
In apis/src/main/kotlin/org/yapp/apis/user/dto/response/UserProfileResponse.kt
around lines 44 to 48, add a defaultValue = "true" to the @field:Schema
annotation for notificationEnabled to improve Swagger readability; update the
annotation to include defaultValue = "true" while keeping the existing
description and example so generated docs show the default.
| @Entity | ||
| @Table(name = "notification") | ||
| class NotificationEntity( |
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.
🧩 Analysis chain
조회 패턴에 맞춘 인덱스 추가를 권장합니다.
is_sent/ is_read + user_id로 조회할 가능성이 높습니다. JPA 레벨에서도 인덱스 선언해 두면 마이그레이션 누락을 줄일 수 있습니다.
예시:
-@Table(name = "notification")
+@Table(
+ name = "notification",
+ indexes = [
+ Index(name = "ix_notification_user_sent", columnList = "user_id,is_sent"),
+ Index(name = "ix_notification_user_read", columnList = "user_id,is_read")
+ ]
+)마이그레이션 파일에도 동일 인덱스를 추가해야 합니다.
🏁 Script executed:
#!/bin/bash
# notification 테이블 생성/인덱스 마이그레이션 존재 여부 확인
fd -t f 'V*__*.sql' --search-path . 2>/dev/null | xargs -I{} rg -n 'notification|ix_notification_user_' {}Length of output: 0
더 정확한 마이그레이션 파일 검색이 필요합니다. 리포지토리 구조를 파악하고 재검증하겠습니다.
---
🏁 Script executed:
```shell
#!/bin/bash
# 마이그레이션 파일 위치 찾기 및 notification 관련 SQL 확인
find . -type f -name "*.sql" 2>/dev/null | head -20
Length of output: 289
🏁 Script executed:
#!/bin/bash
# NotificationEntity.kt 파일 전체 내용 확인
find . -type f -name "NotificationEntity.kt" -exec cat {} \;Length of output: 2399
🏁 Script executed:
#!/bin/bash
# 테이블/인덱스 정의 관련 모든 SQL 파일 검색
rg -l 'notification|ix_notification' --type sql 2>/dev/nullLength of output: 0
🏁 Script executed:
#!/bin/bash
# Flyway 또는 Liquibase 마이그레이션 디렉토리 구조 확인
fd -t d 'migration|migrations|db' --max-depth 3 2>/dev/nullLength of output: 0
마이그레이션 파일 내용을 확인하여 현재 인덱스 정의 상태를 파악하겠습니다.
---
🏁 Script executed:
```shell
#!/bin/bash
# V1__init.sql에서 notification 테이블 생성 부분 확인
cat ./infra/src/main/resources/db/migration/mysql/V1__init.sql | grep -A 30 -i 'notification'
Length of output: 0
🏁 Script executed:
#!/bin/bash
# 모든 SQL 파일에서 notification 또는 ix_notification 포함 내용 확인
rg -n 'notification|ix_notification' ./infra/src/main/resources/db/migration/ 2>/dev/nullLength of output: 0
마이그레이션 파일의 전체 내용을 직접 확인하겠습니다.
---
🏁 Script executed:
```shell
#!/bin/bash
# V1__init.sql 전체 내용 확인
cat ./infra/src/main/resources/db/migration/mysql/V1__init.sql
Length of output: 3254
🏁 Script executed:
#!/bin/bash
# 모든 SQL 파일 내용에서 notification 또는 CREATE TABLE 확인
find ./infra/src/main/resources/db/migration -name "*.sql" -exec echo "=== {} ===" \; -exec cat {} \;Length of output: 3511
notification 테이블 마이그레이션이 누락되었습니다. 즉시 생성 필요합니다.
마이그레이션 파일(V1__init.sql)에 notification 테이블 생성 구문이 완전히 빠져있습니다. NotificationEntity는 정의되어 있으나 DB 테이블이 없어 런타임 오류가 발생합니다.
새 마이그레이션 파일을 생성하여 notification 테이블을 만들되, 조회 패턴(user_id + is_sent/is_read)에 맞춘 인덱스를 함께 추가하세요:
+CREATE TABLE notification
+(
+ id VARCHAR(36) NOT NULL,
+ created_at datetime(6) NOT NULL,
+ updated_at datetime(6) NOT NULL,
+ user_id VARCHAR(36) NOT NULL,
+ fcm_token VARCHAR(255) NOT NULL,
+ title VARCHAR(255) NOT NULL,
+ message VARCHAR(1000) NOT NULL,
+ notification_type VARCHAR(50) NOT NULL,
+ is_read BIT(1) NOT NULL DEFAULT 0,
+ is_sent BIT(1) NOT NULL DEFAULT 0,
+ sent_at datetime(6) NULL,
+ CONSTRAINT pk_notification PRIMARY KEY (id),
+ CONSTRAINT fk_notification_user FOREIGN KEY (user_id) REFERENCES users(id)
+);
+
+CREATE INDEX ix_notification_user_sent ON notification (user_id, is_sent);
+CREATE INDEX ix_notification_user_read ON notification (user_id, is_read);마찬가지로 NotificationEntity.kt의 @table 속성에 인덱스를 명시하면 마이그레이션 누락을 방지할 수 있습니다:
-@Table(name = "notification")
+@Table(
+ name = "notification",
+ indexes = [
+ Index(name = "ix_notification_user_sent", columnList = "user_id,is_sent"),
+ Index(name = "ix_notification_user_read", columnList = "user_id,is_read")
+ ]
+)📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| @Entity | |
| @Table(name = "notification") | |
| class NotificationEntity( | |
| @Entity | |
| @Table( | |
| name = "notification", | |
| indexes = [ | |
| Index(name = "ix_notification_user_sent", columnList = "user_id,is_sent"), | |
| Index(name = "ix_notification_user_read", columnList = "user_id,is_read") | |
| ] | |
| ) | |
| class NotificationEntity( |
🤖 Prompt for AI Agents
In
infra/src/main/kotlin/org/yapp/infra/notification/entity/NotificationEntity.kt
around lines 13-15, the DB migration for the notification table is missing which
causes runtime errors; create a new Flyway migration (e.g.,
V2__create_notification_table.sql) that defines the notification table with
columns matching NotificationEntity (id PK, user_id, message/body, is_sent
boolean default false, is_read boolean default false, created_at/updated_at
timestamps, etc.) and add indexes tuned for the query patterns (at least
idx_notification_user_id_is_sent and idx_notification_user_id_is_read, and
optionally idx_notification_user_id_created_at). Also annotate
NotificationEntity.kt @Table with the corresponding Index declarations to keep
JPA mapping and migrations consistent (add Index definitions for user_id+is_sent
and user_id+is_read).
| @ManyToOne(fetch = FetchType.LAZY) | ||
| @JoinColumn(name = "user_id", nullable = false) | ||
| val user: UserEntity, | ||
|
|
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.
🧩 Analysis chain
User 매핑 생성 전략 확인 필요
UserEntity.fromDomain(notification.user)가 전체 속성을 새 엔티티로 구성하면 불필요한 병합/플러시를 유발할 수 있습니다. 단순 참조(id)만으로 연결(예: EntityManager.getReference)하는 전략인지 확인해 주세요.
도메인→엔티티 변환 경로와 fromDomain 구현을 점검해 주세요. 필요 시 리포지토리 구현에서 getReference를 사용하도록 변경하는 것을 권장합니다.
Also applies to: 48-62
🏁 Script executed:
#!/bin/bash
# UserEntity.fromDomain 구현 확인
fd -t f 'UserEntity.kt' | xargs -I{} sh -c 'echo "== {} =="; sed -n "1,220p" {}' | sed -n '1,220p'
# NotificationRepository 구현에서 getReference 사용 여부 확인
rg -n --type=kt -C2 'getReference|getOne|byId\('Length of output: 3654
🏁 Script executed:
#!/bin/bash
# Find NotificationEntity and check lines 21-24 and 48-62
fd -t f 'NotificationEntity.kt'Length of output: 78
🏁 Script executed:
#!/bin/bash
# Find NotificationRepository and related notification creation logic
fd -t f 'NotificationRepository.kt' -o -t f 'Notification.*Repository.kt'Length of output: 116
🏁 Script executed:
#!/bin/bash
# Search for fromDomain usage in notification-related files
rg -n 'UserEntity\.fromDomain' -A 3 -B 3Length of output: 2596
🏁 Script executed:
#!/bin/bash
# Search for how user field is set in notification creation
rg -n 'val user' -A 2 -B 2 | grep -A 5 -B 5 'NotificationEntity\|notification'Length of output: 8097
🏁 Script executed:
#!/bin/bash
# Get the complete NotificationEntity.kt file to see full context
cat infra/src/main/kotlin/org/yapp/infra/notification/entity/NotificationEntity.ktLength of output: 2399
🏁 Script executed:
#!/bin/bash
# Get the complete NotificationRepositoryImpl.kt file
cat infra/src/main/kotlin/org/yapp/infra/notification/repository/impl/NotificationRepositoryImpl.ktLength of output: 1474
🏁 Script executed:
#!/bin/bash
# Search for JPA NotificationRepository interface
fd -t f 'JpaNotificationRepository.kt'Length of output: 89
🏁 Script executed:
#!/bin/bash
# Check JpaNotificationRepository interface methods
cat infra/src/main/kotlin/org/yapp/infra/notification/repository/JpaNotificationRepository.ktLength of output: 502
🏁 Script executed:
#!/bin/bash
# Check if EntityManager or getReference is used anywhere in the codebase
rg -n 'EntityManager|getReference' --type=kt | head -30Length of output: 26
🏁 Script executed:
#!/bin/bash
# Check if there are other entities using a similar pattern with ID-only references
rg -n '@ManyToOne' -A 2 --type=kt | head -40Length of output: 26
NotificationEntity에서 불필요한 전체 User 엔티티 생성으로 인한 성능 문제 확인됨
검증 결과, 지적사항이 타당합니다. 다음 두 위치에서 성능 문제가 확인되었습니다:
-
NotificationEntity.fromDomain (line 52):
UserEntity.fromDomain(notification.user)를 호출하여 모든 user 속성(id, email, nickname, profileImageUrl 등)을 포함한 완전한 엔티티를 생성합니다. @manytoone 관계에서는 user_id만 필요하므로 불필요합니다. -
NotificationRepositoryImpl.findByUser (line 24): 동일하게 전체 UserEntity를 생성하는데, 이미 존재하는
findByUserId(userId: UUID)메서드로 대체 가능합니다.
권장 조치:
NotificationRepositoryImpl.findByUser()를findByUserId(user.id.value)호출로 변경NotificationEntity.fromDomain()에서 user 설정 시 ID만으로 참조를 설정하도록 개선 (예: EntityManager.getReference 활용 또는 경량 엔티티 생성)
🤖 Prompt for AI Agents
In
infra/src/main/kotlin/org/yapp/infra/notification/entity/NotificationEntity.kt
(around lines 21-24 and 52) and
infra/src/main/kotlin/org/yapp/infra/notification/repository/NotificationRepositoryImpl.kt
(around line 24), the code creates full UserEntity instances unnecessarily
causing performance issues; change NotificationRepositoryImpl.findByUser(...) to
call the existing findByUserId(user.id.value) instead of building a full
UserEntity, and update NotificationEntity.fromDomain(...) to set the user
relation by reference only (use
EntityManager.getReference(UserEntity::class.java, user.id.value) or construct a
lightweight UserEntity containing only the id) so only user_id is persisted/used
for the @ManyToOne association.
| @Column(nullable = false) | ||
| var fcmToken: String, | ||
|
|
||
| @Column(nullable = false) | ||
| var title: String, | ||
|
|
||
| @Column(nullable = false) | ||
| var message: String, | ||
|
|
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.
🛠️ Refactor suggestion | 🟠 Major
컬럼 길이/타입 명시로 데이터 손실을 예방하세요.
- fcmToken, title 길이 지정, message는 TEXT/LOB 권장.
예시:
- @Column(nullable = false)
- var fcmToken: String,
+ @Column(nullable = false, length = 255)
+ var fcmToken: String,
- @Column(nullable = false)
- var title: String,
+ @Column(nullable = false, length = 128)
+ var title: String,
- @Column(nullable = false)
- var message: String,
+ @Lob
+ @Column(nullable = false, columnDefinition = "TEXT")
+ var message: String,DB 방언에 따라 TEXT/LONGTEXT 등으로 스키마를 조정해 주세요.
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| @Column(nullable = false) | |
| var fcmToken: String, | |
| @Column(nullable = false) | |
| var title: String, | |
| @Column(nullable = false) | |
| var message: String, | |
| @Column(nullable = false, length = 255) | |
| var fcmToken: String, | |
| @Column(nullable = false, length = 128) | |
| var title: String, | |
| @Lob | |
| @Column(nullable = false, columnDefinition = "TEXT") | |
| var message: String, |
🤖 Prompt for AI Agents
infra/src/main/kotlin/org/yapp/infra/notification/entity/NotificationEntity.kt
around lines 25 to 33: the nullable=false columns fcmToken, title and message
lack explicit length/type metadata which can cause schema defaults and potential
data truncation; update fcmToken and title with explicit length attributes (e.g.
fcmToken length ~512, title length ~255) via the @Column(length=...,
nullable=false) and mark message as a large object by using @Lob or
@Column(columnDefinition="TEXT", nullable=false) so the generated DDL uses an
appropriate TEXT/LOB type for your DB dialect.
| interface JpaNotificationRepository : JpaRepository<NotificationEntity, UUID> { | ||
| fun findByUser(user: UserEntity): NotificationEntity? | ||
| fun findByUserId(userId: UUID): List<NotificationEntity> | ||
| fun findByIsSent(isSent: Boolean): List<NotificationEntity> | ||
| } |
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.
🧹 Nitpick | 🔵 Trivial
카디널리티와 조회 전략 확인
- findByUser(user): 단건 반환은 사용자별 NotificationEntity가 1:1임을 전제합니다. 1:N일 수 있으면 List 반환으로 바꾸세요.
- findByIsSent(isSent): 대량 미전송 조회 시 페이징/정렬이 필요할 수 있습니다. Page 시그니처와 인덱스(is_sent, created_at 등) 추가를 고려하세요.
스키마 제약(유니크/인덱스) 및 전송 워크플로우 요구사항을 공유해 주시면 최적화 제안 드리겠습니다.
🤖 Prompt for AI Agents
In
infra/src/main/kotlin/org/yapp/infra/notification/repository/JpaNotificationRepository.kt
around lines 8–12, the repository methods assume cardinality and retrieval
strategy that may be incorrect: change findByUser(user) to return
List<NotificationEntity> if a user can have multiple notifications (or enforce a
DB unique constraint if truly 1:1), and change findByIsSent(isSent) to a
paginated signature (Page<NotificationEntity> with a Pageable parameter) to
support bulk processing and sorting; additionally, add DB indexes on is_sent and
created_at (and a unique index on user_id only if you enforce 1:1) to support
efficient queries and update the service/consumer code to use paging
accordingly.
| @Column(name = "fcm_token", length = 1024) | ||
| var fcmToken: String? = null, | ||
|
|
||
| @Column(name = "notification_enabled", nullable = false) | ||
| var notificationEnabled: Boolean = true, | ||
|
|
||
| @Column(name = "last_activity") | ||
| var lastActivity: LocalDateTime? = null |
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.
🧩 Analysis chain
스키마 마이그레이션 누락 위험(Flyway placeholder만 추가됨)
새 컬럼(fcm_token, notification_enabled, last_activity)이 엔티티에 추가되었지만, PR 설명에 따르면 Flyway에 비어 있는 placeholder만 있습니다. 배포 시 런타임 오류가 발생합니다. 머지 전 반드시 DDL을 포함해 주세요.
[ raise_critical_issue ][ offer_assistance ][ request_verification ]
예시(PostgreSQL):
ALTER TABLE users
ADD COLUMN IF NOT EXISTS fcm_token varchar(1024),
ADD COLUMN IF NOT EXISTS notification_enabled boolean NOT NULL DEFAULT true,
ADD COLUMN IF NOT EXISTS last_activity timestamp without time zone;
-- 스케줄 쿼리 최적화용 인덱스
CREATE INDEX IF NOT EXISTS idx_users_last_activity_enabled
ON users (last_activity)
WHERE notification_enabled = true;검증 스크립트:
🏁 Script executed:
#!/bin/bash
# Flyway 마이그레이션 파일과 users 컬럼/인덱스 존재 확인
fd -HI 'V*__*.sql' -- 'infra|batch|api|*resources*/db/migration' -x bash -lc 'echo "== {} =="; sed -n "1,200p" "{}" | rg -n "users|fcm_token|notification_enabled|last_activity|INDEX" -n -C2 || true'
# UserRepository에 lastActivity/notificationEnabled 조건 쿼리 존재 확인
rg -nP "findByLastActivityBeforeAndNotificationEnabled" -C2Length of output: 3819
🏁 Script executed:
#!/bin/bash
# Find Flyway migration files
echo "=== Searching for Flyway migration files ==="
find . -type f -name "V*.sql" 2>/dev/null | head -20
echo ""
echo "=== Checking db/migration directories ==="
find . -type d -name "migration" 2>/dev/null
echo ""
echo "=== Listing migration files in resources ==="
find . -path "*/db/migration/*.sql" 2>/dev/null | sortLength of output: 601
🏁 Script executed:
#!/bin/bash
# Examine V1 init migration
echo "=== V1__init.sql content (users table section) ==="
cat ./infra/src/main/resources/db/migration/mysql/V1__init.sql | grep -A 50 -i "CREATE TABLE.*users"
echo ""
echo "=== Checking for fcm_token, notification_enabled, last_activity in V1 ==="
grep -i "fcm_token\|notification_enabled\|last_activity" ./infra/src/main/resources/db/migration/mysql/V1__init.sql && echo "FOUND" || echo "NOT FOUND in V1"
echo ""
echo "=== Listing all SQL files in migration directory ==="
ls -la ./infra/src/main/resources/db/migration/mysql/
echo ""
echo "=== Check for placeholder or incomplete files ==="
find ./infra/src/main/resources/db/migration -type f \( -name "*.sql" -o -name "*.placeholder" \) | xargs ls -lahLength of output: 2750
🏁 Script executed:
#!/bin/bash
# Check the ISBN migration content
echo "=== V20250820_001__ALTER_User_books_Book_isbn13_To_Varchar13.sql content ==="
cat ./infra/src/main/resources/db/migration/mysql/V20250820_001__ALTER_User_books_Book_isbn13_To_Varchar13.sql
echo ""
echo "=== Verify UserEntity has these new fields ==="
grep -n -A 10 "fcm_token\|notification_enabled\|last_activity" ./infra/src/main/kotlin/org/yapp/infra/user/entity/UserEntity.kt | head -30Length of output: 759
Flyway 마이그레이션 파일 누락 확인됨 - 배포 전 반드시 수정 필요
검증 결과 review 지적이 정확합니다:
users테이블 스키마(V1__init.sql)에fcm_token,notification_enabled,last_activity컬럼이 존재하지 않음- UserEntity.kt에는 이 3개 필드가 추가됨 (45-52줄)
- 배치 작업(FcmNotificationJobConfig.kt 32줄, 68줄)이 이미
findByLastActivityBeforeAndNotificationEnabled()쿼리 사용 중
배포 시 런타임 오류 발생: Hibernate가 존재하지 않는 컬럼을 매핑하려 할 때 SQLException 발생하고 배치 작업 실패
필수 조치: 다음 DDL을 포함한 새 마이그레이션 파일(예: V20250818_002__Add_User_FCM_And_Activity_Columns.sql)을 infra/src/main/resources/db/migration/mysql/ 디렉토리에 추가하세요:
ALTER TABLE users
ADD COLUMN fcm_token varchar(1024),
ADD COLUMN notification_enabled boolean NOT NULL DEFAULT true,
ADD COLUMN last_activity timestamp;
CREATE INDEX idx_users_last_activity_enabled
ON users (last_activity)
WHERE notification_enabled = true;🤖 Prompt for AI Agents
In infra/src/main/kotlin/org/yapp/infra/user/entity/UserEntity.kt around lines
45-52, the entity now maps fcmToken, notificationEnabled and lastActivity but
the Flyway V1__init.sql schema lacks these columns; add a new migration SQL file
(e.g. V20250818_002__Add_User_FCM_And_Activity_Columns.sql) under
infra/src/main/resources/db/migration/mysql/ that ALTERs the users table to add
fcm_token varchar(1024), notification_enabled boolean NOT NULL DEFAULT true, and
last_activity timestamp, and create the partial index
idx_users_last_activity_enabled on last_activity WHERE notification_enabled =
true; commit the file, run/verify Flyway locally so the table and index exist
before deploying.
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: 10
♻️ Duplicate comments (1)
batch/src/main/kotlin/org/yapp/batch/job/fcm/FcmNotificationJobConfig.kt (1)
151-174: FCM 토큰 수집 로직: 사용자 최신 토큰 미포함·중복 제거 없음기존 알림에 저장된 토큰만 사용해 현재 user.fcmToken을 반영하지 못하고, 중복/공백 제거도 없습니다. 이전 리뷰 지적과 동일합니다.
- val fcmTokens = userNotifications.map { it.fcmToken }.filter { it.isNotBlank() } + // 후보 토큰 = (기존 알림 토큰 + 현재 사용자 토큰) 정규화/중복 제거 + val candidateTokens = ( + userNotifications.map { it.fcmToken } + + listOfNotNull(user.fcmToken) + ).map { it.trim() }.filter { it.isNotBlank() }.distinct() ... - if (fcmTokens.isEmpty()) { + if (candidateTokens.isEmpty()) { if (existingNotification != null) { - logger.info("No valid FCM tokens found for user: ${user.id.value}, using existing notification") + // 기존 알림에 토큰이 없고 사용자에 유효 토큰이 있으면 보정 + if (!user.fcmToken.isNullOrBlank() && existingNotification.fcmToken.isBlank()) { + val updated = Notification.reconstruct( + id = existingNotification.id, user = user, + fcmToken = user.fcmToken!!.trim(), title = existingNotification.title, message = existingNotification.message, + notificationType = notificationType, isRead = existingNotification.isRead, + isSent = existingNotification.isSent, sentAt = existingNotification.sentAt, + createdAt = existingNotification.createdAt, updatedAt = existingNotification.updatedAt + ) + val saved = notificationRepository.save(updated) + val idx = userNotifications.indexOfFirst { it.id == existingNotification.id } + if (idx >= 0) userNotifications[idx] = saved + } return Pair(false, userNotifications) } ... - val results = fcmService.sendMulticastNotification(fcmTokens, title, message) - logger.info("Sent notifications to user ${user.id.value}: ${results.size} successful out of ${fcmTokens.size}") + val results = fcmService.sendMulticastNotification(candidateTokens, title, message) + logger.info("Sent notifications to user ${user.id.value}: ${results.size} successful out of ${candidateTokens.size}")또한 영구 오류(UNREGISTERED 등) 토큰은 정리(null)하세요.
Also applies to: 176-218
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (1)
batch/src/main/kotlin/org/yapp/batch/job/fcm/FcmNotificationJobConfig.kt(1 hunks)
🧰 Additional context used
🪛 detekt (1.23.8)
batch/src/main/kotlin/org/yapp/batch/job/fcm/FcmNotificationJobConfig.kt
[warning] 28-28: The function checkAndSendNotifications appears to be too complex based on Cognitive Complexity (complexity: 34). Defined complexity threshold for methods is set to '25'
(detekt.complexity.CognitiveComplexMethod)
[warning] 218-218: The caught exception is too generic. Prefer catching specific exceptions to the case that is currently handled.
(detekt.exceptions.TooGenericExceptionCaught)
⏰ 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: build-validation
🔇 Additional comments (2)
batch/src/main/kotlin/org/yapp/batch/job/fcm/FcmNotificationJobConfig.kt (2)
42-65: 알림 설정 세분화(타입별 on/off) 확인 필요현재 필터링은 notificationEnabled(전역)만 고려합니다. 타입별 설정(예: DORMANT만 수신 거부)이 있다면 반영해야 합니다.
도메인에 타입별 설정이 존재하는지 확인 부탁드립니다. 존재한다면 사용자 필터/hasActive 계산 시 해당 타입의 opt-out을 적용해 주세요.
Also applies to: 76-99
67-69: 검토 의견의 구현 불가능성: 제안된 메서드가 코드베이스에 존재하지 않음검토 의견에서 제안한
recordRepository.existsByUserIdAndCreatedAtAfter(user.id.value, thirtyDaysAgo)메서드는 실제 코드베이스에 존재하지 않습니다.ReadingRecordRepository는findByUserBookIdInAndCreatedAtAfter(userBookIds, after)메서드만 제공하므로 UserBook을 거쳐 간접적으로 조회해야 합니다.제안된 수정이 가능하려면:
- UserBook을 먼저 조회한 후(userId 기반)
- 그 책들에 속한 ReadingRecord를 조회해야 함
- 이는 단순 필터링보다 복잡한 로직 필요
요구사항 확인 필요: issue #122의 정확한 DORMANT 정의와 이 변경의 필요성을 수동으로 검증하세요. 현재 lastActivity 기반 접근과 실제 record 생성 기반 접근의 업무 차이를 확인 후, 저장소 메서드 추가 또는 쿼리 재설계를 진행하시기 바랍니다.
| @Scheduled(fixedRate = 60000) | ||
| @Transactional | ||
| fun checkAndSendNotifications() { |
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.
외부 호출을 트랜잭션 내부에서 수행 — 지연·롤백 폭탄·락 확장 리스크
@scheduled 메서드가 @transactional이며 내부에서 FCM 네트워크 호출을 실행합니다. 장기 트랜잭션, DB 락 확장, 부분 실패 시 대규모 롤백 위험이 큽니다. 외부 호출은 트랜잭션 밖에서 수행하거나, 결과 저장 시 REQUIRES_NEW로 분리하세요.
- 1단계(READ): 대상 사용자/알림 판정 조회 — @transactional(readOnly=true)
- 2단계(CALL): FCM 전송 — no TX
- 3단계(WRITE): 전송 결과 저장 — @transactional(propagation=REQUIRES_NEW)
또는 비동기 아웃박스 패턴/큐를 이용해 전송과 영속화를 분리하세요.
Also applies to: 136-218
🧰 Tools
🪛 detekt (1.23.8)
[warning] 28-28: The function checkAndSendNotifications appears to be too complex based on Cognitive Complexity (complexity: 34). Defined complexity threshold for methods is set to '25'
(detekt.complexity.CognitiveComplexMethod)
🤖 Prompt for AI Agents
In batch/src/main/kotlin/org/yapp/batch/job/fcm/FcmNotificationJobConfig.kt
around lines 26-28 (and similarly for 136-218): the scheduled method is
annotated @Transactional and performs FCM network calls inside the transaction;
change the flow so external calls occur outside long-lived transactions by (1)
removing @Transactional from the @Scheduled method, (2) implement a read-only
transactional method to load/decide targets (@Transactional(readOnly = true)),
(3) perform FCM sends with no transaction context, and (4) persist/send results
in a separate transactional boundary using @Transactional(propagation =
REQUIRES_NEW) for each write (or publish to an outbox/queue and handle
persistence asynchronously) to avoid prolonged locks and large rollbacks.
스케줄러 동시 실행(멀티 인스턴스)·중복 발송 위험
@scheduled(fixedRate=60s) 작업에 분산 락이 없어 다중 인스턴스에서 동시에 실행되면 동일 사용자에게 중복 발송됩니다. 또한 작업 시간이 60초를 넘으면 중첩 실행 가능성이 있습니다. ShedLock 등 분산 락을 적용하거나 고유 락을 확보하세요.
예시:
+import net.javacrumbs.shedlock.spring.annotation.SchedulerLock
...
-@Scheduled(fixedRate = 60000)
+@Scheduled(fixedRate = 60000)
+@SchedulerLock(name = "fcmNotificationJob", lockAtMostFor = "PT55S", lockAtLeastFor = "PT30S")또는 고유 테이블 락/DB-기반 락으로 단일 실행을 보장하세요.
Committable suggestion skipped: line range outside the PR's diff.
🧰 Tools
🪛 detekt (1.23.8)
[warning] 28-28: The function checkAndSendNotifications appears to be too complex based on Cognitive Complexity (complexity: 34). Defined complexity threshold for methods is set to '25'
(detekt.complexity.CognitiveComplexMethod)
🤖 Prompt for AI Agents
In batch/src/main/kotlin/org/yapp/batch/job/fcm/FcmNotificationJobConfig.kt
around lines 26 to 28, the scheduled method checkAndSendNotifications() runs
every 60s without a distributed or reentrant lock, which can cause concurrent
execution and duplicate sends across multiple instances or if execution exceeds
the interval; apply a distributed lock (e.g., ShedLock) or a DB-backed unique
lock so only one instance runs the job and to prevent overlap: annotate the
scheduled method with the appropriate locking annotation/config (e.g.,
@SchedulerLock with sensible lockAtMostFor and lockAtLeastFor) or implement an
explicit single-row DB lock acquisition/release around the job logic, and ensure
transactional/exception handling releases the lock on failure.
| @Scheduled(fixedRate = 60000) | ||
| @Transactional | ||
| fun checkAndSendNotifications() { | ||
| logger.info("Starting notification check job") | ||
|
|
||
| val sevenDaysAgo = LocalDateTime.now().minusDays(7) | ||
| val unrecordedUsers = userRepository.findByLastActivityBeforeAndNotificationEnabled(sevenDaysAgo, true) | ||
|
|
||
| val userNotificationsCache = mutableMapOf<UUID, List<Notification>>() | ||
|
|
||
| var unrecordedSuccessCount = 0 | ||
| unrecordedUsers.forEach { user -> | ||
| val userNotifications = userNotificationsCache.getOrPut(user.id.value) { | ||
| notificationRepository.findByUserId(user.id.value) | ||
| } | ||
|
|
||
| val hasActiveUnrecordedNotification = userNotifications.any { | ||
| it.notificationType == NotificationType.UNRECORDED && it.isSent | ||
| } | ||
| if (!hasActiveUnrecordedNotification) { | ||
| if (userNotifications.isNotEmpty()) { | ||
| val resetNotifications = userNotifications.filter { | ||
| it.notificationType == NotificationType.UNRECORDED && !it.isSent | ||
| } | ||
| if (resetNotifications.isNotEmpty()) { | ||
| logger.info("Sending new unrecorded notification to user ${user.id.value} after previous notification was reset. User lastActivity: ${user.lastActivity}") | ||
| } | ||
| } | ||
| val (success, updatedNotifications) = sendNotificationToUser( | ||
| user = user, | ||
| title = "\uD83D\uDCDA 잠시 멈춘 기록.. 다시 이어가 볼까요?", | ||
| message = "이번주에 읽은 책, 잊기 전에 기록해 보세요!", | ||
| notificationType = NotificationType.UNRECORDED, | ||
| existingNotifications = userNotifications | ||
| ) | ||
| if (success) unrecordedSuccessCount++ | ||
| if (updatedNotifications.isNotEmpty()) { | ||
| userNotificationsCache[user.id.value] = updatedNotifications | ||
| } | ||
| } | ||
| } | ||
| val thirtyDaysAgo = LocalDateTime.now().minusDays(30) | ||
| val dormantUsers = userRepository.findByLastActivityBeforeAndNotificationEnabled(thirtyDaysAgo, true) | ||
|
|
||
| var dormantSuccessCount = 0 | ||
| dormantUsers.forEach { user -> | ||
| val userNotifications = userNotificationsCache.getOrPut(user.id.value) { | ||
| notificationRepository.findByUserId(user.id.value) | ||
| } | ||
|
|
||
| val hasActiveDormantNotification = userNotifications.any { | ||
| it.notificationType == NotificationType.DORMANT && it.isSent | ||
| } | ||
| if (!hasActiveDormantNotification) { | ||
| if (userNotifications.isNotEmpty()) { | ||
| val resetNotifications = userNotifications.filter { | ||
| it.notificationType == NotificationType.DORMANT && !it.isSent | ||
| } | ||
| if (resetNotifications.isNotEmpty()) { | ||
| logger.info("Sending new dormant notification to user ${user.id.value} after previous notification was reset. User lastActivity: ${user.lastActivity}") | ||
| } | ||
| } | ||
| val (success, updatedNotifications) = sendNotificationToUser( | ||
| user = user, | ||
| title = "\uD83D\uDCDA Reed와 함께 독서 기록 시작", | ||
| message = "그동안 읽은 책을 모아 기록해 보세요!", | ||
| notificationType = NotificationType.DORMANT, | ||
| existingNotifications = userNotifications | ||
| ) | ||
| if (success) dormantSuccessCount++ | ||
| if (updatedNotifications.isNotEmpty()) { | ||
| userNotificationsCache[user.id.value] = updatedNotifications | ||
| } | ||
| } | ||
| } | ||
| resetNotificationsForActiveUsers() | ||
|
|
||
| logger.info("Completed notification check job. Successfully sent unrecorded notifications to $unrecordedSuccessCount out of ${unrecordedUsers.size} users and dormant notifications to $dormantSuccessCount out of ${dormantUsers.size} users") | ||
| } |
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.
🛠️ Refactor suggestion | 🟠 Major
메서드 복잡도↓ 및 중복 제거: 공통 처리 함수로 추출
detekt가 Cognitive Complexity 34로 경고합니다. 타입별 처리 로직을 공통 함수로 추출해 반복을 제거하세요.
+private fun processCohort(
+ users: List<User>,
+ type: NotificationType,
+ title: String,
+ message: String,
+ cache: MutableMap<UUID, List<Notification>>
+): Int {
+ var success = 0
+ users.forEach { user ->
+ val userNotifications = cache.getOrPut(user.id.value) { notificationRepository.findByUserId(user.id.value) }
+ val hasActive = userNotifications.any { it.notificationType == type && it.isSent }
+ if (!hasActive) {
+ val (ok, updated) = sendNotificationToUser(user, title, message, type, userNotifications)
+ if (ok) success++
+ if (updated.isNotEmpty()) cache[user.id.value] = updated
+ }
+ }
+ return success
+}
...
- var unrecordedSuccessCount = 0
- unrecordedUsers.forEach { ... }
+ val unrecordedSuccessCount = processCohort(
+ unrecordedUsers, NotificationType.UNRECORDED,
+ "📚 잠시 멈춘 기록.. 다시 이어가 볼까요?", "이번주에 읽은 책, 잊기 전에 기록해 보세요!", userNotificationsCache
+ )
...
- var dormantSuccessCount = 0
- dormantUsers.forEach { ... }
+ val dormantSuccessCount = processCohort(
+ dormantUsers, NotificationType.DORMANT,
+ "📚 Reed와 함께 독서 기록 시작", "그동안 읽은 책을 모아 기록해 보세요!", userNotificationsCache
+ )Committable suggestion skipped: line range outside the PR's diff.
🧰 Tools
🪛 detekt (1.23.8)
[warning] 28-28: The function checkAndSendNotifications appears to be too complex based on Cognitive Complexity (complexity: 34). Defined complexity threshold for methods is set to '25'
(detekt.complexity.CognitiveComplexMethod)
| val sevenDaysAgo = LocalDateTime.now().minusDays(7) | ||
| val unrecordedUsers = userRepository.findByLastActivityBeforeAndNotificationEnabled(sevenDaysAgo, true) | ||
|
|
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.
🧹 Nitpick | 🔵 Trivial
경계 시각 일관성 확보
LocalDateTime.now()를 여러 번 호출하지 말고 now를 1회만 계산해 사용하세요. 경계 케이스(정각 전후)에서 포함/제외 오차를 줄일 수 있습니다.
- val sevenDaysAgo = LocalDateTime.now().minusDays(7)
+ val now = LocalDateTime.now()
+ val sevenDaysAgo = now.minusDays(7)
...
- val thirtyDaysAgo = LocalDateTime.now().minusDays(30)
+ val thirtyDaysAgo = now.minusDays(30)Also applies to: 67-69
🤖 Prompt for AI Agents
In batch/src/main/kotlin/org/yapp/batch/job/fcm/FcmNotificationJobConfig.kt
around lines 31-33 (and similarly lines 67-69), LocalDateTime.now() is being
called multiple times causing inconsistent boundary timestamps; compute a single
val now = LocalDateTime.now() once at the start of the method or job, then
derive sevenDaysAgo and any other time-based boundaries from that single now
variable and use it throughout the method so all comparisons use the exact same
reference time.
동일 사용자 중복 발송 가능: DORMANT 우선순위로 한 번만 발송하도록 교집합 제거
30일 미접속(또는 무기록) 사용자는 7일 미기록에도 포함될 수 있어 한 번의 배치에서 두 건이 발송됩니다. 더 강한 조건(DORMANT)을 우선하고, UNRECORDED 집합에서 제외하세요.
- val sevenDaysAgo = LocalDateTime.now().minusDays(7)
- val unrecordedUsers = userRepository.findByLastActivityBeforeAndNotificationEnabled(sevenDaysAgo, true)
+ val now = LocalDateTime.now()
+ val sevenDaysAgo = now.minusDays(7)
+ val unrecordedAll = userRepository.findByLastActivityBeforeAndNotificationEnabled(sevenDaysAgo, true)
- val thirtyDaysAgo = LocalDateTime.now().minusDays(30)
- val dormantUsers = userRepository.findByLastActivityBeforeAndNotificationEnabled(thirtyDaysAgo, true)
+ val thirtyDaysAgo = now.minusDays(30)
+ val dormantUsers = userRepository.findByLastActivityBeforeAndNotificationEnabled(thirtyDaysAgo, true)
+ val dormantIds = dormantUsers.map { it.id.value }.toSet()
+ val unrecordedUsers = unrecordedAll.filterNot { it.id.value in dormantIds }또한 now를 1회만 계산해 경계시각 불일치를 방지하세요.
Also applies to: 67-69, 36-66, 70-100
| var unrecordedSuccessCount = 0 | ||
| unrecordedUsers.forEach { user -> | ||
| val userNotifications = userNotificationsCache.getOrPut(user.id.value) { | ||
| notificationRepository.findByUserId(user.id.value) | ||
| } | ||
|
|
||
| val hasActiveUnrecordedNotification = userNotifications.any { | ||
| it.notificationType == NotificationType.UNRECORDED && it.isSent | ||
| } | ||
| if (!hasActiveUnrecordedNotification) { | ||
| if (userNotifications.isNotEmpty()) { | ||
| val resetNotifications = userNotifications.filter { | ||
| it.notificationType == NotificationType.UNRECORDED && !it.isSent | ||
| } | ||
| if (resetNotifications.isNotEmpty()) { | ||
| logger.info("Sending new unrecorded notification to user ${user.id.value} after previous notification was reset. User lastActivity: ${user.lastActivity}") | ||
| } | ||
| } | ||
| val (success, updatedNotifications) = sendNotificationToUser( | ||
| user = user, | ||
| title = "\uD83D\uDCDA 잠시 멈춘 기록.. 다시 이어가 볼까요?", | ||
| message = "이번주에 읽은 책, 잊기 전에 기록해 보세요!", | ||
| notificationType = NotificationType.UNRECORDED, | ||
| existingNotifications = userNotifications | ||
| ) | ||
| if (success) unrecordedSuccessCount++ | ||
| if (updatedNotifications.isNotEmpty()) { | ||
| userNotificationsCache[user.id.value] = updatedNotifications | ||
| } | ||
| } | ||
| } |
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.
🧹 Nitpick | 🔵 Trivial
처리량 관리·관찰 가능성 개선 제안
대상 사용자가 많을 경우 매 분 전량 처리·상세 로그는 부담이 큽니다. 청크 처리, 속도 제한, 재시도/백오프, 메트릭(성공/실패/무효토큰 수), 샘플링된 로그로 조정하세요.
- 페이징(예: 500/러닝) + 각 청크 트랜잭션
- Counter/Gauge: attempted, sent, skipped(reason별), invalidTokens
- structured logging(level=info 집계, per-user는 debug로 다운)
Also applies to: 71-100, 103-104
| @Transactional | ||
| protected fun resetNotificationsForActiveUsers() { | ||
| val sentNotifications = notificationRepository.findBySent(true) | ||
|
|
||
| sentNotifications.forEach { notification -> | ||
| val user = notification.user | ||
| val sentAt = notification.sentAt | ||
| val lastActivity = user.lastActivity | ||
|
|
||
| if (sentAt != null && lastActivity != null && lastActivity.isAfter(sentAt)) { | ||
| val updatedNotification = Notification.reconstruct( | ||
| id = notification.id, | ||
| user = user, | ||
| fcmToken = notification.fcmToken, | ||
| title = notification.title, | ||
| message = notification.message, | ||
| notificationType = notification.notificationType, | ||
| isRead = notification.isRead, | ||
| isSent = false, | ||
| sentAt = null, | ||
| createdAt = notification.createdAt, | ||
| updatedAt = notification.updatedAt | ||
| ) | ||
|
|
||
| notificationRepository.save(updatedNotification) | ||
| logger.info("Reset notification status for user ${user.id.value} as they have been active since the notification was sent (lastActivity: $lastActivity, sentAt: $sentAt)") | ||
| } | ||
| } | ||
| } |
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.
🧹 Nitpick | 🔵 Trivial
매 분 전체 sent 알림 스캔 — 비용 과다 및 인덱스 의존
findBySent(true)로 전체를 매번 로드하면 데이터 증가 시 과부하입니다. 조인/조건을 확장해 DB 측에서 선별하거나 배치로 페이징 처리하세요.
- 리포지토리 메서드: findByIsSentTrueAndUserLastActivityAfterSentAt() 등으로 축소
- 또는 페이징(batchSize)으로 반복 처리
- 가능한 경우 단일 UPDATE로 리셋 처리
또한 updatedAt 갱신 보장 여부 확인 필요합니다.
🤖 Prompt for AI Agents
In batch/src/main/kotlin/org/yapp/batch/job/fcm/FcmNotificationJobConfig.kt
around lines 106-134, the current implementation loads all sent notifications
via findBySent(true) causing scalability and index pressure; change to perform
DB-side filtering or paging and avoid full-table scans: add a repository query
that selects only notifications where isSent = true AND user.lastActivity >
sentAt (or join on user and compare timestamps) or implement a paged loop using
a size-limited query (batchSize) to process and update chunks; where possible
perform a single UPDATE statement to set isSent = false and sentAt = null for
qualifying records; finally ensure the entity updatedAt is set/updated (either
via DB trigger, @PreUpdate, or explicit timestamp) so updatedAt reflects the
reset, and verify tests cover both query and updatedAt behavior.
| val results = fcmService.sendMulticastNotification(fcmTokens, title, message) | ||
| logger.info("Sent notifications to user ${user.id.value}: ${results.size} successful out of ${fcmTokens.size}") | ||
|
|
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.
🧹 Nitpick | 🔵 Trivial
전송 결과 활용 부족: 실패 토큰 정리·성공 토큰 식별 필요
results의 성공/실패·오류코드에 따라 토큰을 정리하고(예: unregistered → 제거/무효화), 성공 토큰을 저장해야 재시도·품질 지표가 정확해집니다.
예시: FcmService가 (successTokens, invalidTokens) 형태로 반환하도록 확장하고 invalidTokens를 사용자/알림에 반영해 정리하세요.
🤖 Prompt for AI Agents
In batch/src/main/kotlin/org/yapp/batch/job/fcm/FcmNotificationJobConfig.kt
around lines 176-178, the code only logs results.size and does not handle
per-token success/failure; change the flow to have FcmService return a
structured result (e.g., successTokens and invalidTokens or a list with
status/error codes), then iterate that result to persist successful tokens (for
retries/metrics) and remove/mark invalid tokens (e.g., unregistered) in the
user/notification store; update the call site to accept the new return type,
process invalidTokens to delete or mark tokens as inactive, and save
successTokens associated with the sent notification.
| if (results.isNotEmpty()) { | ||
| if (existingNotification != null) { | ||
| val updatedNotification = Notification.reconstruct( | ||
| id = existingNotification.id, | ||
| user = user, | ||
| fcmToken = existingNotification.fcmToken, | ||
| title = title, | ||
| message = message, | ||
| notificationType = notificationType, | ||
| isRead = existingNotification.isRead, | ||
| isSent = true, | ||
| sentAt = LocalDateTime.now(), | ||
| createdAt = existingNotification.createdAt, | ||
| updatedAt = existingNotification.updatedAt | ||
| ) | ||
|
|
||
| val savedNotification = notificationRepository.save(updatedNotification) | ||
| val index = userNotifications.indexOfFirst { it.id == existingNotification.id } | ||
| if (index >= 0) { | ||
| userNotifications[index] = savedNotification | ||
| } | ||
| logger.info("Updated existing notification for user ${user.id.value} to sent") | ||
| } else { | ||
| val notification = Notification.create( | ||
| user = user, | ||
| title = title, | ||
| message = message, | ||
| notificationType = notificationType, | ||
| isSent = true, | ||
| sentAt = LocalDateTime.now() | ||
| ) | ||
|
|
||
| val savedNotification = notificationRepository.save(notification) | ||
| userNotifications.add(savedNotification) | ||
| logger.info("Created new notification for user ${user.id.value} with sent status") | ||
| } | ||
| } |
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.
발송된 Notification에 fcmToken 미기록/불일치
멀티캐스트 전송 후 저장되는 Notification에 fcmToken이 비어있거나(신규 생성), 기존 값만 유지되어 실제 사용 토큰과 불일치합니다. 최소 1개 “대표 토큰(primary)”이라도 기록하세요.
- val updatedNotification = Notification.reconstruct(
+ val primaryToken = candidateTokens.first() // 위 수정안 기준
+ val updatedNotification = Notification.reconstruct(
...
- fcmToken = existingNotification.fcmToken,
+ fcmToken = primaryToken,
...
)
...
- val notification = Notification.create(
+ val notification = Notification.create(
user = user, title = title, message = message, notificationType = notificationType,
isSent = true, sentAt = LocalDateTime.now()
+ // 팩토리에 fcmToken 파라미터가 없다면 오버로드 추가 또는 생성 후 reconstruct로 토큰 설정
)팩토리가 토큰을 받지 않으면 오버로드를 추가하거나 저장 직전에 reconstruct로 토큰을 주입하세요.
Committable suggestion skipped: line range outside the PR's diff.
🤖 Prompt for AI Agents
In batch/src/main/kotlin/org/yapp/batch/job/fcm/FcmNotificationJobConfig.kt
around lines 179 to 215, notifications saved after multicast do not record the
actual FCM token (new notifications leave fcmToken empty and updates keep the
old token), so set a representative primary token before persisting: take the
primary/send token used for the multicast (e.g. the token variable used to send
or results.first().token) and supply it to Notification.reconstruct when
updating existingNotification and to Notification.create when creating a new
notification (or add an overload to the factory that accepts fcmToken), then
save that object so the stored notification has the correct fcmToken.
| } catch (e: Exception) { | ||
| logger.error("Error sending notification to user ${user.id.value}", e) | ||
| return Pair(false, existingNotifications) | ||
| } |
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.
너무 일반적인 예외 처리
Exception 전체를 포착하면 장애 원인 분류·회복 전략이 어렵습니다. FCM 전송 예외(FirebaseMessagingException 등), DB 예외(DataAccessException) 등으로 구체화하고 나머지는 상위로 전파하거나 재시도 정책을 적용하세요.
-} catch (e: Exception) {
+} catch (e: FirebaseMessagingException) {
+ // 토큰 정리·재시도 불가 판정
+ ...
+} catch (e: DataAccessException) {
+ // 저장 실패 처리(재시도/알림)
+ ...
+} catch (e: RuntimeException) {
+ // 마지막 방어
logger.error("Error sending notification to user ${user.id.value}", e)
return Pair(false, existingNotifications)
}(detekt.exceptions.TooGenericExceptionCaught)
Committable suggestion skipped: line range outside the PR's diff.
🧰 Tools
🪛 detekt (1.23.8)
[warning] 218-218: The caught exception is too generic. Prefer catching specific exceptions to the case that is currently handled.
(detekt.exceptions.TooGenericExceptionCaught)
🤖 Prompt for AI Agents
In batch/src/main/kotlin/org/yapp/batch/job/fcm/FcmNotificationJobConfig.kt
around lines 218 to 221, the current catch-all Exception is too generic; replace
it with targeted catches for likely failure types (e.g.,
FirebaseMessagingException for FCM send failures with retry/backoff logic,
DataAccessException for DB errors) and handle each appropriately (log detailed
context and return Pair(false, existingNotifications) only for handled
recoverable cases), while allowing unknown/unexpected exceptions to propagate
(or wrap and rethrow) so higher-level error classification/retry policies can
apply; ensure each catch logs the exception details and context.
move-hoon
left a 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.
고생하셨습니다!
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
♻️ Duplicate comments (2)
batch/src/main/resources/application.yml (1)
43-43: 배치 모듈의 모듈명이 잘못 설정되어 있습니다.배치 모듈의 설정 파일에서
module-name이apis로 설정되어 있습니다. 이는 명백한 오류입니다.다음과 같이 수정하세요:
- module-name: apis + module-name: batchbuildSrc/src/main/kotlin/Dependencies.kt (1)
80-82: Firebase Admin SDK 버전을 9.7.0으로 업데이트하세요.현재 사용 중인 9.2.0은 최신 버전(9.7.0)보다 뒤떨어져 있습니다. 알려진 보안 취약점은 없지만, 최신 버전으로 업데이트하면 버그 수정 및 기능 개선사항을 적용할 수 있습니다.
다음과 같이 수정하세요:
object Firebase { - const val FIREBASE_ADMIN = "com.google.firebase:firebase-admin:9.2.0" + const val FIREBASE_ADMIN = "com.google.firebase:firebase-admin:9.7.0" }
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (4)
batch/build.gradle.kts(1 hunks)batch/src/main/kotlin/org/yapp/batch/BatchApplication.kt(1 hunks)batch/src/main/resources/application.yml(2 hunks)buildSrc/src/main/kotlin/Dependencies.kt(1 hunks)
⏰ 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: build-validation
🔇 Additional comments (2)
batch/build.gradle.kts (1)
12-12: LGTM!JPA 및 Firebase Admin 의존성 추가가 PR 목표(사용자 활동 추적 및 FCM 알림 전송)와 정확히 일치합니다. 의존성 추가가 적절합니다.
Also applies to: 17-17
batch/src/main/kotlin/org/yapp/batch/BatchApplication.kt (1)
8-15: 리뷰 코멘트 승인 - 설정 변경이 올바릅니다.검증 결과, batch 모듈은
org.yapp.batch,org.yapp.infra,org.yapp.domain,org.yapp.globalutils패키지의 컴포넌트만 사용하며, 명시적scanBasePackages설정에 모든 필요한 패키지가 포함되어 있습니다. 외부 패키지(observability,apis,gateway,admin)의 컴포넌트들은 각각 독립적인 Spring Boot 애플리케이션 모듈로, batch 모듈과는 무관합니다.JpaRepositoriesAutoConfiguration제외를 제거하고 명시적 패키지 스캔으로 변경한 것은 build.gradle.kts에 JPA 의존성이 추가된 것과 일치하며 올바른 접근입니다.
| import org.springframework.boot.autoconfigure.SpringBootApplication | ||
| import org.springframework.boot.autoconfigure.data.jpa.JpaRepositoriesAutoConfiguration | ||
| import org.springframework.boot.runApplication | ||
| import org.springframework.context.annotation.ComponentScan |
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.
🧹 Nitpick | 🔵 Trivial
사용하지 않는 import를 제거하세요.
@ComponentScan 어노테이션을 제거했으므로 6번 라인의 ComponentScan import도 더 이상 필요하지 않습니다.
다음과 같이 수정하세요:
import org.springframework.boot.autoconfigure.SpringBootApplication
import org.springframework.boot.autoconfigure.data.jpa.JpaRepositoriesAutoConfiguration
import org.springframework.boot.runApplication
-import org.springframework.context.annotation.ComponentScan📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| import org.springframework.context.annotation.ComponentScan | |
| import org.springframework.boot.autoconfigure.SpringBootApplication | |
| import org.springframework.boot.autoconfigure.data.jpa.JpaRepositoriesAutoConfiguration | |
| import org.springframework.boot.runApplication |
🤖 Prompt for AI Agents
In batch/src/main/kotlin/org/yapp/batch/BatchApplication.kt around line 6,
remove the now-unused import
"org.springframework.context.annotation.ComponentScan" since the @ComponentScan
annotation was removed; delete that import line and save the file to avoid
unused-import warnings.
| springdoc: | ||
| swagger-ui: | ||
| path: /swagger-ui | ||
| enabled: true | ||
| api-docs: | ||
| path: /v3/api-docs | ||
| enabled: true | ||
| override-with-generic-response: false |
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.
🧩 Analysis chain
배치 모듈에서 Swagger 설정이 필요한지 확인하세요.
배치 애플리케이션은 일반적으로 REST API를 노출하지 않으므로 Swagger 설정이 필요하지 않습니다. 배치 모듈에서 REST 엔드포인트를 제공할 의도가 있는지, 또는 이 설정이 다른 모듈(예: APIs 모듈)의 설정과 혼동된 것은 아닌지 확인이 필요합니다.
다음 스크립트로 배치 모듈에 REST 컨트롤러가 있는지 확인하세요:
🏁 Script executed:
#!/bin/bash
# 배치 모듈에 REST 컨트롤러 존재 여부 확인
# RestController 또는 Controller 어노테이션 검색
rg -n --type kotlin -C3 '@(RestController|Controller)' batch/src/Length of output: 0
🏁 Script executed:
#!/bin/bash
# 배치 모듈 디렉토리 구조 확인 및 파일 목록 확인
echo "=== Checking batch/src directory structure ==="
fd --type f --type d . batch/src 2>/dev/null | head -20
echo -e "\n=== Searching for any Controller-like files in batch module ==="
rg -n "Controller|@RestController|@GetMapping|@PostMapping|@RequestMapping" batch/src --type kotlin --type java 2>/dev/null | head -30
echo -e "\n=== Checking for Spring Web dependency in batch module pom.xml or build.gradle ==="
if [ -f batch/pom.xml ]; then
echo "--- batch/pom.xml ---"
grep -i "spring-boot-starter-web\|spring-web" batch/pom.xml
fi
if [ -f batch/build.gradle ]; then
echo "--- batch/build.gradle ---"
grep -i "spring-boot-starter-web\|spring-web" batch/build.gradle
fi
echo -e "\n=== Listing batch module root files ==="
ls -la batch/ 2>/dev/null | grep -E "\.(xml|gradle|properties)$"Length of output: 1021
🏁 Script executed:
#!/bin/bash
# application.yml의 다른 포트 설정 확인 - 배치 모듈이 실제로 웹서버를 구동하는지 확인
echo "=== Checking if batch runs as web application ==="
if [ -f batch/src/main/resources/application.yml ]; then
grep -n "server:\|port:" batch/src/main/resources/application.yml
fi
echo -e "\n=== Checking main application class in batch module ==="
fd --type f "Application\.kt|Application\.java" batch/src/main/java batch/src/main/kotlin 2>/dev/null | xargs cat 2>/dev/nullLength of output: 829
🏁 Script executed:
#!/bin/bash
# 프로젝트 전체 구조 확인 - 다른 모듈과 비교하여 배치 모듈의 특성 파악
echo "=== Overall module structure ==="
ls -d */ 2>/dev/null | head -20
echo -e "\n=== Checking if springdoc swagger config appears in other modules ==="
find . -name "application.yml" -o -name "application.yaml" 2>/dev/null | xargs grep -l "springdoc" 2>/dev/nullLength of output: 322
🏁 Script executed:
#!/bin/bash
# 배치 모듈의 Actuator 의존성 및 관리 엔드포인트 확인
echo "=== Checking for Spring Boot Actuator dependency in batch module ==="
if [ -f batch/pom.xml ]; then
grep -n "spring-boot-starter-actuator\|micrometer" batch/pom.xml
fi
if [ -f batch/build.gradle ]; then
grep -n "spring-boot-starter-actuator\|micrometer" batch/build.gradle
fi
echo -e "\n=== Checking batch application.yml for management or actuator config ==="
cat batch/src/main/resources/application.yml | grep -n "management:\|actuator"
echo -e "\n=== Full application.yml for batch module ==="
cat batch/src/main/resources/application.ymlLength of output: 1607
배치 모듈에서 Swagger 설정 제거 필요
배치 모듈은 REST API를 노출하지 않습니다. 확인 결과:
- REST 컨트롤러 없음
- Spring Web 의존성 없음
- FCM 배치 작업만 구성됨
또한 app.module-name: apis라는 설정이 존재하여 이 구성이 apis 모듈에서 복사된 것으로 보입니다. 배치 모듈의 application.yml에서 lines 33-40의 springdoc 설정을 제거하세요.
🤖 Prompt for AI Agents
In batch/src/main/resources/application.yml around lines 33 to 40, the
springdoc/swagger configuration (path, enabled flags and
override-with-generic-response) is leftover from the apis module and should be
removed because the batch module does not expose REST endpoints; delete those
springdoc entries (lines 33-40) from the file and ensure there are no other
springdoc references or an incorrect app.module-name pointing to "apis" in this
batch config.
✦ 🔗 관련 이슈
📘 작업 유형
📙 작업 내역
🧪 테스트 내역
🎨 스크린샷 또는 시연 영상 (선택)
✅ PR 체크리스트
💬 추가 설명 or 리뷰 포인트 (선택)
사용자 알림 시스템 플로우를 다이어그램으로 표현했습니다. 매 1분마다 실행되는 메인 프로세스에서 두 가지 조건(7일 미기록, 30일 미접속)을 병렬로 확인하고, 각각에 대해 이전 전송 여부를 체크한 후 필요하면 알림을 전송하고 DB를 업데이트하는 구조입니다.
Summary by CodeRabbit
새로운 기능
기타