카페 검색 기능 개발 #443

hum02 · 2023-09-10T13:43:16Z

#️⃣ 연관된 이슈

검색 기능 추가 #433

📝 작업 내용

querydsl 적용 위한 의존성 추가 및 설정
검색 위한 cafeQueryRepository와 api구현

💬 리뷰 요구사항

테스트 전에 빌드를 해서 qClass를 생성해야 정상동작합니다

full text search 쪽 설명이 부족해서 comment

full text search 적용 커밋 지정 범위

CafeQueryRepository.searchCafesByWord()가 꽤 클린하지 못합니다...

검색 기준에 따른 join문을 작성하는 걸 다른객체의 책임으로 넘길까도 생각했지만
이게 쿼리문이다 보니 더 읽기 어려워지는 듯했어요.
더 고민해보겠지만 좋은 의견 있으면 말해주세요.

Cafe.images의 fetchjoin문제

querydsl이 embeded객체에 대한 fetch join을 지원해주지 않아서 images를 효율적으로 가져오려면 조금 다른 방법이 필요한데

Images를 entity로 바꾸고 cafe에 oneToOne매핑하는 등의 엔티티 변경
CafeQueryRepository.searchCafesByWord()에서 엔티티가 아닌 dto를 반환하게
하고 직접 매핑
-> 2의 방법이 fetch join으로 인한 위험성을 피하고 엔티티의 수정도 없어서 합리적이라고 생각해요. 이에 대해 의견 주시면 생각해서 반영할게요

검색 체크박스가 어떤식으로 검색되어야 할까- and식으로? or식으로?

지금 구현은 아무 검색 기준도 설정하지 않으면 카페명or카페메뉴or카페주소 범위에서 검색을 하고,
검색 기준을 하나씩 더 설정할 때마다 and식으로 덧붙여가요.
ex) 아무 체크 안함 -> 검색어가 카페명이나 카페 메뉴나 카페 주소 중 하나라도 포함되는 카페 반환
카페명,카페메뉴 체크 -> 검색어가 카페명과 카페메뉴에 모두 포함되어야 카페 반환

그런데 혹시 이걸 or조건식으로 검색하는게 나을지가 궁금하네요
ex)아무 체크 안 함 -> 검색 안됨
카페이름,카페메뉴 체크 -> 검색어가 카페이름이나 카페 메뉴 중에 하나라도 포함되는 카페 반환
이런 식으로요

검색 기능에 페이징까지도 해야하는데, 좀 오래 붙들고 있기도 했고 pr이 커질 것 같아서 1차적으로 리뷰 요청해봅니다.

donghae-kim

고생하셨어요 도치~ 역시 querydsl 어렵네요..

진행한 사항에 대해서만 의견 남겨봤습니다

CafeQueryRepository.searchCafesByWord()가 꽤 클린하지 못합니다...
댓글에 궁금한점을 먼저 달았습니다 !

Cafe.images의 fetchjoin문제

querydsl이 embeded객체에 대한 fetch join을 지원해주지 않아서 images를 효율적으로 가져오려면 조금 다른 방법이 필요한데
Images를 entity로 바꾸고 cafe에 oneToOne매핑하는 등의 엔티티 변경
CafeQueryRepository.searchCafesByWord()에서 엔티티가 아닌 dto를 반환하게
하고 직접 매핑
-> 2의 방법이 fetch join으로 인한 위험성을 피하고 엔티티의 수정도 없어서 합리적이라고 생각해요. 이에 대해 의견 주시면 생각해서 반영할게요

결국 image랑 cafe 간 onetomany 관계인데 단순하게 embedded로 되어있다고 해서 아예 안되는것인가요 ? 잘 이해가 좀 안가네요 ㅠㅠ jpql 기반이면 될 것 같아서요 !! 혹시 자료같은게 있을까요? 만약에 안된다고 하면 entity를 변경하는 것 외의 방법을 선택하는게 좋아보입니다!

검색 체크박스가 어떤식으로 검색되어야 할까- and식으로? or식으로?
저는 or 식이 현재 상황에서 좀 더 적절하지 않나 라는 생각이 들어요 ! 다만 모두 체크가 되지않았을때는 다르게 고려를 해야될 것 같고요

donghae-kim · 2023-09-11T05:24:15Z

server/src/main/java/com/project/yozmcafe/controller/CafeController.java

+
+    @GetMapping("/search")
+    public ResponseEntity<List<CafeSearchResponse>> getCafeBySearch(@RequestParam("query") final String searchWord,
+                                                      @RequestParam(value = "isCafeName", required = false, defaultValue = "false") final boolean isCafeName,


개인적인 생각으로는 전체/카페명/메뉴/주소 이런식으로 검색되면 되지 않을까요 ? CafeName && Menu 로 굳이 검색할 것 같지는 않아서요. 그렇게 진행하면 Keyword 타입을 받아서 더 간단하게 진행할 수 있을 것 같아요 !

카페 이름, 메뉴, 주소마다 검색어를 설정하도록 기획 수정했어요!

donghae-kim · 2023-09-11T07:47:28Z

server/src/main/java/com/project/yozmcafe/service/CafeService.java

@@ -21,11 +23,14 @@
 public class CafeService {

    private final CafeRepository cafeRepository;
+    private final CafeQueryRepository cafeQueryRepository;


따로 repository를 분리해야할까요 ? 결국에는 CafeRepository라고 생각해서 이를 QueryRepository를 extends해서 CafeRepository하나로 사용하는게 더 좋아보여서요 !

donghae-kim · 2023-09-11T07:51:16Z

server/src/main/java/com/project/yozmcafe/domain/cafe/CafeQueryRepository.java

+                    .fetch();
+        }
+
+        final JPAQuery<Cafe> baseQuery = queryFactory.selectFrom(cafe)


저는 처음에 이런식으로 구현되는 것을 생각했었는데 위아래 추가적인 코드를 보니 조금 헷갈리네요 .. menu를 join 한 것과 join을 종류별로 나눈 것에 대해 왜 이렇게 진행했는지 도치의 의견이 궁금합니다!

menu만을 조회할 때에는 inner join이 효율적이지만,
menu조건 뿐 아니라 다른 조건도 걸려있다면 inner join을 하면 다른 조건을 위한 데이터가 삭제가 되기에 outer join을 해야해서 분기문이 생겼었어요!

green-kong

어렵습니다 queryDsl..!
갖고있는 초라한 지식 내에서 최대한 리뷰해봤습니다..!
몇가지 궁금한 점 질문 남겼어요!

green-kong · 2023-09-11T12:31:31Z

server/src/main/java/com/project/yozmcafe/domain/cafe/CafeQueryRepository.java

+        if (!isCafeName && !isMenu && !isAddress) {
+            return queryFactory.selectFrom(cafe)
+                    .leftJoin(menu).on(menu.cafe.eq(cafe))
+                    .where(booleanBuilder
+                            .or(containsCafeName(true, searchWord))
+                            .or(containsAddress(true, searchWord))
+                            .or(containsMenu(true, searchWord)))
+                    .fetch();
+        }


이 케이스가 카페이름, 주소, 메뉴 3가지가 체크박스에 체크가 안돼있는 경우인 듯 한데 맞나요?
음 모든 검색조건이 false 인경우는 검색이 안돼야 하는거 아닌가요...?
사용자의 의도와 반대로 동작하는거 같은데.. 이렇게 하신 이유가 있으신가요?

음 저라면, 검색조건이 설정되지 않았다는 ErrorResponse를 반환할거 같은데... 이렇게 작성하신 도치의 의도가 궁금하네요!

보통 서비스를 사용하면서 아무 필터를 걸지 않고 검색을 하면 모든 기준 안에서 검색되기를 기대한다고 판단했어요!
꼭 하나 이상의 검색 기준을 선택한 후에야 검색이 가능하다면 사용 시 너무 귀찮을 것 같아요..

green-kong · 2023-09-11T12:40:41Z

server/src/main/java/com/project/yozmcafe/domain/cafe/CafeQueryRepository.java

+        final JPAQuery<Cafe> baseQuery = queryFactory.selectFrom(cafe)
+                .where(booleanBuilder
+                        .and(containsCafeName(isCafeName, searchWord))
+                        .and(containsAddress(isAddress, searchWord))
+                        .and(containsMenu(isMenu, searchWord)));


and가 사용자가 원한 결과를 줄 수 있을까요?
현재라면 isCafeName==true && isMenu==true 인 경우 소금빵 으로 검색을하면,
카페이름과 메뉴 둘다 소금빵이 포함된 결과가 조회되지 않나요?

isCafeName==true && isMenu==true 인경우 소금빵으로 검색하면,
카페이름에 소금빵이 포함되거나, 메뉴이름에 소금빵이 포함된 모든 카페를 원하지 않을까요..?

연어도 이런 말을 했는데 이렇게 하나의 검색어만 강제된다면 querydsl을 쓰는 의미가 적기도 하고, 검색의 사용성도 떨어지는 게 맞습니다..

검색 기준마다 검색어를 설정할 수 있도록 기능에 대해서 아인과 더 애기해봐야겠어요 ㅠㅠ

green-kong · 2023-09-11T12:45:57Z

server/src/main/java/com/project/yozmcafe/domain/cafe/CafeQueryRepository.java

+        if (isMenu && !isCafeName && !isAddress) {
+            return baseQuery
+                    .innerJoin(menu).on(menu.cafe.eq(cafe))
+                    .fetch();
+        }
+
+        if (isMenu) {
+            return baseQuery
+                    .leftJoin(menu).on(menu.cafe.eq(cafe))
+                    .fetch();
+        }


이 분기의 의미를 모르겠어요....
isMenu가 true인 경우에만 menu를 join 해주면 되는거 아닌가요?
isMenu && !isCafeName && !isAddress 이 분기를 따로 나누신 이유를 잘 모르겠어요..!

메뉴가 포함될 경우 - menu테이블까지 join을 해야하는데
-메뉴만 검색할 경우 : inner join이 효율적
-메뉴말고 다른 기준도 존재할 경우: inner join하면 다른 기준 검색위한 데이터가 제거되기에 outer join
이외의 경우 join할 필요가 없기에 join안 하는 게 효율적

이러한 차이점들이 있었어요!

green-kong · 2023-09-11T12:52:50Z

server/src/main/java/com/project/yozmcafe/domain/cafe/CafeQueryRepository.java

+        if (isMenu) {
+            return baseQuery
+                    .leftJoin(menu).on(menu.cafe.eq(cafe))
+                    .fetch();
+        }


예를들어 1번 카페에 딸기맛 소금빵, 초코맛 소금빵 두가지의 메뉴가 있고,
isMenu=true(나머지는 false) searchWord가 소금빵 인 경우는,
조회 결과가 어떻게 나오나요?
1번 카페가 중복돼서 조회되진 않던가요?

green-kong · 2023-09-11T12:53:14Z

server/src/main/java/com/project/yozmcafe/domain/cafe/CafeQueryRepository.java

+    private BooleanExpression containsCafeName(final boolean isCafeName, final String searchWord) {
+        if (!isCafeName) return null;
+        return cafe.name.containsIgnoreCase(searchWord);
+    }
+
+    private BooleanExpression containsMenu(final boolean isMenu, final String searchWord) {
+        if (!isMenu) return null;
+        return menu.name.containsIgnoreCase(searchWord);
+    }
+
+    private BooleanExpression containsAddress(final boolean isAddress, final String searchWord) {
+        if (!isAddress) return null;
+        return cafe.address.containsIgnoreCase(searchWord);
+    }


선생님.. 어디서 가져온 컨벤션이신가요....

크흠.. 시정하겠습니다

green-kong · 2023-09-11T13:08:27Z

server/src/test/java/com/project/yozmcafe/domain/cafe/CafeQueryRepositoryTest.java

+    @Test
+    @DisplayName("메뉴 이름으로 카페를 조회한다")
+    void findByMenuName() {
+        // given
+        final String searchWord = "도치 음료";
+        final boolean isCafeName = false;
+        final boolean isMenu = true;
+        final boolean isAddress = false;
+        menuRepository.save(Fixture.getMenu(null, cafe1, 1, "도치 음료", "imageUrl1", "description", "3000원", true));
+        menuRepository.save(Fixture.getMenu(null, cafe1, 2, "아무 음료", "imageUrl2", "description", "3500원", true));
+        menuRepository.save(Fixture.getMenu(null, cafe2, 2, "도치 음료", "imageUrl3", "description", "4000원", false));
+        menuRepository.save(Fixture.getMenu(null, cafe3, 1, "도치", "imageUrl4", "description", "2000원", true));
+
+        // when
+        final List<Cafe> cafes = cafeQueryRepository.searchCafesByWord(searchWord, isCafeName, isMenu, isAddress);
+
+        // then
+        assertAll(
+                () -> assertThat(cafes).extracting(Cafe::getName).containsOnly(cafe1.getName(), cafe2.getName()),
+                () -> assertThat(cafes).hasSize(2)
+        );
+    }


searchWord를 음료 로 변경했을 때 테스트가 깨질거라 예상했는데... 잘 돌아가네요..?
QueryRepository에서 groupBy 사용이 없어서 중복되는 데이터가 반환되진 않을까 했는데,
정상적으로 동작하네요... 왜죠? queryDsl에서 기본으로 중복된 데이터를 제거해주는건가요???

아래에서 연어가 말했는데 hibernate6에서부터는 자동으로 distinct해서 조회한다고 해요
요런 메서드를 테스트 했더니 성공하네요

nuyh99

메서드의 동작을 잘 모르겠어서 내일 기획에 대해서 물어볼게요...!

nuyh99 · 2023-09-11T15:36:22Z

server/src/main/java/com/project/yozmcafe/controller/dto/cafe/CafeThumbnailResponse.java

+public record CafeThumbnailResponse(Long cafeId, String imageUrl) {
+
+    public static CafeThumbnailResponse from(LikedCafe likedCafe) {
+        final Cafe cafe = likedCafe.getCafe();


필요한 건 Cafe 클래스인데 이 DTO는 LikedCafe와의 참조도 가지고 있어요!

비록 게터이지만 DTO에서 도메인의 메서드를 호출합니다. 외부에서 Cafe를 주는 게 더 좋을 것 같다고 생각해요.

파라미터에 final이 없어요!

수정했어요!

hum02 · 2023-09-14T09:56:41Z

결국 image랑 cafe 간 onetomany 관계인데 단순하게 embedded로 되어있다고 해서 아예 안되는것인가요 ? 잘 이해가 좀 안가네요 ㅠㅠ jpql 기반이면 될 것 같아서요 !! 혹시 자료같은게 있을까요? 만약에 안된다고 하면 entity를 변경하는 것 외의 방법을 선택하는게 좋아보입니다!`

제가 설명이 부족했네요.. 그리고 embedded타입 안의 필드에 직접 접근해서 fetch join도 해올 수 있습니다.

이렇게 직접 해봐서 안되었었고 https://kkambi.tistory.com/193 이 글에서도 embedded의 지원이 안된다는 내용이 있는데 잘못 읽은 겁니다... 공식문서에도 자세한 설명이 없어 파악이 어려웠네요ㅠ

이렇게 직접 필드에 접근하면

cafe필드에 잘 포함해서 가져오네요

nuyh99

고생하셨습니다!!

도치가 한 부분은 아니지만 관련 부분도 리팩토링하면 어떨까 싶어서 코멘트로만 남겨봤습니다!
반영은 원하는 대로 하시면 좋겠군요!
Approve 하겠습니당

nuyh99 · 2023-09-14T14:58:46Z

server/src/main/java/com/project/yozmcafe/controller/CafeController.java

+                                                      @RequestParam(value = "cafeName", required = false, defaultValue = "") final String cafeName,
+                                                      @RequestParam(value = "menu", required = false, defaultValue = "") final String menu,
+                                                      @RequestParam(value = "address", required = false, defaultValue = "") final String address) {


defaultValue 설정은 없어도 될 것 같아요!!
null로 들어와도 아무 문제가 없으니까...?

그리고 여기는 DTO를 쓰지 않은 이유가 궁금해요!

지금 현재의 구조에서는 검색 조건이 추가되면 컨트롤러, 서비스, 리포지토리 모두 바뀔 것 같아요.

그리고 추가로 구글 자바 컨벤션에서는 한 라인에 120자까지이고 인텔리제이에서도 줄을 그어주니까 개행을 추가하는 건 어떠신가요...?!

DTO로 받는게 더 좋을 것 같아요

nuyh99 · 2023-09-14T15:09:32Z

server/src/main/java/com/project/yozmcafe/domain/cafe/CafeCustomRepositoryImpl.java

+                .leftJoin(menu).on(menu.cafe.eq(cafe))
+                .innerJoin(cafe.images.urls).fetchJoin()
+                .where(


먼저 batch size가 걸려있어서 In 절로 이미지들을 가져올 수 있는데 innerJoin으로 가져오는 이유가 궁금합니다!
물론 쿼리 갯수는 1번으로 줄지만 Cartesian Product가 발생하니 더 빠를 지는 측정해봐야 알 것 같아요.

그리고 Hibernate 6에서는 부모 엔티티에 대해서 항상 중복 제거를 해줍니다.
따라서 distinct나 쿼리 힌트를 추가로 날릴 필요가 없어요. 여기의 DISTINCT 절을 참고해주세요
혹시나 다른 이유 때문에 쓰신 걸까요?

쿼리 갯수를 1번으로 하기 위해서 수정했어요! 그런데 N+1을 해결하면서 batch size설정을 했기에 굳이 이렇게 할 필요는 없을 것 같네요.
distinct잘 먹겠습니다

nuyh99 · 2023-09-14T15:20:57Z

server/src/main/java/com/project/yozmcafe/service/CafeService.java

@@ -66,4 +69,18 @@ public CafeResponse getCafeById(final long cafeId) {

        return CafeResponse.fromUnLoggedInUser(foundCafe);
    }
+
+    public List<CafeSearchResponse> getCafesByKeyWord(final String cafeNameKeyWord, final String menuKeyWord, final String addressKeyWord) {


컨트롤러에서 Search라는 용어를 썼으니 여기도 getCafesBySearch 등이 혼란이 없을 것 같아요!
아니면 keyword로 통일을 하거나요!

nuyh99 · 2023-09-14T15:23:43Z

server/src/main/java/com/project/yozmcafe/service/CafeService.java

+
+    public List<CafeSearchResponse> getCafesByKeyWord(final String cafeNameKeyWord, final String menuKeyWord, final String addressKeyWord) {
+        List<Cafe> cafes;
+        if (StringUtils.isBlank(menuKeyWord)) {


static import가 좋을 것 같습니다!

분기에 따른 재할당보다는 DTO를 파라미터로 받아서 List를 리턴해주는 메서드를 만드는 건 어떨까요?

final List<Cafe> cafes = findAllBy(searchRequest); return cafes.stream() .map(CafeSearchResponse::from) .toList();

해당 메서드를 사용하면 이런 식으로 바뀔 것 같네요!

menu검색이 아닐때에는 join을 하지 않으려고 하니 service나 repository둘 중 하나에는 분기문이 생기는 듯 해요! (searchRequest를 넘겨도 repository에서 결국 분기문이 생김)
그래서 menu검색을 하든 말든 leftjoin하도록 통일해볼까 생각했는데
#443 (comment)
join을 하지 않을 경우의 성능이 유의미하게 좋아서 나눠야겠다 생각했어요
그래서 supplier를 통해 쿼리메서드가 정해지도록 수정해봤어요!

저는 함수형 인터페이스가 필요 없다고 생각해요!

public List<CafeSearchResponse> getCafesBySearch(final CafeSearchRequest searchRequest) { final List<Cafe> cafes = mapSearchMethod(searchRequest); return cafes.stream() .map(CafeSearchResponse::from) .toList(); } private List<Cafe> mapSearchMethod(final CafeSearchRequest cafeSearchRequest) { if (isBlank(cafeSearchRequest.menu())) { return cafeRepository.findAllBy(cafeSearchRequest.cafeName(), cafeSearchRequest.address()); } return cafeRepository.findAllBy(cafeSearchRequest.cafeName(), cafeSearchRequest.menu(), cafeSearchRequest.address()); }

이렇게 쓰면 되지 않나요...?

nuyh99 · 2023-09-14T15:36:50Z

server/src/main/java/com/project/yozmcafe/service/LikedCafeService.java

        final Member member = memberService.findMemberByIdOrElseThrow(memberId);

        final List<LikedCafe> likedCafes = getLikedCafes(pageable, member);

        return likedCafes.stream()
-                .map(LikedCafeThumbnailResponse::from)
+                .map(LikedCafe::getCafe)


애초에 Member에서 List를 받을 수 없을까 고민해보다가 옛날 코드들을 보게 됐는데요!

Member 내부 getLikedCafesSection 메서드에 reverseLikedCafes라고 돼있는데 그냥 likedCafes라고 하는게 나을 것 같네요...!

LikedCafe를 서비스에서 사용하기보다는 Member 내부로 숨기는 것이 나을 것 같아요. Member에서 애초에 List<Cafe>를 리턴하는 것이 좋을 것 같다는 생각입니다.

아래의 getLikedCafes 내부의 첫 번째 줄은 pageable.getOffset() 메서드를 활용하면 깔끔해질 것 같네요!

위의 사항을 다 적용하면,

Suggested change

.map(LikedCafe::getCafe)

public List<CafeThumbnailResponse> findLikedCafeThumbnailsByMemberId(final String memberId, final Pageable pageable) {

final Member member = memberService.findMemberByIdOrElseThrow(memberId);

final List<Cafe> likedCafes = member.getLikedCafes(pageable.getOffset(), pageable.getPageSize());

return likedCafes.stream()

.map(CafeThumbnailResponse::from)

.toList();

}

이러한 형태가 될 것 같고 LikedCafeService#getLikedCafes 메서드는 필요가 없어보여요!

offset을 통해서 더 깔끔해질 수 있겟네요 ... ㅋㅋ... 이것도 모르고 getLikedCafes의 로직 짠게 😂

이거 반영했는데 LikedCafeServiceTest쪽 테스트가 실패하더라고요!
왜인지 보니 원래 transactional을 붙여서 테스트 코드에서 변경감지를 이용했었던 걸 transactional을 뺀 후 수정하지 않아서 같아요.
변경감지를 이용해서 작성했던 테스트 코드가 더 있을 수도...? 당장 실패하지는 않지만 고치는 게 좋겠네요

nuyh99 · 2023-09-14T15:38:06Z

server/src/test/java/com/project/yozmcafe/controller/CafeControllerTest.java

@@ -115,7 +119,7 @@ void updateLikes() {
        assertAll(
                () -> assertThat(likeResponse.getStatusCode()).isEqualTo(200),
                () -> assertThat(cafeResponses).hasSize(4),
-                () -> assertThat(cafeResponse.likeCount()).isEqualTo(0),
+                () -> assertThat(cafeResponse.likeCount()).isZero(),


nuyh99 · 2023-09-14T15:42:54Z

server/src/test/java/com/project/yozmcafe/domain/cafe/CafeCustomRepositoryImplTest.java

+        final String menu = "";
+        final String address = "";
+


이러한 부분은 다 제거하는 것이 좋을 것 같습니다..!
null을 직접 대입하는 것이 테스트를 읽을 때도 더 빠르게 읽힐 것 같아서요.

nuyh99 · 2023-09-14T15:44:25Z

server/src/test/java/com/project/yozmcafe/domain/cafe/CafeCustomRepositoryImplTest.java

+import static org.assertj.core.api.Assertions.assertThat;
+import static org.junit.jupiter.api.Assertions.*;
+
+class CafeCustomRepositoryImplTest extends BaseTest {


메뉴 + 주소 조합의 테스트 하나와 아무 조건이 없을 때의 테스트 하나가 더 추가되면 좋을 것 같아요!

nuyh99 · 2023-09-14T15:45:33Z

server/src/test/java/com/project/yozmcafe/domain/cafe/CafeCustomRepositoryImplTest.java

+import java.util.List;
+
+import static org.assertj.core.api.Assertions.assertThat;
+import static org.junit.jupiter.api.Assertions.*;


도치 혹시 와일드카드 설정 안 꺼두셨나요...??
물론 테스트라서 딱히 상관 없긴 합니다.

nuyh99 · 2023-09-14T15:46:59Z

server/src/test/java/com/project/yozmcafe/fixture/Fixture.java

+    public static Menu getMenu(final Long id, final Cafe cafe, final int priority,
+                               final String name, final String imageUrl, final String description,
+                               final String price, final boolean isRecommended) {
+        return new Menu(id, cafe, priority, name, imageUrl, description, price, isRecommended);
+    }


이건 그냥 Menu의 생성자 같은데 Fixture로 쓰는 의미가 궁금합니다...!

아 설정할 필요없는 인자는 여기서 정해진 값으로 들어가게 수정할게요!

donghae-kim

Repository 쪽 관련 질문만 조금 달아봤습니다! 연어가 댓글을 잘 달아줘서 이것 정도만 말해주면 될 것 같아요 !

donghae-kim · 2023-09-16T03:40:16Z

server/src/main/java/com/project/yozmcafe/controller/dto/cafe/CafeSearchResponse.java

+
+import com.project.yozmcafe.domain.cafe.Cafe;
+
+public record CafeSearchResponse(long id, String name, String address, String image, int likeCount) {


CafeThumbnailResponse 의 id 는 래핑된 Long 이고 여기는 long이네요 ㅎㅎ..

donghae-kim · 2023-09-16T03:41:47Z

server/src/main/java/com/project/yozmcafe/controller/CafeController.java

+                                                      @RequestParam(value = "cafeName", required = false, defaultValue = "") final String cafeName,
+                                                      @RequestParam(value = "menu", required = false, defaultValue = "") final String menu,
+                                                      @RequestParam(value = "address", required = false, defaultValue = "") final String address) {


DTO로 받는게 더 좋을 것 같아요

donghae-kim · 2023-09-16T03:45:09Z

server/src/main/java/com/project/yozmcafe/domain/cafe/CafeRepository.java

@@ -8,7 +8,7 @@

 import java.util.List;

-public interface CafeRepository extends JpaRepository<Cafe, Long> {
+public interface CafeRepository extends JpaRepository<Cafe, Long>, CafeCustomRepository {


저도 이런 구조를 생각했었는데 반영 👍

donghae-kim · 2023-09-16T03:46:18Z

server/src/main/java/com/project/yozmcafe/service/CafeService.java

+        if (StringUtils.isBlank(menuKeyWord)) {
+            cafes = cafeRepository.findAllBy(cafeNameKeyWord, addressKeyWord);
+        }
+        else {


객체지향생활체조원칙 else를 사용하지 않는다.

아 이 부분 join을 안할때는 다른 메서드로 실행하려 하니 어렵네요....
아래에서 폴로가 언급한 대로 메서드 분리해서 수정해보았어요!

donghae-kim · 2023-09-16T03:53:42Z

server/src/main/java/com/project/yozmcafe/service/LikedCafeService.java

        final Member member = memberService.findMemberByIdOrElseThrow(memberId);

        final List<LikedCafe> likedCafes = getLikedCafes(pageable, member);

        return likedCafes.stream()
-                .map(LikedCafeThumbnailResponse::from)
+                .map(LikedCafe::getCafe)


offset을 통해서 더 깔끔해질 수 있겟네요 ... ㅋㅋ... 이것도 모르고 getLikedCafes의 로직 짠게 😂

donghae-kim · 2023-09-16T04:01:38Z

server/src/main/java/com/project/yozmcafe/domain/cafe/CafeCustomRepositoryImpl.java

+        super(Cafe.class);
+    }
+
+    public List<Cafe> findAllBy(final String cafeNameWord, final String menuWord, final String addressWord) {


아직 쿼리 dsl을 학습하지 않아서 잘은 모르겠지만 제가 이번에 근로 학습을 하면서 봤던 것은 지금처럼 cafeNameWord, menuWord, addressWord에 따라 메소드 오버로딩을 할 필요 없이 한 메소드를 통해서 진행할 수 있었는데 지금은 불가한 것일까요 ?

왜 오버로딩했는지가 궁금합니다! 단순하게 leftJoin, innerJoin 때문에 분리한건지도요 !

오버로딩 했던 이유가 menu검색을 하지 않을 경우에는 cafe엔티티만으로 검색이 가능해서에요!
그래서 분기문으로 menu검색이 없을 때는 불필요한 조인을 하지 않도록 하려 했습니다.

그런데 불필요한 join이라면 db측의 옵티마이저가 알아서 최적화해주지 않을까? 하는 생각도 들어서 50000개 정도의 카페에 메뉴2개씩 저장해서 테스트 해봤어요

알아서 최적화 해주지는 않네요.. join시 메뉴 개수만큼 곱해져서 조회되기에 그만큼의 성능 차이가 납니다.
결론은 leftjoin,inner join이냐의 문제보다는 join이 필요없을 때는 안하는게 게 성능 상 좋아서 분리하려해요!

green-kong

죄송합니다. 리뷰가 너무 늦었네요 죄송합니다.
연어랑 오션이 이미 많은 리뷰 남겨 주셔서 크게 남길건 없었습니다.

service쪽 cafe 재할당이랑, 쿼리 관련해서 코멘트 남겨놓았습니다.
감사합니다. 죄송합니다.

green-kong · 2023-09-17T03:11:54Z

server/src/main/java/com/project/yozmcafe/domain/cafe/CafeCustomRepositoryImpl.java

+        return from(cafe)
+                .leftJoin(menu).on(menu.cafe.eq(cafe))
+                .where(
+                        contains(menu.name, menuWord),
+                        contains(cafe.name, cafeNameWord),
+                        contains(cafe.address, addressWord))
+                .fetch();


요거 left join 일 필요가 있나요?? 검색이면 있는 것중에서 찾는 걸텐데.. 라는 생각이 드네요ㅎㅎ

아 맞아요! 이 경우라면 innerJoin이 효율적일 것 같아요

green-kong · 2023-09-17T03:23:25Z

server/src/main/java/com/project/yozmcafe/service/CafeService.java

+    public List<CafeSearchResponse> getCafesByKeyWord(final String cafeNameKeyWord, final String menuKeyWord, final String addressKeyWord) {
+        List<Cafe> cafes;
+        if (StringUtils.isBlank(menuKeyWord)) {
+            cafes = cafeRepository.findAllBy(cafeNameKeyWord, addressKeyWord);
+        }
+        else {
+            cafes = cafeRepository.findAllBy(cafeNameKeyWord, menuKeyWord, addressKeyWord);
+        }
+
+        return cafes.stream()
+                .map(CafeSearchResponse::from)
+                .toList();
+    }


Suggested change

public List<CafeSearchResponse> getCafesByKeyWord(final String cafeNameKeyWord, final String menuKeyWord, final String addressKeyWord) {

List<Cafe> cafes;

if (StringUtils.isBlank(menuKeyWord)) {

cafes = cafeRepository.findAllBy(cafeNameKeyWord, addressKeyWord);

}

else {

cafes = cafeRepository.findAllBy(cafeNameKeyWord, menuKeyWord, addressKeyWord);

}

return cafes.stream()

.map(CafeSearchResponse::from)

.toList();

}

public List<CafeSearchResponse> getCafesByKeyWord(final String cafeNameKeyWord, final String menuKeyWord,

final String addressKeyWord) {

final List<Cafe> cafes = asdf(cafeNameKeyWord, menuKeyWord, addressKeyWord).get();

return cafes.stream()

.map(CafeSearchResponse::from)

.toList();

}

private Supplier<List<Cafe>> asdf(final String cafeNameKeyWord, final String menuKeyWord,

final String addressKeyWord) {

if (menuKeyWord.isBlank()) {

return () -> cafeRepository.findAllBy(cafeNameKeyWord, addressKeyWord);

}

return () -> cafeRepository.findAllBy(cafeNameKeyWord, menuKeyWord, addressKeyWord);

}

저도 재할당은 좀 보기싫어서 좋은 방법 없을까 하다가.
이런건 어떤가요?
펑셔널인터페이스 맛도리~

폴로 천재!

hum02 · 2023-09-19T12:15:10Z

그.. 보니까 저도 모르게 이상한 커밋 push해놨더라고요 이거 되돌리려했는데 잘 안되서 이상한 commit하나 껴있습니다. 죄송합니다ㅠ

donghae-kim · 2023-09-20T12:25:16Z

고생하셨어요 도치 ~

hum02 · 2023-09-25T04:50:58Z

full text search를 적용할 때 functionContributor가 낯설텐데요
참고자료 남길게요

hibernate6부터 사용자 설정 dialect를 위한 설정이 변했더라고요
FunctionContributor로 match_against란 함수를 등록하고, cafeCustomRepositroyImpl쪽에서
이 match_against함수를 사용해서 쿼리를 작성해요.
functionContributor를 사용하게 된 과정에 대해 간략하게 정리했어요.
functionContributor사용한 과정

numberTemplate으로 쿼리를 만들때 gt(0.0)이 붙은 이유는
querydsl에서 where절에 boolean expression이 들어가야하는데 match against의 반환값은 숫자(매치율)이다 보니 boolean으로 만들어주기 위해 사용했습니다.

https://discourse.hibernate.org/t/migration-of-dialect-to-hibernate-6/6956
https://discourse.hibernate.org/t/user-defined-functions-in-dialect/6962/5
https://docs.jboss.org/hibernate/orm/6.1/userguide/html_single/Hibernate_User_Guide.html

nuyh99

고생하셨습니다!!
Hibernate에 의존적인 코드가 추가로 생긴 것 같아요.
필요한 부분인지 궁금하고, 여러 선택지들 중 이것을 선택한 이유가 궁금해요!

nuyh99 · 2023-09-25T04:49:27Z

server/src/main/java/com/project/yozmcafe/domain/cafe/CafeCustomRepositoryImpl.java

+            return null;
+        }
+
+        final String literalSearchWord = MATCH_LITERALLY_OPERATOR + searchWord + MATCH_LITERALLY_OPERATOR;


이 부분이 무슨 뜻인지 잘 모르겠어요ㅠㅠ

searchWord와 literalSearchWord가 둘다 그냥 문자 같은데 무슨 차이인지...? 네이밍에서 잘 드러나지 않는다고 생각해요...!

습 저는 상수화를 통해 가독성을 개선했다 생각했는데 오히려 이해를 어렵게 했군요..
MATCH_LITERALLY_OPERATOR 는 상수화 하지 않고
literalSearchWord 라는 변수명은 formattedSearchWord로 수정하려합니다!

nuyh99 · 2023-09-25T04:52:56Z

server/src/main/java/com/project/yozmcafe/domain/cafe/CustomFunctionContributor.java

+public class CustomFunctionContributor implements FunctionContributor {
+
+    private static final String FUNCTION_NAME = "match_against";
+    private static final String FUNCTION_PATTERN = "match (?1) against (?2 in boolean mode)";
+


RepositoryImpl에서 match_against라는 것을 봤을 때 이게 뭐지?? 싶었는데 패턴 매칭으로 여기서 함수를 정의하고 있었군요.

match against 문을 바로 쓰지 않고 함수화하는 이유가 궁금합니다.
어떤 이점이 있나요??

저도 가능하면 특화구문을 최소한의 의존성으로 사용하고 싶어요!

private BooleanExpression contains(final StringPath target, final String searchWord) { if (isBlank(searchWord)) { return null; } return booleanTemplate("match({0}) against ({1} in boolean mode) > 0", target, searchWord); }

이런 식으로 match against문을 바로 쓰는 것을 생각하신 게 맞나요?
이렇게 사용할 시 hql이 이런 특화 구문을 지원해주지 않아 hibernate에서 parsing할 때 exception이 발생합니다 ㅜ.

그렇다고 native query사용을 위해 entityManager를 직접 사용하면 querydsl을 통한 동적쿼리의 의미가 없어지는 것 같고....
때문에 함수 등록을 하는 방법을 택했습니다! 다른 방법이 있을까요?

query dsl은 말 그대로 DSL로 편하게 동적 쿼리를 작성하기 위함이고 native query를 사용하는 것은 피할 수 없다고 생각해요.
필요하면 JDBC Template을 쓰거나, Entity Manager로 날리거나 하는 것이 당연하다고 생각합니다!

근데 만약 여기서 Entity Manager를 사용하면 좀 더 추상객체(Entity Manager)의 의존만으로 처리할 수 있지 않나 해서 남겼어요!
어떤 장점 때문에 했는지 궁금해서요.

native query가 필요할 때마다 native query가 아닌 Query DSL로 하실 것인가요??

아 Entity Manager로 변경하라는 뜻은 아닙니다!
지금 그대로도 좋아요!! 단순 궁금증입니다.

querydsl의 where에 native쿼리가 들어가야 처음 querydsl을 쓸 때 의도했던 대로
하나의 쿼리 template으로 동적인 쿼리가 가능하다 생각해요!

Entity Manager를 쓰면서 이를 querydsl에 적용하려면 EntityManager의 쿼리 형식을 booleanExpression으로 바꿔야 하는 데 그게 가능한지 모르겠네요...

nuyh99 · 2023-09-25T04:55:16Z

server/src/main/java/com/project/yozmcafe/domain/cafe/CafeCustomRepositoryImpl.java

+public class CafeCustomRepositoryImpl extends QuerydslRepositorySupport implements CafeCustomRepository {
+
+    private static final double MATCH_THRESHOLD = 0.0;
+    private static final String MATCH_LITERALLY_OPERATOR = "\"";


DOUBLE_QUOTES로 하거나 상수화를 안하는 것도 오히려 좋을 것 같은데 어떤가요??
MySQL Match 문법용 연산자인가 생각했는데 단순 문자열인 것 같아서요!

boolean모드에서 " operator를 통해 literal하게 문자열 그대로 포함하는 것을 검색하는 것
에 대해 의미를 드러내고자 했는데 쉽지 않네요....
상수화를 하지 않겠습니다.

그렇다고 functionContributor쪽에서 " 를 끼워 넣는것은 operator가 추가되었다는 인지를 힘들게 할 것 같아 하고싶지 않네요

nuyh99 · 2023-09-25T04:56:09Z

server/src/main/java/com/project/yozmcafe/domain/cafe/CafeCustomRepositoryImpl.java

+        final String literalSearchWord = MATCH_LITERALLY_OPERATOR + searchWord + MATCH_LITERALLY_OPERATOR;
+
+        return numberTemplate(Double.class, "function('match_against', {0}, {1})",
+                target, literalSearchWord).gt(MATCH_THRESHOLD);


gt() 같은 메서드 체이닝은 개행하면 더 좋을 것 같아요...!

nuyh99 · 2023-09-25T04:58:42Z

server/src/main/resources/db/migration/V20230925125500__full_text_idx.sql

+alter table cafe add fulltext index cafe_name_idx (name) with parser ngram;
+
+alter table cafe add fulltext index address_idx (address) with parser ngram;
+
+alter table menu add fulltext index menu_name_idx (name) with parser ngram;


이건 사실 반영 안해도 되는데,
ALTER TABLE menu 이런 식으로 다른 SQL문들처럼 대문자를 쓰면 좋을 것 같아요!

음 반영해주시면 좋을것 같은 내용이라 생각해요!

nuyh99 · 2023-09-25T05:18:41Z

server/src/main/resources/META-INF/services/org.hibernate.boot.model.FunctionContributor

@@ -0,0 +1 @@
+com.project.yozmcafe.domain.cafe.CustomFunctionContributor


함수를 등록하려면 이 부분이 필요한 건가요??
어떤 부분인지 궁금해요

여기서 fuctionContirbutor를 사용하려면 sericeLoader에서 스캔하도록 META-INF경로에 파일을 만들거나
코드를 통해 등록하려면 applyFunctions등을 이용하라 안내하는데요.
첫번째 방법을 위해 serviceLoader가 스캔하는 경로에 파일을 등록했습니다.

두번 째 방법인

이 방식으로 해보니 여전히 직접 설정을 집어넣어야 하는 게 존재해서 이러느니 META-INF에 파일을 만드는 지금 방식이 더 낫다고 생각합니다

green-kong

사실 어제는 도저히 내용이 이해가 되질 않았었습니다.
그런데 오늘 연어의 코멘트 내용과, 도치 블로그 포스팅 내용을 보고 난 이후 다시보니 개괄적인 부분이 이해가 되네요!

새로운 기술을 접목하시느라 너무 수고 많으셨어요! 감사합니다!

hum02 added 5 commits September 8, 2023 17:10

refactor:LikedCafeThumbnail dto명 변경

f0d02b2

chore:QueryDsl 의존성 및 환경설정(#433)

dd80429

feat:카페 동적 쿼리 repository구현(#433)

b4e29c1

feat:카페 검색 API 구현(#433)

f86685e

refactor:가독성 위한 개행

1d74670

hum02 requested review from donghae-kim, nuyh99 and green-kong as code owners September 10, 2023 13:43

hum02 self-assigned this Sep 10, 2023

hum02 added 카테고리:기능🛠️ 만들어줘잉 BE 개발은 백이징 labels Sep 10, 2023

donghae-kim requested changes Sep 11, 2023

View reviewed changes

green-kong requested changes Sep 11, 2023

View reviewed changes

nuyh99 requested changes Sep 11, 2023

View reviewed changes

hum02 added 4 commits September 14, 2023 20:01

feat:검색 기준마다의 검색어로 카페 조회 기능(#433)

d294873

refactor:cafeThumbnailResponse 메서드가 Cafe를 인자로 받도록 수정

9e22d9f

refactor:불필요한 서브쿼리 삭제(#433)

dfb99f9

refactor:querydsl 메서드에 distinct 추가(#433)

5f80dd5

nuyh99 approved these changes Sep 14, 2023

View reviewed changes

refactor:불필요한 join과 distinct삭제

c18154a

donghae-kim requested changes Sep 16, 2023

View reviewed changes

green-kong approved these changes Sep 17, 2023

View reviewed changes

hum02 added 2 commits September 18, 2023 17:24

임시 커밋 push 안됨

00185fa

refactor:검색 기능 가독성 개선(#433)

20c4c88

hum02 added 2 commits September 19, 2023 21:15

refactor:불필요한 파일 삭제

2fab8e0

refactor:menu에 따른 분기처리 가독성 개선

806b0f8

donghae-kim approved these changes Sep 20, 2023

View reviewed changes

hum02 added 2 commits September 25, 2023 12:58

feat:검색 기능에 fulltext search 적용(#433)

c0f46c5

Merge branch 'dev' into feat/433-search

022b301

nuyh99 approved these changes Sep 25, 2023

View reviewed changes

hum02 added 3 commits September 25, 2023 20:01

refactor:fulltext 인덱스 설정 sql문 컨벤션 수정(#433)

3ad24e4

refactor:match against문 querydsl 가독성 개선(#433)

e8d3325

Merge branch 'dev' into feat/433-search

595cbba

green-kong approved these changes Sep 26, 2023

View reviewed changes

hum02 merged commit cb81c57 into dev Sep 26, 2023
1 check passed

hum02 deleted the feat/433-search branch September 26, 2023 07:52


		import com.project.yozmcafe.domain.cafe.Cafe;

		public record CafeSearchResponse(long id, String name, String address, String image, int likeCount) {

		@@ -0,0 +1 @@
		com.project.yozmcafe.domain.cafe.CustomFunctionContributor

카페 검색 기능 개발 #443

카페 검색 기능 개발 #443

Conversation

hum02 commented Sep 10, 2023 • edited Loading

#️⃣ 연관된 이슈

📝 작업 내용

💬 리뷰 요구사항

full text search 쪽 설명이 부족해서 comment

CafeQueryRepository.searchCafesByWord()가 꽤 클린하지 못합니다...

Cafe.images의 fetchjoin문제

검색 체크박스가 어떤식으로 검색되어야 할까- and식으로? or식으로?

donghae-kim left a comment • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

green-kong left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

hum02 Sep 15, 2023 • edited Loading

Choose a reason for hiding this comment

nuyh99 left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

hum02 commented Sep 14, 2023 • edited Loading

nuyh99 left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

nuyh99 Sep 14, 2023 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

nuyh99 Sep 14, 2023 • edited Loading

Choose a reason for hiding this comment

nuyh99 Sep 14, 2023 • edited Loading

Choose a reason for hiding this comment

hum02 Sep 19, 2023 • edited Loading

Choose a reason for hiding this comment

nuyh99 Sep 20, 2023 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

hum02 Sep 19, 2023 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

nuyh99 Sep 14, 2023 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

donghae-kim left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

hum02 Sep 19, 2023 • edited Loading

Choose a reason for hiding this comment

green-kong left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

hum02 Sep 19, 2023 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

hum02 commented Sep 10, 2023 •

edited

Loading

donghae-kim left a comment •

edited

Loading

hum02 Sep 15, 2023 •

edited

Loading

hum02 commented Sep 14, 2023 •

edited

Loading

nuyh99 Sep 14, 2023 •

edited

Loading

nuyh99 Sep 14, 2023 •

edited

Loading

nuyh99 Sep 14, 2023 •

edited

Loading

hum02 Sep 19, 2023 •

edited

Loading

nuyh99 Sep 20, 2023 •

edited

Loading

hum02 Sep 19, 2023 •

edited

Loading

nuyh99 Sep 14, 2023 •

edited

Loading

hum02 Sep 19, 2023 •

edited

Loading

hum02 Sep 19, 2023 •

edited

Loading

hum02 commented Sep 25, 2023 •

edited

Loading

hum02 Sep 25, 2023 •

edited

Loading

hum02 Sep 25, 2023 •

edited

Loading

hum02 Sep 26, 2023 •

edited

Loading

hum02 Sep 25, 2023 •

edited

Loading