Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[BE] 의존성 리팩토링 #634

Open
cpot5620 opened this issue Nov 29, 2023 · 3 comments
Open

[BE] 의존성 리팩토링 #634

cpot5620 opened this issue Nov 29, 2023 · 3 comments
Assignees
Labels
BE 백엔드 관련 이슈 refactor 리팩토링 관련 이슈 우선순위 : 상

Comments

@cpot5620
Copy link
Collaborator

🛠️ 작업 대상

BE 코드 전체

✅ 작업 내용

  • 기존 코드에 양방향 의존성이 다수 존재함.
  • 이로인해, 새로운 기능 구현시 변경 포인트가 늘어남.
  • 이는 기능 구현 속도 저하로 이어지고 있음.

우선적으로, 도메인 간의 의존성 분리가 우선이 되어야한다고 생각합니다. 그래야, Service 계층에 대한 리팩토링도 진행할 수 있겠죠 !
물론, 도메인 의존성 분리를 하다보면, 서비스도 자연스럽게 될 것 같긴 합니다.
Intellij의 의존성 다이어그램을 통해 의존성 파악을 해보려고 했는데, 너무 지저분하더라구요.
그래서 제가 코드 구조를 파악하기 위해 그렸던 다이어그램 첨부합니다. (Common, OAuth 패키지는 제외하고 주요 도메인만 파악했어요)

검은색은 뭐.. 그냥 의존 방향이구요.
빨간색으로 되어 있는 부분은 Cycle Dependency가 발생하는 부분입니다.
파란색은 구조를 바꿀 수 있지 않을까 싶어서 별도로 처리해두었습니다. (PinHistory가 Pin뿐만 아니라, PinInfo도 알고 있는 구조가 조금 이상해보여서 ? 근데 이 방법이 맞는 것 같기도 해요.)
image

글씨가 개판이고, 한 페이지에 담아내고 싶었는데 잘 안되더라구요.. 넓은 아량으로 이해 부탁드려요 ㅋㅋ.


다이어그램을 작성하면서 고민해본 도메인별 의존성 분리를 어떻게 해야할지 생각해봤는데요.
함께 이야기 나누기 전에 아래 정리된 내용을 바탕으로 고민해보고 오시면 좋을 것 같습니다.

  • Topic <-> Member : Topic -> MemberId 간접 참조

Member 객체를 직접적으로 알 필요는 없을 것 같다고 판단했어요. ID 값만으로도 충분히 커버 가능할 것 같아요.
물론, 작성자의 이름을 알아오기 위한 작업(Mapper?)이 별도로 필요하긴 하지만요.

  • Pin <-> Member : Pin -> MemberId 간접 참조

이 부분도 위와 동일합니다.

  • Pin <-> Location : Pin -> LocationId 간접 참조

Pin조회 시 항상 Location이 필요하다보니, Pin -> Location 단방향으로만 변경할까 싶었는데요.
확장성 고려했을 때, Location 객체 자체로 할 수 있는 영역이 많을 것 같아서 의존성을 끊어내는게 맞지 않을까 싶었어요.
또한, 생명주기로 보면 서로 다르다고 생각이 들더라구요. 근데 이 부분은 특히나 확신이 없네요..

  • Topic <-> Pin: Topic -> PinId 간접 참조

Topic 조회시 Pin이 함께 조회되지만, 그 역방향은 발생하지 않는다고 생각해요.
그래서, 방향성은 Topic -> Pin 으로 생각하는데요. 객체 참조로 가는건 변경 포인트 문제로 인해, 간접 참조로 가면 좋을 것 같아요.

  • PinInfo <- PinHistory -> Pin : PinInfo <- PinHistory -> PinId

PinHistory에서 이전 정보 저장을 위해 PinInfo를 아는 것은 괜찮을 것 같긴해요.
하지만, Pin 객체 참조보다는 간접 참조가 좋아보이네요.

  • Member <-> Permission <-> Topic : MemberId <- Permission -> TopicId 간접 참조 & 패키지 분리(?)

Permission은 일종의 validation을 위한 객체인데요.
id 값들로만 연관관계를 맺어두어도 충분히 검증에 사용할 수 있을 것 같아요.
의존성 주입을 통해, 도메인 계층에서 검증이 수행될 수도 있겠죠 !
문제는 어느 패키지에 두냐인데.. 고민 더 해보죠 !

  • Member <-> Bookmark <-> Topic : MemberId <- Bookmark -> TopicId 간접 참조 & 패키지 분리

Bookmark는 별도의 패키지로 분리해도 괜찮을 것 같다는 생각이 들어요.
Topic과 밀접하게 연관되어 있지만, 별도의 비즈니스 로직이라고 생각하거든요.
객체의 의존성은 위와 같지만, 패키지 분리시에는 Bookmark Package -> Topic Package를 의존하고 있을 것 같아요.
BookmarkQueryService가 추가되면 되지 않을까 싶기도 합니다 !

  • Member <-> Atlas -> Topic : MemberId <-> Atlas <-> TopicId 간접 참조 & 패키지 분리

왜 이 친구는 Topic이랑은 단방향일까요.. 이해할 수 없군요.
이 부분도 위에서 이야기한 Bookmark와 마찬가지로 분리하면 되지 않을까 싶어요.

  • PinComment <-> PinComment : PinComment <-> PinCommentId 간접 참조

현재의 댓글 기능 구조를 크게 바꾸지 않으려면, 우선 부모 댓글의 Id 값만으로도 충분히 커버될거라고 생각해요.
한 가지 걱정되는 부분은, @OnDelete(action = OnDeleteAction.CASCADE) 이 부분인데요.
Event를 적절히 사용한다면, 부모 댓글 삭제시 그 하위 댓글들도 삭제시킬 수 있지 않을까 싶네요 !


도메인 의존성만 우선적으로 이야기해보고 싶다고 말했었는데, 작성하다보니 패키지 및 다른 계층 의존성도 많이 고려한 것 같아요.
어디까지나 제 생각이고, 부족한 부분이 엄청 많이 보여요. 근거가 부족한 부분도 많구요.
단순 참고용으로만 봐주시고, 회의 통해서 패키지 구조까지 함께 고려해보도록 하죠 !

📎 참고 자료

우아한 객체지향

⏰ 추정 시간

비관적 추정 시간 : 2주
낙관적 추정 시간 : 1주

@cpot5620 cpot5620 added BE 백엔드 관련 이슈 우선순위 : 상 refactor 리팩토링 관련 이슈 labels Nov 29, 2023
@kpeel5839
Copy link
Collaborator

  • Topic <-> Member : Topic -> MemberId 간접 참조

Member 객체를 직접적으로 알 필요는 없을 것 같다고 판단했어요. ID 값만으로도 충분히 커버 가능할 것 같아요.
물론, 작성자의 이름을 알아오기 위한 작업(Mapper?)이 별도로 필요하긴 하지만요.

  • Pin <-> Member : Pin -> MemberId 간접 참조

이 부분도 위와 동일합니다.

동의 보감

  • Pin <-> Location : Pin -> LocationId 간접 참조

Pin조회 시 항상 Location이 필요하다보니, Pin -> Location 단방향으로만 변경할까 싶었는데요.
확장성 고려했을 때, Location 객체 자체로 할 수 있는 영역이 많을 것 같아서 의존성을 끊어내는게 맞지 않을까 싶었어요.
또한, 생명주기로 보면 서로 다르다고 생각이 들더라구요. 근데 이 부분은 특히나 확신이 없네요..

저도 생명주기는 서로 다르다는 생각이에요!

특정 위치에 처음으로 Pin 이 생겼을 때, Location 이 같이 생기긴 하지만, 그 이후로는 그렇지 않기도 하구, 무엇보다 삭제될 때 Location 을 같이 삭제하지는 않으니까요

그렇기 때문에 저도 쥬니와 동일하게 간접참조로 끊어내는 것이 좋다고 생각합니다

다만 수정해야할게 많을 것 같네요 ㅋㅋㅋ

  • Topic <-> PinTopic -> PinId 간접 참조

Topic 조회시 Pin이 함께 조회되지만, 그 역방향은 발생하지 않는다고 생각해요.
그래서, 방향성은 Topic -> Pin 으로 생각하는데요. 객체 참조로 가는건 변경 포인트 문제로 인해, 간접 참조로 가면 좋을 것 같아요.

  • PinInfo <- PinHistory -> Pin : PinInfo <- PinHistory -> PinId

PinHistory에서 이전 정보 저장을 위해 PinInfo를 아는 것은 괜찮을 것 같긴해요.
하지만, Pin 객체 참조보다는 간접 참조가 좋아보이네요.

동의!

  • Member <-> Permission <-> Topic : MemberId <- Permission -> TopicId 간접 참조 & 패키지 분리(?)

Permission은 일종의 validation을 위한 객체인데요.
id 값들로만 연관관계를 맺어두어도 충분히 검증에 사용할 수 있을 것 같아요.
의존성 주입을 통해, 도메인 계층에서 검증이 수행될 수도 있겠죠 !
문제는 어느 패키지에 두냐인데.. 고민 더 해보죠 !

여기서 말하는 도메인 계층은 Service 를 말씀하시는것일까요?

간접참조로 끊어주는 것은 킹정입니당

  • Member <-> Bookmark <-> Topic : MemberId <- Bookmark -> TopicId 간접 참조 & 패키지 분리

Bookmark는 별도의 패키지로 분리해도 괜찮을 것 같다는 생각이 들어요.
Topic과 밀접하게 연관되어 있지만, 별도의 비즈니스 로직이라고 생각하거든요.
객체의 의존성은 위와 같지만, 패키지 분리시에는 Bookmark Package -> Topic Package를 의존하고 있을 것 같아요.
BookmarkQueryService가 추가되면 되지 않을까 싶기도 합니다 !

  • Member <-> Atlas -> Topic : MemberId <-> Atlas <-> TopicId 간접 참조 & 패키지 분리

왜 이 친구는 Topic이랑은 단방향일까요.. 이해할 수 없군요.
이 부분도 위에서 이야기한 Bookmark와 마찬가지로 분리하면 되지 않을까 싶어요.

ㅇㅈ!

ㅋㅋㅋㅋ 왜 이것만 단방향일까요 ㅋㅋㅋㅋ

  • PinComment <-> PinComment : PinComment <-> PinCommentId 간접 참조

현재의 댓글 기능 구조를 크게 바꾸지 않으려면, 우선 부모 댓글의 Id 값만으로도 충분히 커버될거라고 생각해요.
한 가지 걱정되는 부분은, @OnDelete(action = OnDeleteAction.CASCADE) 이 부분인데요.
Event를 적절히 사용한다면, 부모 댓글 삭제시 그 하위 댓글들도 삭제시킬 수 있지 않을까 싶네요 !

이거 확실히 제가 할 때, 간접 참조로 의존성을 분리할 수 있다는 사실조차 몰랐었어서, 기계적으로 직접참조로 했었는데, 쥬니 말대로 하는게 훨씬 좋아보이네요

그리고 Event .. 보여 줘 ! 쥬 니 짱!


설계하느라 너무 고생했어요 쥬니짱!

�이대로 다 된다면, 현재보다 훨씬 깔끔해질 것 같네요!

화이팅!

@yoondgu
Copy link
Collaborator

yoondgu commented Nov 29, 2023

먼저 이렇게 정리해주셔서 감사해요 쥬니!! 👍👍
제 의견도 추가로 남겨봅니다.
의존성 분리를 많이 안해봐서 뭐가 더 좋을지 고민이다보니 코멘트가 길어졌네요 😵‍💫 같이 공부하는 셈 치고 얘기해보면 좋을 것 같슴돠~~

어떤 경우에 간접 참조로 변경하는 게 더 좋은 걸까?

일단 이 기준이 조금 헷갈려서 정리해봤어요. 다른 분들도 마찬가지로 생각하실까요?!
이 기준을 가지고 아래에 추가적인 의견 남겼습니당.

  • 연관 관계를 맺은 객체를 직접 참조해서, 특정 로직을 수행할 필요가 없는 경우
  • Id만 알고 있어도 비즈니스 로직 수행이 가능한 경우

1. Member - Topic, Pin / Bookmark - Topic, Atlas - Topic 과 조회 작업

전반적으로 도메인들과 Member의 결합도가 높고 양방향 의존이 발생하기 때문에, 의존성을 없애는 것도 좋다고 생각합니다.
두 가지 관점에서 고민되는 점들이 있어요!!

  1. 간점 참조로 변경하며 쿼리 발생
    Topic, Pin 조회 시 거의 항상 작성자 이름을 조회하게 되는데, 간접 참조로 변경하면 매번 추가 쿼리가 발생하지 않을까요?
    결국 Topic에서 Member 객체를 직접 참조하는 것과 같은 일을 해야 하지 않을까요?
    Bookmark, Atlas와 Topic의 의존성에서도 마찬가지 상황으로 보여요.
    쓰기 작업 시에는 해당 객체들을 직접 참조할 필요가 없어 보이지만, 조회 시에는 Topic 정보를 꼭 보여주게 되므로 결국 Topic을 조회해오게 됩니다.

조회 시 추가 쿼리가 발생하더라도, 말씀해주신 것처럼 코드의 유지보수성과 도메인 간 독립성을 위해서라면 트레이드 오프라고 봐야 할까요?
아니면 id로 참조하면서도 쿼리 개선을 같이 하는 방법이 있으려나요?
제가 생각 못한 다른 부분이 있을 것 같아서 얘기해보고 싶어요!!

  1. 다른 방법으로 순환 참조를 해결하기
    다이어그램으로 정리해주신 것처럼, 순환 참조의 문제도 있죠!!
    1번에 대한 고민이 들면서, 간접 참조 대신 순환 참조 문제를 아래와 같이 해결할 수도 있지 않나..?! 싶었어요.
    이 방법은 어떻게 생각하셨는지 궁금해요!!
  • Member <-> Topic, Pin: Member가 Topic, Pin을 알아야 하는 이유는 AuthMember 생성 방식 때문인데, 이를 바꾸는 건 어려울까요..?
  • Topic <-> Bookmark: Topic이 Bookmark를 모르게 해도 될 것 같아요!

2. PinHistory와 PinInfo, Pin

PinInfo는 엔티티가 아닌 VO이다보니 참조 관계를 고려할 필요 없다고 생각했어요. 물론 PinInfo는 Pin 패키지에서 사용하는 VO이기 때문에 Pin 패키지를 import하고 있다는 점에서 잘 고민해주셨다고 생각해요. 다만 PinHistory가 PinInfo를 참조하는 현상이 발생한 이유는, Pin을 참조하고 있기 때문이니까 Pin에 대해서만 고민해도 같이 해결될거라 생각해요!!
그래서 간접참조로 바꾸면 PinInfo를 참조하는 문제도 함께 해결될 것 같아요. 간접 참조로 바꾸는 것에 완전 동의합니다.
(id만 알아서 저장해도 되니까요)

3. PinComment 삭제와 이벤트

부모 댓글, 자식 댓글은 같은 Aggregate로 보여서 직접 참조해도 좋고, 반대로 지금은 Id만 알아도 되니까 간접참조하는 것도 좋습니다.
다만 어떤 로직들에 이벤트를 적용할 것인지에 대한 기준을 세우고 해도 좋을 것 같다는 생각은 조금 들어요 ㅎㅎㅎ 저도 많이 안써봐가지구

정해진 답은 없으니까 생각하고 기준만 정해져있으면 된다고 생각합니다!

  • 부가적인 작업(알림, 로그 등)만 이벤트를 쓸 것인가?
  • 핵심 로직까지도 이벤트 중심 설계(?)를 통해 의존성 분리를 할 것인가?

@cpot5620
Copy link
Collaborator Author

cpot5620 commented Nov 30, 2023

매튜 도이 의견 감사합니다 !
매튜는 거의 Yes man 수준으로 동의해주셔서, 도이 의견에 제 생각을 조금 더 담아볼게요.

어떤 경우에 간접 참조로 변경하는 게 더 좋은 걸까?
연관 관계를 맺은 객체를 직접 참조해서, 특정 로직을 수행할 필요가 없는 경우
Id만 알고 있어도 비즈니스 로직 수행이 가능한 경우

저는 두 의견에 모두 동의하는데요. (사실 같은 말인 것 같기도 ..ㅎㅎ)
특정 로직의 요구사항이 변경되었을때, 변경 포인트가 다른 곳에서도 일어나는걸 방지하고 싶었어요.
물론 밀접하게 연관되어있는 객체들이라면 어쩔 수 없지만요. (근데 지금도 테스트 코드 말고는 크게 문제 없는 것 같기도 하네요 😥)
또한 간접 참조로 끊어내면, 지속적으로 이야기 나왔던 부자연스러운 연관관계 편의 메서드도 어느정도 해결할수도 있겠네요 !
(이 부분은 단방향만으로도 해결할 수 있는 문제인 것 같기도 하네요)

1. Member - Topic, Pin / Bookmark - Topic, Atlas - Topic 과 조회 작업

- 간접 참조로 변경하며 쿼리 발생

간접 참조로 변경함에 따라, 추가적인 쿼리가 발생하는 것은 성능상으로 기존 로직과 동일하다고 생각해요.
어쨌든 지연 로딩으로 데이터를 한 번 더 조회해오니까요.
근데 우리가 해당 로직에서 필요로 하는건 정말 사용자 이름뿐인데, email, imageUrl 등도 함께 조회해고 있어요.
성능을 고려해본다면, 오히려 PK 값으로 이름만 조회해오는게 낫지 않을까 싶어요.
물론 성능 차이가 월등히 좋다는 것은 아니지만, 간접 참조로 인해 성능 저하가 발생하진 않을 것 같다 ! 로 이해해주시면 될 것 같아요.
(JPQL을 작성해야한다는 귀찮음은 있겠네요..)

- 다른 방법으로 순환 참조를 해결하기

Member <-> Topic, Pin: Member가 Topic, Pin을 알아야 하는 이유는 AuthMember 생성 방식 때문인데, 이를 바꾸는 건 어려울까요..?

만약, Member가 Topic, Pin을 완전히 모르게 하더라도 AuthMember 생성 방식만 변경하면 해결할 수 있을 것 같아요.
Topic -> Member(Creator)Id / TopicId <- Permission -> MemberId 구조로 변경한다면 AuthService 쪽에서 해결할 수 있을 것 같아요.

Topic <-> Bookmark: Topic이 Bookmark를 모르게 해도 될 것 같아요!

이 부분은 저도 동의해요 ! 순환 참조 문제만 해결해도 될 것 같긴 하네요.
한 가지 고민되는 부분은 Bookmark 등은 쓰기 작업시에도 MemberTopic 조회를 하고 있어요.
하지만, 간접 참조를 사용한다면 쓰기 작업시에는 굳이 조회를 할 필요가 없지 않을까 ? 라는 생각이 드네요.

2. PinHistory와 PinInfo, Pin

뭐.. 반박할게 없어서 넘어갈게요 😄

3. PinComment 삭제와 이벤트

부가적인 작업(알림, 로그 등)만 이벤트를 쓸 것인가?
핵심 로직까지도 이벤트 중심 설계(?)를 통해 의존성 분리를 할 것인가?

사실 깊게 고민 안 해본 부분이었던 것 같아요.
그냥 있어보이고 싶어서 Event 이야기를 했던 것 같네요..
생각해보니 Event 없이 삭제할 수 있어서 그냥.. 제가 멍청했던 걸로.. 해주세요 ..
그래도 Event를 어디에서 사용할지는 이야기 해봐야할 것 같네요 !

cpot5620 added a commit that referenced this issue Dec 15, 2023
* refactor: PermissionId 생성자 접근 제어자 수정

* refactor: BookmarkId 구현

* refactor: Bookmark 의존성 분리 (간접 참조)

* refactor: 불필요한 mocking 로직 제거

* refactor: 미사용 import문 제거

* refactor: Bookmark 조회 로직 수정 (JPQL 제거)

* refactor: PermissionId Null 검증 로직 추가

* refactor: BookmarkId Null 검증 로직 추가
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
BE 백엔드 관련 이슈 refactor 리팩토링 관련 이슈 우선순위 : 상
Projects
None yet
Development

No branches or pull requests

3 participants