Skip to content

refactor: 할인 알림 및 SSE 알림 버그 수정#171

Merged
kangcheolung merged 2 commits intodevelopfrom
feature/170
Mar 4, 2026
Merged

refactor: 할인 알림 및 SSE 알림 버그 수정#171
kangcheolung merged 2 commits intodevelopfrom
feature/170

Conversation

@kangcheolung
Copy link
Copy Markdown
Member

@kangcheolung kangcheolung commented Mar 4, 2026

🔍️ 작업 내용

  • Closes #
    할인 알림 및 SSE 알림 버그 수정

✨ 상세 설명

  • SSE emitter 콜백에서 race condition 방지 (remove(userId) → remove(userId, emitter))
  • discountRate 서버 검증 추가 (10/20/30/40 외 값 400 에러 반환)
  • markAllAsRead N+1 개선 (벌크 UPDATE 쿼리로 변경)

🛠️ 추후 리팩토링 및 고도화 계획

📸 스크린샷 (선택)

💬 리뷰 요구사항

Summary by CodeRabbit

  • 새 기능

    • 가격 알림 생성/수정 시 허용 할인율 검증 추가(10, 20, 30, 40만 허용)
  • 버그 수정

    • 실시간 알림 전송 과정의 동시성 문제 완화 — 연결 해제 시 안전한 제거 처리 개선
  • 성능 개선

    • 사용자 미확인 알림을 일괄로 읽음 처리하여 처리 속도 및 효율 향상

- SSE emitter 콜백에서 race condition 방지 (remove(userId) → remove(userId, emitter))
- discountRate 서버 검증 추가 (10/20/30/40 외 값 400 에러 반환)
- markAllAsRead N+1 개선 (벌크 UPDATE 쿼리로 변경)
- INVALID_DISCOUNT_RATE 에러 코드 추가 (ALERT-002)

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
@coderabbitai
Copy link
Copy Markdown
Contributor

coderabbitai bot commented Mar 4, 2026

Caution

Review failed

The pull request is closed.

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

Run ID: 695c4a28-d6bb-4e7e-ab18-6805ee412b05

📥 Commits

Reviewing files that changed from the base of the PR and between b16da1d and f21badc.

📒 Files selected for processing (1)
  • src/main/java/com/ongil/backend/domain/notification/service/NotificationSseService.java

Walkthrough

알림 저장소에 사용자별 미확인 알림을 일괄 읽음으로 표시하는 벌크 업데이트 메서드를 추가했고, 서비스는 이를 사용하도록 변경했습니다. SSE 에미터 제거 시 값 기반 삭제로 경합을 방지하도록 수정했으며, 가격 알림 생성/수정에 할인율 유효성 검증과 에러 코드를 추가했습니다.

Changes

Cohort / File(s) Summary
알림 벌크 업데이트
src/main/java/com/ongil/backend/domain/notification/repository/NotificationRepository.java, src/main/java/com/ongil/backend/domain/notification/service/NotificationService.java
@Modifying/@Query 기반의 markAllAsRead(userId, now) 벌크 업데이트 메서드 추가 및 서비스에서 개별 루프 대신 해당 벌크 호출로 변경
SSE 에미터 정리 (race-condition 방지)
src/main/java/com/ongil/backend/domain/notification/service/NotificationSseService.java
완료/타임아웃/에러 처리 시 emitters.remove(userId, emitter)로 변경하여 값 기반 제거로 경합 보호
가격 알림 할인율 검증 및 에러 코드 추가
src/main/java/com/ongil/backend/domain/pricealert/service/PriceAlertService.java, src/main/java/com/ongil/backend/global/common/exception/ErrorCode.java
허용 할인율 상수(ALLOWED_DISCOUNT_RATES) 추가, createOrUpdate 시 할인율 검증 및 INVALID_DISCOUNT_RATE 에러 코드 추가

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

Possibly related PRs

Suggested labels

🐞 Fix

🚥 Pre-merge checks | ✅ 3
✅ Passed checks (3 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed 제목은 PR의 주요 변경사항(SSE 경합 조건 수정, 할인율 검증 추가, markAllAsRead 벌크 쿼리 최적화)을 정확히 요약하고 있으며, 간결하고 명확합니다.
Docstring Coverage ✅ Passed Docstring coverage is 83.33% which is sufficient. The required threshold is 80.00%.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
  • 📝 Generate docstrings (stacked PR)
  • 📝 Generate docstrings (commit on current branch)
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment
  • Commit unit tests in branch feature/170

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.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

Copy link
Copy Markdown
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 1

🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Inline comments:
In
`@src/main/java/com/ongil/backend/domain/notification/service/NotificationSseService.java`:
- Around line 34-47: In NotificationSseService, the IOException catch blocks
currently call emitters.remove(userId) which can remove a newly registered
emitter from another thread; change those two occurrences to
emitters.remove(userId, emitter) so the removal is conditional on the exact
SseEmitter instance (making it consistent with the onCompletion, onTimeout, and
onError callbacks). Locate the IOException handling in the methods that call
emitter.send(...) and replace the key-only remove with the conditional remove
using the same emitter variable.

ℹ️ Review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

Run ID: b7800dda-dd97-45f4-84f0-ece15e24b127

📥 Commits

Reviewing files that changed from the base of the PR and between 7e39a29 and b16da1d.

📒 Files selected for processing (5)
  • src/main/java/com/ongil/backend/domain/notification/repository/NotificationRepository.java
  • src/main/java/com/ongil/backend/domain/notification/service/NotificationService.java
  • src/main/java/com/ongil/backend/domain/notification/service/NotificationSseService.java
  • src/main/java/com/ongil/backend/domain/pricealert/service/PriceAlertService.java
  • src/main/java/com/ongil/backend/global/common/exception/ErrorCode.java

emitters.remove(userId) → remove(userId, emitter)로 통일
subscribe()와 sendNotification()의 IOException catch 블록 누락 수정

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
@kangcheolung kangcheolung merged commit 136de6c into develop Mar 4, 2026
1 check was pending
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant