[Feat] 전화 알림 기능 - #72
Merged
Merged
Conversation
|
Caution Review failedThe pull request is closed. ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (6)
📝 WalkthroughWalkthroughChanges전화 알림 기능
Estimated code review effort: 4 (Complex) | ~45 minutes Sequence Diagram(s)sequenceDiagram
participant CallReminderScheduler
participant CallReminderCommandService
participant VoiceCallSender
participant CallLogRepository
participant CallResultWebhookController
CallReminderScheduler->>CallReminderCommandService: sendDueFirstCalls(now)
CallReminderCommandService->>VoiceCallSender: call(from, to, headerMessage, bodyMessage)
VoiceCallSender-->>CallReminderCommandService: messageId
CallReminderCommandService->>CallLogRepository: save pending CallLog
CallResultWebhookController->>CallReminderCommandService: applyCallResult(messageId, rawStatus)
CallReminderCommandService->>CallLogRepository: findByMessageId(messageId)
CallReminderCommandService->>CallLogRepository: update ANSWERED, NO_ANSWER, or FAILED
Possibly related PRs
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 4
🧹 Nitpick comments (2)
src/main/java/com/piuda/callcare/domain/calllog/entity/CallLog.java (1)
82-96: 🩺 Stability & Availability | 🔵 Trivial | ⚡ Quick win방어적 코딩: 언박싱 시 NPE 방지
retryCount필드가Integer래퍼 타입이므로, 데이터베이스에 0 기본값이 세팅되지 않은 상태에서 객체가 로드되거나 테스트 데이터가null인 경우++또는+ 1연산 시NullPointerException이 발생할 수 있습니다. 방어적으로 Null 처리를 추가하는 것을 권장합니다.🛠️ 널 안전성(Null-Safety) 적용 예시
public void incrementRetryCount() { - this.retryCount++; + this.retryCount = (this.retryCount == null ? 0 : this.retryCount) + 1; } public void markAsNotified() { this.isNotified = true; } // 재시도 발신 시 호출: 새 messageId/발신 시각으로 갱신하고 결과 대기 상태로 되돌린다 public void markRetried(String messageId, LocalDateTime calledAt) { this.messageId = messageId; this.calledAt = calledAt; this.status = CallStatus.PENDING; - this.retryCount = this.retryCount + 1; + this.retryCount = (this.retryCount == null ? 0 : this.retryCount) + 1; }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/main/java/com/piuda/callcare/domain/calllog/entity/CallLog.java` around lines 82 - 96, Update incrementRetryCount and markRetried in CallLog to handle a null retryCount defensively before incrementing, treating null as zero so both retry paths increment safely without unboxing a null Integer.src/main/java/com/piuda/callcare/domain/calllog/service/CallReminderCommandService.java (1)
51-61: 🚀 Performance & Scalability | 🔵 Trivial | 🏗️ Heavy lift전체 Senior 조회로 인한 메모리 및 성능 문제
seniorRepository.findAll()을 통해 시스템의 모든 사용자를 한 번에 메모리로 로드한 뒤 조건을 검사하고 있습니다. 사용자가 늘어날 경우 심각한 응답 지연과 OOM(Out of Memory)을 유발할 수 있습니다.해당 날짜 및 식사 시간대에 활성화된 복약 일정(Active Schedule)이 존재하는
Senior식별자만 가져오도록 Repository에 별도 쿼리를 추가하거나, 페이징/배치 단위 조회를 적용하는 것을 권장합니다.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/main/java/com/piuda/callcare/domain/calllog/service/CallReminderCommandService.java` around lines 51 - 61, Update sendDueFirstCalls to avoid loading every Senior via seniorRepository.findAll(); add and use a repository query that retrieves only Senior identifiers with active medication schedules for the target date and each CALL_MEAL_TIMES value, or process those results in bounded pages/batches. Preserve the existing phone-number validation and sendFirstCallIfDue behavior for the filtered seniors.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In
`@src/main/java/com/piuda/callcare/domain/calllog/client/CoolVoiceCallSender.java`:
- Around line 47-52: Update the send flow around CoolVoiceCallSender’s
messageService.send(message) so a single recipient failure is isolated and does
not stop processing subsequent recipients. Handle the failure at the per-call
boundary used by CallReminderCommandService or the scheduler loop, skip only the
failed call, and preserve continuation for remaining targets instead of
propagating an uncaught IllegalStateException.
In
`@src/main/java/com/piuda/callcare/domain/calllog/service/CallReminderCommandService.java`:
- Around line 101-109: Update the callback result handling for NO_ANSWER and
FAILED so it only records the outcome via markNoAnswer or markFailed and allows
the retry scheduler flow to proceed. Remove the immediate notifyGuardian calls
and ensure isNotified remains eligible for retry processing; notify the guardian
only after all configured retries fail.
- Around line 30-34: CallReminderCommandService applies a class-level
transaction around external API calls, risking prolonged DB connection usage and
batch-wide rollbacks. Remove class-level `@Transactional`, keep sendDueFirstCalls
and notifyGuardiansForUnansweredCalls outside transactions, and add narrowly
scoped `@Transactional` annotations only to state-update methods such as
applyCallResult or save.
In `@src/main/java/com/piuda/callcare/global/config/SecurityConfig.java`:
- Line 46: Update the webhook handling for POST requests matched by
"/api/calllogs/webhook/**" so requests are authenticated with SOLAPI’s HMAC
signature, such as validating the Authorization header, before processing any
messageId or call-status changes. Add the verification in the webhook controller
or an appropriate security filter, and reject missing or invalid signatures
while preserving access for valid SOLAPI requests.
---
Nitpick comments:
In `@src/main/java/com/piuda/callcare/domain/calllog/entity/CallLog.java`:
- Around line 82-96: Update incrementRetryCount and markRetried in CallLog to
handle a null retryCount defensively before incrementing, treating null as zero
so both retry paths increment safely without unboxing a null Integer.
In
`@src/main/java/com/piuda/callcare/domain/calllog/service/CallReminderCommandService.java`:
- Around line 51-61: Update sendDueFirstCalls to avoid loading every Senior via
seniorRepository.findAll(); add and use a repository query that retrieves only
Senior identifiers with active medication schedules for the target date and each
CALL_MEAL_TIMES value, or process those results in bounded pages/batches.
Preserve the existing phone-number validation and sendFirstCallIfDue behavior
for the filtered seniors.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro Plus
Run ID: cfd15d8b-4d86-41dd-ac98-12358a92b1ab
📒 Files selected for processing (17)
build.gradlesrc/main/java/com/piuda/callcare/CallcareApplication.javasrc/main/java/com/piuda/callcare/domain/calllog/client/CoolVoiceCallSender.javasrc/main/java/com/piuda/callcare/domain/calllog/client/MockVoiceCallSender.javasrc/main/java/com/piuda/callcare/domain/calllog/client/VoiceCallSender.javasrc/main/java/com/piuda/callcare/domain/calllog/controller/CallResultWebhookController.javasrc/main/java/com/piuda/callcare/domain/calllog/controller/CallTestController.javasrc/main/java/com/piuda/callcare/domain/calllog/dto/request/CallResultWebhookRequest.javasrc/main/java/com/piuda/callcare/domain/calllog/entity/CallLog.javasrc/main/java/com/piuda/callcare/domain/calllog/enums/CallStatus.javasrc/main/java/com/piuda/callcare/domain/calllog/repository/CallLogRepository.javasrc/main/java/com/piuda/callcare/domain/calllog/scheduler/CallReminderScheduler.javasrc/main/java/com/piuda/callcare/domain/calllog/service/CallReminderCommandService.javasrc/main/java/com/piuda/callcare/domain/home/service/query/HomeCardQueryService.javasrc/main/java/com/piuda/callcare/domain/medication/repository/MedicationScheduleRepository.javasrc/main/java/com/piuda/callcare/global/config/SecurityConfig.javasrc/test/java/com/piuda/callcare/domain/home/service/query/HomeCardQueryServiceTest.java
| .requestMatchers(HttpMethod.GET, "/api/conflicts/**").permitAll() | ||
| .requestMatchers(HttpMethod.POST, "/api/conflicts/**").permitAll() | ||
| .requestMatchers(HttpMethod.POST, "/api/ocr/**").permitAll() | ||
| .requestMatchers(HttpMethod.POST, "/api/calllogs/webhook/**").permitAll() |
There was a problem hiding this comment.
🔒 Security & Privacy | 🟠 Major | 🏗️ Heavy lift
웹훅 엔드포인트의 무결성 검증이 누락되어 있습니다.
해당 웹훅 API를 permitAll()로 완전 개방할 경우, 악의적인 사용자가 임의의 messageId를 포함한 페이로드를 전송하여 통화 상태(수신/미수신 등)를 조작할 수 있는 보안 취약점이 발생합니다.
SOLAPI에서 제공하는 서명 검증(HMAC) 방식(예: Authorization 헤더 검증)을 컨트롤러나 필터에 추가하여, 실제 SOLAPI 서버에서 보낸 요청인지 검증하는 것을 권장합니다.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/main/java/com/piuda/callcare/global/config/SecurityConfig.java` at line
46, Update the webhook handling for POST requests matched by
"/api/calllogs/webhook/**" so requests are authenticated with SOLAPI’s HMAC
signature, such as validating the Authorization header, before processing any
messageId or call-status changes. Add the verification in the webhook controller
or an appropriate security filter, and reject missing or invalid signatures
while preserving access for valid SOLAPI requests.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
🔍️ 작업 내용
✨ 상세 설명
1. 복용 시간 기반 전화 알림
VoiceCallSender인터페이스를 통해 SOLAPI 음성 전화 발신 로직을 추상화하여 설계2. SOLAPI 기본 재시도 정책 반영
3. 문자(SMS) Fallback 처리
CoolSmsSender등)를 재사용하여 별도의 문자 발송 로직 중복 없이 처리4. 홈 화면 복약 카드 연동
5. 로컬 테스트용 트리거 API
6. CallLog 기반 중복 발신 방지 및 상태 관리
🛠️ 추후 리팩토링 및 고도화 계획
📸 스크린샷 (선택)
💬 리뷰 요구사항
Summary by CodeRabbit
Summary by CodeRabbit