도메인 리팩토링

[donghoony]

현재 도메인 구조를 아래와 같이 바꿔볼까 하는데 어떻게 생각하시나요?

• 템플릿 (질문, 옵션그룹, 옵션, 섹션, 템플릿)
• 리뷰 (답변)
• 하이라이트
• 리뷰그룹
• 질문, 옵션그룹, 옵션

변경된 점은 질문, 옵션이 템플릿 아래로 들어간 건데, 내부적으로는 여전히 간접 참조하고 있지만 응집도 측면에서 보았을 때는 같이 있는 게 자연스러워서요 (템플릿을 조회할 때 질문을 함께 조회함)
조회는 따로 생각해야 하지 않나? 라고 할 수도 있겠습니다만, 나중에 커스텀 질문 + 템플릿이 들어온다면 (만약) -> 생성도 같은 패키지에서만 죽 이루어지면 괜찮지 않을까 해서용
(서비스 계층화도 고려해보고 있습니다, 이번 쿠폰 미션에서 영감을 조금 받아서요)

패키지가 서로 다른 (분야가 서로 다른) 곳의 Repository를 참조하지 않는 편이 좋을까? 예를 들어, TemplateService에서 ReviewGroupRepository를 참조하지 않도록 한다. ReviewGroupService를 참조하고, TemplateService에서 ReviewGroup을 못 찾는 것에 대한 예외의 책임을 지운다.) 리뷰 그룹을 못 찾는 것에 대한 예외를 묻는 사람(템플릿)이 발생시켜야 할까, 찾는 사람(리뷰그룹)이 발생시켜야 할까? 예외도 클래스다. 예외도 다른 패키지에 있다면 의존해야 한다.

만약 지양해야 한다면, 테스트 코드는 어떤 식으로 작성하는가? 다른 패키지의 서비스를 Mocking해야 하나? (극단적인 경우)

문제라고 생각하는 상황: Review 쪽에서 QuestionRepository를 들춰봄. Template을 받아다가 가공할 수는 없었을까? 더 상위(추상적인) 계층 을 바라볼수록 변경에 유연하다. QuestionRepository에 작성된 모든 쿼리 메서드는 리뷰에서 조회를 위해 사용되고 있다.

지금 하고 있는 일: Template 리턴하는 상황 (매핑) 확인 중, 다음과 같은 두 가지 경우를 고려, Template에서 Section을 묶어 리턴해야 함.

1. 섹션 개수만큼의 쿼리는 괜찮다 (캐싱하면 하나의 템플릿에 대해서는 감당할 만하다)
2. 아니다, 그래도 한 방에 다 가져와야 한다

Dto 조합하는 걸 비즈니스 로직이라고 할 수 있나? 내 생각에는 아니다. @Transactional처럼 한 번에 처리해주면 좋을 텐데... 조회를 위한 로직은 비즈니스 로직이라고 볼 수는 없다고 생각한다.

이야기해야 할 것: 도메인 의존이 생각보다 꼬여 있다. 잘 풀어나가 보자!

[donghoony]

DTO가 너무 계층적으로 되어 있어 문제일까?

TemplateResponse
    ㄴ Template Properties
    ㄴ SectionResponse
          ㄴ Section Properties
          ㄴ QuestionResponse
                  ㄴ Question Properties
                  ㄴ OptionGroupResponse
                         ㄴ OptionGroup Properties
                         ㄴ OptionItemResponse
                               ㄴ OptionItem Properties

[nayonsoso]

오늘도 좋은 고민거리를 제공해주어서 고마워요 아루☺️
간단하게(?) 제 생각을 적어봤어요.
다른 분들의 의견도 들어보고, 리팩터링 해보면 좋겠네요 ㅎㅎ

---

1/ Question, OptionGroup, OptionItem 을 template 패키지 하위로 이동 하는 것에 대해서

1. 처음 의도였던 [변경 전파 영역을 확실히 하자 → 패키지도 명확하게 구분하자]에는 아직도 동의를 합니다.
2. 하지만 지금 코드 상에서 우리의 처음 의도가 잘 지켜지지 않는 것 같습니다.
   예를 들어 지금의 상태에서 Section 이라는 개념 없이 Question 이 바로 Template 에 속하도록 변경이 된다고 해봅시다.
   그럼 template 패키지의 변경에 question.domain 패키지에는 아무런 영향을 주지 않을 것입니다👍
   하지만 1) question과 template을 join하는 JPQL 이 있는 question.repository 패키지나 2) review 패키지에는 영향이 있을 것입니다.
   이는 처음 의도와 다르게 코드가 작성되었음을 의미합니다 🥲
3. 그래서 question 패키지에 있는 것들을 template 하위로 옮기는 것이 오히려 '변경 전파 영역을 확실히 하자'라는 취지에 맞는 것 같습니다.
   대신 이렇게 된다면 'review 패키지에서 template 패키지로의 의존을 하나의 통로로만 가능하게 해서 코드의 복잡도를 낮추는게 어떨까?' 라는 생각이 듭니다. 예를 들어서, 요런 캐시 레포지토리를 만드는 것이죠.

@RequiredArgsConstructor
@Component
public class TemplateCacheRepository {

    private final SectionRepository sectionRepository;
    private final QuestionRepository questionRepository;
    private final OptionGroupRepository optionGroupRepository;
    private final OptionItemRepository optionItemRepository;

    Map<Long, List<Section>> templateIdSections;
    Map<Long, List<Question>> sectionIdQuestions;
    Map<Long, OptionGroup> optionGroupIdQuestion;
    Map<Long, List<OptionItem>> optionItemsByOptionGroup;

    @Cacheable(value = "templateCache", key = "#templateId")
    public List<Question> getQuestionsByTemplateId(long templateId) {
    }

    @Cacheable(value = "templateCache", key = "#sectionId")
    public List<Question> getQuestionsBySectionId(long sectionId) {
    }

    @PostConstruct
    public void init() {
    }
}

4. 또 다른 근거로는, 이번 쿠폰 미션에서 힌트를 얻은 것인데, '데이터소스가 달라진다면?' 이라는 가정을 해보았습니다.
   아루도 알고 있겠지만, JPA 는 하나의 데이터소스에 대해서만 실행이 됩니다.
   따라서 만약 우리가 패키지간의 경계를 완전히 나누어 데이터 소스까지 다른 곳에 둔다면,
   (Question 과 Template 이 다른 데이터베이스에 있게 될 경우)
   아래 JPQL은 실행되지 않을 것입니다.

// QuestionRepository.java
@Query("""
        SELECT q.id FROM Question q
        JOIN SectionQuestion sq
        ON q.id = sq.questionId
        JOIN TemplateSection ts
        ON sq.sectionId = ts.sectionId
        WHERE ts.templateId = :templateId
        """)
Set<Long> findAllQuestionIdByTemplateId(long templateId);

정리하자면, 패키지를 옮기는 것에 동의합니다.
그리고 패키지를 옮긴다면, 다른 패키지로의 의존 통로가 최소화되도록 추가적인 리팩터링을 했으면 좋겠습니다!

[nayonsoso]

2/ 서비스 코드가 다른 패키지의 Repository 를 직접 참조하지 않게 하는 것에 대해서

예를 들어, TemplateService에서 ReviewGroupRepository를 참조하지 않도록 한다. ReviewGroupService를 참조하고, TemplateService에서 ReviewGroup을 못 찾는 것에 대한 예외의 책임을 지운다.) 리뷰 그룹을 못 찾는 것에 대한 예외를 묻는 사람(템플릿)이 발생시켜야 할까, 찾는 사람(리뷰그룹)이 발생시켜야 할까? 예외도 클래스다. 예외도 다른 패키지에 있다면 의존해야 한다.

아루가 말한 문제점에 대해서 저도 공감하고, 이를 해결한다면 훨씬 좋은 코드가 될 것이라 생각합니다.
하지만 대안으로 제시한 Service 대신에, 다른 방법이 좋겠다고 생각했어요.
그 이유는 아래와 같습니다.

1. 만약 어떤 서비스 코드에서 서비스 코드를 호출한다면, 패키지간 직접 양방향 참조를 하게 됩니다.
   예를 들어서, 이전까지는 xxx.service 에서 yyy.repository 로, 그리고 yyy.service 에서 xxx.repository 로의 참조가 일어났습니다.
   큰 범위에서는 패키지 순환참조이긴 하지만, 패키지가 직접적으로 참조되는 것은 아니었습니다.
   하지만 만약 도메인을 가져오기 위한 서비스를 만들고 이를 다른 서비스에서 호출하게 한다면,
   xxx.service 에서 yyy.service로, 그리고 yyy.service에서 xxx.service로의 직접적인 순환 참조가 일어날 것입니다.
   이런건 바람직하지 않은 참조라고 생각합니다.

2. 또한 아루가 앞서 언급한 것처럼, ‘단순한 조회는 비지니스 로직이 아니다’라는 측면에서도,
   도메인 조회를 위한 서비스 클래스를 만드는 것은 좋지 않다고 생각합니다.

3. (이건 번외로) 아루가 전에 ‘각 패키지가 api 통신을 하는 것처럼 만들고 싶다’ 라고 했었는데,
   사실상 그것을 완벽하게 하려면 dto 가 등장해야 한다고 생각합니다.
   우리 프로젝트의 방향성이 그것이 아니라면, 그런 구조를 가정해서 구현하는 것은 너무 많은 공수가 들지 않을까 생각해요😅

따라서 제가 생각해본 대안은.. 일급컬렉션의 특성을 가진 repository를 만들어서 이런 역할을 위임하는 것입니다.
예를 들어서 xxxReader 라는 것을 만드는 등..
아니면 현재의 xxxRepository 를 xxxDao 로 만들고 xxxRepository 라는 클래스를 만들어서 여기에 함수들을 만드는 것도 방법일 것 같아요.

이렇게 한다면 아래와 같은 함수들도 repository 안으로 옮길 수도 있지 않을까요..?
(조회 로직만을 모아둔 레포지토리에)

Map<Long, List<OptionItem>> optionItemsByOptionGroup = optionItemRepository.findAllByQuestionIds(questionIds)
        .stream()
        .collect(Collectors.groupingBy(OptionItem::getOptionGroupId));

[donghoony]

xxx.service 에서 yyy.service로, 그리고 yyy.service에서 xxx.service로의 직접적인 순환 참조가 일어날 것입니다.

패키지 간에도 위계가 존재할 텐데, 어떻게 서비스 간 순환 참조가 일어나나요? 예를 들어,
Review -> Template은 존재하더라도 Template -> Review는 서비스 계층끼리 의존하더라도 발생하지 않을 참조 관계라고 생각해요.

[donghoony]

또한 아루가 앞서 언급한 것처럼, ‘단순한 조회는 비지니스 로직이 아니다’라는 측면에서도,
도메인 조회를 위한 서비스 클래스를 만드는 것은 좋지 않다고 생각합니다.

View용 dto를 리턴하지 않고, 비즈니스 로직 (정렬 방식, 한 번에 내려 줄 개수 등)을 가지고 있을 수 있습니다. 아래 서비스에서는 엔티티 내지는 도메인을 반환하고 맨 위에서 소통하는 서비스가 dto를 내려주면 되지 않을까요?

[donghoony]

(이건 번외로) 아루가 전에 ‘각 패키지가 api 통신을 하는 것처럼 만들고 싶다’ 라고 했었는데,
사실상 그것을 완벽하게 하려면 dto 가 등장해야 한다고 생각합니다.
우리 프로젝트의 방향성이 그것이 아니라면, 그런 구조를 가정해서 구현하는 것은 너무 많은 공수가 들지 않을까 생각해요😅

동의해요~ 하지만 이렇게 극단적으로 생각해봐야 좀 풀릴 것 같았습니다

[nayonsoso]

아하 패키지 간 위계를 신경쓴다면 양방향 참조는 일어나지 않겠네요😄👍

지금 우리 코드에서 xxx.service 에서 yyy.repository 로, 그리고 yyy.service 에서 xxx.repository 로의 참조가 일어나는 부분이
두 부분 있긴 한데, 이건 적절하게 참조 방향 통일해주면 해결될 것 같습니다 ㅎㅎ

1.template ↔️ reviewgroup
2. highlight ↔️ review

[nayonsoso]

View용 dto를 리턴하지 않고, 비즈니스 로직 (정렬 방식, 한 번에 내려 줄 개수 등)을 가지고 있을 수 있습니다.

이 부분에서 질문이 있는데, 그럼 앞서 말한 "리뷰 그룹이 없을 때 예외를 던지는 책임" 말고도 다른 부가기능도 들어가야 한다고 생각하시나요?

예를 들어서 지금 mapper 에 요런 함수들이 굉장히 많은데

Map<Long, List<OptionItem>> optionItemsByOptionGroup = optionItemRepository.findAllByQuestionIds(questionIds)
        .stream()
        .collect(Collectors.groupingBy(OptionItem::getOptionGroupId));

이런 것들도 도메인을 반환하는 서비스 안에 넣어야 할지?
아니면 예외를 던지지 않는 것들은, 레포지토리의 함수를 호출하는 것에서 그치는지?
아루가 말한 비지니스 로직을 수행하며 도메인을 내려주는 서비스는 어떤 다른 책임을 갖는지 한가지 예시를 더 들어줄 수 있나요?🥹✨

[nayonsoso]

3/ Dto 조합하는 걸 비즈니스 로직이라고 할 수 있나?

이제는 우리가 불편함을 느끼고 있으니..
도메인을 넘겨서 Dto 를 만드는 함수를 Dto 안에 만들어도 된다 생각합니다.

[Kimprodp]

1.템플릿 패키지에 질문, 옵션 등이 지금 생각하면 왜 같이 안 묶여져있는지 모르겠네요. 우리의 기준은 명확했어요. 생명주기, 변경..근데 질문와 옵션 그룹은 모두 템플릿과 섹션에 종속되어서 템플릿이 삭제되면 모두 삭제 되어야 한다고 생각해요. 만약 하나의 질문이나 옵션이 다른 템플릿에서도 사용되면 생명주기가 다른 것이 아니냐 할 수 있는데, 옵션이나 질문을 재사용하는 것 보단 템플릿에 맞춰서 새로 만드는 게 맞다고 생각합니다. 만약 여러 템플릿에서 사용되고 있는 질문이 있는데 한 템플릿에서 그 질문의 의도가 잘못 사용되고 있어서 수정된다면 그 질문을 사용하는 다른 템플릿에서도 영향을 끼칠 수 있다고 생각해요.
그리고 이 엔티들을 같이 묶어놓으면 조회나 등록이나 모두 편해지겠네요.

2.패키지 의존 관련.이 부분은 일단 그냥 사용하려면 어쩔 수 없는 부분이라고 생각해요.(물론 각각의 레포를 다 넣는다면 되겠지만 비효율적) 그래서 전에 하던 게 템플릿류들을 모두 캐싱해서 가지고 있는 클래스를 하나 만들고 그걸 사용하게 하는거였죠(산초가 올린 클래스랑 비슷) 근데 그때는 캐시에 대해 잘 몰라서 적용이 한참 걸렸는데 스프링 캐시 사용하면 깔끔하게 할 수 있을 것 같아요. 이렇게 하면 서비스마다 템플릿 재료들이 필요하면 그냥 저기서만 가져다 쓰면 되니 저 클래슨느 의존받기만 해서 방향이 일관됩니다. 그리고 이 캐시를 사용하는 곳도 저는 서비스 보단 persistence 쪽이 맞다고 생각해요. 서비스에서는 단순히 persistence에서 필요한 재료를 찾은 것 뿐이고 그 재료를 DB에서 가져올지, 캐시데이터를 가져올지는 몰라도 되는거죠. 그래서 저는 레포지토리 추상화를 사용해서 xxxRepository를 service 패키지에
넣고 실제 구현체는 persistence 에 두면 실제 구현체가 jpa, 캐시 들을 가지고 사용하면 깔끔할 것 같아요. (포트 어댑터 패턴이네용)

3.DTO는 생각할게 많은데 일단 근본적으로 서비스 특성을 고려해도 뎁스가 너무 깊지 않나 생각합니다. 이것부터 논의해보면 좋을 것 같아요.

[donghoony]

Repository를 Service가 아닌 도메인에 넣는 건 어떨까요 ?

[donghoony]

읽기/쓰기 DB 시점의 불일치가 존재해도 된다. 그 근거로는:

1. 리뷰 작성자가 리뷰를 작성했다고 알리는 경우는 적다: 리뷰는 익명으로 작동한다.
2. 리뷰를 작성하는 시점, 확인하는 시점이 동일하더라도 바로 보여줄 필요가 없다.

[nayonsoso]

네오의 조언

우테코 이상의 것을 해보자

지금까지 우테코에서 했던 것은 "학습에 대한 것"
하지만 이제는 우테코에서 했던 것 이상의 것을 해야 한다.
생각해보면 회사에서는 엄청 큰 규모의 플젝을 할테니...
현실과 이상 사이에서 선을 잘 타야 한다 🏂

네오가 제안한 해결 방법

문제 시 했던 조회에 대한 코드가 복잡한 것은 아래와 같은 해결 방식이 있을 수 있다.

• 프로젝션과 같은 것을 사용해서 엄청나게 긴 sql 을 만들거나
  성능이 문제가 될거라고 생각한다면 직접 측정해보면 됨 → ❗️그리고 인덱스 같은게 제대로 되어있으면 성능은 이게 더 좋을 수 있다❗️
  join 이 있다고 모든 데이터를 읽는게 아니고, 반드시 성능이 안좋은 것은 아니니까..
• dto 에 반드시 필요하지 않은 정보까지 넣어주고 클라이언트에서 선택해서 쓰게 하라고 하거나
• 클라이언트에서 api 를 여러번 호출해서 조합해서 쓰게 하라고 하거나

우리가 선택한 것에 대해서 감수를 해야 한다

우리의 의도였던 '각 패키지가 다른 DB 를 사용한다고 가정하는 것' 역시 우리의 선택이다.
그런데 이런 선택을 했다면, 복잡한 join 을 쓰지 않는게 맞고,
이로 인해서 조회를 위한 서비스 코드가 길어지는 것은 감수를 해야 한다.

그리고 우리가 dto 재사용을 막게 한 것도 우리의 선택.
이로 인해서 조회를 위해서 각각의 mapper 를 만들어 주는 것도 우리가 감수를 해야 하는 부분인다.

깊은 dto 에 대해서 네오가 진단한 원인 🩺

우리가 진짜로 msa 환경을 고려했다면, ❗️클라이언트 입장에서 필요한 것들을 조회를 해오는게 맞았다❗️
msa 환경을 고려한 것 자체가 문제일 수도 있다.
아니라면 msa 환경이라는 가정하에, dto 를 조합하는 프록시 역할을 하는 서버가 하나 더 있었어야 했다.

PS) API 에서 내려줘야 하는 응답에 대한 조언

이전에는 하나의 페이지에 하나의 API 호출이었다. 그래서 클라이언트의 변화마다 서버가 변해야 해서 힘들었다.
그래서 REST API 가 나왔고.. 클라이언트는 필요한 것들을 조합해서 쓰면 됐었다.
REST API 는 커뮤니케이션 비용을 줄이기 위해서 나왔다.
그런데 이렇게 되면 호출이 여러번 일어날 수 있다는 문제가 있을 수 있는데...!
이건 또 다른 학습 포인트 : under fetching and over fetching (GraphQL)로 해결할 수도 있다.

[donghoony]

1. 한방쿼리 써라
2. 클라이언트가 조합하게 해라
3. 조합하는 하나의 패키지를 따로 둬라

이 중 하나 골라라

우리가 지금 힘들었던 건.. 분산 환경과 우리가 조합하는 것 모두 욕심을 부렸기 때문이다.