[BE] feat: 축제 대학교 검색 기능 보완 및 정렬(#926) #927

BGuga · 2024-05-01T15:38:27Z

📌 관련 이슈

closed: [BE] 축제 대학교 검색 기능 보완 및 정렬 #926

✨ PR 세부 내용

검색 로직은 기존에
~ 대학교 혹은 ~대로 끝난다면 축제 이름에 해당 대학교 이름이 적힌 축제를 반환하고 나머지 경우에 아티스트 이름을 기반으로 검색하였습니다.
변경된 로직은
해당 이름에 해당하는 아티스트가 있는지 확인하고 없으면 축제 이름에 해당 키워드가 포함되어 있다면 반환하도록 변경하였습니다.
이로 인해 부경만 쳤을 경우 부경이란 키워드가 포함된 모든 축제를 받을 수 있습니다.
축제 이름에 해당 키워드가 포함되었는지 확인하는 것이기 때문에 만약 "축제"로 검색을 한다면 "축제"가 이름에 포함된 모든 축제가 반환됩니다.
이 부분에 대한 방어 로직을 짜려했으나 앱 초기에는 데이터도 얼마 없을 뿐더러 후에 페이징 적용시에 고려해도 좋을 것이라 판단하여 최대한 검색이 자유롭게 되도록 그대로 두었습니다.

github-actions · 2024-05-01T15:40:49Z

Test Results

228 files 228 suites 29s ⏱️
764 tests 764 ✅ 0 💤 0 ❌
783 runs 783 ✅ 0 💤 0 ❌

Results for commit e8c4932.

♻️ This comment has been updated with latest results.

seokjin8678

고생하셨습니다 푸우!
코드의 가독성을 조금 개선해 볼 수 있으면 좋겠네요!

seokjin8678 · 2024-05-02T05:01:32Z

.../src/main/java/com/festago/festival/repository/ArtistFestivalSearchV1QueryDslRepository.java

+    public boolean existsByName(String keyword) {
+        return existsByExpression(getBooleanExpressionByKeyword(keyword));
+    }
+
+    private boolean existsByExpression(BooleanExpression expression) {
+        return !selectFrom(artist)
+            .where(expression)
+            .fetch()
+            .isEmpty();
    }


이렇게 사용할 수 있을 것 같네요!

public boolean existsByName(String keyword) { return !selectFrom(artist) .where(getBooleanExpressionByKeyword(keyword)) .fetch() .isEmpty(); }

또한 #893 이슈에도 남겼지만, 해당 쿼리 실행하고 실행 계획 분석 부탁 드립니다!

private List<FestivalSearchV1Response> searchByExpression(BooleanExpression expression) { return select( new QFestivalSearchV1Response( festival.id, festival.name, festival.festivalDuration.startDate, festival.festivalDuration.endDate, festival.posterImageUrl, festivalQueryInfo.artistInfo)) .from(artist) .innerJoin(stageArtist).on(expression.and(stageArtist.artistId.eq(artist.id))) .innerJoin(stage).on(stage.id.eq(stageArtist.stageId)) .innerJoin(festival).on(festival.id.eq(stage.festival.id)) .innerJoin(festivalQueryInfo).on(festival.id.eq(festivalQueryInfo.festivalId)) .where(expression) .fetch(); }

해당 쿼리에 .innerJoin(stageArtist).on(expression.and(stageArtist.artistId.eq(artist.id))) 실행 분석 결과 다음과 같은 쿼리와 동일한 실행을 보장합니다.
이유는 innerJoin 대상으로 옵티마이저가 최적화를 해줘서 그렇다고 하네요.

private List<FestivalSearchV1Response> searchByExpression(BooleanExpression expression) { return select( new QFestivalSearchV1Response( festival.id, festival.name, festival.festivalDuration.startDate, festival.festivalDuration.endDate, festival.posterImageUrl, festivalQueryInfo.artistInfo)) .from(artist) .innerJoin(stageArtist).on(stageArtist.artistId.eq(artist.id)) .innerJoin(stage).on(stage.id.eq(stageArtist.stageId)) .innerJoin(festival).on(festival.id.eq(stage.festival.id)) .innerJoin(festivalQueryInfo).on(festival.id.eq(festivalQueryInfo.festivalId)) .where(expression) .fetch(); }

그렇게 되면 최종적으로 다음과 같이 코드가 변할 것 같네요.

@Repository public class ArtistFestivalSearchV1QueryDslRepository extends QueryDslRepositorySupport { public ArtistFestivalSearchV1QueryDslRepository() { super(Artist.class); } public List<FestivalSearchV1Response> executeSearch(String keyword) { return select(new QFestivalSearchV1Response( festival.id, festival.name, festival.festivalDuration.startDate, festival.festivalDuration.endDate, festival.posterImageUrl, festivalQueryInfo.artistInfo) ) .from(artist) .innerJoin(stageArtist).on(stageArtist.artistId.eq(artist.id)) .innerJoin(stage).on(stage.id.eq(stageArtist.stageId)) .innerJoin(festival).on(festival.id.eq(stage.festival.id)) .innerJoin(festivalQueryInfo).on(festival.id.eq(festivalQueryInfo.festivalId)) .where(getBooleanExpressionByKeyword(keyword)) .fetch(); } private BooleanExpression getBooleanExpressionByKeyword(String keyword) { int keywordLength = keyword.length(); if (keywordLength == 0) { throw new BadRequestException(ErrorCode.INVALID_KEYWORD); } if (keywordLength == 1) { return artist.name.eq(keyword); } return artist.name.contains(keyword); } public boolean existsByName(String keyword) { return !selectFrom(artist) .where(getBooleanExpressionByKeyword(keyword)) .fetch() .isEmpty(); } }

seokjin8678 · 2024-05-02T05:18:59Z

backend/src/main/java/com/festago/festival/application/FestivalSearchV1QueryService.java

+    private EnumMap<FestivalFilter, List<FestivalSearchV1Response>> divideByStatus(
+        List<FestivalSearchV1Response> festivals) {
+        return festivals.stream()
+            .collect(Collectors.groupingBy(
+                this::determineStatus,
+                () -> new EnumMap<>(FestivalFilter.class),
+                Collectors.toList()
+            ));
+    }


반환 타입이 구체적일 필요가 있을까요?

seokjin8678 · 2024-05-02T06:24:20Z

backend/src/main/java/com/festago/festival/application/FestivalSearchV1QueryService.java

+    private List<FestivalSearchV1Response> sortFestival(List<FestivalSearchV1Response> festivals) {
+        EnumMap<FestivalFilter, List<FestivalSearchV1Response>> festivalByStatus = divideByStatus(festivals);
+        List<FestivalSearchV1Response> result = new ArrayList<>();
+        result.addAll(sortByStatus(PROGRESS, festivalByStatus.get(PROGRESS)));
+        result.addAll(sortByStatus(PLANNED, festivalByStatus.get(PLANNED)));
+        result.addAll(sortByStatus(END, festivalByStatus.get(END)));
+        return result;
+    }


result.addAll()을 호출하는 순서가 결과에 영향을 미치네요
이렇게 되면 코드의 중복이 발생할 것 같구요. 😂
FestivalFilter를 정렬한다면 호출하는 순서에 영향을 주지 않고도 중복을 제거할 수 있을 것 같네요!

seokjin8678 · 2024-05-02T06:25:23Z

backend/src/main/java/com/festago/festival/application/FestivalSearchV1QueryService.java

+    private List<FestivalSearchV1Response> sortByStatus(
+        FestivalFilter status,
+        List<FestivalSearchV1Response> festivals) {
+        if (festivals == null) {
+            return Collections.emptyList();
+        }
+        if (status == END) {
+            sortEndFestival(festivals);
+        }
+        if (status == PROGRESS) {
+            sortProgressFestival(festivals);
+        }
+        if (status == PLANNED) {
+            sortPlannedFestival(festivals);
+        }
+        return festivals;
+    }


인자로 들어오는 festivals이 null이 아니라면 null 검사를 할 필요가 없지 않을까요?
Enum 타입에 대해선 switch문이 가독성이 더 높을 것 같네요!

seokjin8678 · 2024-05-02T06:27:56Z

backend/src/main/java/com/festago/festival/application/FestivalSearchV1QueryService.java

+    private void sortEndFestival(List<FestivalSearchV1Response> festivals) {
+        festivals.sort((festival1, festival2) -> {
+            if (festival1.endDate().isAfter(festival2.endDate())) {
+                return 1;
+            }
+            if (festival1.endDate().isEqual(festival2.endDate())) {
+                return 0;
+            }
+            return -1;
+        });
+    }
+
+    private void sortProgressFestival(List<FestivalSearchV1Response> festivals) {
+        festivals.sort((festival1, festival2) -> {
+            if (festival1.startDate().isBefore(festival2.endDate())) {
+                return 1;
+            }
+            if (festival1.startDate().isEqual(festival2.startDate())) {
+                return 0;
+            }
+            return -1;
+        });
+    }
+
+    private void sortPlannedFestival(List<FestivalSearchV1Response> festivals) {
+        festivals.sort((festival1, festival2) -> {
+            if (festival1.startDate().isBefore(festival2.endDate())) {
+                return 1;
+            }
+            if (festival1.startDate().isEqual(festival2.startDate())) {
+                return 0;
+            }
+            return -1;
+        });
    }


Comparator.comparing() 메서드를 사용하지 않으신 이유가 있을까요?

짤 때 하나의 값 비교가 아니고 StartDate, EndDate를 복합적으로 생각하지 않을까 생각했는데 하나의 값 비교로 가능하군요!! 변경했습니다

seokjin8678 · 2024-05-02T06:28:47Z

backend/src/main/java/com/festago/festival/application/FestivalSearchV1QueryService.java

+            if (festival1.startDate().isBefore(festival2.endDate())) {
+                return 1;
+            }


진행 중, 진행 예정 축제의 경우 정렬 기준이 시작일 내림차순 일텐데, 이 부분은 의도하신걸까요?

앗 아닙니다

seokjin8678

고생하셨습니다 푸우!
덕분에 코드가 보기 쉽게 바뀐것 같네요!
추가 리뷰는 가볍게 생각해주시면 될 것 같습니다.
그리고 제가 작성한 FestivalSearchV1QueryService 코드도 첨부할게요.
전 스트림을 사용해서 리팩터링 해봤네요 😁

@Service
@Transactional(readOnly = true)
@RequiredArgsConstructor
public class FestivalSearchV1QueryService {

    private final ArtistFestivalSearchV1QueryDslRepository artistFestivalSearchV1QueryDslRepository;
    private final SchoolFestivalSearchV1QueryDslRepository schoolFestivalSearchV1QueryDslRepository;
    private final Clock clock;

    public List<FestivalSearchV1Response> search(String keyword) {
        List<FestivalSearchV1Response> festivals = findFestivals(keyword);
        Map<FestivalFilter, List<FestivalSearchV1Response>> statusToFestivals = groupByStatus(festivals);
        return sortAndMerge(statusToFestivals);
    }

    private List<FestivalSearchV1Response> findFestivals(String keyword) {
        if (artistFestivalSearchV1QueryDslRepository.existsByName(keyword)) {
            return artistFestivalSearchV1QueryDslRepository.executeSearch(keyword);
        }
        return schoolFestivalSearchV1QueryDslRepository.executeSearch(keyword);
    }

    private Map<FestivalFilter, List<FestivalSearchV1Response>> groupByStatus(
        List<FestivalSearchV1Response> festivals
    ) {
        return festivals.stream()
            .collect(groupingBy(this::determineStatus, () -> new EnumMap<>(FestivalFilter.class), toList()));
    }

    private FestivalFilter determineStatus(FestivalSearchV1Response festival) {
        LocalDate now = LocalDate.now(clock);
        if (now.isAfter(festival.endDate())) {
            return END;
        }
        if (now.isBefore(festival.startDate())) {
            return PLANNED;
        }
        return PROGRESS;
    }

    private List<FestivalSearchV1Response> sortAndMerge(
        Map<FestivalFilter, List<FestivalSearchV1Response>> statusToFestivals
    ) {
        statusToFestivals.forEach((status, festival) -> festival.sort(getFestivalComparatorByStatus(status)));
        return statusToFestivals.entrySet().stream()
            .sorted(comparing(entry -> getStatusOrder(entry.getKey())))
            .flatMap(entry -> entry.getValue().stream())
            .toList();
    }

    private Comparator<FestivalSearchV1Response> getFestivalComparatorByStatus(FestivalFilter status) {
        return switch (status) {
            case END -> comparing(FestivalSearchV1Response::endDate)
                .thenComparing(FestivalSearchV1Response::id);
            case PROGRESS, PLANNED -> comparing(FestivalSearchV1Response::startDate).reversed()
                .thenComparing(FestivalSearchV1Response::id);
        };
    }

    private int getStatusOrder(FestivalFilter status) {
        return switch (status) {
            case PROGRESS -> 0;
            case PLANNED -> 1;
            case END -> 2;
        };
    }
}

seokjin8678 · 2024-05-02T13:04:31Z

backend/src/main/java/com/festago/festival/application/FestivalSearchV1QueryService.java

+    private void sortByStatus(
+        FestivalFilter status,
+        List<FestivalSearchV1Response> festivals) {
+
+        switch (status) {
+            case END -> festivals.sort(Comparator.comparing(FestivalSearchV1Response::endDate).reversed());
+            case PROGRESS, PLANNED -> festivals.sort(Comparator.comparing(FestivalSearchV1Response::startDate));
        }
-        return artistFestivalSearchV1QueryDslRepository.executeSearch(keyword);
    }


👍👍
추가로 .thenComparing(FestivalSearchV1Response::id) 추가해서 일정이 같은 축제의 경우 두 번째 정렬 조건을 추가해줄 수 있을 것 같네요!

seokjin8678 · 2024-05-02T13:07:13Z

backend/src/main/java/com/festago/festival/application/FestivalSearchV1QueryService.java

+    private List<FestivalFilter> determineFestivalOrder() {
+        return List.of(PROGRESS, PLANNED, END);
+    }


해당 함수 대신 FestivalFilter.values()를 사용할 수 있을 것 같은데..
사용하지 않으신 이유가 있을까요?
만약, 있으시다면 해당 리스트를 static으로 만들어 사용해도 될 것 같네요!

이거 values를 쓸까 고민했는데 FestivalFilter에서 적힌 순서가 정렬 순서라는게 명시적이지 않은 것이라 생각하여 후에 순서를 변경할 수 있다 생각했습니다!! 그래서 정렬이 필요한 곳에서 정렬 조건을 정의하고자 순서를 명시해줬네요

seokjin8678 · 2024-05-02T13:08:11Z

backend/src/main/java/com/festago/festival/application/FestivalSearchV1QueryService.java

+    private Map<FestivalFilter, List<FestivalSearchV1Response>> divideByStatus(
+        List<FestivalSearchV1Response> festivals) {
+        return festivals.stream()
+            .collect(Collectors.groupingBy(
+                this::determineStatus,
+                () -> new EnumMap<>(FestivalFilter.class),
+                Collectors.toList()
+            ));
+    }


줄바꿈을 다음과 같이 사용하는건 어떤가요?

Suggested change

private Map<FestivalFilter, List<FestivalSearchV1Response>> divideByStatus(

List<FestivalSearchV1Response> festivals) {

return festivals.stream()

.collect(Collectors.groupingBy(

this::determineStatus,

() -> new EnumMap<>(FestivalFilter.class),

Collectors.toList()

));

}

private Map<FestivalFilter, List<FestivalSearchV1Response>> divideByStatus(

List<FestivalSearchV1Response> festivals

) {

return festivals.stream()

.collect(Collectors.groupingBy(

this::determineStatus,

() -> new EnumMap<>(FestivalFilter.class),

Collectors.toList()

));

}

seokjin8678 · 2024-05-02T13:10:27Z

backend/src/main/java/com/festago/festival/application/FestivalSearchV1QueryService.java

+            List<FestivalSearchV1Response> festivalsByFilter = festivalByStatus.getOrDefault(festivalFilter,
+                new ArrayList<>());


반환되는 리스트에 변경이 없다면 Collections.emptyList()를 사용해도 좋을 것 같네요!

Suggested change

List<FestivalSearchV1Response> festivalsByFilter = festivalByStatus.getOrDefault(festivalFilter,

new ArrayList<>());

List<FestivalSearchV1Response> festivalsByFilter = festivalByStatus.getOrDefault(festivalFilter,

Collections.emptyList());

feat: 축제 검색 시 ~대 ~대학교를 붙히지 않더라도 검색 가능 및 진행, 예정, 종료 순 정렬 적용

bed93b1

BGuga linked an issue May 1, 2024 that may be closed by this pull request

[BE] 축제 대학교 검색 기능 보완 및 정렬 #926

Closed

BGuga self-assigned this May 1, 2024

seokjin8678 requested changes May 2, 2024

View reviewed changes

seokjin8678 added BE 백엔드에 관련된 작업 ⚙️ 리팩터링 리팩터링에 관련된 작업 labels May 2, 2024

github-actions bot requested review from carsago, seokjin8678 and xxeol2 May 2, 2024 06:51

BGuga added 2 commits May 2, 2024 16:15

refactor: 아티스트 축제 조회 로직 리팩토링

5b771b1

refactor: 축제 정렬 리팩토링

7388c0a

BGuga force-pushed the feat/#926 branch from 825d007 to 7388c0a Compare May 2, 2024 07:38

seokjin8678 approved these changes May 2, 2024

View reviewed changes

refactor: 개행 및 EmptyList 반환

e8c4932

BGuga merged commit a5bf7c3 into dev May 15, 2024
3 checks passed

BGuga deleted the feat/#926 branch May 15, 2024 06:13

seokjin8678 mentioned this pull request May 16, 2024

[BE] FestivalSearchV1QueryService로 반환되는 결과에 정렬을 적용한다. #879

Closed

seokjin8678 mentioned this pull request May 30, 2024

[BE] ArtistFestivalSearchV1QueryDslRepository에서 조회되는 데이터에 limit을 적용한다. #891

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[BE] feat: 축제 대학교 검색 기능 보완 및 정렬(#926) #927

[BE] feat: 축제 대학교 검색 기능 보완 및 정렬(#926) #927

BGuga commented May 1, 2024

github-actions bot commented May 1, 2024 •

edited

Loading

seokjin8678 left a comment

seokjin8678 May 2, 2024

seokjin8678 May 2, 2024

seokjin8678 May 2, 2024

seokjin8678 May 2, 2024

seokjin8678 May 2, 2024

seokjin8678 May 2, 2024

seokjin8678 May 2, 2024

BGuga May 2, 2024

seokjin8678 May 2, 2024

BGuga May 2, 2024

seokjin8678 left a comment •

edited

Loading

seokjin8678 May 2, 2024

seokjin8678 May 2, 2024

BGuga May 7, 2024

seokjin8678 May 2, 2024

seokjin8678 May 2, 2024

		List<FestivalSearchV1Response> festivalsByFilter = festivalByStatus.getOrDefault(festivalFilter,
		new ArrayList<>());

[BE] feat: 축제 대학교 검색 기능 보완 및 정렬(#926) #927

[BE] feat: 축제 대학교 검색 기능 보완 및 정렬(#926) #927

Conversation

BGuga commented May 1, 2024

📌 관련 이슈

✨ PR 세부 내용

github-actions bot commented May 1, 2024 • edited Loading

Test Results

seokjin8678 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

seokjin8678 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

github-actions bot commented May 1, 2024 •

edited

Loading

seokjin8678 left a comment •

edited

Loading