refactor: 동적 쿼리 생성 툴을 도입하여 중복 쿼리 개선 #684
Conversation
backend/.gitignore
Outdated
| @@ -47,3 +47,6 @@ openapi3.yaml | |||
|
|
|||
| ### FCM ### | |||
| /src/main/resources/fcm | |||
|
|
|||
| ### QueryDSL ### | |||
| /src/main/generated | |||
| @@ -36,10 +36,14 @@ dependencies { | |||
| implementation 'io.jsonwebtoken:jjwt:0.9.1' | |||
| implementation 'javax.xml.bind:jaxb-api:2.3.1' | |||
| implementation 'com.google.firebase:firebase-admin:9.3.0' | |||
| implementation "com.querydsl:querydsl-jpa:5.0.0:jakarta" | |||
There was a problem hiding this comment.
queryDSL을 사용하면서도 JPA 기능을 사용하기 위한 의존성 추가인거죠?
There was a problem hiding this comment.
Querydsl은 아래와 같이 여러 모듈과의 연결을 제공하는데요. 그중에서도 저희가 사용하는 JPA와 연동하기 위한 의존성 추가였습니다!
| @PersistenceContext | ||
| EntityManager entityManager; |
There was a problem hiding this comment.
PersistenceContext 어노테이션을 사용해야만 entityManager를 불러올 수 있는 걸로 알고 있으면 되나요?
EntityManager는 Bean들이 생성되기 전에 만들어져있겠네요?
There was a problem hiding this comment.
@Autowired를 통해서도 주입받을 수 있어요!
그럼에도 @PersistenceContext를 사용한 이유는 아래와 같은데요.
기본적으로 EntityManager는 thread-safe하지 않습니다. 따라서 @Autowired로 싱글톤 빈을 주입받게 되면 멀티스레드 환경에서 동시성 문제가 발생할 수 있다고 해요. 하지만 저희는 하나의 트랜잭션당 thread-safe한 각각의 entityManager를 필요로 해요. @PersistenceContext는 entityManager를 프록시로 감싸 반환하기 때문에 동시성 문제 없이 빈을 주입받을 수 있다고 합니다.
하지만 Spring의 특정 버전 이후 @Autowired로 주입받아도 프록시로 감싸 반환하게 했다고 하는데요. 전 그래도 관례적으로 더 익숙한 @PersistenceContext를 사용하였습니다. 저희의 DatabaseCleaner 클래스에서도 위와 같이 사용하고 있었구요!
참고
| @@ -45,7 +42,7 @@ private List<OfferingEntity> fetchOfferings(Long lastId, double lastDiscountRate | |||
|
|
|||
| private Comparator<OfferingEntity> sortCondition() { | |||
| return Comparator | |||
| .comparing(OfferingEntity::getDiscountRate) | |||
| .comparing(OfferingEntity::getDiscountRate, Comparator.reverseOrder()) | |||
There was a problem hiding this comment.
이코드는 기존에 있던 할인율이 내림 차순으로 정럴되지 않은 버그를 잡기 위한 코드라고 보면 될까요?
기존 테스트 코드를 돌렸을 때 실패한게 아닌데 reverseOrder()를 붙인 이유가 궁금합니다
There was a problem hiding this comment.
이 버그는 기존 저희 테스트 코드가 잡지 못하는 버그였습니다. 필터링과 검색 테스트 커버리지가 낮더라구요. 이번에 제가 테스트 케이스를 더 구체화하면서 발견하게 되었어요!
reverseOrder()를 붙인 이유가 궁금합니다
위 메서드는 제목을 기준으로 검색하는 쿼리의 결과와 만남장소를 기준으로 검색하는 쿼리의 결과를 합치는 과정에서 사용되는 정렬 조건입니다. 각각의 쿼리에서는 정상적으로 할인율 기준 내림차순 + id 기준 내림차순으로 정렬하고 있는데, 그 쿼리의 결과들을 병합하는 위 코드에서는 할인율 기준 오름차순 + id 기준 내림차순으로 정렬하고 있더라고요. 따라서 위와 같이 버그를 수정해주었습니다.
| assertEquals(10, response.offerings().size()); | ||
| assertEquals(50.0, response.offerings().get(0).discountRate()); | ||
| assertEquals(40.0, response.offerings().get(1).discountRate()); | ||
| assertEquals(33.3, response.offerings().get(2).discountRate()); |
There was a problem hiding this comment.
위에서 물어본 테스트 코드입니다. 이 테스트로 기존 내림차순 정렬이 된 것을 확인한걸로 알고 있습니다!
There was a problem hiding this comment.
위에서 언급했듯이 '제목 기준 검색 쿼리 결과'와 '만남장소 기준 검색 쿼리 결과'를 합칠 때 발생하는 버그였는데요, 포케가 언급주신 위 테스트 케이스는 '검색을 하지 않는 경우'이기 때문에 버그가 존재하지 않았습니다!
|
|
||
| @Override | ||
| public List<OfferingEntity> findRecentOfferings(Long lastId, String keyword, Pageable pageable) { | ||
| return queryFactory.selectFrom(offeringEntity) |
There was a problem hiding this comment.
궁금한점이 있어서 여쭈어봅니다~~ offeringEntity는 어디서 가져오는건가요? bean주입도 아니고 생성자가 있는것도 아니여서 조금 어리둥절했습니다.
Intellij로 추적해보니까 queryDSL이 QOfferingEntity를 OfferingEntity와 같은 패키지 안에서 생성 하는것 같은데, 어떤 원리가 있는건가요?
만약 OfferingMemberEntity도 QueryDSL 도입하면 그냥 offeringMemberEntity도 DI 없이 그냥 가져올 수 있는건가요?
There was a problem hiding this comment.
QOfferingEntity가 public static final로 선언되어 있네요! 메타모델이라고 하는데, 타입 안전성과 가독성을 확보하기 위해 사용한다고 합니다.
There was a problem hiding this comment.
offeringEntity는 어디서 가져오는건가요? bean주입도 아니고 생성자가 있는것도 아니여서 조금 어리둥절했습니다.
위 offeringEntity는 아래와 같이 static import 되어있습니다!
import static com.zzang.chongdae.offering.repository.entity.QOfferingEntity.offeringEntity;Intellij로 추적해보니까 queryDSL이 QOfferingEntity를 OfferingEntity와 같은 패키지 안에서 생성 하는것 같은데, 어떤 원리가 있는건가요?
QOfferingEntity는 QClass라고 불리는 클래스인데요, QClass는 Querydsl이 빌드하는 과정에서 엔티티를 대상으로 생성되는 클래스입니다. QClass가 존재하기 때문에 Querydsl이 타입 안전이라는 장점을 가질 수 있어요!
만약 OfferingMemberEntity도 QueryDSL 도입하면 그냥 offeringMemberEntity도 DI 없이 그냥 가져올 수 있는건가요?
아래 이미지와 같이 @Entity가 붙은 모든 클래스에 대해 QClass가 생성됩니다. 따라서 offeringMemberEntity도 같은 방식으로 사용할 수 있어요. 저도 Querydsl 적용할 때 이 점이 매우 흥미로웠습니다 ㅎㅎ
There was a problem hiding this comment.
오호라 자동으로 build할때 class를 생성해주는군요! 감사합니다
| @Override | ||
| public List<OfferingEntity> findRecentOfferings(Long lastId, String keyword, Pageable pageable) { | ||
| return queryFactory.selectFrom(offeringEntity) | ||
| .where(offeringEntity.id.lt(lastId), |
There was a problem hiding this comment.
offeringEntity의 id값은 private로 닫아놨을텐데..? 라고 생각한 라인이었어요!
위의 메서드 주체는 QOfferingEntity인 것 같은데, qOfferingEntity같은 네이밍으로 구별해야 덜 혼란스러울 것 같은데 어떻게 생각하시는지 궁금합니다!
There was a problem hiding this comment.
일반적으로는 메타모델 클래스를 나타낼 땐 Q를 접두사로 붙이지만, 변수명에는 q를 붙이지 않는다고 하네요. OfferingEntity를 여러 개 사용해야 한다면 q를 붙이는 것도 좋지만 현재 상황에는 붙이지 않아도 괜찮다고 생각합니다.
There was a problem hiding this comment.
offeringEntity의 id값은 private로 닫아놨을텐데..? 라고 생각한 라인이었어요!
QClass는 아래 이미지와 같은 형태로 생성되어서 필드를 public하게 접근할 수 있어요!
위의 메서드 주체는 QOfferingEntity인 것 같은데, qOfferingEntity같은 네이밍으로 구별해야 덜 혼란스러울 것 같은데 어떻게 생각하시는지 궁금합니다!
위의 offeringEntity의 경우는 위에서 언급드렸듯이, Querydsl이 빌드하는 과정에서 생긴 QClass 내부에 static import 되어있는 필드입니다! 선언되어있는 모습은 아래에서 확인할 수 있어요. 따라서 제가 지은 변수명이 아니었습니다!
| .where(offeringEntity.id.lt(lastId), | ||
| inOfferingStatus(AVAILABLE, IMMINENT), | ||
| likeTitleOrMeetingAddress(keyword)) |
There was a problem hiding this comment.
where에 있는 파라미터들은 전부 and 조건이라고 이해했는데 맞을까요??
There was a problem hiding this comment.
where 메서드 내부에서 ,로 구분된 조건절들은 모두 and로 이어진 것이 맞습니다!
| } | ||
| } | ||
|
|
||
| @DisplayName("최신순으로 공모 목록을 10개씩 조회할 수 있다") | ||
| @DisplayName("최신순 공모 목록 조회: 검색어 X, 마지막페이지 X, 페이지사이즈 10") |
There was a problem hiding this comment.
오호 displayname이 완전 템플릿화되었네요 ㅎㅎ 직관적이어서 읽기가 쉬었어요!
There was a problem hiding this comment.
으아 고민하다가 적용해본 방식이었는데 좋게 보인다니 다행이에요! 감사해요 :)
| @@ -370,7 +494,7 @@ void should_updateOffering_when_givenOfferingIdAndOfferingUpdateRequest() { | |||
| "수정할 모집 장소 주소", | |||
| "수정할 모집 상세 주소", | |||
| "수정된동", | |||
| LocalDateTime.parse("2024-12-31T00:00:00"), | |||
| LocalDateTime.parse("2030-01-11T00:00:00"), | |||
|
P.S) 제 로컬 환경에서 테스트와 실행 둘 다 잘되었습니다~ |
| public JPAQueryFactory jpaQueryFactory() { | ||
| return new JPAQueryFactory(entityManager); | ||
| } |
There was a problem hiding this comment.
이렇게 설정하면 QueryDsl로 검색한 결과들이 영속성 컨텍스트에서 관리되는 거죠?
There was a problem hiding this comment.
JPAQueryFactory는 Querydsl에서 사용될 '쿼리'들을 관리하는 클래스로 알고 있어요! Querydsl로 쿼리를 구현할 때 주입받아 사용하기 위해 config에서 빈으로 등록해준 것이구요! JPAQueryFactory 안을 들여다보면 select, selectFrom 등의 쿼리들이 존재합니다. 그들을 통해서 아래와 같이 '메서드 기반으로' 쿼리를 만들 수 있구요!
(P.S. 문자열 기반이라 컴파일 타임에 에러를 잡지 못했던 JPQL과 달리, 메서드 기반이기 때문에 컴파일 타임에 에러를 잡을 수 있다는 사실.. 갓Querydsl)
QueryDsl로 검색한 결과들이 영속성 컨텍스트에서 관리되는 거죠?
이 질문은 제가 잘 이해하지 못한 것 같아요...! 다시 설명해주시면 감사하겠습니다 🥲
| @RequiredArgsConstructor | ||
| public class OfferingCustomRepositoryImpl implements OfferingCustomRepository { |
There was a problem hiding this comment.
설마 이부분 Impl 붙여야만 커스텀으로 쓸 수 있어서 그런건가요?
https://docs.spring.io/spring-data/jpa/reference/repositories/custom-implementations.html
|
|
||
| @Override | ||
| public List<OfferingEntity> findRecentOfferings(Long lastId, String keyword, Pageable pageable) { | ||
| return queryFactory.selectFrom(offeringEntity) |
There was a problem hiding this comment.
QOfferingEntity가 public static final로 선언되어 있네요! 메타모델이라고 하는데, 타입 안전성과 가독성을 확보하기 위해 사용한다고 합니다.
| @Override | ||
| public List<OfferingEntity> findRecentOfferings(Long lastId, String keyword, Pageable pageable) { | ||
| return queryFactory.selectFrom(offeringEntity) | ||
| .where(offeringEntity.id.lt(lastId), |
There was a problem hiding this comment.
일반적으로는 메타모델 클래스를 나타낼 땐 Q를 접두사로 붙이지만, 변수명에는 q를 붙이지 않는다고 하네요. OfferingEntity를 여러 개 사용해야 한다면 q를 붙이는 것도 좋지만 현재 상황에는 붙이지 않아도 괜찮다고 생각합니다.
📌 관련 이슈
close #682
✨ 작업 내용
동적 쿼리 생성 라이브러리 JOOQ와 고민하다가 QueryDSL을 선택하였습니다.
저희의 메인 레포지토리인 OfferingRepository에 한해 적용하였고, 다른 레포지토리에는 필요하지 않다고 판단했습니다.
어떤 부분을 중점으로 개선했는지 확인하기 위해서는 블로그 글을 참고해주시면 편할 것 같아요!
추가로, Gradle와 IntelliJ 설정에 따라서 QClass 빌드 방식이 달라, 각자의 로컬 환경에서 동작하지 않을 수 있다고 합니다. 그러니 코드리뷰할 때 로컬에서 한번 테스트를 돌려봐주세요! 문제가 발생하면 코드 리뷰란에 남겨주시고 제가 관련하여 해결하도록 하겠습니다! 지금 당장에도 그런 문제가 발생하지 않도록 스크립트는 찾을 수 있지만, 문제가 발생한 시점에 고치고 싶어요. 현재 저희의 상황에서 문제가 되지 않는다면 필요하지 않은 대처이니까요!
📚 기타