refactor: 서비스 계층의 DTO 변환 로직 위임을 위한 HomeMapper 구현

[jyun-KIM]

Desc

HomeReportService에 비대하게 섞여 있던 데이터 변환(DTO 생성) 및 비즈니스 계산 로직을 분리하기 위해 HomeMapper 클래스를 신설했습니다.

• HomeMapper 신설: 기존 서비스 코드에 있던 stream 기반의 DTO 변환 로직, 다음 복약 시간 계산(calculateNextTime), 그리고 데이터 부재 시의 빈 응답 생성 로직(mapToEmptyHomeReport)을 이관

논의 사항

1. 매퍼 도입 후 서비스와 컨트롤러를 오케스트레이션 구조로 변경하려 했으나, 메서드 시그니처와 반환 타입(DTO -> Entity)이 변경되면서 HomeControllerTest를 포함한 기존 테스트들이 컴파일 에러로 실패하는 상황입니다.

   • 질문: 처음 논의 시에는 테스트를 가급적 건드리지 않기로 했으나, 현재는 Mock 객체 추가 등 최소한의 수정 없이는 테스트 실행 자체가 불가능합니다. 최소한의 Mocking 수정을 통해 컴파일 에러를 해결하고 다음 PR에서 최종 구조를 완성하는 방식으로 진행해도 될까요?

2. 설계 구조 확인: 제가 이해하고 적용하려는 아래의 역할 분담이 우리 팀의 리팩토링 컨벤션과 일치하는지 확인받고 싶습니다.

   • Service: DB에 접근하여 순수한 entity 혹은 원천 데이터를 조회 및 반환
   • Mapper: 서비스에서 받은 엔티티들을 조합하여 최종 DTO를 생성
   • Controller: 여러 서비스와 매퍼를 호출하여 데이터 흐름을 조율하는 오케스트레이터 역할

3. 매퍼 클래스 자체를 검증하는 단위 테스트도 추가해야 할까요?

[coderabbitai]

Important

Review skipped

Auto reviews are disabled on base/target branches other than the default branch.

Please check the settings in the CodeRabbit UI or the .coderabbit.yaml file in this repository. To trigger a single review, invoke the @coderabbitai review command.

You can disable this status message by setting the reviews.review_status to false in the CodeRabbit configuration file.

---

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands and usage tips.}

[kisusu115]

@jyun-KIM

저 같은 경우는 Mapper가 단순 변환 로직을 담당한다고 생각해서, 실제 로직에 포함되어 동작하도록 Spy로 설정하여 기존 테스트를 구동시켰습니다. 다만 팀적으로 추가적인 의논이 필요해보이긴 합니다.

또한 Mapper가 Entity 만 받는다기 보단 DTO 구성에 필요한 값을 받아 DTO로 매핑한다고 생각했는데, 전체적인 의견이 저도 궁금합니다!

[sudo-Terry]

처음 논의 시에는 테스트를 가급적 건드리지 않기로 했으나, 현재는 Mock 객체 추가 등 최소한의 수정 없이는 테스트 실행 자체가 불가능합니다. 최소한의 Mocking 수정을 통해 컴파일 에러를 해결하고 다음 PR에서 최종 구조를 완성하는 방식으로 진행해도 될까요?

테스트를 아예 바꾸지 말자는 이야기는 아니구요, 테스트가 논리적으로 변경되어서는 안된다는 뜻이에요. 테스트가 논리적으로 변경되어야만 테스트가 통과한다는 것은 비즈니스 로직이 달라졌다는 뜻이니까요!

매퍼 클래스 자체를 검증하는 단위 테스트도 추가해야 할까요?

매퍼 클래스에 대한 단위 테스트가 없다면 매퍼 클래스에서 발생한 버그를 테스트에서 잡지 못하겠죠? 테스트가 없을 때 어떤 영향이 있을지를 생각해보면 될 것 같아요

[sudo-Terry]

테스트 없이 머지되면 안됩니다!

[mjkhub]

지현님이 논의사항 1, 2에 작성해주신 내용이 지난 회의에서 제안한 팀 컨벤션과 일치합니다!

다만, 제가 카톡방에도 말씀드렸듯이 해당 컨벤션을 실제로 적용하는 과정에서 테스트를 수정해야하는 번거로움을 느꼈고 팀 컨벤션을 수정하는 것도 나쁘지 않을 것 같다는 생각이 들었습니다!