Conversation
|
Caution Review failedThe pull request is closed. Note Other AI code review bot(s) detectedCodeRabbit has detected other AI code review bot(s) in this pull request and will avoid duplicating their findings in the review comments. This may lead to a less comprehensive review. Walkthrough그룹 V2 기능에 이벤트 기반 알림 시스템을 도입합니다. 그룹 생성, 참여, 탈퇴, 삭제 등의 주요 작업에 도메인 이벤트를 발행하고, 리스너가 트랜잭션 커밋 후 반응하여 핸들러로 위임되어 알림을 생성 및 전송합니다. Changes
Sequence Diagram(s)sequenceDiagram
participant Service as GroupV2 Service
participant EventBus as Spring Event Bus
participant Listener as Event Listener
participant Handler as Notification Handler
participant Repo as Repository
participant Dispatcher as Notification<br/>Dispatcher
participant SSE as SseEmitterService
Service->>Service: Group action (create/join/leave/etc)
activate Service
Service->>EventBus: publishEvent(GroupXxxEvent)
deactivate Service
EventBus->>Listener: onXxx(event) [AFTER_COMMIT]
activate Listener
Listener->>Handler: handle(event params)
deactivate Listener
activate Handler
Handler->>Repo: findById(userId/groupId)
Repo-->>Handler: Entity
alt Event Type
Handler->>Repo: createNotification(receiver, actor, group)
Repo-->>Handler: Notification
Handler->>Dispatcher: dispatch(notifications, actor, group)
else
Handler->>Repo: createNotification(...)
Repo-->>Handler: Notification
Handler->>SSE: sendNotification(userId, event)
end
activate Dispatcher
Dispatcher->>Repo: save all notifications
Repo-->>Dispatcher: Notification with IDs
Dispatcher->>Dispatcher: build NotificationEvent per notification
Dispatcher->>SSE: sendNotification(userId, event)
deactivate Dispatcher
activate SSE
SSE->>SSE: emit to client
SSE-->>Handler: success/failure
deactivate SSE
deactivate Handler
Estimated code review effort🎯 4 (Complex) | ⏱️ ~50 minutes Possibly related PRs
Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
📜 Recent review detailsConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro 📒 Files selected for processing (12)
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 |
There was a problem hiding this comment.
Actionable comments posted: 12
🧹 Nitpick comments (13)
src/main/java/team/wego/wegobackend/group/v2/application/service/GroupV2Service.java (1)
59-60: 의존성 주입 주석 개선 제안주석 "SSE 이벤트 호출"이 다소 모호합니다.
ApplicationEventPublisher는 Spring의 범용 이벤트 발행 메커니즘이며 SSE에 한정되지 않습니다.🔎 주석 개선 제안
- // SSE 이벤트 호출 + // 도메인 이벤트 발행 (알림 트리거) private final ApplicationEventPublisher eventPublisher;src/main/java/team/wego/wegobackend/group/v2/application/event/GroupCreatedEvent.java (1)
3-6: 이벤트 레코드 필드 검증 고려현재
GroupCreatedEvent레코드는 필드 검증을 수행하지 않습니다. 호출자(GroupV2Service)가 저장 후 ID를 전달하므로 실제로는 null이 전달될 가능성이 낮지만, 방어적 프로그래밍 측면에서 compact constructor에 검증을 추가하는 것을 고려할 수 있습니다.🔎 필드 검증 추가 예시
public record GroupCreatedEvent( Long groupId, Long hostUserId ) { - + public GroupCreatedEvent { + if (groupId == null || hostUserId == null) { + throw new IllegalArgumentException("groupId and hostUserId must not be null"); + } + } }src/main/java/team/wego/wegobackend/group/v2/application/listener/GroupNotificationEventListener.java (1)
18-22: 이벤트 리스너들 전체에서 예외 처리 메커니즘 추가 필요이 리스너를 포함한 모든 이벤트 리스너(
GroupJoinDecisionEventListener,GroupJoinEventListener,GroupJoinRequestEventListener,GroupKickEventListener,GroupDeleteEventListener,GroupLeaveEventListener)에서@TransactionalEventListener(phase = AFTER_COMMIT)단계의 핸들러 호출이 명시적 예외 처리 없이 진행되고 있습니다.현재 설계는 의도적인 eventual consistency 패턴으로 보이지만, 핸들러 실패 시 다음과 같은 문제가 발생할 수 있습니다:
- 알림 전송 실패가 로깅되지 않아 디버깅이 어려움
- 실패한 작업에 대한 재시도 메커니즘이 없음
- 예외가 자동으로 전파되지만 처리 전략이 불명확함
전체 이벤트 리스너에 대해 try-catch와 재시도 로직 또는 실패 로깅 메커니즘을 추가하는 것을 권장합니다.
src/main/java/team/wego/wegobackend/user/repository/FollowRepositoryCustom.java (1)
5-5: 사용되지 않는 import 제거 권장
Userimport가 이 인터페이스에서 사용되지 않습니다. 불필요한 import는 제거하는 것이 좋습니다.🔎 수정 제안
-import team.wego.wegobackend.user.domain.User;src/main/java/team/wego/wegobackend/notification/application/SseEmitterService.java (1)
41-67: 두sendNotification메서드 간 코드 중복 고려
NotificationResponse와NotificationEvent를 받는 두 메서드가 거의 동일한 로직을 가지고 있습니다. 제네릭 메서드나 공통 인터페이스를 활용하여 중복을 줄일 수 있습니다.🔎 리팩토링 예시
- public void sendNotification(Long userId, NotificationResponse notification) { - SseEmitter emitter = emitters.get(userId); - log.debug("Connected emitter Info -> {} ", emitter); - if (emitter != null) { - try { - emitter.send(SseEmitter.event() - .name("notification") - .data(notification)); - } catch (IOException e) { - emitters.remove(userId); - } - } - } - - public void sendNotification(Long userId, NotificationEvent notification) { - SseEmitter emitter = emitters.get(userId); - log.debug("NotificationEvent Connected emitter Info -> {} ", emitter); - if (emitter != null) { - try { - emitter.send(SseEmitter.event() - .name("notification") - .data(notification)); - } catch (IOException e) { - emitters.remove(userId); - } - } - } + public void sendNotification(Long userId, Object notification) { + SseEmitter emitter = emitters.get(userId); + log.debug("Connected emitter Info -> {} ", emitter); + if (emitter != null) { + try { + emitter.send(SseEmitter.event() + .name("notification") + .data(notification)); + } catch (IOException e) { + emitters.remove(userId); + } + } + }src/main/java/team/wego/wegobackend/group/v2/application/listener/GroupJoinRequestEventListener.java (1)
2-3: 불필요한 빈 줄패키지 선언 후 두 개의 빈 줄이 있습니다. 다른 리스너 파일들과 일관성을 위해 하나만 유지하는 것이 좋습니다.
🔎 수정 제안
package team.wego.wegobackend.group.v2.application.listener; - import lombok.RequiredArgsConstructor;src/main/java/team/wego/wegobackend/group/v2/application/handler/GroupCreateNotificationHandler.java (1)
38-42: 예외 처리 일관성 검토
orElseThrow()가 커스텀 예외 없이 사용되어NoSuchElementException이 발생합니다. 다른 핸들러(GroupLeaveNotificationHandler,GroupJoinNotificationHandler)에서는UserNotFoundException을 사용하고 있어 일관성이 부족합니다.🔎 수정 제안
User host = userRepository.findById(hostUserId) - .orElseThrow(); + .orElseThrow(UserNotFoundException::new); GroupV2 group = groupV2Repository.findById(groupId) - .orElseThrow(); + .orElseThrow(() -> new GroupNotFoundException(groupId));src/main/java/team/wego/wegobackend/notification/application/dispatcher/NotificationDispatcher.java (1)
42-48: SSE 전송 실패 처리 고려 필요
sseEmitterService.sendNotification이 예외를 던질 경우 루프가 중단되어 이후 알림이 전송되지 않을 수 있습니다. 각 수신자에 대한 SSE 전송 실패가 다른 수신자의 알림 전송에 영향을 주지 않도록 예외 처리를 추가하는 것을 권장합니다.🔎 수정 제안
// SSE 전송 for (Notification notification : notifications) { - NotificationEvent event = - NotificationEvent.of(notification, actor, group); - - Long receiverId = notification.getReceiver().getId(); - sseEmitterService.sendNotification(receiverId, event); + try { + NotificationEvent event = + NotificationEvent.of(notification, actor, group); + + Long receiverId = notification.getReceiver().getId(); + sseEmitterService.sendNotification(receiverId, event); + } catch (Exception e) { + log.warn("[NOTI] SSE send failed for notification id={}", notification.getId(), e); + // 실패해도 계속 진행 + } }src/main/java/team/wego/wegobackend/user/repository/FollowRepositoryImpl.java (1)
76-76: 커서 조건 처리 일관성 개선 제안Line 42에서는
itCursor(cursorFollowId)헬퍼 메서드를 사용하는 반면, 여기서는 인라인 조건cursorFollowId == null ? null : follow.id.lt(cursorFollowId)을 사용하고 있습니다. 일관성을 위해 헬퍼 메서드를 재사용하는 것을 권장합니다.🔎 수정 제안
.where( follow.followee.id.eq(followeeId), - cursorFollowId == null ? null : follow.id.lt(cursorFollowId), + itCursor(cursorFollowId), user.notificationEnabled.isTrue(), user.deleted.isFalse() )src/main/java/team/wego/wegobackend/group/v2/application/handler/GroupDeleteNotificationHandler.java (2)
30-31: 예외 처리 구체화 권장
orElseThrow()호출 시 커스텀 예외를 사용하지 않아 기본NoSuchElementException이 발생합니다. 다른 핸들러들과 일관성을 위해 명확한 예외 타입을 사용하는 것을 권장합니다.🔎 수정 제안
- User host = userRepository.findById(hostUserId).orElseThrow(); - GroupV2 group = groupV2Repository.findById(groupId).orElseThrow(); + User host = userRepository.findById(hostUserId) + .orElseThrow(UserNotFoundException::new); + GroupV2 group = groupV2Repository.findById(groupId) + .orElseThrow(() -> new GroupException(GroupErrorCode.GROUP_NOT_FOUND_BY_ID, groupId));
36-36: 주석 명확화 필요"호스트 자신 제외(원하면 제거)" 주석이 요구사항이 불명확함을 나타냅니다. 호스트에게 자신의 모임 삭제 알림을 보낼지 여부를 명확히 정하고, 확정된 정책에 따라 주석을 업데이트하거나 제거하는 것을 권장합니다.
src/main/java/team/wego/wegobackend/group/v2/application/event/NotificationEvent.java (1)
30-40:actor파라미터에 대한 null 처리 불일치
group파라미터는 Line 38에서 명시적으로 null 체크를 수행하지만,actor파라미터는 직접ActorUser.from(actor)로 전달됩니다.ActorUser.from()이 내부적으로 null을 처리하긴 하지만,of()메서드 레벨에서 일관된 null 처리 패턴을 적용하는 것이 더 명확합니다.🔎 일관된 null 처리 제안
public static NotificationEvent of(Notification n, User actor, GroupV2 group) { return NotificationEvent.builder() .id(n.getId()) .message(n.getMessage()) .type(n.getType()) .createdAt(n.getCreatedAt()) .readAt(n.getReadAt()) - .user(ActorUser.from(actor)) + .user(actor != null ? ActorUser.from(actor) : null) .group(group != null ? GroupInfo.from(group) : null) .build(); }src/main/java/team/wego/wegobackend/notification/domain/Notification.java (1)
96-197:relatedType문자열 상수화 권장모든 GROUP 관련 알림에서
"GROUP"문자열이 반복 사용되고 있습니다. 오타 방지 및 유지보수성을 위해 상수로 추출하는 것을 권장합니다.🔎 상수 추출 예시
private static final String RELATED_TYPE_GROUP = "GROUP"; // 사용 예시 .relatedType(RELATED_TYPE_GROUP)또는
RelatedTypeenum을 정의하여 타입 안전성을 높일 수 있습니다.
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (40)
src/main/java/team/wego/wegobackend/group/v2/application/event/GroupCreatedEvent.javasrc/main/java/team/wego/wegobackend/group/v2/application/event/GroupDeletedEvent.javasrc/main/java/team/wego/wegobackend/group/v2/application/event/GroupJoinApprovedEvent.javasrc/main/java/team/wego/wegobackend/group/v2/application/event/GroupJoinKickedEvent.javasrc/main/java/team/wego/wegobackend/group/v2/application/event/GroupJoinRejectedEvent.javasrc/main/java/team/wego/wegobackend/group/v2/application/event/GroupJoinRequestedEvent.javasrc/main/java/team/wego/wegobackend/group/v2/application/event/GroupJoinedEvent.javasrc/main/java/team/wego/wegobackend/group/v2/application/event/GroupLeftEvent.javasrc/main/java/team/wego/wegobackend/group/v2/application/event/NotificationEvent.javasrc/main/java/team/wego/wegobackend/group/v2/application/handler/GroupCreateNotificationHandler.javasrc/main/java/team/wego/wegobackend/group/v2/application/handler/GroupDeleteNotificationHandler.javasrc/main/java/team/wego/wegobackend/group/v2/application/handler/GroupJoinDecisionNotificationHandler.javasrc/main/java/team/wego/wegobackend/group/v2/application/handler/GroupJoinNotificationHandler.javasrc/main/java/team/wego/wegobackend/group/v2/application/handler/GroupJoinRequestNotificationHandler.javasrc/main/java/team/wego/wegobackend/group/v2/application/handler/GroupKickNotificationHandler.javasrc/main/java/team/wego/wegobackend/group/v2/application/handler/GroupLeaveNotificationHandler.javasrc/main/java/team/wego/wegobackend/group/v2/application/listener/GroupDeleteEventListener.javasrc/main/java/team/wego/wegobackend/group/v2/application/listener/GroupJoinDecisionEventListener.javasrc/main/java/team/wego/wegobackend/group/v2/application/listener/GroupJoinEventListener.javasrc/main/java/team/wego/wegobackend/group/v2/application/listener/GroupJoinRequestEventListener.javasrc/main/java/team/wego/wegobackend/group/v2/application/listener/GroupKickEventListener.javasrc/main/java/team/wego/wegobackend/group/v2/application/listener/GroupLeaveEventListener.javasrc/main/java/team/wego/wegobackend/group/v2/application/listener/GroupNotificationEventListener.javasrc/main/java/team/wego/wegobackend/group/v2/application/service/GroupV2AttendanceService.javasrc/main/java/team/wego/wegobackend/group/v2/application/service/GroupV2DeleteService.javasrc/main/java/team/wego/wegobackend/group/v2/application/service/GroupV2Service.javasrc/main/java/team/wego/wegobackend/group/v2/domain/repository/GroupUserV2Repository.javasrc/main/java/team/wego/wegobackend/notification/application/SseEmitterService.javasrc/main/java/team/wego/wegobackend/notification/application/dispatcher/NotificationDispatcher.javasrc/main/java/team/wego/wegobackend/notification/application/dto/NotificationType.javasrc/main/java/team/wego/wegobackend/notification/application/dto/response/NotificationEventResponse.javasrc/main/java/team/wego/wegobackend/notification/domain/Notification.javasrc/main/java/team/wego/wegobackend/notification/presentation/NotificationController.javasrc/main/java/team/wego/wegobackend/user/repository/FollowRepositoryCustom.javasrc/main/java/team/wego/wegobackend/user/repository/FollowRepositoryImpl.javasrc/main/java/team/wego/wegobackend/user/repository/query/FollowerNotifyRow.javasrc/test/http/group/group-attend.httpsrc/test/http/group/v2/V2-group-left.httpsrc/test/http/group/v2/v2-group-create.httpsrc/test/http/group/v2/v2-group-left.http
💤 Files with no reviewable changes (2)
- src/test/http/group/v2/V2-group-left.http
- src/main/java/team/wego/wegobackend/notification/presentation/NotificationController.java
🧰 Additional context used
🧬 Code graph analysis (12)
src/main/java/team/wego/wegobackend/group/v2/application/listener/GroupJoinEventListener.java (4)
src/main/java/team/wego/wegobackend/group/v2/application/listener/GroupDeleteEventListener.java (1)
Component(10-20)src/main/java/team/wego/wegobackend/group/v2/application/listener/GroupJoinDecisionEventListener.java (1)
Component(11-26)src/main/java/team/wego/wegobackend/group/v2/application/listener/GroupJoinRequestEventListener.java (1)
Component(11-21)src/main/java/team/wego/wegobackend/group/v2/application/listener/GroupKickEventListener.java (1)
Component(10-20)
src/main/java/team/wego/wegobackend/notification/application/dispatcher/NotificationDispatcher.java (3)
src/main/java/team/wego/wegobackend/group/v2/application/handler/GroupCreateNotificationHandler.java (1)
Slf4j(20-76)src/main/java/team/wego/wegobackend/group/v2/application/handler/GroupDeleteNotificationHandler.java (1)
Slf4j(17-44)src/main/java/team/wego/wegobackend/notification/application/SseEmitterService.java (1)
Slf4j(12-68)
src/main/java/team/wego/wegobackend/group/v2/application/listener/GroupDeleteEventListener.java (4)
src/main/java/team/wego/wegobackend/group/v2/application/listener/GroupJoinDecisionEventListener.java (1)
Component(11-26)src/main/java/team/wego/wegobackend/group/v2/application/listener/GroupJoinEventListener.java (1)
Component(10-20)src/main/java/team/wego/wegobackend/group/v2/application/listener/GroupJoinRequestEventListener.java (1)
Component(11-21)src/main/java/team/wego/wegobackend/group/v2/application/listener/GroupKickEventListener.java (1)
Component(10-20)
src/main/java/team/wego/wegobackend/notification/application/dto/response/NotificationEventResponse.java (1)
src/main/java/team/wego/wegobackend/group/v2/application/event/NotificationEvent.java (3)
Getter(12-73)Getter(42-57)Getter(59-72)
src/main/java/team/wego/wegobackend/group/v2/application/handler/GroupLeaveNotificationHandler.java (3)
src/main/java/team/wego/wegobackend/group/v2/application/listener/GroupLeaveEventListener.java (1)
Slf4j(11-23)src/main/java/team/wego/wegobackend/notification/application/dispatcher/NotificationDispatcher.java (1)
Slf4j(16-50)src/main/java/team/wego/wegobackend/group/v2/application/handler/GroupJoinNotificationHandler.java (1)
Service(17-38)
src/main/java/team/wego/wegobackend/group/v2/application/listener/GroupLeaveEventListener.java (5)
src/main/java/team/wego/wegobackend/group/v2/application/listener/GroupNotificationEventListener.java (1)
Slf4j(11-23)src/main/java/team/wego/wegobackend/group/v2/application/listener/GroupDeleteEventListener.java (1)
Component(10-20)src/main/java/team/wego/wegobackend/group/v2/application/listener/GroupJoinEventListener.java (1)
Component(10-20)src/main/java/team/wego/wegobackend/group/v2/application/listener/GroupJoinRequestEventListener.java (1)
Component(11-21)src/main/java/team/wego/wegobackend/group/v2/application/listener/GroupKickEventListener.java (1)
Component(10-20)
src/main/java/team/wego/wegobackend/group/v2/application/listener/GroupKickEventListener.java (4)
src/main/java/team/wego/wegobackend/group/v2/application/listener/GroupDeleteEventListener.java (1)
Component(10-20)src/main/java/team/wego/wegobackend/group/v2/application/listener/GroupJoinDecisionEventListener.java (1)
Component(11-26)src/main/java/team/wego/wegobackend/group/v2/application/listener/GroupJoinEventListener.java (1)
Component(10-20)src/main/java/team/wego/wegobackend/group/v2/application/listener/GroupJoinRequestEventListener.java (1)
Component(11-21)
src/main/java/team/wego/wegobackend/group/v2/application/handler/GroupKickNotificationHandler.java (3)
src/main/java/team/wego/wegobackend/group/v2/application/handler/GroupJoinDecisionNotificationHandler.java (1)
Service(17-54)src/main/java/team/wego/wegobackend/group/v2/application/handler/GroupJoinNotificationHandler.java (1)
Service(17-38)src/main/java/team/wego/wegobackend/group/v2/application/handler/GroupJoinRequestNotificationHandler.java (1)
Service(17-43)
src/main/java/team/wego/wegobackend/group/v2/application/service/GroupV2Service.java (3)
src/main/java/team/wego/wegobackend/group/v2/application/handler/GroupCreateNotificationHandler.java (1)
Slf4j(20-76)src/main/java/team/wego/wegobackend/group/v2/application/listener/GroupNotificationEventListener.java (1)
Slf4j(11-23)src/main/java/team/wego/wegobackend/group/v2/application/service/GroupV2AttendanceService.java (1)
Slf4j(35-609)
src/main/java/team/wego/wegobackend/group/v2/application/handler/GroupJoinDecisionNotificationHandler.java (3)
src/main/java/team/wego/wegobackend/group/v2/application/handler/GroupJoinNotificationHandler.java (1)
Service(17-38)src/main/java/team/wego/wegobackend/group/v2/application/handler/GroupJoinRequestNotificationHandler.java (1)
Service(17-43)src/main/java/team/wego/wegobackend/group/v2/application/handler/GroupKickNotificationHandler.java (1)
Service(17-38)
src/main/java/team/wego/wegobackend/group/v2/application/listener/GroupJoinRequestEventListener.java (4)
src/main/java/team/wego/wegobackend/group/v2/application/listener/GroupDeleteEventListener.java (1)
Component(10-20)src/main/java/team/wego/wegobackend/group/v2/application/listener/GroupJoinDecisionEventListener.java (1)
Component(11-26)src/main/java/team/wego/wegobackend/group/v2/application/listener/GroupJoinEventListener.java (1)
Component(10-20)src/main/java/team/wego/wegobackend/group/v2/application/listener/GroupKickEventListener.java (1)
Component(10-20)
src/main/java/team/wego/wegobackend/group/v2/application/event/NotificationEvent.java (1)
src/main/java/team/wego/wegobackend/notification/application/dto/response/NotificationEventResponse.java (3)
Getter(11-62)Getter(33-47)Getter(49-61)
⏰ 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). (2)
- GitHub Check: Agent
- GitHub Check: CodeQL analysis (java)
🔇 Additional comments (35)
src/test/http/group/group-attend.http (1)
49-49: LGTM! 코드 일관성 개선주석에서 이모지를 제거하여 코드 일관성을 개선한 좋은 변경입니다. 주석의 의미는 그대로 유지되며 가독성도 향상되었습니다.
src/main/java/team/wego/wegobackend/group/v2/application/event/GroupDeletedEvent.java (1)
5-11: LGTM! 이벤트 구조가 적절합니다.레코드 기반의 불변 이벤트 구조가 명확하고, 모임 삭제 시 필요한 정보(그룹 ID, 호스트 ID, 참석자 ID 목록)를 모두 포함하고 있습니다.
src/test/http/group/v2/v2-group-create.http (2)
29-61: LGTM! 팔로워 알림 플로우가 잘 구성되어 있습니다.팔로워 회원가입, 로그인, 알림 허용, 팔로우 액션 및 모임 생성 전 알림 카운트 조회까지 완전한 테스트 시나리오를 제공합니다. 이는 모임 생성 이벤트 시 팔로워 알림 기능을 검증하는 데 적합합니다.
124-130: 팔로워 알림 검증 단계가 적절합니다.모임 생성 후 팔로워의 읽지 않은 알림 카운트와 알림 목록을 조회하여 알림이 정상적으로 전달되었는지 DB 레벨과 API 레벨 모두에서 검증합니다.
src/test/http/group/v2/v2-group-left.http (2)
79-100: SSE 구독 방식을 두 가지로 테스트하는 것이 좋습니다.헤더 기반 인증(Bearer 토큰)과 쿼리 파라미터 기반 인증 두 가지 방식을 모두 테스트하여, 클라이언트 환경에 따른 다양한 SSE 연결 시나리오를 검증할 수 있습니다.
165-184: 모임 탈퇴 후 호스트 알림 검증이 적절합니다.팔로워가 모임에 참여한 후 탈퇴할 때 호스트에게 알림이 전달되는지 확인하는 플로우가 완성도 있게 구성되어 있습니다. DB 레벨(unread-count)과 API 레벨(알림 목록) 모두에서 검증하고 있습니다.
src/main/java/team/wego/wegobackend/group/v2/application/service/GroupV2DeleteService.java (1)
98-114: 트랜잭션 동기화 로직이 적절합니다.커밋 후 이벤트 발행 패턴이 올바르게 구현되어 있으며, 트랜잭션이 없는 엣지 케이스도 방어하고 있습니다. S3 삭제 로직과 동일한 패턴을 사용하여 일관성이 있습니다.
src/main/java/team/wego/wegobackend/notification/application/dto/NotificationType.java (2)
22-31: 도메인 필드 추가로 알림 분류가 개선되었습니다.
domain필드를 추가하여 알림 타입을 논리적 도메인(test, user, group)별로 그룹화할 수 있게 되었습니다. 이는 향후 도메인별 필터링이나 라우팅에 유용합니다.
8-31: Lombok 기반 getter에서 레코드 스타일 접근자로의 API 변경 확인 필요
NotificationType열거형이 Lombok 기반@Getter에서 레코드 스타일의domain(),description()접근자로 변경되었습니다. 기존 코드에서getDescription()또는getDomain()메서드를 사용하는 경우 컴파일 에러가 발생할 수 있습니다. 전체 프로젝트를 확인하여 이러한 이전 메서드 호출이 모두 새로운 접근자로 업데이트되었는지 검증하세요.src/main/java/team/wego/wegobackend/group/v2/application/event/GroupJoinedEvent.java (1)
3-9: LGTM! 모임 참여 이벤트 구조가 적절합니다.레코드 기반의 불변 이벤트로, 모임 참여 시 필요한 정보(그룹 ID, 호스트 ID, 참여자 ID)를 명확하게 전달합니다. 다른 이벤트 레코드들과 일관된 패턴을 따르고 있습니다.
src/main/java/team/wego/wegobackend/group/v2/application/event/GroupJoinRequestedEvent.java (1)
3-9: LGTM! 참여 요청 이벤트 구조가 명확합니다.
hostUserId와requesterUserId로 역할을 명확히 구분하여, 누가 요청하고 누가 승인 권한을 가지는지 코드에서 명확하게 드러납니다.src/main/java/team/wego/wegobackend/group/v2/application/event/GroupJoinApprovedEvent.java (1)
3-9: LGTM! 승인 이벤트 구조가 적절합니다.승인자(
approverUserId)와 대상자(targetUserId)를 명확히 구분하여, 승인 프로세스의 의미를 코드로 명확하게 표현하고 있습니다. 일관된 이벤트 레코드 패턴을 따릅니다.src/main/java/team/wego/wegobackend/group/v2/application/service/GroupV2Service.java (1)
250-254: 이벤트 발행 로직 확인 완료그룹 생성 후 이벤트 발행이 올바르게 구현되었습니다.
groupV2Repository.save(group)후에 이벤트를 발행하여 ID가 할당된 상태에서 이벤트를 전달하고 있으며, 로깅도 적절합니다.
@Transactional메서드 내에서 이벤트를 발행하므로 리스너의AFTER_COMMIT단계가 트랜잭션 커밋 후에 실행됩니다. 이는 그룹 생성이 완전히 완료된 후 알림이 전송되도록 보장합니다.src/main/java/team/wego/wegobackend/user/repository/query/FollowerNotifyRow.java (1)
3-8: 쿼리 프로젝션 레코드 구현 확인
FollowerNotifyRow는 팔로워 알림 쿼리 결과를 전달하는 표준적인 DTO 레코드입니다. 생성자 기반 프로젝션으로 사용되며, 필드의 nullable 여부는 데이터베이스 스키마와 쿼리 로직에 따라 결정됩니다.src/main/java/team/wego/wegobackend/group/v2/application/event/GroupJoinRejectedEvent.java (1)
3-7: 이벤트 레코드 구현 확인
GroupJoinRejectedEvent는 모임 참여 거절 이벤트를 나타내는 불변 데이터 캐리어입니다. 다른 이벤트 레코드들과 일관된 패턴을 따르고 있습니다.필드 검증은 선택적이며, 필요 시
GroupCreatedEvent에 제안한 것과 동일한 패턴으로 compact constructor를 추가할 수 있습니다.src/main/java/team/wego/wegobackend/group/v2/application/event/GroupJoinKickedEvent.java (1)
3-7: 이벤트 레코드 구현 확인
GroupJoinKickedEvent는 모임 강제 퇴장 이벤트를 나타내는 불변 데이터 캐리어로, 다른 이벤트 레코드들과 일관된 설계를 따릅니다.src/main/java/team/wego/wegobackend/group/v2/application/event/GroupLeftEvent.java (1)
3-7: 이벤트 레코드 구현 확인
GroupLeftEvent는 모임 퇴장 이벤트를 나타내는 불변 데이터 캐리어로, 프로젝트의 다른 이벤트 레코드들과 일관된 설계 패턴을 따릅니다.src/main/java/team/wego/wegobackend/user/repository/FollowRepositoryCustom.java (1)
16-16: 새로운 팔로워 알림 조회 메서드 추가 확인커서 기반 페이지네이션을 사용하는
findFollowersForNotify메서드 선언이 적절합니다. 대량의 팔로워에게 알림을 보낼 때 메모리 효율적인 배치 처리가 가능합니다.src/main/java/team/wego/wegobackend/group/v2/application/listener/GroupDeleteEventListener.java (1)
10-20: LGTM!다른 이벤트 리스너들(
GroupJoinEventListener,GroupKickEventListener등)과 일관된 패턴을 따르고 있습니다.AFTER_COMMIT단계에서 실행되어 트랜잭션 완료 후 알림이 처리됩니다.src/main/java/team/wego/wegobackend/group/v2/application/listener/GroupKickEventListener.java (1)
10-20: LGTM!
GroupJoinKickedEvent를 처리하는 리스너가 일관된 패턴으로 구현되었습니다.src/main/java/team/wego/wegobackend/group/v2/application/listener/GroupJoinEventListener.java (1)
10-21: LGTM!모임 참여 이벤트 리스너가 다른 리스너들과 동일한 패턴으로 잘 구현되었습니다.
src/main/java/team/wego/wegobackend/group/v2/application/listener/GroupJoinRequestEventListener.java (1)
11-21: LGTM!모임 참여 요청 이벤트 리스너가 다른 리스너들과 동일한 패턴으로 잘 구현되었습니다.
src/main/java/team/wego/wegobackend/group/v2/application/handler/GroupLeaveNotificationHandler.java (1)
28-40: LGTM! 전반적인 핸들러 구조
REQUIRES_NEW트랜잭션 전파를 사용하여 이벤트 리스너의 트랜잭션과 분리하고, 알림 저장 후 SSE로 실시간 전송하는 흐름이GroupJoinNotificationHandler와 일관되게 구현되었습니다.src/main/java/team/wego/wegobackend/group/v2/application/handler/GroupCreateNotificationHandler.java (1)
73-73: Java 21 설정으로List.getLast()호환성 문제 없습니다.프로젝트는 build.gradle에서 Java 21을 대상으로 설정하고 있어 SequencedCollection 인터페이스의
getLast()메서드를 사용할 수 있습니다.src/main/java/team/wego/wegobackend/group/v2/application/service/GroupV2AttendanceService.java (3)
129-131: 이벤트 발행 로직 확인 완료즉시 참여(FREE 정책) 완료 후
GroupJoinedEvent를 발행하는 로직이 적절합니다. 이벤트는 트랜잭션 커밋 후 리스너에 의해 처리됩니다.
168-169: 이벤트 발행 로직 확인 완료승인 필요(APPROVAL_REQUIRED) 정책에서 참가 신청 후
GroupJoinRequestedEvent를 발행하는 로직이 적절합니다.
207-214: 조건부 이벤트 발행 로직이 적절합니다
LEFT상태일 때만 탈퇴 알림 이벤트를 발행하는 조건 로직이 좋습니다.CANCELLED등 다른 상태에 대한 알림 요구사항이 명확하지 않은 경우 불필요한 알림을 방지합니다.src/main/java/team/wego/wegobackend/group/v2/application/listener/GroupLeaveEventListener.java (1)
18-22: 이벤트 리스너 구현이 적절합니다
@TransactionalEventListener(AFTER_COMMIT)사용으로 트랜잭션 커밋 후 핸들러가 실행되며, 로깅과 위임 패턴이 다른 리스너들과 일관되게 구현되어 있습니다.src/main/java/team/wego/wegobackend/user/repository/FollowRepositoryImpl.java (2)
49-53: 메서드 네이밍 컨벤션 개선메서드명을
ItCursor에서itCursor로 변경하여 Java 네이밍 컨벤션(camelCase)을 따르도록 수정한 것이 좋습니다.
77-78: 알림 필터링 로직이 적절합니다알림을 활성화한 사용자(
notificationEnabled.isTrue())이면서 삭제되지 않은 사용자(deleted.isFalse())만 필터링하는 로직이 알림 기능 요구사항에 적합합니다.src/main/java/team/wego/wegobackend/group/v2/application/handler/GroupDeleteNotificationHandler.java (1)
37-37: 효율적인 엔티티 참조 로딩
getReferenceById를 사용하여 수신자 엔티티를 지연 로딩하는 것이 대량 알림 생성 시 성능에 유리합니다.src/main/java/team/wego/wegobackend/group/v2/application/listener/GroupJoinDecisionEventListener.java (1)
17-25: 구현이 적절합니다.
@TransactionalEventListener(phase = TransactionPhase.AFTER_COMMIT)사용으로 트랜잭션 커밋 후에만 알림이 전송되도록 보장하며, 핸들러로의 위임 패턴이 깔끔합니다.다만, 핸들러에서 예외 발생 시 별도의 에러 처리가 없어 조용히 실패할 수 있습니다. 알림 전송 실패가 비즈니스 크리티컬하지 않다면 현재 구현도 괜찮지만, 로깅 추가를 고려해 볼 수 있습니다.
src/main/java/team/wego/wegobackend/group/v2/application/handler/GroupJoinNotificationHandler.java (1)
26-37: 전체적인 구현 패턴이 적절합니다.
Propagation.REQUIRES_NEW를 사용하여 이벤트 핸들러가 독립적인 트랜잭션에서 실행되도록 하는 패턴이 다른 핸들러들과 일관성 있게 적용되어 있습니다.src/main/java/team/wego/wegobackend/group/v2/application/handler/GroupJoinDecisionNotificationHandler.java (1)
27-35:handleApproved와handleRejected의 중복 트랜잭션 어노테이션두 메서드 모두
@Transactional(propagation = Propagation.REQUIRES_NEW)를 선언하고 있지만, 실제 로직은 privatehandle()메서드에 위임됩니다. Spring AOP 특성상 public 메서드에 어노테이션이 필요하므로 현재 구현이 올바르지만, 두 메서드의 역할이 단순 위임이라면 통합을 고려해볼 수 있습니다.현재 구현도 괜찮습니다.
src/main/java/team/wego/wegobackend/notification/domain/Notification.java (1)
175-185: 삭제된 모임 알림의redirectUrl처리가 적절합니다.
createGroupDeleteNotification에서redirectUrl을/groups로 설정하여 삭제된 모임 대신 목록 페이지로 리다이렉트하는 것은 좋은 UX 고려입니다.
src/main/java/team/wego/wegobackend/group/v2/application/event/NotificationEvent.java
Show resolved
Hide resolved
| List<FollowerNotifyRow> test = followRepository.findFollowersForNotify(hostUserId, null, 10); | ||
| log.info("[NOTI] follower rows size={}", test.size()); |
There was a problem hiding this comment.
디버그/테스트 코드 제거 필요
Lines 35-36은 10개의 팔로워를 조회하여 로깅하는 테스트용 코드로 보입니다. 실제 배치 처리 로직(Lines 47-74)과 중복되며 프로덕션 코드에는 불필요합니다.
🔎 수정 제안
@Transactional(propagation = Propagation.REQUIRES_NEW)
public void handle(Long groupId, Long hostUserId) {
log.info("[NOTI] start handle. groupId={}, hostId={}", groupId, hostUserId);
- List<FollowerNotifyRow> test = followRepository.findFollowersForNotify(hostUserId, null, 10);
- log.info("[NOTI] follower rows size={}", test.size());
-
User host = userRepository.findById(hostUserId)
.orElseThrow();📝 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.
| List<FollowerNotifyRow> test = followRepository.findFollowersForNotify(hostUserId, null, 10); | |
| log.info("[NOTI] follower rows size={}", test.size()); | |
| @Transactional(propagation = Propagation.REQUIRES_NEW) | |
| public void handle(Long groupId, Long hostUserId) { | |
| log.info("[NOTI] start handle. groupId={}, hostId={}", groupId, hostUserId); | |
| User host = userRepository.findById(hostUserId) | |
| .orElseThrow(); |
🤖 Prompt for AI Agents
In
src/main/java/team/wego/wegobackend/group/v2/application/handler/GroupCreateNotificationHandler.java
around lines 35-36, remove the debug/test code that calls
followRepository.findFollowersForNotify(hostUserId, null, 10) and the
corresponding log line; this duplicates the real batch processing later (lines
47-74) and is unnecessary in production. Delete those two lines so only the
intended batch notification logic remains, and run tests to ensure no
compilation or unused-import issues remain.
...team/wego/wegobackend/group/v2/application/handler/GroupJoinDecisionNotificationHandler.java
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
# Find the target file and verify its existence
fd -t f "GroupJoinNotificationHandler.java"Repository: WeGo-Together/WeGo_BackEnd
Length of output: 168
🏁 Script executed:
# Check for GroupNotFoundException class
fd -t f "GroupNotFoundException.java"Repository: WeGo-Together/WeGo_BackEnd
Length of output: 52
🏁 Script executed:
# Search for any exception classes related to Group
fd -t f -e java | xargs grep -l "GroupNotFoundException" | head -20Repository: WeGo-Together/WeGo_BackEnd
Length of output: 52
🏁 Script executed:
# Search for the pattern mentioned in the review
rg -n "groupV2Repository\.findById.*UserNotFoundException" --type=javaRepository: WeGo-Together/WeGo_BackEnd
Length of output: 889
🏁 Script executed:
# Also check for similar patterns with other exception types
rg -n "groupV2Repository\.findById.*orElseThrow" --type=java -A 1Repository: WeGo-Together/WeGo_BackEnd
Length of output: 1605
🏁 Script executed:
# Find exception classes in the group domain
fd -t f -e java | xargs grep -l "class.*Exception" | grep -E "group|exception" | head -20Repository: WeGo-Together/WeGo_BackEnd
Length of output: 1708
🏁 Script executed:
# Search for EntityNotFoundException
fd -t f "EntityNotFoundException.java"Repository: WeGo-Together/WeGo_BackEnd
Length of output: 52
🏁 Script executed:
# Check what exceptions are used in other parts of the group domain
rg -n "throw.*Exception" src/main/java/team/wego/wegobackend/group/ --type=java | grep -i group | head -10Repository: WeGo-Together/WeGo_BackEnd
Length of output: 1638
🏁 Script executed:
# Look at the exception hierarchy and packages
find . -path "*/exception*" -name "*.java" -type f | grep -E "(group|exception)" | head -20Repository: WeGo-Together/WeGo_BackEnd
Length of output: 1739
🏁 Script executed:
# Check imports in the GroupJoinNotificationHandler file
cat src/main/java/team/wego/wegobackend/group/v2/application/handler/GroupJoinNotificationHandler.java | head -40Repository: WeGo-Together/WeGo_BackEnd
Length of output: 1949
🏁 Script executed:
# Read GroupException class
cat src/main/java/team/wego/wegobackend/group/domain/exception/GroupException.javaRepository: WeGo-Together/WeGo_BackEnd
Length of output: 482
🏁 Script executed:
# Read GroupErrorCode enum
cat src/main/java/team/wego/wegobackend/group/domain/exception/GroupErrorCode.javaRepository: WeGo-Together/WeGo_BackEnd
Length of output: 10702
🏁 Script executed:
# Check how GroupException is used elsewhere for not found scenarios
rg "GroupException.*NOT_FOUND\|GROUP_NOT_FOUND" --type=java -A 2 -B 2Repository: WeGo-Together/WeGo_BackEnd
Length of output: 52
🏁 Script executed:
# Search for how other repositories handle findById().orElseThrow for GroupV2
rg -n "findById.*orElseThrow" src/main/java/team/wego/wegobackend/group/ --type=javaRepository: WeGo-Together/WeGo_BackEnd
Length of output: 2666
잘못된 예외 타입 사용: GroupV2 조회 실패 시 UserNotFoundException 사용
GroupV2 엔티티를 찾지 못했을 때 UserNotFoundException을 던지고 있습니다. 이는 의미적으로 잘못되었으며 디버깅 시 혼란을 야기할 수 있습니다. 동일한 문제가 GroupJoinRequestNotificationHandler, GroupKickNotificationHandler, GroupLeaveNotificationHandler에도 존재합니다.
🔎 적절한 예외 타입 사용 제안
- GroupV2 group = groupV2Repository.findById(groupId).orElseThrow(UserNotFoundException::new);
+ GroupV2 group = groupV2Repository.findById(groupId).orElseThrow(() -> new GroupException(GroupErrorCode.GROUP_NOT_FOUND_BY_ID, groupId));GroupException과 기존 GroupErrorCode.GROUP_NOT_FOUND_BY_ID를 사용하세요.
📝 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.
| GroupV2 group = groupV2Repository.findById(groupId).orElseThrow(UserNotFoundException::new); | |
| GroupV2 group = groupV2Repository.findById(groupId).orElseThrow(() -> new GroupException(GroupErrorCode.GROUP_NOT_FOUND_BY_ID, groupId)); |
🤖 Prompt for AI Agents
In
src/main/java/team/wego/wegobackend/group/v2/application/handler/GroupJoinNotificationHandler.java
around line 30, the code throws UserNotFoundException when a GroupV2 is not
found; replace this with throwing a GroupException using
GroupErrorCode.GROUP_NOT_FOUND_BY_ID (e.g. use
groupV2Repository.findById(groupId).orElseThrow(() -> new
GroupException(GroupErrorCode.GROUP_NOT_FOUND_BY_ID))), and apply the same fix
to the other affected handlers: GroupJoinRequestNotificationHandler,
GroupKickNotificationHandler, and GroupLeaveNotificationHandler so that group
lookup failures consistently throw GroupException with the GROUP_NOT_FOUND_BY_ID
error code.
.../team/wego/wegobackend/group/v2/application/handler/GroupJoinRequestNotificationHandler.java
Outdated
Show resolved
Hide resolved
src/main/java/team/wego/wegobackend/group/v2/application/service/GroupV2DeleteService.java
Outdated
Show resolved
Hide resolved
src/main/java/team/wego/wegobackend/group/v2/domain/repository/GroupUserV2Repository.java
Show resolved
Hide resolved
There was a problem hiding this comment.
Null 안전성 부족
ActorUserDto.from(User u) 메서드가 null 파라미터 처리를 하지 않습니다. NotificationEvent.ActorUser.from() (관련 코드 스니펫 참조)과 달리 null 체크가 없어 NPE가 발생할 수 있습니다.
🔎 수정 제안
public static ActorUserDto from(User u) {
+ if (u == null) return null;
return ActorUserDto.builder()
.id(u.getId())
.nickName(u.getNickName())
.profileImage(u.getProfileImage())
.build();
}🤖 Prompt for AI Agents
In
src/main/java/team/wego/wegobackend/notification/application/dto/response/NotificationEventResponse.java
around lines 40 to 46, the static factory ActorUserDto.from(User u) does not
handle a null User and can throw an NPE; update the method to check if u is null
and return null (or an appropriate empty value) immediately, otherwise build the
ActorUserDto from u's fields — keep the existing builder logic for the non-null
path.
There was a problem hiding this comment.
Null 안전성 부족
GroupDto.from(GroupV2 groupV2) 메서드가 null 파라미터 처리를 하지 않습니다. NotificationEvent.GroupInfo.from() (관련 코드 스니펫 참조)과 달리 null 체크가 없어 NPE가 발생할 수 있습니다.
🔎 수정 제안
public static GroupDto from(GroupV2 groupV2) {
+ if (groupV2 == null) return null;
return GroupDto.builder()
.id(groupV2.getId())
.title(groupV2.getTitle())
.build();
}📝 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.
| public static GroupDto from(GroupV2 groupV2) { | |
| return GroupDto.builder() | |
| .id(groupV2.getId()) | |
| .title(groupV2.getTitle()) | |
| .build(); | |
| } | |
| public static GroupDto from(GroupV2 groupV2) { | |
| if (groupV2 == null) return null; | |
| return GroupDto.builder() | |
| .id(groupV2.getId()) | |
| .title(groupV2.getTitle()) | |
| .build(); | |
| } |
🤖 Prompt for AI Agents
In
src/main/java/team/wego/wegobackend/notification/application/dto/response/NotificationEventResponse.java
around lines 55-60, the static factory GroupDto.from(GroupV2 groupV2) lacks a
null check and can NPE; add a guard at the top that returns null when groupV2 is
null (matching the pattern used by NotificationEvent.GroupInfo.from()),
otherwise proceed to build and return the DTO with id and title.
src/main/java/team/wego/wegobackend/notification/domain/Notification.java
Show resolved
Hide resolved
There was a problem hiding this comment.
Pull request overview
This PR implements a comprehensive notification system for V2 group events. The implementation follows an event-driven architecture where group-related actions trigger events that are handled asynchronously to send notifications to relevant users via SSE (Server-Sent Events).
Key changes:
- Event-driven notification system with publishers, listeners, and handlers for various group actions
- New notification types for group creation, join/leave, approval/rejection, kick, and deletion
- Follower-based notification targeting with pagination support for group creation events
- SSE integration for real-time notification delivery
Reviewed changes
Copilot reviewed 40 out of 40 changed files in this pull request and generated 17 comments.
Show a summary per file
| File | Description |
|---|---|
NotificationType.java |
Added new enum values for group-related notification types |
Notification.java |
Added factory methods for creating group-related notifications |
NotificationEvent.java |
New DTO for SSE notification events |
GroupCreateNotificationHandler.java |
Handles notifications when a group is created, notifying followers |
GroupJoinNotificationHandler.java |
Handles notifications when a user joins a group |
GroupLeaveNotificationHandler.java |
Handles notifications when a user leaves a group |
GroupJoinRequestNotificationHandler.java |
Handles notifications for join requests |
GroupJoinDecisionNotificationHandler.java |
Handles approval/rejection notifications |
GroupKickNotificationHandler.java |
Handles kick notifications |
GroupDeleteNotificationHandler.java |
Handles notifications when a group is deleted |
Group*EventListener.java (7 files) |
Event listeners that delegate to respective handlers |
Group*Event.java (7 files) |
Event record classes for different group actions |
NotificationDispatcher.java |
Common dispatcher for batch notification sending |
FollowerNotifyRow.java |
Query projection for follower notification data |
FollowRepositoryImpl.java |
Added query method to find followers with notifications enabled |
FollowRepositoryCustom.java |
Interface for custom follower query |
GroupV2Service.java |
Publishes GroupCreatedEvent after group creation |
GroupV2AttendanceService.java |
Publishes events for join, leave, approve, reject, kick |
GroupV2DeleteService.java |
Publishes GroupDeletedEvent after group deletion |
GroupUserV2Repository.java |
Added query to find user IDs by group and status |
SseEmitterService.java |
Added overloaded method for NotificationEvent |
NotificationController.java |
Removed unused imports |
v2-group-left.http |
New HTTP test file for group leave notification flow |
v2-group-create.http |
Added follower setup and notification verification tests |
V2-group-left.http |
Deleted (case-sensitivity fix) |
group-attend.http |
Removed emoji from comment |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| .receiver(receiver) // host | ||
| .actor(actor) // joiner reject |
There was a problem hiding this comment.
Incorrect parameter documentation in comment. The comment states "receiver" is the "host", but in the context of GROUP_JOIN_REJECTED, the receiver should be the person who made the join request (the applicant), not the host. The actor is the host who rejected. This comment is misleading.
| .join(follow.follower, user) | ||
| .where( | ||
| follow.followee.id.eq(followeeId), | ||
| cursorFollowId == null ? null : follow.id.lt(cursorFollowId), |
There was a problem hiding this comment.
Inconsistent cursor condition handling. The findFollowersForNotify method uses an inline ternary operator for the cursor condition, while the findFollowingList method uses a separate private method itCursor. For consistency and maintainability, both methods should use the same approach, preferably extracting the cursor logic to the reusable itCursor method.
| List<FollowerNotifyRow> test = followRepository.findFollowersForNotify(hostUserId, null, 10); | ||
| log.info("[NOTI] follower rows size={}", test.size()); |
There was a problem hiding this comment.
Debug code should be removed before merging. These test log statements appear to be leftover from development and should be cleaned up.
There was a problem hiding this comment.
Incorrect exception type is thrown. When a GroupV2 entity is not found, a UserNotFoundException is thrown which is misleading. This should throw an appropriate GroupException or GroupNotFoundException instead.
There was a problem hiding this comment.
Incorrect exception type is thrown. When a GroupV2 entity is not found, a UserNotFoundException is thrown which is misleading. This should throw an appropriate GroupException or GroupNotFoundException instead.
| @Transactional(propagation = Propagation.REQUIRES_NEW) | |
| public void handle(Long groupId, Long hostUserId, Long joinerUserId) { | |
| User actor = userRepository.findById(joinerUserId).orElseThrow(UserNotFoundException::new); | |
| User receiver = userRepository.findById(hostUserId).orElseThrow(UserNotFoundException::new); | |
| GroupV2 group = groupV2Repository.findById(groupId).orElseThrow(UserNotFoundException::new); | |
| private static class GroupNotFoundException extends RuntimeException { | |
| } | |
| @Transactional(propagation = Propagation.REQUIRES_NEW) | |
| public void handle(Long groupId, Long hostUserId, Long joinerUserId) { | |
| User actor = userRepository.findById(joinerUserId).orElseThrow(UserNotFoundException::new); | |
| User receiver = userRepository.findById(hostUserId).orElseThrow(UserNotFoundException::new); | |
| GroupV2 group = groupV2Repository.findById(groupId).orElseThrow(GroupNotFoundException::new); |
There was a problem hiding this comment.
Potential N+1 query problem. For each attendee user ID, userRepository.getReferenceById is called individually. Consider using a batch query approach like userRepository.findAllById to fetch all users at once, which would be more efficient.
| for (Long receiverId : attendeeUserIds) { | |
| if (receiverId.equals(hostUserId)) continue; // 호스트 자신 제외(원하면 제거) | |
| User receiver = userRepository.getReferenceById(receiverId); | |
| List<Long> receiverIds = new ArrayList<>(attendeeUserIds.size()); | |
| for (Long receiverId : attendeeUserIds) { | |
| if (receiverId.equals(hostUserId)) continue; // 호스트 자신 제외(원하면 제거) | |
| receiverIds.add(receiverId); | |
| } | |
| if (receiverIds.isEmpty()) { | |
| return; | |
| } | |
| List<User> receivers = userRepository.findAllById(receiverIds); | |
| for (User receiver : receivers) { |
| import team.wego.wegobackend.user.application.dto.response.QFollowResponse; | ||
| import team.wego.wegobackend.user.domain.QFollow; | ||
| import team.wego.wegobackend.user.domain.QUser; | ||
| import team.wego.wegobackend.user.domain.User; |
There was a problem hiding this comment.
Unused import should be removed. The User class is imported but not used anywhere in this implementation file.
| import team.wego.wegobackend.user.domain.User; | ||
|
|
||
| @Getter(AccessLevel.PUBLIC) | ||
| @Builder(access = AccessLevel.PUBLIC) |
There was a problem hiding this comment.
Default toString(): GroupInfo inherits toString() from Object, and so is not suitable for printing.
Default toString(): ActorUser inherits toString() from Object, and so is not suitable for printing.
There was a problem hiding this comment.
Default toString(): GroupDto inherits toString() from Object, and so is not suitable for printing.
Default toString(): ActorUserDto inherits toString() from Object, and so is not suitable for printing.
|
|
||
| > {% | ||
| client.global.set("SSE_URL_HEADER", "http://localhost:8080/api/v1/notifications/subscribe"); | ||
| client.global.set("SSE_URL_QUERY", "http://localhost:8080/api/v1/notifications/subscribe?accessToken=" + client.global.get("followerAccessToken")); |
There was a problem hiding this comment.
The construction of SSE_URL_QUERY embeds the accessToken directly in the query string, which indicates the backend SSE endpoint likely accepts access tokens via URL parameters. Access tokens in URLs are frequently logged by servers, proxies, and client tooling, and can also leak via referrer headers and browser history, allowing an attacker with log or monitoring access to replay the token and impersonate the user. Avoid supporting accessToken as a query parameter on the server and use the Authorization: Bearer header (or another non-URL channel) exclusively for authentication to prevent token leakage.
📝 Pull Request
📌 PR 종류
해당하는 항목에 체크해주세요.
✨ 변경 내용
모임 알림을 설정합니다.
🔍 관련 이슈
🧪 테스트
변경된 기능에 대한 테스트 범위 또는 테스트 결과를 작성해주세요.
🚨 확인해야 할 사항 (Checklist)
PR을 제출하기 전에 아래 항목들을 확인해주세요.
🙋 기타 참고 사항
리뷰어가 참고하면 좋을 만한 추가 설명이 있다면 적어주세요.
Summary by CodeRabbit
릴리스 노트
새로운 기능
테스트
✏️ Tip: You can customize this high-level summary in your review settings.