https://velog.io/@byeongju/속닥속닥-알림-기능-개선기-1

이전의 포스팅에서 새 댓글, 새 대댓글, 댓글 신고 등에 대한 알림 등록을 비동기 이벤트 방식으로 변경했습니다.

원래는 알림 기능 개선기를 깔끔하게 포스팅을 가져가려고 했는데, 개발을 하면서 느낀 점들과 고민들도 함께 정리해보려고 합니다.

그리고, 졸업작품 준비로 인해.. 많은 시간을 할당하지 못할 것 같아서 시간 날 때마다 조금씩 개선해볼 예정입니다.

게시글, 댓글 삭제에 따른 알림 삭제

게시글, 댓글 삭제에 따른 알림 삭제도 아래와 같은 비동기 이벤트 방식으로 변경했습니다. 게시글 삭제(혹은 댓글 삭제) 트랜잭션과 이에 따른 알림 삭제 트랜잭션이 분리된 상태입니다.

@Service
public class PostService {

    // ...

    @Transactional
    public void deletePost(Long id, AuthInfo authInfo) {
        // ...

        applicationEventPublisher.publishEvent(new PostDeletionEvent(post.getId()));

        postRepository.delete(post);
    }

@Service
public class CommentService {

    @Transactional
    public void deleteComment(Long commentId, AuthInfo authInfo) {
        // ...
        applicationEventPublisher.publishEvent(new CommentDeletionEvent(comment.getId()));

        deleteCommentOrReply(comment);
    }

@Component
@Async("asyncExecutor")
@Transactional(propagation = Propagation.REQUIRES_NEW)
public class DeletionEventHandler {

    private final NotificationRepository notificationRepository;

    public DeletionEventHandler(NotificationRepository notificationRepository) {
        this.notificationRepository = notificationRepository;
    }

    @TransactionalEventListener
    public void handleCommentDeletion(CommentDeletionEvent commentDeletionEvent) {
        notificationRepository.deleteAllByCommentId(commentDeletionEvent.getCommentId());
    }

    @TransactionalEventListener
    public void handlePostDeletion(PostDeletionEvent postDeletionEvent) {
        notificationRepository.deleteAllByPostId(postDeletionEvent.getPostId());
    }
}

삭제 트랜잭션이 분리되면 정합성은?

알림 도메인의 특성

모든 알림은 특정 댓글이나 특정 게시글에 대한 알림이기 때문에 알림이란 도메인은 게시글과 댓글에 종속적입니다. DB에서도 notification 테이블은 postId와 commentId를 통해 post와 comment를 참조하고 있습니다.
따라서, 댓글이나 게시글이 존재하지 않는 경우에 알림이 존재할 수 없는 것이죠.

삭제 트랜잭션이 분리되면?

위와 같은 특징을 가진 알림 도메인의 삭제 트랜잭션과 게시글, 댓글의 트랜잭션이 분리된다면
게시글 삭제 트랜잭션은 성공하고 알림 삭제 트랜잭션은 실패하는 경우에 데이터의 정합성이 맞지 않는 상태가 생길 수 있습니다.

따라서, 사용자가 위 알림 페이지에서 해당 알림을 클릭해서 게시글로 접근하려고 하면 404 Not Found를 마주칠 것입니다.

따라서, 게시글(혹은 댓글) 삭제 트랜잭션과 알림 삭제 트랜잭션을 분리하지 않고 하나의 트랜잭션으로 처리해야겠다고 생각했습니다.

@Component
@Transactional
public class DeletionEventHandler {

    private final NotificationRepository notificationRepository;

    public DeletionEventHandler(NotificationRepository notificationRepository) {
        this.notificationRepository = notificationRepository;
    }

    @EventListener
    public void handleCommentDeletion(CommentDeletionEvent commentDeletionEvent) {
        notificationRepository.deleteAllByCommentId(commentDeletionEvent.getCommentId());
    }

    @EventListener
    public void handlePostDeletion(PostDeletionEvent postDeletionEvent) {
        notificationRepository.deleteAllByPostId(postDeletionEvent.getPostId());
    }
}

@TransactionalEventListener 와 @Async를 사용해서 비동기 이벤트 방식으로 트랜잭션을 분리한 구조에서 위처럼 @EventListener 만을 사용해서 동기 이벤트 방식으로 알림 삭제를 처리하는 구조로 변경했습니다.

이벤트 방식이 아닌, notificationService.deleteNotificationByPostId(postI) 와 같은 방식으로 처리할 수도 있지만, notification 패키지에 대한 다른 패키지들의 참조를 끊어내고 싶어서 이벤트 방식은 유지했습니다.

이벤트객체와 이벤트리스너 하나로 통일

개선기 1에서 비동기 이벤트 방식으로 구조를 변경하면서 아래와 같이 여러 개의 Event 객체와 EventHandler를 만들었습니다. 아래와 같이 알림의 종류에 따라 Event 객체에 담기는 값이 조금씩 달랐기 때문에 알림의 종류에 따라 Event 객체와 EventListener를 나눴습니다.

@Component
@Async("asyncExecutor")
@Transactional(propagation = Propagation.REQUIRES_NEW)
public class NewNotificationEventHandler {

    private final NotificationRepository notificationRepository;

    public NewNotificationEventHandler(NotificationRepository notificationRepository) {
        this.notificationRepository = notificationRepository;
    }

    @TransactionalEventListener
    public void handleNewCommentNotification(NewCommentEvent newCommentEvent) {
        if (isNotifiableNewComment(newCommentEvent)) {
            Notification notification = Notification.newComment(
                    newCommentEvent.getNotificationTargetMemberId(), newCommentEvent.getPostId());
            notificationRepository.save(notification);
        }
    }

    private boolean isNotifiableNewComment(NewCommentEvent newCommentEvent) {
        return !newCommentEvent.getNotificationTargetMemberId()
                .equals(newCommentEvent.getCommentWritingMemberId());
    }

    @TransactionalEventListener
    public void handleNewReplyNotification(NewReplyEvent newReplyEvent) {
        if (isNotifiableNewReply(newReplyEvent)) {
            Notification notification = Notification.newReply(
                    newReplyEvent.getNotificationTargetMemberId(),
                    newReplyEvent.getPostId(),
                    newReplyEvent.getCommentId());
            notificationRepository.save(notification);
        }
    }

    //...

    @TransactionalEventListener
    public void handlePostHotBoardNotification(PostHotBoardEvent postHotBoardEvent) {
        Notification notification = Notification.postHotBoard(
                postHotBoardEvent.getNotificationTargetMemberId(), postHotBoardEvent.getPostId());
        notificationRepository.save(notification);
    }

    @TransactionalEventListener
    public void handlePostReportNotification(PostReportEvent postReportEvent) {
        Notification notification = Notification.postHotBoard(
                postReportEvent.getNotificationTargetMemberId(), postReportEvent.getPostId());
        notificationRepository.save(notification);
    }

    @TransactionalEventListener
    public void handleCommentReportNotification(CommentReportEvent commentReportEvent) {
        Notification notification = Notification.commentReport(
                commentReportEvent.getNotificationTargetMemberId(),
                commentReportEvent.getPostId(),
                commentReportEvent.getCommentId());
        notificationRepository.save(notification);
    }
}

위와 같은 구조에서 2가지 문제가 있다고 생각했습니다.

1. 같은 일을 하는 코드들이 중복된다.

동일하게 알림 관련 이벤트를 받아서 Notification으로 저장하는 5개의 메서드가 생깁니다.

2. 위 5개의 메서드에 동일한 AOP를 적용하려하면 불편하다.

알림 저장이 실패했을 경우에, 실패 정보를 로깅하는 AOP를 개발할 계획이었습니다. 그런데 5개의 메서드에 매개변수 타입이 다 다르다보니, 다른 5개의 매개변수를 받는 하나의 AOP 만들기가 번거로웠습니다. Event에 상속구조를 만들던가 혹은 다른 방법이 필요해 보였습니다.

따라서, 아래와 같은 3가지 방식을 생각해보았습니다.

1. 현재 구조에서 하나의 알림 종류에 따라 하나의 AOP를 개발

사실상 말이 안됩니다. 중복되는 코드가 계속 생기고 개발 비용이 너무 많이 들어갑니다.

2. 이벤트 객체들의 상위 타입을 정의해서 AOP에서 하나의 타입으로 처리할 수 있도록 변경

여기에 더해 위의 코드에서 isNotifiableNewComment 와 같은 메서드에서 존재하는 도메인 로직을 상위타입으로 추상화하고 각 이벤트 객체들 마다 각각의 로직을 구현할 수 있도록 할 수 있었습니다.

3. 단순하게 범용적인 하나의 이벤트 객체와 하나의 이벤트 핸들러를 사용한다.

범용적인 이벤트 객체를 만들어야 하기 때문에 이벤트의 종류에 따라 필드 값에 null이 할당 되는 경우가 발생합니다.

2번 방법과 3번 방법에 대한 고민을 많이 했습니다. 이벤트 객체에 대한 정의를 잘 내리지 못했기 때문이었습니다.

처음엔 이벤트 객체를 DTO로 정의 했습니다. CommentService나 PostReportService 등에서 발생한 알림 관련 알림 관련 데이터들을 이벤트 객체에 데이터를 넣어서 Handler로 전달하는 기능만을 수행한다고 생각했기 때문에 이벤트를 단순 DTO라고 생각했습니다. 그리고 Spring의 ApplicationEventPublisher와 함께 사용하는 Event라고 생각해서 Spring에 종속적인 DTO라고 생각했습니다. 따라서, DTO에는 상속 구조와 도메인 로직을 넣지 말고 3번 방식으로 개발을 진행했습니다. 하지만, 이벤트를 도메인으로 정의해서 2번 방식으로 진행하면 참 좋을텐데.. 라는 생각을 계속 하고 있었습니다.

3번 방식으로 개발을 진행하고 https://github.com/woowacourse-teams/2022-sokdak/pull/776 PR에 대해 팀원들과 이야기를 나눴습니다.

이야기를 나누면서 이벤트 객체를 도메인 객체로 정의하면 안되나..? 라는 생각을 계속 했습니다. 계속 고민한 결과 아래와 같은 결론을 내렸습니다.

이벤트 객체를 도메인으로 정의했습니다.

일단 이벤트를 적용하기 전에는 아래와 같이 CommentService에서 댓글을 저장한 후에, NotificationService를 참조하여 도메인 객체들을 넘겨 알림을 저장하게 했습니다.

notificationService.notifyCommentIfNotMine(post.getMember(), post, comment);

이벤트 방식도 방식이 이벤트 방식일뿐, 도메인 로직을 위해서 어떤 객체를 넘긴다는 의미에서 충분히 도메인 객체로 정의해도 된다는 생각이 들었습니다.

또한, 이벤트 객체는 Spring에 종속적인 것이 아닌가에 대한 의문에 대해서는 다음과 같이 생각했습니다.

일단 이벤트 객체는 Spring 4.2 부터 ApplicationEvent를 상속하지 않고 POJO로 가져갈 수 있습니다.

또한, Spring을 걷어내서 @EventListener가 존재하지 않더라도 이벤트 객체를 생성하는 객체와 이벤트 객체를 소비하는 객체를 연결해주는 어떤 객체를 생성하기만 하면 이벤트 객체를 계속 사용할 수 있다는 생각이 들었습니다.

따라서, 이벤트 객체를 도메인으로 정의하고 아래와 같은 알림 관련 이벤트들의 상위 타입을 선언해서 중복되는 정보를 추출하고

public abstract class NotificationEvent {

    protected final Long notificationTargetMemberId;
    private final Long postId;

    protected NotificationEvent(Long notificationTargetMemberId, Long postId) {
        this.notificationTargetMemberId = notificationTargetMemberId;
        this.postId = postId;
    }

    public Long getNotificationTargetMemberId() {
        return notificationTargetMemberId;
    }

    public Long getPostId() {
        return postId;
    }

    public abstract Long getNotificationAdditionalContentDataId();

    public abstract boolean isNotifiable();

    public abstract NotificationType getNotificationType();
}

알림의 성격에 따라 다른 구현체가 위의 상위 타입을 5개를 구현하도록 했습니다.

@Component
@Async("asyncExecutor")
@Transactional(propagation = Propagation.REQUIRES_NEW)
public class NewNotificationEventHandler {

    private final NotificationRepository notificationRepository;

    public NewNotificationEventHandler(NotificationRepository notificationRepository) {
        this.notificationRepository = notificationRepository;
    }

    @TransactionalEventListener
    public void handleNotificationEvent(NotificationEvent notificationEvent) {
        if (notificationEvent.isNotifiable()) {
            Notification notification = new Notification(
                    notificationEvent.getNotificationType(),
                    notificationEvent.getNotificationTargetMemberId(),
                    notificationEvent.getPostId(),
                    notificationEvent.getNotificationAdditionalContentDataId()
            );
            notificationRepository.save(notification);
        }
    }
}

EventHandler 부분도 이전에는 DTO로 정의했던 이벤트 객체에서 값을 꺼내서 notifiable인지를 체크했는데,
NotificationEvent 객체에게 notifiable 인지를 물어보면서 훨씬 객체지향적으로 풀어낼 수 있었습니다.

이렇게 이벤트 방식을 구축해보았습니다.

현재, 댓글 저장과 알림 저장 Transaction이 분리되면서 댓글을 저장이 되었지만, 알림은 저장되지 않는 경우가 발생할 수 있다고 생각이 들었습니다.

다음 포스팅에서는, 알림이 저장되지 않는 경우 이를 AOP를 통해 Logging하고 Batch와 같은 방식으로 후처리 해주는 글을 작성해보겠습니다.

다음 포스팅이 언제가 될지 모르겠지만, 꼭 위의 내용들을 적용해보려고 합니다.