Driven(Command) 구현 및 단위 테스트 #26
Conversation
github-insu
added
type: feature
github-insu
requested review from
sweetykr7,
wch-os,
taewookimm,
shin-jingyu,
wowddok99,
jilpoom,
merge-simpson and
silberbullet
and removed request for
taewookimm
There was a problem hiding this comment.
25/01/15 라이브 코드 리뷰 결과
@taewookimm 님, @github-insu 님, 유익한 작업 결과 감사드립니다. 👍
정기회의에서 거론된 리뷰 리포트 입니다.
다음 내용을 참고해서 작업을 이어가 주시면 좋겠습니다.
❗ @EnableJpaAuditing을 메인 애플리케이션 클래스에서 제거
- EnableJpaAuditing JPA 라이브러리에 포함된 기능입니다.
- Application의 메인 클래스에 포함하기보다는 JPA와 관련된 설정이나 기능을 별도로 관리하는 것이 좋습니다.
- 이를 통해 코드의 응집도를 높이고, 구조를 더욱 명확하게 하는것이 좋아보입니다
❗ Adapter는 Soft Delete를 결정하지 않습니다.
- 소프트 삭제 여부는 서버의 정책에 따라 결정되며, 이는 Application 레이어에서 관리하는 것이 좋다고 생각합니다.
- Driven Adapter에서는 delete 메서드는 실제 삭제(하드 딜리트)를 수행하는 의미로 사용해야 합니다.
- 존재 여부만 확인할때는 findById() 보다는 existsById() 를 사용하는 것이 좋다고 생각합니다.
- findById -> existsById: 조회하는 데이터 양이 줄어들기 때문에 성능상 이점을 기대할 수 있습니다.
src/main/java/me/nettee/board/adapter/driven/persistence/BoardCommandAdapter.java
Outdated
Show resolved
Hide resolved
…to feature/board-driven-command
…s using abstract classes
There was a problem hiding this comment.
👍 장기간 바쁘신 중에도 합을 맞춰 가며 만들어 주신 유익한 결과물 감사드립니다!
역시 @taewookimm 님의 작업은 명료하고 깔끔해서, 사소한 실수 찾기 외에는 리뷰를 남겨 드릴 부분이 거의 없었던 것 같습니다!
그리고 기존 방식에서 세세한 부분을 개선할 수 있었습니다.
회사 일과 외부 행사로 쉴 새 없이 바쁘신 일정 와중에도:
JPA 엔티티 빌더의 대상을 적절한 생성자로 옮기고, JPA Auditing을 활성화하셨고, 컬럼에 알맞은 제약조건을 추가하셨다가 논의 후 팀의 결정에 따라 초기 개발에서는 느슨한 데이터 제약 조건을 적용해서 변화에 유연하게 대응하기로 했던 것들이 기억납니다!
그리고 다른 계층의 문제들에도 참여하시면서, 팀 내 방향을 잘 파악하고 멤버들에게 적절한 아이디어를 제시해 주셨던 것 같습니다!
바쁘신 중에도 정말 감사드립니다.
👍 페어 간 적절한 소통과 협업
페어 안에서 작업 분담이 명료하면서도,
서로의 작업 영역까지 파악하기 위해 소통이 끊이지 않았던 것으로 압니다.
서로 바쁠 때도 배려하며 소통하여, 완성해 가시는 게
다른 멤버들에게도 잘 전달되었던 것 같습니다!
테스트 코드
👍 JPA + Kotest + 트랜잭션 테스트에 기다리고 있던 많은 시행착오를 극복해 온 결과물이 잘 담기고 있습니다! 😎
단위 테스트가 괜찮을지 시도하는 것부터 시작해서 🤔
결국 persistence 어댑터 레이어는 단위 테스트를 제외하고 테스트 DB로 통합 테스트부터 하는 것이 충분히 효용이 있다고 논의되어 큰 방향을 수정했었습니다.
그 후로도, 다른 계층의 테스트에 비해 시행착오가 더 많이 숨어 있는 테스트였죠. 🧐
깊은 고민의 흔적이 잘 녹아 있고, 그 결과 JPA 기반 테스트 코드의 구조를 개선할 수 있었다고 생각합니다.
리서치하신 내용들도 꾸준히 공유해 주셨고, 다른 어댑터 작업자들도 영감을 받아 서로 리서치를 주고 받았습니다. 👍
테스트 코드의 작성이나, 표현에 대한 세세한 방식은 앞으로도 쭉 논의해 보면 항상 나은 방향이 제시될 것 같습니다!
잘 작성해 주셔서 감사드립니다. 🚀
💯 실습에 맞는 테스트 사례만 요약하여 잘 정리해 주셨습니다!
다른 페어에도 언급한 내용이지만, 만약 불필요 테스트 케이스를 과도하게 추가했다면 실습에 참여하거나 이 결과를 참관하는 사람들이 파악하며 적응하기에 오히려 혼란스러웠을 거라고 생각합니다. 거부감을 증폭시킬지도 모르죠. 하지만 딱 명시적인 테스트만 작성하여 불필요한 작업 낭비를 줄이고 테스트 코드의 '방식'을 개선하는 데에 집중할 수 있었던 것 같습니다!
🛠️ 탐구적인 논의와 트러블 슈팅
가장 많은 트러블 슈팅 문서를 남겨 주신 것 같습니다! 👍
다른 구성원이 확인하기 쉽도록 글이 잘 정리되었습니다.
다음 트러블 슈팅들이 인상 깊었습니다.
JPA 테스트 시, Auditing 시간 정밀도 문제
이 문제 해결에서는 JPA Repository 사용 시, save() 함수로 반환된 엔티티의 시간 필드들이 나노초, 업데이트 함수로 H2 인메모리 데이터베이스로부터 얻어 온 엔티티의 시간 필드들이 마이크로초 단위의 시간 정밀도를 갖고 있는 문제가 배경이었습니다.
그 간극을 없애기 위해 테스트와 프로덕션에서 각각 어떤 정책에 따라 어떤 조치를 할 수 있는지 여러 단계에 걸쳐 고민하고 시도하며 결과를 제시하는 과정이 매우 인상 깊었습니다.
JPA 테스트 공통 설정
이 문제 해결에서는, JPA 테스트에서 우리가 공통으로 '자주' 사용하는 설정을 어떻게 관리할지 여러 멤버들과 논의하며 아이디어를 도출했습니다.
'자주'는 '항상'은 아닙니다. JPA 의존성이 없는 테스트에서는 특히나 제외되어야 하면서도, JPA를 사용하는 테스트에서는 대부분 적용할 설정이었죠.
공통 설정 파일에만 의존하기보다, 우리가 선택할 수 있는 구조를 만들기 위해 노력한 팀워크가 좋았고, 코틀린 신택스에 아직 구체적인 부분은 적응 중이면서도 잘 모르는 신택스에도 시도해 보며 정리된 코드를 만들어 낸 활동이 인상 깊었습니다.
| 🚀 | 🏅 Approve! |
| 🥇 | 트러블 슈팅 선두주자 |
| package me.nettee.board.adapter.driven.persistence; | ||
|
|
||
|
|
||
| import lombok.RequiredArgsConstructor; |
There was a problem hiding this comment.
🎨 연속적인 공백줄이 추가되었습니다.
구글 Java 코드 스타일 가이드에서 단일 공백줄(single blank line)만 허용하고 있습니다.
사소한 부분이지만 눈에 보여서 말씀 남겨 드립니다!
| import org.springframework.boot.autoconfigure.SpringBootApplication; | ||
|
|
||
|
|
||
| @SpringBootApplication |
There was a problem hiding this comment.
🎨 연속적인 공백줄이 추가되었습니다.
구글 Java 코드 스타일 가이드에서 단일 공백줄(single blank line)만 허용하고 있습니다.
사소한 부분이지만 눈에 보여서 말씀 남겨 드립니다!
| import jakarta.persistence.*; | ||
| import lombok.*; |
There was a problem hiding this comment.
🎨 와일드카드(*) 임포트를 사용하지 않습니다.
구글 Java 코드 스타일 가이드에서 다음 임포트를 사용하지 않거나 제한적으로만 허용합니다.
(테스트 코드에서는 와일드카드 임포트도 허용합니다.)
- 와일드카드 임포트 (
import ~.*;) - 정적 와일드카드 임포트 (
import static ~.*;) - 정적 임포트 (제한적으로 허용)
사소한 부분이지만 눈에 보여서 말씀 남겨 드립니다!
참고: 인텔리제이에서 임포트 설정
다른 분들도 참고하실 수 있어서 덧붙입니다!
인텔리제이 사용 시 settings(preferences)에서
- Editor > Code Style > Java 항목을 선택합니다.
- 선택된 윈도우에서 'imports' 탭을 선택하고, 와일드카드 임포트로 자동으로 전환하는 개수를 999 등 큰 숫자로 바꿉니다.
설정하시는 김에 inner class 임포트도 허용해 주시면 좋습니다.
There was a problem hiding this comment.
@merge-simpson 좋은 내용 감사합니다. 혹시 아래와 설정을 하는게 맞는지 궁금합니다.
There was a problem hiding this comment.
안녕하세요, @sweetykr7 님!
올려 주신 설정 중 다음 항목들을 언급했습니다!
- 내부 클래스를 임포트하기 편리하도록 설정합니다.
- 와일드카드 임포트로 처리되는 개수를 늘립니다. (정적 임포트 포함)
개수 설정은 특정 값을 넣을 때 설정을 off 할 수 있는지 확인해 보진 않았습니다. 999로 충분하다고 생각했습니다.
(예를 들어 0을 넣어 보고 테스트하는 것도 좋은 시도라고 생각합니다.)
캡처해서 올려 주셔서 설명이 한층 명료한 것 같습니다!
자료를 보충해 주셔서 감사합니다. 👍
| private Instant updatedAt; | ||
|
|
||
| private Instant deletedAt; | ||
| // private Instant deletedAt; |
There was a problem hiding this comment.
👍 애플리케이션 및 도메인 계층의 작업이지만, 조치 감사드립니다!
방금 서비스 쪽 브랜치를 확인해 보니, 이 작업이 누락되어 있었네요!
|
|
||
|
|
There was a problem hiding this comment.
🎨 연속적인 공백줄이 추가되었습니다.
구글 Java 코드 스타일 가이드에서 단일 공백줄(single blank line)만 허용하고 있습니다.
사소한 부분이지만 눈에 보여서 말씀 남겨 드립니다!
| @CreatedDate | ||
| private Instant createdAt; |
There was a problem hiding this comment.
💬 createdAt 컬럼을 수정 불가로 관리하는 게 어떨까 합니다.
| 💬 | 이번 병합 이후 다시 논의하려고 하는 부분이었습니다! 나중에 깜빡하지 않으려고 작성하였습니다. |
주요 시간 타입 컬럼을 JPA에 위탁하여 관리할 때는,
updatable 등 실수를 방지할 수 있는 속성을 추가해 두는 것이 어떨까 생각합니다.
이에 대해 다른 분들과 생각하시는 바를 나누면 좋을 것 같습니다!
(간단하게 결정되면 좋겠지만, 혹시나 필요하다면 논의를 생성하려고 합니다.)
There was a problem hiding this comment.
이와 같이 설정을 하는 걸까요?
@CreatedDate
@Column(nullable = false, updatable = false)
private Instant createdAt;
There was a problem hiding this comment.
@sweetykr7 님, 좋은 예시라고 생각합니다.
Nullable도 애플리케이션 사이드에서 관리하는 것이 가능하니, nullable, updatable 등을 설정하는 이야기입니다.
이번 병합 전에 반영하는 것은 아니고, 병합 후 작업할 내용으로 생각하고 있습니다.
src/test/kotlin/me/nettee/adapter/driven/persistence/BoardCommandAdapterTest.kt
Show resolved
Hide resolved
src/test/kotlin/me/nettee/adapter/driven/persistence/BoardCommandAdapterTest.kt
Show resolved
Hide resolved
There was a problem hiding this comment.
👍 코틀린에 적응하는 도중에도 사람들과 모여 나은 코드를 고민한 결과물입니다.
여러 사람이 발의하고 또 다른 인원 조합이 참여하며, '코드로' 고민하면서 🧐
공통 설계를 어떻게 선택 가능한 구조로 관리할지 아이디어를 한 단계 도출해 냈다고 생각합니다.
논의한 주제
- JPA 테스트에서 우리가 대부분의 케이스에 사용할 공통 설정을 한 곳에서 관리하고 싶습니다.
- 우리가 원하지 않는 테스트에는 관련 설정이 침투하지 않으면 좋겠습니다.
페어 외의 참여자
이 페어의 멤버분들 외에도:
- 논의 발의에 추가로 참여해 주신 것은 @silberbullet 님과 저였습니다.
- 최종안을 코드로 고민할 때 추가로 참여해 주신 것은 @shin-jingyu 님과 저였습니다.
참여해 주신 분들 모두 감사드립니다!
| } | ||
| } | ||
| } | ||
| }) No newline at end of file |
There was a problem hiding this comment.
테스트 코드가 전반적으로 깔끔하게 작성되셔서 저도 참고해서 작성하였습니다.
많은 고민을 하신것 같아서 도움을 많이 받았습니다.
감사합니다~!
This comment was marked as resolved.
This comment was marked as resolved.
Sorry, something went wrong.
There was a problem hiding this comment.
🚀 훌륭한 탐구 결과가 돋보인 작업, 진심으로 감사드립니다!
특히 @merge-simpson, @taewookimm, @github-insu 세 분 덕분에 저 Config 부분과 Kotest에 관련해서 고민이 많았던 부분에 배워 갑니다 😄
더 멀리 나아가, 추후 아래와 같은 설정 덕분에 테스트 성능에 긍정적인 영향이 미칠거라 생각합니다! 고생하셨습니다!
package me.nettee.core.jpa
import io.kotest.core.spec.style.FreeSpec
import io.kotest.extensions.spring.SpringExtension
import me.nettee.core.jpa.config.JpaConfig
import org.springframework.boot.test.autoconfigure.jdbc.AutoConfigureTestDatabase
import org.springframework.context.annotation.ComponentScan
@AutoConfigureTestDatabase(replace = AutoConfigureTestDatabase.Replace.NONE)
@ComponentScan(basePackageClasses = [JpaConfig::class])
abstract class JpaTransactionalFreeSpec(body: FreeSpec.() -> Unit) : FreeSpec(body) /* super(body) */ {
override fun extensions() = listOf(SpringExtension)
}| 🚀 | 🏅 Approve! |
| @Override | ||
| public Board create(Board board) { | ||
| var boardEntity = boardEntityMapper.toEntity(board); | ||
|
|
||
| return boardEntityMapper.toDomain(boardJpaRepository.save(boardEntity)); | ||
| } | ||
|
|
||
| @Override | ||
| public Board update(Board board) { | ||
| var existBoard = boardJpaRepository.findById(board.getId()) | ||
| .orElseThrow(() -> new IllegalArgumentException("board not found")); | ||
|
|
||
| existBoard.prepareUpdate() | ||
| .title(board.getTitle()) | ||
| .content(board.getContent()) | ||
| .status(board.getStatus()) | ||
| .update(); | ||
|
|
||
| return boardEntityMapper.toDomain(boardJpaRepository.save(existBoard)); | ||
| } | ||
|
|
||
| @Override | ||
| @Transactional | ||
| public void delete(Long id) { | ||
| if(!boardJpaRepository.existsById(id)) throw new IllegalArgumentException("board not found"); | ||
|
|
||
| boardJpaRepository.deleteById(id); | ||
| } |
There was a problem hiding this comment.
해당 부분의 코드가 읽는 사람에게 전달이 잘되고 깔끔하다는 생각이 들었습니다!
작성하신 코드에서 메서드 이름인 create, update, delete가 각 기능을 명확하게 설명하고 있어 코드의 의도를 쉽게 파악할 수 있었습니다.
- 또한, 전체적인 코드가 간결하고 명확하게 작성되어 있어 가독성이 뛰어납니다.
- 불필요한 복잡함이 없고, 각 메서드의 흐름이 자연스럽게 이어져 있어 이해하기 쉬웠습니다.
merge-simpson
added
review: provided
Pull Request
Issues
Resolves #16
Description
BoardCommandAdapter 구현
Create 단위 테스트 진행
관련 이슈 번호: Driven 어댑터 Command 및 단위 테스트 #16
How Has This Been Tested?
Additional Notes
라이브 코드 리뷰로 리뷰 받은 항목들