refactor: ETA 목록 조회 API web socket으로 수정 #531

eun-byeol · 2024-09-12T09:19:59Z

🚩 연관 이슈

#482

📝 작업 내용

약속 참여자 도착 현황 조회 API -> 웹소켓으로 이전

🏞️ 스크린샷 (선택)

웹소켓 동작 흐름도 (오디톡 예정)

🗣️ 리뷰 요구사항 (선택)

웹 소켓 로직 위주로 확인 부탁드립니다.
추후 동시성 테스트 및 에러 핸들링 이슈 추가 예정입니다.

+) merge 후 몇가지 깨지는 테스트가 있어 우선 더럽지만 기능 동작 확인 위주로 돌아가게끔만 테스트 구현해놓았습니다. 테스트 리팩터링은 새로 이슈파서 진행할 예정입니다.

Co-authored-by: coli-geonwoo <[email protected]>

github-actions · 2024-09-12T09:22:28Z

Test Results

143 tests +4 136 ✅ +4 46s ⏱️ +42s
42 suites +1 7 💤 ±0
42 files +1 0 ❌ ±0

Results for commit 6d3e3e8. ± Comparison against base commit e4df144.

This pull request removes 6 and adds 10 tests. Note that renamed tests count towards both.

com.ody.auth.JwtTokenProviderTest$validateAccessToken ‑ [1] accessToken=com.ody.auth.token.AccessToken@4042399c
com.ody.auth.JwtTokenProviderTest$validateAccessToken ‑ [2] accessToken=com.ody.auth.token.AccessToken@5dcb5eb6
com.ody.common.validator.FutureOrPresentDateTimeValidatorTest ‑ [1] date=2024-09-12, time=07:53:55.548551274, expected=false
com.ody.common.validator.FutureOrPresentDateTimeValidatorTest ‑ [2] date=2024-09-12, time=08:53:55.548582101, expected=true
com.ody.common.validator.FutureOrPresentDateTimeValidatorTest ‑ [3] date=2024-09-12, time=06:53:55.548569007, expected=false
com.ody.common.validator.FutureOrPresentDateTimeValidatorTest ‑ [4] date=2024-09-13, time=07:53:55.548551274, expected=true

com.ody.auth.JwtTokenProviderTest$validateAccessToken ‑ [1] accessToken=com.ody.auth.token.AccessToken@5fea7c5c
com.ody.auth.JwtTokenProviderTest$validateAccessToken ‑ [2] accessToken=com.ody.auth.token.AccessToken@5e171446
com.ody.common.validator.FutureOrPresentDateTimeValidatorTest ‑ [1] date=2024-09-15, time=18:14:12.086844640, expected=false
com.ody.common.validator.FutureOrPresentDateTimeValidatorTest ‑ [2] date=2024-09-15, time=19:14:12.086893091, expected=true
com.ody.common.validator.FutureOrPresentDateTimeValidatorTest ‑ [3] date=2024-09-15, time=17:14:12.086878854, expected=false
com.ody.common.validator.FutureOrPresentDateTimeValidatorTest ‑ [4] date=2024-09-16, time=18:14:12.086844640, expected=true
com.ody.eta.controller.EtaSocketControllerTest ‑ /publish/open/{meetingId} 호출 시 1초 뒤에 위치 호출 함수가 예약된다
com.ody.eta.controller.EtaSocketControllerTest ‑ /topic/etas/{meetingId}에 구독한 사람들이 요청을 받는다
com.ody.eta.controller.EtaSocketControllerTest ‑ 약속 시간 이후, /publish/etas/{meetingId} 호출 시 disconnect 트리거를 당긴다.
com.ody.eta.controller.EtaSocketControllerTest ‑ 호출 한지 10초가 지난 경우, 다시 update 요청을 예약한다.

♻️ This comment has been updated with latest results.

github-actions · 2024-09-13T15:10:47Z

📝 Test Coverage Report

Overall Project	79.43% `-0.82%`	❌
Files changed	83.65%	🍏

File	Coverage
EtaSocketController.java	100%	🍏
WebSocketConfig.java	100%	🍏
EtaSocketService.java	95.17% `-4.83%`	🍏
WebSocketArgumentResolver.java	0%	❌

hyeon0208

웹소켓 동작 흐름도를 깔끔하게 그려주셔서 흐름이 이해가 잘 됐어요 ~ 👍
웹 소켓 설정과 어노테이션도 생각보단 간단하네요

eta 목록 조회를 10초마다 스케줄링하셨는데 이 부분은 동시성 문제가 발생할 확률이 높아서 비동기 처리를 고려해야 할것 같아요

몇가지 질문만 남기고 어프로브 드릴게요 !

질문

10초마다면 갱신되는 주기가 생각보다 길어서 사용자 입장에선 불편할 것 같은데 10초인 이유가 있나요 ?
에러 핸들링 이슈가 있다고 했는데 이슈가 동작에 문제가 있는 건가요?

hyeon0208 · 2024-09-14T05:46:05Z

backend/src/main/java/com/ody/eta/argumentresolver/WebSocketArgumentResolver.java

+    public Object resolveArgument(
+            MethodParameter parameter,
+            Message<?> message
+    ) {


[필수]

Suggested change

public Object resolveArgument(

MethodParameter parameter,

Message<?> message

) {

public Object resolveArgument(MethodParameter parameter, Message<?> message) {

개행하지 않아도 될 것 같아요 😃

반영했습니다!

hyeon0208 · 2024-09-14T06:14:15Z

backend/src/test/java/com/ody/eta/controller/EtaSocketControllerTest.java

+        verify(taskScheduler, times(2))
+                .schedule(any(Runnable.class), any(Instant.class));
+    }
+//    }


[필수]

Suggested change

// }

반영했습니다!

hyeon0208 · 2024-09-14T06:23:17Z

backend/src/main/java/com/ody/eta/config/WebSocketConfig.java

+    @Override
+    public void registerStompEndpoints(StompEndpointRegistry registry) {
+        registry.addEndpoint("/connect")
+                .setAllowedOrigins("*");


[질문]
cors 방지를 위해 추가해주신 것 같은데
안드로이드 쪽에서 요청을 보내는 주소의 origin이 계속 바뀌나요 ??

우선 모든 요청지로부터의 요청을 허용한다는 설정으로만 알고 있는데 CORS와 관련된 부분인지 조금 더 공부해볼게요! socket통신이 처음이다보니 저도 익숙치 않은 설정이 많네요 😢

안드와 이야기해보고 와일드카드 사용하지 않는 방향으로 수정할게요!

coli-geonwoo · 2024-09-15T18:17:21Z

비동기 처리를 고려해야 할것 같아요

#536 PR에 동시성 테스트와 비동기 처리를 진행하였습니다

10초마다면 갱신되는 주기가 생각보다 길어서 사용자 입장에선 불편할 것 같은데 10초인 이유가 있나요 ?

기준은 조조와 깊게 논의하지 못했어요. 우선 돌아가게끔 10초로 구현을 한 이후 서버가 버틸 수 있는 선 안에서 점차 줄여나가는 방식이 좋을 것 같아요. 카키는 유저 경험 상 몇 초가 적당하다고 생각하시나요? 🤔

에러 핸들링 이슈가 있다고 했는데 이슈가 동작에 문제가 있는 건가요?

stomp 웹 통신 과정에서 에러가 발생하면 에러를 핸들링할 errorHandler를 지정할 수 있습니다. 저희가 tossPayment 연결 과정에서 직접 ResponseErrorHandler를 구현해주었던 것 처럼요!

Stomp에서도 각 에러 상황에 따라 400으로 반환할지 500으로 반환할지에 대한 처리를 직접 커스텀 에러 핸들러를 지정하여 처리해주어야 하는데 아직 웹 소켓 구현만 했을 뿐 그 부분까지 구현이 되지 않았다는 이야기였습니다!

hyeon0208 · 2024-09-16T04:25:49Z

코멘트로 친절한 설명 감사해요 ~
질문에 대한 답변 코멘트 남깁니다

기준은 조조와 깊게 논의하지 못했어요. 우선 돌아가게끔 10초로 구현을 한 이후 서버가 버틸 수 있는 선 안에서 점차 줄여나가는 방식이 좋을 것 같아요. 카키는 유저 경험 상 몇 초가 적당하다고 생각하시나요? 🤔

웹 소켓은 완벽한 실시간성 보장이 필요할 때 적합하다 생각하는데
10초마다 스케줄링되어 갱신된다면 웹소켓을 적용하는 장점이 없는 것 같아서 질문 남겼어요 🤔
이 부분에 대해서는 다같이 고민해봐야겠네요

mzeong

동작 흐름도가 이해에 많은 도움이 되었어요 👍 웹소켓 구현하느라 고생했어요 🐥🥦

질문과 제안을 남겼는데, 제안은 웹소켓 리팩터링이 예정되어 있다고 해서 만약 반영한다면 그때 반영을 고려해주시면 될 것 같아요!!

mzeong · 2024-10-10T00:39:13Z

backend/src/main/java/com/ody/eta/service/EtaSocketService.java

+        taskScheduler.schedule(
+                () -> template.convertAndSend("topic/coordinates/" + meetingId, ""), startTime
+        );
+        LATEST_TRIGGER_TIME_CACHE.put(meetingId, LocalDateTime.now());


[질문] 스케줄링 성공과 무관하게 latest trigger time을 업데이트해줘도 되나요?
startTime이 아니라 now로 업데이트하는 이유는 무엇인가요?

스케쥴링에 실패하면 lastest trigger time cache에 시간을 넣기 이전에 에러가 반환되므로 업데이트가 되지 않습니다.

lastest trigger time은 상태를 업데이트 한 가장 최근 시각을 의미합니다. 즉, trigger 작업 [eta 상태 업데이트 -> 10초 뒤 스케쥴링 예약 ]-> 최근 업데이트 시간인 현재 시간으로 cache 저장의 순서를 따르고 있어요.

mzeong · 2024-10-10T00:40:30Z

backend/src/main/java/com/ody/eta/service/EtaSocketService.java

+    private boolean isTimeToSchedule(Long meetingId) {
+        LocalDateTime lastTriggerTime = LATEST_TRIGGER_TIME_CACHE.get(meetingId);
+        Duration duration = Duration.between(lastTriggerTime, LocalDateTime.now());
+        return duration.toSeconds() >= 10;


[제안] 상수로 둬도 좋을 것 같네요

mzeong · 2024-10-10T00:42:06Z

backend/src/main/java/com/ody/eta/service/EtaSocketService.java

+    private boolean isOverMeetingTime(Long meetingId) {
+        LocalDateTime meetingTime = MEETING_TIME_CACHE.get(meetingId);
+        LocalDateTime lastTriggerTime = meetingTime.plusMinutes(1L);
+        return TimeUtil.nowWithTrim().isAfter(lastTriggerTime);
+    }


[질문] 약속 시간이 지나면 더 이상 웹소켓 연결을 유지하지 않기 위해 검증하는 것 같은데 meetingTime.plusMinutes(1L)을 왜 lastTriggerTime이라는 변수명으로 받나요? 의미가 이해가 잘 안 가요

변수명에 대해서는 다시 생각해보는 게 맞을 것 같에요. lastCallTime, endCallTime 등의 변수명이 더 적절하다고 느껴지네요.

약속 시간 +1 분을 하는 이유는 약속 시간 이후에도 1분간 상태 업데이트가 되어야 지각 위기/ 도착 예정 -> 지각 / 도착 여부가 판별되기 때문입니다. 따라서 약속 시간 1분후 까지 eta 상태를 계속해서 업데이트하고 요청 시각이 1분후를 지났다면 disconnect 요청을 보내는 방식입니다.
+

eun-byeol and others added 8 commits September 6, 2024 17:58

feat: 웹 소켓 관련 설정 작성

99c5da6

Co-authored-by: coli-geonwoo <[email protected]>

feat: 참여자 Eta 목록 반환 로직 구현

2614c14

Co-authored-by: coli-geonwoo <[email protected]>

feat: eta 요청 트리거 스케줄링 구현

ba206f4

Co-authored-by: coli-geonwoo <[email protected]>

fix: enum 제약조건 대문자로 수정

cc78568

Co-authored-by: coli-geonwoo <[email protected]>

test: 웹 소켓 테스트 작성

927bcf1

test: 웹 소켓 테스트 동시성, no destination 오류 수정

df70539

Co-authored-by: coli-geonwoo <[email protected]>

refactor: EtaSocketService 로직 분리

dc8c54a

Co-authored-by: coli-geonwoo <[email protected]>

chore: h2, datasource 설정 제거

0bbd51a

Co-authored-by: coli-geonwoo <[email protected]>

eun-byeol linked an issue Sep 12, 2024 that may be closed by this pull request

refactor: ETA 목록 조회 API web socket으로 수정 #482

Open

5 tasks

eun-byeol requested review from hyeon0208 and mzeong September 12, 2024 09:20

eun-byeol assigned eun-byeol and coli-geonwoo Sep 12, 2024

eun-byeol added BE REFACTOR labels Sep 12, 2024

Merge branch 'develop' into feature/482

3ab40b7

test: 웹 소켓 테스트 오류 해결

bf35c8b

hyeon0208 requested changes Sep 14, 2024

View reviewed changes

hyeon0208 approved these changes Sep 14, 2024

View reviewed changes

style: 컨벤션 준수

6d3e3e8

mzeong approved these changes Oct 10, 2024

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

refactor: ETA 목록 조회 API web socket으로 수정 #531

refactor: ETA 목록 조회 API web socket으로 수정 #531

eun-byeol commented Sep 12, 2024 •

edited by coli-geonwoo

Loading

github-actions bot commented Sep 12, 2024 •

edited

Loading

github-actions bot commented Sep 13, 2024 •

edited

Loading

hyeon0208 left a comment •

edited

Loading

hyeon0208 Sep 14, 2024

coli-geonwoo Sep 15, 2024

hyeon0208 Sep 14, 2024

coli-geonwoo Sep 15, 2024

hyeon0208 Sep 14, 2024

coli-geonwoo Sep 15, 2024

eun-byeol Sep 16, 2024

coli-geonwoo commented Sep 15, 2024

hyeon0208 commented Sep 16, 2024

mzeong left a comment

mzeong Oct 10, 2024

coli-geonwoo Oct 11, 2024

mzeong Oct 10, 2024

mzeong Oct 10, 2024

coli-geonwoo Oct 11, 2024

refactor: ETA 목록 조회 API web socket으로 수정 #531

Are you sure you want to change the base?

refactor: ETA 목록 조회 API web socket으로 수정 #531

Conversation

eun-byeol commented Sep 12, 2024 • edited by coli-geonwoo Loading

🚩 연관 이슈

📝 작업 내용

🏞️ 스크린샷 (선택)

웹소켓 동작 흐름도 (오디톡 예정)

🗣️ 리뷰 요구사항 (선택)

github-actions bot commented Sep 12, 2024 • edited Loading

Test Results

github-actions bot commented Sep 13, 2024 • edited Loading

📝 Test Coverage Report

hyeon0208 left a comment • edited Loading

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

coli-geonwoo commented Sep 15, 2024

hyeon0208 commented Sep 16, 2024

mzeong 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

eun-byeol commented Sep 12, 2024 •

edited by coli-geonwoo

Loading

github-actions bot commented Sep 12, 2024 •

edited

Loading

github-actions bot commented Sep 13, 2024 •

edited

Loading

hyeon0208 left a comment •

edited

Loading