[LO-222] 북마크 오픈타입 정책 변경

hyuk0309 commented 2 years ago

🛠️ 작업 내용

북마크 오픈타입 정책 변경

🗨️ 기타

원래 북마크 오픈타입에는 ALL, PARTIAL, PRIVATE이 있었습니다. PARTIAL은 자신의 팔로워들에게만 북마크 접근을 허용하는 개념이었습니다. 하지만 저희 서비스는 팔로워, 팔로이 정책이기 때문에 아무나 나를 팔로우 할 수 있습니다. 따라서 PARTIAL이 의미가 없고, 오히려 비즈니스 로직만 복잡하게 만들었습니다. 그래서 저번에 팀 회의에서 제거하기로 결정했었습니다.

현재 피드 북마크 목록조회, 다른 사람 북마크 목록 조회, 내 북마크 목록 조회를 CustomBookmarkRepositoryImpl의 findBookmarks 메소드에서 해결하고 있습니다. 저희가 초반에는 querydsl의 동적쿼리를 최대한 이용하고, 중복 코드를 없애기 위해 통합했었습니다. 그런데 지금와서 보니까 유지보수 하기 어려운 코드 같다고 느꼈습니다. 북마크 목록 조회와, 피드 북마크 목록조회는 어떻게 보면 다른 기능이라고 생각합니다. 따라서 이를 분리하면 코드 중복이 생길지라도 유지보수와 확장성 측면에서는 더 좋아질 것 같다는 생각을 했습니다. 🤔

😎 리뷰어

@ndy2 , @jk05018

github-actions[bot] commented 2 years ago

Unit Test Results

241 tests 241 :heavy_check_mark: 59s :stopwatch:   73 suites     0 :zzz:   73 files     0 :x:

Results for commit 44fca955.

hyuk0309 commented 2 years ago

우기 멘토님이 공유해주신 영상에서 유림님의 말씀이 떠올랏습니다.

클린코드는 짧은 코드가 아니라 원하는 로직을 빠르게 찾을 수 있는 코드다

현재 영속성 계층에 있는 북마크 목록 조회 메소드는 개인적으로 원하는 로직을 빠르게 찾을 수 없다는 생각이 들어요.. 뭔가 코드 중복을 극한으로 줄이기 위해 너무 로직이 너무 뭉쳐진 것 같아요. 🤔

ndy2 commented 2 years ago

저는 사싳 내 북마크 목록 조회랑 타겟 북마크 목록 조회는 같은 기능(이하 타겟)이라고 생각하고

문제가 피드랑 타겟의 구분인데 얘네가 기능이 화면단에서 구분이 안 되는게 이런 유지보수에 앞서서 근본적인 문제? 이런 고민의 원인? 인것같에요

현재 피드목록 조회에서 저희가 작성자 정보를 내려드리긴하는데 화면에 크게 반영이 안되고 타겟 조회 시리즈랑 같은 화면이 잖아요?

특히 타겟에 즐겨찾기 필터링을 먹인 경우는 다양한 작성자가 있을 수도 있는 경우인 데도요

저는 이런점 때문에 두(세)기능의 유지보수 사이클이 같다고 여겨지는 점이 자꾸만 통합하고 싶어지는 원인이라고 생각해요.

추가로 저는 피드 조회와 타겟 조회의 경우 영속성 레벨, 북마크 애그리거트 에서만 딱 끊어서 생각하면 기능 자체는 북마크 목록 조회 하나의 기능이라고 생각해요. (혹시 동의가 안되실까요?)

사실 영속성 레벨에서는 같은 기능이니까 최대한 윗단 (서비스, 컨트롤러)에서 화면에 맞게 바인딩해주는 방식으로 풀고싶어지네요

저는 하나의 메서드에서 로직에서 분기처리를 최대한 dsl 처럼 푸는 방식을 선택했는데

이게 매 분기로직을 내부로 숨겨서 확인이 힘들다면 findcond를 여러개로 분리하는 방법으로 해결할 수 있을것 같아요.

저는 만약 이렇게 한다면 달라지는 분기부분인 joinIf, whereIf부분을 따로 빼서 템플릿 메서드 패턴 같은걸 활용하면 좋겠네요

근데 저는 사실 지금 맘에 안드는 부분은 영속성 계층에서 타겟 조회 피드 조회 등의 용어를 사용하는 부분도 있어요. 만약 findcond를 재 설계 하거나, 리포지토리의 메서드를 나누는 등의 재 설계가 이루어진다면 순수한 도메인,영속성 계층의 용어로만 작성 하고싶은 마음도 있네요

hyuk0309 commented 2 years ago

하하 의견을 크게 세 부분으로 나눠서 제 의견을 말해보겠습니다! 😄

첫 번째 의견에 동의합니다. 타겟 조회와 피드 조회가 현재 화면에 같이 보여져 유지보수 라이프 사이클이 헷갈리는 것 같아요... 근데 저는 타켓 북마크랑 피드 북마크는 개념적으로 다른 것 같아 의견을 드렸습니다. (생각해보니 내 즐겨찾기 목록을 다른 사람이 볼 수 있게 하는게 맞는걸까? 라는 생각이 듭니다.)

두 번째 의견에서 하하는 영속성 계층에서 타겟 북마크 조회와 피드 북마크 조회를 북마크 목록 조회라는 개념으로 하나의 기능이라고 생각하신 것 같아요. 그런데 저는 필터링이 걸리전에 기본 풀이 타겟 북마크와 피드 북마크가 달라서 나눠도 좋겠다는 생각이 들었어요. (타겟 북마크의 기본 풀은 특정 사용자의 북마크고, 피드는 모든 사용자의 공개 북마크기 때문에요.)

세 번째 의견에는 동의합니다. 😄

ndy2 commented 2 years ago

기본 조회 풀을 기준으로 기능을 분리해서 생각/관리 의견에 동의합니다. 허허 의견이랄것 까진 없었는데 언급하신 두 번째 의견에 대해서 제 생각을 한번 써보겠읍니다. 편하게 음슴체로 쓰겠읍니다.

본인의 생각 : 두 조회 (타겟 조회, 피드 조회) 의 기본 풀은 같음

즉 타겟 북마크의 기본 풀은 특정 사용자의 북마크고, 피드는 모든 사용자의 공개 북마크기 때문에요. 요 말에 동의가 되지 안됨 기본 조회 풀 = 필터링 조건에 따라 결정되는 것이 아니라 항상 적용될 수 있는 where 절 로 판단 할 수 있을거 같음

title, category, registered, tag 등의 where 절 은 모든 북마크 조회에서 제공하며 현재 코드의 where(always(...)) 에 나타남

나머지를 뜯어 보겠음 두 조회는 크게 구분하면 세 조회임

피드 조회
다른 머구리 타겟 조회
자신 타겟 조회 여기서 타겟 조회는 각각 즐겨찾기 조회와 비 즐겨찾기 조회로 나눌 수 있음

즉 뜯어보면 아래 5개임

피드 조회	다른 사람 타겟 즐겨찾기 조회	다른 사람 타겟 비 즐겨찾기 조회	자신 타겟 즐겨찾기 조회	자신 비 즐겨찾기 조회

always 를 거르고 현재 어거지로 분기 친 부분을 풀어서 테이블에 써보면 아래와 같음

기준	피드 조회	다른 사람 타겟 즐겨찾기 조회	다른 사람 타겟 비 즐겨찾기 조회	자신 타겟 즐겨찾기 조회	자신 비 즐겨찾기 조회
openType	public 만	public 만	public 만	private 이상	private 이상
사용자 아이디	없음	없음	있음(다른사람만)	없음	있음(자기자신만)

여기서 문제는 우리가 현재 구분하는 기능의 큰 두 덩어리는 위와 전혀 다른 피드 조회, 나머지 4개 (타겟 조회)임 그렇게 큰 두 덩어리로 짤라 보면 명확한 경계가 없음

우리는 오른쪽 4개를 묶어서 한 기능으로 취급하므로 가장 넒은 기준인 openType 기준 - private 이상, 사용자 아이디 기준 - 없음 을 전체 적용하는 수 밖에 업다고 생각함

하지만 openType 기준의 경우 타겟 조회의 특수한 경우 (자기 자신의 비 즐겨찾기 조회) 인 경우에만 private 이상으로 적용되어야 함 그리고 이 룰은 크게 바뀔 일이 없을것 같음

이에 본인은 우리가 구분짓는 두가지 기능 (피드 조회, 타겟 조회)의 전체 풀이 같다고 생각함

현재 같은 방식으로 whereIf 하는게 일반적이지 않은 방식 같기는 함

근데 writerIdEq 나 followedBy 조건의 경우 적용되지 않는 경우 알아서 empty booleanBuilder 로 변환되는 점을 고려하고 openType 이 정책변경으로 간단해진 점, 즐겨찾기 필터링을 위한 bookmarkIdsIn 조건의 호출 형태가 동일한 점을 고려하면 사실 아래 처럼 변경 될 수도 있을 것 같음

코드상 이런 점을 보아도 두 기능의 조회 풀이 같다고 생각함

음슴체 끝

크러쉬가 언급한 내 즐겨찾기 목록을 다른 사람이 볼 수 있게 하는게 맞는걸까? 이 문제에 대해서 전에 한번 프론트에게 질문했었는데 오픈하는것으로 결론 냈었ㅇ읍니다. 사실 저도 크게 이해가 안되는 정책이긴 합니다.

위 문제와 더불어 제가 한번씩 얘기 했던것(타겟 조회의 전체 풀에 대한 뭔가 용어가 있어야 할것 같다) +저희 백엔드가 한마음 한뜻으로 프론트에게 어필도 했던(해당 전체 풀의 화면 노출)

관련된 문제가 계속 이런 고민을 야기 시키네요

hyuk0309 commented 2 years ago

자세한 설명 감사합니다! 👍

다시 생각해보니, 피드 북마크 조회와 타겟 북마크 조회는 기본풀이 동일한 것 같아요. 그래서 영속성 계층에서 하나의 메소드로 해결할 수도 있다고 생각합니다.

기본 풀 관점에서는 피드 목록 조회, 타켓 북마크 목록 조회가 동일한 풀이기 때문에 같이 가져가도 된다고 생각하지만 비지니스 관점에서는 그렇지 않다고 생각해요. 타겟 북마크 목록 필터링 조건과 피드 북마크 조건 필터링 조건은 변경 주기가 다르다고 생각해요(서로 다른 페이지고, 요구사항에 따라 충분히 서로 다르게 변화할 수 있을 것 같아요. 지금도 피드 북마크는 태그 필터링이 요구사항에 없지만, 타겟 북마크 목록은 태그 필터링이 요구 사항에 있어요). 그러면 지금처럼 하나의 메소드로 구현하는 방향은 변경 주기가 다른 코드가 하나의 메소드에 섞여 있기 때문에 유지 보수 하기 어려울 것 같다는 생각이 들어요. 🤔

hyuk0309 commented 2 years ago

위 코드에서 joinIf 절도 결국 피드랑 타켓 북마크를 구분하기 위해 분기 로직이 들어간 것 같아요. 근데 저는 이 코드를 처음 봤을 때는 무슨 의도인지 파악하기 힘들었어요. 😅 타켓 북마크와 피트 북마크를 서비스에서 말아주는 로직을 확실하게 알고 있으면 바로 이해할 수 있지만, 말아주는 로직을 까먹었다면 처음에 보고 이해하기 힘든 것 같아요.

hyuk0309 commented 2 years ago

추가로 저는 피드 조회와 타겟 조회의 경우 영속성 레벨, 북마크 애그리거트 에서만 딱 끊어서 생각하면 기능 자체는 북마크 목록 조회 하나의 기능>이라고 생각해요. (혹시 동의가 안되실까요?)

사실 영속성 레벨에서는 같은 기능이니까 최대한 윗단 (서비스, 컨트롤러)에서 화면에 맞게 바인딩해주는 방식으로 풀고싶어지네요

저는 하나의 메서드에서 로직에서 분기처리를 최대한 dsl 처럼 푸는 방식을 선택했는데

이게 매 분기로직을 내부로 숨겨서 확인이 힘들다면 findcond를 여러개로 분리하는 방법으로 해결할 수 있을것 같아요.

저는 만약 이렇게 한다면 달라지는 분기부분인 joinIf, whereIf부분을 따로 빼서 템플릿 메서드 패턴 같은걸 활용하면 좋겠네요

저는 개인적으로 이 의견이 마음에 들어요 😄

prgrms-web-devcourse / Team-Meoguri-Linkocean-BE