Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Refactor/#649 댓글 행사 알림 스펙 변경 #672

Merged
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
@@ -0,0 +1,37 @@
package com.emmsale.event_publisher;

import com.emmsale.comment.domain.Comment;
import com.emmsale.member.domain.Member;
import java.time.LocalDateTime;
import lombok.Getter;
import lombok.RequiredArgsConstructor;

@RequiredArgsConstructor
@Getter
public class CommentNotificationEvent {

private static final String UPDATE_NOTIFICATION_COMMENT_TYPE = "COMMENT";
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

여기에 있는 static field를 단순 String이 아니라 NotificationType을 사용해보는 건 어떨까요?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

CommentNotificationEvent가 굳이 Notification을 알아야할까 라는 생각했어요.

enum을 쓴다고 해서 단지 패키지 의존성만 생길 뿐, 큰 장점을 얻지 못한다고 생각했습니다.


private final Long receiverId;
private final Long redirectId;
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

혹시 redirectId가 댓글의 id인가요?
만약 맞다면 redirectId로 추상화하여 EventNotificationEvent와 필드를 통일한 것도 좋지만 개인적으로 commentId 혹은 eventId와 같이 명시적으로 작성하는게 오해를 불러일으킬 확률을 낮출 수 있을 것 같아요.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

오 맞네여 이제 구분지어줄 필요가 없을 것 같습니다

감사합니다

private final LocalDateTime createdAt;
private final String notificationType;
private final String content;
private final String writer;
private final String writerImageUrl;

public static CommentNotificationEvent of(final Comment comment, final Comment trigger) {
final Member member = comment.getMember();
final Member triggerMember = trigger.getMember();

return new CommentNotificationEvent(
member.getId(),
trigger.getId(),
LocalDateTime.now(),
UPDATE_NOTIFICATION_COMMENT_TYPE,
trigger.getContent(),
triggerMember.getName(),
triggerMember.getImageUrl()
);
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,29 @@
package com.emmsale.event_publisher;

import com.emmsale.event.domain.Event;
import java.time.LocalDateTime;
import lombok.Getter;
import lombok.RequiredArgsConstructor;

@RequiredArgsConstructor
@Getter
public class EventNotificationEvent {

private static final String UPDATE_NOTIFICATION_EVENT_TYPE = "EVENT";

private final Long receiverId;
private final Long redirectId;
private final String notificationType;
private final LocalDateTime createdAt;
private final String title;

public static EventNotificationEvent of(final Event event, final Long receiverId) {
return new EventNotificationEvent(
receiverId,
event.getId(),
UPDATE_NOTIFICATION_EVENT_TYPE,
LocalDateTime.now(),
event.getName()
);
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,7 @@ public void publish(final Comment trigger, final Member loginMember) {
.orElse(Collections.emptySet());

notificationCommentCandidates.stream()
.map(it -> UpdateNotificationEvent.of(it, trigger.getId()))
.map(it -> CommentNotificationEvent.of(it, trigger))
.forEach(applicationEventPublisher::publishEvent);
}

Expand Down Expand Up @@ -71,7 +71,7 @@ public void publish(final Event event) {
private void publishEvent(final Event event, final List<Member> members) {
members.forEach(
it -> applicationEventPublisher.publishEvent(
UpdateNotificationEvent.of(event, it.getId())
EventNotificationEvent.of(event, it.getId())
)
);
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -6,19 +6,25 @@

import com.emmsale.event_publisher.MessageNotificationEvent;
import com.emmsale.member.domain.MemberRepository;
import com.emmsale.notification.application.generator.CommentNotificationMessageGenerator;
import com.emmsale.notification.application.generator.EventNotificationMessageGenerator;
import com.emmsale.notification.application.generator.MessageNotificationMessageGenerator;
import com.emmsale.notification.application.generator.NotificationMessageGenerator;
import com.emmsale.notification.application.generator.RequestNotificationMessageGenerator;
import com.emmsale.notification.application.generator.UpdateNotificationMessageGenerator;
import com.emmsale.notification.domain.FcmToken;
import com.emmsale.notification.domain.FcmTokenRepository;
import com.emmsale.notification.domain.Notification;
import com.emmsale.notification.domain.NotificationType;
import com.emmsale.notification.domain.RequestNotification;
import com.emmsale.notification.domain.UpdateNotification;
import com.emmsale.notification.exception.NotificationException;
import com.fasterxml.jackson.databind.ObjectMapper;
import com.google.auth.oauth2.GoogleCredentials;
import java.io.IOException;
import java.util.List;
import java.util.Map;
import java.util.function.Function;
import lombok.RequiredArgsConstructor;
import lombok.extern.slf4j.Slf4j;
import org.springframework.beans.factory.annotation.Value;
Expand All @@ -41,6 +47,12 @@ public class FirebaseCloudMessageClient {
private static final String POSTFIX_FCM_REQUEST_URL = "/messages:send";
private static final String FIREBASE_KEY_PATH = "kerdy-submodule/firebase-kerdy.json";
private static final String GOOGLE_AUTH_URL = "https://www.googleapis.com/auth/cloud-platform";
private static final Map<NotificationType, Function<Notification, NotificationMessageGenerator>> GENERATOR_MAP =
Map.of(
NotificationType.EVENT, EventNotificationMessageGenerator::new,
NotificationType.COMMENT, CommentNotificationMessageGenerator::new
);


private final ObjectMapper objectMapper;
private final MemberRepository memberRepository;
Expand Down Expand Up @@ -71,6 +83,13 @@ public void sendMessageTo(final MessageNotificationEvent messageNotificationEven
);
}

public void sendMessageTo(final Notification notification, final Long receiverId) {
sendMessageTo(
receiverId,
GENERATOR_MAP.get(notification.getType()).apply(notification)
);
}

private void sendMessageTo(
final Long receiverId,
final NotificationMessageGenerator messageGenerator
Expand Down
Original file line number Diff line number Diff line change
@@ -1,10 +1,17 @@
package com.emmsale.notification.application;

import com.emmsale.event_publisher.CommentNotificationEvent;
import com.emmsale.event_publisher.EventNotificationEvent;
import com.emmsale.event_publisher.MessageNotificationEvent;
import com.emmsale.event_publisher.UpdateNotificationEvent;
import com.emmsale.notification.domain.Notification;
import com.emmsale.notification.domain.NotificationRepository;
import com.emmsale.notification.domain.NotificationType;
import com.emmsale.notification.domain.UpdateNotification;
import com.emmsale.notification.domain.UpdateNotificationRepository;
import com.emmsale.notification.domain.UpdateNotificationType;
import com.fasterxml.jackson.core.JsonProcessingException;
import com.fasterxml.jackson.databind.ObjectMapper;
import lombok.RequiredArgsConstructor;
import lombok.extern.slf4j.Slf4j;
import org.springframework.stereotype.Component;
Expand All @@ -19,6 +26,8 @@ public class NotificationEventListener {

private final UpdateNotificationRepository updateNotificationRepository;
private final FirebaseCloudMessageClient firebaseCloudMessageClient;
private final NotificationRepository notificationRepository;
private final ObjectMapper objectMapper;

@Transactional(propagation = Propagation.REQUIRES_NEW)
@TransactionalEventListener
Expand All @@ -40,6 +49,50 @@ public void createUpdateNotification(final UpdateNotificationEvent updateNotific
}
}

@Transactional(propagation = Propagation.REQUIRES_NEW)
@TransactionalEventListener
public void createCommentNotification(final CommentNotificationEvent commentNotificationEvent) {
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

이 둘의 코드가 거의 동일한 것 같아요. 차이라고 한다면,
Notification을 생성할 때, Type과 파라미터 정도겠군요.

이정도로 공통된 부분은 추상화가 가능하지 않을까요?
(이건 의견이라 반영 안하셔도 됩니다.)

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

NotificationEvent에서, EventType을 들고있어요.

이를 이용해서 한다면 추상화가 될 것 같아요.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

다음 이슈에서 코드 삭제하면서 한번 고려해보도록 할게요 !

try {
final String jsonData = objectMapper.writeValueAsString(commentNotificationEvent);

final Notification notification = notificationRepository.save(
new Notification(NotificationType.COMMENT, jsonData)
);

firebaseCloudMessageClient.sendMessageTo(
notification,
commentNotificationEvent.getReceiverId()
);

} catch (JsonProcessingException e) {
log.error("json 에러");
} catch (Exception e) {
log.error("파이어베이스 관련 에러, 알림 재요청 필요, {}", e.getMessage(), e);
}
}

@Transactional(propagation = Propagation.REQUIRES_NEW)
@TransactionalEventListener
public void createEventNotification(final EventNotificationEvent eventNotificationEvent) {
try {
final String jsonData = objectMapper.writeValueAsString(eventNotificationEvent);

final Notification notification = notificationRepository.save(
new Notification(NotificationType.EVENT, jsonData)
);

firebaseCloudMessageClient.sendMessageTo(
notification,
eventNotificationEvent.getReceiverId()
);

} catch (JsonProcessingException e) {
log.error("json 에러");
} catch (Exception e) {
log.error("파이어베이스 관련 에러, 알림 재요청 필요, {}", e.getMessage(), e);
}
}

@Transactional(propagation = Propagation.REQUIRES_NEW)
@TransactionalEventListener
public void createMessageNotification(final MessageNotificationEvent messageNotificationEvent) {
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,35 @@
package com.emmsale.notification.application.dto;

import com.fasterxml.jackson.annotation.JsonProperty;
import lombok.Getter;
import lombok.RequiredArgsConstructor;

@RequiredArgsConstructor
@Getter
public class CommentNotificationMessage {

@JsonProperty("validate_only")
private final boolean validateOnly;
private final Message message;

@RequiredArgsConstructor
@Getter
public static class Message {

private final Data data;
private final String token;
}

@RequiredArgsConstructor
@Getter
public static class Data {

private final String receiverId;
private final String redirectId;
private final String notificationType;
private final String createdAt;
private final String content;
private final String writer;
private final String writerImageUrl;
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,33 @@
package com.emmsale.notification.application.dto;

import com.fasterxml.jackson.annotation.JsonProperty;
import lombok.Getter;
import lombok.RequiredArgsConstructor;

@RequiredArgsConstructor
@Getter
public class EventNotificationMessage {

@JsonProperty("validate_only")
private final boolean validateOnly;
private final Message message;

@RequiredArgsConstructor
@Getter
public static class Message {

private final Data data;
private final String token;
}

@RequiredArgsConstructor
@Getter
public static class Data {

private final String receiverId;
private final String redirectId;
private final String notificationType;
private final String createdAt;
private final String title;
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,53 @@
package com.emmsale.notification.application.generator;

import static com.emmsale.notification.exception.NotificationExceptionType.BAD_REQUEST_MEMBER_ID;
import static com.emmsale.notification.exception.NotificationExceptionType.CONVERTING_JSON_ERROR;

import com.emmsale.member.domain.MemberRepository;
import com.emmsale.notification.application.dto.CommentNotificationMessage;
import com.emmsale.notification.application.dto.CommentNotificationMessage.Data;
import com.emmsale.notification.application.dto.CommentNotificationMessage.Message;
import com.emmsale.notification.domain.Notification;
import com.emmsale.notification.exception.NotificationException;
import com.fasterxml.jackson.core.JsonProcessingException;
import com.fasterxml.jackson.databind.ObjectMapper;
import lombok.RequiredArgsConstructor;

@RequiredArgsConstructor
public class CommentNotificationMessageGenerator implements NotificationMessageGenerator {

private final Notification notification;

@Override
public String makeMessage(
final String targetToken,
final ObjectMapper objectMapper,
final MemberRepository memberRepository
) {

final String jsonData = notification.getJsonData();

try {

final Data data = objectMapper.readValue(jsonData, Data.class);

validateIsExistedReceiver(memberRepository, Long.valueOf(data.getReceiverId()));

final CommentNotificationMessage message = new CommentNotificationMessage(
DEFAULT_VALIDATE_ONLY, new Message(data, targetToken)
);

return objectMapper.writeValueAsString(message);

} catch (JsonProcessingException e) {
throw new NotificationException(CONVERTING_JSON_ERROR);
}
}

private void validateIsExistedReceiver(final MemberRepository memberRepository,
final Long receiverId) {
if (!memberRepository.existsById(receiverId)) {
throw new NotificationException(BAD_REQUEST_MEMBER_ID);
}
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,52 @@
package com.emmsale.notification.application.generator;

import static com.emmsale.notification.application.dto.EventNotificationMessage.Data;
import static com.emmsale.notification.exception.NotificationExceptionType.BAD_REQUEST_MEMBER_ID;
import static com.emmsale.notification.exception.NotificationExceptionType.CONVERTING_JSON_ERROR;

import com.emmsale.member.domain.MemberRepository;
import com.emmsale.notification.application.dto.EventNotificationMessage;
import com.emmsale.notification.application.dto.EventNotificationMessage.Message;
import com.emmsale.notification.domain.Notification;
import com.emmsale.notification.exception.NotificationException;
import com.fasterxml.jackson.core.JsonProcessingException;
import com.fasterxml.jackson.databind.ObjectMapper;
import lombok.RequiredArgsConstructor;

@RequiredArgsConstructor
public class EventNotificationMessageGenerator implements NotificationMessageGenerator {

private final Notification notification;

@Override
public String makeMessage(
final String targetToken,
final ObjectMapper objectMapper,
final MemberRepository memberRepository
) {
final String jsonData = notification.getJsonData();

try {

final Data data = objectMapper.readValue(jsonData, Data.class);

validateIsExistedReceiver(memberRepository, Long.valueOf(data.getReceiverId()));

final EventNotificationMessage message = new EventNotificationMessage(
DEFAULT_VALIDATE_ONLY, new Message(data, targetToken)
);

return objectMapper.writeValueAsString(message);

} catch (JsonProcessingException e) {
throw new NotificationException(CONVERTING_JSON_ERROR);
}
}

private void validateIsExistedReceiver(final MemberRepository memberRepository,
final Long receiverId) {
if (!memberRepository.existsById(receiverId)) {
throw new NotificationException(BAD_REQUEST_MEMBER_ID);
}
}
}
Loading
Loading