페이지네이션 처리 시 최대 다음 페이지 수를 5개로 제한 #902
Conversation
| long nextFixedElementCounts = Objects.requireNonNull(queryFactory | ||
| .selectFrom(entityPath) | ||
| .where(conditions) | ||
| .offset(pageable.getOffset()) | ||
| .limit(maximumElementsCount) | ||
| .fetch()) | ||
| .size(); |
There was a problem hiding this comment.
fetch() 메서드의 결과가 List 타입이라 결과가 없으면 Null 대신 빈 리스트가 반환되지 않나요?
만약 그렇다면 fetch()의 반환값에 대해 null check을 하지 않아도 될 것 같아요.
그래서 다음과 같이 개선할 수 있을 것 같아 제안해요 몰리~!
| long nextFixedElementCounts = Objects.requireNonNull(queryFactory | |
| .selectFrom(entityPath) | |
| .where(conditions) | |
| .offset(pageable.getOffset()) | |
| .limit(maximumElementsCount) | |
| .fetch()) | |
| .size(); | |
| List<?> entities = queryFactory | |
| .selectFrom(entityPath) | |
| .where(conditions) | |
| .offset(pageable.getOffset()) | |
| .limit(maximumElementsCount) | |
| .fetch(); | |
| long nextFixedElementCounts = entities.size(); |
만약 여전히 null check을 해줘야 한다고 하면, 아래와 같이 에러 메시지를 명시하는 게 좋을 것 같아요!
List<?> entities = queryFactory
.selectFrom(entityPath)
.where(conditions)
.offset(pageable.getOffset())
.limit(maximumElementsCount)
.fetch();
Objects.requireNonNull(entities, "쿼리 결과는 null이 될 수 없습니다.");
long nextFixedElementCounts = entities.size();There was a problem hiding this comment.
처음에 select(entityPath.count()) + fetchOne로 작성하려고 했었는데요. 그때 null check가 남아있었네요 �변경할게요 최고 👍
| } | ||
|
|
||
| @Test | ||
| @DisplayName("페이지가 10개까지 있어도, 최대 5페이지까지만 카운팅") |
There was a problem hiding this comment.
search.sql에는 총 7개의 템플릿이 저장되어 있는 것 같아요.
한 페이지에 1개씩 뽑으면 총 페이지는 10이 아니라 7일 것 같은데 맞나요??
혹시 제가 잘못 이해하고 있는 부분이 있으면 얘기해주세요!!
There was a problem hiding this comment.
앗, @DisplayName을 확인하지 못했네요!
총 페이지는 5가 되어야 합니다!
정책이 전체 페이지를 계산하지 않고 5까지만 세기입니답!
There was a problem hiding this comment.
아 아뇨 5페이지까지만 나오는건 맞는데 @DisplayName이 페이지가 7개까지 있어도, 최대 5페이지까지만 카운팅 이어야 한다는 얘기였습니다ㅎㅎ
| @Import({DataSourceConfig.class, QueryDSLConfig.class, FixedPageCounter.class}) | ||
| @AutoConfigureTestDatabase(replace = AutoConfigureTestDatabase.Replace.NONE) | ||
| @Sql(scripts = "classpath:search.sql", executionPhase = Sql.ExecutionPhase.BEFORE_TEST_METHOD) | ||
| class FixedPageCounterTest { |
There was a problem hiding this comment.
몰리가 걱정했던 부분인 최대 페이지보다 데이터의 양이 적은 경우 있는 만큼만 탐색한다에 대한 테스트도 있으면 좋을 것 같네요!!
There was a problem hiding this comment.
추가로 제가 간단하게 작성해봤는데 정상적으로 동작하는 것 같습니다ㅎㅎ
@Test
@DisplayName("데이터가 최대 페이지보다 적을 경우, 있는만큼만 카운팅")
void tmp() {
PageRequest pageRequest = PageRequest.of(0, 7);
QTemplate template = QTemplate.template;
int nextFixedPage = fixedPageCounter.countNextFixedPage(
queryFactory,
template,
pageRequest
);
assertThat(nextFixedPage).isEqualTo(1);
}| return new FixedPage<>(content, countNextFixedPage(memberId, pageable)); | ||
| } | ||
|
|
||
| private int countNextFixedPage(Long memberId, Pageable pageable) { |
There was a problem hiding this comment.
오버로딩을 사용한 것 같은데 개인적으로는 이해하기 조금 힘들었던 것 같아요.
매개변수에 따라서 역할이 드러나는 것 같지도 않아서 오버로딩이 적절한지 조금 고민이네요..
조금만 더 명시적으로 바꿔주는 건 어떤가요??
하지만 조금은 사소하다고 생각해서 몰리가 편한대로 결정해주세요!!
There was a problem hiding this comment.
저는 완전하게 동일한 역할이라고 생각했어요!
조회 조건이 달라질 수 있지만, 조건에 맞는 로우의 갯수를 조회하는 것이여서요!
초롱이 명시적으로 어떤 부분을 나타내고 싶은지 의견을 말해주면 제가 좀 더 고민해볼 수 있을 것 같아요 🤔
There was a problem hiding this comment.
그 조회 조건이 파라미터를 통해 구분되지 않는다는 것이 위험하다고 생각해요.
저는 개인적으로 오버로딩을 사용할 때는 메소드명과 파라미터만 봐도 그 메소드의 역할이 무엇인지 알고, 명확하게 구분되어야 한다고 생각해요.
하지만 현재는 로직을 봤을 때는 두 메소드가 구분이 되기는 하지만 파라미터만으로는 전혀 구분이 되지 않는 것 같아요.
제가 만약 두 메소드 중 하나를 사용하게 된다면, 어떤 메소드를 사용해야 할지 확인하기 어려울 것 같아요.
고정된 페이지를 가져온다라는 동일한 동작을 하더라도 그 동작을 하는 조건이 다르고 그 조건을 파라미터를 통해 나타낼 수 없다면 두 메소드는 다른 역할을 하는 것이라고 생각됩니다.
몰리의 의견은 어떤가요??
There was a problem hiding this comment.
앗 이해했습니다~!
예시로 좋아요한 템플릿 목록이 아닌 스크랩한 템플릿 목록이 생기면 초롱 의견처럼 구분이 불가능할 것 같네요 좋은 의견 감사합니다 👍
| class FindAllByMemberId { | ||
|
|
||
| @Test | ||
| @DisplayName("성공: 다른 사람의 private은 제외, 내 private은 포함") |
There was a problem hiding this comment.
아까 가서 물어본 거에 대한 정답이 여기 있었네요ㅋㅋ👍
backend/src/main/java/codezap/global/pagination/FixedPageCounter.java
Outdated
Show resolved
Hide resolved
backend/src/main/java/codezap/template/repository/TemplateQueryDSLRepository.java
Outdated
Show resolved
Hide resolved
⚡️ 관련 이슈
close #899
📍주요 변경 사항
그 과정에서 전체 로우를 풀스캔하기 때문에 비효율적입니다.
(조회 조건에 해당되는 컬럼이 100만건이라면 100만 로우를 전체 카운팅)
현재 페이지네이션 바가 5개까지 나타나기 때문에 전체 로우를 카운팅하지 않고 현재 페이지 포함 5개 페이지까지가 있는지 반환합니다.
내가 좋아요한 템플릿 목록 조회 API에서 동일한 페이지네이션 바가 사용되기 때문에템플릿 목록 조회 API와 동일하게 5개까지만 반환하는 처리를 하였습니다.그 과정에서
내가 좋아요한 "템플릿" 목록 조회레포지토리 메서드를 template 패키지로 이동하였습니다. (이전 담당자와 상의 완, 참고바람~~)🎸기타
+) Repository 단 테스트 시에 관련 Config 들을 Import해야하는데 @TemplateSearchRepositoryTest와 @RepositoryTest의 @import가 중복으로 수정되네욥ㅠ 이번 PR에서 변경사항이 많이 잡힐 것 같아 따로 PR 올릴게욥
🍗 PR 첫 리뷰 마감 기한
11/11 18:00