게시글 댓글, 마감완료 알림 기능 구현 #762

jeomxon · 2023-10-17T06:00:15Z

🔥 연관 이슈

close: #743

📝 작업 요약

게시글에 댓글이 달리거나, 작성한 게시글이 마감되었을 때 알림이 저장되고,
해당 알림을 조회할 수 있는 기능을 구현했습니다.

회원가입 시에 리프레시 토큰 ttl을 설정하고 있지 않아서 추가했습니다!

⏰ 소요 시간

2.5일...
기능 구현은 빠르게 끝났는데 테스트가 너무 힘들었네요...
이벤트 및 비동기 테스트 정말 어려운 것 같아요🥲

감기 때문에 컨디션이 너무 안좋네요 ㅠ^ㅠ

🔎 작업 상세 설명

댓글 생성 혹은 게시글이 마감될 때, 이벤트를 발행하고 이를 이벤트 리스너가 받아서 알림을 생성한다.
이를 페이징 처리를 통해 조회한다.

🌟 논의 사항

비동기를 좀 더 효율적으로 처리할 수 있는 방법이 있을까요?

github-actions · 2023-10-17T06:02:43Z

Test Results

440 tests 440 ✔️ 26s ⏱️
143 suites     0 💤
143 files     0 ❌

Results for commit 3ae282d.

♻️ This comment has been updated with latest results.

woo-chang

안녕하세요 저문 :) 다즐입니다 ㅎㅎ

비동기, 이벤트 어려운 개념들이 가득하네요 🥲
저는 한번도 사용해보지 않았는데, 코드를 읽으면서 어느정도 이해해볼 수 있었던 것 같아요!

수정할 부분보다는 모르는 부분에 대한 질문 위주로 남겨보았는데 답변해주시면 감사합니다 ㅎㅎ
몸도 좋지 않은 상황에서 어려운 기능 구현하느라 수고하셨습니다 🙇🏻‍♂️

woo-chang · 2023-10-17T07:10:52Z

backend/src/main/java/com/votogether/domain/alarm/entity/Alarm.java

+    private boolean isChecked;
+
+    @Builder
+    public Alarm(


@Builder를 사용하는 생성자는 일관성 있게 private 접근 제어자를 사용하면 좋을 것 같아요 !

오랜만에 만드는 엔티티라 간과했네요..ㅠㅠ
수정하겠습니다!

woo-chang · 2023-10-17T07:34:06Z

backend/src/main/java/com/votogether/domain/post/service/PostCommentService.java

+                AlarmType.COMMENT,
+                post.getTitle()
+        );
+        applicationEventPublisher.publishEvent(postAlarmEvent);


Q
동일한 타입을 매개변수로 받는 @EventListener가 2개 이상 존재한다면 예외가 발생하나요?

docs에 들어가서 EventListener관련된 내용을 읽어봤을 때는 모든 메서드를 호출한다고 되어있더라구요.
따라서 예외가 발생하지 않는 것으로 이해했습니다.

오 배워갑니다 ㅎㅎ 감사합니다

woo-chang · 2023-10-17T07:36:17Z

backend/src/main/java/com/votogether/domain/alarm/service/AlarmEventListener.java

+    @Transactional(propagation = Propagation.REQUIRES_NEW)
+    public void handlePostAlarmEvent(final PostAlarmEvent postAlarmEvent) {
+        final Alarm alarm = new Alarm(
+                postAlarmEvent.member(),


Q
별도의 스레드에서 동작하기에 member의 필드 중 지연 로딩되는 필드를 접근한다면 예외가 발생하나요?!

저도 이 부분이 궁금하네요.

현재는 member의 필드 중 지연로딩되는 필드가 없어서 직접 확인하기는 어려울 것 같아요.
별도의 스레드에서 동작한다면 지연로딩 시에 문제가 생길 수 있을 것 같긴하네요.
현재는 별도의 스레드에서 별도의 트랜잭션으로 동작하기 때문에 지연로딩이 되지 않을 것 같습니다.

찾아보니 이런 문제는 OSIV 설정을 통해 해결할 수 있다고 합니다.
하나의 요청에 대해서 트랜잭션이 끝나더라도 커넥션을 반납하지 않고 가지고 있기 때문에 예외가 발생하지 않을 수 있을 것 같아요.

woo-chang · 2023-10-17T07:37:32Z

backend/src/main/java/com/votogether/domain/alarm/controller/AlarmController.java

+
+    @GetMapping("/content")
+    public ResponseEntity<List<PostAlarmResponse>> getPostAlarm(
+            @RequestParam @PositiveOrZero(message = "페이지는 0이상 정수만 가능합니다.") final int page


클래스 레벨에 @validated 어노테이션을 붙여야 검증되는 것으로 알고 있는데 검증이 동작하는지 궁금합니다 !

아까 코드리뷰를 하다가 봤는데 수정해야할 것 같아요..^^

woo-chang · 2023-10-17T07:41:06Z

backend/src/main/java/com/votogether/domain/alarm/repository/AlarmRepository.java

+
+public interface AlarmRepository extends JpaRepository<Alarm, Long> {
+
+    Slice<Alarm> findAllByOrderByCreatedAtDesc(final Pageable pageable);


Slice 👍👍

woo-chang · 2023-10-17T07:49:23Z

backend/src/test/java/com/votogether/domain/alarm/service/AlarmEventListenerTest.java

+import org.springframework.test.context.jdbc.Sql.ExecutionPhase;
+import org.springframework.test.context.transaction.TestTransaction;
+
+@Sql(scripts = "classpath:truncate.sql", executionPhase = ExecutionPhase.AFTER_TEST_METHOD)


테스트 종료 시 마다 sql을 실행시켜서 테스트 환경을 초기화시켜주는 방법으로 문제를 해결하셨네요 👍

어려운 작업이었을텐데 수고하셨습니다 🙇🏻‍♂️

deleteAll()을 통해서 데이터를 지워주려고하니 데이터가 삭제되지 않는 문제가 발생하더라구요.
이유를 알 수가 없어서 직접 쿼리문을 날려주는 방식으로 변경해보았어요.

현재 상황에서는 테이블이 추가되는 경우 table truncate의 일관성을 위해 truncate.sql파일을 변경해줘야한다는 단점이 존재할 것 같아요.
더 좋은 방법이 있다면 변경해보고 싶네요.

woo-chang · 2023-10-17T07:50:05Z

backend/src/test/java/com/votogether/domain/alarm/service/AlarmServiceTest.java

+            TestTransaction.flagForCommit();
+            TestTransaction.end();
+
+            Thread.sleep(1000);


여기서 sleep은 비동기 메서드 처리 대기를 위해 어쩔 수 없는 부분일까요? 🥲

현 상황에서는 Thread.sleep()을 해주지 않으면 테스트가 실패하더라구요 ㅠㅠ
뭔가 더 좋은 방법을 찾고 싶었는데, 현재로써는 이게 최선인 것 같아요.
비동기 로직이 많아지고 관련 테스트가 많아지면 문제가 있는 부분이라는 생각은 들어요.
어떻게 수정하면 좋을지 고민해봐야할 것 같아요.

woo-chang · 2023-10-17T07:51:47Z

backend/src/main/java/com/votogether/domain/alarm/controller/AlarmControllerDocs.java

+            @ApiResponse(
+                    responseCode = "200",
+                    description = "게시글 내역 알림 조회 성공"
+            )


@ApiResponse가 단독으로 존재하는 경우 @ApiResponses를 사용하지 않는 것은 어떠신가요? 🤓

페이지 음수 관련 예외가 추가될 것 같아서 해당 부분을 추가하는 것으로 대체하겠습니다

woo-chang · 2023-10-17T07:56:17Z

backend/src/main/java/com/votogether/domain/alarm/service/AlarmEventListener.java

+    private final AlarmRepository alarmRepository;
+
+    @Async
+    @TransactionalEventListener


Q
디폴트값으로는 이전 트랜잭션 커밋 직전에 실행되는 건가요?!

비동기 메서드를 실행했을 때와 비동기 이벤트 로직을 실행할 때 어떤 차이가 존재하고, 이벤트 방식을 왜 선택하셨는지 궁금해요 🤓

첫번째 질문에는 어떤게 실행되는지 다시 한번 달아주시면 답 적어두도록 하겠습니다.

알림 로직을 구현할 때 생각할 수 있는 방식이 두가지가 있었어요.

서비스 간 참조를 통해서 구현

이벤트를 이용한 구현
이 중 이벤트를 이용한 구현을 선택한 이유는 의존관계를 끊어주기 위함이었습니다.
CommentService에서 AlarmService의 로직을 수행하게 된다면 AlarmService에 대한 의존이 생기지만,
이벤트 발행만 해준다면 의존을 제거할 수 있습니다.

비동기로 처리하려고 한 이유는 Requires New옵션을 사용했을 때 데드락 문제가 발생할 수 있기 때문인데요.
애쉬가 글을 정말 잘 작성해주신게 있어서 함께 첨부합니다.
블로그 링크

Thanks to @xxeol2

일단 저는 디폴트 값은 이전 트랜잭션 커밋 이후에 실행되는 것으로 알고 있는데,
다즐의 2번째 질문에 대해선 저도 궁금하네요 :)

woo-chang · 2023-10-17T07:57:22Z

backend/src/main/java/com/votogether/domain/alarm/service/AlarmService.java

+public class AlarmService {
+
+    private static final int BASIC_PAGE_SIZE = 10;
+    private static final String NICKNAME_WHEN_POST_CLOSING = "";


Q
게시글인 경우 빈 값, null 중 빈 값을 보내주기로 했었나요?

네 Empty String을 보내주기로 했습니다.

tjdtls690

정말 수고 많았습니다 저문

몇 가지 리뷰 남겼는데, 질문이 좀 있습니다 ㅎ

tjdtls690 · 2023-10-17T08:12:17Z

backend/src/main/java/com/votogether/domain/alarm/controller/AlarmController.java

+@RequiredArgsConstructor
+@RequestMapping("/alarms")
+@RestController
+public class AlarmController implements AlarmControllerDocs {


조회하는 기능만 있으니 AlarmController -> AlarmQueryController 로 변경하는 것이 좋을 것 같습니다 :)

tjdtls690 · 2023-10-17T08:13:00Z

backend/src/main/java/com/votogether/domain/alarm/controller/AlarmControllerDocs.java

+import org.springframework.web.bind.annotation.RequestParam;
+
+@Tag(name = "알림", description = "알림 API")
+public interface AlarmControllerDocs {


ditto
AlarmControllerDocs -> AlarmQueryControllerDocs

tjdtls690 · 2023-10-17T08:14:08Z

backend/src/main/java/com/votogether/domain/alarm/service/AlarmService.java

+
+@RequiredArgsConstructor
+@Service
+public class AlarmService {


이 부분도 일관성을 위해 AlarmQueryService 로 변경하고 @transactional(readOnly = true)를 메서드가 아닌 클래스에 붙이는 것이 좋을 것 같아요.

tjdtls690 · 2023-10-17T08:38:11Z

backend/src/main/java/com/votogether/domain/alarm/service/AlarmEventListener.java

+    @Transactional(propagation = Propagation.REQUIRES_NEW)
+    public void handlePostAlarmEvent(final PostAlarmEvent postAlarmEvent) {
+        final Alarm alarm = new Alarm(
+                postAlarmEvent.member(),


저도 이 부분이 궁금하네요.

tjdtls690 · 2023-10-17T08:41:11Z

backend/src/main/java/com/votogether/domain/alarm/service/AlarmEventListener.java

+
+    @Async
+    @TransactionalEventListener
+    @Transactional(propagation = Propagation.REQUIRES_NEW)


Q
@async를 통해 비동기로 구현할 땐 꼭 새로운 쓰레드를 만들어서 그 새로 만든 쓰레드를 통해 진행시켜야 하나요? 아니면 같은 쓰레드 내에서도 이게 가능한건가요?

비동기로 구현할 때는 기본적으로 새로운 스레드에서 진행되는 것으로 알고 있습니다.
같은 쓰레드 내에서 한다면 비동기가 어떤 식으로 처리될 수 있을까요?

일단 제가 생각하기에도 없을 것 같은데 혹시나 궁금해서 물어봤습니다 :)

tjdtls690 · 2023-10-17T08:42:09Z

backend/src/main/java/com/votogether/domain/alarm/service/AlarmEventListener.java

+    private final AlarmRepository alarmRepository;
+
+    @Async
+    @TransactionalEventListener


일단 저는 디폴트 값은 이전 트랜잭션 커밋 이후에 실행되는 것으로 알고 있는데,
다즐의 2번째 질문에 대해선 저도 궁금하네요 :)

tjdtls690 · 2023-10-17T09:01:33Z

backend/src/test/java/com/votogether/domain/alarm/service/AlarmEventListenerTest.java

+import org.springframework.test.context.jdbc.Sql.ExecutionPhase;
+import org.springframework.test.context.transaction.TestTransaction;
+
+@Sql(scripts = "classpath:truncate.sql", executionPhase = ExecutionPhase.AFTER_TEST_METHOD)


엄청난 고민의 흔적이 느껴지는 코드 한 줄이네요 👍

tjdtls690 · 2023-10-17T09:03:57Z

backend/src/test/java/com/votogether/domain/alarm/service/AlarmEventListenerTest.java

+        TestTransaction.flagForCommit();
+        TestTransaction.end();


테스트에선 이런식으로 바로 트랜잭션을 커밋하고 종료 시킬 수 있는 기능을 제공하는군요. 👍

tjdtls690 · 2023-10-17T09:04:25Z

backend/src/test/java/com/votogether/domain/alarm/service/AlarmEventListenerTest.java

+        TestTransaction.flagForCommit();
+        TestTransaction.end();
+
+        Thread.sleep(1000);


Q
여기서 1초 기다리는 이유는 이벤트 발행 시간때문인가요?

비동기 응답이 오기 전에 스레드가 종료되는 것을 방지하기 위해서 1초정도의 시간적 여유를 두었습니다.
약 1초 미만의 여유시간이 걸린다는 이야기가 있어서 1초정도로 정했는데,
이 부분은 추후에 더 알아봐야할 것 같아요!

aiaiaiai1

저문 이벤트리스너를 통해서 잘 구현해주셨네요
매번 새로운 기술 적용하시는 모습 멋찝니다 👍
몇가지 궁금한점 커멘트 달았어요!
몸조리 잘하시구 마지막 까지 화이팅입니다~!

aiaiaiai1 · 2023-10-17T10:36:26Z

backend/src/main/java/com/votogether/domain/alarm/repository/AlarmRepository.java

+
+public interface AlarmRepository extends JpaRepository<Alarm, Long> {
+
+    Slice<Alarm> findAllByOrderByCreatedAtDesc(final Pageable pageable);


list대신 slice를 사용하신 이유가 궁금해요~

페이징처리를 하기 위해서 Page, Slice 두가지의 선택지를 생각했었는데,
Page는 count쿼리가 추가적으로 나가기 때문에, 불필요한 count쿼리를 막고자 Slice를 선택했습니다.

aiaiaiai1 · 2023-10-17T10:40:03Z

backend/src/main/java/com/votogether/domain/alarm/service/AlarmService.java

+public class AlarmService {
+
+    private static final int BASIC_PAGE_SIZE = 10;
+    private static final String NICKNAME_WHEN_POST_CLOSING = "";


P3
변수이름을 BLANK로 해두어도 될거 같은데요?! 개인적 의견이니 저문 편하신대로 하시면 될거같습니다!

Blank로 바꾸는 경우, 예를들어 게시글 마감 시에 주는 값이 blank가 아니라 null로 변경된다면
상수의 값 뿐만 아니라 변수명도 변경되어야한다고 생각했습니다.
따라서 게시글 마감 시에 보내주는 닉네임이라는 뜻의 변수명을 가지고 값만 변경할 수 있도록 지정했습니다.

aiaiaiai1 · 2023-10-17T10:58:12Z

backend/src/main/java/com/votogether/domain/alarm/service/AlarmEventListener.java

+    private final AlarmRepository alarmRepository;
+
+    @Async
+    @TransactionalEventListener


aiaiaiai1 · 2023-10-17T13:33:53Z

backend/src/main/java/com/votogether/domain/post/controller/PostCommandController.java

@@ -25,7 +25,7 @@
 @RequiredArgsConstructor
 @RequestMapping("/posts")
 @RestController
-public class PostCommandController {
+public class PostCommandController implements PostCommandControllerDocs {


aiaiaiai1 · 2023-10-17T13:45:01Z

backend/src/test/java/com/votogether/domain/alarm/service/AlarmServiceTest.java

+        @Test
+        @DisplayName("댓글을 생성할 때 가능하다.")
+        void whenCreateComment() {
+            // given
+            Member commentWriter = memberTestPersister.builder().save();
+            Post post = postTestPersister.postBuilder().save();
+            CommentCreateRequest commentCreateRequest = new CommentCreateRequest("댓글입니다.");
+
+            postCommentService.createComment(post.getId(), commentCreateRequest, commentWriter);
+
+            TestTransaction.flagForCommit();
+            TestTransaction.end();
+
+            // when
+            List<PostAlarmResponse> postAlarmResponses = alarmService.getPostAlarm(0);
+
+            // then
+            PostAlarmResponse postAlarmResponse = postAlarmResponses.get(0);
+            PostAlarmDetailResponse postAlarmDetailResponse = postAlarmResponse.detail();
+            assertSoftly(softly -> {
+                softly.assertThat(postAlarmResponses).hasSize(1);
+                softly.assertThat(postAlarmResponse.isChecked()).isEqualTo(false);
+                softly.assertThat(postAlarmDetailResponse.postTitle()).isEqualTo("title");
+            });
+        }


Q.
댓글 알림은 sleep이 없어도 테스트가 통과되는데 게시글 마감알림과 어떤 차이가 있어서 sleep이 없어도 되는건가요??

++ 회원가입 탈퇴할때 관련 알림도 삭제해야할거같네요 😱

로컬에서는 통과가 되지만 github actions에서는 통과가 되지 않는 문제가 있습니다!
현재 비동기 로직이 들어가는 부분에는 터지는 상황이 발생하면 sleep을 걸어주고 있었습니다 ㅠㅠ

회원탈퇴할 때 알림삭제는 생각도 못했는데 얼른 추가하겠습니다..!

tjdtls690

수고하셨습니다 저문!
ㅡ 문 ㅡ

woo-chang

리뷰 반영 확인했습니다 ㅎㅎ 수고하셨어요 👍

aiaiaiai1

확인했습니다~! 고생하셨어요!

jeomxon added 12 commits October 16, 2023 01:27

feat: (#743) 알람 엔티티 구현

c2a00f9

feat: (#743) 댓글 작성 시 이벤트 발행 로직 추가

f0d9f1b

feat: (#743) 댓글 작성 이벤트 핸들러 구현

2967e04

feat: (#743) 게시글 내역 알림 조회 기능 구현

b69220f

refactor: (#743) 게시글 내역 알림 조회 로직 메서드 분리 및 가독성 개선

c717ae3

refactor: (#743) 패키지 구조 수정

4a99b4e

refactor: (#743) 회원가입 시 저장하는 리프레시 토큰의 ttl 설정

1e6b88e

refactor: (#743) 응답 필드명 수정 및 일부 검증 추가

119a711

test: (#743) AlarmEventListener의 handle메서드 검증 추가

6843ea2

chore: (#743) swagger 명세 관련 어노테이션 추가

5bc36a4

test: (#743) AlarmController 검증 추가

6f46197

test: (#743) 이벤트 및 비동기 테스트 환경 격리

bec6dfe

jeomxon added BE 🐾 개발 labels Oct 17, 2023

jeomxon added this to the 6차 데모데이 milestone Oct 17, 2023

jeomxon requested review from woo-chang, tjdtls690 and aiaiaiai1 October 17, 2023 06:00

jeomxon self-assigned this Oct 17, 2023

jeomxon added 3 commits October 17, 2023 15:05

test: (#743) 이벤트 리스너 비동기 처리를 위한 대기시간 설정

ae04576

test: (#743) 테스트 필드 접근 제어자 제거

4ae1221

refactor: (#743) 알림 조회 정렬 추가

d440984

woo-chang requested changes Oct 17, 2023

View reviewed changes

tjdtls690 requested changes Oct 17, 2023

View reviewed changes

aiaiaiai1 requested changes Oct 17, 2023

View reviewed changes

jeomxon added 4 commits October 18, 2023 10:36

refactor: (#743) 생성자 접근제어자 변경

e525b03

refactor: (#743) docs 예외사항 추가 및 검증 어노테이션 추가

f110aba

refactor: (#743) query, command 분리

3378a46

feat: (#743) 회원 탈퇴 시 알림 데이터 삭제 기능 추가

3ae282d

tjdtls690 approved these changes Oct 18, 2023

View reviewed changes

woo-chang approved these changes Oct 18, 2023

View reviewed changes

aiaiaiai1 approved these changes Oct 18, 2023

View reviewed changes

jeomxon merged commit eb9b650 into dev Oct 18, 2023
3 checks passed

jeomxon deleted the feat/#743 branch October 18, 2023 03:25


		public interface AlarmRepository extends JpaRepository<Alarm, Long> {

		Slice<Alarm> findAllByOrderByCreatedAtDesc(final Pageable pageable);

게시글 댓글, 마감완료 알림 기능 구현 #762

게시글 댓글, 마감완료 알림 기능 구현 #762

Conversation

jeomxon commented Oct 17, 2023

🔥 연관 이슈

📝 작업 요약

⏰ 소요 시간

🔎 작업 상세 설명

🌟 논의 사항

github-actions bot commented Oct 17, 2023 • edited Loading

Test Results

woo-chang left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

tjdtls690 left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

jeomxon Oct 18, 2023 • edited Loading

Choose a reason for hiding this comment

aiaiaiai1 left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

tjdtls690 left a comment

Choose a reason for hiding this comment

woo-chang left a comment

Choose a reason for hiding this comment

aiaiaiai1 left a comment

Choose a reason for hiding this comment

github-actions bot commented Oct 17, 2023 •

edited

Loading

jeomxon Oct 18, 2023 •

edited

Loading