Refactor: 포인트관련 로직을 포인트 서비스계층으로 통합, 개별 트랜잭션 적용, 테스트 코드 개선 #132

zzoe2346 · 2024-10-31T16:38:49Z

#️⃣ 연관된 이슈

ex) #이슈번호, #이슈번호

📝 작업 내용

이번 PR에서 작업한 내용을 간략히 설명해주세요.(이미지 첨부 가능)

포인트관련 로직을 포인트 서비스계층으로 통합
- 콜백과 안부전화에서 포인트 관련 로직을 포인트 서비스 계층으로 옮겼습니다
스케쥴되는 메서드에서 콜백 상태 변환에 개별 트랜잭션 적용하였습니다.
- 기존에는 트랜잭션이 길어질 가능성이 존재했는데 이제 개별 트랜잭션 적용으로 이 부분 해소
- !!! 그런데 이렇게 하면 DB I/O 가 많아지는 단점이 있을거 같긴합니다...혹시 의견있으시면 주시면 좋겠습니담.
콜백 테스트 코드 추가 및 가독성 개선
- @Nested 를 써서 테스트 코드 파악이 더 쉽게 해보았습니다
- 콜백 도메인에서 테스트 못한 것들 모두 추가완료 하였습니다.
콜백 서비스에서 불필요한 private 메서드 통합
사소히 변경된것들
- 메서드 명 변경
- @Transactional(readOnly = true) 추가
10분마다 완료대기상태가 2일 지난 콜백 완료로 자동전환해주는 로직의 SQL 수정

findAllByStatusAndPendingCompleteTimeBefore ->findAllByStatusAndPendingCompleteTimeBetween

Hibernate: 
    select
        c1_0.id,
        c1_0.assigned_member_id,
        c1_0.pending_complete_time,
        c1_0.post_time,
        c1_0.senior_id,
        c1_0.status 
    from
        callback c1_0 
    where
        c1_0.status=? 
        and c1_0.pending_complete_time between ? and ?

스크린샷 (선택)

💬 리뷰 요구사항(선택)

리뷰어가 특별히 봐주었으면 하는 부분이 있다면 작성해주세요

ex) 메서드 XXX의 이름을 더 잘 짓고 싶은데 혹시 좋은 명칭이 있을까요?

⏰ 현재 버그

✏ Git Close

close #이슈번호

close #115

- 메모리 소유 기간 줄일 수 있다 - 유저에게 더욱 신속한 피드백 할 수 있다. - 예외 발생에 전보다 덜 취약.

…p3/Team8_BE into Refactor-#115

GitJIHO

메서드명이 명확하고 깔끔해서 알아보기 좋은 것 같아요~~
@nested 사용은 처음보는데 기능적으로나 코드 가독성적으로나 많이 좋아보이네요 👍

스케쥴되는 메서드에서 콜백 상태 변환에 개별 트랜잭션 적용하였습니다.
기존에는 트랜잭션이 길어질 가능성이 존재했는데 이제 개별 트랜잭션 적용으로 이 부분 해소
!!! 그런데 이렇게 하면 DB I/O 가 많아지는 단점이 있을거 같긴합니다...혹시 의견있으시면 주시면 좋겠습니담.

관련하여 의견남겼습니다 :)

GitJIHO · 2024-11-02T04:53:07Z

src/main/java/com/example/sinitto/callback/service/CallbackService.java

    public void changeOldPendingCompleteToCompleteByPolicy() {

        LocalDateTime referenceDateTimeForComplete = LocalDateTime.now().minusDays(DAYS_FOR_AUTO_COMPLETE);

-        List<Callback> callbacks = callbackRepository.findAllByStatusAndPendingCompleteTimeBefore(Callback.Status.PENDING_COMPLETE, referenceDateTimeForComplete);
+        List<Callback> callbacks = callbackRepository.findAllByStatusAndPendingCompleteTimeBetween(Callback.Status.PENDING_COMPLETE, referenceDateTimeForComplete.minusMinutes(5), referenceDateTimeForComplete.plusMinutes(5));


GitJIHO · 2024-11-02T04:54:30Z

src/main/java/com/example/sinitto/callback/service/CallbackService.java

+        try {
+            pointService.deductPoint(senior.getMember().getId(), CALLBACK_PRICE, PointLog.Content.SPEND_COMPLETE_CALLBACK);
+        } catch (Exception e) {
+            return TwilioHelper.convertMessageToTwiML(FAIL_MESSAGE_NOT_ENOUGH_POINT);
+        }


point관련 로직이라 포괄적인 try-catch문 사용 좋은 것 같습니다 👍

GitJIHO · 2024-11-02T04:59:25Z

src/test/java/com/example/sinitto/callback/service/CallbackServiceTest.java

@nested가 여러개의 내부 클래스들로 그룹화하는 느낌인가보네요..!
코드가 길어서 효율적인 방법인 것 같습니다 👍 배워갑니다 ㅎㅎ
다시금 느끼지만 서비스 테스트코드 엣지케이스들까지 너무 잘 작성된 것 같아요 ~~~

GitJIHO · 2024-11-02T05:09:40Z

src/main/java/com/example/sinitto/callback/service/CallbackService.java

+            completeCallbackIndividually(callback);
        }
    }

+    @Transactional
+    public void completeCallbackIndividually(Callback callback) {
+
+        pointService.earnPoint(callback.getAssignedMemberId(), CALLBACK_PRICE, PointLog.Content.COMPLETE_CALLBACK_AND_EARN);
+        callback.changeStatusToComplete();
+    }
+


개인적으로는
공통 트랜잭션 상태일 때 여러 완료상태로 가야할 콜백 중 하나의 문제만으로도 모든 변화가 롤백되어 재수행해야 하는 상황을 고려했을 때를 생각해본다면
해당 개별 트랙잭션이 현재까지 진행된 콜백 상태변화는 저장되어 재수행하지 않아도 된다는 점에서 얻는 이점이 DB I/O가 잦아진다는 단점보다 더 크다는 생각이 들어 좋은 것 같습니다.
다만 개별 트랙잭션 로직 안에 엔티티 메서드 수행은 오버헤드가 크지 않을 것 같은데 pointService 메서드 호출은 오버헤드가 상대적으로 높지 않을까 걱정이네요..! 스프링이 해결해주려나요...??!

의견 감사합니다! 지호님 말씀대로 개별 트랜잭션이 콜백 상태를 좀 신속히? 저장시켜 좋은거 같네요.

pointService 메서드 호출에 관한 말씀은 제가 잘 이해했는지 모르겠는데 나름 찾아본거 공유해드려보면, JVM 에서 최적화도 해주는것도 있고 메모리에 이미 올라가있는 객체의 메서드를 참조하는 방식이라 오버헤드가 발생하긴 하지만 엄청 크지는 않을걸로 예상이 됩니다. 또한 이 방식이 다소 리소스를 더 먹더라도 개발자 입장에서 유지보수 측면에서의 이득이 더 크다라는 관점에서 보면 좋은점도 있는거 같아요! 장단점이 다있어 보여요.

아니면 pointService 메서드 호출에서 포인트 DB 관련해서 오버헤드를 말씀하신 걸까요? 요 부분은 콜백 상태변화랑 포인트 적립/차감이 땔 수 없는 부분이라 어쩔수 없을거 같기는 합니다.🥲

JVM 에서 최적화도 해주는것도 있고 메모리에 이미 올라가있는 객체의 메서드를 참조하는 방식이라 오버헤드가 발생하긴 하지만 엄청 크지는 않을걸로 예상이 됩니다

아하! 하나 배워갑니다 감사합니다 ㅎㅎ 해당 의견 관련한 부분 남긴게 맞습니다 👍

2iedo

포인트 관련 로직 수정 및 테스트 코드 수정한 부분 확인했습니다! 고생하셨어요~~

2iedo · 2024-11-02T16:16:23Z

src/main/java/com/example/sinitto/callback/service/CallbackService.java

-
-            earnPointForSinitto(callback.getAssignedMemberId());
-            callback.changeStatusToComplete();
+            completeCallbackIndividually(callback);
        }
    }



transaction의 크기를 줄일 수 있어서 좋은 것 같습니다!!

2iedo · 2024-11-02T16:19:39Z

src/main/java/com/example/sinitto/helloCall/service/HelloCallService.java

기존에는 PointRepository만 불러왔었는데, PointService 서비스 레이어를 불러오게 되면서 Repository를 불러올 때보다 비용이나 의존성이 커지진 않을까 라는 생각이 들긴 하네용...
그래도 포인트 관련 서비스는 하나로 합친 것 자체는 가독성 면에서 좋은 것 같습니다!

저도 step2 때 서비스에서 서비스 레이어를 의존성 주입하는 건 피하는 것이 좋다고 배워서 걱정스럽긴 하네용...
이 부분 나중에 여유있으시면 멘토님께 여쭤봐도 좋을 것 같아요!!

eunsoni

수고하셨습니다~!

eunsoni · 2024-11-03T06:43:07Z

src/main/java/com/example/sinitto/helloCall/service/HelloCallService.java

저도 step2 때 서비스에서 서비스 레이어를 의존성 주입하는 건 피하는 것이 좋다고 배워서 걱정스럽긴 하네용...
이 부분 나중에 여유있으시면 멘토님께 여쭤봐도 좋을 것 같아요!!

…p3/Team8_BE into Refactor-#115

@onDelete

* Fix, Refactor : refreshToken 만료 시 InvalidJwtException 반환하도록 변경, accessToken의 만료 시간 수정 (#139) * fix : InvalidjwtException 에러 던지도록 수정 * refactor : accessToken의 만료시간 5분으로 수정 * Feat: helloCallDetailResponse에 ServiceTime 필드 추가 (프론트 요청) (#143) feat: 안부전화 상세보기 응답필드에 serviceTime추가 * Refactor: 카카오 DTO 4차 코드리뷰 반영 (#141) * Feat : 각 도메인별 테스트 코드 추가 및 수정 및 각 멤버 별(보호자, 시니또) 할 수 있는 행동 제약 추가 (#131) * feat : 보호자만 시니어 등록할 수 있도록 createSenior 메소드 수정 * refactor : 시니또 제거하는 메소드 삭제 * Test : Guard 도메인의 서비스 레이어 테스트 코드 작성. update 시 테스트를 위해 guardService 클래스 수정 * test : Sinitto 도메인에 대한 서비스 레이어의 테스트 코드 작성 * refactor : verify 메소드 사용하여 테스트 코드 수정 * test : GuardGuideline 도메인의 서비스 레이어 테스트 코드 작성 * refactor : accessToken 만료시간 10분으로 변경 * refactor : accessToken의 만료시간 5분으로 변경 * refactor : refreshToken 만료 시 InvalidJwtException 에러 발생하도록 변경 * test : auth 도메인의 서비스 레이어 테스트 코드 작성 * refactor : 오탈자 수정 * refactor : 변수명 변경 * test : Member 도메인의 서비스 레이어에 대한 테스트 코드 작성 * test : HelloCall 도메인의 서비스 레이어 테스트 코드 작성 * style : intellij로 코드 서식 변경 * refactor : 사용하지 않는 mock 객체 삭제 * refactor : update 메소드 관련 테스트 코드 수정 * Refactor: 포인트 도메인 운영코드 개선과 테스트코드 개선 및 추가 (#144) * refactor: 포인트 어드민 컨트롤러에 있던 서비스로직 포인트 어드민 서비스 클래스로 옮김 * test: 포인트 어드민 서비스 테스트 * test: 포인트 서비스 테스트 추가 및 개선 * refactor: 포인트 출금 요청시 로그에는 출금 요청한 포인트 그대로 찍히도록 변경. 단, 관리자페이지에서는 수수료가 적용된 포인트로 보이도록 함. * refactor: 관리자 페이지 약간의 수정,css 더 깔끔히 수정 * refactor: 관리자 페이지 가독성을 위해 LocalDateTime을 포맷팅해서 String으로 등록 시간을 표현한다. * fix: 테스트 정상 통과하도록 수정 * chore: 버튼 이름 수정 * Deploy: 무중단 배포 구현 중 health check를 위한 dependency 추가 (#146) deploy: health check를 위한 dependency 추가 * Fix: 배포 과정중 dependency 에러처리 (#148) fix: dependency 추가 * Test: 무중단배포 테스트를 위한 pr (#150) * fix: dependency 추가 * test: 무중단배포 테스트를 위한 값 변경 * HotFix: 토큰 보안로직 추가 구현 (#152) * fix: 엑세스 토큰과 리프레쉬 토큰 구별 claim값 및 검증로직 추가 * fix: 테스트코드 오류 수정 * HotFix: swagger server 환경따라 변경 및 도메인 상수화 구현 (#154) * swagger 환경변수 확인하여 다른 server에서 try it out 하도록 구현 * refactor: 구별용 url 상수화 * fix: 테스트코드 오류 수정 * Refactor: 포인트관련 로직을 포인트 서비스계층으로 통합, 개별 트랜잭션 적용, 테스트 코드 개선 (#132) * refactor: 불필요한 private method 통합 * refactor: 개별 트랜잭션으로 변경 - 메모리 소유 기간 줄일 수 있다 - 유저에게 더욱 신속한 피드백 할 수 있다. - 예외 발생에 전보다 덜 취약. * refactor: 포인트 관련 로직 서비스계층으로 옮김 * test: 테스트 추가하고, 가독성 개선 * refactor: SQL 수정 * test: 안부전화 서비스에서 포인트 로직 서비스층으로 통합으로 인한 수정 * remove: 깃헙 액션 테스트 통과를 위한 수정 * Deploy: 프론트 배포주소 변경에 따른 상수화값 변경 (#157) * deploy: 프론트 배포주소 변경에 따른 서버 변경 * test: 테스트 mock값 변경 * hotfix : Exception 변경 (#160) * Feat: 카카오메시지 전송 프로토타입 생성 완료 (#161) feat: 카카오 메시지 (포인트 충전 접수, 포인트 충전 완료, 포인트 출금 접수, 포인트 출금 완료) 추가 * HotFix: 카카오메시지 형식 수정 완료 (#164) * fix: 수수료 변경 * fix: 카카오메시지 형식 수정 * HotFix : refresh Token 1분 이내에 발급되었을 경우 다시 발급되지 않게 수정 (#168) * feat, hotfix : 회원탈퇴 api 추가 및 redisTemplate 수정 (#170) * feat, hotfix : 회원탈퇴 api 추가 및 redisTemplate 수정 * feat : 경로 추가 * Deploy: DB mysql로 마이그레이션 구현 (#171) * deploy: h2 -> mysql 마이그레이션 * test: 내부 저장소를 사용하지 않고 mysql을 사용하도록 수정 * test: 테스트 mock 추가 * git action 테스트에 mysql 정보 주입 및 secret 관리 * deploy: mysql 준비과정까지 대기 aciton 추가: * deploy: 도커 사용을 위해 root 비밀번호 설정 * deploy: action 로직 개편 * deploy: 설정된 환경변수 스프링 실행 때 삽입 * test: ddl 설정 변경 * test: 테스트에는 트랜잭션 적용 * HotFix: 테스트 ddl 설정 변경 (#174) fix: ddl 타입 변경 및 더미데이터 추가는 배포서버 재부팅때만 적용 * feat, test : redis 전체 삭제 기능 구현 및 테스트 코드 추가 (#175) * feat, test : redis 전체 삭제 기능 구현 및 테스트 코드 추가 * fix: 레디스 포트 명시적으로 배포서버로 변경 * deploy: 보안상 속성파일 위치 변경 --------- Co-authored-by: gitjiho <[email protected]> * Fix: 환불하는데 돈이 오히려 차감되던 버그 수정과 시니어 등록, 수정 시 폰 번호 중복 확인 로직 추가 (#166) * fix: 환불하는데 돈이 오히려 차감되던 버그 수정 * feat: 시니어 등록, 수정 시 폰 번호 중복 확인 로직 추가 * remove: 안부전화 수정 api 제거 * HotFix: 로그아웃 시 멤버가 없으면 로그아웃 안되는 에러 해결 (#178) fix: 멤버가 탈퇴한 경우에도 로그아웃 가능하도록 수정 * Feat: 시니또 탈퇴시 본인이 할당받은 서비스가 있으면 다시 대기상태로 전환되는 로직 추가, SET_NULL 설정 (#181) * feat: 콜백 서비스층에 탈퇴시 들어가는 로직 구현 * feat: 안부전화 서비스층에 탈퇴시 들어가는 로직 구현 * feat: 멤버 탈퇴 로직에 콜백, 안부전화 진행중인거 확인하고 진행중인거 있으면 대기상태로 전환하는 로직 추가 * fix: 안부전화는 시니어만 다르면 동시에 여러개 가능하다 * fix: CallbackRepository 잘못된 메서드 제거 후 의존하는 코드 수정 * feat: HelloCall 엔티티에 `@OnDelete(action = OnDeleteAction.SET_NULL)` * feat: 개별 트랜잭션으로 분리 * feat: HelloCallTimeLog의 멤버속성에 ` @onDelete(action = OnDeleteAction.SET_NULL) ` 추가, 시니또이름 분기 추가 * fix: 개별 트랜잭션 적용. 기존에는 개별이 아니라 하나의 트랜잭션에 포함되어 있었다. * Refactor: 5차 코드리뷰 기반 개편 (안부전화 요일 변환, URL 환경변수화 관련) 구현 (#185) * refactor: 안부전화 요일 변환 로직 Map 사용방식으로 변환 * refactor: SLACK_WEBHOOK_URL 값을 환경변수화 * Refactor: 카카오 메시지 필드 개편, 입금 계좌 정보 gitignore, 프론트 구분용 uri gitignore화 (#186) * fix: 한글 제외 * HotFix: 시니어 정보 수정 시 전화번호를 수정하지 않고 다른 필드를 수정하면 전화번호 중복 오류가 뜨는 문제 해결 (#183) fix: 시니어 정보 수정 시 전화번호를 수정하지 않고 다른 필드를 수정하면 전화번호 중복 오류가 뜨는 문제 해결 * HotFix: 슬랙 웹 훅 URL 환경변수화 과정에서 오류 해결 (#188) fix: 오류 수정 * Deploy: 충전 및 출금 요청 발생시 슬랙메시지 전송 기능 추가 (#190) * feat: 충전 및 출금 요청 발생시 슬랙메시지 전송 기능 추가 * test: 테스트코드 수정 * test: 테스트용 yml 추가 * test: 테스트용으로는 value null값으로 지정 * HotFix: 속성 Value를 항상 null로 받는 문제 해결 (#192) * fix: 속성 Value를 항상 null로 받는 문제 해결 * deploy: 환경변수 추가 * HotFix: 중복 환경변수 이분화 (#197) fix: 중복 환경변수 이분화 --------- Co-authored-by: 2iedo <[email protected]> Co-authored-by: JIHO LEE <[email protected]> Co-authored-by: gitjiho <[email protected]>

zzoe2346 added 5 commits October 29, 2024 00:27

refactor: 불필요한 private method 통합

d77cff3

refactor: 개별 트랜잭션으로 변경

5c15109

- 메모리 소유 기간 줄일 수 있다 - 유저에게 더욱 신속한 피드백 할 수 있다. - 예외 발생에 전보다 덜 취약.

Merge branch 'Weekly' of https://github.com/kakao-tech-campus-2nd-ste…

e6a6982

…p3/Team8_BE into Refactor-#115

refactor: 포인트 관련 로직 서비스계층으로 옮김

3e865f0

test: 테스트 추가하고, 가독성 개선

7642b1f

zzoe2346 added ✅ Test Code 테스트 관련 작업을 진행하는 경우 ♻️ Refactoring 코드 리팩토링 & 클린 코드 작업을 진행하는 경우 labels Oct 31, 2024

zzoe2346 self-assigned this Oct 31, 2024

zzoe2346 linked an issue Oct 31, 2024 that may be closed by this pull request

Refactor: 콜백 도메인 4차 코드리뷰 반영 #115

Closed

4 tasks

zzoe2346 requested review from eunsoni, GitJIHO and 2iedo November 1, 2024 10:46

refactor: SQL 수정

b2f8984

GitJIHO reviewed Nov 2, 2024

View reviewed changes

GitJIHO approved these changes Nov 2, 2024

View reviewed changes

2iedo approved these changes Nov 2, 2024

View reviewed changes

eunsoni approved these changes Nov 3, 2024

View reviewed changes

zzoe2346 added 4 commits November 4, 2024 22:37

Merge branch 'Weekly' of https://github.com/kakao-tech-campus-2nd-ste…

7b5aa1a

…p3/Team8_BE into Refactor-#115

test: 안부전화 서비스에서 포인트 로직 서비스층으로 통합으로 인한 수정

97cd2f2

merge: conflict solve

ec6622b

remove: 깃헙 액션 테스트 통과를 위한 수정

0084704

zzoe2346 merged commit 41e0a54 into Weekly Nov 4, 2024
1 check passed

zzoe2346 deleted the Refactor-#115 branch November 4, 2024 14:19

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Refactor: 포인트관련 로직을 포인트 서비스계층으로 통합, 개별 트랜잭션 적용, 테스트 코드 개선 #132

Refactor: 포인트관련 로직을 포인트 서비스계층으로 통합, 개별 트랜잭션 적용, 테스트 코드 개선 #132

zzoe2346 commented Oct 31, 2024 •

edited

Loading

GitJIHO left a comment

GitJIHO Nov 2, 2024

GitJIHO Nov 2, 2024

GitJIHO Nov 2, 2024

GitJIHO Nov 2, 2024

zzoe2346 Nov 3, 2024 •

edited

Loading

GitJIHO Nov 3, 2024

2iedo left a comment

2iedo Nov 2, 2024

2iedo Nov 2, 2024 •

edited

Loading

eunsoni Nov 3, 2024

eunsoni left a comment

eunsoni Nov 3, 2024

Refactor: 포인트관련 로직을 포인트 서비스계층으로 통합, 개별 트랜잭션 적용, 테스트 코드 개선 #132

Refactor: 포인트관련 로직을 포인트 서비스계층으로 통합, 개별 트랜잭션 적용, 테스트 코드 개선 #132

Conversation

zzoe2346 commented Oct 31, 2024 • edited Loading

#️⃣ 연관된 이슈

📝 작업 내용

스크린샷 (선택)

💬 리뷰 요구사항(선택)

⏰ 현재 버그

✏ Git Close

GitJIHO 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

zzoe2346 Nov 3, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

2iedo left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

2iedo Nov 2, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

eunsoni left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

zzoe2346 commented Oct 31, 2024 •

edited

Loading

zzoe2346 Nov 3, 2024 •

edited

Loading

2iedo Nov 2, 2024 •

edited

Loading