[feat/CK-240] 골룸 관련 부분 의존성 리팩토링을 한다 #207

miseongk · 2023-12-21T14:57:08Z

📌 작업 이슈 번호

CK-240

✨ 작업 내용

골룸 관련된 부분 의존성 리팩토링을 진행했습니다.
패키지를 생명주기에 따라 goalroom, todo, checkfeed로 나눴습니다.
- 패키지 내의 객체는 객체를 직접참조 했고, 패키지 외의 객체는 id로 간접참조 했습니다.
- goalroom -> roadmap, member, common
- todo -> goalroom, member, common
- checkfeed -> goalroom, member, common

패키지를 분리하면서 컨트롤러와 서비스도 각각 분리했습니다.
goalroom 패키지의 GoalRoomService에서 todo와 checkfeed를 의존하는 부분을 인터페이스를 활용해 의존성 역전했습니다.

💬 리뷰어에게 남길 멘트

너무 많은 수정이 있어서 혹시나 놓친 부분이 있을 수 있습니다..! 😭
현재 로드맵 관련된 테스트 몇개가 실패합니다. 로드맵과 골룸이 의존하고 있는 부분이 있는데, 이 부분을 로드맵 의존성 분리해주실 때 분명 변경될 것 같아서 처리하기가 힘들더라구요. 그래서 일단 실패하는대로 놔뒀습니다! 추후에 머지할 때 수정하겠습니다.
- 로드맵 정렬 할 때 골룸 개수와 참여자 수로 정렬하는 조건이 있는데 기존에는 쿼리에서 직접 골룸에 접근해서 문제 없었는데, 골룸과 로드맵 의존을 끊어서 직접 접근할 수 없게 되었습니다. 일단 로드맵 부분을 건드리기 어려워서 주석처리 해두었습니다.
- 로드맵 쪽에 수정된 코드는 임시로 해둔거라 무시하셔도 좋아요..!

🚀 요구사항 분석

goalroom 관련된 로직을 패키지(goalroom, todo, checkfeed)로 분리한다.

github-actions · 2023-12-21T14:59:23Z

Unit Test Results

  88 files   88 suites 34s ⏱️
757 tests 757 ✔️ 0 💤 0 ❌
771 runs 771 ✔️ 0 💤 0 ❌

Results for commit c30ea37.

♻️ This comment has been updated with latest results.

Ohjintaek

수고하셨어요 밀리~!!
요즘 코드를 잘 안 보다보니 집중력이 떨어져서 띄엄띄엄 보느라 너무 오래 걸렸습니다...
그냥 보는 것만 해도 이렇게 오래 걸렸는데 의존성 분리하고 테스트 코드까지 많이 건드리신 것 같아서 엄청 고생하셨을 것 같네요 😵
GoalRoom만을 분리하는 게 아니라 내부적으로 패키지를 더 나눈 점이 인상적이었습니다. 코멘트를 단 시간대가 자유분방해서 작성한 코멘트 중에서 맥락이 이상한 점이 있지 않을까 우려되는데 제 의견이 궁금하신 부분은 편하게 다시 질문주세요!!

backend/kirikiri/src/main/java/co/kirikiri/goalroom/service/GoalRoomReadService.java

backend/kirikiri/src/main/java/co/kirikiri/checkfeed/service/GoalRoomCheckFeedService.java

backend/kirikiri/src/main/java/co/kirikiri/common/service/dto/FileInformation.java

backend/kirikiri/src/main/java/co/kirikiri/goalroom/domain/GoalRoom.java

backend/kirikiri/src/main/java/co/kirikiri/goalroom/service/GoalRoomCreateService.java

backend/kirikiri/src/main/java/co/kirikiri/goalroom/service/mapper/GoalRoomMapper.java

backend/kirikiri/src/main/java/co/kirikiri/goalroom/service/GoalRoomReadService.java

backend/kirikiri/src/main/java/co/kirikiri/todo/domain/exception/GoalRoomToDoException.java

backend/kirikiri/src/main/java/co/kirikiri/todo/persistence/GoalRoomToDoCheckRepository.java

younghoondoodoom

안녕하세요 밀리! 작년까지 리뷰를 끝냈어야했는데 새해 들어서야 끝내네요.. 죄송합니다.
골룸 관련된 의존성 분리 정말 쉽지 않았겠네요.. 로드맵이랑도 엮인 부분이 많아서 더 힘드셨을거 같아요. 고생하셨습니다!

저는 의존성 분리의 목적이 조회를 편하게 위한 것이 아니라, 유지보수하기 좋은 형태로 바꾸는 것 같습니다. 어떤 클래스가 요구사항에 의해 변경 되었을때, 최대한 다른 클래스들에 최소한의 영향을 주는것이 좋다고 생각해요! 그래서 도메인 모델이 꼭 필요한 의존성만 가지고 있고, 불필요한 의존성은 제거하는게 맞다고 생각합니다.
양방향 의존성을 가지게 되면 두 클래스가 서로의 변경에 크게 영향을 받게 되어서 유지보수성이 떨어지게 됩니다. 조회 시에 불편하더라도 이러한 의존성은 꼭 끊는게 좋을 것 같아요!

추가적으로 제 생각 남겼으니까 참고 부탁드립니다. 나중에 의존성 관련해서 얘기 해봐도 좋을 것 같네요! 취업 준비 파이팅입니다🙌

backend/kirikiri/src/main/java/co/kirikiri/goalroom/domain/GoalRoom.java

backend/kirikiri/src/main/java/co/kirikiri/goalroom/service/GoalRoomCreateService.java

backend/kirikiri/src/main/java/co/kirikiri/checkfeed/service/GoalRoomCheckFeedService.java

backend/kirikiri/src/main/java/co/kirikiri/goalroom/domain/GoalRoom.java

backend/kirikiri/src/main/java/co/kirikiri/goalroom/service/GoalRoomCreateService.java

younghoondoodoom

안녕하세요 밀리! 롱타임노씨네요.. 골룸 의존성 분리하느라 고생하셨습니다!
이벤트를 발행할때 어떤 것을 이벤트로 발행해야하는지에 대해서 의견 남겼으니 참고 부탁드립니다.

고생하셨습니다👍

younghoondoodoom · 2024-01-20T07:18:10Z

...ikiri/src/main/java/co/kirikiri/checkfeed/service/AccomplishmentRateUpdateEventListener.java

이벤트랑 이벤트 리스너 자체에 무엇을 할지 들어나있는거 같아요! 이벤트는 단순히 인증피드 등록을 한다는 것을 발행하고 그것을 Listen 하는 쪽에서 구체적인 행위를 하면 좋을 것 같네요.

https://techblog.woowahan.com/7835/

위 링크의 무엇을 이벤트로 발행할 것인가? 부분 읽어보시면 좋을 것 같아요!

오 그렇군요!
이벤트 자체에 의도를 담으면 개념적인 결합도는 아직 높은 상태인 것이군요!
덕분에 아주 중요한 것을 깨달았어요 👍👍
이벤트에 의도가 드러나지 않게 모두 수정하겠습니다!

younghoondoodoom · 2024-01-20T07:22:03Z

...irikiri/src/main/java/co/kirikiri/checkfeed/service/event/AccomplishmentRateUpdateEvent.java

+public record AccomplishmentRateUpdateEvent(
+        Long goalRoomId,
+        Long goalRoomMemberId,
+        int pastCheckCount


pastCheckCount을 이벤트로 직접 넘겨주면 좋지만, 저는 이벤트에 너무 구체적인 정보가 들어가는것 같아요! 이 이벤트를 다른 쪽에서도 Listen 할 수 있으니까 특정 행위(달성률 계산)에 대한 구체적인 정보는 최대한 없애는게 좋다고 생각합니다.

eventListener애서 checkFeedRepository를 통해서 이 정보를 가져오는건 어떨까요?

��저도 그렇게 생각해요!
이벤트에는 최소한의 필요한 정보만 전달하는게 좋겠네요!

younghoondoodoom · 2024-01-20T07:24:39Z

...d/kirikiri/src/main/java/co/kirikiri/goalroom/service/GoalRoomLeaderUpdateEventListener.java

이 클래스도 이름에 행위의 목적이 들어있는거 같아요! 골룸 생성에 대한 이벤트를 발행하고, 그걸 들어서 처리하는 쪽에서 구체적인 행위를 하면 더 좋을 것 같습니다! 이벤트명도 마찬가지 입니다!

younghoondoodoom · 2024-01-20T07:27:16Z

...nd/kirikiri/src/main/java/co/kirikiri/goalroom/service/EmptyGoalRoomDeleteEventListener.java

이 클래스랑 이벤트 네이밍도 위 의견들이랑 같습니다~!

Ohjintaek

일단 요즘 리뷰가 너무 늦어져 죄송합니다 😥 두 번째도 많은 수정을 하셨겠군요...
제가 코멘트한 부분에 대해서는 추가적인 제의사항이 없어 approve합니다.

그와 별개로 두둠이 전에 코멘트하신 부분과 관련해서 GoalRoom과 GoalRoomMember, GoalRoomPendingMember 간의 양방향 의존성을 없애기 위해서 현재 GoalRoom에서 참가한 멤버들에 대한 의존성을 다 끊은 것에 대해서 의견드릴 부분이 있습니다.

GoalRoom의 생명주기가 멤버들과 달라서 의존성을 끊어버렸는데, 멤버들 개개인은 다르지만 GoalRoom과 GoalRoomMembers 혹은 GoalRoomPendingMembers, 즉 멤버들과는 같다는 생각이 듭니다. 골룸에 참가할 때 반드시 멤버들에 대한 정보를 필요로 하고, 골룸에서 나갈 때도 모든 멤버들이 나갔을 때 골룸을 삭제하거나 리더가 나갔다면 새롭게 리더를 선정하기 위해 멤버들에 대한 정보를 필요로 합니다.
비즈니스 규칙상 엄청 연관되어 있는 객체다 보니 분리하고 보니 서비스 레이어에 코드가 다 넘어가서 제 개인적으로는 코드가 더 읽기 어려워진다는 단점도 있는 것 같구요.
해당 부분에 대해서는 무조건 했으면 좋겠다가 아닌 의견 제시일 뿐이니 밀리가 맞다고 생각하신 대로 진행하시면 좋을 거 같습니다.
수고하셨어요👍👍

backend/kirikiri/src/main/java/co/kirikiri/goalroom/service/GoalRoomReadService.java

miseongk · 2024-02-17T10:09:28Z

@Ohjintaek 썬샷의 코멘트를 보고 엄청 많이 고민을 해봤어요!
저도 GoalRoom에서 GoalRoomMembers 의존성을 끊으면서 서비스 레이어에 모든 로직이 넘어가는게 고민이 많았어요!

그래서 다시 한번 곰곰히 생각을 해봤는데,
GoalRoomMembers가 변경되면 GoalRoom에도 영향을 줄 수 있기 때문에 의존성을 끊었는데, 사실 GoalRoomMembers가 하는 일이 거의 없더라구요. 거의 대부분의 메서드가 검증하는데 쓰이고, 이 말은 GoalRoom과 GoalRoomMember가 도메인 제약조건을 강하게 공유한다는 뜻인 것 같아요. 이런 경우에는 굳이 의존성을 끊을 필요가 없다고 생각해요!
또한 이후에도 GoalRoomMember를 갖고 로직을 수행할 여지도 별로 없는 것 같아요. (혹시 떠오르는 상황이 있다면 알려주세요.. 저는 정말 모르겠어요 🥲)

하지만 양방향 연관관계는 없는게 좋은 것 같아요. 전에는 생각을 못했는데 GoalRoomMember가 GoalRoom을 참조하는 것을 없애서 양방향을 끊어주는 쪽이 더 나을 것 같아요! GoalRoomMember에서 GoalRoom을 사용해서 로직을 처리하는 메서드도 전혀 없더라구요. 😅
이 부분은 어떻게 생각하시나요?!

혹시 제가 생각하지 못한 부분이나, 다른 관점으로 봐야 한다거나 그런 것들이 있으면 알려주시면 감사하겠습니다. 🙇‍♀️🙇‍♀️🙇‍♀️
@younghoondoodoom

Ohjintaek · 2024-02-17T12:14:14Z

저도 GoalRoom과 GoalRoomMember간의 양방향 연관관계를 없애는 게 낫다고 생각합니다. GoalRoomMember를 가지고 GoalRoom을 찾을일도 없을 거 같구요

GoalRoomMembers가 변경되면 GoalRoom에도 영향을 줄 수 있기 때문에 의존성을 끊었는데, 사실 GoalRoomMembers가 하는 일이 거의 없더라구요. 거의 대부분의 메서드가 검증하는데 쓰이고, 이 말은 GoalRoom과 GoalRoomMember가 도메인 제약조건을 강하게 공유한다는 뜻인 것 같아요. 이런 경우에는 굳이 의존성을 끊을 필요가 없다고 생각해요!
또한 이후에도 GoalRoomMember를 갖고 로직을 수행할 여지도 별로 없는 것 같아요. (혹시 떠오르는 상황이 있다면 알려주세요.. 저는 정말 모르겠어요 🥲)

그런데 이 부분에 대해서는 제가 잘 이해를 못 했는데 혹시 한 번 더 설명해주실 수 있나요? GoalRoomMembers가 하는 일이 거의 없어서 의존성을 끊을 필요가 없다는 부분이 어떤 의미인지 잘 모르겠어서요 😅

miseongk · 2024-02-18T06:57:39Z

@Ohjintaek

기존 GoalRoom이 GoalRoomMembers를 의존하는 구조에서는 GoalRoom 내부에서 GoalRoomMembers를 통해 validate 하는 로직을 수행했었는데, GoalRoom에서 GoalRoomMembers를 제거하니 서비스 레이어에 검증 부분이 모두 넘어갔죠!
그런 문제가 있음에도 의존성을 끊었던 이유는, GoalRoomMembers가 변경되었을 때 서로 양방향 의존을 하고 있어서 양쪽에 변경지점이 발생할 여지가 있어 유지보수성이 떨어지기 때문이었어요!

하지만 GoalRoomMembers가 검증 로직 이외에 별다른 역할이 없기 때문에 변경될 가능성이 적다고 판단했고,
양방향 의존 문제는 GoalRoomMember의 GoalRoom을 제거하면서 해결할 수 있으니까
굳이 도메인 제약조건이 강하게 결합되어 있는 GoalRoom과 GoalRoomMember의 의존성을 끊을 필요가 없다고 생각했어요!

github-actions · 2024-05-18T17:42:20Z

🪄 Test Coverage Report

File	Coverage [95.79%]	🍏
RoadmapController.java	100%	🍏
GoalRoomCheckFeedController.java	100%	🍏
AmazonS3FileService.java	100%	🍏
GoalRoomCreateEvent.java	100%	🍏
GoalRoomLeaveEvent.java	100%	🍏
GoalRoomToDoController.java	100%	🍏
GoalRoomToDoService.java	100%	🍏
DashBoardToDoServiceImpl.java	100%	🍏
RoadmapSaveArgumentResolverImpl.java	100%	🍏
RoadmapCreateService.java	100%	🍏
RoadmapCreateEventListener.java	100%	🍏
GoalRoomMemberQueryRepositoryImpl.java	100%	🍏
GoalRoomPendingMemberQueryRepositoryImpl.java	100%	🍏
GoalRoomQueryRepositoryImpl.java	100%	🍏
GoalRoomMemberJdbcRepositoryImpl.java	100%	🍏
GoalRoomScheduler.java	100%	🍏
MemberQueryRepositoryImpl.java	100%	🍏
RandomNumberGenerator.java	100%	🍏
UUIDFilePathGenerator.java	100%	🍏
CheckFeedCreateEvent.java	100%	🍏
RoadmapReviewQueryRepositoryImpl.java	100%	🍏
RoadmapContentQueryRepositoryImpl.java	100%	🍏
RoadmapNodeQueryRepositoryImpl.java	100%	🍏
GoalRoomCreateEventListener.java	100%	🍏
GoalRoomLeaveEventListener.java	100%	🍏
GoalRoomController.java	100%	🍏
DashBoardCheckFeedServiceImpl.java	100%	🍏
RoadmapReadService.java	98.06%	🍏
GoalRoomCreateService.java	97.1%	🍏
CheckFeedCreateEventListener.java	95.73%	🍏
RoadmapQueryRepositoryImpl.java	94.84%	🍏
GoalRoomCheckFeedService.java	94.4%	🍏
GoalRoomReadService.java	92.4%	🍏
RoadmapGoalRoomServiceImpl.java	85.67%	❌
QuerydslRepositorySupporter.java	49.55%	❌

Total Project Coverage	96.48%	🍏

refactor: GoalRoom 관련 패키지 의존성 분리

b58e6db

miseongk added enhancement New feature or request BE 👨‍👩‍👧‍👦 BackEnd labels Dec 21, 2023

miseongk requested review from younghoondoodoom and Ohjintaek December 21, 2023 14:57

miseongk self-assigned this Dec 21, 2023

miseongk added refactor🧹 refactor and removed enhancement New feature or request labels Dec 21, 2023

Ohjintaek requested changes Dec 25, 2023

View reviewed changes

younghoondoodoom requested changes Jan 1, 2024

View reviewed changes

miseongk added 5 commits January 10, 2024 17:10

refactor: 리뷰 반영 및 골룸 생성 로직 변경

1d3c2b6

feat: GoalRoom과 GoalRoomMember, GoalRoomPendingMember의 양방향 의존성 제거

8081ae4

feat: 골룸 생성 시 리더 업데이트하는 로직 이벤트 분리

c36adf9

feat: 골룸 나갈 때 빈 골룸을 삭제하는 로직 이벤트 분리

4aa6c86

feat: 인증피드 등록 시 달성률 업데이트 로직 이벤트 분리

838c213

younghoondoodoom requested review from younghoondoodoom and Ohjintaek January 15, 2024 05:10

younghoondoodoom requested changes Jan 20, 2024

View reviewed changes

Ohjintaek approved these changes Jan 20, 2024

View reviewed changes

backend/kirikiri/src/main/java/co/kirikiri/goalroom/service/GoalRoomReadService.java Outdated Show resolved Hide resolved

miseongk added 3 commits January 24, 2024 18:41

refactor: 요구 사항 반영 (DashBoardCheckFeedService)

343c6d1

refactor: CheckFeed 생성 시 달성률 업데이트 이벤트 네이밍 변경

127df8e

refactor: 골룸 생성, 나가기 이벤트 네이밍 변경

660af6f

miseongk requested a review from younghoondoodoom February 17, 2024 10:13

miseongk added 2 commits April 3, 2024 15:01

feat: 골룸 참여 시, 골룸 펜딩 멤버 테이블에 락이 걸리도록 수정

a18c81a

refactor: 골룸 시작, 나가기 쿼리 수정

731c8a4

miseongk removed the request for review from younghoondoodoom May 14, 2024 15:38

chore: merge conflict 해결

c30ea37

miseongk requested a review from younghoondoodoom May 18, 2024 17:40

miseongk merged commit 837aaf6 into develop-backend May 18, 2024
3 checks passed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[feat/CK-240] 골룸 관련 부분 의존성 리팩토링을 한다 #207

[feat/CK-240] 골룸 관련 부분 의존성 리팩토링을 한다 #207

miseongk commented Dec 21, 2023 •

edited

Loading

github-actions bot commented Dec 21, 2023 •

edited

Loading

Ohjintaek left a comment

younghoondoodoom left a comment

younghoondoodoom left a comment

younghoondoodoom Jan 20, 2024

miseongk Feb 10, 2024

younghoondoodoom Jan 20, 2024

miseongk Feb 10, 2024

younghoondoodoom Jan 20, 2024

younghoondoodoom Jan 20, 2024

Ohjintaek left a comment

miseongk commented Feb 17, 2024

Ohjintaek commented Feb 17, 2024

miseongk commented Feb 18, 2024 •

edited

Loading

github-actions bot commented May 18, 2024

[feat/CK-240] 골룸 관련 부분 의존성 리팩토링을 한다 #207

[feat/CK-240] 골룸 관련 부분 의존성 리팩토링을 한다 #207

Conversation

miseongk commented Dec 21, 2023 • edited Loading

📌 작업 이슈 번호

✨ 작업 내용

💬 리뷰어에게 남길 멘트

🚀 요구사항 분석

github-actions bot commented Dec 21, 2023 • edited Loading

Unit Test Results

Ohjintaek left a comment

Choose a reason for hiding this comment

younghoondoodoom left a comment

Choose a reason for hiding this comment

younghoondoodoom left a comment

Choose a reason for hiding this comment

younghoondoodoom Jan 20, 2024

Choose a reason for hiding this comment

miseongk Feb 10, 2024

Choose a reason for hiding this comment

younghoondoodoom Jan 20, 2024

Choose a reason for hiding this comment

miseongk Feb 10, 2024

Choose a reason for hiding this comment

younghoondoodoom Jan 20, 2024

Choose a reason for hiding this comment

younghoondoodoom Jan 20, 2024

Choose a reason for hiding this comment

Ohjintaek left a comment

Choose a reason for hiding this comment

miseongk commented Feb 17, 2024

Ohjintaek commented Feb 17, 2024

miseongk commented Feb 18, 2024 • edited Loading

github-actions bot commented May 18, 2024

🪄 Test Coverage Report

miseongk commented Dec 21, 2023 •

edited

Loading

github-actions bot commented Dec 21, 2023 •

edited

Loading

miseongk commented Feb 18, 2024 •

edited

Loading