Spring/spring-graphql
1
0
Fork 0
Code Issues Packages Projects Releases Wiki Activity

Apply default ScrollSubrange correctly

Browse Source 
The defaults are now applied before the call to ScrollSubrange#create.
After is too late given that for offset positions, the direction may
switch from backward to forward.

Closes gh-900
This commit is contained in:
rstoyanchev
2024-02-14 08:53:03 +00:00
parent 5248a3253b
commit dab10e7383
4 changed files with 183 additions and 96 deletions
Download Patch File Download Diff File Expand all files Collapse all files

64
spring-graphql/src/main/java/org/springframework/graphql/data/query/QueryByExampleDataFetcher.java
View File

@@ -1,5 +1,5 @@
/*
* Copyright 2002-2023 the original author or authors.
* Copyright 2002-2024 the original author or authors.
*
* Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License.
@@ -107,15 +107,9 @@ public abstract class QueryByExampleDataFetcher<T> {
private final GraphQlArgumentBinder argumentBinder;
@Nullable
private final CursorStrategy<ScrollPosition> cursorStrategy;
QueryByExampleDataFetcher(
TypeInformation<T> domainType, @Nullable CursorStrategy<ScrollPosition> cursorStrategy) {
QueryByExampleDataFetcher(TypeInformation<T> domainType) {
this.domainType = domainType;
this.cursorStrategy = cursorStrategy;
this.argumentBinder = new GraphQlArgumentBinder();
}
@@ -174,11 +168,6 @@ public abstract class QueryByExampleDataFetcher<T> {
return Collections.emptyList();
}
protected ScrollSubrange buildScrollSubrange(DataFetchingEnvironment environment) {
Assert.state(this.cursorStrategy != null, "Expected CursorStrategy");
return RepositoryUtils.buildScrollSubrange(environment, this.cursorStrategy);
}
@Override
public String toString() {
return getDescription();
@@ -434,7 +423,7 @@ public abstract class QueryByExampleDataFetcher<T> {
/**
* Configure a {@link ScrollSubrange} to use when a paginated request does
* not specify a cursor and/or a count of items.
* <p>By default, this is {@link OffsetScrollPosition#initial()} with a
* <p>By default, this is {@link OffsetScrollPosition#offset()} with a
* count of 20.
* @return a new {@link Builder} instance with all previously configured
* options and {@code Sort} applied
@@ -468,7 +457,7 @@ public abstract class QueryByExampleDataFetcher<T> {
* Build a {@link DataFetcher} to fetch many object instances.
*/
public DataFetcher<Iterable<R>> many() {
return new ManyEntityFetcher<>(this.executor, this.domainType, this.resultType, null, this.sort);
return new ManyEntityFetcher<>(this.executor, this.domainType, this.resultType, this.sort);
}
/**
@@ -581,7 +570,7 @@ public abstract class QueryByExampleDataFetcher<T> {
/**
* Configure a {@link ScrollSubrange} to use when a paginated request does
* not specify a cursor and/or a count of items.
* <p>By default, this is {@link OffsetScrollPosition#initial()} with a
* <p>By default, this is {@link OffsetScrollPosition#offset()} with a
* count of 20.
* @return a new {@link Builder} instance with all previously configured
* options and {@code Sort} applied
@@ -666,7 +655,7 @@ public abstract class QueryByExampleDataFetcher<T> {
SingleEntityFetcher(
QueryByExampleExecutor<T> executor, TypeInformation<T> domainType, Class<R> resultType, Sort sort) {
super(domainType, null);
super(domainType);
this.executor = executor;
this.resultType = resultType;
this.sort = sort;
@@ -712,10 +701,10 @@ public abstract class QueryByExampleDataFetcher<T> {
private final Sort sort;
ManyEntityFetcher(
QueryByExampleExecutor<T> executor, TypeInformation<T> domainType, Class<R> resultType,
@Nullable CursorStrategy<ScrollPosition> cursorStrategy, Sort sort) {
QueryByExampleExecutor<T> executor, TypeInformation<T> domainType,
Class<R> resultType, Sort sort) {
super(domainType, cursorStrategy);
super(domainType);
this.executor = executor;
this.resultType = resultType;
this.sort = sort;
@@ -756,23 +745,24 @@ public abstract class QueryByExampleDataFetcher<T> {
private static class ScrollableEntityFetcher<T, R> extends ManyEntityFetcher<T, R> {
private final CursorStrategy<ScrollPosition> cursorStrategy;
private final ScrollSubrange defaultSubrange;
private final ResolvableType scrollableResultType;
ScrollableEntityFetcher(
QueryByExampleExecutor<T> executor, TypeInformation<T> domainType, Class<R> resultType,
CursorStrategy<ScrollPosition> cursorStrategy,
ScrollSubrange defaultSubrange,
Sort sort) {
CursorStrategy<ScrollPosition> cursorStrategy, ScrollSubrange defaultSubrange, Sort sort) {
super(executor, domainType, resultType, cursorStrategy, sort);
super(executor, domainType, resultType, sort);
Assert.notNull(cursorStrategy, "CursorStrategy is required");
Assert.notNull(defaultSubrange, "Default ScrollSubrange is required");
Assert.isTrue(defaultSubrange.position().isPresent(), "Default ScrollPosition is required");
Assert.isTrue(defaultSubrange.count().isPresent(), "Default scroll limit is required");
this.cursorStrategy = cursorStrategy;
this.defaultSubrange = defaultSubrange;
this.scrollableResultType = ResolvableType.forClassWithGenerics(Window.class, resultType);
}
@@ -785,10 +775,10 @@ public abstract class QueryByExampleDataFetcher<T> {
@SuppressWarnings("OptionalGetWithoutIsPresent")
@Override
protected Iterable<R> getResult(FluentQuery.FetchableFluentQuery<R> queryToUse, DataFetchingEnvironment env) {
ScrollSubrange subrange = buildScrollSubrange(env);
int limit = subrange.count().orElse(this.defaultSubrange.count().getAsInt());
ScrollPosition position = subrange.position().orElse(this.defaultSubrange.position().get());
return queryToUse.limit(limit).scroll(position);
ScrollSubrange range = RepositoryUtils.getScrollSubrange(env, this.cursorStrategy, this.defaultSubrange);
int count = range.count().getAsInt();
ScrollPosition position = range.position().get();
return queryToUse.limit(count).scroll(position);
}
}
@@ -807,7 +797,7 @@ public abstract class QueryByExampleDataFetcher<T> {
ReactiveQueryByExampleExecutor<T> executor, TypeInformation<T> domainType,
Class<R> resultType, Sort sort) {
super(domainType, null);
super(domainType);
this.executor = executor;
this.resultType = resultType;
this.sort = sort;
@@ -855,7 +845,7 @@ public abstract class QueryByExampleDataFetcher<T> {
ReactiveQueryByExampleExecutor<T> executor, TypeInformation<T> domainType,
Class<R> resultType, Sort sort) {
super(domainType, null);
super(domainType);
this.executor = executor;
this.resultType = resultType;
this.sort = sort;
@@ -899,6 +889,8 @@ public abstract class QueryByExampleDataFetcher<T> {
private final ResolvableType scrollableResultType;
private final CursorStrategy<ScrollPosition> cursorStrategy;
private final ScrollSubrange defaultSubrange;
private final Sort sort;
@@ -907,7 +899,7 @@ public abstract class QueryByExampleDataFetcher<T> {
ReactiveQueryByExampleExecutor<T> executor, TypeInformation<T> domainType, Class<R> resultType,
CursorStrategy<ScrollPosition> cursorStrategy, ScrollSubrange defaultSubrange, Sort sort) {
super(domainType, cursorStrategy);
super(domainType);
Assert.notNull(cursorStrategy, "CursorStrategy is required");
Assert.notNull(defaultSubrange, "Default ScrollSubrange is required");
@@ -917,6 +909,7 @@ public abstract class QueryByExampleDataFetcher<T> {
this.executor = executor;
this.resultType = resultType;
this.scrollableResultType = ResolvableType.forClassWithGenerics(Iterable.class, resultType);
this.cursorStrategy = cursorStrategy;
this.defaultSubrange = defaultSubrange;
this.sort = sort;
}
@@ -943,11 +936,10 @@ public abstract class QueryByExampleDataFetcher<T> {
queryToUse = queryToUse.project(buildPropertyPaths(env.getSelectionSet(), this.resultType));
}
ScrollSubrange subrange = buildScrollSubrange(env);
int limit = subrange.count().orElse(this.defaultSubrange.count().getAsInt());
ScrollPosition position = subrange.position().orElse(this.defaultSubrange.position().get());
return queryToUse.limit(limit).scroll(position).map(Function.identity());
ScrollSubrange range = RepositoryUtils.getScrollSubrange(env, this.cursorStrategy, this.defaultSubrange);
int count = range.count().getAsInt();
ScrollPosition position = range.position().get();
return queryToUse.limit(count).scroll(position).map(Function.identity());
});
}

93
spring-graphql/src/main/java/org/springframework/graphql/data/query/QuerydslDataFetcher.java
View File

@@ -1,5 +1,5 @@
/*
* Copyright 2002-2023 the original author or authors.
* Copyright 2002-2024 the original author or authors.
*
* Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License.
@@ -120,17 +120,10 @@ public abstract class QuerydslDataFetcher<T> {
private final QuerydslBinderCustomizer<EntityPath<?>> customizer;
@Nullable
private final CursorStrategy<ScrollPosition> cursorStrategy;
QuerydslDataFetcher(
TypeInformation<T> domainType, QuerydslBinderCustomizer<EntityPath<?>> customizer,
@Nullable CursorStrategy<ScrollPosition> cursorStrategy) {
QuerydslDataFetcher(TypeInformation<T> domainType, QuerydslBinderCustomizer<EntityPath<?>> customizer) {
this.domainType = domainType;
this.customizer = customizer;
this.cursorStrategy = cursorStrategy;
}
@@ -198,11 +191,6 @@ public abstract class QuerydslDataFetcher<T> {
return Collections.emptyList();
}
protected ScrollSubrange buildScrollSubrange(DataFetchingEnvironment environment) {
Assert.state(this.cursorStrategy != null, "Expected CursorStrategy");
return RepositoryUtils.buildScrollSubrange(environment, this.cursorStrategy);
}
@Override
public String toString() {
return getDescription();
@@ -481,7 +469,7 @@ public abstract class QuerydslDataFetcher<T> {
/**
* Configure a {@link ScrollSubrange} to use when a paginated request does
* not specify a cursor and/or a count of items.
* <p>By default, this is {@link OffsetScrollPosition#initial()} with a
* <p>By default, this is {@link OffsetScrollPosition#offset()} with a
* count of 20.
* @return a new {@link Builder} instance with all previously configured
* options and {@code Sort} applied
@@ -536,7 +524,7 @@ public abstract class QuerydslDataFetcher<T> {
*/
public DataFetcher<Iterable<R>> many() {
return new ManyEntityFetcher<>(
this.executor, this.domainType, this.resultType, null, this.sort, this.customizer);
this.executor, this.domainType, this.resultType, this.sort, this.customizer);
}
/**
@@ -653,7 +641,7 @@ public abstract class QuerydslDataFetcher<T> {
/**
* Configure a {@link ScrollSubrange} to use when a paginated request does
* not specify a cursor and/or a count of items.
* <p>By default, this is {@link OffsetScrollPosition#initial()} with a
* <p>By default, this is {@link OffsetScrollPosition#offset()} with a
* count of 20.
* @return a new {@link Builder} instance with all previously configured
* options and {@code Sort} applied
@@ -758,13 +746,11 @@ public abstract class QuerydslDataFetcher<T> {
private final Sort sort;
@SuppressWarnings({"unchecked", "rawtypes"})
SingleEntityFetcher(QuerydslPredicateExecutor<T> executor,
TypeInformation<T> domainType,
Class<R> resultType,
Sort sort,
QuerydslBinderCustomizer<? extends EntityPath<T>> customizer) {
SingleEntityFetcher(
QuerydslPredicateExecutor<T> executor, TypeInformation<T> domainType, Class<R> resultType,
Sort sort, QuerydslBinderCustomizer<? extends EntityPath<T>> customizer) {
super(domainType, (QuerydslBinderCustomizer) customizer, null);
super(domainType, (QuerydslBinderCustomizer) customizer);
this.executor = executor;
this.resultType = resultType;
this.sort = sort;
@@ -810,13 +796,11 @@ public abstract class QuerydslDataFetcher<T> {
private final Sort sort;
@SuppressWarnings({"unchecked", "rawtypes"})
ManyEntityFetcher(QuerydslPredicateExecutor<T> executor,
TypeInformation<T> domainType,
Class<R> resultType,
@Nullable CursorStrategy<ScrollPosition> cursorStrategy,
Sort sort,
QuerydslBinderCustomizer<? extends EntityPath<T>> customizer) {
super(domainType, (QuerydslBinderCustomizer) customizer, cursorStrategy);
ManyEntityFetcher(
QuerydslPredicateExecutor<T> executor, TypeInformation<T> domainType, Class<R> resultType,
Sort sort, QuerydslBinderCustomizer<? extends EntityPath<T>> customizer) {
super(domainType, (QuerydslBinderCustomizer) customizer);
this.executor = executor;
this.resultType = resultType;
this.sort = sort;
@@ -857,6 +841,8 @@ public abstract class QuerydslDataFetcher<T> {
private static class ScrollableEntityFetcher<T, R> extends ManyEntityFetcher<T, R> {
private final CursorStrategy<ScrollPosition> cursorStrategy;
private final ScrollSubrange defaultSubrange;
ScrollableEntityFetcher(QuerydslPredicateExecutor<T> executor,
@@ -867,23 +853,24 @@ public abstract class QuerydslDataFetcher<T> {
Sort sort,
QuerydslBinderCustomizer<? extends EntityPath<T>> customizer) {
super(executor, domainType, resultType, cursorStrategy, sort, customizer);
super(executor, domainType, resultType, sort, customizer);
Assert.notNull(cursorStrategy, "CursorStrategy is required");
Assert.notNull(defaultSubrange, "Default ScrollSubrange is required");
Assert.isTrue(defaultSubrange.position().isPresent(), "Default ScrollPosition is required");
Assert.isTrue(defaultSubrange.count().isPresent(), "Default scroll limit is required");
this.cursorStrategy = cursorStrategy;
this.defaultSubrange = defaultSubrange;
}
@SuppressWarnings("OptionalGetWithoutIsPresent")
@Override
protected Iterable<R> getResult(FetchableFluentQuery<R> queryToUse, DataFetchingEnvironment env) {
ScrollSubrange subrange = buildScrollSubrange(env);
int limit = subrange.count().orElse(this.defaultSubrange.count().getAsInt());
ScrollPosition position = subrange.position().orElse(this.defaultSubrange.position().get());
return queryToUse.limit(limit).scroll(position);
ScrollSubrange range = RepositoryUtils.getScrollSubrange(env, this.cursorStrategy, this.defaultSubrange);
int count = range.count().getAsInt();
ScrollPosition position = range.position().get();
return queryToUse.limit(count).scroll(position);
}
}
@@ -899,13 +886,12 @@ public abstract class QuerydslDataFetcher<T> {
private final Sort sort;
@SuppressWarnings({"unchecked", "rawtypes"})
ReactiveSingleEntityFetcher(ReactiveQuerydslPredicateExecutor<T> executor,
TypeInformation<T> domainType,
Class<R> resultType,
Sort sort,
QuerydslBinderCustomizer<? extends EntityPath<T>> customizer) {
ReactiveSingleEntityFetcher(
ReactiveQuerydslPredicateExecutor<T> executor, TypeInformation<T> domainType, Class<R> resultType,
Sort sort, QuerydslBinderCustomizer<? extends EntityPath<T>> customizer) {
super(domainType, (QuerydslBinderCustomizer) customizer);
super(domainType, (QuerydslBinderCustomizer) customizer, null);
this.executor = executor;
this.resultType = resultType;
this.sort = sort;
@@ -950,13 +936,12 @@ public abstract class QuerydslDataFetcher<T> {
private final Sort sort;
@SuppressWarnings({"unchecked", "rawtypes"})
ReactiveManyEntityFetcher(ReactiveQuerydslPredicateExecutor<T> executor,
TypeInformation<T> domainType,
Class<R> resultType,
Sort sort,
QuerydslBinderCustomizer<? extends EntityPath<T>> customizer) {
ReactiveManyEntityFetcher(
ReactiveQuerydslPredicateExecutor<T> executor, TypeInformation<T> domainType, Class<R> resultType,
Sort sort, QuerydslBinderCustomizer<? extends EntityPath<T>> customizer) {
super(domainType, (QuerydslBinderCustomizer) customizer);
super(domainType, (QuerydslBinderCustomizer) customizer, null);
this.executor = executor;
this.resultType = resultType;
this.sort = sort;
@@ -1000,6 +985,8 @@ public abstract class QuerydslDataFetcher<T> {
private final ResolvableType scrollableResultType;
private final CursorStrategy<ScrollPosition> cursorStrategy;
private final ScrollSubrange defaultSubrange;
private final Sort sort;
@@ -1012,7 +999,7 @@ public abstract class QuerydslDataFetcher<T> {
Sort sort,
QuerydslBinderCustomizer<? extends EntityPath<T>> customizer) {
super(domainType, (QuerydslBinderCustomizer) customizer, cursorStrategy);
super(domainType, (QuerydslBinderCustomizer) customizer);
Assert.notNull(cursorStrategy, "CursorStrategy is required");
Assert.notNull(defaultSubrange, "Default ScrollSubrange is required");
@@ -1022,6 +1009,7 @@ public abstract class QuerydslDataFetcher<T> {
this.executor = executor;
this.resultType = resultType;
this.scrollableResultType = ResolvableType.forClassWithGenerics(Iterable.class, resultType);
this.cursorStrategy = cursorStrategy;
this.defaultSubrange = defaultSubrange;
this.sort = sort;
}
@@ -1048,11 +1036,10 @@ public abstract class QuerydslDataFetcher<T> {
queryToUse = queryToUse.project(buildPropertyPaths(env.getSelectionSet(), this.resultType));
}
ScrollSubrange subrange = buildScrollSubrange(env);
int limit = subrange.count().orElse(this.defaultSubrange.count().getAsInt());
ScrollPosition position = subrange.position().orElse(this.defaultSubrange.position().get());
return queryToUse.limit(limit).scroll(position).map(Function.identity());
ScrollSubrange range = RepositoryUtils.getScrollSubrange(env, this.cursorStrategy, this.defaultSubrange);
int count = range.count().getAsInt();
ScrollPosition position = range.position().get();
return queryToUse.limit(count).scroll(position).map(Function.identity());
});
}

18
spring-graphql/src/main/java/org/springframework/graphql/data/query/RepositoryUtils.java
View File

@@ -88,14 +88,18 @@ class RepositoryUtils {
return ScrollSubrange.create(ScrollPosition.offset(), 20, true);
}
public static ScrollSubrange buildScrollSubrange(
DataFetchingEnvironment environment, CursorStrategy<ScrollPosition> cursorStrategy) {
public static ScrollSubrange getScrollSubrange(
DataFetchingEnvironment env, CursorStrategy<ScrollPosition> strategy,
ScrollSubrange defaultSubrange) {
boolean forward = !env.getArguments().containsKey("last");
Integer count = env.getArgument(forward ? "first" : "last");
count = (count != null ? count : defaultSubrange.count().getAsInt());
String cursor = env.getArgument(forward ? "after" : "before");
ScrollPosition position = (cursor != null ? strategy.fromCursor(cursor) : defaultSubrange.position().get());
Assert.notNull(cursorStrategy, "CursorStrategy is required to build a ScrollSubrange");
boolean forward = !environment.getArguments().containsKey("last");
Integer count = environment.getArgument(forward ? "first" : "last");
String cursor = environment.getArgument(forward ? "after" : "before");
ScrollPosition position = (cursor != null ? cursorStrategy.fromCursor(cursor) : null);
return ScrollSubrange.create(position, count, forward);
}

104
spring-graphql/src/test/java/org/springframework/graphql/data/query/RepositoryUtilsTests.java Normal file
View File

@@ -0,0 +1,104 @@
/*
* Copyright 2002-2024 the original author or authors.
*
* Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License.
* You may obtain a copy of the License at
*
* https://www.apache.org/licenses/LICENSE-2.0
*
* Unless required by applicable law or agreed to in writing, software
* distributed under the License is distributed on an "AS IS" BASIS,
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
* See the License for the specific language governing permissions and
* limitations under the License.
*/
package org.springframework.graphql.data.query;
import java.util.Collections;
import java.util.Map;
import graphql.schema.DataFetchingEnvironment;
import graphql.schema.DataFetchingEnvironmentImpl;
import org.junit.jupiter.api.Test;
import org.springframework.data.domain.OffsetScrollPosition;
import org.springframework.data.domain.ScrollPosition;
import org.springframework.graphql.data.pagination.CursorStrategy;
import static org.assertj.core.api.Assertions.assertThat;
/**
* Unit tests for {@link RepositoryUtils}.
*/
@SuppressWarnings("OptionalGetWithoutIsPresent")
public class RepositoryUtilsTests {
private final CursorStrategy<ScrollPosition> cursorStrategy = RepositoryUtils.defaultCursorStrategy();
private final ScrollSubrange defaultSubrange = ScrollSubrange.create(ScrollPosition.offset(50), 20, true);
@Test
void buildScrollSubrangeForward() {
OffsetScrollPosition offset = ScrollPosition.offset(10);
int count = 5;
DataFetchingEnvironment env = environment(
Map.of("first", count, "after", cursorStrategy.toCursor(offset)));
ScrollSubrange range = RepositoryUtils.getScrollSubrange(env, cursorStrategy, defaultSubrange);
assertThat(range.position().get()).isEqualTo(offset);
assertThat(range.count().getAsInt()).isEqualTo(count);
assertThat(range.forward()).isTrue();
}
@Test
void buildScrollSubrangeBackward() {
OffsetScrollPosition offset = ScrollPosition.offset(10);
int count = 5;
DataFetchingEnvironment env = environment(
Map.of("last", count, "before", cursorStrategy.toCursor(offset)));
ScrollSubrange range = RepositoryUtils.getScrollSubrange(env, cursorStrategy, defaultSubrange);
assertThat(range.position().get()).isEqualTo(offset.advanceBy(-count-1));
assertThat(range.count().getAsInt()).isEqualTo(count);
assertThat(range.forward()).isTrue();
}
@Test
void buildScrollSubrangeForwardWithDefaultScrollSubrange() {
DataFetchingEnvironment env = environment(Collections.emptyMap());
ScrollSubrange range = RepositoryUtils.getScrollSubrange(env, cursorStrategy, defaultSubrange);
assertThat(range.position().get()).isEqualTo(getDefaultPosition());
assertThat(range.count().getAsInt()).isEqualTo(this.defaultSubrange.count().getAsInt());
assertThat(range.forward()).isTrue();
}
@Test // gh-900
void buildScrollSubrangeBackwardFromDefaultPosition() {
int count = 5;
DataFetchingEnvironment env = environment(Map.of("last", count));
ScrollSubrange range = RepositoryUtils.getScrollSubrange(env, cursorStrategy, defaultSubrange);
assertThat(range.position().get()).isEqualTo(getDefaultPosition().advanceBy(-count-1));
assertThat(range.count().getAsInt()).isEqualTo(count);
assertThat(range.forward()).isTrue();
}
private static DataFetchingEnvironment environment(Map<String, Object> arguments) {
return DataFetchingEnvironmentImpl.newDataFetchingEnvironment()
.arguments(arguments)
.build();
}
private OffsetScrollPosition getDefaultPosition() {
return (OffsetScrollPosition) defaultSubrange.position().get();
}
}