Driven(query) 구현

[wowddok99]

Pull Request

Issues

Resolves #15

Description

• 원래 작업이 일차적으로 완료된 후 PR을 올리는 것이 일반적인 절차이지만, 1월 15일에 진행된 정기 회의에서 논의된 내용에 따라 작업 도중에 먼저 PR을 올리기로 결정하였습니다. PR에 포함된 변경 사항을 확인하시고, 의견 주시면 감사하겠습니다.

• 하기의 목록을 작업 하였습니다.

  • BoardQueryAdapter 구현
    • Board 상세 조회 구현
    • Board 목록 조회 구현
    • 파라미터로 제공받은 status 목록에 대한 Board 목록 조회 구현

• 관련 이슈 번호

  • [DISCUSSION] Domain BoardStatus와 Entity BoardStatus 관련 질문입니다. #22
  • [DISCUSSION] port(BoardQueryPort)에서 사용되고 있는 Pageable에 대한 질문입니다. #23

How Has This Been Tested?

• 테스트 코드는 추후 추가 예정입니다.

Additional Notes

QueryDsl 사용을 위한 의존성 추가

// Querydsl
implementation("com.querydsl:querydsl-jpa:${dependencyManagement.importedProperties["querydsl.version"]}:jakarta")
annotationProcessor("com.querydsl:querydsl-apt:${dependencyManagement.importedProperties["querydsl.version"]}:jakarta")
annotationProcessor("jakarta.persistence:jakarta.persistence-api")
annotationProcessor("jakarta.annotation:jakarta.annotation-api")

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

• 단건 조회 시 메서드 명(의도)과 구현 코드 일치
  • 메서드 이름에 명시가 없다면 REMOVED 상태를 필터하지 않습니다.

[taewookimm]

25/01/15 라이브 코드 리뷰 결과

@sweetykr7 님, @wowddok99 님 유익하고, 깔끔한 작업 감사드립니다! :)

QueryDSL을 사용함으로써, 쿼리의 각 절이 명확하게 구분되어 있어 쿼리를 눈으로 읽기 쉬워 가독성이 좋아진 것 같습니다!
또한, QClass를 사용함으로써, 유지 보수 측면에도 좋다고 생각했습니다 👍

다음 내용은 정기회의에서 진행한 라이브 코드리뷰 리포트 입니다.

리포트 내용을 참고해서 작업해 주시면 좋을 것 같습니다.

---

💊 단건 조회 시 메서드 명(의도)과 구현 코드 일치

• 메서드 명을 보고 구현 내용과 의도가 드러나는 것이 좋다고 생각합니다.
• 단건 조회 시 메서드를 다음과 같이 구현하는 것은 어떤가 제안 드립니다.
• 메서드 명을 그대로 사용하고 REMOVED 필터를 사용하지 않습니다.
• 메서드 명에 REMOVED 상태를 제외한다고 명시하고 REMOVED 필터를 사용합니다.

backend-sample-hexagonal-simple-crud/src/main/java/me/nettee/board/adapter/driven/persistence/BoardQueryAdapter.java

Lines 41 to 73 in f695a0e

	@Override
	public Page<Board> findAll(Pageable pageable) {
	// 기본 쿼리 생성
	var query = getQuerydsl().createQuery()
	.select(boardEntity)
	.from(boardEntity)
	.where(boardEntity.status.ne(BoardStatus.REMOVED));
	// pageable 정렬 조건 적용
	pageable.getSort().forEach(order -> {
	if (order.isAscending()) {
	query.orderBy(boardEntity.createdAt.asc()); // createdAt 필드를 기준으로 오름차순 정렬
	} else {
	query.orderBy(boardEntity.createdAt.desc()); // createdAt 필드를 기준으로 내림차순 정렬
	}
	});
	var result = query
	.offset(pageable.getOffset()) // 현재 페이지의 오프셋 설정
	.limit(pageable.getPageSize()) // 페이지 크기 설정
	.fetch(); // 쿼리 실행
	var totalCount = getQuerydsl().createQuery()
	.select(boardEntity.count())
	.from(boardEntity)
	.where(boardEntity.status.ne(BoardStatus.REMOVED));
	return PageableExecutionUtils.getPage(
	result.stream().map(boardEntityMapper::toDomain).toList(),
	pageable,
	totalCount::fetchOne
	);
	}

[sweetykr7]

test comment

[merge-simpson]

👍 장기간 바쁘신 중에도 합을 맞춰 가며 만들어 주신 유익한 결과물 감사드립니다!

@sweetykr7 님, @wowddok99 님의 작업이 명료하고 깔끔해서, 사소한 실수 찾기 외에는 리뷰를 남겨 드릴 부분이 거의 없었던 것 같습니다!

코드보다는 주로 깃과 깃허브 협업에 관련한 이야기가 많았던 것 같네요!

그리고 다른 계층과 소통하며, 멤버들과 적절한 아이디어를 도출해 주셨던 것 같습니다!
바쁘신 중에도 정말 감사드립니다.

👍 페어 간 적절한 소통과 협업

페어 안에서 서로의 작업 영역까지 파악하기 위해 소통이 끊이지 않았던 것으로 압니다!
서로 바쁠 때도 배려하며 소통하여, 완성해 가시는 게 인상깊었습니다!

깃허브 협업 적응

시도하며 질문하는 용기가 정말 인상 깊었습니다! 👏

코드리뷰를 남기기 위해 코드스페이스 등 여러 UI를 건드려 보시고,
거기서 생긴 의문들도 라이브 회의 때 바로바로 전달해 주셨습니다.

깃허브 협업에 무지할수록 부끄러움을 느끼는 분들이 많다고 생각하는데,
정작 많은 사람들이 깃허브를 사용하는 협업에 능숙하지 않죠! 😅

그래서 누군가 먼저 질문해 주고, 서로 시도해 보면서 간단하게 파악해 보 게 중요한데
그렇게 시도하고 질문해 주시는 모습이 정말 인상 깊었습니다. 👍

덕분에 다들 낯설게만 느끼던, 하지만 막상 해 보면 어렵지 않던
협업 문화 도입에 한 번 더 도움이 됐다고 생각합니다.

테스트 코드

👍 JPA + Kotest 테스트에 기다리고 있던 많은 시행착오를 극복해 온 결과물이 잘 담기고 있습니다! 😎

단위 테스트가 괜찮을지 시도하는 것부터 시작해서 🤔
결국 persistence 어댑터 레이어는 단위 테스트를 제외하고 테스트 DB로 통합 테스트부터 하는 것이 충분히 효용이 있다고 논의되어 큰 방향을 수정했었습니다.

그 후로도, 다른 계층의 테스트에 비해 시행착오가 더 많이 숨어 있는 테스트였죠. 🧐

테스트 코드의 작성이나, 표현에 대한 세세한 방식은 앞으로도 쭉 논의해 보면 항상 나은 방향이 제시될 것 같습니다! 잘 작성해 주셔서 감사드립니다. 🚀

💯 실습에 맞는 테스트 사례만 요약하여 잘 정리해 주셨습니다!

다른 페어들에도 언급한 내용이지만, 만약 불필요 테스트 케이스를 과도하게 추가했다면 실습에 참여하거나 이 결과를 참관하는 사람들이 파악하며 적응하기에 오히려 혼란스러웠을 거라고 생각합니다. 거부감을 증폭시킬지도 모르죠. 하지만 딱 명시적인 테스트만 작성하여 불필요한 작업 낭비를 줄이고 테스트 코드의 '방식'을 개선하는 데에 집중할 수 있었던 것 같습니다!

🛠️ 탐구적인 논의

많은 discussion 이슈를 남겨 주신 것 같습니다! 👍
대부분의 논의 이슈에도 참여도가 좋았던 것 같습니다.

다음 논의들이 인상 깊었습니다.

Port에서 Pageable 사용 가능 여부

예리한 지적이 잘 담긴 논의였습니다. 👍

• [DISCUSSION] port(BoardQueryPort)에서 사용되고 있는 Pageable에 대한 질문입니다. #23

Application 계층에 발생하는 종속성에 대한 논의였습니다.
Pageable은 spring data commons의 것으로 비교적 호환성이 좋습니다.
JPA가 아닌 다른 라이브러리 사용 시, 충돌이 적다고 생각했기 때문에 허용하는 것으로 했습니다.

DeletedAt 컬럼의 유지 여부

• [DISCUSSION] DeletedAt 컬럼 유무 (Soft 딜리트 전략) #20

deletedAt 컬럼을 없애고, updatedAt과 status를 사용하면서, 추후 삭제 액션에 대한 테이블을 따로 관리하는 것으로 이야기되었던 논의였죠. 👍

컬럼의 불일치를 가장 먼저 찾아내고 논의를 개설해 주신 사례였습니다!
감사합니다.

🚀	🏅 Approve!

[merge-simpson]

🎨 (수정) 코틀린에서 타입 명시는 앞을 붙이고 뒤를 띄웁니다. 상속 등은 앞뒤를 모두 띄웁니다.

코틀린 타입 명시는 콜론(:)의 앞뒤를 띄우기로 했습니다.

지난 번에 @silberbullet 님의 설명이 '상속'에 대해서 띄우는 것을 이야기하신 것 같은데,
그걸 제가 오해했던 내용인 것 같아 수정합니다.

[merge-simpson]

@sweetykr7 님, 수정된 내용을 참고하시고 오해 없으시기 바랍니다.
작업은 새 페어가 이어 갈 예정입니다.

[merge-simpson]

🎨 코틀린에서는 실행문 구분에 세미콜론을 생략하는 게 좋아 보입니다.

[merge-simpson]

🎨 코틀린에서는 실행문 구분에 세미콜론을 생략하는 게 좋아 보입니다.

[merge-simpson]

❗ 이곳에서는 병합 시 createdAt, updatedAt은 제외됩니다.

엔티티 관리에서 주요 시간 타입 필드(createdAt, updatedAt)는 JPA Auditing으로 자동으로 관리되도록 했습니다.
병합 시 수정될 코드여서 언급해 드립니다.

---

💊 규칙적인 요소의 생성은 하드코딩으로 하기보다 코틀린의 람다식 등을 활용할 수 있어 보입니다.

코틀린에서 반복적인 요소를 담은 리스트를 생성하는 요령이 몇 가지 있을 겁니다.
@shin-jingyu 님과 @silberbullet 님께서 작업하신 web adpater 테스트 코드에 제시되어 있습니다.

각 구성원이 코틀린 신택스에 적응하는 중이기 때문에, 이런 공유가 소중한 것 같습니다.

backend-sample-hexagonal-simple-crud/src/test/kotlin/me/nettee/board/adapter/driving/web/BoardQueryApiTest.kt

Lines 117 to 122 in 7ce0835

	boardList = (1..14).flatMap {
	listOf(
	BoardReadSummaryModel(it.toLong(), "title$it", "content$it", Instant.now(), Instant.now()),
	BoardReadSummaryModel(it.toLong(), "title$it", "content$it", Instant.now(), Instant.now())
	)
	}

[merge-simpson]

💊 리터럴 상수로 명시하는 것보다(3) 생성 전 리스트의 크기로 확인하는 게 좋아 보입니다.

페이지 객체에 대한 테스트를 추가한 것은 아주 바람직하다고 생각합니다! 👍

지금은 위쪽에서 saveAll 함수를 사용하고 있고, 그 결과도 받아 오지 않았기 때문에
생성 전 리스트가 재사용될 수 없어 그 크기를 갖고 올 수 없는 코드일 겁니다.

하지만 재사용을 해야 하는 상황이라면, 변수에 담아서 그 크기를 갖고 오시게 될 것 같습니다.

[merge-simpson]

🎨 단일 공백 줄을 추가하여 섹션을 구분합니다.

import 섹션은 클래스 선언과 연속되지 않도록 구분합니다.
다른 곳이 모두 잘되어 있는 거 봐서, 이 부분은 실수로 빠뜨리신 것 같습니다!

[merge-simpson]

💬 원래 Common 쪽 작업을 이 브랜치에서 하면 안 됩니다.

"브랜치 역할에 맞는 작업을 하기"가 원칙입니다!
그러지 않으면 잦은 충돌의 원인이 되기 때문입니다.

• 이 기능 브랜치의 역할에 맞지 않는 작업이고
• 다른 팀원들에게 미리 공유된 바가 없어서

  공유하더라도 모두가 인지하고 확인해야 합니다. 그게 확실하지 않고 기억하기 어려운 영역이기 때문에, 굳이 번거로운 소통을 추가하기보다는 애초에 브랜치를 나눕니다.

일반적으로 깃을 통한 협업에서 이런 작업류가 컨플릭트의 주된 요인이 됩니다.

참고
이 코멘트를 남긴 브랜치의 이름은 'feature/board-driven-query'입니다.
common 쪽이나, core 쪽 작업을 위한 브랜치가 아니므로
'브랜치 역할에 맞지 않은 작업'이 발생한 것입니다.

[merge-simpson]

💬 원래 Core 쪽 작업을 이 브랜치에서 하면 안 됩니다.

"브랜치 역할에 맞는 작업을 하기"가 원칙입니다!
그러지 않으면 잦은 충돌의 원인이 되기 때문입니다.

• 이 기능 브랜치의 역할에 맞지 않는 작업이고
• 다른 팀원들에게 미리 공유된 바가 없어서

  공유하더라도 모두가 인지하고 확인해야 합니다. 그게 확실하지 않고 기억하기 어려운 영역이기 때문에, 굳이 번거로운 소통을 추가하기보다는 애초에 브랜치를 나눕니다.

일반적으로 깃을 통한 협업에서 이런 작업류가 컨플릭트의 주된 요인이 됩니다.

참고
이 코멘트를 남긴 브랜치의 이름은 'feature/board-driven-query'입니다.
common 쪽이나, core 쪽 작업을 위한 브랜치가 아니므로
'브랜치 역할에 맞지 않은 작업'이 발생한 것입니다.

[merge-simpson]

👍 (지금은 사용되지 않는 것 같지만) 훌륭한 코드 같습니다.

제가 IDE에서 확인 중인 게 아니라서, 이 인터페이스가 사용되고 있는 곳을 모두 확인하진 않았지만,
팀의 논의가 꾸준히 잘 반영된 것 같고,

제가 다른 영역에 사소하게 남긴 것들을 제외하면 프로덕션 코드, 테스트 코드 모두 깔끔하게 잘 정리되어 있다는 인상입니다! 👍

[merge-simpson]

👍 전체적으로 가독성이 매우 좋고, 잘 정리된 코드입니다.

앞으로 추가적인 논의가 생긴다면 반영하게 될 수도 있지만,
더 구체적인 다음 단계 작업은 멀티모듈 프로젝트 때 담길 것 같습니다. 👍

예를 들면, 페이지를 생성할 때 테이블의 크기를 이대로 조회해서 만드는 것 대신,
더 빠른 조회를 지원하는 여러 단계의 아이디어 중 무엇을 할지 논의하며 리서치할 수 있습니다.

[silberbullet]

🚀 전체적으로 고민이 많았던 코드 작성에 수고가 많으셨습니다!

@merge-simpson 님이 언급하신 코드 스타일은 저 또한 찬성이며, 추후 보완이 이루어졌으면 좋겠습니다!

❗다만, 저는 이 2가지는 추후에 꼭 보완이 됐으면 좋겠습니다!

1. 폴더 응집도를 높이기 위한 폴더 구조 개선, Entity Mapper의 패키지 위치를 재구성하여 응집도를 높이고자 합니다. (Update README: Change mappers package location #9)

현재 폴더 구조

  ├─board
  │  ├─adapter              
  │  │  └─driven                 
  │  │      ├─mapper            
  │  │      └─persistence      

추후 개선된 폴더 구조

└── board
    │   └── driven              
    │       └── persistence     
    │           ├── entity
    │           └── mapper      

2. BeforeEach 구문은 테스트케이스 밑으로 보내기

🤔 테스트 코드의 가독성을 생각한다면 초기 설정 부분들은 가급적으로 밑으로 보내는게 어떨까요?

해당 클래스의 태스트 케이스가 먼저 보이면 다른 개발자가 테스트 의도를 한눈에 보기 편하지 않을까 합니다 😄

🚀	🏅 Approve!

[github-insu]

작업하시느라 고생많으셨습니다

[github-insu]

@EnableJpaAuditing 은 프로덕트 코드 또는 테스트 코드에서 한 번만 작성하면 되는 것 같습니다.
프로덕트 코드와 테스트 코드에서 중복 작성하면 에러가 발생합니다.
@EnableJpaAuditing 을 테스트 코드에서 작성하면 프로덕트 코드에서 사용하지 못하므로 프로덕트 코드에서 작성하는 것이 좋을 것 같습니다.

[github-insu]

private method로 작성하는 방식도 생각해볼 수 있을 것 같습니다.
findyByStatusesList 메서드와 중복되는 부분인 것 같습니다. 이 부분을 "정렬 조건 적용"이라는 private method로 작성하고 findAll 메서드와 findByStatusesList 메서드에 사용하는 방식도
생각해볼 수 있을 것 같습니다.

[github-insu]

바쁘신 와중에 페어간 활발하게 소통하셨다고 들었습니다. 정말 감사드립니다!

• pageable에 대한 종속 고찰
• deletedAt 컬럼 유무 고찰
• Querydsl을 이용한 검색 조건 적용 관련 리뷰 항목 적용
• 단위 테스트에서 통합 테스트로 리팩토링 등

페어간 소통을 하며 위에 작성한 작업 항목보다 더 많은 작업을 진행해 주신 점에 대해 다시 한번 감사드립니다!

[shin-jingyu]

🚀 바쁜 일정에도 작업하시느라 고생하셨습니다. 🙌

• Pageable에 대한 고찰
• DeletedAt 컬럼의 유지 여부 논의
• 테스트 코드에 대한 고찰

페어 간의 소통과 협업을 통해 많은 작업을 진행하시느라 수고 많으셨습니다.
감사합니다! 🚀💪

[taewookimm]

바쁘신 중에도 작업하시느라 고생하셨습니다. 👍

전반적으로 사소한 부분까지 고민을 많이 하시고 작업하신게 보입니다.
또한 작업해 주신 코드를 보며 배우는 점도 많았습니다!

다양한 방식과 논의로 팀내에서도 소통해주시고, 많은 작업을 진행하시느라 수고 많으셨습니다.

감사합니다! 👍🙌