SCRUM-125 알림 템플릿 구현#9
SCRUM-125 알림 템플릿 구현#9unifolio0 merged 2 commits intoTeam-soft-macaron:developfrom unifolio0:SCRUM-125
Conversation
|
Caution Review failedThe pull request is closed. Note Other AI code review bot(s) detectedCodeRabbit has detected other AI code review bot(s) in this pull request and will avoid duplicating their findings in the review comments. This may lead to a less comprehensive review. Walkthrough알림 템플릿 체계를 도입해 다섯 종의 예약 템플릿과 팩토리를 추가하고, WebPush 전송을 템플릿 기반으로 전환했습니다. TEMPLATE_NOT_FOUND 에러 코드가 추가되었고, 테스트 푸시 엔드포인트/서비스 및 관련 테스트가 제거되었습니다. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
participant Client as Client
participant Controller as WebPushController
participant Service as WebPushService
participant Sender as WebPushSender
participant Factory as NotificationTemplateFactory
participant Template as NotificationTemplate(impl)
participant Push as Web Push Provider
Client->>Controller: 알림 전송 요청 (type, payload)
Controller->>Service: 전달 (type, payload)
Service->>Sender: send(type, payload, subscriptions)
Sender->>Factory: createTemplateData(type, payload)
Factory->>Template: supports(type) 검사 및 선택
Template-->>Factory: NotificationTemplateData 반환
Factory-->>Sender: TemplateData(title,body,url,requireInteraction)
Sender->>Push: 템플릿 기반 페이로드 전송
Push-->>Client: 푸시 표시
rect rgba(255,230,230,0.35)
note right of Factory: 템플릿 미발견 시
Factory-->>Sender: 예외 (TEMPLATE_NOT_FOUND)
Sender-->>Service: 에러 전파
end
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
Suggested reviewers
Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
📜 Recent review detailsConfiguration used: CodeRabbit UI Review profile: CHILL Plan: Pro 📒 Files selected for processing (6)
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 |
Summary of ChangesHello @unifolio0, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed! 이 PR은 다양한 알림 메시지 생성을 표준화하고 간소화하기 위한 알림 템플릿 시스템을 구현합니다. 이 시스템은 특정 알림 유형과 페이로드에 기반하여 알림 제목, 본문, URL을 동적으로 생성하여 유지보수성과 일관성을 향상시킵니다. 또한, 웹 푸시 전송 메커니즘을 새로운 템플릿 접근 방식과 통합하도록 리팩토링하고, 기존의 테스트 푸시 전용 엔드포인트를 제거합니다. Highlights
Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for Github and other Google products, sign up here. You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension. Footnotes
|
|
|
||
| public NotificationTemplateData createTemplateData(NotificationType type, Map<String, Object> payload) { | ||
| NotificationTemplate template = templates.stream() | ||
| .filter(low -> low.supports(type)) |
| NotificationTemplate template = templates.stream() | ||
| .filter(low -> low.supports(type)) | ||
| .findFirst() | ||
| .orElseThrow(() -> new WellMeetNotificationException(ErrorCode.TEMPLATE_NOT_FOUND)); |
There was a problem hiding this comment.
| String restaurantName = getStringOrDefault(payload, "restaurantName", "식당"); | ||
| String reservationTime = getStringOrDefault(payload, "reservationTime", ""); | ||
| String reservationId = getStringOrDefault(payload, "reservationId", ""); |
There was a problem hiding this comment.
페이로드에서 값을 가져오기 위해 사용되는 키 문자열들("restaurantName", "reservationTime", "reservationId")이 하드코딩되어 있습니다. 이러한 '매직 스트링'은 오타에 취약하고 유지보수를 어렵게 만듭니다. 이 키들을 별도의 상수 클래스(e.g., NotificationPayloadKeys)에 public static final String 상수로 정의하여 사용하는 것을 권장합니다. 이렇게 하면 타입 안정성을 높이고 코드의 일관성을 유지할 수 있습니다. 이 제안은 다른 템플릿 구현체에도 동일하게 적용됩니다.
| private String getStringOrDefault(Map<String, Object> payload, String key, String defaultValue) { | ||
| Object value = payload.get(key); | ||
| return value != null ? value.toString() : defaultValue; | ||
| } |
There was a problem hiding this comment.
이 getStringOrDefault 헬퍼 메서드는 ReservationConfirmedTemplate, ReservationCreatedTemplate, ReservationReminderTemplate, ReservationUpdatedTemplate 등 여러 템플릿 클래스에 중복으로 구현되어 있습니다. 코드 중복을 피하고 유지보수성을 높이기 위해 이 메서드를 모든 템플릿 클래스가 상속할 수 있는 추상 기본 클래스(e.g., AbstractNotificationTemplate)로 옮기는 것을 고려해 보세요.
또한, 메서드 구현을 java.util.Objects.toString(payload.get(key), defaultValue)를 사용하여 더 간결하게 만들 수 있습니다.
private String getStringOrDefault(Map<String, Object> payload, String key, String defaultValue) {
return java.util.Objects.toString(payload.get(key), defaultValue);
}| Map<String, Object> notificationPayload = new HashMap<>(); | ||
| notificationPayload.put("title", request.title()); | ||
| notificationPayload.put("body", request.body()); | ||
| notificationPayload.put("title", templateData.title()); | ||
| notificationPayload.put("body", templateData.body()); | ||
| notificationPayload.put("icon", "/icon-192x192.png"); | ||
| notificationPayload.put("badge", "/badge-72x72.png"); | ||
| notificationPayload.put("vibrate", new int[]{100, 50, 100}); | ||
| notificationPayload.put("requireInteraction", false); | ||
| notificationPayload.put("requireInteraction", templateData.requireInteraction()); | ||
|
|
||
| Map<String, Object> defaultData = new HashMap<>(); | ||
| defaultData.put("url", "/notifications"); | ||
| defaultData.put("url", templateData.url()); | ||
| defaultData.put("timestamp", System.currentTimeMillis()); | ||
| notificationPayload.put("data", defaultData); | ||
| return notificationPayload; |
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (3)
src/main/java/com/wellmeet/notification/template/impl/ReservationCreatedTemplate.java (1)
31-34: 중복 코드 (이미 언급됨)이 헬퍼 메서드의 중복에 대해서는
ReservationUpdatedTemplate.java에서 이미 리팩토링을 제안했습니다.src/main/java/com/wellmeet/notification/template/impl/ReservationConfirmedTemplate.java (1)
31-34: 중복 코드 (이미 언급됨)이 헬퍼 메서드의 중복에 대해서는
ReservationUpdatedTemplate.java에서 이미 리팩토링을 제안했습니다.src/main/java/com/wellmeet/notification/template/impl/ReservationCanceledTemplate.java (1)
31-34: 헬퍼 메서드 중복을 제거하는 것을 고려하세요.이 메서드는
ReservationReminderTemplate을 포함한 모든 템플릿에서 중복됩니다.ReservationReminderTemplate.java의 리뷰 코멘트를 참고하여 공통 유틸리티 클래스로 추출하는 것을 권장합니다.
🧹 Nitpick comments (3)
src/main/java/com/wellmeet/notification/template/impl/ReservationUpdatedTemplate.java (1)
31-34: 중복 코드 제거를 고려해 보세요.
getStringOrDefault헬퍼 메서드가 모든 템플릿 구현에서 동일하게 반복됩니다. 이 메서드를 추상 베이스 클래스나 유틸리티 클래스로 추출하여 코드 중복을 줄이는 것을 권장합니다.예시: 공통 유틸리티 클래스 생성
package com.wellmeet.notification.template; public class TemplateUtils { public static String getStringOrDefault(Map<String, Object> payload, String key, String defaultValue) { Object value = payload.get(key); return value != null ? value.toString() : defaultValue; } }그런 다음 각 템플릿에서:
- private String getStringOrDefault(Map<String, Object> payload, String key, String defaultValue) { - Object value = payload.get(key); - return value != null ? value.toString() : defaultValue; - } + // 삭제하고 TemplateUtils.getStringOrDefault 사용src/main/java/com/wellmeet/notification/template/NotificationTemplateFactory.java (1)
17-24: 팩토리 로직이 올바르지만 변수명 개선을 고려해 보세요.템플릿 선택 로직이 정확하게 구현되었습니다. 다만 Line 19의 람다 파라미터명
low는 오타로 보입니다.template또는t와 같이 더 명확한 이름을 사용하는 것을 권장합니다.- NotificationTemplate template = templates.stream() - .filter(low -> low.supports(type)) - .findFirst() - .orElseThrow(() -> new WellMeetNotificationException(ErrorCode.TEMPLATE_NOT_FOUND)); + NotificationTemplate template = templates.stream() + .filter(t -> t.supports(type)) + .findFirst() + .orElseThrow(() -> new WellMeetNotificationException(ErrorCode.TEMPLATE_NOT_FOUND));src/main/java/com/wellmeet/notification/template/impl/ReservationReminderTemplate.java (1)
32-35: 헬퍼 메서드 중복을 제거하는 것을 고려하세요.
getStringOrDefault메서드가 모든 템플릿 구현체에서 동일하게 중복됩니다. 공통 유틸리티 클래스로 추출하면 유지보수성이 향상됩니다.다음과 같이 공통 클래스로 추출할 수 있습니다:
패키지 루트에 유틸리티 클래스 생성:
package com.wellmeet.notification.template; import java.util.Map; public final class TemplatePayloadUtils { private TemplatePayloadUtils() { // utility class } public static String getStringOrDefault(Map<String, Object> payload, String key, String defaultValue) { Object value = payload.get(key); return value != null ? value.toString() : defaultValue; } }그런 다음 각 템플릿에서 사용:
- private String getStringOrDefault(Map<String, Object> payload, String key, String defaultValue) { - Object value = payload.get(key); - return value != null ? value.toString() : defaultValue; - } + + // import static com.wellmeet.notification.template.TemplatePayloadUtils.getStringOrDefault; + // 메서드에서 static import로 직접 사용
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (13)
src/main/java/com/wellmeet/exception/ErrorCode.java(1 hunks)src/main/java/com/wellmeet/notification/template/NotificationTemplate.java(1 hunks)src/main/java/com/wellmeet/notification/template/NotificationTemplateData.java(1 hunks)src/main/java/com/wellmeet/notification/template/NotificationTemplateFactory.java(1 hunks)src/main/java/com/wellmeet/notification/template/impl/ReservationCanceledTemplate.java(1 hunks)src/main/java/com/wellmeet/notification/template/impl/ReservationConfirmedTemplate.java(1 hunks)src/main/java/com/wellmeet/notification/template/impl/ReservationCreatedTemplate.java(1 hunks)src/main/java/com/wellmeet/notification/template/impl/ReservationReminderTemplate.java(1 hunks)src/main/java/com/wellmeet/notification/template/impl/ReservationUpdatedTemplate.java(1 hunks)src/main/java/com/wellmeet/notification/webpush/WebPushController.java(0 hunks)src/main/java/com/wellmeet/notification/webpush/WebPushService.java(0 hunks)src/main/java/com/wellmeet/notification/webpush/sender/WebPushSender.java(3 hunks)src/test/java/com/wellmeet/webpush/WebPushControllerTest.java(0 hunks)
💤 Files with no reviewable changes (3)
- src/main/java/com/wellmeet/notification/webpush/WebPushController.java
- src/main/java/com/wellmeet/notification/webpush/WebPushService.java
- src/test/java/com/wellmeet/webpush/WebPushControllerTest.java
🧰 Additional context used
🧬 Code graph analysis (6)
src/main/java/com/wellmeet/notification/template/impl/ReservationReminderTemplate.java (5)
src/main/java/com/wellmeet/notification/template/NotificationTemplateFactory.java (1)
Component(11-25)src/main/java/com/wellmeet/notification/template/impl/ReservationCanceledTemplate.java (1)
Component(9-35)src/main/java/com/wellmeet/notification/template/impl/ReservationConfirmedTemplate.java (1)
Component(9-35)src/main/java/com/wellmeet/notification/template/impl/ReservationCreatedTemplate.java (1)
Component(9-35)src/main/java/com/wellmeet/notification/template/impl/ReservationUpdatedTemplate.java (1)
Component(9-35)
src/main/java/com/wellmeet/notification/template/impl/ReservationConfirmedTemplate.java (5)
src/main/java/com/wellmeet/notification/template/NotificationTemplateFactory.java (1)
Component(11-25)src/main/java/com/wellmeet/notification/template/impl/ReservationCanceledTemplate.java (1)
Component(9-35)src/main/java/com/wellmeet/notification/template/impl/ReservationCreatedTemplate.java (1)
Component(9-35)src/main/java/com/wellmeet/notification/template/impl/ReservationReminderTemplate.java (1)
Component(9-36)src/main/java/com/wellmeet/notification/template/impl/ReservationUpdatedTemplate.java (1)
Component(9-35)
src/main/java/com/wellmeet/notification/template/impl/ReservationCreatedTemplate.java (5)
src/main/java/com/wellmeet/notification/template/NotificationTemplateFactory.java (1)
Component(11-25)src/main/java/com/wellmeet/notification/template/impl/ReservationCanceledTemplate.java (1)
Component(9-35)src/main/java/com/wellmeet/notification/template/impl/ReservationConfirmedTemplate.java (1)
Component(9-35)src/main/java/com/wellmeet/notification/template/impl/ReservationReminderTemplate.java (1)
Component(9-36)src/main/java/com/wellmeet/notification/template/impl/ReservationUpdatedTemplate.java (1)
Component(9-35)
src/main/java/com/wellmeet/notification/template/impl/ReservationUpdatedTemplate.java (5)
src/main/java/com/wellmeet/notification/template/NotificationTemplateFactory.java (1)
Component(11-25)src/main/java/com/wellmeet/notification/template/impl/ReservationCanceledTemplate.java (1)
Component(9-35)src/main/java/com/wellmeet/notification/template/impl/ReservationConfirmedTemplate.java (1)
Component(9-35)src/main/java/com/wellmeet/notification/template/impl/ReservationCreatedTemplate.java (1)
Component(9-35)src/main/java/com/wellmeet/notification/template/impl/ReservationReminderTemplate.java (1)
Component(9-36)
src/main/java/com/wellmeet/notification/template/impl/ReservationCanceledTemplate.java (5)
src/main/java/com/wellmeet/notification/template/NotificationTemplateFactory.java (1)
Component(11-25)src/main/java/com/wellmeet/notification/template/impl/ReservationConfirmedTemplate.java (1)
Component(9-35)src/main/java/com/wellmeet/notification/template/impl/ReservationCreatedTemplate.java (1)
Component(9-35)src/main/java/com/wellmeet/notification/template/impl/ReservationReminderTemplate.java (1)
Component(9-36)src/main/java/com/wellmeet/notification/template/impl/ReservationUpdatedTemplate.java (1)
Component(9-35)
src/main/java/com/wellmeet/notification/template/NotificationTemplateFactory.java (5)
src/main/java/com/wellmeet/notification/template/impl/ReservationCanceledTemplate.java (1)
Component(9-35)src/main/java/com/wellmeet/notification/template/impl/ReservationConfirmedTemplate.java (1)
Component(9-35)src/main/java/com/wellmeet/notification/template/impl/ReservationCreatedTemplate.java (1)
Component(9-35)src/main/java/com/wellmeet/notification/template/impl/ReservationReminderTemplate.java (1)
Component(9-36)src/main/java/com/wellmeet/notification/template/impl/ReservationUpdatedTemplate.java (1)
Component(9-35)
⏰ 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-and-push
🔇 Additional comments (13)
src/main/java/com/wellmeet/exception/ErrorCode.java (1)
19-20: 새로운 에러 코드 추가가 적절합니다.
TEMPLATE_NOT_FOUND에러 코드가 올바르게 추가되었습니다.INTERNAL_SERVER_ERROR상태 코드는 템플릿이 구현되지 않은 경우를 서버 측 설정/구현 문제로 간주하므로 적절합니다.src/main/java/com/wellmeet/notification/template/impl/ReservationUpdatedTemplate.java (2)
12-15: 타입 지원 확인 로직이 올바릅니다.
supports()메서드가RESERVATION_UPDATED타입을 정확하게 식별합니다.
17-29: 템플릿 데이터 생성 로직을 확인해 주세요.페이로드에서 값을 안전하게 추출하고 있습니다. 다만
reservationId가 페이로드에 없을 경우 빈 문자열을 기본값으로 사용하여 URL이/reservations/로 생성될 수 있습니다. 이러한 경우 적절한 폴백 처리가 있는지 확인해 주세요.src/main/java/com/wellmeet/notification/template/NotificationTemplateData.java (1)
3-8: 레코드 설계가 적절합니다.불변 데이터 전달을 위한 레코드 사용이 적절하며, 간결하고 명확한 구조입니다.
src/main/java/com/wellmeet/notification/webpush/sender/WebPushSender.java (2)
57-61: 템플릿 팩토리 통합이 잘 되었습니다.하드코딩된 페이로드 생성 로직을 템플릿 기반 접근 방식으로 깔끔하게 전환했습니다. 팩토리가 템플릿 선택과 에러 처리를 담당하여 관심사 분리가 잘 이루어졌습니다.
63-75: 페이로드 구성이 올바릅니다.템플릿 데이터를 사용하여 동적 값(title, body, url, requireInteraction)을 채우고, 정적 설정(icon, badge, vibrate)은 유지하는 적절한 균형을 보여줍니다.
src/main/java/com/wellmeet/notification/template/NotificationTemplate.java (1)
6-11: 인터페이스 설계가 우수합니다.전략 패턴을 지원하는 명확하고 간결한 인터페이스입니다.
supports와create메서드를 통해 템플릿 선택과 데이터 생성의 책임이 잘 분리되어 있습니다.src/main/java/com/wellmeet/notification/template/NotificationTemplateFactory.java (1)
11-15: Spring 통합이 적절합니다.
@Component와@RequiredArgsConstructor를 사용한 의존성 주입이 올바르게 구현되었습니다. Spring이 모든NotificationTemplate빈을 자동으로 수집하여 주입합니다.src/main/java/com/wellmeet/notification/template/impl/ReservationCreatedTemplate.java (1)
12-29: 템플릿 구현이 올바릅니다.
RESERVATION_CREATED타입에 대한 템플릿이 정확하게 구현되었습니다. 새 예약 알림에requireInteraction=true를 설정한 것이 적절합니다.src/main/java/com/wellmeet/notification/template/impl/ReservationConfirmedTemplate.java (1)
12-29: 템플릿 구현이 올바릅니다.
RESERVATION_CONFIRMED타입에 대한 템플릿이 정확하게 구현되었습니다. 확정 알림에requireInteraction=false를 설정한 것이 적절합니다.src/main/java/com/wellmeet/notification/template/impl/ReservationReminderTemplate.java (1)
9-15: LGTM!템플릿 선택 로직이 올바르게 구현되었습니다.
supports()메서드가RESERVATION_REMINDER타입을 정확히 식별합니다.src/main/java/com/wellmeet/notification/template/impl/ReservationCanceledTemplate.java (2)
9-15: LGTM!템플릿 선택 로직이 올바르게 구현되었습니다.
RESERVATION_CANCELED타입을 정확히 식별합니다.
17-29: 알림 데이터 구성이 적절합니다.예약 취소 알림의 제목, 본문, URL이 올바르게 구성되었고,
requireInteraction=false설정도 정보성 알림의 특성에 적합합니다.
ReservationReminderTemplate과 동일하게, 빈 문자열 기본값으로 인한 사용자 경험 저하 가능성을 검토해주세요.
| @Override | ||
| public NotificationTemplateData create(Map<String, Object> payload) { | ||
| String restaurantName = getStringOrDefault(payload, "restaurantName", "식당"); | ||
| String reservationTime = getStringOrDefault(payload, "reservationTime", ""); | ||
| String reservationId = getStringOrDefault(payload, "reservationId", ""); | ||
| String minutesBefore = getStringOrDefault(payload, "minutesBefore", "30"); | ||
|
|
||
| String title = "곧 예약 시간입니다"; | ||
| String body = String.format("%s 예약 시간이 %s분 남았습니다. 예약 시간: %s", | ||
| restaurantName, minutesBefore, reservationTime); | ||
| String url = "/reservations/" + reservationId; | ||
|
|
||
| return new NotificationTemplateData(title, body, url, true); | ||
| } |
There was a problem hiding this comment.
🧩 Analysis chain
알림 데이터 구성이 적절합니다.
예약 리마인더 알림의 제목, 본문, URL이 올바르게 구성되었고, requireInteraction=true 설정도 사용자가 반드시 확인해야 하는 리마인더 특성에 적합합니다.
단, 빈 문자열 기본값 처리를 확인해주세요. reservationTime과 reservationId가 비어있을 경우 사용자 경험이 저하될 수 있습니다.
다음 스크립트로 페이로드 검증 로직이 upstream에 존재하는지 확인할 수 있습니다:
🏁 Script executed:
#!/bin/bash
# 알림 전송 시 페이로드 필수 필드 검증이 있는지 확인
# NotificationTemplateFactory를 사용하는 코드 검색
rg -n -C5 "createTemplateData|NotificationTemplateFactory" --type=java
# 페이로드 생성 및 검증 로직 검색
rg -n -C5 "restaurantName|reservationTime|reservationId" --type=java -g "!**/template/**"Length of output: 6016
페이로드 필수 필드 검증 필요
현재 payload에 reservationTime과 reservationId에 대한 검증 없이 빈 문자열 기본값이 적용되어, 잘못된 알림이 전송될 우려가 있습니다. createTemplateData 호출 전 null/empty 검사 및 누락 시 예외 처리나 명확한 기본값 로직을 추가하세요.
🤖 Prompt for AI Agents
In
src/main/java/com/wellmeet/notification/template/impl/ReservationReminderTemplate.java
around lines 17 to 30, validate that payload contains non-null, non-empty
reservationTime and reservationId before building NotificationTemplateData; if
either is missing or empty, throw an IllegalArgumentException (or a
domain-specific exception) with a clear message indicating the missing field(s)
so the caller can handle the error, or alternatively return a failed
Result/Optional per project conventions — do the check immediately after
extracting values (or validate payload first), and do not rely on empty-string
defaults for these required fields.
🚩 Jira Ticket
SCRUM-125
🗣️ 리뷰 요구사항 (선택)
Summary by CodeRabbit