Skip to content

Commit 2acb557

Browse files
bbakermanKatrina Lee
bbakerman
and
Katrina Lee
authored
Make the graphql-java Asserts truly lazy and hence more efficient (graphql-java#1913)
* Add lazy assertTrue function for performance improvement * This is a revamp of klee97's PR about lazy asserts. It takes it much further and makes them ALL lazy in their message retrieval and encourages efficient asserts by not providing the string.format rope with which to hang yourself. We now require that a supplier be provided to asserts that want messages to be thrown * Fixed up asserts Co-authored-by: Katrina Lee <katlee@katlee-mn1.linkedin.biz>
1 parent 341e22c commit 2acb557

76 files changed

Lines changed: 281 additions & 282 deletions

File tree

Some content is hidden

Large Commits have some content hidden by default. Use the searchbox below for content that may be hidden.

‎src/main/java/graphql/Assert.java‎

Lines changed: 20 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -1,25 +1,26 @@
11
package graphql;
22

33
import java.util.Collection;
4+
import java.util.function.Supplier;
45

56
import static java.lang.String.format;
67

78
@SuppressWarnings("TypeParameterUnusedInFormals")
89
@Internal
910
public class Assert {
1011

11-
public static <T> T assertNotNull(T object, String format, Object... args) {
12+
public static <T> T assertNotNull(T object, Supplier<String> msg) {
1213
if (object != null) {
1314
return object;
1415
}
15-
throw new AssertException(format(format, args));
16+
throw new AssertException(msg.get());
1617
}
1718

18-
public static <T> T assertNotNullWithNPE(T object, String format, Object... args) {
19+
public static <T> T assertNotNullWithNPE(T object, Supplier<String> msg) {
1920
if (object != null) {
2021
return object;
2122
}
22-
throw new NullPointerException(format(format, args));
23+
throw new NullPointerException(msg.get());
2324
}
2425

2526
public static <T> T assertNotNull(T object) {
@@ -29,11 +30,11 @@ public static <T> T assertNotNull(T object) {
2930
throw new AssertException("Object required to be not null");
3031
}
3132

32-
public static <T> void assertNull(T object, String format, Object... args) {
33+
public static <T> void assertNull(T object, Supplier<String> msg) {
3334
if (object == null) {
3435
return;
3536
}
36-
throw new AssertException(format(format, args));
37+
throw new AssertException(msg.get());
3738
}
3839

3940
public static <T> void assertNull(T object) {
@@ -62,18 +63,18 @@ public static <T> Collection<T> assertNotEmpty(Collection<T> collection) {
6263
return collection;
6364
}
6465

65-
public static <T> Collection<T> assertNotEmpty(Collection<T> collection, String format, Object... args) {
66+
public static <T> Collection<T> assertNotEmpty(Collection<T> collection, Supplier<String> msg) {
6667
if (collection == null || collection.isEmpty()) {
67-
throw new AssertException(format(format, args));
68+
throw new AssertException(msg.get());
6869
}
6970
return collection;
7071
}
7172

72-
public static void assertTrue(boolean condition, String format, Object... args) {
73+
public static void assertTrue(boolean condition, Supplier<String> msg) {
7374
if (condition) {
7475
return;
7576
}
76-
throw new AssertException(format(format, args));
77+
throw new AssertException(msg.get());
7778
}
7879

7980
public static void assertTrue(boolean condition) {
@@ -83,11 +84,18 @@ public static void assertTrue(boolean condition) {
8384
throw new AssertException("condition expected to be true");
8485
}
8586

86-
public static void assertFalse(boolean condition, String format, Object... args) {
87+
public static void assertFalse(boolean condition, Supplier<String> msg) {
8788
if (!condition) {
8889
return;
8990
}
90-
throw new AssertException(format(format, args));
91+
throw new AssertException(msg.get());
92+
}
93+
94+
public static void assertFalse(boolean condition) {
95+
if (!condition) {
96+
return;
97+
}
98+
throw new AssertException("condition expected to be false");
9199
}
92100

93101
private static final String invalidNameErrorMessage = "Name must be non-null, non-empty and match [_A-Za-z][_0-9A-Za-z]* - was '%s'";
@@ -97,7 +105,6 @@ public static void assertFalse(boolean condition, String format, Object... args)
97105
* currently non null, non empty,
98106
*
99107
* @param name - the name to be validated.
100-
*
101108
* @return the name if valid, or AssertException if invalid.
102109
*/
103110
public static String assertValidName(String name) {

‎src/main/java/graphql/ExecutionInput.java‎

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -32,7 +32,7 @@ public class ExecutionInput {
3232

3333
@Internal
3434
private ExecutionInput(String query, String operationName, Object context, Object root, Map<String, Object> variables, DataLoaderRegistry dataLoaderRegistry, CacheControl cacheControl, ExecutionId executionId, Locale locale, Object localContext) {
35-
this.query = assertNotNull(query, "query can't be null");
35+
this.query = assertNotNull(query, () -> "query can't be null");
3636
this.operationName = operationName;
3737
this.context = context;
3838
this.root = root;
@@ -189,7 +189,7 @@ public static class Builder {
189189
private ExecutionId executionId;
190190

191191
public Builder query(String query) {
192-
this.query = assertNotNull(query, "query can't be null");
192+
this.query = assertNotNull(query, () -> "query can't be null");
193193
return this;
194194
}
195195

@@ -259,7 +259,7 @@ public Builder root(Object root) {
259259
}
260260

261261
public Builder variables(Map<String, Object> variables) {
262-
this.variables = assertNotNull(variables, "variables map can't be null");
262+
this.variables = assertNotNull(variables, () -> "variables map can't be null");
263263
return this;
264264
}
265265

‎src/main/java/graphql/GraphQL.java‎

Lines changed: 13 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -176,13 +176,13 @@ private GraphQL(GraphQLSchema graphQLSchema,
176176
Instrumentation instrumentation,
177177
PreparsedDocumentProvider preparsedDocumentProvider,
178178
ValueUnboxer valueUnboxer) {
179-
this.graphQLSchema = assertNotNull(graphQLSchema, "graphQLSchema must be non null");
179+
this.graphQLSchema = assertNotNull(graphQLSchema, () -> "graphQLSchema must be non null");
180180
this.queryStrategy = queryStrategy != null ? queryStrategy : new AsyncExecutionStrategy();
181181
this.mutationStrategy = mutationStrategy != null ? mutationStrategy : new AsyncSerialExecutionStrategy();
182182
this.subscriptionStrategy = subscriptionStrategy != null ? subscriptionStrategy : new SubscriptionExecutionStrategy();
183-
this.idProvider = assertNotNull(idProvider, "idProvider must be non null");
183+
this.idProvider = assertNotNull(idProvider, () -> "idProvider must be non null");
184184
this.instrumentation = assertNotNull(instrumentation);
185-
this.preparsedDocumentProvider = assertNotNull(preparsedDocumentProvider, "preparsedDocumentProvider must be non null");
185+
this.preparsedDocumentProvider = assertNotNull(preparsedDocumentProvider, () -> "preparsedDocumentProvider must be non null");
186186
this.valueUnboxer = valueUnboxer;
187187
}
188188

@@ -242,37 +242,37 @@ public Builder(GraphQLSchema graphQLSchema) {
242242
}
243243

244244
public Builder schema(GraphQLSchema graphQLSchema) {
245-
this.graphQLSchema = assertNotNull(graphQLSchema, "GraphQLSchema must be non null");
245+
this.graphQLSchema = assertNotNull(graphQLSchema, () -> "GraphQLSchema must be non null");
246246
return this;
247247
}
248248

249249
public Builder queryExecutionStrategy(ExecutionStrategy executionStrategy) {
250-
this.queryExecutionStrategy = assertNotNull(executionStrategy, "Query ExecutionStrategy must be non null");
250+
this.queryExecutionStrategy = assertNotNull(executionStrategy, () -> "Query ExecutionStrategy must be non null");
251251
return this;
252252
}
253253

254254
public Builder mutationExecutionStrategy(ExecutionStrategy executionStrategy) {
255-
this.mutationExecutionStrategy = assertNotNull(executionStrategy, "Mutation ExecutionStrategy must be non null");
255+
this.mutationExecutionStrategy = assertNotNull(executionStrategy, () -> "Mutation ExecutionStrategy must be non null");
256256
return this;
257257
}
258258

259259
public Builder subscriptionExecutionStrategy(ExecutionStrategy executionStrategy) {
260-
this.subscriptionExecutionStrategy = assertNotNull(executionStrategy, "Subscription ExecutionStrategy must be non null");
260+
this.subscriptionExecutionStrategy = assertNotNull(executionStrategy, () -> "Subscription ExecutionStrategy must be non null");
261261
return this;
262262
}
263263

264264
public Builder instrumentation(Instrumentation instrumentation) {
265-
this.instrumentation = assertNotNull(instrumentation, "Instrumentation must be non null");
265+
this.instrumentation = assertNotNull(instrumentation, () -> "Instrumentation must be non null");
266266
return this;
267267
}
268268

269269
public Builder preparsedDocumentProvider(PreparsedDocumentProvider preparsedDocumentProvider) {
270-
this.preparsedDocumentProvider = assertNotNull(preparsedDocumentProvider, "PreparsedDocumentProvider must be non null");
270+
this.preparsedDocumentProvider = assertNotNull(preparsedDocumentProvider, () -> "PreparsedDocumentProvider must be non null");
271271
return this;
272272
}
273273

274274
public Builder executionIdProvider(ExecutionIdProvider executionIdProvider) {
275-
this.idProvider = assertNotNull(executionIdProvider, "ExecutionIdProvider must be non null");
275+
this.idProvider = assertNotNull(executionIdProvider, () -> "ExecutionIdProvider must be non null");
276276
return this;
277277
}
278278

@@ -299,9 +299,9 @@ public Builder valueUnboxer(ValueUnboxer valueUnboxer) {
299299
}
300300

301301
public GraphQL build() {
302-
assertNotNull(graphQLSchema, "graphQLSchema must be non null");
303-
assertNotNull(queryExecutionStrategy, "queryStrategy must be non null");
304-
assertNotNull(idProvider, "idProvider must be non null");
302+
assertNotNull(graphQLSchema, () -> "graphQLSchema must be non null");
303+
assertNotNull(queryExecutionStrategy, () -> "queryStrategy must be non null");
304+
assertNotNull(idProvider, () -> "idProvider must be non null");
305305
final Instrumentation augmentedInstrumentation = checkInstrumentationDefaultState(instrumentation, doNotAddDefaultInstrumentations);
306306
return new GraphQL(graphQLSchema, queryExecutionStrategy, mutationExecutionStrategy, subscriptionExecutionStrategy, idProvider, augmentedInstrumentation, preparsedDocumentProvider, valueUnboxer);
307307
}

‎src/main/java/graphql/GraphqlErrorBuilder.java‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -92,7 +92,7 @@ public GraphqlErrorBuilder extensions(Map<String, Object> extensions) {
9292
* @return a newly built GraphqlError
9393
*/
9494
public GraphQLError build() {
95-
assertNotNull(message, "You must provide error message");
95+
assertNotNull(message, () -> "You must provide error message");
9696
return new GraphqlErrorImpl(message, locations, errorType, path, extensions);
9797
}
9898

‎src/main/java/graphql/analysis/MaxQueryComplexityInstrumentation.java‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -72,7 +72,7 @@ public MaxQueryComplexityInstrumentation(int maxComplexity, FieldComplexityCalcu
7272
public MaxQueryComplexityInstrumentation(int maxComplexity, FieldComplexityCalculator fieldComplexityCalculator,
7373
Function<QueryComplexityInfo, Boolean> maxQueryComplexityExceededFunction) {
7474
this.maxComplexity = maxComplexity;
75-
this.fieldComplexityCalculator = assertNotNull(fieldComplexityCalculator, "calculator can't be null");
75+
this.fieldComplexityCalculator = assertNotNull(fieldComplexityCalculator, () -> "calculator can't be null");
7676
this.maxQueryComplexityExceededFunction = maxQueryComplexityExceededFunction;
7777
}
7878

‎src/main/java/graphql/analysis/NodeVisitorWithTypeTracking.java‎

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -33,6 +33,7 @@
3333
import static graphql.Assert.assertNotNull;
3434
import static graphql.schema.GraphQLTypeUtil.unwrapAll;
3535
import static graphql.util.TraverserContext.Phase.LEAVE;
36+
import static java.lang.String.format;
3637

3738
/**
3839
* Internally used node visitor which delegates to a {@link QueryVisitor} with type
@@ -138,8 +139,9 @@ public TraversalControl visitFragmentSpread(FragmentSpread fragmentSpread, Trave
138139
QueryTraversalContext parentEnv = context.getVarFromParents(QueryTraversalContext.class);
139140

140141
GraphQLCompositeType typeCondition = (GraphQLCompositeType) schema.getType(fragmentDefinition.getTypeCondition().getName());
141-
assertNotNull(typeCondition, "Invalid type condition '%s' in fragment '%s'", fragmentDefinition.getTypeCondition().getName(),
142-
fragmentDefinition.getName());
142+
assertNotNull(typeCondition,
143+
() -> format("Invalid type condition '%s' in fragment '%s'", fragmentDefinition.getTypeCondition().getName(),
144+
fragmentDefinition.getName()));
143145
context.setVar(QueryTraversalContext.class, new QueryTraversalContext(typeCondition, parentEnv.getEnvironment(), fragmentDefinition));
144146
return TraversalControl.CONTINUE;
145147
}

‎src/main/java/graphql/analysis/QueryTransformer.java‎

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -44,11 +44,11 @@ private QueryTransformer(GraphQLSchema schema,
4444
GraphQLCompositeType rootParentType,
4545
Map<String, FragmentDefinition> fragmentsByName,
4646
Map<String, Object> variables) {
47-
this.schema = assertNotNull(schema, "schema can't be null");
48-
this.variables = assertNotNull(variables, "variables can't be null");
49-
this.root = assertNotNull(root, "root can't be null");
47+
this.schema = assertNotNull(schema, () -> "schema can't be null");
48+
this.variables = assertNotNull(variables, () -> "variables can't be null");
49+
this.root = assertNotNull(root, () -> "root can't be null");
5050
this.rootParentType = assertNotNull(rootParentType);
51-
this.fragmentsByName = assertNotNull(fragmentsByName, "fragmentsByName can't be null");
51+
this.fragmentsByName = assertNotNull(fragmentsByName, () -> "fragmentsByName can't be null");
5252
}
5353

5454
/**

‎src/main/java/graphql/analysis/QueryTraverser.java‎

Lines changed: 8 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -48,10 +48,10 @@ private QueryTraverser(GraphQLSchema schema,
4848
Document document,
4949
String operation,
5050
Map<String, Object> variables) {
51-
assertNotNull(document, "document can't be null");
51+
assertNotNull(document, () -> "document can't be null");
5252
NodeUtil.GetOperationResult getOperationResult = NodeUtil.getOperation(document, operation);
53-
this.schema = assertNotNull(schema, "schema can't be null");
54-
this.variables = assertNotNull(variables, "variables can't be null");
53+
this.schema = assertNotNull(schema, () -> "schema can't be null");
54+
this.variables = assertNotNull(variables, () -> "variables can't be null");
5555
this.fragmentsByName = getOperationResult.fragmentsByName;
5656
this.roots = singletonList(getOperationResult.operationDefinition);
5757
this.rootParentType = getRootTypeFromOperation(getOperationResult.operationDefinition);
@@ -62,12 +62,12 @@ private QueryTraverser(GraphQLSchema schema,
6262
GraphQLCompositeType rootParentType,
6363
Map<String, FragmentDefinition> fragmentsByName,
6464
Map<String, Object> variables) {
65-
this.schema = assertNotNull(schema, "schema can't be null");
66-
this.variables = assertNotNull(variables, "variables can't be null");
67-
assertNotNull(root, "root can't be null");
65+
this.schema = assertNotNull(schema, () -> "schema can't be null");
66+
this.variables = assertNotNull(variables, () -> "variables can't be null");
67+
assertNotNull(root, () -> "root can't be null");
6868
this.roots = Collections.singleton(root);
69-
this.rootParentType = assertNotNull(rootParentType, "rootParentType can't be null");
70-
this.fragmentsByName = assertNotNull(fragmentsByName, "fragmentsByName can't be null");
69+
this.rootParentType = assertNotNull(rootParentType, () -> "rootParentType can't be null");
70+
this.fragmentsByName = assertNotNull(fragmentsByName, () -> "fragmentsByName can't be null");
7171
}
7272

7373
public Object visitDepthFirst(QueryVisitor queryVisitor) {

‎src/main/java/graphql/execution/Async.java‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -49,7 +49,7 @@ public static <T, U> CompletableFuture<List<U>> each(Iterable<T> list, BiFunctio
4949
CompletableFuture<U> cf;
5050
try {
5151
cf = cfFactory.apply(t, index++);
52-
Assert.assertNotNull(cf, "cfFactory must return a non null value");
52+
Assert.assertNotNull(cf, () -> "cfFactory must return a non null value");
5353
} catch (Exception e) {
5454
cf = new CompletableFuture<>();
5555
// Async.each makes sure that it is not a CompletionException inside a CompletionException
@@ -75,7 +75,7 @@ private static <T, U> void eachSequentiallyImpl(Iterator<T> iterator, CFFactory<
7575
CompletableFuture<U> cf;
7676
try {
7777
cf = cfFactory.apply(iterator.next(), index, tmpResult);
78-
Assert.assertNotNull(cf, "cfFactory must return a non null value");
78+
Assert.assertNotNull(cf, () -> "cfFactory must return a non null value");
7979
} catch (Exception e) {
8080
cf = new CompletableFuture<>();
8181
cf.completeExceptionally(new CompletionException(e));

‎src/main/java/graphql/execution/ExecutionContextBuilder.java‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -181,7 +181,7 @@ public ExecutionContextBuilder valueUnboxer(ValueUnboxer valueUnboxer) {
181181

182182
public ExecutionContext build() {
183183
// preconditions
184-
assertNotNull(executionId, "You must provide a query identifier");
184+
assertNotNull(executionId, () -> "You must provide a query identifier");
185185

186186
return new ExecutionContext(
187187
instrumentation,

0 commit comments

Comments
 (0)