JangBomi commented 1 year ago

요약

메시지 text에서 멘션 아이디를 대체하는 부분 리팩토링

작업 내용

메시지 text에서 멘션 아이디를 대체하는 부분 리팩토링

참고 사항

아직 미완이지만 기록해야할 부분이 있어서 Draft로 PR 올립니다.

현재 코드

ResponseDto가 MessageTextResponses, MessageTextResponse 인터페이스를 구현하도록 하고, 타입을 제네릭으로 구성
@MentionIdReplaceable 어노테이션을 도입해 해당 어노테이션이 붙은 메서드는 반환 값의 메시지 텍스트에서 멘션 아이디가 등장하면 닉네임으로 대치하는 코드가 동작하도록 Aspect 도입

현재 코드의 장점

깔끔하게 멘션 아이디를 닉네임으로 대치하는 로직을 조회 로직에서 빼냈다는 생각이 들어요.
개선된 코드인데, 어노테이션의 사용만으로 멘션 아이디를 대체할 수 있다는게 매력적임
빈 주입을 할 필요가 없음

현재 코드의 단점

ResponseDto는 말그대로 DTO라 값을 전달하는 역할만 해야할 것 같은데, MessageTextResponse, MessageTextResponses라는 인터페이스를 구현하면서 명세가 생긴 느낌이라 DTO 본래의 역할을 하지 못하는 것 같음.
중복 로직을 제거하기 위해 코드의 복잡도가 올라간 것 같음

멘션 아이디를 닉네임으로 대치하는게 AOP로 뺄만한 기능일까?

이 부분은 이전에 연로그가 닉네임 대치가 AOP로 뺄만한 기능인가? 라고 이야기해줘서 좀 더 깊게 고민해볼 수 있었어요. 공통 로직과 횡단 관심사의 관점으로 봤을 때 각 Service가 아래 그림에서의 핵심 로직이고, 멘션 아이디를 대체하는 로직은 핵심로직을 관통하는 횡단관심사라고 생각했어요. MessageService, ReminderService, BookmarkService 의 각 find() 또는 findOne() 메서드의 관심사는 Request에 넘어오는 조회 조건에 맞게 조회하는 것 이지, 텍스트의 멘션 아이디를 닉네임으로 대치하는 것은 핵심 관심사가 아니라고 생각이 들었고, 횡단 관심사로 뺄 수 있는 영역이 아닐까? 라고 생각했어요.

너무 멘션아이디를 Aspect로 보는 관점에 매몰되어있나? 싶어서 Aspect를 사용 안하고 제네릭 없이 최대한 중복 로직 제거한 버전과 제네릭 사용하고 중복 로직 제거한 버전 (각 브랜치에 ReminderService쪽 보시면 가장 보기 편할 것 같아요!) 도 구현해보면서 어떤 방법이 가장 좋을까 고민해봤을 때도 멘션아이디를 대체하는 부분은 횡단관심사라는 생각이 들었습니다!

아무튼 그래서 이 코드의 개선 방향은

Dto가 인터페이스를 구현하지 않도록 수정 이 될 것 같아요. 어제 써머랑 이야기하면서 DTO가 인터페이스를 구현하는게 이상하다, 인터페이스가 있는건 명세가 생긴건다. 인터페이스를 구현하고 제네릭을 사용한건 쉬운 방법을 돌아간 것 같다. 라는 이야기를 나눴어서 우선 이 코드를 리플렉션으로 개선할 예정입니다. 현재 코드는 ResponseDto가 MessageTextResponse를 구현한 경우에 대해 @MentionIdReplaceable을 붙이면 멘션 아이디 대체 코드가 동작하는데, 도메인 정책이 복잡한 것 같아서 리플렉션으로 필드 가져와서 수정해주는게 낫겠다 싶었어요.

Draft PR이고 아직 리플렉션 적용해보기 전이지만 혹시라도 의견 있으시다면 코멘트로 남겨주세요!! 바쁘실텐데 감삼당 🙇‍♀️