Skip to content

Driving Adapter 와 UseCase 구현 #24

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

Merged
merged 56 commits into from
Jan 29, 2025
Merged

Conversation

silberbullet
Copy link
Contributor

@silberbullet silberbullet commented Jan 15, 2025

Pull Request

Issues

Resolves #17

Description

How Has This Been Tested?

  • 테스트 환경: OpenJDK 21(Amazon Corretto 21), kotest (mockk 라이브러리) with FreeSpec.
  • BoardQueryControllerTest.kt 와 BoardCommandControllerTest.kt 는 Run 명령어로 테스트 결과 확인이 가능합니다.

Additional Notes

Kotest Spec 인스턴스가 Spring Context의 Bean 주입을 받기 위한 라이브러리 추가

testImplementation("io.kotest.extensions:kotest-extensions-spring:1.1.3")

Contoller 계층 Test를 위해 설정한 annotation

@ExtendWith(SpringExtension::class) // Spring Context에 생성된 빈을 JUnit과 연결
@WebMvcTest(BoardCommandController::class) // Spring MVC 환경에서 웹 계층(Controller)에 대한 테스트를 진행할 때 사용
@AutoConfigureMockMvc  // 해당 테스트 클래스에 MockMvc 객체를 자동으로 주입 

🤔 더 나은 테스트 코드를 위한 고찰

테스트 코드의 고찰할 기회를 주신 @merge-simpson@shin-jingyu 님에게 감사드립니다.

✅ 테스트 코드는 해당 파일에 역할과 기능 파악이 쉬워야 한다고 생각합니다.

  • 대괄호([])를 이용한 테스트 케이스명 기능 정의
  "[POST] 게시판 생성 요청" - {
        // given
        val boardCreateResponse = boardDomain

        "[정상 요청] 제목과 내용이 3글자 이상과 공백이 아닐 때" - {
            // when
            val createCommand = BoardCreateCommand("테스트게시판", "테스트게시판내용")

            "2xx 응답 상태 반환" {

추후 테스트 명세서 다운로드 시, 해당 클래스의 기능들 파악 용이

image

✅ 테스트 코드는 읽기 쉽지만, 어느정도 노가다는 들어간다고 생각합니다.

  • 사용자는 해당 테스트의 설정은 신경 쓰지 않도록, Mocking 한 클래스의 행위는 밑으로 둡니다.

테스트 클래스 시작점
image

테스트 클래스 종료점
image

✅ 추후 클래스의 기능 변경, 추가 및 제약사항이 생기더라도 테스트 코드 수정이 용이로워야 한다고 생각합니다.

  • BDD를 기반으로 테스트 케이스 명 작성, when절의 성격을 대괄호([])로 나타내어 추가된 테이스 케이스 추가 용이
image

라이브 코드 리뷰로 리뷰 받은 항목들

  • Stream.map()으로 데이터를 매핑하여 리스트로 변환한 뒤 PageImpl에 담아 반환하는 방식 대신, Page.map()을 사용해 간결하게 작성

shin-jingyu and others added 30 commits January 5, 2025 23:17
- Add `BoardDtoMapper.java` and DTO classes for board operations
- Add `BoardReadUseCase` for board read logic
- Update `build.gradle.kts` with validation and MapStruct dependencies
- Adapter(Port 구현체)를 아직 구현하지 않았으므로, 빌드 에러
- 관련 에러로 인한 테스트 실행 불가
- annotationProcessor 관련이 없어, Lombok이 컴파일 타임에 정상적으로 동작하지 않는 문제가 발생함.
- 05ab61 커밋에서 추가함으로써, Service에 Port를 주입이 가능해지고, bulid와 test가 정상적으로 동작할 수 있게 됨.
- Added Lombok dependencies
- Created BoardQueryController and BoardQueryDto for handling board queries
- Updated BoardDto and BoardDtoMapper
@merge-simpson merge-simpson self-requested a review January 25, 2025 19:02
Copy link
Member

@merge-simpson merge-simpson left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

👍 전체적인 작업에 고민과 발전이 묻어나고, 완성도가 높습니다!

BoardCommandControllerBoardQueryController도 각 테스트 코드도 어느 하나 고민이 묻어나지 않은 곳이 없었습니다! 🚀

고민하시는 만큼 많은 것을 찾아 보고, 공부도 많이 하신 것 같구요! 📖 🧐

그 고민을 팀원에게도 열심히 공유하며 전달해 주시고, 사람들의 인사이트를 최대한 이끌어 주셔서 정말 감사드립니다! 👍

이하 수정 사항 및 제안은 사소한 항목들이며, 이번 병합 이후 새 이슈를 작성할 때 누락되지 않게 하기 위해 이번에 작성해 두었습니다! 이번 병합은, 기존 수정사항이 모두 반영되었으며 동작에 문제가 없기 때문에 이하 수정사항과 별개로 approve하기로 팀 내 회의에서 결정하였습니다.

작업의 수준이 높아서 많은 분들이 참고할 수 있도록
이대로 병합하면 좋을 것 같습니다! 🙏 🥹

🚀 🏅 Approve!

import me.nettee.board.application.usecase.BoardDeleteUseCase;
import me.nettee.board.application.usecase.BoardUpdateUseCase;
import org.springframework.http.HttpStatus;
import org.springframework.web.bind.annotation.*;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🎨 와일드카드 임포트(*, asterisk) 사용을 삼가는 것이 좋습니다.

구글 Java 코드 스타일 가이드에서 제안하는 내용입니다.

다음은 최대한 사용을 삼가는 것이 좋습니다.

  • 와일드카드 임포트(import ~.*)
  • 정적 와일드카드 임포트(static import ~.*)
  • 정적 임포트(static import ~)

단, 정적 임포트(static import ~)는 이 파일 내에서 자주 사용되거나, 기타 이유로 팀이 적절하다고 생각 시 팀 내에서 사용을 허용할 수 있습니다. (유틸리티 함수 등)

private final BoardCreateUseCase boardCreateUseCase;
private final BoardUpdateUseCase boardUpdateUseCase;
private final BoardDeleteUseCase boardDeleteUseCase;
private final BoardDtoMapper boardDtoMapper;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💊 기호의 차이이지만, mapper로 명명하는 것을 제안드립니다.

boardDtoMapper는 그 역할이 매우 단순하며, 이 클래스 내에서 가장 기본이 되는 매퍼입니다.

따라서 복잡한 기능을 수행하는 다른 빈들에 비해 단순한 네이밍으로도 충분히 식별할 수 있습니다.
전체적으로 코드를 읽을 때도 중요한 작업을 수행하기보다 보조적으로 사용되므로,
mapper라는 이름만으로 충분히 그 역할을 잘 표현할 수 있다고 생각합니다.

하지만 취향의 차이이기 때문에, 생각하시는 것과 다르다면 어떤 부분에서 다르게 생각하시는지 등 편하게 말씀 부탁드립니다!

Comment on lines 14 to 16
import org.mockito.ArgumentMatchers.any
import org.mockito.Mockito.doNothing
import org.mockito.Mockito.`when`
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💊 프로덕션 코드와 달리 테스트 코드에서는 와일드카드 임포트를 허용합니다.

물론 반드시 그래야 하는 것은 아닙니다! 일반적인 습관이 그렇다고 생각하는 거고, 이번 실습에선 이런 부분보다는 테스트 코드의 작성 방식에 집중해 주신 것 같아서 오히려 좋았습니다! 💯

Comment on lines 31 to 32


Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🎨 섹션 구분은 '싱글 블랭크 라인(한 줄짜리 빈 줄)'만 허용됩니다.

구글의 Java 코드 스타일 가이드에 있는 항목입니다!

lateinit var boardList: List<BoardReadSummaryModel>

"[GET]게시판 상세 조회" - {

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🎨 함수의 첫 줄 및 마지막 줄은 공백 줄로 두지 않는 것을 권합니다.

이것은 구글의 자바 코드 스타일에 명시된 내용은 아니고, 해석에 따른 것입니다.

@shin-jingyu
Copy link
Contributor

@merge-simpson 확인 감사합니다 👍


#24 (comment)
#24 (comment)
#24 (comment)

놓쳤던 부분을 확인해주셔서 감사합니다. 수정 완료했습니다.

@merge-simpson
Copy link
Member

merge-simpson commented Jan 28, 2025

@silberbullet 님, 작성해 주신 내용을 이제 확인했습니다!

안 그래도 명시적 매핑이나 자바 함수를 호출하는 매핑, 내부적인 매핑 등을 전달드리려고 했습니다!
노션에 일부 내용을 짧은 문장들로 소개한 적은 있지만, 자세한 소개를 할 만한 타이밍이 좀처럼 없었네요.

💊 우선 생각하시는 부분과 제 생각의 차이를 좁힐 수 있게 견해를 먼저 전달드리겠습니다!

🎨💊 명시적인 매핑은 제한적인 케이스에 필요하다고 생각합니다.

명시적인 매핑이 제한적인 용도라고 생각하는 이유는 다음과 같습니다.

장래에 작업적인 부담

말씀해 주신 명시적인 매핑은 종종 필요에 따라 사용해 오던 방식입니다.
하지만 모든 케이스에 매핑 대상을 명시하는 것은 복잡한 매핑 전략에서 과도한 작업 부담이 될 수 있습니다.

복잡한 객체의 자동화된 매핑

MapStruct를 사용하는 이유 중 큰 부분을 차지하는 것은 그 성능도 있겠지만, 역시 편의성을 놓쳐선 안 된다고 생각합니다.
그중에서도 '복잡한 객체 구조'의 매핑이 매우 수월한 점이 그렇습니다.

덧붙임: ModelMapper를 비선호하는 사례를 보면, 다음과 같은 이야기가 있습니다.

복잡한 객체 매핑에서 과도하게 생각을 요구하여, 직접 함수를 짤 때보다
오히려 시간을 더 소요하게 될 수 있습니다.

이러한 사례와 유사하게 되는 것을 피하기 위해 MapStruct 사용 시 과도한 명시를 피하는 것도 방법이 될 거라 생각합니다.

MapStruct는 복잡한 포함 관계를 갖는, 다차원 객체 매핑에서 특히 효과적입니다.
예를 들면, 다른 매핑함수의 내부에서 필요한 매핑 함수를, 우리가 직접 제공하는 매핑 전략을 사용할 수 있습니다.

// ModelA는 내부에 NestedModelA를 포함합니다.
// ModelB는 내부에 NestedModelB를 포함합니다.
// 내부에서 NestedModelB는 NestedModelA 타입으로 매핑되어야 합니다.
ModelA toTarget(ModelB model);

default NestedModelA anotherFunction(NestedModelB nestedModel) {
    // 이곳에서 두 내부 필드의 타입 간 변환 전략을 명시적으로 작성할 수도 있습니다.
    // 이 함수는 우리가 직접 호출하지 않아도, 위에 작성된 toTarget(...) 함수에서
    // NestedModelB to NestedModelA 매핑에 자동으로 사용됩니다.
    return ...
}

애노테이션을 통한 명시적 매핑은 다음 경우 필요하다고 생각합니다.

  • 매핑 대상의 이름이 중복될 수 있을 때
  • 매핑 소스의 이름이 중복될 수 있을 때
  • 매핑 대상을 의도적으로 무시할 때
  • 매핑 소스를 의도적으로 무시할 때
  • 매핑 소스를 의도적으로 덮어쓰기 할 때
    @Mapping(target = "title", source = "newTitle")
    Domain toDomain(Dto dto, String newTitle); // dto.title은 무시됩니다.

그 외 오류가 발생하는 케이스가 아니라면 명시적인 매핑 애노테이션이 반드시 필요하지 않아 보여서 예시에서 가급적 제외했습니다.
(특히 레이어드 예제에서는 적응하기 쉬운 코드를 위해 더욱 간단한 형태의 예시가 필요했습니다.)


다음 두 코드가 같은 역할을 합니다.

기존 코드 A

@BeanMapping(ignoreByDefault = true)
@Mapping(target = "title", source = "title")
@Mapping(target = "content", source = "content")
Board toDomain(BoardCreateCommand command);

동일한 코드 B

Board toDomain(BoardCreateCommand dto);

저는 여기서 편의성을 위해 코드 B를 선호합니다.
사용성을 택하지 않는다면, MapStruct 사용 이유가 꽤나 반감된다고 느끼고 있습니다.

@silberbullet silberbullet merged commit 7ce0835 into main Jan 29, 2025
@silberbullet silberbullet deleted the feature/driving-adapter branch February 6, 2025 03:18
@merge-simpson merge-simpson added review: provided Reviews have been provided and removed review: ing Reviews are in progress and taking time labels Feb 17, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
in: board Issues in the board module review: provided Reviews have been provided type: feature 새로운 기능 또는 기능적 개선 A new feature or enhancement
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Driving Adapter 와 UseCase 구현
5 participants