[Feature] - 나라 기반 검색 기능 구현

[slimsha2dy]

✅ 작업 내용

• 여행기 작성 시 여행지 국가 갯수 저장
• 국가 코드 기반 검색 기능 구현

🙈 참고 사항

여러 이름으로 불리는 나라 북한, 한국, 터키, 스페인 정도 추가했는데 더 생각나시는 거 있으시면 말씀해주세요구르트아줌마요구르트주세요

[github-actions]

Test Results

 30 files   30 suites   8s ⏱️
271 tests 271 ✅ 0 💤 0 ❌
283 runs  283 ✅ 0 💤 0 ❌

Results for commit 7e271a0.

[eunjungL]

수고하셨습니다 낙낙알파카쓰

TravelogueQuertRepository 쪽에 제안 하나 남겨뒀는데 확인 부탁드려요!
RC를 날려야되는 제안 같긴하지만 빠른 머지가 필요할 상황을 대비해 approve 해두겠습니다

[eunjungL]

와하우 이건 진짜 정성이네요 수고하셨습니다 👍

[eunjungL]

WoW 미역국

[eunjungL]

findByKeywordAndSearchType이랑 한 번에 처리가 가능하면 참 좋을 것 같은데 합쳐볼 수 없을까요?

jpaQueryFactory.selectFrom(travelogue)와 offset, limit 이쪽은 공통 로직이니까 jpaQueryFactory.selectFrom(travelogue)에 SearchCondition에 따라 쿼리를 동적으로 추가하게 바꿔볼 수 있지 않을까 싶은데 어떻게 생각하시나요?
CountryCode는 Enum인만큼 Reopository에서 직접 꺼내써도 괜찮을 것 같으니 인자는 충분히 합칠 수 있을 것 같아요.

[eunjungL]

@Override
public Page<Travelogue> findByKeywordAndSearchType(SearchCondition condition, Pageable pageable) {
    String keyword = condition.getKeyword();

    JPAQuery<Travelogue> baseQuery = jpaQueryFactory.selectFrom(travelogue);
    if (condition.getSearchType().equals(SearchType.COUNTRY)) {
        baseQuery.join(travelogueCountry)
                .on(travelogue.id.eq(travelogueCountry.travelogue.id))
                .where(travelogueCountry.countryCode.eq(CountryCode.findByName(condition.getKeyword())))
                .orderBy(travelogueCountry.count.desc());
    } else {
        baseQuery.where(Expressions.stringTemplate(TEMPLATE, getTargetField(condition.getSearchType()))
                .containsIgnoreCase(keyword.replace(BLANK, EMPTY)))
                .orderBy(travelogue.id.desc());
    }
    
    List<Travelogue> results = baseQuery.offset(pageable.getOffset())
            .limit(pageable.getPageSize())
            .fetch();
    
    return new PageImpl<>(results, pageable, results.size());
}

그냥 의사코드 형식으로 작성해본거라 될지 안될지는 모르겠지만 이런 식으로 합쳐볼 수 있지 않을까 싶습니다!

[hangillee]

클로버 의견에 동의합니다. 분기 처리를 통해서 동적으로 처리할 수 있을 것 같습니다.

[slimsha2dy]

오호라리 생각하지 못한 방법인데 이렇게 하면 확실히 중복된 부분을 같이 사용하고 동적으로 바뀐 쿼리를 사용할 수 있겠네요.

그런데 걱정되는 부분도 있긴 한데요.
특히 가독성 부분에서 조금 아쉬워지는 것 같아요. 이번에 처음 querydsl을 써봤지만 함수형 체이닝을 따라 쭉 읽으면 하나의 쿼리처럼 읽히게 되어서 잘 사용할 수 있었던 것이 장점이라 생각했습니다. 그런데 이렇게 중간에 분기가 들어가게 될 경우 하나의 완성된 쿼리처럼 읽기에는 쬐애끔 불편해질 수도 있을 것 같아요. 물론 공통 로직은 select와 페이징 처리 뿐이라 큰 상관은 없어 보이긴 합니다! 그래도 한 메서드의 길이가 너무 길기도 한 만큼 가독성에서는 살짝 단점이 생길 것 같습니다.

그리고 분기 처리로 인해 바뀌는 부분이 쿼리의 핵심 로직이면서 공통 로직보다 훨씬 큰 비중을 차지하는 만큼 두 쿼리는 아예 다른 쿼리라고 생각되는데요. 또한 다른 테이블에 대한 JOIN 연산이 필요한 만큼 성격이 전혀 다른 쿼리라고 생각합니다. 보통 레포지터리의 메서드는 한 개의 쿼리 단위로 사용하는데 한 메서드 안에서 전혀 성격이 다른 두 쿼리를 분기 처리로 함께 사용하는 것은 자연스럽지 않은 것 같기도 해요.
두분은 이에 대해서는 어떻게 생각하시나요??

[eunjungL]

특히 가독성 부분에서 조금 아쉬워지는 것 같아요. 이번에 처음 querydsl을 써봤지만 함수형 체이닝을 따라 쭉 읽으면 하나의 쿼리처럼 읽히게 되어서 잘 사용할 수 있었던 것이 장점이라 생각했습니다. 그런데 이렇게 중간에 분기가 들어가게 될 경우 하나의 완성된 쿼리처럼 읽기에는 쬐애끔 불편해질 수도 있을 것 같아요.

이 부분은 메서드 분리를 통해 충분히 해결될 수 있다고 봅니다. 현재 태그 필터링 쪽도 동적 코드를 완성하기 위해 addTagFilter(query, filter);와 같은 메서드 분리가 들어가있는데 이런 방식 활용해보면 충분히 가독성도 챙길 수 있을 것 같네요.

분기 처리로 인해 바뀌는 부분이 쿼리의 핵심 로직이면서 공통 로직보다 훨씬 큰 비중을 차지하는 만큼 두 쿼리는 아예 다른 쿼리라고 생각되는데요. 또한 다른 테이블에 대한 JOIN 연산이 필요한 만큼 성격이 전혀 다른 쿼리라고 생각합니다.

저는 QueryDSL을 동적 쿼리를 위해 적용한 만큼 조금 다른 관점을 적용할 수 있다고 보는데요, 검색이라는 기능을 위해 작성되는 동적 쿼리라고 생각하면 크게 어색하지 않다고 생각합니다. 한 개의 메서드에서 다양한 형태의 쿼리를 작성할 수 있다는게 QueryDSL의 장점이자 도입 이유라고 생각해서요.

또한 검색/필터링 API가 하나로 합쳐지게 될 것을 생각하면 지금처럼 서비스 단에서 분기 처리가 많은게 오히려 가독성을 해치게 될 수도 있지 않나 싶습니다. 서비스 단에서는 검색 쿼리를 호출한다.라는 비즈니스 로직에 집중하고 쿼리를 분기 처리를 통해 동적으로 생성하는건 레포지토리에 위임하는게 좋다는 생각이에요.

여전히 한 개의 메서드에서 처리 됐으면 좋겠다는 의견이 강한데, 어디까지나 저의 의견이니 낙낙과 알파카가 협의 후에 결정하시면 될 것 같습니다. 다만 이 부분이 확정되어야 제가 검색/필터링 API를 통합할 수 있어서 내일 낮까지 결정 부탁드려용! 🙇

[hangillee]

고생하셨습니다, 알파카, 낙낙!
로직만 조금 고려해보면 좋을 것 같아서 Approve 하겠습니다!

[hangillee]

클로버 의견에 동의합니다. 분기 처리를 통해서 동적으로 처리할 수 있을 것 같습니다.

[slimsha2dy]

일단 머지하고 또 다른 의견이 나올 경우 추가로 이슈 파서 반영하도록 하겠습니다 여러분~!