Conversation
- 프록시 환경에서도 클라이언트 IP를 추출할 수 있도록 로직 변경 및 설정 적용
- 알람 울릴 때 푸시 알림 구현 - 알람 울리기 1시간 전 푸시 알림에 route 데이터 추가
|
Warning Rate limit exceeded@strongmhk has exceeded the limit for the number of commits or files that can be reviewed per hour. Please wait 13 minutes and 29 seconds before requesting another review. ⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. 📒 Files selected for processing (3)
Walkthrough알람 울림 API/스케줄러 및 울림 관련 FCM 전송이 추가되었습니다. DTO·매퍼·리포지토리·서비스·컨트롤러·엔티티 확장과 오류코드·테스트 보강, 보안/로깅/프로필·설정 변경이 포함되어 있습니다. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
actor Client
participant API as AlarmController
participant UC as AlarmUseCase
participant Svc as AlarmCommandServiceImpl
participant Repo as Repositories
participant Mapper as AlarmMapper
Client->>API: POST /api/alarms/{alarmId}/ring (JWT)
API->>UC: ringAlarm(memberId, alarmId)
UC->>Svc: ringAlarm(memberId, alarmId)
Svc->>Repo: load Alarm(alarmId) & verify owner
Svc->>Repo: find latest AlarmOccurrence(DeactivateType.NONE)
alt missing / deactivated / not time
Svc-->>API: throw ApplicationException (ALARM_OCCURRENCE_NOT_FOUND / ALREADY_DEACTIVATED / NOT_ALARM_TIME)
API-->>Client: Error response
else ok
Svc->>Repo: occurrence.ring() (increment count)
Svc->>Mapper: mapToAlarmRingingLogEntity(occurrence, ringIndex, now)
Mapper-->>Svc: AlarmRingingLogEntity
Svc->>Repo: save AlarmRingingLogEntity
Svc-->>API: return
API-->>Client: 200 OK
end
sequenceDiagram
autonumber
participant Sch as AlarmRingingNotificationScheduler
participant Qry as AlarmQueryService
participant Redis as RedisService
participant FCM as FcmService
Sch->>Qry: getRingingNotificationTargets()
Qry-->>Sch: List<RingingPushInfo>
alt empty
Sch-->>Sch: return
else targets exist
loop targets
Sch->>Redis: getFcmTokens(memberId)
Sch-->>Sch: build RingingPushTargetDto(token, alarmId, memberId)
end
alt no tokens
Sch-->>Sch: return
else
Sch->>FCM: sendRingingNotifications(targets)
FCM-->>Sch: result processed / invalid tokens pruned
end
end
sequenceDiagram
autonumber
participant Sch2 as AlarmReminderScheduler
participant FCM as FcmService
Sch2->>FCM: sendPreAlarmNotifications()
note right of FCM #88ccff: data includes route="MAIN_VIEW"
FCM-->>Sch2: multicast result logged
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Assessment against linked issues
Assessment against linked issues: Out-of-scope changes
Possibly related PRs
✨ Finishing Touches
🧪 Generate unit tests
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. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. CodeRabbit Commands (Invoked using PR/Issue comments)Type Other keywords and placeholders
Status, Documentation and Community
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 37
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (11)
src/main/java/akuma/whiplash/global/config/security/jwt/JwtAuthenticationFilter.java (4)
24-27: 컴포넌트 스캔 대상 제거 권장(@component 삭제) — SecurityConfig에서 Bean으로 관리
SecurityConfig에서 Bean으로 등록하여 사용하는 방식으로 통일하면, 여기의@Component는 제거하는 것이 깔끔합니다. 두 경로를 동시에 쓰면 인스턴스가 중복됩니다.-@Component @RequiredArgsConstructor @Slf4j public class JwtAuthenticationFilter extends OncePerRequestFilter {
88-90: 모든 요청마다 INFO 레벨 URI 로깅은 과다합니다 — DEBUG로 내리거나 로컬에서만 로깅운영 환경에서 매 요청 INFO 로그는 신호대잡음비가 나빠집니다. 아래 둘 중 하나를 권장합니다.
- 단순히 DEBUG로 레벨 하향
- 혹은 로컬 프로파일에서만 출력
- log.info("Request URI = {}", request.getRequestURI()); + if (env.acceptsProfiles(Profiles.of("local"))) { + log.info("Request URI = {}", request.getRequestURI()); + } else { + log.debug("Request URI = {}", request.getRequestURI()); + }
101-110: IP 추출 로직 중복 제거: LogUtils.clientIp(...) 재사용본 PR에서
LogUtils에 Forwarded 우선 파서가 추가되었습니다. 여기서도 동일 경로를 사용해 일관성을 보장하세요.- private String getClientIp(HttpServletRequest request) { - String ip = request.getHeader("X-Forwarded-For"); - - if (ip != null && !ip.isEmpty() && !"unknown".equalsIgnoreCase(ip)) { - // 여러 IP가 있을 수 있으므로 첫 번째 꺼냄 - return ip.split(",")[0]; - } - - return request.getRemoteAddr(); // 프록시 없을 경우 - } + private String getClientIp(HttpServletRequest request) { + return akuma.whiplash.global.log.LogUtils.clientIp(request); + }
95-97:Authorization파싱 안전성 강화: 공백 없는 “Bearer” 처리 시 AIOOBE 가능현재 구현은
"Bearer"만 온 경우split(" ")[1]에서ArrayIndexOutOfBoundsException이 발생할 수 있습니다. 다음과 같이 접두어 길이를 기준으로 잘라내고, 대소문자를 무시하세요.- if (StringUtils.hasText(authorizationHeader) && authorizationHeader.startsWith(BEARER)) { - return authorizationHeader.split(" ")[1]; - } + String prefix = BEARER + " "; + if (StringUtils.hasText(authorizationHeader) + && authorizationHeader.regionMatches(true, 0, prefix, 0, prefix.length())) { + String candidate = authorizationHeader.substring(prefix.length()).trim(); + return StringUtils.hasText(candidate) ? candidate : null; + }src/test/java/akuma/whiplash/common/config/IntegrationTest.java (1)
18-21: Redis 테스트 간 상태 격리를 위한 Redis 키 정리 로직 추가 필요현재
RedisContainerInitializer를 통해 Redis 컨테이너를 기동하더라도, 전역@Transactional만으로는 Redis에 저장된 키가 자동으로 롤백되지 않아 테스트 간 키 오염이 발생할 수 있습니다.확인 결과, 테스트 코드 내에 Redis 키를 정리(
flushAll,flushdb등)하는 유틸이나 리스너가 존재하지 않습니다:
- RedisContainerInitializer 클래스만 확인됨 (src/test/java/akuma/whiplash/common/config/RedisContainerInitializer.java:9)
- 테스트 코드 전반에
flushAll,flushdb,deleteAll,removeAll호출 없음따라서, 아래 중 하나 이상의 방안을 반드시 적용해 주세요:
- 공통
@AfterEach훅에서redisService.flushAll()호출을 통해 테스트 종료 시점에 Redis 키를 전부 삭제- Spring TestExecutionListeners를 활용해 Redis 전용 정리 리스너 추가
import org.springframework.test.context.ContextConfiguration;
import org.springframework.test.context.TestExecutionListeners;
import org.springframework.test.context.support.DependencyInjectionTestExecutionListener;
@ContextConfiguration(initializers = {
MySQLContainerInitializer.class,
RedisContainerInitializer.class
})@TestExecutionListeners(
listeners = {
DependencyInjectionTestExecutionListener.class,RedisFlushTestExecutionListener.class},
mergeMode = TestExecutionListeners.MergeMode.MERGE_WITH_DEFAULTS
)
- (옵션) 테스트 시작/종료 시점에 현재 Redis 키 개수를 로깅하여 누수 여부를 추적 가능하도록 설정 위 중 최소 하나라도 반영하여, 테스트 간 Redis 상태 누수 문제를 해결해 주시기 바랍니다. </blockquote></details> <details> <summary>src/main/java/akuma/whiplash/domains/alarm/application/scheduler/AlarmReminderScheduler.java (4)</summary><blockquote> `37-40`: **시간 윈도우가 분 단위로 2분 중첩되어 중복 조회 가능** - `now.truncatedTo(MINUTES)` 기준 `[+59, +61]` 분 윈도우는 매 실행 간 1~2분 중첩됩니다. 성공 건은 `reminderSent`로 억제되지만, 멀티 인스턴스/지연 시 중복 전송 위험이 있습니다. - 제안 - 윈도우를 `[now+60m, now+60m+1m)`로 축소. - 또는 “클레임-후-전송” 패턴 도입: DB에서 `reminder_sent=false` 행을 `UPDATE ... SET reminder_sent=true WHERE id IN (...) AND reminder_sent=false`로 선점하고, 영향 받은 id에게만 전송. 참고 패치(윈도우 축소): ```diff - LocalDateTime now = LocalDateTime.now().truncatedTo(ChronoUnit.MINUTES); - LocalDateTime windowStart = now.plusMinutes(59); - LocalDateTime windowEnd = now.plusMinutes(61); + LocalDateTime now = LocalDateTime.now().truncatedTo(ChronoUnit.MINUTES); + LocalDateTime windowStart = now.plusHours(1); + LocalDateTime windowEnd = windowStart.plusMinutes(1);분산 환경 중복 방지(개념 스케치):
- 1단계:
claimReminderTargets(start,end)가reminder_sent=false인 id를 선점(UPDATE ... AND reminder_sent=false) 후 영향받은 id 반환.- 2단계: 선점한 id에 한해 FCM 전송 → 실패 id는 롤백 테이블/상태로 재시도 큐에 적재.
48-59: Redis 토큰 조회 N+1 최적화 제안
- 동일
memberId가 다수의 발생건에 등장하면redisService.getFcmTokens()가 중복 호출됩니다.다음처럼 멤버 단위로 토큰을 캐시하면 호출 수를 줄일 수 있습니다.
- List<PushTargetDto> targets = infos.stream() - .flatMap(info -> - redisService.getFcmTokens(info.memberId()).stream() - .map(token -> PushTargetDto.builder() - .token(token) - .address(info.address()) - .memberId(info.memberId()) - .occurrenceId(info.occurrenceId()) - .build() - ) - ) - .toList(); // <-- 여기서 한 번만 호출 + Map<Long, List<String>> tokensByMember = infos.stream() + .map(OccurrencePushInfo::memberId) + .distinct() + .collect(java.util.stream.Collectors.toMap( + mid -> mid, mid -> redisService.getFcmTokens(mid) + )); + + List<PushTargetDto> targets = infos.stream() + .flatMap(info -> tokensByMember.getOrDefault(info.memberId(), List.of()).stream() + .map(token -> PushTargetDto.builder() + .token(token) + .address(info.address()) + .memberId(info.memberId()) + .occurrenceId(info.occurrenceId()) + .build() + ) + ) + .toList();
64-69: 전송 실패 예외에 대한 보호막 부족
fcmService.sendBulkNotification(targets)에서 예외가 발생하면 이후 정리/로깅 없이 스케줄이 중단됩니다.전송 구간만 국소
try-catch로 감싸고 실패 카운터/메트릭을 남겨 주세요.- FcmSendResult result = fcmService.sendBulkNotification(targets); + FcmSendResult result; + try { + result = fcmService.sendBulkNotification(targets); + } catch (Exception e) { + log.error("[AlarmReminderScheduler] FCM 벌크 전송 실패: targets={}, cause={}", targets.size(), e.toString(), e); + return; + }
31-59: 동적data.route매핑 보강 필요현재 구현을 보면, FcmService.sendBulkNotification(…)에서 FCM 페이로드의
"route"를 모두 고정된 문자열("MAIN_VIEW")로 설정하고 있습니다.
하지만 AlarmReminderScheduler에서는PushTargetDto에address만 담아 넘기고, 실제 네비게이션 경로(route)로는 사용되지 않고 있어 다음과 같은 불일치가 발생합니다.
FcmService.sendBulkNotification (src/main/java/akuma/whiplash/infrastructure/firebase/FcmService.java: 약 82행)
.addData("route", "MAIN_VIEW")— 고정 라우트FcmService.sendRingingNotifications (src/main/java/akuma/whiplash/infrastructure/firebase/FcmService.java: 약 134행)
.addData("route", "ALARM_LINGING_VIEW")— 고정 라우트AlarmReminderScheduler.sendPreAlarmNotifications (src/main/java/akuma/whiplash/domains/alarm/application/scheduler/AlarmReminderScheduler.java: 40–56행)
PushTargetDto.builder()…address(info.address())…— address만 전달위 상태로는 PR 목표인 “
data.route추가”가 실질적으로 반영되지 않았습니다.
따라서 다음과 같이 수정을 제안드립니다.• PushTargetDto에
route필드를 추가
• Scheduler에서 Route를 조합한 값을 Builder에 설정
• FcmService.sendBulkNotification/sendRingingNotifications에서 고정문자열 대신dto.getRoute()사용예시 패치:
// AlarmReminderScheduler.java - .address(info.address()) + .address(info.address()) + .route("/alarms/" + info.occurrenceId() + "?addr=" + + URLEncoder.encode(info.address(), StandardCharsets.UTF_8)) // FcmService.java (sendBulkNotification) - .addData("route", "MAIN_VIEW") + .addData("route", dto.getRoute())위와 같이 동적 경로를 매핑하도록 보강해주세요.
src/main/java/akuma/whiplash/infrastructure/firebase/FcmService.java (2)
200-204: 문서와 구현 불일치: 무효 토큰 ‘즉시 제거’가 구현되지 않음상단 주석(“무효 토큰은 즉시 Redis에서 제거”)과 달리 bulk 경로에선 invalidTokens만 수집합니다. Redis 즉시 제거를 일관되게 수행하도록 보강하세요.
- if (isTokenInvalid(fme)) { - invalidTokens.add(dto.token()); - } + if (isTokenInvalid(fme)) { + invalidTokens.add(dto.token()); + // align with ringing path behavior (immediate cleanup) + redisService.removeInvalidToken(dto.memberId(), dto.token()); + }
50-55: 주석의 ‘data-only’ 설명과 iOS 설정 불일치현재 iOS는 ApsAlert(표시 알림)으로 전송됩니다. “data-only”와 충돌하니 주석을 실제 동작에 맞게 정정하거나, 진짜 data-only(배경 푸시: apns-push-type=background + contentAvailable)로 바꾸세요. 의도 확인 필요.
주석 수정안(동작에 맞춤):
- * 데이터 전용(data-only) 멀티캐스트 전송 - * - Notification payload 제거, data만 사용 - * - Android priority=HIGH, iOS apns-priority=10 + content-available=1 + * 사전 알림 멀티캐스트 전송 + * - Android: data-only, priority=HIGH + * - iOS: APNs alert payload 사용(apns-push-type=alert, apns-priority=10)또는 진짜 data-only로 전환(의도 시):
- iOS: apns-push-type=background, Aps.contentAvailable(true), alert 제거.
- 앱 포어그라운드/백그라운드 처리 전략 확인 필요.
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (29)
build.gradle(1 hunks)src/main/java/akuma/whiplash/domains/alarm/application/dto/etc/RingingPushInfo.java(1 hunks)src/main/java/akuma/whiplash/domains/alarm/application/dto/etc/RingingPushTargetDto.java(1 hunks)src/main/java/akuma/whiplash/domains/alarm/application/mapper/AlarmMapper.java(2 hunks)src/main/java/akuma/whiplash/domains/alarm/application/scheduler/AlarmReminderScheduler.java(2 hunks)src/main/java/akuma/whiplash/domains/alarm/application/scheduler/AlarmRingingNotificationScheduler.java(1 hunks)src/main/java/akuma/whiplash/domains/alarm/application/usecase/AlarmUseCase.java(1 hunks)src/main/java/akuma/whiplash/domains/alarm/domain/service/AlarmCommandService.java(1 hunks)src/main/java/akuma/whiplash/domains/alarm/domain/service/AlarmCommandServiceImpl.java(2 hunks)src/main/java/akuma/whiplash/domains/alarm/domain/service/AlarmQueryService.java(2 hunks)src/main/java/akuma/whiplash/domains/alarm/domain/service/AlarmQueryServiceImpl.java(2 hunks)src/main/java/akuma/whiplash/domains/alarm/exception/AlarmErrorCode.java(1 hunks)src/main/java/akuma/whiplash/domains/alarm/persistence/entity/AlarmOccurrenceEntity.java(1 hunks)src/main/java/akuma/whiplash/domains/alarm/persistence/entity/AlarmRingingLogEntity.java(1 hunks)src/main/java/akuma/whiplash/domains/alarm/persistence/repository/AlarmOccurrenceRepository.java(2 hunks)src/main/java/akuma/whiplash/domains/alarm/presentation/AlarmController.java(2 hunks)src/main/java/akuma/whiplash/global/config/security/SecurityConfig.java(3 hunks)src/main/java/akuma/whiplash/global/config/security/jwt/JwtAuthenticationFilter.java(3 hunks)src/main/java/akuma/whiplash/global/log/LogUtils.java(1 hunks)src/main/java/akuma/whiplash/infrastructure/firebase/FcmService.java(7 hunks)src/main/resources/application.yml(1 hunks)src/main/resources/sentry.yml(1 hunks)src/test/java/akuma/whiplash/common/config/IntegrationTest.java(1 hunks)src/test/java/akuma/whiplash/common/fixture/AlarmOccurrenceFixture.java(1 hunks)src/test/java/akuma/whiplash/domains/alarm/domain/service/AlarmCommandServiceImplTest.java(0 hunks)src/test/java/akuma/whiplash/domains/alarm/domain/service/AlarmCommandServiceTest.java(1 hunks)src/test/java/akuma/whiplash/domains/alarm/persistence/repository/AlarmRepositoryTest.java(2 hunks)src/test/java/akuma/whiplash/domains/alarm/presentation/AlarmControllerIntegrationTest.java(3 hunks)src/test/java/akuma/whiplash/domains/alarm/presentation/AlarmControllerTest.java(5 hunks)
💤 Files with no reviewable changes (1)
- src/test/java/akuma/whiplash/domains/alarm/domain/service/AlarmCommandServiceImplTest.java
🧰 Additional context used
📓 Path-based instructions (8)
**/*{Controller,Service,Repository}.java
📄 CodeRabbit inference engine (AGENT.md)
Controller/Service는
getUser,createUser,removeUser등 동사 기반으로 명명하고, Repository는findByUser,insertUser,deleteUser처럼 명확히 구분합니다.
Files:
src/main/java/akuma/whiplash/domains/alarm/domain/service/AlarmQueryService.javasrc/main/java/akuma/whiplash/domains/alarm/domain/service/AlarmCommandService.javasrc/main/java/akuma/whiplash/domains/alarm/persistence/repository/AlarmOccurrenceRepository.javasrc/main/java/akuma/whiplash/domains/alarm/presentation/AlarmController.javasrc/main/java/akuma/whiplash/infrastructure/firebase/FcmService.java
**/*.java
📄 CodeRabbit inference engine (AGENT.md)
**/*.java: 세미콜론 누락, 들여쓰기 불일치, import 순서 등의 형식 오류를 검토합니다.
사용되지 않는 변수나 주석은 제거를 제안합니다.
한 줄이 너무 길거나 중첩이 과도한 경우 구조 개선을 권장합니다.
Files:
src/main/java/akuma/whiplash/domains/alarm/domain/service/AlarmQueryService.javasrc/main/java/akuma/whiplash/domains/alarm/domain/service/AlarmCommandService.javasrc/main/java/akuma/whiplash/domains/alarm/application/usecase/AlarmUseCase.javasrc/main/java/akuma/whiplash/global/config/security/SecurityConfig.javasrc/main/java/akuma/whiplash/domains/alarm/persistence/entity/AlarmOccurrenceEntity.javasrc/main/java/akuma/whiplash/domains/alarm/application/scheduler/AlarmReminderScheduler.javasrc/main/java/akuma/whiplash/domains/alarm/application/mapper/AlarmMapper.javasrc/test/java/akuma/whiplash/common/config/IntegrationTest.javasrc/main/java/akuma/whiplash/domains/alarm/application/scheduler/AlarmRingingNotificationScheduler.javasrc/main/java/akuma/whiplash/domains/alarm/application/dto/etc/RingingPushInfo.javasrc/test/java/akuma/whiplash/domains/alarm/presentation/AlarmControllerIntegrationTest.javasrc/test/java/akuma/whiplash/domains/alarm/domain/service/AlarmCommandServiceTest.javasrc/main/java/akuma/whiplash/domains/alarm/domain/service/AlarmQueryServiceImpl.javasrc/main/java/akuma/whiplash/global/log/LogUtils.javasrc/test/java/akuma/whiplash/domains/alarm/persistence/repository/AlarmRepositoryTest.javasrc/main/java/akuma/whiplash/domains/alarm/exception/AlarmErrorCode.javasrc/main/java/akuma/whiplash/domains/alarm/persistence/repository/AlarmOccurrenceRepository.javasrc/main/java/akuma/whiplash/domains/alarm/application/dto/etc/RingingPushTargetDto.javasrc/test/java/akuma/whiplash/domains/alarm/presentation/AlarmControllerTest.javasrc/main/java/akuma/whiplash/domains/alarm/persistence/entity/AlarmRingingLogEntity.javasrc/main/java/akuma/whiplash/domains/alarm/presentation/AlarmController.javasrc/main/java/akuma/whiplash/domains/alarm/domain/service/AlarmCommandServiceImpl.javasrc/test/java/akuma/whiplash/common/fixture/AlarmOccurrenceFixture.javasrc/main/java/akuma/whiplash/global/config/security/jwt/JwtAuthenticationFilter.javasrc/main/java/akuma/whiplash/infrastructure/firebase/FcmService.java
**/*{Entity,Dto}.java
📄 CodeRabbit inference engine (AGENT.md)
모든 PK는
alarmId,userId처럼 도메인명을 포함하여 명명합니다.
Files:
src/main/java/akuma/whiplash/domains/alarm/persistence/entity/AlarmOccurrenceEntity.javasrc/main/java/akuma/whiplash/domains/alarm/application/dto/etc/RingingPushTargetDto.javasrc/main/java/akuma/whiplash/domains/alarm/persistence/entity/AlarmRingingLogEntity.java
**/*Test.java
📄 CodeRabbit inference engine (AGENT.md)
**/*Test.java: 테스트명은@DisplayName을 사용하고, "음료를 추가하면 주문 목록에 담긴다"와 같이 문장형으로 작성합니다.
테스트는Given-When-Then구조로 작성되었는지 확인합니다.
테스트 설명은 도메인 관점에서 작성되었는지 확인합니다.
Files:
src/test/java/akuma/whiplash/common/config/IntegrationTest.javasrc/test/java/akuma/whiplash/domains/alarm/presentation/AlarmControllerIntegrationTest.javasrc/test/java/akuma/whiplash/domains/alarm/domain/service/AlarmCommandServiceTest.javasrc/test/java/akuma/whiplash/domains/alarm/persistence/repository/AlarmRepositoryTest.javasrc/test/java/akuma/whiplash/domains/alarm/presentation/AlarmControllerTest.java
src/test/**
⚙️ CodeRabbit configuration file
src/test/**: - @DisplayName 문장형, 도메인 용어 사용.
- Given/When/Then BDD 스타일 유지.
Files:
src/test/java/akuma/whiplash/common/config/IntegrationTest.javasrc/test/java/akuma/whiplash/domains/alarm/presentation/AlarmControllerIntegrationTest.javasrc/test/java/akuma/whiplash/domains/alarm/domain/service/AlarmCommandServiceTest.javasrc/test/java/akuma/whiplash/domains/alarm/persistence/repository/AlarmRepositoryTest.javasrc/test/java/akuma/whiplash/domains/alarm/presentation/AlarmControllerTest.javasrc/test/java/akuma/whiplash/common/fixture/AlarmOccurrenceFixture.java
**/*ErrorCode.java
📄 CodeRabbit inference engine (AGENT.md)
**/*ErrorCode.java: 에러 코드는Domain_x0n형태로 작성되었는지 확인합니다.
각 도메인에서 발생 가능한 에러는@CustomErrorCodes로 명시합니다.
message는~입니다.형태의 문장으로 작성되어야 합니다.
Files:
src/main/java/akuma/whiplash/domains/alarm/exception/AlarmErrorCode.java
**/*{ErrorCode,Exception}.java
📄 CodeRabbit inference engine (AGENT.md)
허용된 HttpStatus만 사용합니다: 400, 401, 403, 404, 409
Files:
src/main/java/akuma/whiplash/domains/alarm/exception/AlarmErrorCode.java
**/*Dto.java
📄 CodeRabbit inference engine (AGENT.md)
**/*Dto.java: 요청/응답 객체의 필드명은 DB 컬럼명을 기준으로 하며, 필요시userNickname,adminNickname등 식별자를 붙입니다.
리스트 응답 필드는alarms,tickets처럼 복수형 도메인명으로 작성합니다.
Enum은.name()값 그대로, 날짜는 ISO_LOCAL_DATE 또는 ISO_LOCAL_DATE_TIME 형식으로 반환합니다.
Files:
src/main/java/akuma/whiplash/domains/alarm/application/dto/etc/RingingPushTargetDto.java
🧬 Code graph analysis (18)
src/main/resources/sentry.yml (1)
src/main/java/akuma/whiplash/global/config/sentry/SentryConfig.java (2)
Configuration(7-32)Bean(10-31)
src/main/java/akuma/whiplash/domains/alarm/domain/service/AlarmQueryService.java (1)
src/main/java/akuma/whiplash/domains/alarm/persistence/repository/AlarmRingingLogRepository.java (1)
AlarmRingingLogRepository(9-18)
src/main/java/akuma/whiplash/global/config/security/SecurityConfig.java (2)
src/main/java/akuma/whiplash/global/config/security/jwt/JwtUtils.java (1)
Component(36-132)src/main/java/akuma/whiplash/global/config/security/jwt/JwtProvider.java (1)
Component(23-91)
src/main/java/akuma/whiplash/domains/alarm/application/scheduler/AlarmReminderScheduler.java (1)
src/main/java/akuma/whiplash/domains/alarm/application/scheduler/AlarmOccurrenceBatchScheduler.java (2)
Slf4j(10-47)Scheduled(22-27)
src/test/java/akuma/whiplash/common/config/IntegrationTest.java (3)
src/test/java/akuma/whiplash/common/config/PersistenceTest.java (1)
Target(14-28)src/main/java/akuma/whiplash/global/config/jpa/JpaAuditingConfig.java (1)
JpaAuditingConfig(6-10)src/test/java/akuma/whiplash/common/config/MySQLContainerInitializer.java (1)
MySQLContainerInitializer(9-40)
src/main/java/akuma/whiplash/domains/alarm/application/scheduler/AlarmRingingNotificationScheduler.java (3)
src/main/java/akuma/whiplash/domains/alarm/application/scheduler/AlarmReminderScheduler.java (1)
Slf4j(21-85)src/main/java/akuma/whiplash/infrastructure/firebase/FcmService.java (1)
Slf4j(31-265)src/main/java/akuma/whiplash/domains/alarm/domain/service/AlarmQueryServiceImpl.java (1)
Slf4j(33-186)
src/main/java/akuma/whiplash/domains/alarm/application/dto/etc/RingingPushInfo.java (2)
src/main/java/akuma/whiplash/domains/alarm/application/dto/etc/PushTargetDto.java (1)
Builder(5-13)src/main/java/akuma/whiplash/domains/alarm/application/dto/etc/OccurrencePushInfo.java (1)
Builder(5-12)
src/test/java/akuma/whiplash/domains/alarm/presentation/AlarmControllerIntegrationTest.java (3)
src/main/java/akuma/whiplash/domains/alarm/application/mapper/AlarmMapper.java (1)
AlarmMapper(23-109)src/test/java/akuma/whiplash/domains/alarm/persistence/repository/AlarmRepositoryTest.java (1)
Nested(25-52)src/test/java/akuma/whiplash/domains/alarm/presentation/AlarmControllerTest.java (2)
Nested(96-156)Nested(158-193)
src/test/java/akuma/whiplash/domains/alarm/domain/service/AlarmCommandServiceTest.java (1)
src/test/java/akuma/whiplash/domains/alarm/domain/service/AlarmCommandServiceImplTest.java (5)
ExtendWith(29-96)DisplayName(52-74)alarmCommandService(93-93)DisplayName(76-95)BeforeEach(46-50)
src/main/java/akuma/whiplash/global/log/LogUtils.java (2)
src/main/java/akuma/whiplash/global/log/HttpLoggingFilter.java (2)
Override(49-157)Slf4j(29-197)src/main/java/akuma/whiplash/global/log/MethodLoggingAspect.java (1)
Slf4j(20-79)
src/main/java/akuma/whiplash/domains/alarm/exception/AlarmErrorCode.java (3)
src/main/java/akuma/whiplash/domains/alarm/application/dto/request/AlarmRegisterRequest.java (1)
Schema(12-50)src/main/java/akuma/whiplash/global/response/code/CommonErrorCode.java (1)
CommonErrorCode(7-22)src/main/java/akuma/whiplash/domains/alarm/persistence/entity/AlarmEntity.java (1)
Entity(28-66)
src/main/java/akuma/whiplash/domains/alarm/application/dto/etc/RingingPushTargetDto.java (1)
src/main/java/akuma/whiplash/domains/alarm/application/dto/etc/PushTargetDto.java (1)
Builder(5-13)
src/test/java/akuma/whiplash/domains/alarm/presentation/AlarmControllerTest.java (1)
src/test/java/akuma/whiplash/domains/alarm/presentation/AlarmControllerIntegrationTest.java (2)
Nested(51-107)Nested(110-203)
src/main/java/akuma/whiplash/domains/alarm/persistence/entity/AlarmRingingLogEntity.java (5)
src/main/java/akuma/whiplash/domains/alarm/persistence/entity/AlarmOccurrenceEntity.java (1)
Entity(27-94)src/main/java/akuma/whiplash/domains/alarm/persistence/entity/AlarmOffLogEntity.java (1)
Entity(19-38)src/main/java/akuma/whiplash/domains/alarm/persistence/entity/AlarmEntity.java (1)
Entity(28-66)src/main/java/akuma/whiplash/global/entity/BaseTimeEntity.java (1)
BaseTimeEntity(15-29)src/main/java/akuma/whiplash/domains/alarm/persistence/repository/AlarmRingingLogRepository.java (1)
AlarmRingingLogRepository(9-18)
src/main/java/akuma/whiplash/domains/alarm/domain/service/AlarmCommandServiceImpl.java (1)
src/main/java/akuma/whiplash/domains/alarm/application/mapper/AlarmMapper.java (1)
AlarmMapper(23-109)
src/test/java/akuma/whiplash/common/fixture/AlarmOccurrenceFixture.java (1)
src/main/java/akuma/whiplash/domains/alarm/persistence/entity/AlarmEntity.java (1)
Entity(28-66)
src/main/java/akuma/whiplash/global/config/security/jwt/JwtAuthenticationFilter.java (2)
src/main/java/akuma/whiplash/domains/auth/presentation/AuthController.java (1)
Slf4j(24-72)src/main/java/akuma/whiplash/global/config/security/jwt/JwtUtils.java (1)
Component(36-132)
src/main/java/akuma/whiplash/infrastructure/firebase/FcmService.java (1)
src/main/java/akuma/whiplash/domains/alarm/application/dto/etc/PushTargetDto.java (1)
Builder(5-13)
🔇 Additional comments (16)
src/test/java/akuma/whiplash/domains/alarm/persistence/repository/AlarmRepositoryTest.java (3)
13-13: JUnit 5 @nested import 추가 적절중첩 테스트 도입을 위한 필수 import 제대로 추가되었습니다. 문제 없습니다.
25-27: @nested로 use-case 단위 그룹핑 — 가독성과 테스트 의도 명확성 향상그룹 설명용 @DisplayName도 함께 적용되어 테스트 탐색성이 좋아졌습니다.
25-27: @PersistenceTest 트랜잭션 롤백/DB 격리 확인 완료
src/test/java/akuma/whiplash/common/config/PersistenceTest.java를 확인한 결과, 해당 어노테이션에 다음 설정이 포함되어 있습니다.
@DataJpaTest: 기본적으로 각 테스트 메서드마다 트랜잭션을 시작하고, 실행 후 자동으로 롤백합니다.@AutoConfigureTestDatabase(replace = AutoConfigureTestDatabase.Replace.NONE): 실제 MySQL 컨테이너를 사용해 테스트하며, 테스트 간 데이터 충돌을 방지합니다.따라서 Nested 테스트를 포함한 모든 테스트 메서드에서 독립적인 트랜잭션 격리 및 롤백이 보장되므로, 추가 조치는 필요하지 않습니다.
build.gradle (1)
35-37: JUnit Platform 런처 의존성 유지 권장
Gradle 8.14.2 환경에서는 Gradle이 번들된junit-platform-launcher(v1.8.2)를 기본으로 사용하며, JUnit 5의 최신 버전(v5.12.0 이상)과 호환되지 않을 수 있습니다. 따라서 명시적으로testRuntimeOnly 'org.junit.platform:junit-platform-launcher'의존성을 유지하여 런처 버전 불일치로 인한 테스트 실행 실패를 예방하는 것이 좋습니다. 현 상태 그대로 유지해 주세요. (dev.to)
src/main/java/akuma/whiplash/domains/alarm/persistence/entity/AlarmRingingLogEntity.java (2)
17-23: @SuperBuilder 추가는 매퍼/상속 구조에 잘 맞습니다
BaseTimeEntity도@SuperBuilder이므로 빌더 체인이 안전하게 작동합니다. 매퍼에서의 생성 편의성 향상에 동의합니다.
36-41: 리팩터링 제안:ringIndex를 primitiveint로 변경하고ringedAt에 non-null 제약 추가아래와 같이 수정하면 NPE 가능성을 제거하고, 비즈니스 상 항상 기록되는
ringedAt에도 무결성 제약을 부여할 수 있습니다.
- 대상 파일:
src/main/java/akuma/whiplash/domains/alarm/persistence/entity/AlarmRingingLogEntity.java- 변경 사항
ringIndex타입을Integer→int로 변경ringedAt컬럼에nullable = false추가--- a/src/main/java/akuma/whiplash/domains/alarm/persistence/entity/AlarmRingingLogEntity.java +++ b/src/main/java/akuma/whiplash/domains/alarm/persistence/entity/AlarmRingingLogEntity.java @@ - @Column(name = "ring_index", nullable = false) - private Integer ringIndex; + @Column(name = "ring_index", nullable = false) + private int ringIndex; - @Column(name = "ringed_at") - private LocalDateTime ringedAt; + @Column(name = "ringed_at", nullable = false) + private LocalDateTime ringedAt;추가 검토 사항:
- 데이터베이스 스키마 마이그레이션 시 운영 환경 영향(DDL 적용 시점·롤백 전략) 반드시 검토 요망
- 대량 로그 조회 성능을 위해
alarm_occurrence_id, ring_index복합 인덱스 도입 검토 (페이징·정렬 최적화)src/main/java/akuma/whiplash/domains/alarm/exception/AlarmErrorCode.java (1)
22-22: 에러코드 가이드라인 준수 및 테스트 검증 완료
- AlarmErrorCode.java에서
- NOT_ALARM_TIME(HttpStatus.BAD_REQUEST, "ALARM_011", "알람이 울릴 시간이 아닙니다.") 정의가 도메인 접두어·일련번호·메시지 형식(
~입니다.) 가이드라인에 부합함- AlarmController.java에서
@CustomErrorCodes(alarmErrorCodes = {..., NOT_ALARM_TIME}, ...)로 NOT_ALARM_TIME 상수 포함 확인됨 (122행)- AlarmControllerTest.java에서
fail_notAlarmTime테스트 케이스가 ringAlarm 예외(ApplicationException.from(AlarmErrorCode.NOT_ALARM_TIME)) 상황 후 400 Bad Request를 검증함 (lines 178–192)src/test/java/akuma/whiplash/common/config/IntegrationTest.java (1)
15-15: 검증 결과: AFTER_COMMIT 이벤트 리스너 미사용
코드베이스 전체를 검사한 결과@TransactionalEventListener애노테이션과TransactionPhase(AFTER_COMMIT) 사용처가 전혀 발견되지 않았습니다.
- 통합 테스트 클래스에 전역
@Transactional을 적용해도 이벤트 리스너나 비동기 흐름 검증에 영향이 없습니다.- 별도
@Commit/NOT_SUPPORTED오버라이드나 강제 커밋 패턴 적용은 불필요합니다.기존 리뷰 코멘트는 이 코드베이스에는 적용되지 않으므로 무시해주세요.
Likely an incorrect or invalid review comment.
src/main/java/akuma/whiplash/domains/alarm/application/dto/etc/RingingPushTargetDto.java (1)
5-10: 레코드 + @builder 조합 일관성 양호
PushTargetDto와 필드 네이밍/스타일 일치합니다.- PK 필드 네이밍(
alarmId,memberId)이 가이드와 부합합니다.src/main/java/akuma/whiplash/domains/alarm/domain/service/AlarmQueryService.java (1)
14-14: API 시그니처 및 구현 검증 완료
getRingingNotificationTargets()네이밍과 역할이 도메인 용어와 일치하며, 호출 의도가 명확합니다.- AlarmQueryServiceImpl.java(184)에서
DeactivateType.NONE파라미터로findRingingNotificationTargets를 호출하고 있으며,
AlarmOccurrenceRepository.java(130–133)의 JPA 쿼리에서o.alarmRinging = true와o.deactivateType = :status필터가 모두 적용된 것을 확인했습니다.src/main/java/akuma/whiplash/domains/alarm/application/mapper/AlarmMapper.java (1)
83-93: 울림 로그 매핑 메서드 추가 LGTM. ringIndex/타임소스 일관성만 점검
ringIndex가 Occurrence의ringingCount와 동일 의미로 1부터 증가하는지(첫 울림=1) 확인해 주세요. 서비스에서 증가·기록 순서가 뒤바뀌면 불일치가 납니다.ringedAt은 서버 기준 시각을 사용해야 재현성과 신뢰도가 확보됩니다.src/test/java/akuma/whiplash/domains/alarm/domain/service/AlarmCommandServiceTest.java (1)
112-175: ringAlarm 단위 테스트 복원 및 findById 스텁 추가 필요현재
AlarmCommandServiceTest에서ringAlarm흐름 검증을 주석 처리함으로써
- 시간 검증
- 카운트 증가
- 로그 저장
등 핵심 로직이 누락된 상태입니다. 또한,
AlarmCommandService.ringAlarm내부에서 호출되는 권한 검증(alarmRepository.findById) 스텁이 없어, 실제로는 권한 검증 단계에서Optional.empty()가 반환되어 예외가 발생할 수 있습니다.다음 사항을 반영해 테스트를 수정해 주세요:
- 주석 처리된
RingAlarmTest클래스 복원alarmRepository.findById(alarm.getId())스텁 추가AlarmFixture.ALARM_05.toMockEntity()는member정보를 포함하므로 별도 member 연결 코드는 불필요합니다 (.toMockEntity()내부에서member.toEntity()로 매핑됨)-/* @Nested + @Nested @DisplayName("ringAlarm - 알람 울림") class RingAlarmTest { @Test @DisplayName("알람이 울리면 울림 정보가 갱신되고 로그가 저장된다") void success() { // given MemberEntity member = MemberFixture.MEMBER_5.toMockEntity(); AlarmEntity alarm = AlarmFixture.ALARM_05.toMockEntity(); + given(alarmRepository.findById(alarm.getId())) + .willReturn(Optional.of(alarm)); AlarmOccurrenceEntity occurrence = AlarmOccurrenceEntity.builder() .id(1L) .alarm(alarm) @@ given( alarmOccurrenceRepository .findTopByAlarmIdAndDeactivateTypeInOrderByDateDescTimeDesc( eq(alarm.getId()), anyList() ) ).willReturn(Optional.of(occurrence)); // when alarmCommandService.ringAlarm(member.getId(), alarm.getId()); // then verify(alarmRingingLogRepository).save(any()); assertThat(occurrence.isAlarmRinging()).isTrue(); assertThat(occurrence.getRingingCount()).isEqualTo(1); } @Test @DisplayName("알람 시간이 되지 않았으면 예외를 던진다") void fail_notAlarmTime() { // given MemberEntity member = MemberFixture.MEMBER_5.toMockEntity(); AlarmEntity alarm = AlarmFixture.ALARM_05.toMockEntity(); + given(alarmRepository.findById(alarm.getId())) + .willReturn(Optional.of(alarm)); AlarmOccurrenceEntity occurrence = AlarmOccurrenceEntity.builder() .id(1L) @@ given( alarmOccurrenceRepository .findTopByAlarmIdAndDeactivateTypeInOrderByDateDescTimeDesc( eq(alarm.getId()), anyList() ) ).willReturn(Optional.of(occurrence)); // when & then assertThatThrownBy(() -> alarmCommandService.ringAlarm(member.getId(), alarm.getId())) .isInstanceOf(ApplicationException.class); } } - }*/ +}위 수정으로
ringAlarm엔드투엔드 흐름이 검사되고- 권한 검증 단계의
findById호출이 정상 동작하며AlarmFixture.toMockEntity()내 member 매핑 덕분에 별도 setter/빌더 호출 없이도 소유자 정보가 일치함을 보장할 수 있습니다.src/main/java/akuma/whiplash/domains/alarm/presentation/AlarmController.java (1)
66-66: 엔드포인트 설명 업데이트 적절도메인 관점에서 사용 흐름이 더 명확해졌습니다. 스웨거 문서 가독성 향상에 도움이 됩니다.
src/main/java/akuma/whiplash/domains/alarm/application/scheduler/AlarmRingingNotificationScheduler.java (1)
33-41: 안내:getFcmTokens는 null을 반환하지 않습니다. 불필요한 null 방어 로직 제거 요망
RedisService.getFcmTokens(Long memberId)구현을 확인한 결과, 내부에서Optional.ofNullable(redisTemplate.opsForSet().members(key)) .orElse(Set.of());로 null을 빈
Set으로 대체하고 있어, 항상 non-null을 보장합니다.
따라서 기존 제안된 null 체크(Optional.ofNullable(...).orElseGet(List::of))는 불필요합니다.Likely an incorrect or invalid review comment.
src/test/java/akuma/whiplash/domains/alarm/presentation/AlarmControllerTest.java (2)
96-156: 알람 등록 슬라이스 테스트 구성 적절
- @DisplayName 문장형, 도메인 용어 사용
- Given/When/Then 구조 주석 구분
- useCase 상호작용 검증까지 포함
162-175: 알람 울림 성공 케이스 검증 적절보안 필터를 배제한 상황에서 SecurityContext 세팅을 통해
@AuthenticationPrincipal바인딩을 검증하는 접근이 명확합니다.
| @Builder | ||
| public record RingingPushInfo( | ||
| Long alarmId, | ||
| Long memberId | ||
| ) { | ||
| } No newline at end of file |
There was a problem hiding this comment.
💡 Verification agent
🧩 Analysis chain
Record에 @builder는 동작하나 과한 표면적 복잡도—실사용 여부 확인 후 제거 권고
이 DTO는 JPQL new ...RingingPushInfo(a.id, m.id)로 생성됩니다. 이 경우 빌더는 사용되지 않아 노이즈가 됩니다. 실제 빌더 사용처가 없다면 제거해 단순화하세요.
아래 스크립트로 빌더 사용 여부를 확인해 주세요.
빌더 사용이 없다면 다음 변경을 권장합니다:
-package akuma.whiplash.domains.alarm.application.dto.etc;
-
-import lombok.Builder;
-
-@Builder
+package akuma.whiplash.domains.alarm.application.dto.etc;
public record RingingPushInfo(
Long alarmId,
Long memberId
) {
}🏁 Script executed:
#!/bin/bash
# RingingPushInfo 빌더/정적 생성자 사용처 점검
rg -nP 'RingingPushInfo\.builder\(|new\s+RingingPushInfo\(' -C2Length of output: 65
아래 스크립트로 RingingPushInfo의 빌더 및 인스턴스화 사용처를 재검증해 주세요.
출력 결과가 없다면 빌더 사용이 없다고 판단할 수 있습니다.
🏁 Script executed:
#!/bin/bash
# RingingPushInfo 빌더/정적 생성자/JPQL 사용처 재검증
# 1) Lombok 빌더 호출 검토
rg -n 'RingingPushInfo\.builder' -C2
# 2) 코드 내 직접 생성자 호출 검토
rg -n 'new\s\+RingingPushInfo' -C2
# 3) 메서드 호출 형태 검토 (정적 팩토리 메서드 등)
rg -n 'RingingPushInfo\(' -C2
# 4) JPQL 문자열 내 사용 검토
rg -n '"new\s+.*RingingPushInfo' -C2Length of output: 1321
RingingPushInfo DTO에서 @builder 제거 권고
RingingPushInfo의 Lombok 빌더 호출은 코드베이스 어디에서도 사용되지 않고, JPQL new akuma.whiplash.domains.alarm.application.dto.etc.RingingPushInfo(a.id, m.id)만 활용되고 있습니다. 불필요한 복잡도를 줄이기 위해 @Builder와 관련 import를 제거하세요.
-
확인된 사용처
• AlarmOccurrenceRepository.java (라인 128):SELECT new akuma.whiplash.domains.alarm.application.dto.etc.RingingPushInfo(a.id, m.id) -
변경 제안 (src/main/java/akuma/whiplash/domains/alarm/application/dto/etc/RingingPushInfo.java)
-package akuma.whiplash.domains.alarm.application.dto.etc;
-
-import lombok.Builder;
-
-@Builder
+package akuma.whiplash.domains.alarm.application.dto.etc;
public record RingingPushInfo(
Long alarmId,
Long memberId
) {
}🤖 Prompt for AI Agents
In
src/main/java/akuma/whiplash/domains/alarm/application/dto/etc/RingingPushInfo.java
around lines 5 to 10, the Lombok @Builder annotation (and any related import) is
unused and unnecessary because the DTO is instantiated via JPQL using the record
canonical constructor; remove the @Builder annotation and its import(s) so the
file only declares the record RingingPushInfo(Long alarmId, Long memberId)
without Lombok builder-related code.
| // 매 분 마다 실행 | ||
| @Scheduled(cron = "0 * * * * *") | ||
| public void sendPreAlarmNotifications() { |
There was a problem hiding this comment.
🛠️ Refactor suggestion
크론에 타임존 누락 — 배치 스케줄과 동일한 Asia/Seoul로 고정 권장
- 다른 배치(
AlarmOccurrenceBatchScheduler)는zone = "Asia/Seoul"을 명시합니다. 운영 서버 TZ가 다를 경우 알림 타이밍이 어긋날 수 있습니다.
다음과 같이 수정해 주세요.
- // 매 분 마다 실행
- @Scheduled(cron = "0 * * * * *")
+ // 매 분 마다 실행 (KST 기준)
+ @Scheduled(cron = "0 * * * * *", zone = "Asia/Seoul")
public void sendPreAlarmNotifications() {📝 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.
| // 매 분 마다 실행 | |
| @Scheduled(cron = "0 * * * * *") | |
| public void sendPreAlarmNotifications() { | |
| // 매 분 마다 실행 (KST 기준) | |
| @Scheduled(cron = "0 * * * * *", zone = "Asia/Seoul") | |
| public void sendPreAlarmNotifications() { |
🤖 Prompt for AI Agents
In
src/main/java/akuma/whiplash/domains/alarm/application/scheduler/AlarmReminderScheduler.java
around lines 31 to 33, the @Scheduled annotation is missing an explicit timezone
which can cause timing drift versus other batches; update the annotation to
specify zone = "Asia/Seoul" to match AlarmOccurrenceBatchScheduler so the method
runs on the intended Korea time zone.
| @Scheduled(fixedRate = 10000) | ||
| public void sendRingingAlarmNotifications() { | ||
| log.info("[AlarmRingingNotificationScheduler.sendRingingAlarmNotifications] 알람 울림 푸시 알림 전송 스케줄러 시작"); |
There was a problem hiding this comment.
🧹 Nitpick (assertive)
스케줄 중복 실행/중첩 방지: 고정 레이트 대신 지연 기반 또는 외부 설정 사용 고려
fixedRate = 10000은 작업이 10초를 넘기는 경우 실행 중첩 위험이 있습니다.
- 단일 인스턴스:
fixedDelayString = "${scheduler.ringing.fixedDelay:10000}"로 외부 설정 + 이전 실행 종료 기준 재시작 권장 - 다중 인스턴스(클러스터): ShedLock/Redis 기반 분산 락으로 동시 실행 차단 권장
🤖 Prompt for AI Agents
In
src/main/java/akuma/whiplash/domains/alarm/application/scheduler/AlarmRingingNotificationScheduler.java
around lines 24-26, the scheduler currently uses @Scheduled(fixedRate = 10000)
which can cause overlapping executions and is unsafe in clustered deployments;
change to a delay-based, externally configurable schedule and add distributed
locking: replace fixedRate with fixedDelayString =
"${scheduler.ringing.fixedDelay:10000}" so the next run waits for the previous
to finish and the delay is configurable, and integrate a distributed lock (e.g.,
ShedLock or a Redis-based lock) around the sendRingingAlarmNotifications
execution so only one instance runs the job in a multi-node cluster.
| List<RingingPushTargetDto> targets = infos.stream() | ||
| .flatMap(info -> redisService.getFcmTokens(info.memberId()).stream() | ||
| .map(token -> RingingPushTargetDto.builder() | ||
| .token(token) | ||
| .alarmId(info.alarmId()) | ||
| .memberId(info.memberId()) | ||
| .build())) | ||
| .toList(); | ||
|
|
||
| if (targets.isEmpty()) { | ||
| return; | ||
| } | ||
|
|
||
| fcmService.sendRingingNotifications(targets); | ||
| } finally { |
There was a problem hiding this comment.
💡 Verification agent
🧩 Analysis chain
중복 푸시 방지(Idempotency) 설계 필요
10초마다 같은 대상을 반복 수집/전송할 수 있습니다. 사전알림은 reminderSent로 중복을 방지하는데, 울림 푸시에도 유사한 제어가 필요합니다. 권장 방안:
- Redis에
(alarmId or occurrenceId) → lastSentRingIndex저장하여 신규 ringIndex에만 발송 - 또는 DB에
ring_push_sent_index(또는 로그 기준MAX(ring_index))를 참고하여 미전송 건만 전송 - 실패/성공에 대한 재시도/클린업 정책 정의
의사 코드 예시:
for each info:
last = redis.get("ring:last:"+info.alarmId) default -1
current = queryOrLoadCurrentRingIndex(info.alarmId)
if current > last:
send; on success redis.set("ring:last:"+info.alarmId, current, ttl)
현 구현에서 중복 방지 로직이 존재하는지 확인하려면:
🏁 Script executed:
#!/bin/bash
rg -nP --type=java 'ring(ing)?(Push|Notification)|ringIndex|dedup|lastSent' -C2Length of output: 4626
중복 푸시(Idempotency) 처리 로직 추가 필요
현재 AlarmRingingNotificationScheduler(33–47행)에서는
infos를 순회하며 Redis에서 FCM 토큰만 조회하고,FcmService.dedupByToken으로 같은 토큰 내 중복만 제거한 뒤fcmService.sendRingingNotifications(targets)를 호출합니다.
이때 동일한 alarmId와 같은 ringIndex에 대해서는 전송 여부를 기록·확인하는 로직이 없어, 10초 주기로 스케줄이 실행될 때마다 중복 푸시가 발생할 수 있습니다.
AlarmRingingLogEntity(ringIndex)를 저장·조회하는 기능은 있으나, 스케줄러에 해당 조회나 비교 로직이 전혀 포함되어 있지 않습니다.
아래 중 한 가지 방안을 반드시 도입해 중복 전송을 방지해 주시기 바랍니다:
- Redis에
(alarmId:ringIndex) → 전송 완료 플래그를 저장하고, 새로 증가한ringIndex에만 푸시 전송 - 또는 DB의
AlarmRingingLogEntity에서 마지막 전송된ringIndex를 조회해 미전송 건만 선별 - 전송 실패/성공에 따른 재시도 및 캐시(TTL)·클린업 정책 정의
예시 의사 코드:
for (info : infos) {
key = "ring:last:" + info.alarmId;
lastSent = redisService.get(key).orElse(-1);
current = info.ringIndex();
if (current > lastSent) {
targets = buildTargets(info);
fcmService.sendRingingNotifications(targets);
redisService.set(key, current, ttlSeconds);
}
}
점검 대상:
- src/main/java/akuma/whiplash/domains/alarm/application/scheduler/AlarmRingingNotificationScheduler.java (33–47행)
- RedisService 호출부, FcmService.dedupByToken(토큰 중복 제거 전용)
🤖 Prompt for AI Agents
src/main/java/akuma/whiplash/domains/alarm/application/scheduler/AlarmRingingNotificationScheduler.java
(lines 33–47): currently you build targets and call
fcmService.sendRingingNotifications without checking prior sends, causing
duplicate pushes on each scheduler run; change the logic to perform idempotency
by checking and updating a per-alarm last-sent ringIndex (prefer Redis for fast
checks): for each info compute a Redis key like "ring:last:{alarmId}", read
lastSent (default -1), if info.ringIndex() <= lastSent skip building/sending for
that alarm, otherwise build targets, call fcmService.dedupByToken as before,
call fcmService.sendRingingNotifications, and only on successful send update the
Redis key to the new ringIndex with a sensible TTL; ensure atomicity (use SET
with NX or a short lock) to avoid race conditions, handle send failures by not
updating the key and logging/retrying per existing retry policy, and keep
RedisService and FcmService calls consistent with current interfaces.
| public void ringAlarm(Long memberId, Long alarmId) { | ||
| alarmCommandService.ringAlarm(memberId, alarmId); | ||
| } |
There was a problem hiding this comment.
🧹 Nitpick (assertive)
ringAlarm 유스케이스 추가는 타당. 멱등성·서버시간 사용 여부 확인 필요
- 동일 알람에 대해 클라이언트가 재시도(네트워크 재전송, 더블탭 등)할 수 있으므로, Service 레벨에서 같은 발생건(occurrence)·같은 분/초 내 중복 호출을 멱등 처리하는지 확인해 주세요.
- alarmOff는
clientNow를 받지만 ring은 받지 않습니다. 울림 로그의ringedAt이 반드시 서버 시간(신뢰원)으로 기록되는지 확인 바랍니다. 클라이언트 조작 가능 시간을 신뢰하지 않는 설계가 바람직합니다.
| // when | ||
| List<AlarmEntity> alarms = alarmRepository.findAllByMemberId(999L); | ||
|
|
There was a problem hiding this comment.
🧹 Nitpick (assertive)
매직 넘버(999L) 지양 + BDD 주석 일관성 보강
테스트 데이터가 누적되면 999L가 실제 ID와 충돌할 수 있습니다. 충돌 가능성이 사실상 0에 가까운 값(Long.MAX_VALUE) 등으로 대체하고, Given 블록을 명시해 BDD 일관성을 유지하세요.
- // when
- List<AlarmEntity> alarms = alarmRepository.findAllByMemberId(999L);
+ // given
+ long nonExistingMemberId = Long.MAX_VALUE;
+ // when
+ List<AlarmEntity> alarms = alarmRepository.findAllByMemberId(nonExistingMemberId);📝 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.
| // when | |
| List<AlarmEntity> alarms = alarmRepository.findAllByMemberId(999L); | |
| // given | |
| long nonExistingMemberId = Long.MAX_VALUE; | |
| // when | |
| List<AlarmEntity> alarms = alarmRepository.findAllByMemberId(nonExistingMemberId); |
🤖 Prompt for AI Agents
In
src/test/java/akuma/whiplash/domains/alarm/persistence/repository/AlarmRepositoryTest.java
around lines 46 to 48, replace the magic literal 999L with a clearly named
constant or use Long.MAX_VALUE (or another near-impossible ID) to avoid future
collisions, and add an explicit "Given" BDD comment/block before the action to
keep test structure consistent; update any variable declarations accordingly so
the test uses the constant (e.g., NON_EXISTENT_MEMBER_ID) and insert a "//
Given" comment and any necessary setup lines above the "// when" line.
| String accessToken = jwtProvider.generateAccessToken(member.getId(), member.getRole(), "mock_device_id"); | ||
|
|
||
| // when | ||
| mockMvc.perform(post("/api/alarms") | ||
| .header(HttpHeaders.AUTHORIZATION, "Bearer " + accessToken) | ||
| .contentType(MediaType.APPLICATION_JSON) | ||
| .content(objectMapper.writeValueAsString(request))) |
There was a problem hiding this comment.
🧹 Nitpick (assertive)
중복 헤더/토큰 생성 로직 헬퍼로 추출
Bearer 토큰 헤더 세팅이 반복됩니다. 가독성/유지보수성을 위해 공통 헬퍼 메서드로 추출하세요.
예시(클래스 내부 유틸):
private String bearer(Long memberId, String role) {
return "Bearer " + jwtProvider.generateAccessToken(memberId, role, "mock_device_id");
}사용 예:
.header(HttpHeaders.AUTHORIZATION, bearer(member.getId(), member.getRole()))Also applies to: 121-126, 138-141, 153-156, 168-171, 183-186, 199-201
🤖 Prompt for AI Agents
In
src/test/java/akuma/whiplash/domains/alarm/presentation/AlarmControllerIntegrationTest.java
around lines 70-76 (and similarly at 121-126, 138-141, 153-156, 168-171,
183-186, 199-201), the test repeatedly constructs the "Bearer " +
jwtProvider.generateAccessToken(...) header; extract a private helper method
inside the test class (e.g., bearer(memberId, role)) that returns the full
Authorization header value by calling jwtProvider.generateAccessToken with
"mock_device_id", and replace all inline header constructions with
.header(HttpHeaders.AUTHORIZATION, bearer(member.getId(), member.getRole())) to
eliminate duplication and improve readability.
| @Test | ||
| @DisplayName("실패: 존재하지 않는 알람 ID로 요청하면 404 응답을 반환한다") | ||
| void fail_alarmNotFound() throws Exception { | ||
| // given | ||
| MemberEntity member = memberRepository.save(MemberFixture.MEMBER_2.toEntity()); | ||
| String accessToken = jwtProvider.generateAccessToken(member.getId(), member.getRole(), "mock_device_id"); | ||
| long nonExistentAlarmId = 999L; | ||
|
|
||
| // when & then | ||
| mockMvc.perform(post("/api/alarms/{alarmId}/ring", nonExistentAlarmId) | ||
| .header(HttpHeaders.AUTHORIZATION, "Bearer " + accessToken)) | ||
| .andExpect(status().isNotFound()); | ||
| } |
There was a problem hiding this comment.
🧹 Nitpick (assertive)
에러 응답 본문 검증 보강 제안
상태코드만 확인합니다. 도메인 에러코드/메시지(jsonPath)까지 검증하면 회귀 방지에 유리합니다. 실제 응답 스키마/코드 값을 확인해 반영하세요.
예시(스키마 확인 후 적용):
mockMvc.perform(post("/api/alarms/{alarmId}/ring", nonExistentAlarmId)
.header(HttpHeaders.AUTHORIZATION, "Bearer " + accessToken))
.andExpect(status().isNotFound())
.andExpect(jsonPath("$.code").value("ALARM_NOT_FOUND"))
.andExpect(jsonPath("$.message").exists());🤖 Prompt for AI Agents
In
src/test/java/akuma/whiplash/domains/alarm/presentation/AlarmControllerIntegrationTest.java
around lines 129-141, the test only asserts the 404 status; enhance it to also
verify the error response body by adding jsonPath assertions for the domain
error code and message (e.g.,
.andExpect(jsonPath("$.code").value("ALARM_NOT_FOUND")) and
.andExpect(jsonPath("$.message").exists()) or use the exact message constant
from your error enum), appending these to the existing
mockMvc.perform(...).andExpect(...) chain so the test fails if the API returns
the wrong error payload schema or values.
| @DisplayName("실패: 이미 비활성화된 알람이면 400 응답을 반환한다") | ||
| void fail_alreadyDeactivated() throws Exception { | ||
| // given |
There was a problem hiding this comment.
테스트 설명과 검증 불일치(400 vs 404)
DisplayName은 400을 말하지만 실제 검증은 404입니다. 현재 동작에 맞춰 DisplayName을 404로 맞추고, 추후 API가 400으로 바뀌면 검증과 함께 수정하세요.
- @DisplayName("실패: 이미 비활성화된 알람이면 400 응답을 반환한다")
+ @DisplayName("실패: 이미 비활성화된 알람이면 404 응답을 반환한다")
...
- .andExpect(status().isNotFound()); // TODO: isBadRequest로 검증해야함.
+ .andExpect(status().isNotFound()); // TODO: API가 400으로 변경되면 isBadRequest로 수정Also applies to: 185-185
🤖 Prompt for AI Agents
In
src/test/java/akuma/whiplash/domains/alarm/presentation/AlarmControllerIntegrationTest.java
around lines 174-176 (also applies to line 185), the DisplayName text says the
test expects a 400 response but the assertion verifies a 404; update the
DisplayName(s) to state 404 to match the actual assertion and behavior (or if
you intend to change behavior to 400 later, update both the assertion and
DisplayName together).
| @Test | ||
| @DisplayName("실패: 알람 시간이 아니면 400을 반환한다") | ||
| void fail_notAlarmTime() throws Exception { | ||
|
|
||
| setSecurityContext(buildContextFromFixture(MEMBER_4)); | ||
| // given | ||
| setSecurityContext(buildContextFromFixture(MEMBER_3)); | ||
|
|
||
| when(alarmUseCase.createAlarm(any(AlarmRegisterRequest.class), anyLong())) | ||
| .thenThrow(ApplicationException.from(MemberErrorCode.MEMBER_NOT_FOUND)); | ||
| // when | ||
| doThrow(ApplicationException.from(AlarmErrorCode.NOT_ALARM_TIME)) | ||
| .when(alarmUseCase) | ||
| .ringAlarm(eq(MEMBER_3.getId()), eq(1L)); | ||
|
|
||
| // when & then | ||
| mockMvc.perform(post("/api/alarms") | ||
| .contentType(MediaType.APPLICATION_JSON) | ||
| .content(objectMapper.writeValueAsString(request))) | ||
| .andExpect(status().isNotFound()); | ||
| // then | ||
| mockMvc.perform(post("/api/alarms/{alarmId}/ring", 1L)) | ||
| .andExpect(status().isBadRequest()); | ||
| } |
There was a problem hiding this comment.
🧹 Nitpick (assertive)
에러 케이스 확장 제안: Not Found/Forbidden도 함께 검증
현재 NOT_ALARM_TIME(400)만 검증합니다. 컨트롤러의 @CustomErrorCodes에 맞춰 다음 케이스도 추가하면 커버리지가 좋아집니다.
ALARM_NOT_FOUND→ 404PERMISSION_DENIED→ 403
예시 추가:
@@ class RingAlarmTest {
+ @Test
+ @DisplayName("실패: 존재하지 않는 알람이면 404를 반환한다")
+ void fail_alarmNotFound() throws Exception {
+ setSecurityContext(buildContextFromFixture(MEMBER_3));
+ doThrow(ApplicationException.from(AlarmErrorCode.ALARM_NOT_FOUND))
+ .when(alarmUseCase)
+ .ringAlarm(eq(MEMBER_3.getId()), eq(1L));
+ mockMvc.perform(post("/api/alarms/{alarmId}/ring", 1L))
+ .andExpect(status().isNotFound());
+ }
+
+ @Test
+ @DisplayName("실패: 다른 사용자의 알람이면 403을 반환한다")
+ void fail_permissionDenied() throws Exception {
+ setSecurityContext(buildContextFromFixture(MEMBER_3));
+ doThrow(ApplicationException.from(akuma.whiplash.domains.auth.exception.AuthErrorCode.PERMISSION_DENIED))
+ .when(alarmUseCase)
+ .ringAlarm(eq(MEMBER_3.getId()), eq(1L));
+ mockMvc.perform(post("/api/alarms/{alarmId}/ring", 1L))
+ .andExpect(status().isForbidden());
+ }추가 케이스 반영 후 테스트 통과를 확인해 주세요. 또한 통합 테스트의 ALREADY_DEACTIVATED 기대 상태(현재 TODO)를 400으로 정렬하는 것도 권장합니다.
📝 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.
| @Test | |
| @DisplayName("실패: 알람 시간이 아니면 400을 반환한다") | |
| void fail_notAlarmTime() throws Exception { | |
| setSecurityContext(buildContextFromFixture(MEMBER_4)); | |
| // given | |
| setSecurityContext(buildContextFromFixture(MEMBER_3)); | |
| when(alarmUseCase.createAlarm(any(AlarmRegisterRequest.class), anyLong())) | |
| .thenThrow(ApplicationException.from(MemberErrorCode.MEMBER_NOT_FOUND)); | |
| // when | |
| doThrow(ApplicationException.from(AlarmErrorCode.NOT_ALARM_TIME)) | |
| .when(alarmUseCase) | |
| .ringAlarm(eq(MEMBER_3.getId()), eq(1L)); | |
| // when & then | |
| mockMvc.perform(post("/api/alarms") | |
| .contentType(MediaType.APPLICATION_JSON) | |
| .content(objectMapper.writeValueAsString(request))) | |
| .andExpect(status().isNotFound()); | |
| // then | |
| mockMvc.perform(post("/api/alarms/{alarmId}/ring", 1L)) | |
| .andExpect(status().isBadRequest()); | |
| } | |
| @Test | |
| @DisplayName("실패: 알람 시간이 아니면 400을 반환한다") | |
| void fail_notAlarmTime() throws Exception { | |
| // given | |
| setSecurityContext(buildContextFromFixture(MEMBER_3)); | |
| // when | |
| doThrow(ApplicationException.from(AlarmErrorCode.NOT_ALARM_TIME)) | |
| .when(alarmUseCase) | |
| .ringAlarm(eq(MEMBER_3.getId()), eq(1L)); | |
| // then | |
| mockMvc.perform(post("/api/alarms/{alarmId}/ring", 1L)) | |
| .andExpect(status().isBadRequest()); | |
| } | |
| @Test | |
| @DisplayName("실패: 존재하지 않는 알람이면 404를 반환한다") | |
| void fail_alarmNotFound() throws Exception { | |
| // given | |
| setSecurityContext(buildContextFromFixture(MEMBER_3)); | |
| // when | |
| doThrow(ApplicationException.from(AlarmErrorCode.ALARM_NOT_FOUND)) | |
| .when(alarmUseCase) | |
| .ringAlarm(eq(MEMBER_3.getId()), eq(1L)); | |
| // then | |
| mockMvc.perform(post("/api/alarms/{alarmId}/ring", 1L)) | |
| .andExpect(status().isNotFound()); | |
| } | |
| @Test | |
| @DisplayName("실패: 다른 사용자의 알람이면 403을 반환한다") | |
| void fail_permissionDenied() throws Exception { | |
| // given | |
| setSecurityContext(buildContextFromFixture(MEMBER_3)); | |
| // when | |
| doThrow(ApplicationException.from(akuma.whiplash.domains.auth.exception.AuthErrorCode.PERMISSION_DENIED)) | |
| .when(alarmUseCase) | |
| .ringAlarm(eq(MEMBER_3.getId()), eq(1L)); | |
| // then | |
| mockMvc.perform(post("/api/alarms/{alarmId}/ring", 1L)) | |
| .andExpect(status().isForbidden()); | |
| } |
- Repository 호출시 이미 DeactivateType.NONE으로 필터링하므로 불필요한 검증 코드 삭제 - 현재 시간과 scheduledDateTime 비교시 요청 시간과 미세한 차이가 생길 수 있어 now 변수로 대체
- 푸시 알림 수신 동의를 하지 않은 회원에게는 푸시 알림을 전송하지 않도록 findRingingNotificationTargets에서 필터링 조건에 m.pushNotificationPolicy = true추가
- FCM 푸시 전송 실패시 로깅되는 토큰에 마스킹 추가 - route 필드에 들어가는 값 오타 수정 ALARM_LINGING_VIEW -> ALARM_RINGING_VIEW
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (3)
src/main/java/akuma/whiplash/domains/alarm/domain/service/AlarmCommandServiceImpl.java (3)
189-191: 외부 I/O(Google Sheets) 호출이 트랜잭션 내부에서 수행됨 — 트랜잭션 시간 증가 및 롤백/재시도 부작용 위험
@Transactional범위 내부에서 네트워크 I/O를 수행하면 DB 락 보유 시간이 늘고, 실패 시 재시도/부분 실패에 따른 불일치가 발생할 수 있습니다. 삭제 완료 후 커밋이 보장된 시점(AFTER_COMMIT)으로 옮기세요.서비스 내 교체 diff(이벤트 발행으로 대체):
- // 2. 삭제 사유를 Google Sheets에 로그로 기록 - logDeleteReason(alarm.getAlarmPurpose(), reason); + // 2. 삭제 사유는 커밋 이후 비동기 로깅 + eventPublisher.publishEvent(new AlarmDeletedEvent(alarm.getId(), alarm.getAlarmPurpose(), reason));서비스 외 추가 코드(참고):
// 필드 주입 private final ApplicationEventPublisher eventPublisher; // 이벤트 public record AlarmDeletedEvent(Long alarmId, String alarmPurpose, String reason) {} // 리스너 @Component @RequiredArgsConstructor public class AlarmDeletionLogListener { private final AlarmCommandServiceImpl svc; // or a dedicated SheetLogService @TransactionalEventListener(phase = TransactionPhase.AFTER_COMMIT) public void on(AlarmDeletedEvent e) { svc.logDeleteReason(e.alarmPurpose(), e.reason()); } }
193-197: N+1 삭제 루프 → 벌크 삭제로 전환 권장발생 내역 개수만큼
deleteAllByAlarmOccurrenceId호출은 불필요한 라운드트립을 유발합니다. 벌크 삭제 메서드로 교체하세요.적용 diff:
- for (AlarmOccurrenceEntity occ : occurrences) { - alarmRingingLogRepository.deleteAllByAlarmOccurrenceId(occ.getId()); - } + var occurrenceIds = occurrences.stream().map(AlarmOccurrenceEntity::getId).toList(); + alarmRingingLogRepository.deleteAllByAlarmOccurrenceIdIn(occurrenceIds);리포지토리에 아래 시그니처 추가(참고):
void deleteAllByAlarmOccurrenceIdIn(Collection<Long> occurrenceIds);또는 DB FK에 ON DELETE CASCADE/
orphanRemoval = true를 고려해 서비스 단 삭제 로직을 단순화할 수 있습니다.
236-243: AlarmEntity latitude/longitude null 안전성 확인 완료 (자동 언박싱 NPE 발생 불가)검증 결과
- AlarmEntity 클래스에서
latitude와longitude필드에@Column(nullable = false)가 선언되어 있어, JPA 매핑 및 DB 레벨에서 null 저장이 차단됩니다. 따라서alarm.getLatitude()/getLongitude()호출 시 auto-unboxing으로 인한 NPE 발생 가능성은 없습니다.선택적 리팩터 제안
필드가 향후 변경되거나 DB 제약이 흐려지는 상황을 대비해 방어적(null) 체크를 추가하려면 아래 예시와 같이 구현할 수 있습니다.- boolean isInRange = isWithinDistance( + if (alarm.getLatitude() == null || alarm.getLongitude() == null) { + // TODO: 적절한 도메인 에러 코드(INVALID_ALARM_LOCATION 등)로 교체 + throw ApplicationException.from(INVALID_ALARM_LOCATION); + } + boolean isInRange = isWithinDistance( alarm.getLatitude(), alarm.getLongitude(), request.latitude(), request.longitude(), CHECKIN_RADIUS_METERS);
♻️ Duplicate comments (2)
src/main/java/akuma/whiplash/domains/alarm/domain/service/AlarmCommandServiceImpl.java (2)
114-114: validAlarmOwner 인자 순서 일관성 유지 필요본 파일 내 다른 호출부는
(memberId, alarm.getMember().getId())순서를 사용합니다. 여기만 반대로 되어 있어 가독성을 해칩니다(기능상 문제는 없으나 통일 권장).적용 diff:
- validAlarmOwner(findAlarm.getMember().getId(), memberId); + validAlarmOwner(memberId, findAlarm.getMember().getId());다음 스크립트로 레포 전체 호출부를 점검하세요.
#!/bin/bash # validAlarmOwner 호출부와 인자 순서를 확인 rg -nP --type=java -C1 '\bvalidAlarmOwner\s*\('
254-273: 동시 호출 시 ringIndex 경합 및 로그 중복 가능 — 잠금/유니크 제약/낙관적 락으로 무결성 보강 필요동일 알람에 대한
ringAlarm이 거의 동시에 들어오면 동일 occurrence를 기준으로 같은ringIndex를 계산/저장할 수 있습니다. 비관적 잠금으로 조회하고,(occurrence, ringIndex)유니크 제약 및@Version을 통해 이중 방어를 권장합니다.서비스 내 비관적 잠금 사용(diff):
- AlarmOccurrenceEntity occurrence = alarmOccurrenceRepository - .findTopByAlarmIdAndDeactivateTypeInOrderByDateDescTimeDesc( + AlarmOccurrenceEntity occurrence = alarmOccurrenceRepository + .findTopByAlarmIdAndDeactivateTypeInOrderByDateDescTimeDescForUpdate( alarmId, List.of(DeactivateType.NONE) ) .orElseThrow(() -> ApplicationException.from(ALARM_OCCURRENCE_NOT_FOUND));레포/엔티티 보강(참고 코드):
// AlarmOccurrenceRepository @Lock(LockModeType.PESSIMISTIC_WRITE) Optional<AlarmOccurrenceEntity> findTopByAlarmIdAndDeactivateTypeInOrderByDateDescTimeDescForUpdate(Long alarmId, List<DeactivateType> types); // AlarmOccurrenceEntity @Version private Long version; // AlarmRingingLogEntity @Table( name = "alarm_ringing_log", uniqueConstraints = @UniqueConstraint( name = "uk_occurrence_ringindex", columnNames = {"alarm_occurrence_id", "ring_index"} ) )추가로, 유니크 제약 위반 시 재시도 또는 무시 정책을 명확히 하세요(
DataIntegrityViolationException처리).#!/bin/bash # 잠금/버전/유니크 제약 존재 여부 점검 rg -nP --type=java 'LockModeType\.PESSIMISTIC_WRITE|@Version|uniqueConstraints|uk_occurrence_ringindex'
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (1)
src/main/java/akuma/whiplash/domains/alarm/domain/service/AlarmCommandServiceImpl.java(2 hunks)
🧰 Additional context used
📓 Path-based instructions (1)
**/*.java
📄 CodeRabbit inference engine (AGENT.md)
**/*.java: 세미콜론 누락, 들여쓰기 불일치, import 순서 등의 형식 오류를 검토합니다.
사용되지 않는 변수나 주석은 제거를 제안합니다.
한 줄이 너무 길거나 중첩이 과도한 경우 구조 개선을 권장합니다.
Files:
src/main/java/akuma/whiplash/domains/alarm/domain/service/AlarmCommandServiceImpl.java
🧬 Code graph analysis (1)
src/main/java/akuma/whiplash/domains/alarm/domain/service/AlarmCommandServiceImpl.java (3)
src/main/java/akuma/whiplash/domains/alarm/application/mapper/AlarmMapper.java (1)
AlarmMapper(23-109)src/main/java/akuma/whiplash/domains/alarm/domain/service/AlarmCommandService.java (2)
AlarmCommandService(11-19)removeAlarm(16-16)src/main/java/akuma/whiplash/domains/alarm/persistence/entity/AlarmRingingLogEntity.java (1)
AlarmRingingLogEntity(19-39)
🔇 Additional comments (3)
src/main/java/akuma/whiplash/domains/alarm/domain/service/AlarmCommandServiceImpl.java (3)
16-20: 알람 울림 로그 import 추가 적절FCM/울림 기록 흐름을 위한
AlarmRingingLogEntity,AlarmRingingLogRepositoryimport 추가는 목적에 부합합니다. 별도 이슈 없음.
261-273: now 단일 캡처로 시간 정합성 보장한 점 좋습니다검증(now < scheduled)과 로그 시각이 동일 now를 사용하도록 개선돼 미세 오차가 제거되었습니다.
276-281: 벌크 상태 업데이트 위임 적절입력 검증(null/empty) 후 레포지토리 벌크 업데이트로 위임한 구현은 단순하고 효율적입니다.
| @Override | ||
| public void ringAlarm(Long memberId, Long alarmId) { | ||
| AlarmEntity alarm = findAlarmById(alarmId); | ||
| validAlarmOwner(memberId, alarm.getMember().getId()); | ||
|
|
||
| AlarmOccurrenceEntity occurrence = alarmOccurrenceRepository | ||
| .findTopByAlarmIdAndDeactivateTypeInOrderByDateDescTimeDesc( | ||
| alarmId, | ||
| List.of(DeactivateType.NONE) | ||
| ) | ||
| .orElseThrow(() -> ApplicationException.from(ALARM_OCCURRENCE_NOT_FOUND)); | ||
|
|
||
| LocalDateTime now = LocalDateTime.now(); | ||
| LocalDateTime scheduledDateTime = LocalDateTime.of(occurrence.getDate(), occurrence.getTime()); | ||
| if (now.isBefore(scheduledDateTime)) { | ||
| throw ApplicationException.from(NOT_ALARM_TIME); | ||
| } | ||
|
|
||
| int ringIndex = occurrence.ring(); | ||
| AlarmRingingLogEntity log = AlarmMapper.mapToAlarmRingingLogEntity( | ||
| occurrence, | ||
| ringIndex, | ||
| now | ||
| ); | ||
| alarmRingingLogRepository.save(log); | ||
| } |
There was a problem hiding this comment.
🧹 Nitpick (assertive)
알람 울림 푸시(Firebase) 연동 지점 확인 필요 — 트랜잭션 경계/멱등성 고려
PR 목표에 “알람 울릴 때 FCM 푸시 알림 추가”가 포함되어 있으나, 본 메서드에서는 푸시 발송이 보이지 않습니다(타 컴포넌트에서 처리 중일 수 있음). 울림 로그 저장 후 AFTER_COMMIT 이벤트에서 멱등 키(occurrenceId:ringIndex)로 발송하는 구조를 권장합니다.
예시(서비스 내 이벤트 발행):
alarmRingingLogRepository.save(log);
+ eventPublisher.publishEvent(new AlarmRangEvent(occurrence.getId(), ringIndex));예시(리스너에서 푸시, AFTER_COMMIT):
public record AlarmRangEvent(Long occurrenceId, int ringIndex) {}
@Component
@RequiredArgsConstructor
public class AlarmRangPushListener {
private final PushService pushService;
@TransactionalEventListener(phase = TransactionPhase.AFTER_COMMIT)
public void on(AlarmRangEvent e) {
// 멱등키 예: occurrenceId:ringIndex
pushService.sendAlarmRing(e.occurrenceId(), e.ringIndex());
}
}필요하시면 푸시/이벤트 골격 코드까지 생성해 드리겠습니다.
- zone = "Asia/Seoul" 설정
1 participant
📄 PR 요약
✍🏻 PR 상세
👀 참고사항
✅ 체크리스트
🚪 연관된 이슈 번호
Closes #58
Closes #59
Closes #60
Summary by CodeRabbit
New Features
Documentation
Chores
Tests