속닥속닥 알림 기능 개선기 1

공병주(Chris)·2022년 11월 17일
개선기우아한테크코스
2
post-thumbnail

해당 포스팅은 속닥속닥 알림 기능 개선기 2편으로도 이어집니다.
속닥속닥 서비스에는 알림 기능이 존재한다. 아래의 경우에 사용자에게 알림을 등록해준다.

  • 작성한 게시글에 새로운 댓글이 달림
  • 작성한 댓글에 새로운 대댓글이 달림
  • 작성한 게시글 핫게시판 등록됨
  • 작성한 게시글이 5회 이상 신고 당함
  • 작성한 댓글 5회 이상 신고 당함
  • 작성한 대댓글 5회 이상 신고 당함

프로젝트를 진행할 때 알림 기능 개발을 담당했는데, 개선하고 싶은 부분들이 있어서 개선해보려고 한다.

개선 사항

1. Notification의 불필요한 객체 참조

현재 Notification 객체는 아래와 같다.

@Entity
@Getter
@EntityListeners(AuditingEntityListener.class)
public class Notification {

    @Id
    @GeneratedValue(strategy = GenerationType.IDENTITY)
    @Column(name = "notification_id")
    private Long id;

    private NotificationType notificationType;

    @ManyToOne(fetch = FetchType.LAZY)
    @JoinColumn(name = "member_id")
    private Member member;

    @ManyToOne(fetch = FetchType.LAZY)
    @JoinColumn(name = "post_id")
    private Post post;

    @ManyToOne(fetch = FetchType.LAZY)
    @JoinColumn(name = "comment_id")
    private Comment comment;

    private boolean inquired;

    @CreatedDate
    private LocalDateTime createdAt;

    // ...
}

Member를 참조하고 있지만, 실제로 Notification에서 사용하는 Member에 대한 정보는 member_id 뿐이다.

Post를 참조하고 있지만, post_id만을 사용하고 NotificationType에 따라 post의 title이 사용되기도 한다.

Comment는 null인 경우도 있고, NotificationType에 따라 맥락에서 comment의 내용이 사용된다.

NotificationType에 따라 사용되는 Post의 title과 comment의 message는 아래와 같이 알림의 내용을 반환하는데 쓰인다.

@Entity
@Getter
@EntityListeners(AuditingEntityListener.class)
public class Notification {

    // ...
    public String getContent() {
        if (getComment() == null || notificationType == NotificationType.NEW_COMMENT) {
            return post.getTitle();
        }
        return getComment().getMessage();
    }
    // ...
}

위와 같이 Notification은 필요한 참조만을 하는 것이 아니라, 불필요한 객체 참조를 하고 있다는 생각이 들었다. JPA 기술을 처음 사용할 때, 의존성을 고려하지 않고 객체 참조를 맺었던 것이 문제다.

후회해도 소용없다. 일단, 필요한 정보는 NotificationType, post_id, member_id, content 4가지라고 정의를 내렸고 아래와 같이 객체 설계를 바꾸었다.

@Entity
@Getter
@EntityListeners(AuditingEntityListener.class)
public class Notification {

    @Id
    @GeneratedValue(strategy = GenerationType.IDENTITY)
    @Column(name = "notification_id")
    private Long id;

    private NotificationType notificationType;

    private Long memberId;

    private Long postId;

    private String content;

    private boolean inquired;

    @CreatedDate
    private LocalDateTime createdAt;
    // ...
}

기존에 Notification이 Post와 Comment, Member와 의존 관계를 가지고 있어서 테스트의 given 절이 상당히 복잡했는데, 의존관계를 끊고 id를 통해 간접 참조를 하니까 테스트를 하기가 훨씬 수월해졌다.

2. notification 패키지와의 양방향 참조

여러 알림의 종류가 있지만 새로운 댓글의 경우를 대표적으로 보겠다.

댓글에 대한 알림의 경우 아래와 같이 구현이 되어있다.

//comment 패키지

@Service
@Transactional(readOnly = true)
public class CommentService {
    
    private final CommentRepository commentRepository;
    private final MemberRepository memberRepository;
    private final PostRepository postRepository;
    private final NotificationService notificationService;
    private final CommentLikeRepository commentLikeRepository;
    private final AuthService authService;
    // ... 생성자

    @Transactional
    public Long addComment(Long postId, NewCommentRequest newCommentRequest, AuthInfo authInfo) {
        validateAuthority(authInfo);
        Comment comment = generateComment(postId, newCommentRequest, authInfo);
        // 댓글을 작성하기 위한 일련의 과정
        commentRepository.save(comment);

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

        return comment.getId();
    }
    // ...
}

새로운 댓글이 달리는 로직에서에 아래 NotificationService의 메서드를 한 트랜잭션 내에서 호출해서 새로운 알림을 등록한다.

//notification 패키지

@Service
@Transactional(readOnly = true)
public class NotificationService {

    private final NotificationRepository notificationRepository;

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

    @Transactional
    public void notifyCommentIfNotMine(Member member, Post post, Comment comment) {
        if (!comment.isAuthorized(member.getId())) {
            notify(member, post, comment, NEW_COMMENT);
        }
    }
    // ...
    private void notify(Member member, Post post, Comment comment, NotificationType notificationType) {
        Notification notification = Notification.builder()
                .notificationType(notificationType)
                .member(member)
                .post(post)
                .comment(comment)
                .build();
        notificationRepository.save(notification);
    }
}

2-1. notification이 가지는 post, member, comment 패키지에 대한 의존

위의 NotificationService를 보면 알림을 저장하는 경우에 Post와 Member, Comment들을 통해 알림을 저장할지 말지 판단을 한다. 그리고 아래처럼 Notification은 Post, Member, Comment를 객체 참조하고 있다.

따라서, notification 패키지는 post, member, comment에 대한 패키지 의존를 가진다.

2-2. post, comment, board, report 패키지가 가지는 notification에 대한 패키지 의존과 Service 간의 강한 의존

또한, PostService, CommentService, BoardService, PostReportService, CommentReportService에서 NotificationService를 참조하고 NotificationService의 메서드를 통해 알림 저장 로직을 수행하기 때문에 post, comment, board, report 패키지는 notification에 대한 패키지 의존성을 가진다.

  • post ↔  notification (양방향)

  • comment ↔  notification (양방향)

  • board ➡️ notification

  • report ➡️ notification

    따라서, 위와 같은 패키지 의존 관계가 생기고 이를 이벤트 방식으로 모두 끊어보려고 했다. 아래에서 확인해보자.

2. Transaction 범위

위처럼 새 댓글 작성의 트랜잭션에 알림 등록이 포함되었기 때문에, 알림 등록이 실패하는 경우에 새 댓글 작성이 함께 실패하는 결과를 불러일으킨다.

댓글을 작성하는 사용자는 알림 등록 실패 여부에 관계없이 자신의 댓글이 등록되는 것이 중요하다고 생각했다. 알림이 등록이 실패하는 경우에 댓글 작성이 실패한다면 주객이 전도된 것이라고 생각한다.

알림에 의존적이고 알림이 큰 영향을 차지하는 서비스의 경우에는 알림 등록을 한 트랜잭션으로 잡아야한다고 생각한다. 하지만, 서비스 사용자들에게 직접 물어본 결과, 알림에 크게 의존적이지 않고 부가 기능정도로 생각하고 있었다.

따라서, 알림 등록에 대한 Transaction을 분리해야겠다고 생각했다.

Nested 전파레벨

Nested 전파레벨을 사용하면 트랜잭션을 위에서 원했던 것처럼 분리할 수 있다. 댓글 작성에 실패했을 경우에 알림 저장도 진행되지 않고 알림 저장에만 실패했을 경우에 댓글 작성은 롤백 되지 않는다.
하지만, Nested만으로는 notification과 다른 패키지들과의 양방향 참조를 끊을 수 없다.
결국, post나 comment 패키지에서 notification 패키지의 알림 저장 메서드를 호출해야하기 때문이다.

의존 역전은?

그렇다면 의존을 역전시키는 건 어떨까? post와 comment에 알림 저장 인터페이스를 각각 두고 notification 패키지에서 이들을 구현하는 방식이다. 이렇게 하면 동일한 일을 수행하는 인터페이스가 post와 comment에 존재한다. 알림 생성이 필요한 로직이 늘어감에 따라 동일한 인터페이스들이 계속 생겨날 것이다.

제 3의 패키지에 인터페이스를 두는 것은?

제 3의 패키지 A라는 곳에 알림 저장 패키지를 두고 post와 comment에서는 해당 인터페이스를 참조하고 notification 패키지에서는 해당 인터페이스를 구현하는 방식을 떠올릴 수 있다.
하지만, 이 방식은 패키지의 복잡도가 증가해서 좋지 않아 보인다.

ApplicationEventPublisher와 TransactionalEventListener

1. ApplicationEventPublisher를 통해 양방향 패키지 의존 삭제

아래와 같이 NotificationService를 의존하는 것이 아닌, ApplicationEventPublisher를 통해 이벤트를 발행했다. 

@Service
@Transactional(readOnly = true)
public class CommentService {
    
    private final CommentRepository commentRepository;
    private final MemberRepository memberRepository;
    private final PostRepository postRepository;
    private final CommentLikeRepository commentLikeRepository;
    private final AuthService authService;
    private final ApplicationEventPublisher applicationEventPublisher;
    // ... 생성자

    @Transactional
    public Long addComment(Long postId, NewCommentRequest newCommentRequest, AuthInfo authInfo) {
        validateAuthority(authInfo);
        Comment comment = generateComment(postId, newCommentRequest, authInfo);
        // 댓글을 작성하기 위한 일련의 과정(post, member..)
        commentRepository.save(comment);

        NewCommentEvent newCommentEvent = new NewCommentEvent(
                post.getMember().getId(), post.getId(), comment.getId(), member.getId());
        applicationEventPublisher.publishEvent(newCommentEvent); // 이벤트 발행

        return comment.getId();
    }

또한, 이벤트 발행에 사용되는 NewCommentEvent 객체의 경우에 comment 패키지에 위치시켰다. 결과적으로, comment에서 가졌던 notification 패키지에 대한 의존을 제거하고 notification에서만 comment에 대한 패키지를 의존하도록 했다. post도 위의 방식으로 양방향 의존 관계를 제거하였다.

알림 등록이 필요한 board와 report 패키지에서도 알림을 등록할 때 말고는 notification에 대한 참조가 필요없었기 때문에, 알림 등록을 이벤트 방식으로 처리하고 notification 패키지에 대한 참조를 끊었다.

2. @TransactionalEventListener를 통한 Transaction 분리

위에서 의존성을 분리하는데 사용한 ApplicationEventPublisher로 발행한 이벤트를 Listen하는 방법은 크게 EventListener와 TransactionEventListner가 있다.

EventListener

EventListener는 ApplicationEventPublisher가 발행한 event를 Listen하고 바로 로직을 실행한다. 댓글 등록 시에 알림 등록 이벤트를 발행하고 EventListener로 발행한다면, 댓글 등록 트랜잭션에서 예외가 발생해 댓글 등록이 되지 않은 상황에도 알림이 등록될 수 있다. DB 정합성이 깨지는 순간이 있다.

TransactionEventListener

하지만 TransactionEventListener는 event를 Listen하고 로직을 실행할 시점을 phase라는 값으로 지정할 수 있다. 아래와 같이 4개의 phase를 설정할 수 있다.

  • BEFORE_COMMIT
  • AFTER_COMMIT (default)
  • AFTER_ROLLBACK
  • AFTER_COMPLETION

댓글 등록 Transaction이 commit 되었을 때, 알림 등록을 해야하기 때문에 AFTER_COMMIT을 선택했다.

@Component
@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 (isDifferentCommentMemberFromPostMember(newCommentEvent)) {
            Notification notification =
                    Notification.newComment(newCommentEvent.getTargetMemberId(), newCommentEvent.getPostId());
            notificationRepository.save(notification);
        }
    }
    // ...
}

주의점

@Transactional(propagation = Propagation.REQUIRES_NEW)

REQUIRES_NEW로 새로운 Transaction을 열어주지 않으면, Event를 Listen하는 곳에서 추가적인 DB 작업을 수행할 수 없다. 공식문서를 보면 아래와 같이 설명이 나와있다.

이벤트를 발행한 쪽에서의 Transaction은 커밋되었지만, 트랜잭션 리소스는 여전히 active하고 accesible 하다. 따라서, 새 트랜잭션으로 분리해주지 않는다면 Listen하는 곳에서의 로직은 더 이상의 커밋을 할 수 없는 이벤트 발행처의 Transaction에 속하게 된다.

따라서, REQUIRE_NEW propagation 설정을 통해 새 트랜잭션을 열어주어야한다.

비동기 처리

https://velog.io/@byeongju/TransactionalEventListener를-동기로-처리할-때의-발생가능한-문제

위의 링크에서와 같이 동기 방식으로 이벤트를 처리한다면 이벤트 발행처에서 Transaction이 commit되어도 connection을 놓지 않는 문제가 있다. 따라서 비동기로 처리하는 것이 안전하다고 판단해서 비동기를 적용하였다. 또한, 댓글을 작성하는 사용자의 입장에서 알림을 저장하는 것까지 동기로 동기로 처리하는 것이 불필요하다고 생각도 들었다.

@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 (isDifferentCommentMemberFromPostMember(newCommentEvent)) {
            Notification notification =
                    Notification.newComment(newCommentEvent.getTargetMemberId(), newCommentEvent.getPostId());
            notificationRepository.save(notification);
        }
    }
    // ...
}

비동기로 처리할 경우에 새로운 스레드가 할당되기 때문에 Repository의 save를 호출 할때 새로운 트랜잭션이 열려서 @Transactional(propagation = Propagation.REQUIRES_NEW)를 적어주어도 되지 않지만, 이도 한번은 더 생각해야하는 것이기 때문에 휴먼 에러를 방지하고자 명시해주었다.

이렇게 비동기 이벤트 방식으로 알림 저장을 구현하였다.

테스트

테스트는 어떻게 작성했나.

1. 이벤트 발행처에서의 테스트

@Service
@Transactional(readOnly = true)
public class CommentService {
    
    private final CommentRepository commentRepository;
    private final MemberRepository memberRepository;
    private final PostRepository postRepository;
    private final CommentLikeRepository commentLikeRepository;
    private final AuthService authService;
    private final ApplicationEventPublisher applicationEventPublisher;
    // ... 생성자

    @Transactional
    public Long addComment(Long postId, NewCommentRequest newCommentRequest, AuthInfo authInfo) {
        validateAuthority(authInfo);
        Comment comment = generateComment(postId, newCommentRequest, authInfo);
        // 댓글을 작성하기 위한 일련의 과정(post, member..)
        commentRepository.save(comment);

        NewCommentEvent newCommentEvent = new NewCommentEvent(
                post.getMember().getId(), post.getId(), comment.getId(), member.getId());
        applicationEventPublisher.publishEvent(newCommentEvent); // 이벤트 발행

        return comment.getId();
    }

위의 addComment 메서드에서의 관심사는 댓글을 저장하고 NewCommentEvent를 한번 가져가는 것이기 때문에 아래와 같이 댓글이 저장되었는지 확인하고, NewCommentEvent가 한번 발행되었는지만을 확인하도록 했다.

class CommentServiceTest extends ServiceTest {
    
    @Autowired
    private CommentService commentService;

    @Autowired
    private PostRepository postRepository;

    @Autowired
    private CommentRepository commentRepository;

    @Autowired
    private ApplicationEvents applicationEvents;
    
    @DisplayName("익명 게시글에서 게시글 작성자가 기명으로 댓글 등록")
    @Test
    void addComment_Identified_PostWriterAnonymous_PostWriterCommentWriterSame() {
        Post anonymousPost = saveAnonymoustPost()

        Long commentId = commentService.addComment(anonymousPost.getId(), NON_ANONYMOUS_COMMENT_REQUEST, AUTH_INFO);

        Comment foundComment = commentRepository.findById(commentId).orElseThrow();
        long newCommentEventCount = applicationEvents.stream(NewCommentEvent.class).count();
        assertAll(
                () -> assertThat(foundComment.getMessage()).isEqualTo(NON_ANONYMOUS_COMMENT_REQUEST.getContent()),
                () -> assertThat(foundComment.getMember()).isEqualTo(member),
                () -> assertThat(foundComment.getNickname()).isEqualTo(member.getNickname()),
                () -> assertThat(newCommentEventCount).isEqualTo(1)
        );
    }
    // ...
}

2. 이벤트 소비처

@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 (isDifferentCommentMemberFromPostMember(newCommentEvent)) {
            Notification notification =
                    Notification.newComment(newCommentEvent.getTargetMemberId(), newCommentEvent.getPostId());
            notificationRepository.save(notification);
        }
    }
    // ...
}

위와 같은 이벤트 소비처에서는 아래와 같이 테스트를 작성했다.

@SpringBootTest
class NewNotificationEventHandlerTest {

    @Autowired
    private NewNotificationEventHandler newNotificationEventHandler;

    @Autowired
    private NotificationRepository notificationRepository;

    @Autowired
    private DatabaseCleaner databaseCleaner;

    @AfterEach
    void cleanUp() {
        databaseCleaner.clear();
    }

    @DisplayName("댓글 작성자와 게시글 작성자가 다르다면 NEW_COMMENT 알림을 저장한다")
    @Test
    void handleNewCommentNotification() {
        NewCommentEvent newCommentEvent = new NewCommentEvent(TARGET_MEMBER_ID, POST_ID, COMMENT_ID, COMMENT_WRITER_ID);

        newNotificationEventHandler.handleNewCommentNotification(newCommentEvent);

        assertThat(notificationRepository.findAll()).hasSize(1);
    }

    @DisplayName("댓글 작성자와 게시글 작성자가 같다면 NEW_COMMENT 알림을 저장하지 않는다")
    @Test
    @Transactional
    void handleNewCommentNotification_unsaved() {
        Long sameIdWithPostWriterId = TARGET_MEMBER_ID;
        NewCommentEvent newCommentEvent =
                new NewCommentEvent(TARGET_MEMBER_ID, POST_ID, COMMENT_ID, sameIdWithPostWriterId);

        newNotificationEventHandler.handleNewCommentNotification(newCommentEvent);

        assertThat(notificationRepository.findAll()).isEmpty();
    }
}

실패

하지만, 테스트가 실패하는 경우가 있었다. 그 이유는 NewNotificationEventHandler의 메서드들이 모두 비동기로 동작하기 때문이다. 따라서, NewNotification의 알림을 저장하는 save 메서드가 실행되기 전에 테스트 코드에 존재하는 검증을 위한 findAll 메서드가 실행되기 때문에 테스트가 실패했다.

해결방안

테스트 코드에서 비동기 처리가 끝날 때까지 검증을 하지 않고 기다린다.

  1. Thread.sleep()

메인스레드를 sleep() 시켜서 비동기 처리가 이뤄질 때까지 기다리는 방법이 있다. 하지만, 환경에 따라서 비동기 처리에 소요되는 시간이 상이할 것이라고 생각이 든다. 따라서 테스트가 일관되게 성공할 것이라는 보장이 없어서 채택하지 않았다.

  1. Awaitility 라이브러리 사용

비동기 테스트를 위한 라이브러리이다. 비동기 처리가 이뤄질 때까지 await 할 수 있는 기능이 존재한다. 비동기 처리가 늦어진다면 마냥 기다릴 수 없기 때문에 timeout을 설정할 수 있다. 하지만, 문제는 환경에 따라 비동기 처리의 소요 시간이 다르다는 것이다. 결국, timeout에 따라 검증의 정도가 달라지고 timeout일 경우에 의도한 테스트가 동작하지 않는 것이 좋지 않은 테스트 환경이라고 생각했다. 그리고 timeout의 경우에 테스트가 실패했다고 봐야할까? 도 의문이었다.

  1. 테스트만을 동기로 실행한다. 

처음에는 프로덕션 환경과 테스트 환경의 방식이 다르기 때문에 좋지 않다고 생각을 했다. 하지만, 다시 생각해보니 동기인지 비동기인지를 테스트에서 검증하는 것이 중요한가라는 생각이 들었다. 동기인지 비동기인지에 따라서 프로덕션의 로직과 결과가 달라지는 것이 아니다. 비동기에 대한 테스트를 진행하는 것은 스프링 프레임워크에 대한 도전이 아닌가 라는 생각도 들었다.

3가지를 고려해보고 각각의 이유로 인해, 동기 방식으로 테스트를 실행하는 방식을 택했다. 동기로 테스트를 실행한 방식은 아래 링크에 작성해두었다.

https://velog.io/@byeongju/비동기-로직-동기-방식으로-테스트하기

비동기 스레드를 관리하는 ThreadPoolTaskExecutor Bean을 동기 방식의 SyncTaskExecutor로 바꿔주면 된다.

추가적인 개선 사항

  • 알림 삭제 이벤트 방식
  • 댓글은 저장되었지만, 알림이 등록되지 않았을 때의 처리 (feat. AOP, BATCH?)

참고자료

https://kwonnam.pe.kr/wiki/springframework/transaction/transactional_event_listener

https://dzone.com/articles/transaction-synchronization-and-spring-application

https://blog.pragmatists.com/spring-events-and-transactions-be-cautious-bdb64cb49a95

https://docs.spring.io/spring-framework/docs/4.3.2.RELEASE/javadoc-api/org/springframework/transaction/support/TransactionSynchronization.html#afterCommit--

도움 주신분

우아한테크코스 4기 BE 더즈도 나와 비슷한 고민들을 하고 있어서 대화를 정말 많이 나눴다. 많은 것을 공유해서 좋은 시간이었다. 더즈 즐거웠어! 고마워~!

profile
공병주(Chris)
이전 포스트

비동기 로직 동기 방식으로 테스트하기

다음 포스트

2022년 회고(부제 : 우아한테크코스 회고)

4개의 댓글

comment-user-thumbnail
June
2022년 11월 18일

잘보고가요

1개의 답글