Conversation
- notification DB 저장은 상위 트랜잭션 커밋시에 동기적으로 수행 - 상위 트랜잭션이 있을 경우에만 정상 동작하도록 Propagation.MANDATORY 제한 설정 - 푸시알림을 위한 이벤트 퍼블리시는 비동기적으로 수행 - 이벤트 리스너가 트랜잭션 커밋시에 이벤트를 받아서 fcm 서버로 푸시알림 보내는 구조는 유지
- notification DB 저장은 상위 트랜잭션 커밋시에 동기적으로 수행 - 상위 트랜잭션이 있을 경우에만 정상 동작하도록 Propagation.MANDATORY 제한 설정 - 푸시알림을 위한 이벤트 퍼블리시는 비동기적으로 수행 - 이벤트 리스너가 트랜잭션 커밋시에 이벤트를 받아서 fcm 서버로 푸시알림 보내는 구조는 유지
- 모든 이벤트에 title, content 추가 - 이벤트 리스너는 이벤트에 포함된 raw data 로 title, content 를 구성하는게 아니라, 이벤트에 포함된 title, content 정보를 그대로 사용하도록 하기 위해 이벤트 구조 수정
- 이벤트에 포함된 title, content 정보를 받아서 바로 사용하도록 수정 - title, content 의 생성 책임은 template enum 만 담당
- NotificationOrchestrator 를 호출하도록 수정 - NotificationOrchestrator 내부에서 알림센터 저장 + 이벤트 퍼블리시 수행
- NotificationOrchestrator 구현체를 의존하도록 수정
- 테스트 클래스 레벨에 트랜잭션이 걸려있으므로 teardown 메서드 삭제
- userJpaRepository 의 delete 전에 NotificationJpaRepository의 delete 를 선행하도록 코드 수정
- notification 패키지에서 알림센터의 title, content 생성을 담당하므로 notification/domain 하위로 이동
… 를 바라보도록 수정 (#296) - 일단 whitelist enum 객체를 도입 - 필요하다면 yml 에 명시하고, 환경변수로 주입받아도 될 듯
Walkthrough알림 경로를 이벤트 포트에서 오케스트레이터 기반(템플릿 → DB 저장 → 이벤트 발행)으로 전환했고, 알림 페이로드에 title/content를 추가 및 전파했다. 보안 화이트리스트를 enum으로 중앙화하고 Jwt 필터/시큐리티 설정을 연동했다. 관련 단위·통합 테스트가 추가·수정되었다. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
participant Svc as 비즈니스 서비스
participant Orc as NotificationOrchestrator (Sync)
participant Exec as NotificationSyncExecutor
participant Cmd as NotificationCommandPort
participant EPort as EventCommandPort (Feed/Room)
participant Listener as DispatchListener
participant FCM as Firebase
rect rgba(220,235,255,0.35)
note over Svc,Orc: 비즈니스 트랜잭션 내부 (동기 호출, MANDATORY)
Svc->>Orc: notifyX(targetId, actorId, actorName, ...context)
Orc->>Exec: execute(template, args, targetId, invoker)
Exec->>Cmd: save(Notification.withoutId(title, content, targetId))
Exec->>EPort: invoker.publish(title, content, ...ids)
end
rect rgba(235,255,230,0.35)
note over Listener,FCM: 트랜잭션 커밋 후 (비동기)
EPort-->>Listener: Event(title, content, ids...)
Listener->>FCM: send(batch 메시지(title, content, data...))
end
sequenceDiagram
autonumber
participant Client as HTTP 요청
participant JwtF as JwtAuthenticationFilter
participant WL as SecurityWhitelist
participant Sec as SecurityConfig
Client->>JwtF: 요청 경로 조회
JwtF->>WL: patternsList()
alt 경로가 화이트리스트 매칭
JwtF-->>Client: 인증 스킵(허용)
else
JwtF->>JwtF: JWT 추출/검증/블랙리스트 검사
JwtF-->>Client: 인증 정보 설정 후 전달
end
Estimated code review effort🎯 4 (Complex) | ⏱️ ~75 minutes Possibly related PRs
Poem
Pre-merge checks and finishing touches❌ Failed checks (2 warnings)
✅ Passed checks (3 passed)
✨ Finishing touches
🧪 Generate unit tests
Tip 👮 Agentic pre-merge checks are now available in preview!Pro plan users can now enable pre-merge checks in their settings to enforce checklists before merging PRs.
Please see the documentation for more information. Example: reviews:
pre_merge_checks:
custom_checks:
- name: "Undocumented Breaking Changes"
mode: "warning"
instructions: |
Pass/fail criteria: All breaking changes to public APIs, CLI flags, environment variables, configuration keys, database schemas, or HTTP/GraphQL endpoints must be documented in the "Breaking Change" section of the PR description and in CHANGELOG.md. Exclude purely internal or private changes (e.g., code not exported from package entry points or explicitly marked as internal).Please share your feedback with us on this Discord post. 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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 6
🧹 Nitpick comments (72)
src/test/java/konkuk/thip/roompost/adapter/in/web/RecordCreateApiTest.java (1)
44-46: 클래스명 변경에 맞게 DisplayName도 일관되게 정리하세요클래스는
RecordCreateApiTest로 바뀌었는데,@DisplayName은 여전히 "RecordCommandController 테스트"로 남아 혼동을 줄 수 있습니다. 아래처럼 문구만 정리하면 됩니다.-@DisplayName("[통합] RecordCommandController 테스트") +@DisplayName("[통합] RecordCreate API 테스트") class RecordCreateApiTest {src/main/java/konkuk/thip/config/SecurityConfig.java (1)
3-3: 화이트리스트 중앙화 적용은 적절합니다.
SecurityWhitelist.patterns()로 일원화한 점 👍. 다만 CORS preflight(OPTIONS) 요청이 인증 없이 항상 통과되어야 하는 경우가 많습니다. 필요시 아래처럼 명시 허용을 검토해 주세요..authorizeHttpRequests(auth -> auth + .requestMatchers(org.springframework.http.HttpMethod.OPTIONS, "/**").permitAll() .requestMatchers(SecurityWhitelist.patterns()).permitAll() .anyRequest().authenticated() )Also applies to: 68-69
src/main/java/konkuk/thip/common/security/constant/SecurityWhitelist.java (2)
28-38: per-request 할당을 줄이기 위한 캐싱을 권장합니다.
patternsList()가 매 요청마다 배열과 리스트를 생성합니다. JWT 필터에서 자주 호출되므로 정적 캐시로 바꿔 할당을 줄이세요.- public static String[] patterns() { - return Arrays.stream(values()) - .map(SecurityWhitelist::getPattern) - .toArray(String[]::new); - } + private static final String[] CACHED_PATTERNS = Arrays.stream(values()) + .map(SecurityWhitelist::getPattern) + .toArray(String[]::new); + + public static String[] patterns() { + return CACHED_PATTERNS; + } - public static List<String> patternsList() { - return Arrays.asList(patterns()); - } + private static final List<String> CACHED_PATTERNS_LIST = List.of(CACHED_PATTERNS); + + public static List<String> patternsList() { + return CACHED_PATTERNS_LIST; + }
19-20: 헬스 엔드포인트 범위 확인 요청.운영에서
"/actuator/health/liveness"등 서브 경로를 쓰면 현재"/actuator/health"만으로는 누락됩니다. 필요하면"/actuator/health/**"로 확장하세요.src/main/java/konkuk/thip/notification/adapter/out/persistence/NotificationQueryPersistenceAdapter.java (1)
12-15: 메서드 없는 빈(bean) — 사용처 없으면 제거 권장.
필드만 있고 공개 메서드가 전혀 없어 사실상 dead bean입니다. 실제 조회 기능을 다른 컴포넌트로 이전했다면 본 클래스를 삭제하거나 TODO를 명시해 혼란을 줄이세요. 유지가 필요하다면 최소한 공개 query 메서드를 추가해 역할을 드러내는 편이 좋습니다.src/main/java/konkuk/thip/message/adapter/out/firebase/FirebaseAdapter.java (3)
22-25: 마스킹 정책 판단에 별도server.profile사용은 설정 드리프트 리스크 — SpringEnvironment기반으로 일원화 제안실행 프로필과 별개 속성(
server.profile)이 어긋나면 운영에서 토큰이 노출될 수 있습니다.Environment의 활성 프로필로 판단하도록 바꾸면 안전합니다.다음 변경을 고려해 주세요.
- @Value("${server.profile}") - private String profile; + private final org.springframework.core.env.Environment env;- private boolean isDev() { - return profile != null && profile.trim().equalsIgnoreCase("dev"); - } + private boolean isDev() { + if (env == null) return false; + for (String p : env.getActiveProfiles()) { + if ("dev".equalsIgnoreCase(p.trim())) return true; + } + return false; + }추가 필요 import:
import org.springframework.core.env.Environment;Also applies to: 83-85
63-66: 부분 실패 발생 시 예외에 요약 정보를 포함하면 상위 레이어 관찰성↑현재 로그에만 실패 개수가 남고, 던지는 예외는 정보가 없습니다. 호출부 재시도/모니터링을 위해 실패 요약을 예외에 포함해 주세요.
- if (batchResponse.getFailureCount() > 0) { - log.warn("[FCM:BATCH] 일부 메시지 전송 실패: {}/{}", batchResponse.getFailureCount(), messages.size()); - throw new FirebaseException(); - } + if (batchResponse.getFailureCount() > 0) { + log.warn("[FCM:BATCH] 일부 메시지 전송 실패: {}/{}", batchResponse.getFailureCount(), messages.size()); + throw new FirebaseException(new IllegalStateException( + String.format("FCM 배치 전송 부분 실패: %d/%d", batchResponse.getFailureCount(), messages.size()) + )); + }
92-95: 마스킹 유틸: 빈 문자열 처리 일관성
null/빈 문자열 모두"null"로 반환되면 혼동될 수 있어요. 빈 문자열은"",null은"null"로 구분해 주는 편이 로그 가독성에 좋습니다.- if (s == null || s.isEmpty()) return "null"; + if (s == null) return "null"; + if (s.isEmpty()) return "";src/test/java/konkuk/thip/roompost/adapter/in/web/VoteCreateApiTest.java (1)
76-85: 테스트 정리 로직 공통화 제안동일한 deleteAllInBatch 체인이 여러 통합 테스트에 반복됩니다. 공통 BaseIntegrationCleanup(추상 클래스 또는 JUnit Extension)으로 정리 순서를 한 곳에서 관리하면 누락/순서 드리프트를 방지하고 유지보수를 줄일 수 있습니다.
src/main/java/konkuk/thip/notification/domain/value/NotificationCategory.java (1)
1-17: 패키지 이동 및 enum 설계 적절도메인 value 패키지로의 이동과 display 기반 prefix 메서드 구성이 명확합니다. 추가로 직렬화 시 표시값 노출이 필요하면 toString() 오버라이드나 @jsonvalue 적용을 검토해도 좋습니다.
src/test/java/konkuk/thip/roompost/application/service/VoteCreateServiceTest.java (1)
62-71: 정리 유틸/Extension 도입으로 중복 제거여기서도 정리 체인이 반복됩니다. 공통 테스트 유틸로 위임해 중복을 제거하면 유지보수가 쉬워집니다.
src/main/java/konkuk/thip/roompost/application/service/RecordCreateService.java (1)
6-6: 오케스트레이터 전환 LGTM + 대량 수신자 배치화 제안트랜잭션 내 오케스트레이터 호출로 after-commit 보장 흐름이 명확합니다. 다만 참여자 수가 많은 방에서 N회 호출로 트랜잭션 시간이 늘 수 있으므로, 수신자 리스트를 한 번에 넘기는 배치 API를 제공하면 DB 라운드트립과 객체 생성 비용을 줄일 수 있습니다. (이전 피드백 선호도에 따라 성능 근거를 중시하시는 점을 반영했습니다.)
예시(신규 오케스트레이터 메서드가 있다고 가정):
- for (RoomParticipant targetUser : targetUsers) { - if (targetUser.getUserId().equals(command.userId())) continue; // 본인 제외 - roomNotificationOrchestrator.notifyRoomRecordCreated(targetUser.getUserId(), actorUser.getId(), actorUser.getNickname(), room.getId(), room.getTitle(), record.getPage(), newRecordId); - } + var recipientIds = targetUsers.stream() + .map(RoomParticipant::getUserId) + .filter(id -> !id.equals(command.userId())) + .toList(); + roomNotificationOrchestrator.notifyRoomRecordCreated( + recipientIds, actorUser.getId(), actorUser.getNickname(), + room.getId(), room.getTitle(), record.getPage(), newRecordId + );Also applies to: 41-41, 85-85
src/main/java/konkuk/thip/feed/application/service/FeedCreateService.java (1)
15-15: 오케스트레이터 연동 적절 + 대량 알림 배치 전송 고려팔로워 수가 많은 계정에서 per-recipient 호출은 트랜잭션 커밋 시간을 키울 수 있습니다. 수신자 리스트 기반 배치 알림 메서드를 오케스트레이터에 추가하면 성능과 안정성이 좋아집니다.
예시(신규 메서드 가정):
- for (User targetUser : targetUsers) { - feedNotificationOrchestrator.notifyFolloweeNewPost(targetUser.getId(), actorUser.getId(), actorUser.getNickname(), savedFeedId); - } + var recipientIds = targetUsers.stream().map(User::getId).toList(); + feedNotificationOrchestrator.notifyFolloweeNewPost( + recipientIds, actorUser.getId(), actorUser.getNickname(), savedFeedId + );Also applies to: 36-36, 74-74
src/main/java/konkuk/thip/notification/domain/Notification.java (2)
21-28: 팩토리 메서드에 필수 파라미터 검증/트리밍 추가 제안title/content 공백/널 방어가 없으면 DB 제약 위반이나 빈 알림이 저장될 수 있습니다. 간단한 널 체크와 trim을 권장합니다.
다음 패치를 고려해 주세요:
+import java.util.Objects; @@ - public static Notification withoutId (String title, String content, Long targetUserId) { - return Notification.builder() - .title(title) - .content(content) - .isChecked(false) - .targetUserId(targetUserId) - .build(); - } + public static Notification withoutId(String title, String content, Long targetUserId) { + Objects.requireNonNull(targetUserId, "targetUserId must not be null"); + String safeTitle = Objects.requireNonNull(title, "title must not be null").trim(); + String safeContent = Objects.requireNonNull(content, "content must not be null").trim(); + if (safeTitle.isEmpty() || safeContent.isEmpty()) { + throw new IllegalArgumentException("title/content must not be blank"); + } + return Notification.builder() + .title(safeTitle) + .content(safeContent) + .isChecked(false) + .targetUserId(targetUserId) + .build(); + }
19-20: 조회 성능을 위한 인덱스/소트 키 권장사용자별 알림 조회(use-case)에 대비해 JPA 엔터티 레벨에서 targetUserId, isChecked, createdAt(또는 id desc)에 복합 인덱스를 두는 것을 권장합니다. 읽기 경로의 필터와 정렬을 커버해 스캔을 줄일 수 있습니다.
src/main/java/konkuk/thip/roompost/application/service/VoteCreateService.java (2)
55-57: Book 조회 널 안전성 확인 필요bookCommandPort.findById(...)가 null 가능이면 NPE가 납니다. 해당 메서드가 non-null 보장인지 확인하거나, 보장되지 않으면 명시적 예외로 전환하는 가드 추가를 권장합니다.
80-86: 다수 수신자 처리 최적화 고려참여자 수가 많을 때 per-user 호출 등록 비용이 커집니다. 오케스트레이터에 배치 API(예: notifyRoomVoteStartedBatch)를 마련하거나 내부에서 일괄 저장/발행하도록 확장 여지 검토를 제안합니다.
src/main/java/konkuk/thip/notification/application/port/out/NotificationCommandPort.java (1)
4-9: 포트 확장 LGTM + 계약 주석화 제안save(Notification) 추가 방향 좋습니다. 트랜잭션 요구사항(예: 커밋 직전 호출 전제)을 Javadoc로 명시하면 사용처 오해를 줄일 수 있습니다.
src/main/java/konkuk/thip/comment/application/service/CommentLikeService.java (1)
75-81: 분기 가독성 소폭 개선FEED/RECORD|VOTE가 상호배타면 else-if로 미세 가독성 향상 가능합니다.
- if (comment.getPostType() == PostType.FEED) { + if (comment.getPostType() == PostType.FEED) { feedNotificationOrchestrator.notifyFeedCommentLiked(...); - } - if (comment.getPostType() == PostType.RECORD || comment.getPostType() == PostType.VOTE) { + } else if (comment.getPostType() == PostType.RECORD || comment.getPostType() == PostType.VOTE) { PostQueryDto postQueryDto = postHandler.getPostQueryDto(...); roomNotificationOrchestrator.notifyRoomCommentLiked(...); }src/main/java/konkuk/thip/room/application/service/RoomRecruitCloseService.java (1)
51-53: 네이밍 정밀도actorUsers는 수신자 의미라 participants(또는 recipients)가 더 읽기 쉽습니다.
- List<RoomParticipant> actorUsers = roomParticipantCommandPort.findAllByRoomId(roomId); - for (RoomParticipant participant : actorUsers) { + List<RoomParticipant> participants = roomParticipantCommandPort.findAllByRoomId(roomId); + for (RoomParticipant participant : participants) {src/main/java/konkuk/thip/notification/application/service/template/feed/FeedLikedTemplate.java (1)
15-17: 배지/멘션 포맷 안전성actorUsername가 이미 '@'를 포함하거나 공백이 섞인 경우를 흡수하면 안전합니다.
- return "@" + args.actorUsername() + " 님이 내 글에 좋아요를 눌렀어요!"; + String username = args.actorUsername() == null ? "" : args.actorUsername().trim().replaceAll("^@+", ""); + return "@" + username + " 님이 내 글에 좋아요를 눌렀어요!";src/main/java/konkuk/thip/notification/adapter/out/persistence/NotificationCommandPersistenceAdapter.java (2)
23-30: 트랜잭션 경계 확인오케스트레이터가 “커밋 시점”에 저장을 수행한다면, save(...)가 활성 트랜잭션 내에서 실행되는지 확인이 필요합니다. afterCommit에서 호출된다면 REQUIRES_NEW 등 별도 트랜잭션이 필요할 수 있습니다. 현재 클래스/메서드에 @transactional이 없어 환경에 따라 저장 실패 가능성이 있습니다.
원하시면 오케스트레이터의 beforeCommit/afterCommit 동작에 맞춘 트랜잭션 전략 패치를 제안하겠습니다.
25-27: User 조회 비용 최적화 여지User를 단순 연관 설정 용도로만 쓰면 프록시 참조로 SELECT를 피할 수 있습니다(전제: userId가 PK이거나 getReferenceById 사용 가능). 레포지토리/매핑 제약을 확인해 적용 검토 바랍니다.
src/test/java/konkuk/thip/room/application/service/RoomJoinServiceTest.java (1)
5-5: 테스트 의존성: 구현체 대신 인터페이스 모킹 권장구현 교체에 덜 민감하도록 RoomNotificationOrchestrator를 모킹하는 편이 안정적입니다.
- import konkuk.thip.notification.application.service.RoomNotificationOrchestratorSyncImpl; + import konkuk.thip.notification.application.port.in.RoomNotificationOrchestrator; ... - private RoomNotificationOrchestratorSyncImpl roomNotificationOrchestratorSyncImpl; + private RoomNotificationOrchestrator roomNotificationOrchestrator; ... - roomNotificationOrchestratorSyncImpl = mock(RoomNotificationOrchestratorSyncImpl.class); + roomNotificationOrchestrator = mock(RoomNotificationOrchestrator.class); ... - roomNotificationOrchestratorSyncImpl + roomNotificationOrchestratorAlso applies to: 35-36, 49-49
src/main/java/konkuk/thip/message/application/service/FeedNotificationDispatchService.java (3)
35-47: 가독성: 컬렉션 변수명 명확화 제안
tk,dev는 의미가 불명확합니다.tokens,deviceIds등으로 이름을 풀면 유지보수성이 좋아집니다.- List<Message> msgs = new ArrayList<>(tokens.size()); - List<String> tk = new ArrayList<>(tokens.size()); - List<String> dev = new ArrayList<>(tokens.size()); + List<Message> messages = new ArrayList<>(tokens.size()); + List<String> tokenStrings = new ArrayList<>(tokens.size()); + List<String> deviceIds = new ArrayList<>(tokens.size()); ... - msgs.add(m); tk.add(t.getFcmToken()); dev.add(t.getDeviceId()); + messages.add(m); + tokenStrings.add(t.getFcmToken()); + deviceIds.add(t.getDeviceId()); ... - firebasePort.sendBatch(msgs, tk, dev); + firebasePort.sendBatch(messages, tokenStrings, deviceIds);
46-47: 전송 결과 관찰 가능성 확보(로깅/메트릭)
sendBatch실패/부분성공 케이스에 대한 로깅/메트릭을 추가해 운영 가시성을 확보하세요. 배치 크기, 성공/실패 토큰 수, 예외 메시지 등을 기록하는 것이 좋습니다.
105-107: 타이틀/본문 null·공백 대비 기본값 처리이벤트가 잘못 구성되면 FCM에 빈 타이틀/본문이 전달될 수 있습니다. 기본값을 설정해 방어적으로 처리하세요.
- return Notification.builder().setTitle(title).setBody(body).build(); + final String safeTitle = (title == null || title.isBlank()) ? DEFAULT_TITLE : title; + final String safeBody = (body == null || body.isBlank()) ? DEFAULT_BODY : body; + return Notification.builder().setTitle(safeTitle).setBody(safeBody).build();추가 코드(파일 상단 클래스 본문에 배치):
private static final String DEFAULT_TITLE = "알림"; private static final String DEFAULT_BODY = "새 소식이 도착했어요.";src/main/java/konkuk/thip/notification/application/service/template/NotificationTemplate.java (1)
3-8: 계약 명시화(Javadoc)로 일관성 보장타이틀/본문 길이, 개행/이모지 허용 여부 등 UI 제약을 인터페이스 수준에서 문서화해 구현체 간 일관성을 확보하세요.
src/main/java/konkuk/thip/notification/application/service/template/feed/FollowedTemplate.java (2)
5-9: 불필요한 Lombok @Getter 제거필드가 없는 enum에 @Getter는 불필요합니다. 임포트와 애너테이션 제거를 권장합니다.
-import lombok.Getter; ... -@Getter public enum FollowedTemplate implements NotificationTemplate<FollowedTemplate.Args> {
16-19: 문구 스타일 제안(선택)브랜드 표현(“띱”) 유지하되, 사용자명 뒤 공백·구두점 일관성을 다른 템플릿과 맞추면 좋습니다. 현재 형태도 기능상 문제는 없습니다.
src/test/java/konkuk/thip/user/application/service/UserFollowServiceTest.java (6)
32-40: 구체 클래스 모킹 대신 인터페이스/포트 모킹 권장테스트 의존성을 구현체(FeedNotificationOrchestratorSyncImpl)로 고정하면 결합도가 올라갑니다. 가능한 경우 오케스트레이터 포트/인터페이스를 모킹 대상으로 유지하세요.
69-76: 테스트 데이터 명확화: 액터/타깃 사용자 분리동일 User 인스턴스를 두 역할에 재사용하면 의도 파악이 어렵습니다. ID를 반영한 별도 인스턴스로 명확히 구분하세요.
- User user = createUserWithFollowerCount(0); - when(userCommandPort.findById(targetUserId)).thenReturn(user); - when(userCommandPort.findById(userId)).thenReturn(user); // 알림 전송용 + User target = createUserWithFollowerCount(targetUserId, 0); + User actor = createUserWithFollowerCount(userId, 0); + when(userCommandPort.findById(targetUserId)).thenReturn(target); + when(userCommandPort.findById(userId)).thenReturn(actor); // 알림 전송용보조 메서드 변경은 하단 코멘트를 참고하세요.
55-57: 기존 케이스도 동일 원칙 적용기존 테스트에서도 타깃 사용자 ID를 반영한 인스턴스로 교체하세요.
- User user = createUserWithFollowerCount(0); - when(userCommandPort.findById(targetUserId)).thenReturn(user); + User target = createUserWithFollowerCount(targetUserId, 0); + when(userCommandPort.findById(targetUserId)).thenReturn(target);
105-111: 언팔로우 케이스도 타깃 사용자 ID 반영일관성 차원에서 동일하게 적용하세요.
- User user = createUserWithFollowerCount(1); - when(userCommandPort.findById(targetUserId)).thenReturn(user); + User target = createUserWithFollowerCount(targetUserId, 1); + when(userCommandPort.findById(targetUserId)).thenReturn(target);
150-160: 헬퍼 시그니처 개선: ID 주입 가능하게 변경테스트 가독성을 위해 ID를 인자로 받도록 변경하세요.
- private User createUserWithFollowerCount(int count) { + private User createUserWithFollowerCount(Long id, int count) { return User.builder() - .id(100L) + .id(id) .nickname("tester") .userRole("USER") .oauth2Id("oauth-id") .followerCount(count) .alias(null) .build(); }
46-63: 오케스트레이터 상호작용 검증 추가 권장행위 기반 검증으로 알림 오케스트레이션 경로가 호출되는지 함께 보장하면 테스트 가치가 높아집니다(구체 메서드명은 실제 API에 맞게 적용).
src/main/java/konkuk/thip/notification/application/service/template/feed/FolloweeNewPostTemplate.java (2)
10-17: NPE 방지용 입력값 방어 로직 추가 제안템플릿은 외부 입력(사용자명)에 의존합니다.
args/actorUsername에 대한 널 방어를 추가해 조기 실패로 안전성을 높이는 것을 권장합니다.다음처럼 간단히 보강할 수 있습니다:
package konkuk.thip.notification.application.service.template.feed; import konkuk.thip.notification.application.service.template.NotificationTemplate; import konkuk.thip.notification.domain.value.NotificationCategory; +import java.util.Objects; ... @Override public String title(Args args) { + Objects.requireNonNull(args, "args"); return NotificationCategory.FEED.prefixedTitle("새 글 알림"); } @Override public String content(Args args) { - return "@" + args.actorUsername() + " 님이 새로운 글을 작성했어요!"; + Objects.requireNonNull(args, "args"); + String username = Objects.requireNonNull(args.actorUsername(), "actorUsername"); + return "@" + username + " 님이 새로운 글을 작성했어요!"; }Also applies to: 3-3
11-16: 문장부호/스타일 일관성 확인제목에는 느낌표가 없고 본문에는 느낌표가 있습니다. 다른 템플릿들과 톤앤매너(느낌표 사용, 경어 띄어쓰기 등)를 맞출지 확인 부탁드립니다.
src/main/java/konkuk/thip/notification/application/service/template/room/RoomPostCommentedTemplate.java (2)
16-16: 용어 표기 일관성: ‘독서기록’ → ‘독서 기록’다른 파일(RoomRecordCreatedTemplate)에서는 ‘독서 기록’으로 띄어 씁니다. 동일하게 맞추는 것을 권장합니다.
- return "@" + args.actorUsername() + " 님이 내 독서기록에 댓글을 달았어요!"; + return "@" + args.actorUsername() + " 님이 내 독서 기록에 댓글을 달았어요!";
10-17: 입력값 검증(널/공백) 보강 제안안전성을 위해
args/actorUsername에 대한 널 체크를 권장합니다.import konkuk.thip.notification.application.service.template.NotificationTemplate; import konkuk.thip.notification.domain.value.NotificationCategory; +import java.util.Objects; ... @Override public String title(Args args) { + Objects.requireNonNull(args, "args"); return NotificationCategory.ROOM.prefixedTitle("새로운 댓글이 달렸어요"); } ... public String content(Args args) { - return "@" + args.actorUsername() + " 님이 내 독서기록에 댓글을 달았어요!"; + Objects.requireNonNull(args, "args"); + String username = Objects.requireNonNull(args.actorUsername(), "actorUsername"); + return "@" + username + " 님이 내 독서 기록에 댓글을 달았어요!"; }Also applies to: 3-3
src/test/java/konkuk/thip/notification/application/service/FeedNotificationOrchestratorSyncImplTest.java (2)
59-76: 설계(커밋 시 저장)와 테스트 시나리오 간 불일치 가능성PR 설명은 “커밋 시점에 DB 저장/이벤트 발행”인데, 본 테스트는 트랜잭션 안에서 즉시 조회합니다. 실제 구현이 “즉시 저장 + afterCommit 이벤트 발행”이라면 OK지만, “저장 자체도 afterCommit”이라면 이 테스트는 지속적으로 통과하지 않습니다. 구현 의도와 테스트를 일치시켜 주세요.
커밋 기반을 검증하려면 TransactionTemplate를 사용해 커밋 전/후를 구분하는 테스트를 추가하는 방식을 권장합니다(발췌):
@@ class FeedNotificationOrchestratorSyncImplTest { @@ - @Test - @Transactional - @DisplayName("상위 트랜잭션 안에서 호출하면 정상 동작하고, Notification이 저장된다") - void mandatory_with_transaction_succeeds_and_persists() { - // when - orchestrator.notifyFeedCommented( - targetUserId, /*actor*/ 1000L, "bob", /*feedId*/ 777L - ); - - // then (같은 트랜잭션 안에서 즉시 조회 가능) - var all = notificationJpaRepository.findAll(); - assertThat(all).hasSize(1); - ... - } + @Autowired org.springframework.transaction.PlatformTransactionManager txManager; + + @Test + @DisplayName("커밋 후 Notification이 저장되고 내용이 기대대로 생성된다") + void persists_on_commit_and_contains_actor() { + var tt = new org.springframework.transaction.support.TransactionTemplate(txManager); + + // 커밋 이전(트랜잭션 내부) 관측 + tt.executeWithoutResult(status -> { + orchestrator.notifyFeedCommented(targetUserId, 1000L, "bob", 777L); + assertThat(notificationJpaRepository.findAll()).as("커밋 전에는 비어있어야 함(설계가 afterCommit 저장인 경우)").isEmpty(); + }); + + // 커밋 이후 관측 + var all = notificationJpaRepository.findAll(); + assertThat(all).hasSize(1); + assertThat(all.get(0).getContent()).contains("@bob"); + }
74-76: 검증 강화 제안: 멘션 형식 확인내용이 단순히 사용자명 포함 여부만 확인합니다. 멘션 접두어까지 보장하면 회귀에 강합니다.
- assertThat(saved.getContent()).contains("bob"); + assertThat(saved.getContent()).contains("@bob");src/main/java/konkuk/thip/notification/application/service/template/feed/FeedCommentedTemplate.java (1)
10-17: 입력값 방어 로직 추가 권장다른 템플릿들과 동일하게 NPE 방지를 위한 간단한 널 체크를 권장합니다.
import konkuk.thip.notification.application.service.template.NotificationTemplate; import konkuk.thip.notification.domain.value.NotificationCategory; +import java.util.Objects; @@ public String title(Args args) { + Objects.requireNonNull(args, "args"); return NotificationCategory.FEED.prefixedTitle("새로운 댓글이 달렸어요"); } @@ public String content(Args args) { - return "@" + args.actorUsername() + " 님이 내 글에 댓글을 달았어요!"; + Objects.requireNonNull(args, "args"); + String username = Objects.requireNonNull(args.actorUsername(), "actorUsername"); + return "@" + username + " 님이 내 글에 댓글을 달았어요!"; }Also applies to: 3-3
src/main/java/konkuk/thip/notification/application/service/template/room/RoomCommentLikedTemplate.java (2)
11-11: 제목 문구 어색함/일관성 개선“내 댓글을 좋아합니다”는 어색한 경어체입니다. 피드의 유사 템플릿(FeedCommentLikedTemplate)과 톤을 맞추면 좋겠습니다.
- return NotificationCategory.ROOM.prefixedTitle("내 댓글을 좋아합니다"); + return NotificationCategory.ROOM.prefixedTitle("좋아요 알림");또는 “내 댓글에 좋아요가 눌렸어요”도 자연스럽습니다.
10-17: NPE 방지용 입력값 검증 추가 권장다른 템플릿과 동일한 형태의 널 체크를 제안드립니다.
import konkuk.thip.notification.application.service.template.NotificationTemplate; import konkuk.thip.notification.domain.value.NotificationCategory; +import java.util.Objects; @@ public String title(Args args) { + Objects.requireNonNull(args, "args"); return NotificationCategory.ROOM.prefixedTitle("좋아요 알림"); } @@ public String content(Args args) { - return "@" + args.actorUsername() + " 님이 내 댓글에 좋아요를 눌렀어요!"; + Objects.requireNonNull(args, "args"); + String username = Objects.requireNonNull(args.actorUsername(), "actorUsername"); + return "@" + username + " 님이 내 댓글에 좋아요를 눌렀어요!"; }Also applies to: 3-3
src/main/java/konkuk/thip/notification/application/service/template/room/RoomRecordCreatedTemplate.java (2)
10-16: FCM/알림 타이틀 길이 가드(과도한 길이 절단) 권장방 제목이 긴 경우 FCM 타이틀 제한(단말 렌더링 가독성 저하) 이슈가 발생할 수 있습니다. 간단한 절단 로직을 추가하는 것을 권장합니다.
@Override public String title(Args args) { - return NotificationCategory.ROOM.prefixedTitle(args.roomTitle); + String roomTitle = args.roomTitle(); + if (roomTitle == null) roomTitle = ""; + // 필요 시 최대 길이는 정책값으로 추출 + int MAX = 30; + String trimmed = roomTitle.length() <= MAX ? roomTitle : roomTitle.substring(0, MAX - 1) + "…"; + return NotificationCategory.ROOM.prefixedTitle(trimmed); }
14-16: 입력값 널 체크 보강NPE 방지를 위해 간단한 검증을 권장합니다.
import konkuk.thip.notification.application.service.template.NotificationTemplate; import konkuk.thip.notification.domain.value.NotificationCategory; +import java.util.Objects; @@ public String content(Args args) { - return "@" + args.actorUsername() + " 님이 새로운 독서 기록을 작성했어요!"; + Objects.requireNonNull(args, "args"); + String username = Objects.requireNonNull(args.actorUsername(), "actorUsername"); + return "@" + username + " 님이 새로운 독서 기록을 작성했어요!"; }Also applies to: 3-3
src/main/java/konkuk/thip/room/application/service/RoomStateChangeService.java (3)
44-50: 중복 알림 위험: 업데이트 이전 알림 전송 순서현재는 IN_PROGRESS 전환 업데이트 전에 알림을 생성합니다. 스케줄러/멀티스레드 환경에서 두 실행이 겹치면 동일 방에 대해 중복 알림이 생성될 수 있습니다(첫 실행이 커밋하기 전, 두 번째 실행도 동일 대상을 조회).
안전한 패턴 예:
- “상태 전환”을 먼저 수행(영향받은 Room ID 목록을 반환하거나, 행 잠금/분리된 마커 컬럼 업데이트).
- 같은 트랜잭션에서 그 ID 목록에 대해서만 알림 생성.
- 커밋 후 이벤트 발행.
필요 시 포트에 “전환된 Room ID 반환” 메서드를 추가하거나, 전환 시 마커 컬럼을 사용해 재실행에도 중복을 방지하세요.Also applies to: 52-58
52-57: N+1 쿼리 패턴 개선 제안
findProgressTargetRooms()이후 각 Room마다findAllByRoomId호출로 N+1이 됩니다. 방/참여자 수가 큰 경우 비용이 큽니다.다음과 같은 포트 추가를 고려해 주세요:
findAllParticipantsByRoomIds(Set<Long> roomIds)한 번에 로드 후roomId기준 그룹화- 혹은 방 조회 시 참여자를
join fetch로 함께 가져오기(페이징 주의)
41-46: 비동기 실행 시 예외 처리 확인
@Async메서드에서 발생한 예외가 호출자에 전파되지 않습니다. 글로벌AsyncUncaughtExceptionHandler설정 또는 개별 try-catch+로깅을 확인해주세요(재시도/알림 실패 모니터링 포함).src/main/java/konkuk/thip/notification/application/service/template/feed/FeedCommentLikedTemplate.java (1)
10-17: 널 방어 추가 제안다른 템플릿과 동일하게 간단한 널 체크를 권장드립니다.
import konkuk.thip.notification.application.service.template.NotificationTemplate; import konkuk.thip.notification.domain.value.NotificationCategory; +import java.util.Objects; @@ public String title(Args args) { + Objects.requireNonNull(args, "args"); return NotificationCategory.FEED.prefixedTitle("좋아요 알림"); } @@ public String content(Args args) { - return "@" + args.actorUsername() + " 님이 내 댓글에 좋아요를 눌렀어요!"; + Objects.requireNonNull(args, "args"); + String username = Objects.requireNonNull(args.actorUsername(), "actorUsername"); + return "@" + username + " 님이 내 댓글에 좋아요를 눌렀어요!"; }Also applies to: 3-3
src/test/java/konkuk/thip/notification/application/service/FeedNotificationOrchestratorSyncImplUnitTest.java (1)
38-46: 중복 검증 로직 정리 제안ArgumentCaptor 생성, 공통 필드 검증(assert targetUserId/title/content)이 각 테스트에 반복됩니다. 공통 헬퍼 메서드로 추출해 테스트 가독성과 유지보수성을 높여주세요.
src/main/java/konkuk/thip/notification/application/service/template/room/RoomJoinToHostTemplate.java (1)
15-17: 문구 일관성 검토(선택)콘텐츠 내 띄어쓰기/호칭 규칙(“@{username} 님”)은 다른 템플릿과 일관되게 유지되는지 한 번 더 확인해주세요.
src/test/java/konkuk/thip/notification/application/service/RoomNotificationOrchestratorSyncImplUnitTest.java (2)
38-45: 중복 어서션 헬퍼화저장된 Notification 공통 검증(assert targetUserId/title/content 포함 등)을 헬퍼로 추출하면 테스트 길이를 크게 줄일 수 있습니다.
Also applies to: 88-95, 113-120, 137-145, 163-170, 187-191, 212-216, 239-242
91-94: 타이틀에 방 제목 포함 검증 추가 제안일부 시나리오에서는 title에 roomTitle 포함 여부를 검증하지 않습니다. 템플릿 정책상 prefix+roomTitle이 보장되어야 한다면 해당 어서션을 추가해 회귀를 방지하세요.
src/main/java/konkuk/thip/room/application/service/RoomJoinService.java (1)
62-66: 메서드 의도 명확화 네이밍(선택)afterCommit 큐잉 성격을 드러내기 위해 sendNotifications → enqueueNotifications로의 네이밍 변경을 고려해볼 만합니다.
src/main/java/konkuk/thip/comment/application/service/CommentCreateService.java (2)
96-103: 중복 알림 가능성(동일 수신자에 댓글+대댓글 동시 수신)대댓글 시 부모 댓글 작성자와 게시글 작성자가 동일하면 동일 사용자가 2건의 알림을 받을 수 있습니다. 제품 정책상 하나로 합치거나 우선순위를 두는 것이 바람직하면, 수신자·맥락 기반으로 중복 억제 로직을 고려해주세요.
Also applies to: 108-115
96-103: 분기문 단순화(선택)postType 분기를 switch 표현식으로 바꾸면 가독성이 좋아집니다.
- if (postQueryDto.postType().equals(FEED.getType())) { + switch (postQueryDto.postType()) { + case "FEED" -> { // ... - } else if (postQueryDto.postType().equals(RECORD.getType()) || postQueryDto.postType().equals(VOTE.getType())) { + } + case "RECORD", "VOTE" -> { // ... - } + } + }Also applies to: 108-115
src/test/java/konkuk/thip/notification/application/service/RoomNotificationOrchestratorSyncImplTest.java (2)
47-64: MANDATORY 전파 검증 테스트 적정트랜잭션 없이 호출 시 IllegalTransactionStateException 검증 흐름 타당합니다. 예외 메시지까지 검증하면 원인 파악에 도움 됩니다.
- ).isInstanceOf(IllegalTransactionStateException.class); + ).isInstanceOf(IllegalTransactionStateException.class) + .hasMessageContaining("no existing transaction");
41-45: 정리 로직 중복 가능성각 테스트가 롤백된다면 deleteAllInBatch는 일부 케이스에서 불필요할 수 있습니다. 다만 현재 첫 테스트가 비트랜잭셔널이므로 유지 자체는 합리적입니다.
src/main/java/konkuk/thip/post/application/service/PostLikeService.java (2)
49-55: 좋아요 동시성 제어 TODO 구체화 제안카운트 갱신 경쟁 상태를 방지하려면 다음 중 하나를 고려하세요.
- 낙관적 락(버전 필드 기반) + 재시도
- DB 원자적 갱신(UPDATE ... SET like_count = like_count + 1 WHERE id=?)
- 분기별 카운트 테이블(append-only) 집계
원하시면 현재 JPA 매핑 기반으로 낙관적 락 패턴 예제를 드리겠습니다.
69-82: 분기 처리 명확화(switch/else-if)로 이중 호출 가능성 제거현재 if, if 구조는 유지보수 시 FEED가 다른 조건과 겹치는 리팩토링이 들어오면 이중 호출 위험이 있습니다. 가독성을 위해 switch 또는 else-if로 정리하세요.
- if (command.postType() == PostType.FEED) { + if (command.postType() == PostType.FEED) { feedNotificationOrchestrator.notifyFeedLiked(postQueryDto.creatorId(), actorUser.getId(), actorUser.getNickname(), postQueryDto.postId()); - } - if (command.postType() == PostType.RECORD || command.postType() == PostType.VOTE) { + } else if (command.postType() == PostType.RECORD || command.postType() == PostType.VOTE) { roomNotificationOrchestrator.notifyRoomPostLiked(postQueryDto.creatorId(), actorUser.getId(), actorUser.getNickname(), postQueryDto.roomId(), postQueryDto.page(), postQueryDto.postId(), postQueryDto.postType()); }src/main/java/konkuk/thip/notification/application/port/in/FeedNotificationOrchestrator.java (1)
5-9: 인터페이스 주석에 호출 제약 추가 제안“반드시 상위 트랜잭션 내에서 호출(Propagation.MANDATORY)” 제약을 자바독에 명시해 두면 사용자가 오용하기 어렵습니다.
- /** - * 비즈니스 로직 이후, NotificationOrchestrator 를 호출하여 알림 관련 로직 실행 - * -> DB에 notification data save + 푸시알림 - */ + /** + * 비즈니스 로직 이후, NotificationOrchestrator 를 호출하여 알림 관련 로직 실행. + * -> DB 저장 + 푸시 이벤트 발행. + * 제약: 반드시 상위 트랜잭션 내에서 호출해야 함(구현은 Propagation.MANDATORY). + */src/main/java/konkuk/thip/message/application/service/RoomNotificationDispatchService.java (2)
28-33: 미세 최적화: 토큰 없을 때 Notification 생성 피하기빈 토큰이면 조기 반환하므로, 토큰 조회를 먼저 수행해 객체 생성을 줄이세요.
- Notification notification = buildNotification(event.title(), event.content()); - - List<FcmToken> tokens = fcmTokenQueryPort.findEnabledByUserId(event.targetUserId()); - if (tokens.isEmpty()) return; + List<FcmToken> tokens = fcmTokenQueryPort.findEnabledByUserId(event.targetUserId()); + if (tokens.isEmpty()) return; + Notification notification = buildNotification(event.title(), event.content());같은 패턴을 다른 핸들러에도 적용 권장.
198-251: 중복 로직 헬퍼로 통합 제안(토큰 조회 + 메시지 전송 공통화)여러 메서드가 동일한 토큰 루프/배치 전송을 반복합니다. 라우트와 kv만 다르므로 헬퍼로 추출하면 유지보수 용이합니다.
예시(파일 내 추가 코드):
private void sendTo(Long userId, Notification n, MessageRoute route, String... kv) { List<FcmToken> tokens = fcmTokenQueryPort.findEnabledByUserId(userId); if (tokens.isEmpty()) return; List<Message> msgs = new ArrayList<>(tokens.size()); List<String> tk = new ArrayList<>(tokens.size()); List<String> dev = new ArrayList<>(tokens.size()); for (FcmToken t : tokens) { Message m = buildMessage(t.getFcmToken(), n, route, kv); msgs.add(m); tk.add(t.getFcmToken()); dev.add(t.getDeviceId()); } firebasePort.sendBatch(msgs, tk, dev); }적용 예:
public void handleRoomPostLiked(final RoomEvents.RoomPostLikedEvent event) { Notification n = buildNotification(event.title(), event.content()); sendTo(event.targetUserId(), n, MessageRoute.ROOM_POST_DETAIL, "roomId", String.valueOf(event.roomId()), "page", String.valueOf(event.page()), "type", "group", "postId", String.valueOf(event.postId()), "postType", String.valueOf(event.postType())); }src/main/java/konkuk/thip/notification/application/service/FeedNotificationOrchestratorSyncImpl.java (1)
29-44: 입력값 방어 코드 소량 추가 제안템플릿/파라미터가 null일 경우 NPE가 발생합니다. fail-fast로 방지하면 디버깅 용이합니다.
- private <T> void notifyWithTemplate( + private <T> void notifyWithTemplate( NotificationTemplate<T> template, T args, Long targetUserId, Runnable eventPublisher ) { - String title = template.title(args); - String content = template.content(args); + if (template == null || eventPublisher == null || targetUserId == null) + throw new IllegalArgumentException("template/eventPublisher/targetUserId must not be null"); + String title = template.title(args); + String content = template.content(args); saveNotification(title, content, targetUserId); eventPublisher.run(); }src/main/java/konkuk/thip/notification/application/service/RoomNotificationOrchestratorSyncImpl.java (2)
35-39: 이벤트 발행 실패 시 롤백 정책 확인 필요eventPublisher.accept 수행 중 예외가 발생하면 상위 트랜잭션까지 롤백됩니다. 의도라면 OK, 아니라면 이벤트 발행을 분리/격리(예: try-catch 후 별도 실패 큐 적재, 또는 TransactionalApplicationEventPublisher 활용)하는 방안을 검토해 주세요.
정책 질문: 이벤트 발행 실패로 비즈니스 트랜잭션을 롤백시키는 것이 맞나요?
41-44: 관찰성 보강 제안saveNotification 성공/실패, 발행한 이벤트 타입을 debug 로그나 메트릭으로 남기면 운영 관점에서 유용합니다.
src/main/java/konkuk/thip/notification/application/port/in/RoomNotificationOrchestrator.java (3)
5-8: 인터페이스 주석에 트랜잭션 요구사항 명시 제안구현체가 Propagation.MANDATORY를 강제하므로, 인터페이스 주석에도 “반드시 활성 트랜잭션 내에서 호출해야 함”을 명시해 주세요.
10-34: 파라미터 과다(오류 유발 가능) → Args 레코드/VO로 캡슐화 제안메서드별 인자가 많아 순서 실수 가능성이 큽니다. 도메인별 Args 레코드(또는 빌더)로 묶어 가독성과 안정성을 높이길 권장합니다.
11-13: postType은 문자열 대신 Enum 사용 권장타입 안정성과 오타 방지를 위해 postType을 Enum(예: RoomPostType)으로 교체하는 것을 고려해 주세요.
Also applies to: 26-28, 29-31, 32-34
src/main/java/konkuk/thip/message/adapter/out/event/dto/RoomEvents.java (1)
44-47: 네이밍 일관성(host/owner)RoomJoinRequestedToOwnerEvent(레코드) ↔ notifyRoomJoinToHost(오케스트레이터) 간 용어가 혼재합니다. 동일한 용어로 통일해 주세요.
📜 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 (58)
src/main/java/konkuk/thip/comment/application/service/CommentCreateService.java(4 hunks)src/main/java/konkuk/thip/comment/application/service/CommentLikeService.java(3 hunks)src/main/java/konkuk/thip/common/security/constant/SecurityWhitelist.java(1 hunks)src/main/java/konkuk/thip/common/security/filter/JwtAuthenticationFilter.java(2 hunks)src/main/java/konkuk/thip/config/SecurityConfig.java(2 hunks)src/main/java/konkuk/thip/feed/application/service/FeedCreateService.java(3 hunks)src/main/java/konkuk/thip/message/adapter/out/event/dto/FeedEvents.java(1 hunks)src/main/java/konkuk/thip/message/adapter/out/event/dto/RoomEvents.java(1 hunks)src/main/java/konkuk/thip/message/adapter/out/firebase/FirebaseAdapter.java(1 hunks)src/main/java/konkuk/thip/message/application/service/FeedNotificationDispatchService.java(4 hunks)src/main/java/konkuk/thip/message/application/service/RoomNotificationDispatchService.java(9 hunks)src/main/java/konkuk/thip/notification/adapter/out/persistence/NotificationCommandPersistenceAdapter.java(2 hunks)src/main/java/konkuk/thip/notification/adapter/out/persistence/NotificationQueryPersistenceAdapter.java(1 hunks)src/main/java/konkuk/thip/notification/application/port/in/DummyUseCase.java(0 hunks)src/main/java/konkuk/thip/notification/application/port/in/FeedNotificationOrchestrator.java(1 hunks)src/main/java/konkuk/thip/notification/application/port/in/RoomNotificationOrchestrator.java(1 hunks)src/main/java/konkuk/thip/notification/application/port/out/NotificationCommandPort.java(1 hunks)src/main/java/konkuk/thip/notification/application/port/out/NotificationQueryPort.java(0 hunks)src/main/java/konkuk/thip/notification/application/service/FeedNotificationOrchestratorSyncImpl.java(1 hunks)src/main/java/konkuk/thip/notification/application/service/NotificationService.java(0 hunks)src/main/java/konkuk/thip/notification/application/service/RoomNotificationOrchestratorSyncImpl.java(1 hunks)src/main/java/konkuk/thip/notification/application/service/template/NotificationTemplate.java(1 hunks)src/main/java/konkuk/thip/notification/application/service/template/feed/FeedCommentLikedTemplate.java(1 hunks)src/main/java/konkuk/thip/notification/application/service/template/feed/FeedCommentedTemplate.java(1 hunks)src/main/java/konkuk/thip/notification/application/service/template/feed/FeedLikedTemplate.java(1 hunks)src/main/java/konkuk/thip/notification/application/service/template/feed/FeedRepliedTemplate.java(1 hunks)src/main/java/konkuk/thip/notification/application/service/template/feed/FollowedTemplate.java(1 hunks)src/main/java/konkuk/thip/notification/application/service/template/feed/FolloweeNewPostTemplate.java(1 hunks)src/main/java/konkuk/thip/notification/application/service/template/room/RoomActivityStartedTemplate.java(1 hunks)src/main/java/konkuk/thip/notification/application/service/template/room/RoomCommentLikedTemplate.java(1 hunks)src/main/java/konkuk/thip/notification/application/service/template/room/RoomJoinToHostTemplate.java(1 hunks)src/main/java/konkuk/thip/notification/application/service/template/room/RoomPostCommentRepliedTemplate.java(1 hunks)src/main/java/konkuk/thip/notification/application/service/template/room/RoomPostCommentedTemplate.java(1 hunks)src/main/java/konkuk/thip/notification/application/service/template/room/RoomPostLikedTemplate.java(1 hunks)src/main/java/konkuk/thip/notification/application/service/template/room/RoomRecordCreatedTemplate.java(1 hunks)src/main/java/konkuk/thip/notification/application/service/template/room/RoomRecruitClosedEarlyTemplate.java(1 hunks)src/main/java/konkuk/thip/notification/application/service/template/room/RoomVoteStartedTemplate.java(1 hunks)src/main/java/konkuk/thip/notification/domain/Notification.java(1 hunks)src/main/java/konkuk/thip/notification/domain/value/NotificationCategory.java(1 hunks)src/main/java/konkuk/thip/post/application/service/PostLikeService.java(3 hunks)src/main/java/konkuk/thip/room/application/service/RoomJoinService.java(4 hunks)src/main/java/konkuk/thip/room/application/service/RoomRecruitCloseService.java(3 hunks)src/main/java/konkuk/thip/room/application/service/RoomStateChangeService.java(3 hunks)src/main/java/konkuk/thip/roompost/application/service/RecordCreateService.java(3 hunks)src/main/java/konkuk/thip/roompost/application/service/VoteCreateService.java(3 hunks)src/main/java/konkuk/thip/user/application/service/following/UserFollowService.java(3 hunks)src/test/java/konkuk/thip/notification/application/service/FeedNotificationOrchestratorSyncImplTest.java(1 hunks)src/test/java/konkuk/thip/notification/application/service/FeedNotificationOrchestratorSyncImplUnitTest.java(1 hunks)src/test/java/konkuk/thip/notification/application/service/RoomNotificationOrchestratorSyncImplTest.java(1 hunks)src/test/java/konkuk/thip/notification/application/service/RoomNotificationOrchestratorSyncImplUnitTest.java(1 hunks)src/test/java/konkuk/thip/room/adapter/in/web/RoomCloseJoinApiTest.java(0 hunks)src/test/java/konkuk/thip/room/adapter/in/web/RoomJoinApiTest.java(3 hunks)src/test/java/konkuk/thip/room/application/service/RoomJoinServiceTest.java(3 hunks)src/test/java/konkuk/thip/roompost/adapter/in/web/RecordCreateApiTest.java(1 hunks)src/test/java/konkuk/thip/roompost/adapter/in/web/VoteCreateApiTest.java(3 hunks)src/test/java/konkuk/thip/roompost/application/service/VoteCreateServiceTest.java(2 hunks)src/test/java/konkuk/thip/user/adapter/in/web/UserFollowApiTest.java(2 hunks)src/test/java/konkuk/thip/user/application/service/UserFollowServiceTest.java(3 hunks)
💤 Files with no reviewable changes (4)
- src/main/java/konkuk/thip/notification/application/port/in/DummyUseCase.java
- src/main/java/konkuk/thip/notification/application/service/NotificationService.java
- src/main/java/konkuk/thip/notification/application/port/out/NotificationQueryPort.java
- src/test/java/konkuk/thip/room/adapter/in/web/RoomCloseJoinApiTest.java
🧰 Additional context used
🧠 Learnings (4)
📓 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#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#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#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#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 단일 커서 방식을 선호하며, 복합 키 기반 커서보다 구현 단순성과 성능을 우선시한다.
📚 Learning: 2025-08-14T12:29:00.150Z
Learnt from: buzz0331
PR: THIP-TextHip/THIP-Server#183
File: src/main/java/konkuk/thip/roompost/application/service/helper/RoomProgressHelper.java:33-45
Timestamp: 2025-08-14T12:29:00.150Z
Learning: 방 생성 시 항상 HOST 역할의 RoomParticipant가 생성되며, RoomProgressHelper.updateUserAndRoomProgress() 메서드는 사용자가 해당 방의 참여자임이 사전 검증된 상황에서만 호출되므로 참여자 목록이 비어있을 수 없다.
Applied to files:
src/main/java/konkuk/thip/room/application/service/RoomRecruitCloseService.javasrc/main/java/konkuk/thip/roompost/application/service/RecordCreateService.java
📚 Learning: 2025-07-03T03:05:05.031Z
Learnt from: seongjunnoh
PR: THIP-TextHip/THIP-Server#43
File: src/main/java/konkuk/thip/book/application/port/out/BookCommandPort.java:0-0
Timestamp: 2025-07-03T03:05:05.031Z
Learning: THIP 프로젝트에서는 CQRS Port 분리 시 다음 컨벤션을 따름: CommandPort에는 findByXXX를 통해 도메인 엔티티를 찾아오는 메서드를 추가하고, QueryPort에는 조회 API의 response에 해당하는 데이터들을 DB로부터 조회하는 메서드를 추가함.
Applied to files:
src/main/java/konkuk/thip/comment/application/service/CommentLikeService.java
📚 Learning: 2025-07-26T06:09:00.850Z
Learnt from: seongjunnoh
PR: THIP-TextHip/THIP-Server#101
File: src/main/java/konkuk/thip/comment/application/service/CommentCreateService.java:36-39
Timestamp: 2025-07-26T06:09:00.850Z
Learning: THIP 프로젝트에서 Record와 Vote는 Room에 속하지만 Feed는 Room에 속하지 않는 구조이며, 댓글 작성 시 Record/Vote에 대해서만 사용자가 해당 Room의 참가자인지 검증이 필요하다.
Applied to files:
src/main/java/konkuk/thip/message/adapter/out/event/dto/RoomEvents.java
🧬 Code graph analysis (2)
src/test/java/konkuk/thip/notification/application/service/FeedNotificationOrchestratorSyncImplTest.java (1)
src/test/java/konkuk/thip/common/util/TestEntityFactory.java (1)
TestEntityFactory(32-370)
src/test/java/konkuk/thip/notification/application/service/RoomNotificationOrchestratorSyncImplTest.java (1)
src/test/java/konkuk/thip/common/util/TestEntityFactory.java (1)
TestEntityFactory(32-370)
⏰ 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 (45)
src/main/java/konkuk/thip/common/security/constant/SecurityWhitelist.java (1)
13-24: 화이트리스트 항목 구성은 일관적입니다.Swagger, OAuth2, 헬스체크, 인증 관련 엔드포인트가 잘 정리되어 있어 유지보수성이 좋아졌습니다.
src/main/java/konkuk/thip/notification/adapter/out/persistence/NotificationQueryPersistenceAdapter.java (1)
10-15: Port 제거 영향 점검 필요(DIP 약화 가능성) — 리포지토리 검색 결과 참조 없음NotificationQueryPort와 NotificationQueryPersistenceAdapter에 대한 리포지토리 내 참조가 없습니다.
- 대상: src/main/java/konkuk/thip/notification/adapter/out/persistence/NotificationQueryPersistenceAdapter.java
- 확인: 빌드/CI(컴파일·런타임 빈 등록) 오류 없음 확인
- 확인: 멀티모듈/외부 레포/런타임 리플렉션 등에서 사용되지 않는지 확인
- 조치: 의도된 단순화면 커밋/아키텍처 문서에 제거 이유 명시, 아니라면 Port 복원(계층 추상화 유지)
src/test/java/konkuk/thip/user/adapter/in/web/UserFollowApiTest.java (3)
4-4: NotificationJpaRepository import 추가 적절알림 영속 경로가 테스트 플로우에서 열리므로, 테스트 간 데이터 오염 방지를 위한 정리 목적의 import는 타당합니다.
38-40: 알림 리포지토리 주입 추가 OK알림 데이터가 생성되는 시나리오에 대비한 의존성 주입이 합리적입니다. 현재 테스트 클래스 전반의 DI 스타일(필드 주입)과도 일관됩니다.
45-46: 삭제 순서·AFTER_COMMIT 점검 — 현 상태 안전검증: src/main/java/konkuk/thip/notification/adapter/out/jpa/NotificationJpaEntity.java에서 Notification → User(@manytoone) 참조 확인, src/main/java/konkuk/thip/user/adapter/out/jpa/FollowingJpaEntity.java는 User만 참조함. 따라서 notificationJpaRepository.deleteAllInBatch(); userJpaRepository.deleteAllInBatch(); 순서는 FK 제약상 안전합니다.
명시적 @TransactionalEventListener(...AFTER_COMMIT) 패턴은 검색되지 않았음 — 커밋 후 비동기 리스너로 인한 추가 플래키 리스크는 발견되지 않았습니다.
권장: 장기적으로 테스트 초기화는 참조 무결성 비활성화 후 TRUNCATE하는 DatabaseCleaner로 추상화하면 안정성과 속도 향상에 도움이 됩니다.
src/main/java/konkuk/thip/message/adapter/out/firebase/FirebaseAdapter.java (3)
15-17: 프로필 표현식 수정, 의도대로 동작합니다
@Profile("!test & !local")로 바뀌면서 test/local에서는 빈이 로드되지 않습니다. 운영/개발에서만 활성화되어 FCM 실제 연동을 피할 수 있어요.
16-16: 스프링 버전 확인 필요 — @Profile 표현식 '&' 호환성 검증
- 레포 검색 결과(@Profile 사용처):
- src/main/java/konkuk/thip/config/WorkerThreadConfig.java — @Profile("!test")
- src/main/java/konkuk/thip/message/adapter/out/firebase/FirebaseAdapter.java — @Profile("!test & !local")
- src/main/java/konkuk/thip/message/adapter/out/firebase/FakeFirebaseAdapter.java — @Profile("local | test")
- src/test/java/konkuk/thip/config/TestAsyncConfig.java — @Profile("test")
- 빌드파일(pom/gradle)에서 스프링/부트 버전 정보가 검색되지 않음. 스프링/부트 버전(또는 parent BOM) 위치를 알려주거나 확인한 뒤 '&' 연산자 지원 여부를 검증하거나 표현식을 호환되는 형태로 변경하세요.
42-45: sendEach 사용 가능 — 변경 불필요com.google.firebase:firebase-admin:9.3.0은 FirebaseMessaging.sendEach(List)를 지원(반면 sendAll은 deprecated)하므로 현재 sendEach(messages) 호출을 유지하면 됩니다. (mvnrepository.com)
src/test/java/konkuk/thip/roompost/adapter/in/web/VoteCreateApiTest.java (1)
70-72: 알림 리포지토리 주입 및 정리 순서 LGTMNotificationJpaRepository 주입과 tearDown에서의 deleteAllInBatch 추가가 일관된 테스트 격리를 보장합니다. 사용자/연관 엔터티 삭제 전에 알림 정리하는 순서도 안전합니다.
Also applies to: 83-83
src/test/java/konkuk/thip/roompost/application/service/VoteCreateServiceTest.java (1)
59-61: 알림 리포지토리 정리 추가 LGTMNotificationJpaRepository 주입 및 tearDown 정리 추가로 테스트 간 간섭 가능성이 줄었습니다. 다른 통합 테스트와 정리 순서를 맞춘 점도 좋습니다.
Also applies to: 69-69
src/main/java/konkuk/thip/message/adapter/out/event/dto/FeedEvents.java (1)
10-12: 이벤트 시그니처 변경 영향 범위 점검 필요FeedEvents 레코드의 canonical constructor 인자 순서가 변경되어 직접 생성자 호출(new 또는 Kotlin 호출)이나 ::new 메서드 레퍼런스가 있으면 컴파일/런타임 오류가 발생합니다. 빌더만 사용 중인지 확인하세요.
자동 스캔을 시도했으나 샌드박스(/dev/fd) 제약으로 확인 실패. 로컬에서 아래 명령으로 직접 점검해 주세요.
대상: src/main/java/konkuk/thip/message/adapter/out/event/dto/FeedEvents.java (예: 10–12, 16–20, 23–27, 30–34, 37–41, 44–48)
검사 명령(로컬 실행):
# Java/Kotlin 직접 new 또는 ::new 탐지 grep -REn -E --include='*.java' --include='*.kt' "(new[[:space:]]*(FeedEvents\.)?(FollowerEvent|FeedCommentedEvent|FeedCommentRepliedEvent|FolloweeNewPostEvent|FeedLikedEvent|FeedCommentLikedEvent)[[:space:]]*\(|(FeedEvents\.)?(FollowerEvent|FeedCommentedEvent|FeedCommentRepliedEvent|FolloweeNewPostEvent|FeedLikedEvent|FeedCommentLikedEvent)::new)" . # Kotlin 파일의 암시적 생성자 호출 탐지 grep -REn -E --include='*.kt' "(FeedEvents\.)?(FollowerEvent|FeedCommentedEvent|FeedCommentRepliedEvent|FolloweeNewPostEvent|FeedLikedEvent|FeedCommentLikedEvent)[[:space:]]*\(" . # import/직접 참조 확인 grep -REn --include='*.java' --include='*.kt' "import .*FeedEvents\.(FollowerEvent|FeedCommentedEvent|FeedCommentRepliedEvent|FolloweeNewPostEvent|FeedLikedEvent|FeedCommentLikedEvent)|FeedEvents\.(FollowerEvent|FeedCommentedEvent|FeedCommentRepliedEvent|FolloweeNewPostEvent|FeedLikedEvent|FeedCommentLikedEvent)" .또한 FCM 페이로드/타이틀 길이 제약을 고려해 템플릿 단계에서 제목(권장 ~50자)·본문(권장 ~150자) 길이 제한 및 말줄임 적용을 권장합니다.
src/main/java/konkuk/thip/user/application/service/following/UserFollowService.java (1)
4-4: 오케스트레이터 기반 팔로우 알림 전환 LGTM팔로우 상태 변경이 커밋될 때 DB 알림 저장+푸시 발행이 보장됩니다. 호출 위치도 저장 직후로 적절합니다.
Also applies to: 26-26, 58-58
src/main/java/konkuk/thip/roompost/application/service/VoteCreateService.java (1)
38-38: 오케스트레이터 전환 적절트랜잭션 경계 내 호출과 exclude-self 처리 모두 일관적입니다. 👍
Also applies to: 84-84
src/test/java/konkuk/thip/room/adapter/in/web/RoomJoinApiTest.java (1)
7-7: 테스트 정리 로직 개선 LGTMNotificationJpaRepository 주입 및 deleteAllInBatch 순서(참여자→방→도서→알림→유저) 적절합니다. 테스트 격리 보장에 도움됩니다.
Also applies to: 52-52, 115-118
src/main/java/konkuk/thip/comment/application/service/CommentLikeService.java (1)
35-36: 오케스트레이터 전환 및 호출 인자 구성 적절self-like 필터 이후 actor 조회, FEED/ROOM 분기 모두 타당합니다.
Also applies to: 76-81
src/main/java/konkuk/thip/room/application/service/RoomRecruitCloseService.java (1)
5-5: 오케스트레이터 전환 LGTM호스트 제외 로직 유지, 제목 전달 등 인자 구성도 적절합니다.
Also applies to: 24-24, 54-55
src/main/java/konkuk/thip/notification/application/service/template/feed/FeedLikedTemplate.java (1)
6-20: 템플릿 추가 LGTM카테고리 prefix와 Args 구성 적절합니다.
src/main/java/konkuk/thip/notification/application/service/template/feed/FeedRepliedTemplate.java (1)
9-17: LGTM — 복수 템플릿과 문구 일관성 양호카테고리 프리픽스 사용과 문구 톤이 다른 피드 템플릿과 일관됩니다.
src/main/java/konkuk/thip/notification/application/service/template/room/RoomPostCommentRepliedTemplate.java (1)
9-17: LGTM — 룸/피드 답글 템플릿 간 톤·포맷 일관카테고리 프리픽스와 콘텐츠 포맷이 피드 답글 템플릿과 잘 맞습니다.
src/main/java/konkuk/thip/notification/application/service/template/room/RoomPostLikedTemplate.java (1)
9-17: LGTM — 명확한 타이틀과 액터 포함 콘텐츠의도 명확하고 UI에 적합한 길이입니다.
src/main/java/konkuk/thip/notification/application/service/template/feed/FolloweeNewPostTemplate.java (1)
6-20: 구조와 의도 명확, 템플릿 패턴 일관 적용싱글톤 enum+record 패턴이 다른 템플릿과 일관되며 사용성이 좋습니다.
src/test/java/konkuk/thip/notification/application/service/FeedNotificationOrchestratorSyncImplTest.java (1)
28-28: 프록시 주입 의도 주석 좋습니다MANDATORY 전파 확인을 위해 인터페이스 타입으로 주입하는 선택 적절합니다.
src/main/java/konkuk/thip/notification/application/service/template/feed/FeedCommentedTemplate.java (1)
6-20: 구현 간결하고 일관적입니다카테고리 접두어+템플릿 분리가 명확합니다.
src/main/java/konkuk/thip/notification/application/service/template/feed/FeedCommentLikedTemplate.java (1)
6-20: 일관된 템플릿 구성 좋습니다피드 좋아요 템플릿이 간결하고 다른 템플릿과 톤이 맞습니다.
src/test/java/konkuk/thip/notification/application/service/FeedNotificationOrchestratorSyncImplUnitTest.java (2)
17-25: 트랜잭션 컨텍스트 검증 누락 가능성PR 설명상 Orchestrator는 외부 트랜잭션 없이 호출 시 IllegalTransactionStateException을 던져야 합니다. 이 테스트는 MockitoExtension만 사용하고 있어 실제 트랜잭션 없이도 sut 메서드가 호출·검증됩니다. 트랜잭션 활성화 또는 예외 검증 테스트를 추가해 의도와 정합성을 맞춰주세요. 예: 트랜잭션 미존재 시 예외 발생 테스트 또는 afterCommit 시점 검증(통합 테스트) 보강.
47-50: 이벤트 페이로드(title/content) 전달 여부 확인 필요PR 목표에 따르면 FCM 이벤트에 사전 생성된 title/content가 포함되어야 합니다. 현재 포트 호출 검증은 식별자 인자만 확인합니다. 이벤트 객체/DTO에 title/content가 실리는지 캡처/검증하거나, 포트 시그니처가 이를 수용하는지 확인 부탁드립니다.
src/test/java/konkuk/thip/notification/application/service/RoomNotificationOrchestratorSyncImplUnitTest.java (2)
17-25: 트랜잭션 요구사항 테스트 반영 필요Orchestrator가 트랜잭션 없을 때 예외를 던지는지, 또는 afterCommit에 맞춰 동작하는지에 대한 검증이 없습니다. 스프링 트랜잭션 컨텍스트에서 커밋 시점 동작을 검증하는 테스트(통합) 또는 미트랜잭션 예외 테스트(단위)를 추가해주세요.
167-169: 이벤트에 사전 생성 title/content 포함 여부 확인포트 호출은 식별자 위주입니다. 이벤트 페이로드가 DB에 저장된 title/content와 동일한 값을 포함하는지(또는 포함되도록 포트 시그니처가 확장되었는지) 확인·보강이 필요합니다.
src/main/java/konkuk/thip/room/application/service/RoomJoinService.java (3)
54-57: Orchestrator 전환 및 호출 위치 적절JOIN 흐름에서 도메인 업데이트 후 Orchestrator 호출로 커밋 의존 동작을 보장하는 선택이 합리적입니다. 👍
62-66: 호스트 조회 실패 시 예외 전파 확인findHostByRoomId가 호스트를 보장하지 못하는 경우 예외 유형/메시지가 일관되는지 확인해주세요(비정상 상태에서 알림 누락 대비).
68-81: CANCEL 분기 단순화 LGTM불필요한 파라미터 제거와 검증 흐름이 명확해졌습니다.
src/main/java/konkuk/thip/comment/application/service/CommentCreateService.java (2)
58-62: 알림 등록 시점 검토(트랜잭션 의존)알림 등록이 댓글 저장 이전에 수행됩니다. 롤백 시 afterCommit이 실행되지 않으므로 안전하지만, 트랜잭션 없을 때의 방어(예외)와 의도된 시점인지 확인 부탁드립니다.
98-114: Orchestrator 전환 적합이벤트 포트 직접 호출에서 Orchestrator로의 이관이 PR 목표와 부합합니다.
src/test/java/konkuk/thip/notification/application/service/RoomNotificationOrchestratorSyncImplTest.java (3)
28-30: 프록시 관점에서 인터페이스 타입 주입 👍트랜잭션 프록시 적용을 위해 인터페이스 타입으로 주입한 선택 적절합니다.
56-57: postType 타입 일관성 — 문제 없음RoomNotificationOrchestrator 인터페이스와 구현의 notifyRoomPostCommented 시그니처가 모두 String postType을 사용하므로 테스트에서 "RECORD" 문자열을 전달하는 것은 올바릅니다.
Likely an incorrect or invalid review comment.
66-91: AFTER_COMMIT 이벤트(커밋 이후 리스너) 경로는 현재 테스트로 검증되지 않음 — 리스너 존재/동작 수동 확인 필요실행하신 rg 명령이 "No files were searched"를 반환해 코드베이스에서 AFTER_COMMIT 리스너 유무를 확인할 수 없습니다. 아래 중 하나를 수행해 검증하세요.
- 대상: src/test/java/konkuk/thip/notification/application/service/RoomNotificationOrchestratorSyncImplTest.java (lines 66–91) — 현재 테스트는 DB 저장만 검증함; 커밋 이후 이벤트 발행/FCM 전송 경로는 검증되지 않음.
- 검증 방법(예)
// 방법 1: TestTransaction 사용(강제 커밋) @Test @Transactional void after_commit_event_published() { orchestrator.notifyRoomPostCommented(...); TestTransaction.flagForCommit(); TestTransaction.end(); // 커밋 발생 // 이벤트 리스너 관측(예: Spy/Fake adapter 또는 Outbox 상태 확인) }// 방법 2: 트랜잭션 분리 후 명시적 트랜잭션으로 검증 @Test void after_commit_event_published_separate_tx() { // @Transactional(propagation = NOT_SUPPORTED)로 호출한 뒤 // 별도 트랜잭션에서 리스너 효과 확인 }
- 로컬에서 리스너 존재 확인(권장 명령):
rg -nP --hidden --no-ignore '@TransactionalEventListener\s*\(\s*phase\s*=\s*AFTER_COMMIT' -S || rg -n '@TransactionalEventListener' -Ssrc/main/java/konkuk/thip/post/application/service/PostLikeService.java (2)
34-35: 이벤트 포트 → 오케스트레이터 전환 LGTM도메인 서비스가 트랜잭션 경계 내에서 오케스트레이터를 호출하도록 변경된 점이 PR 목표와 일치합니다.
70-76: 확인: PostQueryDto 매핑이 의도대로 동작함
FEED 분기 — postQueryDto.postId()는 feed.postId로 반환됩니다.
RECORD/VOTE 분기 — postQueryDto.postId는 post.postId이고 page와 roomId는 각각 Record/Vote의 page·roomId로 채워져 라우팅(룸 상세: roomId/page/postId/postType) 기준과 일치합니다.
(근거: src/main/java/konkuk/thip/post/adapter/out/persistence/repository/PostQueryRepositoryImpl.java)src/main/java/konkuk/thip/message/application/service/RoomNotificationDispatchService.java (2)
236-238: 제목 prefix 제거 정책 반영 👍템플릿/오케스트레이터에서 제목을 결정하고, 여기서는 raw title을 그대로 사용하도록 변경된 점 일관적입니다.
53-71: postType 전달 일관성 확인 — RoomVoteStartedEvent에 postType 없음RoomVoteStartedEvent 레코드(src/main/java/konkuk/thip/message/adapter/out/event/dto/RoomEvents.java:18)에 postType 필드가 없어 event.postType()를 사용할 수 없습니다.
조치(택1):
- 이벤트 DTO에 postType(String) 추가하고 buildMessage에서 event.postType()로 통일.
- 또는 하드코드 "VOTE" 유지 시 의도(고정값 이유)를 코드 주석으로 남기고 다른 이벤트와의 일관성 여부 검토.
src/main/java/konkuk/thip/notification/application/service/FeedNotificationOrchestratorSyncImpl.java (2)
18-24: 설계 주석 명확 👌정책 주석(동기 저장 + 커밋 후 비동기 전송)이 명확합니다. 구현과도 일치합니다.
25-39: AFTER_COMMIT 보장 여부 확인 필요 — 트랜잭션 롤백 시 이벤트 미발행 보장
검증 결과: @TransactionalEventListener(phase = AFTER_COMMIT)를 사용하는 리스너를 찾지 못했습니다. 현재 이벤트는 ApplicationEventPublisher.publishEvent(...)로 즉시 발행됩니다.
발행 지점: src/main/java/konkuk/thip/message/adapter/out/event/FeedEventPublisherAdapter.java (publishFeed* 메서드들)
호출 지점: src/main/java/konkuk/thip/notification/application/service/FeedNotificationOrchestratorSyncImpl.java (notifyWithTemplate -> feedEventCommandPort.publish...)
조치: 리스너가 TransactionPhase.AFTER_COMMIT로 선언되어 있는지 확인하거나, 퍼블리셔를 트랜잭션 커밋 이후에 안전하게 발행하도록 변경하세요. 단위 테스트는 publish 호출만 검증하므로 롤백 시 이벤트 미발행 여부는 통합/트랜잭션 레벨 테스트로 추가 검증해야 합니다.src/main/java/konkuk/thip/notification/application/service/RoomNotificationOrchestratorSyncImpl.java (1)
48-58: MANDATORY 전파로 트랜잭션 경계 강제 — 의도 적합외부 비즈니스 트랜잭션 없을 때 호출을 막아 DB 저장만 성공하는 상황을 예방합니다. 설계 의도와 맞습니다.
src/main/java/konkuk/thip/message/adapter/out/event/dto/RoomEvents.java (2)
11-22: title/content 선행 추가 좋습니다FCM/DB 양쪽에서 동일 콘텐츠를 재사용할 수 있게 되어 일관성과 비용(템플릿 재계산) 측면에서 이점이 큽니다.
11-69: 검증 완료 — 빌더로 이벤트 생성 중(캐노니컬 생성자 직접 호출 없음)레포 검색 결과 'new RoomEvents.*Event(...)' 호출은 발견되지 않았고, RoomEventPublisherAdapter 및 테스트(MessageRoomEventListenerTest) 등에서 .builder()로 이벤트를 생성하고 있어 파라미터 순서 변경 영향 없음.
src/main/java/konkuk/thip/common/security/filter/JwtAuthenticationFilter.java
Outdated
Show resolved
Hide resolved
.../java/konkuk/thip/notification/application/service/RoomNotificationOrchestratorSyncImpl.java
Outdated
Show resolved
Hide resolved
.../konkuk/thip/notification/application/service/template/room/RoomActivityStartedTemplate.java
Show resolved
Hide resolved
.../java/konkuk/thip/notification/application/service/template/room/RoomJoinToHostTemplate.java
Show resolved
Hide resolved
...nkuk/thip/notification/application/service/template/room/RoomRecruitClosedEarlyTemplate.java
Show resolved
Hide resolved
...java/konkuk/thip/notification/application/service/template/room/RoomVoteStartedTemplate.java
Show resolved
Hide resolved
- 이벤트 리스너가 이벤트를 잘 수신하는지 검증하는 테스트 메서드 추가
buzz0331
left a comment
buzz0331
left a comment
There was a problem hiding this comment.
굿굿 좋습니다! 엄청 고민하신게 보이는 코드였습니다!! 테스트까지 엄청 꼼꼼히 작성해주셔서 놀랐습니다 💯 💯
몇가지 리뷰 남겼는데 확인부탁드려요~
src/main/java/konkuk/thip/common/security/filter/JwtAuthenticationFilter.java
Outdated
Show resolved
Hide resolved
|
|
||
| @Slf4j | ||
| @Component | ||
| @Profile({"!test", "!local"}) |
.../java/konkuk/thip/notification/application/service/RoomNotificationOrchestratorSyncImpl.java
Outdated
Show resolved
Hide resolved
| private <T> void notifyWithTemplate( | ||
| NotificationTemplate<T> template, | ||
| T args, | ||
| Long targetUserId, | ||
| Runnable eventPublisher | ||
| ) { | ||
| String title = template.title(args); | ||
| String content = template.content(args); | ||
| saveNotification(title, content, targetUserId); | ||
| eventPublisher.run(); |
There was a problem hiding this comment.
이건 제가 푸쉬알림하면서 고려하지 못했던 부분이긴 한데 지금처럼 트랜잭션 내부에서 이벤트가 발행될때 예외가 발생하면 API 자체가 실패해버리니 조금 치명적일 것 같은데 eventPublisher.run() 부분만 try-catch 문으로 감싸서 예외가 전파되지 않도록 하는 것은 어떨까요??
There was a problem hiding this comment.
현재 api 서비스에서 시작한 트랜잭션 범위 내에서 이벤트 퍼블리시가 발생하는건 맞지만, 이벤트 리스너의 실행 시점이 after commit 이므로 이벤트 퍼블리시 시점에서는 단순히 콜백 등록만을 수행하니 문제없지 않나? 라는 생각으로 구현을 했긴 합니다.
만약 저희가 api 의 로직과 알림 관련된 로직을 아예 독립적으로 분리하겠다 라고 요구사항을 정한다면, 이벤트 퍼블리시의 시점을 after commit 으로 수정하는 방법이 있을 것 같습니다 (이러면 이벤트 리스너에서의 after commit 설정은 제거해도 괜찮을 것 같습니다)
위와 같이 api 로직과 알림 관련 로직을 분리하는 것으로 결정한다면, DB notification 의 저장 로직 또한 api 로직과 분리할 것인지, 아니면 이건 api 로직의 일부로 생각할 것인지 또한 논의해봐야할 것 같습니다!
@buzz0331 @hd0rable @coderabbitai 관련해서 의견 부탁드립니다!!
There was a problem hiding this comment.
@seongjunnoh 좋은 질문입니다! 현재 구현에 대해 분석해보겠습니다.
현재 구현의 장단점
현재 방식 (DB 저장: 동기, 이벤트 발행: 동기 + AFTER_COMMIT 리스너):
장점:
- DB 알림 저장이 비즈니스 트랜잭션과 강하게 연결되어 데이터 일관성 보장
ApplicationEventPublisher.publishEvent()는 일반적으로 안전하며 콜백 등록만 수행- 구현이 단순하고 이해하기 쉬움
단점:
- 이론적으로 이벤트 발행 시 예외 발생 가능성 (매우 낮음)
- API 로직과 알림 로직이 강하게 결합
대안 접근법들
1. 이벤트 발행만 AFTER_COMMIT으로 분리
@TransactionalEventListener(phase = TransactionPhase.AFTER_COMMIT)
public void publishEvent() {
// 이벤트 발행을 커밋 후로 이동
}- 장점: API 로직과 완전 분리, 더 안전
- 단점: DB 알림은 저장되었지만 FCM 푸시 실패 시 불일치 가능
2. 완전 분리 (둘 다 AFTER_COMMIT)
- 장점: 완전한 독립성, 높은 복원력
- 단점: 알림 데이터 손실 가능성, 복잡한 재시도 로직 필요
3. 현재 + try-catch (buzz0331 제안)
try {
eventPublisher.run();
} catch (Exception e) {
log.error("Event publishing failed", e);
// 예외를 삼키고 계속 진행
}권장사항
현재 구현을 유지하되, 다음 개선사항을 제안합니다:
- 모니터링 강화: 이벤트 발행 실패를 로깅/메트릭으로 추적
- 점진적 개선: 필요시 나중에 AFTER_COMMIT으로 이동 가능한 구조 유지
- 비즈니스 요구사항 우선: DB 알림의 중요도에 따라 결정
// 현재 구현에 로깅만 추가
private <T> void notifyWithTemplate(...) {
String title = template.title(args);
String content = template.content(args);
saveNotification(title, content, targetUserId);
try {
eventPublisher.run();
} catch (Exception e) {
log.error("Event publishing failed for user {}", targetUserId, e);
// 메트릭 기록
}
}seongjunnoh의 분석이 정확합니다. ApplicationEventPublisher는 일반적으로 안전하며, AFTER_COMMIT 설정으로 인해 실제 위험은 매우 낮습니다. 현재 구조가 단순하고 효과적이므로 유지하는 것을 권장합니다.
🧠 Learnings used
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#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#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#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#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 단일 커서 방식을 선호하며, 복합 키 기반 커서보다 구현 단순성과 성능을 우선시한다.
There was a problem hiding this comment.
현준님이 리뷰 주신것처럼 현 상황에서는 이벤트 퍼블리시 과정을 try catch 로 묶는게 가장 수정사항이 적으면서도 합리적일 것 같다는 생각이 들었는데, 이렇게 수정해볼까요??
There was a problem hiding this comment.
Seems like the humans are having a chat. I'll hop back into my burrow for now. If you need me again, just tag @coderabbitai in a new comment, and I'll come hopping out!
There was a problem hiding this comment.
이벤트 리스너의 실행 시점이 after commit 이므로 이벤트 퍼블리시 시점에서는 단순히 콜백 등록만을 수행하니 문제없지 않나? 라는 생각으로 구현을 했긴 합니다.
성준님이 언급해주신것처럼 이벤트 처리과정에서 에러가 발생했을때 트랜잭션이 롤백되는경우는 거의 없을것같긴하지만 만약 문제가 생겼을시 현준님이 언급하신것처럼 try-catch로 이벤트 퍼블리셔 예외를 감싸고 로깅을 추가하면 운영상 버그나 모니터링에도 좋을 것 같습니다!!
There was a problem hiding this comment.
네넵 저는 희진님이 말씀하신 것처럼 단순하게 로깅만 추가하여 모니터링 강화를 말씀드린 거긴 했습니다! 성준님이 말씀하신 것처럼 이벤트 발행 자체를 AFTER_COMMIT 으로 빼면 이벤트 유실률이 더 높아질 것이 우려되네요. 안정성을 위해서 트랜잭션 내부에서 이벤트 발행을 하는 것을 유지하는게 지금 상황에서는 최선이라고 생각합니다~!
There was a problem hiding this comment.
넵 그러면 위와 같이 수정하여 푸시하도록 하겠습니다!!
| package konkuk.thip.notification.application.service.template; | ||
|
|
||
| public interface NotificationTemplate<T> { | ||
|
|
||
| String title(T args); | ||
|
|
||
| String content(T args); | ||
| } |
| @@ -0,0 +1,20 @@ | |||
| package konkuk.thip.notification.application.service.template.feed; | |||
There was a problem hiding this comment.
음 패키지는 조금 애매하네요.. adapter에 두기에는 저희 자체 템플릿이라.. 일단 지금 구조가 최선인거 같긴 하네요 👍🏻
...nkuk/thip/notification/application/service/template/room/RoomRecruitClosedEarlyTemplate.java
Show resolved
Hide resolved
| @Test | ||
| @DisplayName("상위 트랜잭션 없이 호출하면 IllegalTransactionStateException 발생 (MANDATORY)") | ||
| void mandatory_without_transaction_throws() { | ||
| // when & then | ||
| assertThatThrownBy(() -> | ||
| orchestrator.notifyFeedCommented( | ||
| targetUserId, /*actor*/ 999L, "alice", /*feedId*/ 123L | ||
| ) | ||
| ).isInstanceOf(IllegalTransactionStateException.class); | ||
| } | ||
|
|
엇 현준님 제가 title, content 를 이벤트에 넣겠다고 해놓고서 빼먹은 부분이 있어서 방금 수정한 코드를 추가로 푸시했습니다! |
There was a problem hiding this comment.
Actionable comments posted: 0
♻️ Duplicate comments (1)
src/main/java/konkuk/thip/notification/application/service/RoomNotificationOrchestratorSyncImpl.java (1)
31-38: 이전 코멘트에서 지적된 title/content 미전달 이슈 해결됨
- 각 이벤트 퍼블리시 람다에서
title, content를 선두에 정확히 전달합니다. Port 시그니처와 정렬 OK.Also applies to: 45-52, 60-67, 75-81, 88-95, 103-110, 118-124, 132-139, 147-155
🧹 Nitpick comments (17)
src/main/java/konkuk/thip/notification/application/service/EventCommandInvoker.java (1)
6-6: 계약 가시성 보강: Javadoc으로 비즈니스 제약 명시 제안title/content가 비어 있지 않음을 계약으로 문서화하면 구현체/테스트 일관성이 좋아집니다.
public interface EventCommandInvoker { - void publish(String title, String content); + /** + * 알림 이벤트를 발행한다. + * @param title 비어 있지 않은 제목 + * @param content 비어 있지 않은 내용 + */ + void publish(String title, String content); }src/main/java/konkuk/thip/notification/application/service/NotificationSyncExecutor.java (3)
15-29: 입력값 방어 코드 추가로 안전성 높이기외부(오케스트레이터/템플릿)로부터 전달되는 인자에 대한 null/blank 방어가 없어 NPE/빈 알림 저장 가능성이 있습니다. 가벼운 선행 검증을 권장합니다.
import lombok.RequiredArgsConstructor; +import java.util.Objects; @HelperService @RequiredArgsConstructor public class NotificationSyncExecutor { @@ public <T> void execute( NotificationTemplate<T> template, T args, Long targetUserId, EventCommandInvoker invoker ) { - String title = template.title(args); - String content = template.content(args); + Objects.requireNonNull(template, "template must not be null"); + Objects.requireNonNull(invoker, "invoker must not be null"); + Objects.requireNonNull(targetUserId, "targetUserId must not be null"); + + String title = template.title(args); + String content = template.content(args); + if (title == null || title.isBlank() || content == null || content.isBlank()) { + throw new IllegalArgumentException("title/content must be non-blank"); + } // 1. DB 저장 saveNotification(title, content, targetUserId); // 2. 이벤트 퍼블리시 invoker.publish(title, content); }
15-29: 순서 보장 테스트 추가 제안 (save → publish)단위 테스트에서 Notification 저장 후 이벤트 퍼블리시 순서를
InOrder로 검증하면 회귀 방지에 유용합니다. 필요 시 초안 제공 가능합니다.
15-29: 중장기 신뢰성 강화: Outbox 패턴 고려현재는 AFTER_COMMIT 리스너에 의존합니다. 푸시 전송 신뢰성을 더 높이려면 트랜잭셔널 아웃박스(Outbox) + 폴링/스트리밍(예: Debezium)으로 재전송/재처리를 갖추는 방안을 백로그에 올려두면 좋겠습니다.
src/test/java/konkuk/thip/notification/application/service/RoomNotificationOrchestratorSyncImplUnitTest.java (1)
32-33: 가독성 소폭 개선: 한 줄 다중 선언 분리테스트 가독성을 위해 각 파라미터를 줄바꿈해 선언하면 유지보수에 유리합니다.
- Long roomId = 1L; int page = 2; Long postId = 3L; String postType = "RECORD"; + Long roomId = 1L; + int page = 2; + Long postId = 3L; + String postType = "RECORD";src/test/java/konkuk/thip/notification/application/service/FeedNotificationOrchestratorSyncImplUnitTest.java (1)
12-12: 일관성 있는 매처 사용: 정적 임포트로 FQN 제거 제안다른 테스트와 동일하게
any/eq를 정적 임포트로 사용하면 가독성이 좋아집니다.-import static org.mockito.Mockito.verify; +import static org.mockito.ArgumentMatchers.any; +import static org.mockito.ArgumentMatchers.eq; +import static org.mockito.Mockito.verify; @@ - verify(notificationSyncExecutor).execute( - org.mockito.ArgumentMatchers.any(), - org.mockito.ArgumentMatchers.any(), - org.mockito.ArgumentMatchers.eq(targetUserId), - invokerCaptor.capture() - ); + verify(notificationSyncExecutor).execute( + any(), + any(), + eq(targetUserId), + invokerCaptor.capture() + );Also applies to: 37-42
src/main/java/konkuk/thip/notification/application/service/FeedNotificationOrchestratorSyncImpl.java (1)
27-38: actorUsername 등 입력값 사전검증 추가 제안
- 템플릿이 문자열 결합을 가정하므로, null/blank 전달 시 예기치 않은 문구/예외 가능. 최소한
actorUsername에 대한 hasText 검증을 권장합니다. (다른 메서드 동일 적용)적용 예:
@Transactional(propagation = Propagation.MANDATORY) public void notifyFollowed(Long targetUserId, Long actorUserId, String actorUsername) { + org.springframework.util.Assert.hasText(actorUsername, "actorUsername must not be blank"); var args = new FollowedTemplate.Args(actorUsername); notificationSyncExecutor.execute( FollowedTemplate.INSTANCE, args, targetUserId, (title, content) -> feedEventCommandPort.publishFollowEvent( title, content, targetUserId, actorUserId, actorUsername ) ); }클래스 하단에 헬퍼 추가(참고):
private static void requireHasText(String value, String name) { org.springframework.util.Assert.hasText(value, name + " must not be blank"); }src/main/java/konkuk/thip/message/application/port/out/FeedEventCommandPort.java (1)
6-14: 긴 파라미터 리스트로 인한 오사용 리스크 — DTO/커맨드 객체 도입 고려
- 모든 메서드가
title, content+ 식별자 나열 형태라 호출부에서 인자 순서 실수 가능성이 있습니다. 애플리케이션 계층 전용 커맨드(예: record)로 캡슐화하면 안전합니다. 현재 변경은 문제가 없으므로 선택 사항입니다.예시:
public record FeedCommentedCommand(String title, String content, Long targetUserId, Long actorUserId, String actorUsername, Long feedId) {} void publishFeedCommented(FeedCommentedCommand cmd);Also applies to: 17-21, 23-27, 29-33, 35-39
src/main/java/konkuk/thip/notification/application/service/RoomNotificationOrchestratorSyncImpl.java (2)
28-39: postType을 String 대신 Enum으로 타이핑 고려
postType이 문자열이면 오타·케이스 문제로 데이터 품질 저하 가능. 도메인 Enum(PostType) 사용을 검토해 주세요. 외부 포트/이벤트와의 합의가 필요하므로 우선순위는 낮게 제안합니다.
15-24: 운영 관점 안내: 신뢰성 강화를 위한 Outbox/재시도 설계 고려
- 현재 설계는 DB 커밋 보장 + 커밋 후 이벤트 발행으로 손실 최소화는 충족. 다만 FCM 전송 실패 시 재시도/보류가 필요합니다. Outbox(폴링/CDC) + 백오프 재시도, DLQ 모니터링을 후속 작업으로 제안합니다.
src/test/java/konkuk/thip/notification/application/service/RoomNotificationOrchestratorSyncImplTest.java (1)
103-141: 다른 notify 케이스에 대한 파라미터 검증 추가 테스트 제안*
- 동일 패턴이므로 파라미터만 바꿔 파라미터라이즈드 테스트로 확장하면 회귀 방지에 유용합니다. 우선순위 낮음.
src/main/java/konkuk/thip/message/adapter/out/event/FeedEventPublisherAdapter.java (1)
15-26: 퍼블리시 로깅(디버그 레벨) 추가 제안
- 운영 추적성을 위해 이벤트 타입/타겟/액터 ID 중심의 debug 로그 한 줄 추가를 권장합니다. 성능 영향 미미.
예:
@@ public void publishFollowEvent( String title, String content, Long targetUserId, Long actorUserId, String actorUsername) { + // log.debug("Publish FollowerEvent target={}, actor={}", targetUserId, actorUserId); publisher.publishEvent(FeedEvents.FollowerEvent.builder()src/main/java/konkuk/thip/message/adapter/out/event/RoomEventPublisherAdapter.java (2)
94-97: 용어 정합성: host ↔ owner 혼용 최소화메서드명과 파라미터는 “Host”, DTO 필드는 “Owner”로 혼용됩니다. 오해 방지를 위해 네이밍을 통일하는 것을 권장합니다. 아래와 같이 메서드명/파라미터명만 정리해도 가독성이 좋아집니다. (인터페이스도 함께 변경 필요)
- public void publishRoomJoinEventToHost( - String title, String content, - Long hostUserId, Long roomId, String roomTitle, + public void publishRoomJoinRequestedToOwnerEvent( + String title, String content, + Long ownerUserId, Long roomId, String roomTitle, Long actorUserId, String actorUsername) { publisher.publishEvent(RoomEvents.RoomJoinRequestedToOwnerEvent.builder() - .title(title) - .content(content) - .ownerUserId(hostUserId) + .title(title) + .content(content) + .ownerUserId(ownerUserId) .roomId(roomId) .roomTitle(roomTitle) .applicantUserId(actorUserId) .applicantUsername(actorUsername) .build());Also applies to: 99-106
20-31: (선택) 디버그 로깅으로 관측성 보강문제 발생 시 역추적을 쉽게 하려면 발행 직전 debug 로그(핵심 식별자만)를 남기는 것을 권장합니다. 과도한 문자열 연산을 피하기 위해 로그 레벨 가드 또는 파라미터화된 로깅을 사용해 주세요.
src/main/java/konkuk/thip/message/application/port/out/RoomEventCommandPort.java (3)
46-49: 타입 엄격화(선택): postType → enum, page → 원시 타입 고정 검토
- postType(String) 오타 위험이 큽니다. 애플리케이션 레이어 enum(PostType)으로 교체를 고려해 주세요.
- page(Integer)는 null 허용 의도가 아니라면 int로 고정하는 편이 안전합니다. (null 허용이면 Javadoc에 명시)
Also applies to: 6-9, 40-43
34-37: 네이밍 통일성(선택): Host ↔ Owner어댑터/DTO가 owner 용어를 사용하므로 포트도
publishRoomJoinRequestedToOwnerEvent로 맞추면 검색성과 의도 전달이 좋아집니다. 어댑터 변경 패치와 동기화 필요합니다.- void publishRoomJoinEventToHost( + void publishRoomJoinRequestedToOwnerEvent( String title, String content, - Long hostUserId, Long roomId, String roomTitle, + Long ownerUserId, Long roomId, String roomTitle, Long actorUserId, String actorUsername);
6-9: 계약 강화: RoomEventCommandPort의 title/content에 @notblank 추가 및 구현체에 @validated 적용템플릿이 공백일 경우 푸시/DB 품질 저하 우려 — 인터페이스 파라미터에 제약을 추가하고 구현체에서 메서드 레벨 유효성검사를 활성화하세요.
대상: src/main/java/konkuk/thip/message/application/port/out/RoomEventCommandPort.java
구현체 확인/수정: src/main/java/konkuk/thip/message/adapter/out/event/RoomEventPublisherAdapter.java (빈으로 등록된 구현체에 @org.springframework.validation.annotation.Validated 추가)적용 예시(diff — 인터페이스의 각 메서드 첫 두 인자에 어노테이션 추가):
- void publishRoomPostCommentedEvent( - String title, String content, + void publishRoomPostCommentedEvent( + @jakarta.validation.constraints.NotBlank String title, + @jakarta.validation.constraints.NotBlank String content, Long targetUserId, Long actorUserId, String actorUsername, Long roomId, Integer page, Long postId, String postType); - void publishRoomVoteStartedEvent( - String title, String content, + void publishRoomVoteStartedEvent( + @jakarta.validation.constraints.NotBlank String title, + @jakarta.validation.constraints.NotBlank String content, Long targetUserId, Long roomId, String roomTitle, Integer page, Long postId); - void publishRoomRecordCreatedEvent( - String title, String content, + void publishRoomRecordCreatedEvent( + @jakarta.validation.constraints.NotBlank String title, + @jakarta.validation.constraints.NotBlank String content, Long targetUserId, Long actorUserId, String actorUsername, Long roomId, String roomTitle, Integer page, Long postId); - void publishRoomRecruitClosedEarlyEvent( - String title, String content, + void publishRoomRecruitClosedEarlyEvent( + @jakarta.validation.constraints.NotBlank String title, + @jakarta.validation.constraints.NotBlank String content, Long targetUserId, Long roomId, String roomTitle); - void publishRoomActivityStartedEvent( - String title, String content, + void publishRoomActivityStartedEvent( + @jakarta.validation.constraints.NotBlank String title, + @jakarta.validation.constraints.NotBlank String content, Long targetUserId, Long roomId, String roomTitle); - void publishRoomJoinEventToHost( - String title, String content, + void publishRoomJoinEventToHost( + @jakarta.validation.constraints.NotBlank String title, + @jakarta.validation.constraints.NotBlank String content, Long hostUserId, Long roomId, String roomTitle, Long actorUserId, String actorUsername); - void publishRoomCommentLikedEvent( - String title, String content, + void publishRoomCommentLikedEvent( + @jakarta.validation.constraints.NotBlank String title, + @jakarta.validation.constraints.NotBlank String content, Long targetUserId, Long actorUserId, String actorUsername, Long roomId, Integer page, Long postId, String postType); - void publishRoomPostLikedEvent( - String title, String content, + void publishRoomPostLikedEvent( + @jakarta.validation.constraints.NotBlank String title, + @jakarta.validation.constraints.NotBlank String content, Long targetUserId, Long actorUserId, String actorUsername, Long roomId, Integer page, Long postId, String postType); - void publishRoomPostCommentRepliedEvent( - String title, String content, + void publishRoomPostCommentRepliedEvent( + @jakarta.validation.constraints.NotBlank String title, + @jakarta.validation.constraints.NotBlank String content, Long targetUserId, Long actorUserId, String actorUsername, Long roomId, Integer page, Long postId, String postType);
📜 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 (12)
src/main/java/konkuk/thip/message/adapter/out/event/FeedEventPublisherAdapter.java(5 hunks)src/main/java/konkuk/thip/message/adapter/out/event/RoomEventPublisherAdapter.java(7 hunks)src/main/java/konkuk/thip/message/application/port/out/FeedEventCommandPort.java(1 hunks)src/main/java/konkuk/thip/message/application/port/out/RoomEventCommandPort.java(1 hunks)src/main/java/konkuk/thip/notification/application/service/EventCommandInvoker.java(1 hunks)src/main/java/konkuk/thip/notification/application/service/FeedNotificationOrchestratorSyncImpl.java(1 hunks)src/main/java/konkuk/thip/notification/application/service/NotificationSyncExecutor.java(1 hunks)src/main/java/konkuk/thip/notification/application/service/RoomNotificationOrchestratorSyncImpl.java(1 hunks)src/test/java/konkuk/thip/notification/application/service/FeedNotificationOrchestratorSyncImplTest.java(1 hunks)src/test/java/konkuk/thip/notification/application/service/FeedNotificationOrchestratorSyncImplUnitTest.java(1 hunks)src/test/java/konkuk/thip/notification/application/service/RoomNotificationOrchestratorSyncImplTest.java(1 hunks)src/test/java/konkuk/thip/notification/application/service/RoomNotificationOrchestratorSyncImplUnitTest.java(1 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#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#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#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#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 (8)
src/test/java/konkuk/thip/notification/application/service/FeedNotificationOrchestratorSyncImplTest.java (3)
src/test/java/konkuk/thip/common/util/TestEntityFactory.java (1)
TestEntityFactory(32-370)src/main/java/konkuk/thip/message/adapter/out/event/dto/FeedEvents.java (1)
FeedEvents(6-48)src/test/java/konkuk/thip/notification/application/service/RoomNotificationOrchestratorSyncImplTest.java (1)
SpringBootTest(31-167)
src/main/java/konkuk/thip/message/adapter/out/event/FeedEventPublisherAdapter.java (1)
src/main/java/konkuk/thip/message/adapter/out/event/dto/FeedEvents.java (1)
FeedEvents(6-48)
src/main/java/konkuk/thip/notification/application/service/RoomNotificationOrchestratorSyncImpl.java (1)
src/main/java/konkuk/thip/notification/application/service/FeedNotificationOrchestratorSyncImpl.java (1)
HelperService(11-109)
src/main/java/konkuk/thip/message/adapter/out/event/RoomEventPublisherAdapter.java (1)
src/main/java/konkuk/thip/message/adapter/out/event/dto/RoomEvents.java (1)
RoomEvents(6-69)
src/main/java/konkuk/thip/notification/application/service/FeedNotificationOrchestratorSyncImpl.java (1)
src/main/java/konkuk/thip/notification/application/service/RoomNotificationOrchestratorSyncImpl.java (1)
HelperService(11-156)
src/test/java/konkuk/thip/notification/application/service/RoomNotificationOrchestratorSyncImplUnitTest.java (1)
src/test/java/konkuk/thip/notification/application/service/FeedNotificationOrchestratorSyncImplUnitTest.java (1)
ExtendWith(14-51)
src/test/java/konkuk/thip/notification/application/service/RoomNotificationOrchestratorSyncImplTest.java (3)
src/test/java/konkuk/thip/common/util/TestEntityFactory.java (1)
TestEntityFactory(32-370)src/main/java/konkuk/thip/message/adapter/out/event/dto/RoomEvents.java (1)
RoomEvents(6-69)src/test/java/konkuk/thip/notification/application/service/FeedNotificationOrchestratorSyncImplTest.java (1)
SpringBootTest(31-139)
src/test/java/konkuk/thip/notification/application/service/FeedNotificationOrchestratorSyncImplUnitTest.java (1)
src/test/java/konkuk/thip/notification/application/service/RoomNotificationOrchestratorSyncImplUnitTest.java (1)
ExtendWith(16-50)
⏰ 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 (16)
src/main/java/konkuk/thip/notification/application/service/EventCommandInvoker.java (1)
3-7: 인터페이스 명확성은 좋습니다.단일 책임을 가진 콜백으로 깔끔합니다.
src/main/java/konkuk/thip/notification/application/service/NotificationSyncExecutor.java (1)
31-34: 트랜잭션 정책은 상위에서 관리: 현재 형태 유지 확인이 클래스가 트랜잭션을 시작하지 않고(어노테이션 없음) 상위 MANDATORY 트랜잭션에 의존하는 설계는 PR 의도와 부합합니다. 그대로 유지해 주세요.
상위 오케스트레이터의 메서드가
@Transactional(propagation = MANDATORY)혹은 동등 정책인지 재확인 부탁드립니다.src/test/java/konkuk/thip/notification/application/service/RoomNotificationOrchestratorSyncImplUnitTest.java (1)
16-49: 테스트 구성 적절: invoker 캡처로 포트 위임 검증 OK핵심 상호작용(Executor 호출, invoker → Port 위임) 검증이 명확합니다.
src/test/java/konkuk/thip/notification/application/service/FeedNotificationOrchestratorSyncImplUnitTest.java (1)
35-49: 테스트 구성 적절: Executor 호출 인자·포트 위임 검증 OK의도한 시나리오를 간결하게 커버합니다.
src/test/java/konkuk/thip/notification/application/service/FeedNotificationOrchestratorSyncImplTest.java (4)
58-67: MANDATORY 정책 검증 테스트 좋습니다트랜잭션 미존재 시 예외를 명시적으로 보장합니다.
69-86: 동일 트랜잭션 내 즉시 조회 검증 적절커밋 전 영속성 컨텍스트 내 저장 확인이 명확합니다.
88-117: AFTER_COMMIT 동작 검증이 탄탄합니다커밋 트리거와 리스너 파라미터 단언이 정확합니다.
119-138: 롤백 시 미전송/미저장 보장 테스트 훌륭함부작용 방지 측면의 회귀 안전망으로 충분합니다.
src/main/java/konkuk/thip/notification/application/service/FeedNotificationOrchestratorSyncImpl.java (1)
25-38: 정책/흐름 구현 일관 — title/content 전달 OK
- Propagation.MANDATORY로 상위 트랜잭션 강제, 템플릿→DB 저장→이벤트 발행(타이틀/컨텐츠 포함) 흐름이 6개 메서드 모두 동일하게 잘 맞춰졌습니다.
Also applies to: 40-52, 54-66, 68-80, 82-94, 96-108
src/main/java/konkuk/thip/message/application/port/out/FeedEventCommandPort.java (1)
6-14: 확인 완료 — 레거시(구) 시그니처 호출 없음전수 스캔 결과 publish*Event 호출들이 모두 변경된 시그니처(예: feedId/roomId 등 포함)로 호출되어 잔존 호출로 인한 컴파일 에러 가능성 없음.
src/test/java/konkuk/thip/notification/application/service/RoomNotificationOrchestratorSyncImplTest.java (1)
31-41: 통합 테스트 커버리지 적절 — 트랜잭션 경계/AFTER_COMMIT/롤백 모두 검증
- MANDATORY 예외, 커밋 시 이벤트 디스패치, 롤백 시 미발행까지 핵심 시나리오가 잘 커버됩니다. 가독성도 좋습니다.
Also applies to: 76-101, 103-141, 142-167
src/main/java/konkuk/thip/message/adapter/out/event/FeedEventPublisherAdapter.java (1)
15-26: 이벤트 DTO 매핑 정확 — title/content 포함 빌더 사용 OK
- 모든 경로에서
.title(title),.content(content)가 누락 없이 설정됩니다. 나머지 필드 매핑도 이벤트 스키마와 일치합니다.Also applies to: 28-41, 43-56, 58-71, 73-86, 88-101
src/main/java/konkuk/thip/message/adapter/out/event/RoomEventPublisherAdapter.java (2)
16-19: 타이틀/컨텐츠 추가 매핑 일관성 확인 (LGTM)포트/DTO 시그니처와 빌더 매핑이 정확히 일치합니다. 누락/순서 오류 없습니다.
Also applies to: 21-23, 34-37, 39-41, 50-53, 55-57, 68-70, 72-74, 81-83, 85-87, 94-97, 99-101, 110-113, 115-117, 128-131, 133-135, 146-148, 150-152
13-14: 확인 완료 — AFTER_COMMIT 리스너로 발행 시점 보장됨RoomNotificationOrchestratorSyncImpl은 @transactional(propagation = Propagation.MANDATORY) 내에서 roomEventCommandPort를 호출하고, RoomEventPublisherAdapter는 ApplicationEventPublisher로 이벤트를 발행하며 수신측인 MessageRoomEventListener(및 MessageFeedEventListener)가 @TransactionalEventListener(phase = TransactionPhase.AFTER_COMMIT)로 등록되어 있어 이벤트 처리는 트랜잭션 커밋 시점에 실행됩니다. 참조: src/main/java/konkuk/thip/notification/application/service/RoomNotificationOrchestratorSyncImpl.java, src/main/java/konkuk/thip/message/adapter/out/event/RoomEventPublisherAdapter.java, src/main/java/konkuk/thip/message/adapter/in/event/MessageRoomEventListener.java, src/main/java/konkuk/thip/message/adapter/in/event/MessageFeedEventListener.java
src/main/java/konkuk/thip/message/application/port/out/RoomEventCommandPort.java (2)
6-9: 시그니처 변경 방향성 적합 (LGTM)알림 템플릿에서 생성된 title/content를 포트의 선두 인자로 고정한 결정은 호출부 일관성과 가독성 측면에서 타당합니다.
Also applies to: 12-15, 18-21, 24-26, 29-31, 34-37, 40-43, 46-49, 52-55
5-56: 호출부 시그니처 업데이트 누락 여부 재검증 요청이전 rg 패턴이 PCRE 오류로 실패했습니다. 아래 수정된 명령으로 잔존 호출을 찾아 출력 결과를 붙여 주세요.
#!/bin/bash # PCRE(-P) 미사용 — 각 publishRoom...Event 호출 위치 출력(컨텍스트 2줄) rg -n -C2 '\bpublishRoomPostCommentedEvent\s*\(' src/main/java || true rg -n -C2 '\bpublishRoomVoteStartedEvent\s*\(' src/main/java || true rg -n -C2 '\bpublishRoomRecordCreatedEvent\s*\(' src/main/java || true rg -n -C2 '\bpublishRoomRecruitClosedEarlyEvent\s*\(' src/main/java || true rg -n -C2 '\bpublishRoomActivityStartedEvent\s*\(' src/main/java || true rg -n -C2 '\bpublishRoomJoinEventToHost\s*\(' src/main/java || true rg -n -C2 '\bpublishRoomCommentLikedEvent\s*\(' src/main/java || true rg -n -C2 '\bpublishRoomPostLikedEvent\s*\(' src/main/java || true rg -n -C2 '\bpublishRoomPostCommentRepliedEvent\s*\(' src/main/java || true출력된 호출 중 '(' 다음 토큰이 문자열 리터럴(예: "title","content")로 시작하지 않는 라인이 있으면 시그니처 미반영입니다 — 해당 라인들을 붙여 올려 주세요.
- '/swagger-ui/**' 와 같은 Ant 스타일 패턴의 화이트리스트 경로를 정확히 처리하기 위해 기존 startsWith 방식 대신 PathPattern 기반 비교 방식으로 변경 - AntPathMatcher 는 deprecated 기능이므로 JwtAuthenticationFilter 내에서 PathPatternParser 를 활용해 직접 매칭 로직 구현
hd0rable
left a comment
hd0rable
left a comment
There was a problem hiding this comment.
너무너무 수고하셨습니다..🥹🥹💯 성준띠니 테코는 진짜 볼때마다 감탄이네여 굿굿
|
|
||
| @RequiredArgsConstructor | ||
| @Getter | ||
| public enum SecurityWhitelist { |
| @RequiredArgsConstructor | ||
| public class FeedNotificationOrchestratorSyncImpl implements FeedNotificationOrchestrator { | ||
|
|
||
| /** |
| private <T> void notifyWithTemplate( | ||
| NotificationTemplate<T> template, | ||
| T args, | ||
| Long targetUserId, | ||
| Runnable eventPublisher | ||
| ) { | ||
| String title = template.title(args); | ||
| String content = template.content(args); | ||
| saveNotification(title, content, targetUserId); | ||
| eventPublisher.run(); |
There was a problem hiding this comment.
이벤트 리스너의 실행 시점이 after commit 이므로 이벤트 퍼블리시 시점에서는 단순히 콜백 등록만을 수행하니 문제없지 않나? 라는 생각으로 구현을 했긴 합니다.
성준님이 언급해주신것처럼 이벤트 처리과정에서 에러가 발생했을때 트랜잭션이 롤백되는경우는 거의 없을것같긴하지만 만약 문제가 생겼을시 현준님이 언급하신것처럼 try-catch로 이벤트 퍼블리셔 예외를 감싸고 로깅을 추가하면 운영상 버그나 모니터링에도 좋을 것 같습니다!!
| followingJpaRepository.deleteAllInBatch(); | ||
| userJpaRepository.deleteAll(); | ||
| notificationJpaRepository.deleteAllInBatch(); | ||
| userJpaRepository.deleteAllInBatch(); |
|
|
||
| // 롤백 트리거 | ||
| TestTransaction.flagForRollback(); | ||
| TestTransaction.end(); // 커밋이 아니므로 AFTER_COMMIT 리스너는 실행되지 않음 |
There was a problem hiding this comment.
좋네요 트랜잭션 내부 시점마다 꼼꼼히 테스트하신거 뤼스펙
buzz0331
left a comment
buzz0331
left a comment
There was a problem hiding this comment.
코드 수정 확인했습니다~ 아직 이벤트 발행시 try-catch 적용되지 않은 것 같아서 이 부분은 수정하시면 다시 확인해보겠습니다!!
| @PostConstruct | ||
| void initWhitelistPatterns() { | ||
| this.whitelistPatterns = SecurityWhitelist.patternsList().stream() | ||
| .map(pathPatternParser::parse) // 애플리케이션 시작 시 1회 컴파일 | ||
| .toList(); | ||
| } | ||
|
|
||
| @Override | ||
| protected boolean shouldNotFilter(HttpServletRequest request) { | ||
| // 컨텍스트 패스 고려한 실제 경로(= path) 추출 | ||
| String requestUri = request.getRequestURI(); | ||
| String contextPath = request.getContextPath(); | ||
| String path = (contextPath != null && !contextPath.isEmpty() && requestUri.startsWith(contextPath)) | ||
| ? requestUri.substring(contextPath.length()) | ||
| : requestUri; | ||
|
|
||
| PathContainer container = PathContainer.parsePath(path); | ||
|
|
||
| // PathPattern으로 세그먼트 기반 매칭 | ||
| return whitelistPatterns.stream().anyMatch(p -> p.matches(container)); | ||
| } |
There was a problem hiding this comment.
오호 OOME를 피하고자 처음 시작시에만 정적으로 리스트를 만들어둔 건가요??
There was a problem hiding this comment.
스프링 서버가 처음 뜰 때 한번만(= @PostConstruct) enum 으로 관리하는 whitelist 들을 PathPattern 객체로 변환하여 shouldNotFilter 메서드에서 계속 사용하는 것을 의도한 코드이긴 합니다!!
초기화 로직으로 생각해주시면 될 것 같습니다!!
There was a problem hiding this comment.
네넵 좋습니다! Pattern 객체를 매 패턴 매치시에 사용하면 OOME가 발생할 수도 있는 문제는 것으로 알아서 성준님 코드처럼 미리 컴파일 시점에 객체를 만들어두고 사용하는 것이 안정성이 더 높은 것으로 알고 있습니다.
참고 블로그입니다! https://blogshine.tistory.com/687
There was a problem hiding this comment.
오호 좋은 레퍼런스 공유 감사합니닷
| public void publishFollowEvent( | ||
| String title, String content, | ||
| Long targetUserId, Long actorUserId, String actorUsername) { | ||
| publisher.publishEvent(FeedEvents.FollowerEvent.builder() | ||
| .title(title) | ||
| .content(content) | ||
| .targetUserId(targetUserId) | ||
| .actorUserId(actorUserId) | ||
| .actorUsername(actorUsername) | ||
| .build()); | ||
| } |
| notificationSyncExecutor.execute( | ||
| FollowedTemplate.INSTANCE, | ||
| args, | ||
| targetUserId, | ||
| () -> feedEventCommandPort.publishFollowEvent(targetUserId, actorUserId, actorUsername) | ||
| (title, content) -> feedEventCommandPort.publishFollowEvent( | ||
| title, content, targetUserId, actorUserId, actorUsername | ||
| ) |
There was a problem hiding this comment.
title, content가 뒤쪽에서 발생해서 함수형 인터페이스로 notificationSyncExecutor 객체에게 책임을 미루셨군요! 좋습니다!!
- 푸시 알림 이벤트 퍼블리시 과정에서 예외가 발생할 경우, 이를 외부로 던지지 않음을 단위 테스트 코드로 검증
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (8)
src/main/java/konkuk/thip/notification/application/service/NotificationSyncExecutor.java (5)
17-22: 트랜잭션 존재 보증 가드 추가 제안 (IllegalTransactionStateException 명시적으로 던지기)PR 설명대로 “상위 트랜잭션 없이 호출되면 IllegalTransactionStateException”이 보장되어야 합니다. Orchestrator에서 이미 강제하고 있더라도, Executor에도 방어적으로 가드를 두면 오용을 조기에 차단할 수 있습니다.
아래와 같이 가드를 추가하는 것을 제안드립니다(오케스트레이터에서 이미 처리한다면 스킵 가능).
@@ public <T> void execute( NotificationTemplate<T> template, T args, Long targetUserId, EventCommandInvoker invoker ) { + if (!org.springframework.transaction.support.TransactionSynchronizationManager.isActualTransactionActive()) { + throw new org.springframework.transaction.IllegalTransactionStateException( + "NotificationSyncExecutor.execute()는 활성화된 트랜잭션 안에서만 호출되어야 합니다."); + }
29-37: 퍼블리시를 afterCommit으로 이동해 커밋 시점 보장현재 publish가 즉시 호출됩니다. “커밋 시 DB 저장 + 커밋 후 퍼블리시” 목표에 정확히 부합시키려면 afterCommit 훅에서 퍼블리시하는 편이 안전합니다(롤백 시 퍼블리시 방지). Orchestrator에서 이미 afterCommit 등록을 하고 있다면 그대로 두셔도 되나, Executor 레벨에서 보장할 수도 있습니다.
아래처럼 교체 가능:
@@ - // 2. 이벤트 퍼블리시 - try { - invoker.publish(title, content); - } catch (Exception e) { - // 이벤트 발행 실패 시, DB에 저장된 알림을 롤백하지는 않음 - // -> 알림 저장은 비즈니스 트랜잭션과 동일한 경계 내에서 수행되므로, 알림 저장은 유지 - // -> 푸시 알림 이벤트 발행이 실패한 경우, 일단 로깅만 추가 - log.error("푸시 알림 이벤트 퍼블리시 실패 targetUserId = {}, title = {}", targetUserId, title, e); - } + // 2. 커밋 후(afterCommit) 퍼블리시 + org.springframework.transaction.support.TransactionSynchronizationManager.registerSynchronization( + new org.springframework.transaction.support.TransactionSynchronization() { + @Override + public void afterCommit() { + try { + invoker.publish(title, content); + } catch (Exception e) { + log.error("푸시 알림 이벤트 퍼블리시 실패 targetUserId = {}, title = {}", targetUserId, title, e); + } + } + } + );또한 import 정리는 다음과 같이 추가됩니다:
+import org.springframework.transaction.IllegalTransactionStateException; +import org.springframework.transaction.support.TransactionSynchronization; +import org.springframework.transaction.support.TransactionSynchronizationManager;
23-27: title/content 유효성 체크(널/길이)로 DB 제약 위반 사전 방지템플릿이 잘못 구현되면 null/과도한 길이가 들어갈 수 있습니다. 도메인/스키마 제약(예: VARCHAR 길이)에 맞춰 사전 검증/절단을 권장합니다.
예시:
@@ - String title = template.title(args); - String content = template.content(args); + java.util.Objects.requireNonNull(template, "template must not be null"); + java.util.Objects.requireNonNull(targetUserId, "targetUserId must not be null"); + String title = java.util.Objects.requireNonNull(template.title(args), "title must not be null"); + String content = java.util.Objects.requireNonNull(template.content(args), "content must not be null"); + // 필요 시 길이 제한 적용 (예: 100, 500 등 스키마에 맞춰 조정) + // title = StringUtils.truncate(title, 100); + // content = StringUtils.truncate(content, 1000);스키마 최대 길이를 알려주시면 정확한 가드 코드로 제안 드리겠습니다.
29-37: 재시도/관측성(메트릭) 보강 제안퍼블리시 실패를 단순 로깅에 그치지 말고, 실패 카운터(metric)와 알림(DLQ 또는 재시도 큐)로 연결하면 운영 안정성이 크게 향상됩니다. 실패 이벤트를 재처리할 outbox/재시도 전략은 이후 단계에서도 무리 없이 추가 가능합니다.
간단한 시작점:
- 퍼블리시 실패 카운터: meterRegistry.counter("notification.publish.fail").increment()
- 실패 상세 로그에 targetUserId, template 식별자, 해시 등을 포함해 트러블슈팅 용이성 확보
- 장기적으로는 “알림 outbox 테이블 + 비동기 퍼블리셔” 또는 “메시지 브로커 DLQ” 고려
40-43: 저장 결과(식별자) 활용성 개선로그/추적을 위해 notificationId가 있으면 유용합니다. 가능하다면 Port의 save가 저장된 엔티티(ID 포함) 또는 ID를 반환하도록 바꾸고, 실패 로그에도 포함하세요.
예시:
// Port Notification save(Notification notification); // Executor Notification saved = notificationCommandPort.save(notification); log.error("... notificationId = {}", saved.getId(), e);src/test/java/konkuk/thip/notification/application/service/NotificationSyncExecutorTest.java (3)
32-41: 퍼블리시 시도 여부까지 검증하도록 Mockito로 invoker 교체현재 람다로 예외를 던져 퍼블리시 경로는 타지만 호출 횟수 검증은 어렵습니다. Mockito mock을 사용해 호출 여부와(선택) 순서를 검증하면 회귀 방지에 유익합니다.
아래처럼 수정 가능:
- // publish 호출 시 강제로 예외를 던지는 invoker - EventCommandInvoker invoker = (title, content) -> { - throw new RuntimeException("강제 퍼블리시 실패"); - }; + // publish 호출 시 강제로 예외를 던지는 invoker(mock) + EventCommandInvoker invoker = mock(EventCommandInvoker.class); + doThrow(new RuntimeException("강제 퍼블리시 실패")) + .when(invoker).publish(anyString(), anyString()); @@ - assertThatCode(() -> - executor.execute(template, "dummyArgs", 123L, invoker) - ).doesNotThrowAnyException(); + assertThatCode(() -> executor.execute(template, "dummyArgs", 123L, invoker)) + .doesNotThrowAnyException(); + + // 퍼블리시 시도 1회 검증 + verify(invoker, times(1)).publish("테스트제목", "테스트내용");선택:
InOrder로save→publish순서까지 검증 가능합니다.
42-51: 성공 경로 및 경계 케이스 테스트 추가 제안
- 성공 케이스: publish 성공 시에도 save 1회, publish 1회를 검증.
- 유효성 실패(옵션): 템플릿이 null/빈 문자열을 반환하는 경우의 방어 로직이 추가된다면 해당 케이스도 테스트.
추가 예시:
@Test @DisplayName("execute(): 퍼블리시 성공 시 save/publish 모두 1회 호출") void execute_success_calls_save_and_publish_once() { NotificationCommandPort commandPort = mock(NotificationCommandPort.class); NotificationSyncExecutor executor = new NotificationSyncExecutor(commandPort); NotificationTemplate<String> template = new NotificationTemplate<>() { public String title(String a) { return "T"; } public String content(String a) { return "C"; } }; EventCommandInvoker invoker = mock(EventCommandInvoker.class); assertThatCode(() -> executor.execute(template, "args", 1L, invoker)) .doesNotThrowAnyException(); verify(commandPort, times(1)).save(any(Notification.class)); verify(invoker, times(1)).publish("T", "C"); }
17-51: (선택) 트랜잭션 가드가 추가될 경우의 테스트 보완Executor에 트랜잭션 가드를 도입한다면(위 제안) “트랜잭션 없이 호출 시 IllegalTransactionStateException”을 검증하는 테스트를 별도로 추가해 주세요(통합 테스트에서 실제 트랜잭션 경계 하에 호출).
📜 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 (2)
src/main/java/konkuk/thip/notification/application/service/NotificationSyncExecutor.java(1 hunks)src/test/java/konkuk/thip/notification/application/service/NotificationSyncExecutorTest.java(1 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#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#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#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 단일 커서 방식을 선호하며, 복합 키 기반 커서보다 구현 단순성과 성능을 우선시한다.
🔇 Additional comments (1)
src/test/java/konkuk/thip/notification/application/service/NotificationSyncExecutorTest.java (1)
17-41: 퍼블리시 실패 시 예외 전파 억제 테스트 — 좋습니다예외를 삼키고(로그만) 저장이 유지되는 경로를 명확히 검증하고 있습니다. 캡처로 저장된 엔티티 필드도 확인한 점 좋습니다.
| @Test | ||
| @DisplayName("execute() 메서드 : 푸시 알림 이벤트 퍼블리시 과정에서 예외가 발생하더라도 예외를 외부로 던지지 않는다.") | ||
| void execute_publish_failure_does_not_throw() { | ||
| // given | ||
| NotificationCommandPort commandPort = mock(NotificationCommandPort.class); | ||
| NotificationSyncExecutor executor = new NotificationSyncExecutor(commandPort); | ||
|
|
||
| // 간단한 템플릿 스텁 (title/content 고정) | ||
| NotificationTemplate<String> template = new NotificationTemplate<>() { | ||
| @Override | ||
| public String title(String args) { return "테스트제목"; } | ||
| @Override | ||
| public String content(String args) { return "테스트내용"; } | ||
| }; | ||
|
|
||
| // publish 호출 시 강제로 예외를 던지는 invoker | ||
| EventCommandInvoker invoker = (title, content) -> { | ||
| throw new RuntimeException("강제 퍼블리시 실패"); | ||
| }; | ||
|
|
||
| // when & then | ||
| assertThatCode(() -> | ||
| executor.execute(template, "dummyArgs", 123L, invoker) | ||
| ).doesNotThrowAnyException(); | ||
|
|
||
| // NotificationCommandPort은 정상적으로 호출되었는지 검증 | ||
| ArgumentCaptor<Notification> captor = ArgumentCaptor.forClass(Notification.class); | ||
| verify(commandPort, times(1)).save(captor.capture()); | ||
|
|
||
| Notification saved = captor.getValue(); | ||
| // 템플릿에서 설정한 title/content 값이 그대로 들어갔는지 확인 | ||
| assertThat(saved.getTitle()).isEqualTo("테스트제목"); | ||
| assertThat(saved.getContent()).isEqualTo("테스트내용"); | ||
| assertThat(saved.getTargetUserId()).isEqualTo(123L); | ||
| } |
3 participants
#️⃣ 연관된 이슈
📝 작업 내용
푸시알림에 해당하는 로직 실행시, [알림센터 저장 -> 푸시 알림을 위한 이벤트 퍼블리시] 하는 기능을 추가하였습니다.
주요 코드 설명
DB 알림 센터 저장 또한 서비스 메서드의 트랜잭션에 포함시키지 않고 비동기적으로 수행할까 고민하다가,
라고 생각해서 위 코드 흐름처럼 구현하였습니다
추후에 성능 개선 목적을 위하여 이벤트 드리븐 아키텍처를 도입할 때, 알림센터 저장 부분도 같이 고려해보면 좋을 것 같습니다!!
(재시도 로직을 어떻게 할지를 고민하는게 중요해보입니닷)
DB 알림센터 저장 기능이 도입되면서 기존 fcm 푸시알림 코드의 일부분이 수정되었습니다
📸 스크린샷
알림 발생 로직 실행시, DB notification 에 데이터가 insert 되는 것을 확인하였습니다
💬 리뷰 요구사항
📌 PR 진행 시 이러한 점들을 참고해 주세요
Summary by CodeRabbit
New Features
Refactor
Tests