Skip to content
Merged
Show file tree
Hide file tree
Changes from 8 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 4 additions & 0 deletions build.gradle
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,10 @@ repositories {
mavenCentral()
}

tasks.withType(Test).configureEach {
useJUnitPlatform()
}

dependencies {
// --- Spring starters ---
implementation 'org.springframework.boot:spring-boot-starter-web'
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,10 @@
package akuma.whiplash.domains.alarm.application.dto.etc;

import lombok.Builder;

@Builder
public record RingingPushInfo(
Long alarmId,
Long memberId
) {
}
Comment on lines +5 to +10
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 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\(' -C2

Length 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' -C2

Length 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.

Original file line number Diff line number Diff line change
@@ -0,0 +1,11 @@
package akuma.whiplash.domains.alarm.application.dto.etc;

import lombok.Builder;

@Builder
public record RingingPushTargetDto(
String token,
Long alarmId,
Long memberId
) {
}
Original file line number Diff line number Diff line change
Expand Up @@ -10,10 +10,12 @@
import akuma.whiplash.domains.alarm.persistence.entity.AlarmEntity;
import akuma.whiplash.domains.alarm.persistence.entity.AlarmOccurrenceEntity;
import akuma.whiplash.domains.alarm.persistence.entity.AlarmOffLogEntity;
import akuma.whiplash.domains.alarm.persistence.entity.AlarmRingingLogEntity;
import akuma.whiplash.domains.member.persistence.entity.MemberEntity;
import akuma.whiplash.global.exception.ApplicationException;
import java.time.DayOfWeek;
import java.time.LocalDate;
import java.time.LocalDateTime;
import java.time.format.DateTimeFormatter;
import java.util.List;
import java.util.Objects;
Expand Down Expand Up @@ -78,6 +80,18 @@ public static AlarmOccurrenceEntity mapToAlarmOccurrenceForDate(AlarmEntity alar
.build();
}

public static AlarmRingingLogEntity mapToAlarmRingingLogEntity(
AlarmOccurrenceEntity occurrence,
int ringIndex,
LocalDateTime ringedAt
) {
return AlarmRingingLogEntity.builder()
.alarmOccurrence(occurrence)
.ringIndex(ringIndex)
.ringedAt(ringedAt)
.build();
}

public static CreateAlarmOccurrenceResponse mapToCreateAlarmOccurrenceResponse(Long occurrenceId) {
return CreateAlarmOccurrenceResponse.builder()
.occurrenceId(occurrenceId)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@
import java.util.Set;
import lombok.RequiredArgsConstructor;
import lombok.extern.slf4j.Slf4j;
import org.springframework.scheduling.annotation.Scheduled;
import org.springframework.stereotype.Service;

@Slf4j
Expand All @@ -27,7 +28,8 @@ public class AlarmReminderScheduler {
private final FcmService fcmService;
private final AlarmCommandService alarmCommandService;

// @Scheduled(cron = "0 * * * * *") // 매 분
// 매 분 마다 실행
@Scheduled(cron = "0 * * * * *")
public void sendPreAlarmNotifications() {
Comment on lines +31 to 33
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🛠️ 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.

Suggested change
// 매 분 마다 실행
@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.

log.info("[AlarmReminderScheduler.sendPreAlarmNotifications] 알람 울리기 1시간 전 푸시 알림 전송 스케줄러 시작");
try {
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,51 @@
package akuma.whiplash.domains.alarm.application.scheduler;

import akuma.whiplash.domains.alarm.application.dto.etc.RingingPushInfo;
import akuma.whiplash.domains.alarm.application.dto.etc.RingingPushTargetDto;
import akuma.whiplash.domains.alarm.domain.service.AlarmQueryService;
import akuma.whiplash.infrastructure.firebase.FcmService;
import akuma.whiplash.infrastructure.redis.RedisService;
import java.util.List;
import lombok.RequiredArgsConstructor;
import lombok.extern.slf4j.Slf4j;
import org.springframework.scheduling.annotation.Scheduled;
import org.springframework.stereotype.Service;

@Slf4j
@Service
@RequiredArgsConstructor
public class AlarmRingingNotificationScheduler {

private final AlarmQueryService alarmQueryService;
private final RedisService redisService;
private final FcmService fcmService;

// 10초 간격으로 실행
@Scheduled(fixedRate = 10000)
public void sendRingingAlarmNotifications() {
log.info("[AlarmRingingNotificationScheduler.sendRingingAlarmNotifications] 알람 울림 푸시 알림 전송 스케줄러 시작");
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🧹 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.

try {
List<RingingPushInfo> infos = alarmQueryService.getRingingNotificationTargets();
if (infos.isEmpty()) {
return;
}

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 {
Comment on lines +33 to +47
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 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' -C2

Length 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.

log.info("[AlarmRingingNotificationScheduler.sendRingingAlarmNotifications] 알람 울림 푸시 알림 전송 스케줄러 종료");
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -41,6 +41,10 @@ public void checkinAlarm(Long memberId, Long alarmId, AlarmCheckinRequest reques
alarmCommandService.checkinAlarm(memberId, alarmId, request);
}

public void ringAlarm(Long memberId, Long alarmId) {
alarmCommandService.ringAlarm(memberId, alarmId);
}
Comment on lines +44 to +46
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🧹 Nitpick (assertive)

ringAlarm 유스케이스 추가는 타당. 멱등성·서버시간 사용 여부 확인 필요

  • 동일 알람에 대해 클라이언트가 재시도(네트워크 재전송, 더블탭 등)할 수 있으므로, Service 레벨에서 같은 발생건(occurrence)·같은 분/초 내 중복 호출을 멱등 처리하는지 확인해 주세요.
  • alarmOff는 clientNow를 받지만 ring은 받지 않습니다. 울림 로그의 ringedAt이 반드시 서버 시간(신뢰원)으로 기록되는지 확인 바랍니다. 클라이언트 조작 가능 시간을 신뢰하지 않는 설계가 바람직합니다.


public List<AlarmInfoPreviewResponse> getAlarms(Long memberId) {
return alarmQueryService.getAlarms(memberId);
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -15,5 +15,6 @@ public interface AlarmCommandService {
AlarmOffResultResponse alarmOff(Long memberId, Long alarmId, LocalDateTime clientNow);
void removeAlarm(Long memberId, Long alarmId, String reason);
void checkinAlarm(Long memberId, Long alarmId, AlarmCheckinRequest request);
void ringAlarm(Long memberId, Long alarmId);
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🧹 Nitpick (assertive)

ringAlarm 시그니처의 시간 기준·트랜잭션/동시성 정책을 명확히 해주세요

  • 기존 alarmOff(..., LocalDateTime clientNow)와 달리 ringAlarm은 시간 인자를 받지 않습니다. 서버시간 기준만 허용하는 의도라면 Javadoc에 명시해 주세요. 클라이언트시간을 허용해야 한다면 시그니처 정합성도 고려됩니다.
  • 구현부가 발생 내역의 카운트 증가 및 로그 적재를 포함한다면, 구현 클래스에 @Transactional이 선언되어야 합니다(읽기-쓰기 혼합). 또한 동시 다중 호출 시 카운트 유실 방지를 위해 낙관적 락 재시도(아래 엔티티 코멘트 참조)가 필요합니다.
  • 반복 호출의 멱등성/의도(예: 동일 발생에 대한 2회 호출 시 ringIndex=2로 증가가 기대인지)를 명확히 하고 테스트로 보강해 주세요.
🤖 Prompt for AI Agents
In
src/main/java/akuma/whiplash/domains/alarm/domain/service/AlarmCommandService.java
around line 18, the ringAlarm(Long memberId, Long alarmId) signature lacks time
semantics and transaction/concurrency policy: either accept a time parameter or
add Javadoc stating server-time-only; update the implementing class to annotate
the method/class with @Transactional because it performs reads+writes;
implement/ensure optimistic-lock retry when incrementing occurrence counters to
avoid lost updates under concurrent calls; and document/define expected
idempotency (whether repeated calls should increment ringIndex) and add
unit/integration tests that assert the defined behavior and concurrency
handling.

void markReminderSent(Set<Long> occurrenceIds);
}
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@
import akuma.whiplash.domains.alarm.persistence.entity.AlarmEntity;
import akuma.whiplash.domains.alarm.persistence.entity.AlarmOccurrenceEntity;
import akuma.whiplash.domains.alarm.persistence.entity.AlarmOffLogEntity;
import akuma.whiplash.domains.alarm.persistence.entity.AlarmRingingLogEntity;
import akuma.whiplash.domains.alarm.persistence.repository.AlarmOccurrenceRepository;
import akuma.whiplash.domains.alarm.persistence.repository.AlarmOffLogRepository;
import akuma.whiplash.domains.alarm.persistence.repository.AlarmRepository;
Expand Down Expand Up @@ -245,6 +246,36 @@ public void checkinAlarm(Long memberId, Long alarmId, AlarmCheckinRequest reques
occurrence.checkin(LocalDateTime.now());
}

@Override
public void ringAlarm(Long memberId, Long alarmId) {
AlarmEntity alarm = findAlarmById(alarmId);
validAlarmOwner(memberId, alarm.getMember().getId());

Comment on lines +249 to +253
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Verification agent

🧩 Analysis chain

소유자 검증 파라미터 순서의 일관성 유지

본 메서드는 validAlarmOwner(memberId, alarm.getMember().getId())로 호출하지만, 파일 내 다른 위치에서는 인자가 뒤바뀐 형태가 존재합니다(동등 비교라 기능상 문제는 없으나 가독성 저하). 전역적으로 “요청자 ID, 알람 소유자 ID” 순으로 통일 권장합니다.

레포 전체에서 호출부를 점검하려면:


🏁 Script executed:

#!/bin/bash
rg -nP --type=java -C1 '\bvalidAlarmOwner\s*\('

Length of output: 2368


validAlarmOwner 호출 인자 순서 일관성 유지 필요

다음 호출부에서 “요청자 ID, 알람 소유자 ID” 순서를 준수하도록 수정해주세요.

  • 파일: src/main/java/akuma/whiplash/domains/alarm/domain/service/AlarmCommandServiceImpl.java
    위치: 114행
    변경 전:
    validAlarmOwner(findAlarm.getMember().getId(), memberId);
    변경 후:
    - validAlarmOwner(findAlarm.getMember().getId(), memberId);
    + validAlarmOwner(memberId, findAlarm.getMember().getId());
🤖 Prompt for AI Agents
In
src/main/java/akuma/whiplash/domains/alarm/domain/service/AlarmCommandServiceImpl.java
around lines 249-253, ensure the validAlarmOwner call uses the consistent
argument order "requesterId, alarmOwnerId"; locate the other occurrence at line
~114 where it currently calls validAlarmOwner(findAlarm.getMember().getId(),
memberId) and swap the arguments so it calls validAlarmOwner(memberId,
findAlarm.getMember().getId()) to match the desired order.

AlarmOccurrenceEntity occurrence = alarmOccurrenceRepository
.findTopByAlarmIdAndDeactivateTypeInOrderByDateDescTimeDesc(
alarmId,
List.of(DeactivateType.NONE)
)
.orElseThrow(() -> ApplicationException.from(ALARM_OCCURRENCE_NOT_FOUND));

if (occurrence.getDeactivateType() != DeactivateType.NONE) {
throw ApplicationException.from(ALREADY_DEACTIVATED);
}
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🧹 Nitpick (assertive)

중복 상태 검사 제거 제안

이미 레포지토리에서 DeactivateType.NONE으로 필터링했으므로 동일 조건의 즉시 재검사는冗長합니다. 유지하려면 주석으로 의도를 명시하세요(동시성 변경 대비 등).

-        if (occurrence.getDeactivateType() != DeactivateType.NONE) {
-            throw ApplicationException.from(ALREADY_DEACTIVATED);
-        }

findTopBy...InOrderBy...가 확실히 DeactivateType.NONE만 반환하는지 레포지토리 시그니처를 재확인해 주세요. 필요 시 메서드명에 조건을 드러내는 것도 좋습니다.

📝 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.

Suggested change
if (occurrence.getDeactivateType() != DeactivateType.NONE) {
throw ApplicationException.from(ALREADY_DEACTIVATED);
}
🤖 Prompt for AI Agents
In
src/main/java/akuma/whiplash/domains/alarm/domain/service/AlarmCommandServiceImpl.java
around lines 261-263, the explicit re-check of occurrence.getDeactivateType() !=
DeactivateType.NONE is redundant because the repository query already filters
for DeactivateType.NONE; either remove this duplicate check or keep it but add a
clarifying comment that it exists to guard against concurrent state changes.
Also verify the repository method signature (e.g., findTopBy...InOrderBy...)
actually restricts results to DeactivateType.NONE and, if helpful for clarity,
consider renaming the repository method to make that condition explicit in its
name.


LocalDateTime scheduledDateTime = LocalDateTime.of(occurrence.getDate(), occurrence.getTime());
if (LocalDateTime.now().isBefore(scheduledDateTime)) {
throw ApplicationException.from(NOT_ALARM_TIME);
}

int ringIndex = occurrence.ring();
AlarmRingingLogEntity log = AlarmMapper.mapToAlarmRingingLogEntity(
occurrence,
ringIndex,
LocalDateTime.now()
);
alarmRingingLogRepository.save(log);
}
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Verification agent

🧩 Analysis chain

동시 호출 시 울림 카운트/로그 중복 가능성 — 잠금 또는 유니크 제약으로 정합성 보강 필요

동일 알람에 대해 ringAlarm이 거의 동시에 호출되면, 두 트랜잭션이 같은 occurrence를 읽고 각각 ring()을 호출하면서 증가분이 꼬이거나(@Version 부재 시) 동일 ringIndex로 로그가 두 번 적재될 리스크가 있습니다. 조회 시 비관적 잠금 또는 엔티티의 낙관적 잠금, 그리고 로그 테이블의 유니크 제약으로 이중 방어를 권장합니다.

  • JPA 비관적 잠금으로 최신 발생내역 조회
  • AlarmOccurrenceEntity@Version 추가(낙관적 잠금)
  • AlarmRingingLogEntity(alarm_occurrence_id, ring_index) 유니크 제약

변경 제안(서비스 메서드 내 잠금 전용 쿼리 사용):

-        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"}
  )
)

다음 스크립트로 잠금/버전/유니크 제약 존재 여부를 확인해 주세요.


🏁 Script executed:

#!/bin/bash
# AlarmOccurrenceEntity @Version 확인
rg -nP --type=java '(?m)class\s+AlarmOccurrenceEntity\b' -C3
rg -nP --type=java '@Version' -C1

# AlarmRingingLogEntity 유니크 제약/Index 확인
rg -nP --type=java '(?m)class\s+AlarmRingingLogEntity\b' -C5
rg -nP --type=java 'UniqueConstraint|uniqueConstraints|@Index' -C2

# 잠금 전용 쿼리 존재 여부 확인
rg -nP --type=java 'findTopByAlarmIdAndDeactivateTypeInOrderByDateDescTimeDescForUpdate'

Length of output: 4351


동시성 안전성 보강: 잠금 및 유니크 제약 추가 필요

검증 결과,

  • AlarmOccurrenceEntity에 낙관적 잠금을 위한 @Version 애노테이션이 없습니다.
  • AlarmRingingLogEntity에는 (alarm_occurrence_id, ring_index) 유니크 제약이 설정되어 있지 않습니다.
  • PESSIMISTIC_WRITE 잠금을 위한 전용 조회 메서드(findTopByAlarmIdAndDeactivateTypeInOrderByDateDescTimeDescForUpdate)도 정의되어 있지 않습니다.

위 문제로 인해 ringAlarm이 동시 호출될 경우 동일한 발생내역을 기준으로 두 트랜잭션이 같은 인덱스를 계산·저장하거나, 애노테이션 미설정 시 낙관적 잠금 충돌 없이 무결성이 깨질 리스크가 있습니다.

조치 사항:

  • src/main/java/akuma/whiplash/domains/alarm/persistence/entity/AlarmOccurrenceEntity.java
    • @Version private Long version; 추가 (낙관적 잠금)
  • src/main/java/akuma/whiplash/domains/alarm/persistence/entity/AlarmRingingLogEntity.java
    • @Table(name = "alarm_ringing_log", uniqueConstraints = @UniqueConstraint(name = "uk_occurrence_ringindex", columnNames = {"alarm_occurrence_id","ring_index"})) 추가
  • src/main/java/akuma/whiplash/domains/alarm/persistence/repository/AlarmOccurrenceRepository.java
    • 아래 전용 조회 메서드 추가
      @Lock(LockModeType.PESSIMISTIC_WRITE)
      Optional<AlarmOccurrenceEntity> 
        findTopByAlarmIdAndDeactivateTypeInOrderByDateDescTimeDescForUpdate(
            Long alarmId, List<DeactivateType> types);

위 보강 작업을 통해 동시 호출 시에도 정확한 인덱스 계산과 로그 중복 방지를 보장할 수 있습니다.

🤖 Prompt for AI Agents
In
src/main/java/akuma/whiplash/domains/alarm/domain/service/AlarmCommandServiceImpl.java
around lines 249-277: the ringAlarm flow is vulnerable to concurrent calls
because AlarmOccurrenceEntity lacks optimistic locking, AlarmRingingLogEntity
lacks a uniqueness constraint on (alarm_occurrence_id, ring_index), and there is
no PESSIMISTIC_WRITE repository lookup; to fix, add @Version private Long
version; to
src/main/java/akuma/whiplash/domains/alarm/persistence/entity/AlarmOccurrenceEntity.java,
add a unique constraint on (alarm_occurrence_id, ring_index) via @Table(...) to
src/main/java/akuma/whiplash/domains/alarm/persistence/entity/AlarmRingingLogEntity.java,
and add a repository method annotated with @Lock(LockModeType.PESSIMISTIC_WRITE)
Optional<AlarmOccurrenceEntity>
findTopByAlarmIdAndDeactivateTypeInOrderByDateDescTimeDescForUpdate(Long
alarmId, List<DeactivateType> types) to
src/main/java/akuma/whiplash/domains/alarm/persistence/repository/AlarmOccurrenceRepository.java
so the service can perform a for-update lookup before computing/saving ring
indexes.


@Override
public void markReminderSent(Set<Long> occurrenceIds) {
if (occurrenceIds == null || occurrenceIds.isEmpty()) return;
Expand Down
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
package akuma.whiplash.domains.alarm.domain.service;

import akuma.whiplash.domains.alarm.application.dto.etc.OccurrencePushInfo;
import akuma.whiplash.domains.alarm.application.dto.etc.RingingPushInfo;
import akuma.whiplash.domains.alarm.application.dto.response.AlarmInfoPreviewResponse;
import akuma.whiplash.domains.alarm.application.dto.response.AlarmRemainingOffCountResponse;
import java.time.LocalDateTime;
Expand All @@ -10,4 +11,5 @@ public interface AlarmQueryService {
List<AlarmInfoPreviewResponse> getAlarms(Long memberId);
List<OccurrencePushInfo> getPreNotificationTargets(LocalDateTime startInclusive, LocalDateTime endInclusive);
AlarmRemainingOffCountResponse getWeeklyRemainingOffCount(Long memberId);
List<RingingPushInfo> getRingingNotificationTargets();
}
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
package akuma.whiplash.domains.alarm.domain.service;

import akuma.whiplash.domains.alarm.application.dto.etc.OccurrencePushInfo;
import akuma.whiplash.domains.alarm.application.dto.etc.RingingPushInfo;
import akuma.whiplash.domains.alarm.application.dto.response.AlarmInfoPreviewResponse;
import akuma.whiplash.domains.alarm.application.dto.response.AlarmRemainingOffCountResponse;
import akuma.whiplash.domains.alarm.domain.constant.DeactivateType;
Expand Down Expand Up @@ -177,6 +178,11 @@ private AlarmInfoPreviewResponse buildPreviewResponse(AlarmEntity alarm, LocalDa
.secondUpcomingDayOfWeek(DateUtil.getKoreanDayOfWeek(resolvedSecondUpcomingDate))
.build();
}

@Override
public List<RingingPushInfo> getRingingNotificationTargets() {
return alarmOccurrenceRepository.findRingingNotificationTargets(DeactivateType.NONE);
}
Comment on lines +182 to +185
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Verification agent

🧩 Analysis chain

울림 타겟 조회: 수신 동의 정책·중복 제거 보장 여부 확인

  • 현재 구현은 Repository 위임만 수행합니다. Pre-notification 쿼리들(Line 76~117)은 m.pushNotificationPolicy = true를 명시해 수신 동의를 존중하는데, 울림 쿼리에는 해당 필터가 없습니다. Repository 쿼리에 동일 정책을 반영해야 합니다.
  • 동일 멤버·알람 쌍이 중복 반환되지 않도록 DISTINCT 보장(Repository 쿼리 또는 상위 서비스에서 집합화) 필요 여부를 확인해 주세요.

수정 자체는 Repository 단 변경으로 충분합니다(아래 해당 파일 코멘트 참조).


🏁 Script executed:

#!/bin/bash
# Locate the repository method for ringing notification targets
rg -n "interface AlarmOccurrenceRepository" -C2 && \
rg -n "findRingingNotificationTargets" -C3 -g "*.java"

Length of output: 2267


🏁 Script executed:

#!/bin/bash
# Pre-notification 쿼리(라인 76~117) 확인
sed -n '70,130p' src/main/java/akuma/whiplash/domains/alarm/persistence/repository/AlarmOccurrenceRepository.java

# pushNotificationPolicy 필터 사용 여부 검색
rg -n "pushNotificationPolicy" -C3 src/main/java/akuma/whiplash/domains/alarm/persistence/repository/AlarmOccurrenceRepository.java

Length of output: 3038


울림 타겟 조회: 수신 동의 필터와 중복 제거 보장 필요

아래 사항들이 확인되었습니다. 수정은 Repository 쿼리 수준에서 처리하는 것이 가장 간단합니다.

• 대상 파일

  • src/main/java/akuma/whiplash/domains/alarm/persistence/repository/AlarmOccurrenceRepository.java (라인 130~135)
    • 수정 항목
  1. findRingingNotificationTargets JPQL에 AND m.pushNotificationPolicy = true 필터 추가
  2. 동일한 (alarmId, memberId) 쌍이 중복 반환되지 않도록 SELECT DISTINCT 적용

예시 변경안:

@@ AlarmOccurrenceRepository.java
- @Query("""
-    SELECT new akuma.whiplash.domains.alarm.application.dto.etc.RingingPushInfo(a.id, m.id)
-    FROM AlarmOccurrenceEntity o
-    JOIN o.alarm a
-    JOIN a.member m
-    WHERE o.alarmRinging = true
-      AND o.deactivateType = :status
-""")
+ @Query("""
+    SELECT DISTINCT new akuma.whiplash.domains.alarm.application.dto.etc.RingingPushInfo(a.id, m.id)
+    FROM AlarmOccurrenceEntity o
+    JOIN o.alarm a
+    JOIN a.member m
+    WHERE o.alarmRinging = true
+      AND o.deactivateType = :status
+      AND m.pushNotificationPolicy = true
+""")
  List<RingingPushInfo> findRingingNotificationTargets(@Param("status") DeactivateType status);

이렇게 변경하면 서비스 계층에서 별도 집합화 없이 수신 동의 정책을 준수하고, 중복 반환을 방지할 수 있습니다.

📝 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.

Suggested change
@Override
public List<RingingPushInfo> getRingingNotificationTargets() {
return alarmOccurrenceRepository.findRingingNotificationTargets(DeactivateType.NONE);
}
// In AlarmOccurrenceRepository.java around lines 130–135
@Query("""
SELECT DISTINCT new akuma.whiplash.domains.alarm.application.dto.etc.RingingPushInfo(a.id, m.id)
FROM AlarmOccurrenceEntity o
JOIN o.alarm a
JOIN a.member m
WHERE o.alarmRinging = true
AND o.deactivateType = :status
AND m.pushNotificationPolicy = true
""")
List<RingingPushInfo> findRingingNotificationTargets(@Param("status") DeactivateType status);
🤖 Prompt for AI Agents
In
src/main/java/akuma/whiplash/domains/alarm/persistence/repository/AlarmOccurrenceRepository.java
around lines 130-135, the JPQL for findRingingNotificationTargets must enforce
member push consent and prevent duplicate (alarmId, memberId) rows; update the
query to use SELECT DISTINCT and add "AND m.pushNotificationPolicy = true" to
the WHERE clause so the repository returns only consenting members with unique
alarmId/memberId pairs (no service-layer dedup needed).

}


Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@ public enum AlarmErrorCode implements BaseErrorCode {
INVALID_WEEKDAY(HttpStatus.BAD_REQUEST, "ALARM_008", "유효하지 않은 요일 정보입니다."),
CHECKIN_OUT_OF_RANGE(HttpStatus.BAD_REQUEST, "ALARM_009", "지정된 위치 반경 내에 있지 않아 출석할 수 없습니다."),
NEXT_WEEK_ALARM_DEACTIVATION_NOT_ALLOWED(HttpStatus.BAD_REQUEST, "ALARM_010", "다음 주에 울릴 알람은 끌 수 없습니다."),
NOT_ALARM_TIME(HttpStatus.BAD_REQUEST, "ALARM_011", "알람이 울릴 시간이 아닙니다."),

ALARM_NOT_FOUND(HttpStatus.NOT_FOUND, "ALARM_401", "존재하지 않는 알람입니다."),
ALARM_OCCURRENCE_NOT_FOUND(HttpStatus.NOT_FOUND, "ALARM_402", "알람 발생 내역이 존재하지 않습니다.")
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -83,6 +83,11 @@ public void checkin(LocalDateTime now) {
this.checkinTime = now;
}

public int ring() {
this.alarmRinging = true;
return ++this.ringingCount;
}
Comment on lines +86 to +89
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🧹 Nitpick (assertive)

⚠️ Potential issue

동시성 제어 부재로 ringingCount 증분 손실 위험(레이스 컨디션)

현재 ring()은 단순 증가 연산입니다. 동일 발생(occurrence)에 대해 다중 요청이 거의 동시에 들어올 경우, 서로 같은 스냅샷을 기반으로 커밋하여 증가가 1만 반영되는 “lost update”가 발생할 수 있습니다.

권장 대응:

  • 낙관적 락: 엔티티에 @Version 필드를 추가하고, 서비스에서 OptimisticLockException을 감지하여 짧은 재시도를 적용합니다.
  • 실패 시 클라이언트에 재시도 가능한 에러로 응답하거나 내부 재시도 정책을 설정합니다.

엔티티 예시(클래스 상단에 필드 추가):

// import jakarta.persistence.Version;
@Version
private Long version;

서비스 재시도 스케치(의사 코드):

int attempts = 0;
while (true) {
  try {
    alarmCommandService.ringAlarm(memberId, alarmId);
    break;
  } catch (OptimisticLockException e) {
    if (++attempts >= 3) throw e;
    // backoff 후 재시도
  }
}

추가 확인:

  • deactivateType != NONE인 경우에도 ring()이 호출될 수 있는지(서비스에서 방어하고 있다면 OK). 도메인 불변성을 엔티티에서도 한 번 더 방어할지 여부를 결정해 주세요.
  • alarmRinging을 true로만 설정하는데, OFF/CHECKIN 시 false로 되돌리는 지점이 별도로 있는지 확인 바랍니다(스케줄러/상태머신 일관성).

필요하시면 @Version 도입과 재시도 유틸리티(템플릿) 코드를 생성해 드리겠습니다.


public void updateReminderSent(boolean reminderSent) {
this.reminderSent = reminderSent;
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -14,10 +14,12 @@
import lombok.AccessLevel;
import lombok.Getter;
import lombok.NoArgsConstructor;
import lombok.experimental.SuperBuilder;
import org.hibernate.annotations.DynamicInsert;

@Entity
@Getter
@SuperBuilder
@DynamicInsert
@NoArgsConstructor(access = AccessLevel.PROTECTED)
@Table(name = "alarm_ringing_log")
Expand Down
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
package akuma.whiplash.domains.alarm.persistence.repository;

import akuma.whiplash.domains.alarm.application.dto.etc.OccurrencePushInfo;
import akuma.whiplash.domains.alarm.application.dto.etc.RingingPushInfo;
import akuma.whiplash.domains.alarm.domain.constant.DeactivateType;
import akuma.whiplash.domains.alarm.persistence.entity.AlarmOccurrenceEntity;
import java.time.LocalDate;
Expand Down Expand Up @@ -122,4 +123,14 @@ List<OccurrencePushInfo> findPreNotificationTargetsUntilTime(
WHERE o.id IN :ids AND o.reminderSent = false
""")
void markReminderSentIn(@Param("ids") Set<Long> ids);

@Query("""
SELECT new akuma.whiplash.domains.alarm.application.dto.etc.RingingPushInfo(a.id, m.id)
FROM AlarmOccurrenceEntity o
JOIN o.alarm a
JOIN a.member m
WHERE o.alarmRinging = true
AND o.deactivateType = :status
""")
List<RingingPushInfo> findRingingNotificationTargets(@Param("status") DeactivateType status);
}
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🧹 Nitpick (assertive)

대량 전송 대비 배치·인덱스 설계 권고

  • 스케줄러가 본 결과를 그대로 FCM에 전송하면 대량 데이터를 한 번에 처리합니다. 메모리·외부 요청 실패 재시도 측면에서 페이징 또는 PK 기준 배치(chunk) 처리와 재시도 큐(예: 실패 토큰 재시도)를 권장합니다.
  • 쿼리 조건(alarmRinging, deactivateType, date)에 조합 인덱스를 두면 스캔 비용을 줄일 수 있습니다. 예시(엔티티 측 DDL 힌트):
// AlarmOccurrenceEntity
@Table(
  indexes = {
    @Index(name = "idx_occurrence_ringing_status_date", columnList = "alarmRinging,deactivateType,date")
  }
)

🧹 Nitpick (assertive)

🛠️ Refactor suggestion

⚠️ Potential issue

FCM 수신 동의 미반영·중복 가능성·스코프 과다(날짜 미제한) — 쿼리 보강 필요

  • 수신 동의: Pre-notification 쿼리들과 달리 m.pushNotificationPolicy = true 조건이 빠져 있어 옵트아웃 사용자에게도 울림 푸시를 보낼 수 있습니다.
  • 중복: 동일 멤버가 동일 알람의 여러 occurrence에 대해 alarmRinging = true인 데이터가 남아있을 경우 중복 행이 반환될 수 있습니다. DISTINCT 필요합니다.
  • 스코프: 날짜 제한이 없어 과거 울림 플래그 잔존 데이터가 섞일 위험이 있습니다. 당일로 제한하는 것을 권장합니다.

아래와 같이 보강을 제안합니다.

-    @Query("""
-    SELECT new akuma.whiplash.domains.alarm.application.dto.etc.RingingPushInfo(a.id, m.id)
-    FROM AlarmOccurrenceEntity o
-    JOIN o.alarm a
-    JOIN a.member m
-    WHERE o.alarmRinging = true
-      AND o.deactivateType = :status
-    """)
-    List<RingingPushInfo> findRingingNotificationTargets(@Param("status") DeactivateType status);
+    @Query("""
+    SELECT DISTINCT new akuma.whiplash.domains.alarm.application.dto.etc.RingingPushInfo(a.id, m.id)
+    FROM AlarmOccurrenceEntity o
+    JOIN o.alarm a
+    JOIN a.member m
+    WHERE o.alarmRinging = true
+      AND o.deactivateType = :status
+      AND m.pushNotificationPolicy = true
+      AND o.date = CURRENT_DATE
+    """)
+    List<RingingPushInfo> findRingingNotificationTargets(@Param("status") DeactivateType status);

테스트 보강 제안:

  • pushNotificationPolicy=false 사용자를 포함한 테스트에서 본 메서드가 제외시키는지 검증
  • 과거 날짜 occurrence에 alarmRinging=true가 남아 있어도 결과에 포함되지 않음을 검증
📝 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.

Suggested change
@Query("""
SELECT new akuma.whiplash.domains.alarm.application.dto.etc.RingingPushInfo(a.id, m.id)
FROM AlarmOccurrenceEntity o
JOIN o.alarm a
JOIN a.member m
WHERE o.alarmRinging = true
AND o.deactivateType = :status
""")
List<RingingPushInfo> findRingingNotificationTargets(@Param("status") DeactivateType status);
}
@Query("""
SELECT DISTINCT new akuma.whiplash.domains.alarm.application.dto.etc.RingingPushInfo(a.id, m.id)
FROM AlarmOccurrenceEntity o
JOIN o.alarm a
JOIN a.member m
WHERE o.alarmRinging = true
AND o.deactivateType = :status
AND m.pushNotificationPolicy = true
AND o.date = CURRENT_DATE
""")
List<RingingPushInfo> findRingingNotificationTargets(@Param("status") DeactivateType status);
}

Original file line number Diff line number Diff line change
Expand Up @@ -63,7 +63,7 @@ public ApplicationResponse<CreateAlarmResponse> createAlarm(@AuthenticationPrinc
alarmErrorCodes = {ALARM_NOT_FOUND, ALARM_OFF_LIMIT_EXCEEDED, ALREADY_DEACTIVATED, INVALID_CLIENT_DATE},
authErrorCodes = {PERMISSION_DENIED}
)
@Operation(summary = "알람 끄기", description = "알람을 끕니다.")
@Operation(summary = "알람 끄기", description = "알람 목록에 있는 토글을 이용하여 알람을 끌 때, 알람 울림 화면에서 [봐주세요] 버튼을 눌러서 알람을 끌 때 호출하는 API입니다.")
@PostMapping("/{alarmId}/off")
public ApplicationResponse<AlarmOffResultResponse> alarmOff(@AuthenticationPrincipal MemberContext memberContext, @PathVariable Long alarmId, @RequestBody @Valid AlarmOffRequest request) {
AlarmOffResultResponse response = alarmUseCase.alarmOff(memberContext.memberId(), alarmId, request.clientNow());
Expand Down Expand Up @@ -117,4 +117,18 @@ public ApplicationResponse<AlarmRemainingOffCountResponse> getWeeklyRemainingOff
AlarmRemainingOffCountResponse response = alarmUseCase.getWeeklyRemainingOffCount(memberContext.memberId());
return ApplicationResponse.onSuccess(response);
}

@CustomErrorCodes(
alarmErrorCodes = {ALARM_NOT_FOUND, ALARM_OCCURRENCE_NOT_FOUND, ALREADY_DEACTIVATED, NOT_ALARM_TIME},
authErrorCodes = {PERMISSION_DENIED}
)
@Operation(summary = "알람 울림", description = "알람이 울릴 때 호출합니다.")
@PostMapping("/{alarmId}/ring")
public ApplicationResponse<Void> ringAlarm(
@AuthenticationPrincipal MemberContext memberContext,
@PathVariable Long alarmId
) {
alarmUseCase.ringAlarm(memberContext.memberId(), alarmId);
return ApplicationResponse.onSuccess();
}
Comment on lines +121 to +133
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Verification agent

🧩 Analysis chain

알람 울림 API의 에러 매핑/테스트 정합성 확인 필요

@CustomErrorCodesALREADY_DEACTIVATED, NOT_ALARM_TIME가 포함되어 있으나, 통합 테스트에서 ALREADY_DEACTIVATED의 기대 상태 코드가 404로 표기된 TODO가 존재합니다. 컨트롤러 매핑은 보통 400(Bad Request)이 자연스럽습니다. 매핑 전략을 확정하고 슬라이스/통합 테스트 기대값을 함께 정렬하세요.

추가로 슬라이스 테스트에 다음 케이스도 보강 권장:

  • ALARM_NOT_FOUND → 404
  • PERMISSION_DENIED → 403

테스트 기대값/매핑 현황을 점검하려면:


🏁 Script executed:

#!/bin/bash
rg -nP --type=java '\[POST\]\s*/api/alarms/\{alarmId\}/ring' -C3
rg -nP --type=java 'ALREADY_DEACTIVATED|NOT_ALARM_TIME|ALARM_NOT_FOUND|PERMISSION_DENIED' src/test

Length of output: 1119


🏁 Script executed:

#!/bin/bash
# Inspect integration tests for ring alarm error cases
sed -n '100,200p' src/test/java/akuma/whiplash/domains/alarm/presentation/AlarmControllerIntegrationTest.java

# Inspect controller slice tests for error mapping expectations
sed -n '150,260p' src/test/java/akuma/whiplash/domains/alarm/presentation/AlarmControllerTest.java

Length of output: 6665


알람 울림 API 에러 매핑 및 테스트 정합성 확인·수정 필요

  • Integration Test (src/test/java/akuma/whiplash/domains/alarm/presentation/AlarmControllerIntegrationTest.java)
    • fail_alreadyDeactivated()에서
      • 기존: .andExpect(status().isNotFound())
      • 변경: .andExpect(status().isBadRequest())
  • Controller 설정 (@CustomErrorCodes) 검증
    • ALREADY_DEACTIVATED, NOT_ALARM_TIME → 400 Bad Request
    • ALARM_NOT_FOUND, ALARM_OCCURRENCE_NOT_FOUND → 404 Not Found
    • PERMISSION_DENIED → 403 Forbidden
  • 슬라이스 테스트 보강 (src/test/java/akuma/whiplash/domains/alarm/presentation/AlarmControllerTest.java)
    • ALARM_NOT_FOUND → .andExpect(status().isNotFound())
    • PERMISSION_DENIED → .andExpect(status().isForbidden())

위 사항을 반영하여 컨트롤러의 에러 매핑 전략과 통합/슬라이스 테스트의 기대값이 완전히 일치하도록 수정해주세요.

🤖 Prompt for AI Agents
In src/main/java/akuma/whiplash/domains/alarm/presentation/AlarmController.java
around lines 121-133, the controller's error-to-status mapping must match the
tests: change the @CustomErrorCodes configuration so ALREADY_DEACTIVATED and
NOT_ALARM_TIME map to 400 Bad Request, ALARM_NOT_FOUND and
ALARM_OCCURRENCE_NOT_FOUND map to 404 Not Found, and PERMISSION_DENIED maps to
403 Forbidden; then update tests to match these expectations: in the integration
test
src/test/java/akuma/whiplash/domains/alarm/presentation/AlarmControllerIntegrationTest.java
change fail_alreadyDeactivated() expectation from isNotFound() to
isBadRequest(), and in the slice test
src/test/java/akuma/whiplash/domains/alarm/presentation/AlarmControllerTest.java
assert ALARM_NOT_FOUND => isNotFound() and PERMISSION_DENIED => isForbidden();
ensure any related test setup/exceptions still throw the corresponding domain
error codes so the controller mapping is exercised correctly.

}
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@
import org.springframework.beans.factory.annotation.Value;
import org.springframework.context.annotation.Bean;
import org.springframework.context.annotation.Configuration;
import org.springframework.core.env.Environment;
import org.springframework.security.config.annotation.web.builders.HttpSecurity;
import org.springframework.security.config.annotation.web.configuration.EnableWebSecurity;
import org.springframework.security.config.annotation.web.configurers.AbstractHttpConfigurer;
Expand All @@ -27,6 +28,7 @@ public class SecurityConfig {

private final JwtUtils jwtUtils;
private final RequestMatcherHolder requestMatcherHolder;
private final Environment env;

@Value("${swagger.server.url}")
private String serverUrl;
Expand All @@ -49,7 +51,7 @@ public SecurityFilterChain securityFilterChain(HttpSecurity http) throws Excepti
.hasAnyAuthority(ADMIN.name())
.anyRequest().authenticated()
)
.addFilterBefore(new JwtAuthenticationFilter(jwtUtils, requestMatcherHolder),
.addFilterBefore(new JwtAuthenticationFilter(jwtUtils, requestMatcherHolder, env),
UsernamePasswordAuthenticationFilter.class);
Comment on lines +54 to 55
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🧹 Nitpick (assertive)

🛠️ Refactor suggestion

JwtAuthenticationFilter 인스턴스 생성은 Bean으로 통일하세요. (@component와 new 병행 사용 지양)

현재 JwtAuthenticationFilter@Component임에도 여기서 new로 직접 생성하여 필터 체인에 삽입합니다. 이 경우:

  • 컨테이너가 관리하는 빈과 별개 인스턴스가 생겨 AOP/프로파일링/후처리의 일관성이 깨질 수 있고,
  • 불필요한 중복 빈이 생깁니다.

다음처럼 Bean 팩토리 메서드로 등록하고 필터 체인에서 그 빈을 사용해 주세요. (동시에 JwtAuthenticationFilter@Component는 제거 권장)

 @Configuration
 @EnableWebSecurity
 @RequiredArgsConstructor
 public class SecurityConfig {
@@
-            .addFilterBefore(new JwtAuthenticationFilter(jwtUtils, requestMatcherHolder, env),
-                UsernamePasswordAuthenticationFilter.class);
+            .addFilterBefore(jwtAuthenticationFilter(), UsernamePasswordAuthenticationFilter.class);
@@
+    @Bean
+    public JwtAuthenticationFilter jwtAuthenticationFilter() {
+        return new JwtAuthenticationFilter(jwtUtils, requestMatcherHolder, env);
+    }

그리고 JwtAuthenticationFilter 클래스에서는 @Component 애노테이션을 제거해 주세요(아래 파일 코멘트 참고).

📝 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.

Suggested change
.addFilterBefore(new JwtAuthenticationFilter(jwtUtils, requestMatcherHolder, env),
UsernamePasswordAuthenticationFilter.class);
@Configuration
@EnableWebSecurity
@RequiredArgsConstructor
public class SecurityConfig {
private final JwtUtils jwtUtils;
private final RequestMatcherHolder requestMatcherHolder;
private final Environment env;
protected void configure(HttpSecurity http) throws Exception {
http
// ... other config ...
.addFilterBefore(jwtAuthenticationFilter(), UsernamePasswordAuthenticationFilter.class);
}
@Bean
public JwtAuthenticationFilter jwtAuthenticationFilter() {
return new JwtAuthenticationFilter(jwtUtils, requestMatcherHolder, env);
}
}
🤖 Prompt for AI Agents
In src/main/java/akuma/whiplash/global/config/security/SecurityConfig.java
around lines 54-55, a new JwtAuthenticationFilter(...) is instantiated inline
which bypasses Spring container management; instead create a @Bean factory
method in SecurityConfig that constructs and returns the JwtAuthenticationFilter
(injecting jwtUtils, requestMatcherHolder, env into that factory), autowire that
bean into the configuration and call
addFilterBefore(jwtAuthenticationFilterBean,
UsernamePasswordAuthenticationFilter.class); also remove the @Component
annotation from the JwtAuthenticationFilter class so there are not multiple
unmanaged/managed instances.


return http.build();
Expand Down
Loading