You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@sling.apache.org by bd...@apache.org on 2021/05/12 10:25:30 UTC
[sling-org-apache-sling-graphql-core] branch SLING-10309/experiment
updated: SLING-10309 - nullable annotations and checks, maximum limit
This is an automated email from the ASF dual-hosted git repository.
bdelacretaz pushed a commit to branch SLING-10309/experiment
in repository https://gitbox.apache.org/repos/asf/sling-org-apache-sling-graphql-core.git
The following commit(s) were added to refs/heads/SLING-10309/experiment by this push:
new 14f155d SLING-10309 - nullable annotations and checks, maximum limit
14f155d is described below
commit 14f155d4cf54361ea8caa884d14fbf2170bbd24b
Author: Bertrand Delacretaz <bd...@apache.org>
AuthorDate: Wed May 12 12:25:12 2021 +0200
SLING-10309 - nullable annotations and checks, maximum limit
---
.../sling/graphql/api/pagination/Cursor.java | 2 +-
.../apache/sling/graphql/api/pagination/Edge.java | 5 +--
.../sling/graphql/api/pagination/PageInfo.java | 5 +--
.../core/helpers/pagination/GenericConnection.java | 24 +++++++++++--
.../core/pagination/GenericConnectionTest.java | 42 +++++++++++++++++++---
.../core/pagination/PaginatedHumansTest.java | 8 ++---
6 files changed, 70 insertions(+), 16 deletions(-)
diff --git a/src/main/java/org/apache/sling/graphql/api/pagination/Cursor.java b/src/main/java/org/apache/sling/graphql/api/pagination/Cursor.java
index ae455a8..7b21839 100644
--- a/src/main/java/org/apache/sling/graphql/api/pagination/Cursor.java
+++ b/src/main/java/org/apache/sling/graphql/api/pagination/Cursor.java
@@ -34,7 +34,7 @@ public class Cursor {
private final String rawValue;
private final String encoded;
- public Cursor(String rawValue) {
+ public Cursor(@Nullable String rawValue) {
this.rawValue = rawValue == null ? "" : rawValue;
this.encoded = encode(this.rawValue);
}
diff --git a/src/main/java/org/apache/sling/graphql/api/pagination/Edge.java b/src/main/java/org/apache/sling/graphql/api/pagination/Edge.java
index 62fbde7..d1e96d7 100644
--- a/src/main/java/org/apache/sling/graphql/api/pagination/Edge.java
+++ b/src/main/java/org/apache/sling/graphql/api/pagination/Edge.java
@@ -19,11 +19,12 @@
package org.apache.sling.graphql.api.pagination;
+import org.jetbrains.annotations.NotNull;
import org.osgi.annotation.versioning.ConsumerType;
/** Edges provide paginated data as per https://relay.dev/graphql/connections.htm */
@ConsumerType
public interface Edge<T> {
- T getNode();
- Cursor getCursor();
+ @NotNull T getNode();
+ @NotNull Cursor getCursor();
}
diff --git a/src/main/java/org/apache/sling/graphql/api/pagination/PageInfo.java b/src/main/java/org/apache/sling/graphql/api/pagination/PageInfo.java
index 06a6d51..3bf9d27 100644
--- a/src/main/java/org/apache/sling/graphql/api/pagination/PageInfo.java
+++ b/src/main/java/org/apache/sling/graphql/api/pagination/PageInfo.java
@@ -19,13 +19,14 @@
package org.apache.sling.graphql.api.pagination;
+import org.jetbrains.annotations.Nullable;
import org.osgi.annotation.versioning.ConsumerType;
/** Information about a results page as per https://relay.dev/graphql/connections.htm */
@ConsumerType
public interface PageInfo {
- Cursor getStartCursor();
- Cursor getEndCursor();
+ @Nullable Cursor getStartCursor();
+ @Nullable Cursor getEndCursor();
boolean isHasPreviousPage();
boolean isHasNextPage();
}
diff --git a/src/main/java/org/apache/sling/graphql/core/helpers/pagination/GenericConnection.java b/src/main/java/org/apache/sling/graphql/core/helpers/pagination/GenericConnection.java
index d380fb2..163079a 100644
--- a/src/main/java/org/apache/sling/graphql/core/helpers/pagination/GenericConnection.java
+++ b/src/main/java/org/apache/sling/graphql/core/helpers/pagination/GenericConnection.java
@@ -28,6 +28,8 @@ import org.apache.sling.graphql.api.pagination.Connection;
import org.apache.sling.graphql.api.pagination.Cursor;
import org.apache.sling.graphql.api.pagination.Edge;
import org.apache.sling.graphql.api.pagination.PageInfo;
+import org.jetbrains.annotations.NotNull;
+import org.jetbrains.annotations.Nullable;
import org.osgi.annotation.versioning.ConsumerType;
/** As per https://relay.dev/graphql/connections.htm a "connection"
@@ -38,6 +40,9 @@ public class GenericConnection<T> implements Connection<T>, PageInfo {
public static final int DEFAULT_LIMIT = 10;
+ /** We might make this configurable but for now let's stay on the safe side */
+ public static final int MAX_LIMIT = 100;
+
private final List<Edge<T>> edges;
private final Iterator<T> dataIterator;
private final Function<T, String> cursorStringProvider;
@@ -57,7 +62,10 @@ public class GenericConnection<T> implements Connection<T>, PageInfo {
* @param startAfter if not null, data up to and including the item which has this cursor is ignored
* @param maxItemsReturned at most this many items are considered
*/
- private GenericConnection(Iterator<T> dataIterator, Function<T, String> cursorStringProvider) {
+ private GenericConnection(@NotNull Iterator<T> dataIterator, @NotNull Function<T, String> cursorStringProvider) {
+ checkNotNull(dataIterator, "Data iterator");
+ checkNotNull(cursorStringProvider, "Cursor string provider function");
+
edges = new ArrayList<>();
this.dataIterator = dataIterator;
this.cursorStringProvider = cursorStringProvider;
@@ -116,6 +124,12 @@ public class GenericConnection<T> implements Connection<T>, PageInfo {
}
}
+ static private void checkNotNull(Object o, String whatIsThat) {
+ if(o == null) {
+ throw new IllegalArgumentException(whatIsThat + " is null");
+ }
+ }
+
private Edge<T> newEdge(final T node, final Function<T, String> cursorStringProvider) {
return new Edge<T>() {
@Override
@@ -168,11 +182,17 @@ public class GenericConnection<T> implements Connection<T>, PageInfo {
}
public Builder<T> withLimit(int limit) {
+ if(limit < 0) {
+ limit = 0;
+ }
+ if(limit > MAX_LIMIT) {
+ throw new IllegalArgumentException("Invalid limit " + limit + ", the maximum value is " + MAX_LIMIT);
+ }
connection.limit = limit;
return this;
}
- public Builder<T> withStartAfter(Cursor c) {
+ public Builder<T> withStartAfter(@Nullable Cursor c) {
connection.startAfter = c;
return this;
}
diff --git a/src/test/java/org/apache/sling/graphql/core/pagination/GenericConnectionTest.java b/src/test/java/org/apache/sling/graphql/core/pagination/GenericConnectionTest.java
index 6fe247c..f7aa8bd 100644
--- a/src/test/java/org/apache/sling/graphql/core/pagination/GenericConnectionTest.java
+++ b/src/test/java/org/apache/sling/graphql/core/pagination/GenericConnectionTest.java
@@ -27,6 +27,7 @@ import java.util.Iterator;
import java.util.List;
import java.util.concurrent.atomic.AtomicInteger;
import java.util.function.Function;
+import java.util.function.Supplier;
import java.util.stream.StreamSupport;
import org.apache.sling.graphql.api.pagination.Connection;
@@ -36,6 +37,7 @@ import org.apache.sling.graphql.core.helpers.pagination.GenericConnection;
import org.junit.Test;
public class GenericConnectionTest {
+ private static final int HIGH_LIMIT = 99;
private static final List<Integer> data = Arrays.asList(1,2,3,4,5 );
private static final Function<Integer, String> cursorize = (i) -> "cursor-" + i;
@@ -85,7 +87,7 @@ public class GenericConnectionTest {
@Test
public void largeLimit() {
final Connection<Integer> c = new GenericConnection.Builder<>(data.iterator(), cursorize)
- .withLimit(999)
+ .withLimit(HIGH_LIMIT)
.build();
assertValues(c, 1, 5, false, false);
}
@@ -93,7 +95,7 @@ public class GenericConnectionTest {
@Test
public void forcePreviousPage() {
final Connection<Integer> c = new GenericConnection.Builder<>(data.iterator(), cursorize)
- .withLimit(999)
+ .withLimit(HIGH_LIMIT)
.withPreviousPage(true)
.build();
assertValues(c, 1, 5, true, false);
@@ -102,7 +104,7 @@ public class GenericConnectionTest {
@Test
public void forceNextPage() {
final Connection<Integer> c = new GenericConnection.Builder<>(data.iterator(), cursorize)
- .withLimit(999)
+ .withLimit(HIGH_LIMIT)
.withNextPage(true)
.build();
assertValues(c, 1, 5, false, true);
@@ -120,7 +122,7 @@ public class GenericConnectionTest {
@Test
public void startAtFourLargeLimit() {
final Connection<Integer> c = new GenericConnection.Builder<>(data.iterator(), cursorize)
- .withLimit(999)
+ .withLimit(HIGH_LIMIT)
.withStartAfter(new Cursor(cursorize.apply(3)))
.build();
assertValues(c, 4, 5, true, false);
@@ -140,11 +142,41 @@ public class GenericConnectionTest {
try {
new GenericConnection.Builder<>(data.iterator(), cursorize)
.withLimit(2)
- .withStartAfter(new Cursor(cursorize.apply(999)))
+ .withStartAfter(new Cursor(cursorize.apply(HIGH_LIMIT)))
.build();
fail("Expecting a RuntimeException");
} catch(RuntimeException rex) {
assertTrue(rex.getMessage().contains("Start cursor not found"));
}
}
+
+ private static void assertSupplierException(Supplier<?> s) {
+ try {
+ s.get();
+ fail("Expected an Exception");
+ } catch(IllegalArgumentException iarg) {
+ // as expcted
+ }
+ }
+
+ @Test
+ public void testNullValues() {
+ assertSupplierException(() -> new GenericConnection.Builder<Integer>(data.iterator(), null));
+ assertSupplierException(() -> new GenericConnection.Builder<Integer>(null, cursorize));
+ }
+
+ @Test
+ public void testMaxLimit() {
+ final GenericConnection.Builder<Integer> b = new GenericConnection.Builder<Integer>(data.iterator(), cursorize);
+ b.withLimit(-100);
+ b.withLimit(0);
+ b.withLimit(42);
+ b.withLimit(100);
+ try {
+ b.withLimit(101);
+ fail("Expecting an exception, over limit");
+ } catch(IllegalArgumentException iex) {
+ assertTrue(iex.getMessage().contains("aximum"));
+ }
+ }
}
\ No newline at end of file
diff --git a/src/test/java/org/apache/sling/graphql/core/pagination/PaginatedHumansTest.java b/src/test/java/org/apache/sling/graphql/core/pagination/PaginatedHumansTest.java
index 0322ce0..ae52313 100644
--- a/src/test/java/org/apache/sling/graphql/core/pagination/PaginatedHumansTest.java
+++ b/src/test/java/org/apache/sling/graphql/core/pagination/PaginatedHumansTest.java
@@ -66,7 +66,7 @@ public class PaginatedHumansTest extends ResourceQueryTestBase {
@Override
protected void setupAdditionalServices() {
final List<HumanDTO> humans = new ArrayList<>();
- for(int i=1 ; i < 100 ; i++) {
+ for(int i=1 ; i < 90 ; i++) {
humans.add(new HumanDTO("human-" + i, "Luke-" + i, "Tatooine"));
}
TestUtil.registerSlingDataFetcher(context.bundleContext(), "humans/connection", new HumansPageFetcher(humans));
@@ -117,13 +117,13 @@ public class PaginatedHumansTest extends ResourceQueryTestBase {
@Test
public void startCursorNearEnd() throws Exception {
- final Cursor start = new Cursor("human-94");
+ final Cursor start = new Cursor("human-84");
final String json = queryJSON("{ paginatedHumans(after:\"" + start + "\", limit:60) {"
+ " pageInfo { startCursor endCursor hasPreviousPage hasNextPage }"
+ " edges { cursor node { id name }}"
+"}}");
- assertEdges(json, 95, 99);
- assertPageInfo(json, new Cursor("human-95"), new Cursor("human-99"), true, false);
+ assertEdges(json, 85, 89);
+ assertPageInfo(json, new Cursor("human-85"), new Cursor("human-89"), true, false);
}
@Test