From 40b055cfeca09b6a4a30cd0d6f08f4fec28e7cf8 Mon Sep 17 00:00:00 2001 From: Igor Dianov Date: Mon, 30 Sep 2024 20:10:12 -0700 Subject: [PATCH 1/2] Revert "Skip total count if select result is empty" This reverts commit 7ca1394628bf2c2bb842d1358fe0592cf452b5f0. --- .../jpa/query/schema/impl/GraphQLJpaQueryDataFetcher.java | 8 +------- .../graphql/jpa/query/schema/impl/PagedResult.java | 5 ----- .../jpa/query/schema/impl/ResultStreamWrapper.java | 2 -- 3 files changed, 1 insertion(+), 14 deletions(-) diff --git a/schema/src/main/java/com/introproventures/graphql/jpa/query/schema/impl/GraphQLJpaQueryDataFetcher.java b/schema/src/main/java/com/introproventures/graphql/jpa/query/schema/impl/GraphQLJpaQueryDataFetcher.java index f305cb7f5..af06479ff 100644 --- a/schema/src/main/java/com/introproventures/graphql/jpa/query/schema/impl/GraphQLJpaQueryDataFetcher.java +++ b/schema/src/main/java/com/introproventures/graphql/jpa/query/schema/impl/GraphQLJpaQueryDataFetcher.java @@ -34,7 +34,6 @@ import graphql.schema.DataFetcher; import graphql.schema.DataFetchingEnvironment; import graphql.schema.GraphQLScalarType; -import java.util.Collection; import java.util.LinkedHashMap; import java.util.List; import java.util.Map; @@ -118,12 +117,7 @@ public PagedResult get(DataFetchingEnvironment environment) { } if (totalSelection.isPresent() || pagesSelection.isPresent()) { - final var selectResult = pagedResult.getSelect(); - - final long total = recordsSelection.isEmpty() || - selectResult.filter(Predicate.not(Collection::isEmpty)).isPresent() - ? queryFactory.queryTotalCount(environment, restrictedKeys) - : 0L; + final Long total = queryFactory.queryTotalCount(environment, restrictedKeys); pagedResult.withTotal(total); } diff --git a/schema/src/main/java/com/introproventures/graphql/jpa/query/schema/impl/PagedResult.java b/schema/src/main/java/com/introproventures/graphql/jpa/query/schema/impl/PagedResult.java index abba0b1c4..e41c9202d 100644 --- a/schema/src/main/java/com/introproventures/graphql/jpa/query/schema/impl/PagedResult.java +++ b/schema/src/main/java/com/introproventures/graphql/jpa/query/schema/impl/PagedResult.java @@ -20,7 +20,6 @@ import java.util.LinkedHashMap; import java.util.List; import java.util.Map; -import java.util.Optional; public class PagedResult { @@ -136,10 +135,6 @@ public Builder withSelect(List select) { return this; } - public Optional> getSelect() { - return Optional.ofNullable(select); - } - /** * Builder method for select parameter. * @param select field to set diff --git a/schema/src/main/java/com/introproventures/graphql/jpa/query/schema/impl/ResultStreamWrapper.java b/schema/src/main/java/com/introproventures/graphql/jpa/query/schema/impl/ResultStreamWrapper.java index 4ab11f065..1196bb9ab 100644 --- a/schema/src/main/java/com/introproventures/graphql/jpa/query/schema/impl/ResultStreamWrapper.java +++ b/schema/src/main/java/com/introproventures/graphql/jpa/query/schema/impl/ResultStreamWrapper.java @@ -63,8 +63,6 @@ public Object invoke(Object proxy, Method method, Object[] args) throws Throwabl return System.identityHashCode(proxy); } else if ("spliterator".equals(method.getName())) { return stream.spliterator(); - } else if ("isEmpty".equals(method.getName())) { - return size == 0; } throw new UnsupportedOperationException(method + " is not supported"); } From 927d614ae925c876f0f8ace0932178c601940a38 Mon Sep 17 00:00:00 2001 From: Igor Dianov Date: Mon, 30 Sep 2024 20:12:27 -0700 Subject: [PATCH 2/2] chore: Add test coverage for total count edge case --- .../schema/impl/ResultStreamWrapper.java | 2 ++ .../StarwarsQueryExecutorTestsSupport.java | 28 +++++++++++++++++++ 2 files changed, 30 insertions(+) diff --git a/schema/src/main/java/com/introproventures/graphql/jpa/query/schema/impl/ResultStreamWrapper.java b/schema/src/main/java/com/introproventures/graphql/jpa/query/schema/impl/ResultStreamWrapper.java index 1196bb9ab..4ab11f065 100644 --- a/schema/src/main/java/com/introproventures/graphql/jpa/query/schema/impl/ResultStreamWrapper.java +++ b/schema/src/main/java/com/introproventures/graphql/jpa/query/schema/impl/ResultStreamWrapper.java @@ -63,6 +63,8 @@ public Object invoke(Object proxy, Method method, Object[] args) throws Throwabl return System.identityHashCode(proxy); } else if ("spliterator".equals(method.getName())) { return stream.spliterator(); + } else if ("isEmpty".equals(method.getName())) { + return size == 0; } throw new UnsupportedOperationException(method + " is not supported"); } diff --git a/schema/src/test/java/com/introproventures/graphql/jpa/query/support/StarwarsQueryExecutorTestsSupport.java b/schema/src/test/java/com/introproventures/graphql/jpa/query/support/StarwarsQueryExecutorTestsSupport.java index 9a1a6f1f3..5620f5134 100644 --- a/schema/src/test/java/com/introproventures/graphql/jpa/query/support/StarwarsQueryExecutorTestsSupport.java +++ b/schema/src/test/java/com/introproventures/graphql/jpa/query/support/StarwarsQueryExecutorTestsSupport.java @@ -392,6 +392,20 @@ public void queryWhereRoot() { assertThat(result.toString()).isEqualTo(expected); } + @Test + public void queryWhereRoot2() { + //given: + String query = "query { Humans( page: { start: 4, limit: 2 }) { pages, total, select { name } } }"; + + String expected = "{Humans={pages=3, total=5, select=[]}}"; + + //when: + Object result = executor.execute(query).getData(); + + //then: + assertThat(result.toString()).isEqualTo(expected); + } + @Test public void queryWhereRootPagedWithVariables() { //given: @@ -427,6 +441,20 @@ public void queryPaginationWithoutRecords() { assertThat(result.toString()).isEqualTo(expected); } + @Test + public void queryPaginationWithoutRecords2() { + //given: + String query = "query { Humans ( page: { start: 4, limit: 2 }) { pages, total } }"; + + String expected = "{Humans={pages=3, total=5}}"; + + //when: + Object result = executor.execute(query).getData(); + + //then: + assertThat(result.toString()).isEqualTo(expected); + } + @Test public void queryOrderByFields() { //given: