[hotfix] 알림 읽음 처리 api 수정#313
Merged
Merged
Conversation
- 이미 읽음 처리된 알림에 대한 요청을 받더라도 정상 응답 하도록 수정
2 tasks
Walkthrough알림 읽음 처리 흐름을 수정했다. 이미 읽은 알림에 대해서 예외를 무시하고 리다이렉트 데이터로 응답하도록 서비스 로직을 변경했다. 스웨거 응답 설명과 컨트롤러 API 설명을 이에 맞게 조정했다. 관련 테스트는 에러 경로 제거 및 리다이렉트 데이터 검증으로 갱신했다. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
actor U as User
participant C as NotificationCommandController
participant S as NotificationMarkService
participant P as NotificationCommandPort
participant N as Notification (Domain)
U->>C: POST /notifications/{id}/read
C->>S: markToChecked(userId, notificationId)
S->>N: markToChecked()
alt 아직 미읽음
N-->>S: 상태 변경 성공
S->>P: update(notification)
P-->>S: ok
S-->>C: RedirectSpec 반환
C-->>U: 200 OK + redirect data
else 이미 읽음
N-->>S: InvalidStateException
S-->>C: (예외 무시) RedirectSpec 반환
C-->>U: 200 OK + redirect data
end
opt 권한 없음
S-->>C: AccessForbidden
C-->>U: 403 Forbidden
end
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Suggested labels
Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ 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. Comment |
Test Results479 tests 479 ✅ 41s ⏱️ Results for commit 6cb40fe. |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
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 (1)
src/main/java/konkuk/thip/notification/application/service/NotificationMarkService.java (1)
33-37: 긴급: Notification.getRedirectSpec() 널-체크 필요 (NPE 위험, NotificationMarkService.java:33-37)notification.getRedirectSpec().route()가 redirectSpec == null일 때 NPE가 발생합니다. 레포에서 redirectSpec이 누락되거나 null로 생성되는 지점이 확인되었습니다:
- src/main/java/konkuk/thip/message/application/service/RoomNotificationDispatchService.java — Notification.builder()...build() (97-100)
- src/main/java/konkuk/thip/message/application/service/FeedNotificationDispatchService.java — Notification.builder()...build() (88)
- src/test/java/konkuk/thip/notification/domain/NotificationTest.java — .redirectSpec(null) 사용 (50)
- src/main/java/konkuk/thip/common/util/TestEntityFactory.java — createNotification(...)에서 redirectSpec 미설정 빌드 (376-384)
조치(택1): 빌더/데이터로 redirectSpec 존재 보장 또는 널-세이프 처리 적용. 간단한 널-세이프 예시:
- return new NotificationMarkToCheckedResponse( - notification.getRedirectSpec().route(), - notification.getRedirectSpec().params() - ); + var spec = notification.getRedirectSpec(); + if (spec == null) { + // TODO: 합의된 기본값으로 변경하거나 data=null 정책 적용 + return new NotificationMarkToCheckedResponse(null, Map.of()); + } + return new NotificationMarkToCheckedResponse(spec.route(), spec.params());
🧹 Nitpick comments (4)
src/main/java/konkuk/thip/common/swagger/SwaggerResponseDescription.java (1)
358-358: 오타 수정 제안: 'Notiification' → 'Notification'사용자 문서에 노출되는 주석/섹션명은 철자를 맞추는 것이 좋습니다.
- // Notiification + // Notificationsrc/main/java/konkuk/thip/notification/application/service/NotificationMarkService.java (1)
26-31: 예외 삼킴 범위를 축소하세요 — 이미 읽음만 무시, 나머지는 전파InvalidStateException을 전부 무시하면 다른 유효하지 않은 상태도 조용히 통과할 수 있습니다. “이미 읽음” 케이스만 선별해 무시하고, 그 외 상태는 재던지도록 좁혀주세요.
가능한 접근:
- 도메인에 isChecked()가 있으면 사전 가드로 예외 자체를 피합니다.
- “이미 읽음”에 한정된 전용 예외(예: AlreadyCheckedException)로 분리하거나, 예외 내 구분자(에러코드 등)로 필터링 후 나머지는 rethrow.
예시(구분자 필터링이 가능한 경우):
try { notification.markToChecked(); notificationCommandPort.update(notification); - } catch (InvalidStateException e) { - // 이미 알림 읽음 처리된 경우 -> 무시 - } + } catch (InvalidStateException e) { + // 이미 읽음 상태만 무시, 그 외는 전파 + if (!e.getMessage().contains("ALREADY_CHECKED")) { // 예: 구분자 사용 (실제 구현에 맞게 교체) + throw e; + } + }또는 (사전 가드가 가능한 경우):
- try { - notification.markToChecked(); - notificationCommandPort.update(notification); - } catch (InvalidStateException e) { - // 이미 알림 읽음 처리된 경우 -> 무시 - } + if (!notification.isChecked()) { + notification.markToChecked(); + notificationCommandPort.update(notification); + }src/main/java/konkuk/thip/notification/adapter/in/web/NotificationCommandController.java (1)
70-72: 설명 보강 제안: 응답 코드/특성 명시이미 읽음인 경우에도 200(OK)과 동일 스키마로 응답하는 “멱등(idempotent)” 동작임을 설명에 명시하면, 클라이언트 구현과 문서가 더 명확해집니다.
예: “이미 읽음인 경우에도 200 OK와 동일한 redirect 데이터 스키마를 반환합니다(멱등).”
src/test/java/konkuk/thip/notification/adapter/in/web/NotificationMarkToCheckedApiTest.java (1)
115-115: 주석 업데이트 필요: “→ 400” → “→ 200”설명 주석이 과거 기대값(400)으로 남아있습니다. 현재 기대값(200)에 맞게 수정하세요.
- // when & then: 다시 읽음 처리 시도 → 400 + // when & then: 다시 읽음 처리 시도 → 200(OK)
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
Disabled knowledge base sources:
- Jira integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (4)
src/main/java/konkuk/thip/common/swagger/SwaggerResponseDescription.java(1 hunks)src/main/java/konkuk/thip/notification/adapter/in/web/NotificationCommandController.java(1 hunks)src/main/java/konkuk/thip/notification/application/service/NotificationMarkService.java(2 hunks)src/test/java/konkuk/thip/notification/adapter/in/web/NotificationMarkToCheckedApiTest.java(2 hunks)
🧰 Additional context used
🧠 Learnings (1)
📓 Common learnings
Learnt from: seongjunnoh
PR: THIP-TextHip/THIP-Server#113
File: src/main/java/konkuk/thip/recentSearch/adapter/out/persistence/RecentSearchCommandPersistenceAdapter.java:38-44
Timestamp: 2025-07-30T14:05:04.945Z
Learning: seongjunnoh는 코드 최적화 제안에 대해 구체적인 기술적 근거와 효율성 차이를 이해하고 싶어하며, 성능 개선 방식에 대한 상세한 설명을 선호한다.
Learnt from: seongjunnoh
PR: THIP-TextHip/THIP-Server#93
File: src/main/java/konkuk/thip/room/adapter/out/persistence/RoomQueryPersistenceAdapter.java:49-114
Timestamp: 2025-07-28T16:44:31.224Z
Learning: seongjunnoh는 코드 중복 문제에 대한 리팩토링 제안을 적극적으로 수용하고 함수형 인터페이스를 활용한 해결책을 선호한다.
Learnt from: seongjunnoh
PR: THIP-TextHip/THIP-Server#285
File: src/main/java/konkuk/thip/room/adapter/out/jpa/RoomStatus.java:1-7
Timestamp: 2025-08-31T05:25:14.845Z
Learning: seongjunnoh는 enum 의존성에 대해 유연한 접근을 선호하며, 도메인→어댑터 레이어 참조와 같은 아키텍처 layering 원칙보다 실용적인 구현을 우선시한다.
Learnt from: seongjunnoh
PR: THIP-TextHip/THIP-Server#195
File: src/main/java/konkuk/thip/feed/application/mapper/FeedQueryMapper.java:0-0
Timestamp: 2025-08-13T05:22:32.287Z
Learning: seongjunnoh는 데이터 무결성과 중복 방지에 대한 고민이 깊으며, LinkedHashSet을 활용한 중복 제거와 순서 보장을 동시에 달성하는 솔루션을 선호한다.
Learnt from: seongjunnoh
PR: THIP-TextHip/THIP-Server#305
File: src/main/java/konkuk/thip/notification/adapter/out/persistence/repository/NotificationQueryRepository.java:9-13
Timestamp: 2025-09-17T06:40:49.863Z
Learning: seongjunnoh는 메서드 네이밍 시 구현 세부사항보다 비즈니스 의도를 반영하는 것을 선호하며, 미래 확장성과 아키텍처 변화를 고려한 설계 철학을 가지고 있다.
Learnt from: seongjunnoh
PR: THIP-TextHip/THIP-Server#278
File: src/main/java/konkuk/thip/TestTokenController.java:0-0
Timestamp: 2025-08-24T09:33:52.982Z
Learning: seongjunnoh는 Spring의 ConditionalOnProperty 동작 원리를 정확히 이해하고 있으며, 보안 이슈에 대해서도 실질적인 위험성을 기준으로 판단하는 실용적 접근을 선호한다.
Learnt from: seongjunnoh
PR: THIP-TextHip/THIP-Server#278
File: src/main/java/konkuk/thip/common/exception/code/ErrorCode.java:210-211
Timestamp: 2025-08-24T09:40:48.595Z
Learning: seongjunnoh는 HTTP 상태 코드 선택에 대해 기술적 근거와 코드베이스 내 일관성을 중요하게 생각하며, 구체적인 사례 분석을 통한 설명을 선호한다.
Learnt from: seongjunnoh
PR: THIP-TextHip/THIP-Server#180
File: src/main/java/konkuk/thip/room/application/service/AttendanceCheckCreateService.java:22-40
Timestamp: 2025-08-14T09:15:31.371Z
Learning: seongjunnoh는 기술적 완벽성보다 실제 비즈니스 시나리오와 사용자 행동 패턴을 우선 고려하며, 발생 가능성이 낮은 엣지 케이스보다는 실용적인 구현을 선호한다.
Learnt from: seongjunnoh
PR: THIP-TextHip/THIP-Server#112
File: src/main/java/konkuk/thip/feed/adapter/out/persistence/repository/FeedQueryRepositoryImpl.java:272-272
Timestamp: 2025-07-30T10:44:34.115Z
Learning: seongjunnoh는 피드 커서 페이지네이션에서 LocalDateTime 단일 커서 방식을 선호하며, 복합 키 기반 커서보다 구현 단순성과 성능을 우선시한다.
🧬 Code graph analysis (1)
src/test/java/konkuk/thip/notification/adapter/in/web/NotificationMarkToCheckedApiTest.java (1)
src/test/java/konkuk/thip/common/util/TestEntityFactory.java (1)
TestEntityFactory(35-403)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: build
🔇 Additional comments (4)
src/test/java/konkuk/thip/notification/adapter/in/web/NotificationMarkToCheckedApiTest.java (4)
95-96: 테스트 시나리오 갱신 적절이미 읽음 케이스를 성공(200) 경로로 전환한 테스트 설명과 흐름이 변경 의도와 일치합니다.
100-103: redirectSpec 준비 좋습니다리다이렉트 목적지를 명시적으로 부여해 응답 구조를 검증하는 방식이 명확합니다.
105-105: 엔티티 생성 시 redirectSpec 포함 OK프로덕션 경로와 동일 전제(redirectSpec 존재)를 테스트에 반영해 회귀 위험을 줄였습니다.
121-123: 응답 검증 포인트 적절route와 params.userId를 검증해 계약을 충분히 커버합니다.
Comment on lines
381
to
383
| NOTIFICATION_NOT_FOUND, | ||
| NOTIFICATION_ACCESS_FORBIDDEN, | ||
| NOTIFICATION_ALREADY_CHECKED | ||
| NOTIFICATION_ACCESS_FORBIDDEN | ||
| ))), |
There was a problem hiding this comment.
🧩 Analysis chain
스펙 정합성 OK — 이미 읽음 에러 제거가 의도와 일치합니다
NOTIFICATION_MARK_TO_CHECKED에서 NOTIFICATION_ALREADY_CHECKED 제거는 서비스/컨트롤러/테스트 변경과 일관됩니다.
레거시 참조가 남아있지 않은지 확인 부탁드립니다. 아래 스크립트로 잔존 참조를 점검할 수 있습니다.
🏁 Script executed:
#!/bin/bash
# NOTIFICATION_ALREADY_CHECKED 사용 흔적 검사
rg -nC2 'NOTIFICATION_ALREADY_CHECKED'Length of output: 2752
레거시 참조 제거 필요 — NOTIFICATION_ALREADY_CHECKED 참조가 남아있음
Swagger 변경(이미 읽음 에러 제거)의 의도대로라면 아래 참조들을 제거하거나 스펙에 맞게 수정하세요.
- src/main/java/konkuk/thip/common/exception/code/ErrorCode.java:238 — NOTIFICATION_ALREADY_CHECKED 정의
- src/main/java/konkuk/thip/notification/domain/Notification.java:11(import), 51(throw) — 사용 위치
- src/test/java/konkuk/thip/notification/domain/NotificationTest.java:9(import), 40(assertion) — 테스트에서 사용
해당 참조들을 삭제하거나 테스트·도메인 로직을 스펙(이미 읽음 에러 제거)에 맞게 업데이트하세요.
🤖 Prompt for AI Agents
In src/main/java/konkuk/thip/common/swagger/SwaggerResponseDescription.java
around lines 381-383, the code still references a legacy error
NOTIFICATION_ALREADY_CHECKED which should be removed per the recent spec change;
remove that enum constant usage here and then delete or refactor all remaining
references: remove NOTIFICATION_ALREADY_CHECKED from
src/main/java/konkuk/thip/common/exception/code/ErrorCode.java (line ~238),
remove the import and any throw statements in
src/main/java/konkuk/thip/notification/domain/Notification.java (remove the
import at line 11 and the throw at line 51 or replace with the new behavior),
and update src/test/java/konkuk/thip/notification/domain/NotificationTest.java
by removing the import at line 9 and adjusting the assertion at line 40 to
reflect the new spec (either assert success/no-error or the new error type);
ensure compilation and tests pass after these deletions/adjustments.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
#️⃣ 연관된 이슈
📝 작업 내용
이슈 참고해 주시면 됩니다.
개발서버에서 제대로 동작하는것 확인했습니다
📸 스크린샷
💬 리뷰 요구사항
📌 PR 진행 시 이러한 점들을 참고해 주세요
Summary by CodeRabbit