feat: 알림 목록·읽음 API - #273
Conversation
- 앱 알림 화면이 하드코딩 데이터로 돌고 있어(홈 배지도 그 가짜 목록을 셌다) 조회·읽음을 서버에 만든다 - 알림 종류는 TRIP_TOMORROW 하나만 정의했다. 서버가 지금 실제로 만들 근거가 있는 것이 그것뿐이라, 쓰지 않을 종류를 미리 나열하면 앱에 영영 오지 않는 분기가 남는다 - 프론트 초안의 title·body 를 응답에서 뺐다. 프론트가 "type 에 아이콘·문구를 맞추겠다" 고 한 것을 계약에 반영한 것이다. 문구를 서버에 굳히면 표현을 고칠 때 이미 쌓인 알림이 옛 문구로 남아 화면에 두 세대가 섞인다(OpeningStatus 와 같은 방식으로 enum 이름만 싣는다) - 지역명을 대신 실어주는 안은 접었다. 알림 하나당 코스·지역을 되짚는 조회가 붙는데, 앱은 내 코스 목록에서 이미 그 값을 갖고 있다 - 없는 알림과 남의 알림을 똑같이 404 로 답한다. 403 으로 나누면 ID 를 훑어 남의 알림 존재를 확인할 수 있다(코스 상세와 같은 규칙) - 이미 읽은 알림의 재요청은 성공으로 둔다. 원한 상태가 이미 이뤄져 있고, 알림 화면은 같은 요청을 두 번 보내기 쉬운 자리다 - 읽음 응답에도 안읽음 수를 싣는다. 안 주면 배지 하나 때문에 목록을 다시 부른다 - 안읽음 수는 페이지와 무관한 전체 수다. 페이지 안에서 세면 첫 페이지 크기에서 배지가 멈춘다 - 알림은 쌓이는 데이터라 목록을 page·size 로 끊었다. 정렬 tie-break 에 id 를 넣어 같은 초에 만들어진 알림이 페이지 경계에서 겹치거나 빠지지 않게 했다 - 전체 읽음은 벌크 UPDATE 다. 행을 다 읽어 하나씩 고치면 쌓인 만큼 힙에 올린다 - 읽음을 boolean 이 아니라 시각으로 저장한다. 비용이 같은데 "언제 읽었나" 까지 답한다 - 소유 키는 코스·연차와 같은 X-Guest-Id 다. 인증이 붙는 날 세 테이블을 함께 옮기는 것이 안전하다
- 남의 알림은 404 응답만 보지 않고, 그 알림이 그대로 안 읽음으로 남는지까지 단언한다. 404 를 준 뒤 조용히 읽음 처리해버리는 것이 최악이라 그쪽을 잠근다 - 전체 읽음이 남의 알림을 건드리지 않는지 따로 본다 - 안읽음 수가 페이지 크기에 갇히지 않는지 size=1 로 3건을 조회해 확인한다 - 응답 DTO 매핑 단위 테스트는 의도적으로 두지 않았다. readAt → read 접기 말고는 옮겨 담기뿐이고, 그 접기는 도메인 단위 테스트와 통합 테스트의 read 단언이 양쪽에서 잡는다. (처음엔 두려 했으나 영속 전 엔티티는 id 가 없어 응답 record 의 long id 를 채울 수 없었다) - 통합 테스트에 클래스 레벨 @transactional 을 걸지 않는다. 컨벤션 훅이 controller 패키지의 @transactional 을 막고, 다른 컨트롤러 통합 테스트도 소유자를 갈라 격리한다
|
Warning Review limit reached
Next review available in: 78 minutes You've used all free OSS reviews for now. Wait for the free limit to reset to keep reviewing this public repository. How can I continue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews. How do review limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (17)
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 |
- createdAt 이 같은 5건을 size=2 로 세 페이지 훑어, 이어 붙인 결과가 넣은 것과 정확히 일치하는지(중복·누락 없음, 최신순) 확인한다 - 원래 의도는 PR 이 주장한 id 2차 정렬을 잠그는 것이었으나, 실측 결과 쿼리에서 IdDesc 를 떼도 이 테스트는 통과한다. 인덱스가 (guest_id, created_at) 이고 InnoDB 가 그 뒤에 PK 를 붙여, 인덱스 역순 스캔 플랜에서는 id 내림차순이 우연히 따라오기 때문이다. 잘못된 초록을 남기지 않도록 그 사실을 테스트 주석에 적었다 - tie-break 를 지키는 것은 테스트가 아니라 쿼리의 ORDER BY 라는 점도 함께 남겼다
리뷰 — 알림 목록·읽음 API (#263)CodeRabbit 이 레이트리밋이라 대신 봤다. 주장 하나하나를 코드와 실행으로 확인했고, 확인 못 한 것은 그렇다고 적었다. 검증한 것
좋았던 점 (근거를 확인한 것만)
① 응답에서
|
| 안 | 내용 | 대가 |
|---|---|---|
| A. 현행 유지 | 앱이 courseId 로 지역명을 찾는다 |
위 1~4 각각에 앱 쪽 fallback 문구가 필요 — "여행이 내일이에요" 같은 지역명 없는 문구를 프론트가 준비해야 한다 |
| B. 지역명만 응답에 싣는다 | regionName(또는 regionId) 을 알림에 함께 |
문구가 아니라 데이터라 옛 문구 문제가 없다. 대신 알림 생성 시점의 지역명이 굳는다(지역명은 사실상 안 바뀐다). 알림 만들 때 한 번 채우면 조회 시 추가 조회는 없다 |
| C. 도메인이 불변식으로 강제 | TRIP_TOMORROW 는 courseId 필수 |
위 1번만 막는다. 2~4 는 그대로 남는다 |
의견을 붙이면 B + C 가 비용 대비 가장 깨끗해 보인다(생성 시점에 한 번 채우는 것이라 조회 경로에 조회가 늘지 않는다). 다만 "type 만 주면 앱이 맞추겠다" 는 합의가 이미 있으니, 프론트가 fallback 문구를 준비할 수 있다면 A 도 성립한다. 결정은 사람이 한다.
② 페이지 경계 tie-break — 테스트로 잠겨 있지 않았다 (minor, 테스트 추가함)
PR 본문은 "정렬 tie-break 에 id 를 넣어 같은 초에 만들어진 알림이 페이지 경계에서 겹치거나 빠지지 않게 했다" 고 하는데, 그걸 확인하는 테스트가 없었다. size=1 테스트는 createdAt 이 10분씩 벌어져 있어 tie 상황이 아니다.
같은 createdAt 5건을 size=2 로 세 페이지 훑는 테스트를 추가했다(f3cc5d4). 그런데 이 테스트도 tie-break 를 잠그지는 못한다 — 쿼리에서 IdDesc 를 떼고 돌려봤더니 그대로 통과한다. 위에 적은 대로 인덱스가 (guest_id, created_at + PK) 라, 지금 플랜에서는 id 내림차순이 우연히 따라오기 때문이다.
그래서 사실은 이렇다: tie-break 를 지키는 것은 테스트가 아니라 쿼리의 ORDER BY 자체이고, 인덱스나 플랜이 바뀌면 조용히 깨질 수 있는 자리다. 잘못된 초록을 남기지 않으려고 그 한계를 테스트 주석에 그대로 적어 뒀다. 추가한 테스트는 페이지네이션 계약(최신순·중복·누락) 회귀용으로 남긴다.
③ unreadCount 비용 — 실측했고, 문제 없다 (info)
SPRING_JPA_SHOW_SQL 로 실제 statement 를 셌다.
| 요청 | 쿼리 | 비고 |
|---|---|---|
| 목록 (결과가 첫 페이지에 다 들어감) | 2 — content + unreadCount | Spring Data 가 count 쿼리를 생략한다. 알림 20건 미만인 대다수 사용자가 여기 해당 |
| 목록 (페이지가 갈림) | 3 — content + count(*) + unreadCount | |
| 하나 읽음 (처음) | 3 — select + update + count | |
| 하나 읽음 (이미 읽음) | 2 — select + count | update 안 나감 |
| 전체 읽음 | 2 — bulk update + count |
"read-all 뒤에는 항상 0 이라 셀 필요 없다" 는 성립하지 않는다(위 REPEATABLE READ 근거). 늘어난 것은 요청당 인덱스 count 한 번이고, 그 대가로 앱이 목록을 다시 부르지 않는다 — 계산이 맞다. 지적 없음.
④ 그 외 (nit — 고치지 않았다)
PATCH /{id}/read와POST /read-all의 동사가 갈린다. api-convention 은 액션에POST /{id}/action을 든다. 같은 도메인의 같은 동작인데 하나는 PATCH, 하나는 POST 다. 이미 프론트와 합의된 계약이면 그대로 두는 게 낫고, 아직이면 지금이 맞추기 쉬운 때다.Notification.course()를 아무도 안 쓴다. 응답 DTO 는getCourseId()를 직접 읽고,course()는 단위 테스트만 호출한다. 위 ①-C 를 택하면 이 자리가 쓸모를 얻는다.markRead의repository.save(...)는 같은 트랜잭션 안 managed 엔티티라 dirty checking 으로도 반영된다. 동작에 문제는 없고, port 를 명시적으로 거치는 편이 읽기 좋다는 판단이면 그대로 둬도 된다.
고친 것
f3cc5d4test: 알림 목록의 페이지 경계 계약 검증 추가 — 위 ②. 푸시했고, 전체 테스트 1344건 통과를 다시 확인했다.
머지는 하지 않았다.
|
@coderabbitai review |
|
|
@coderabbitai review |
|
|
@coderabbitai review |
|
1 participant
Situation
Task
엔드포인트를 여는 것보다 정할 것이 많았다.
알림을 만드는 일(여행 전날 배치)과 실제 푸시 발송은 이 PR 밖이다. 그래서 계약과 저장소를 먼저 확정하는 것이 이 작업의 전부다.
Action
응답 계약
GET /api/v1/notificationsnotifications[],unreadCount(+ 래퍼의pageResponse)PATCH /api/v1/notifications/{id}/readunreadCountPOST /api/v1/notifications/read-allunreadCount알림 한 건은
id,type,courseId,read,createdAt이다.알림 종류를 하나만 정의했다
TRIP_TOMORROW(내일 여행) 하나다.서버가 지금 실제로 만들 근거가 있는 종류가 그것뿐이다. 여행 날짜를 이미 들고 있어 판단에 외부가 필요 없고, 알림을 받는 시점이 사용자가 할 일(짐 싸기)과 맞는다.
쓰지도 않을 종류를 미리 나열하면 앱이 그 값을 기다리는 분기를 만들고, 영영 오지 않는 분기가 남는다. 값이 느는 것은 클라이언트에 안전한 변경이라 보낼 것이 생길 때 더한다. 추가 기준도 코드에 적어 뒀다: 서버가 그 사실을 이미 알고 있고, 그것을 알릴 주체가 이 레포에 있는가.
앱 쪽에 부탁: 모르는
type은 무시하거나 기본 아이콘으로 그려 달라. 앱을 업데이트하지 않은 사용자에게도 새 종류가 갈 수 있다.프론트 초안에서 문구 필드를 뺐다 (계약 변경)
초안에 있던
title·body를 응답에서 뺐다. 프론트가 "type 에 아이콘·문구를 맞추겠다" 고 한 것을 그대로 계약에 반영한 것이다. 이 레포의OpeningStatus와 같은 방식으로, 응답에는 enum 이름만 싣는다.마지막 줄이 유일한 비용인데, 앱은 이미 그 값을 갖고 있다. 알림은 자기 코스를 가리키고(
courseId), 내 코스 목록 응답에 지역명이 실려 있다. 서버가 알림마다 지역명을 다시 붙이면 알림 하나당 코스·지역을 되짚는 조회가 생긴다.courseId는 없을 수도 있고(코스와 무관한 알림), 지워진 코스를 가리킬 수도 있다. 알림은 코스가 사라져도 남는다.저장
notification테이블 하나. 소유 키는 코스·연차가 쓰는X-Guest-Id를 그대로 따랐다 — 소셜 로그인이 붙는 날 세 테이블을 함께 옮기는 것이 가장 안전하다.read_at)으로 뒀다. 저장 비용이 같은데 "언제 읽었나" 까지 답한다. NULL 이면 안 읽음이다.(guest_id, created_at)하나. 목록은 소유자 안에서 최신순으로만 읽고, 안읽음 개수도 이 인덱스로 소유자 범위까지 좁힌 뒤 훑는다. 한 사람의 알림은 수백 건 규모라 인덱스를 더 두지 않았다.목록을 페이지로 끊었다
알림은 지우지 않고 쌓이는 데이터라, 상한이 없으면 오래 쓴 사용자의 목록 한 번이 계속 커진다. 코스 목록이 실제로 그랬다(#105).
page·size를 규약대로 받고 기본 20·최대 100 은Paging이 단독으로 소유한다. 정렬 tie-break 에id를 넣어 같은 초에 만들어진 알림이 페이지 경계에서 겹치거나 빠지지 않게 했다.검증
size=1로 3건을 조회해 확인한다.readAt→read접기 말고는 옮겨 담기뿐이고, 그 접기는 도메인 단위 테스트와 통합 테스트의read단언이 양쪽에서 잡는다.Result
연관 이슈