From 7d29e37f28c296ebf178ad035c54449bb77f62fb Mon Sep 17 00:00:00 2001 From: Brad Baker Date: Wed, 6 Mar 2024 21:14:31 +1100 Subject: [PATCH 1/6] Cheaper assertions with constant strings --- src/main/java/graphql/Assert.java | 42 ++++++++++++ .../java/graphql/GraphqlErrorBuilder.java | 3 +- src/main/java/graphql/execution/Async.java | 8 +-- .../graphql/execution/ExecutionStepInfo.java | 5 +- .../ExecutionStrategyParameters.java | 6 +- .../execution/FieldCollectorParameters.java | 2 +- .../graphql/execution/FieldValueInfo.java | 3 +- .../java/graphql/execution/ResultPath.java | 15 +++-- .../SubscriptionExecutionStrategy.java | 3 +- .../execution/ValuesResolverConversion.java | 4 +- .../reactive/NonBlockingMutexExecutor.java | 3 +- .../graphql/extensions/ExtensionsBuilder.java | 4 +- .../java/graphql/language/AbstractNode.java | 6 +- .../java/graphql/language/AstPrinter.java | 3 +- .../java/graphql/language/NodeParentTree.java | 5 +- .../graphql/language/PrettyAstPrinter.java | 3 +- .../normalized/ExecutableNormalizedField.java | 4 +- .../normalized/NormalizedInputValue.java | 4 +- .../java/graphql/schema/AsyncDataFetcher.java | 5 +- .../DefaultGraphqlTypeComparatorRegistry.java | 9 ++- .../java/graphql/schema/GraphQLTypeUtil.java | 2 +- .../graphql/schema/InputValueWithState.java | 3 +- src/test/groovy/graphql/AssertTest.groovy | 64 +++++++++++++++++-- 23 files changed, 159 insertions(+), 47 deletions(-) diff --git a/src/main/java/graphql/Assert.java b/src/main/java/graphql/Assert.java index 5fcbf9dbe9..d780e419f3 100644 --- a/src/main/java/graphql/Assert.java +++ b/src/main/java/graphql/Assert.java @@ -31,6 +31,22 @@ public static T assertNotNull(T object) { throw new AssertException("Object required to be not null"); } + /** + * If you use this method, make sure that the message is a constant string + * + * @param object the object to check for null + * @param constantMsg make sure this is a constant message + * @param for two + * + * @return the object + */ + public static T assertNotNull(T object, String constantMsg) { + if (object != null) { + return object; + } + throw new AssertException(constantMsg); + } + public static void assertNull(T object, Supplier msg) { if (object == null) { return; @@ -85,6 +101,19 @@ public static void assertTrue(boolean condition) { throw new AssertException("condition expected to be true"); } + /** + * If you use this method, make sure that the message is a constant string + * + * @param condition the condition that must be true + * @param constantMsg make sure this is a constant message + */ + public static void assertTrue(boolean condition, String constantMsg) { + if (condition) { + return; + } + throw new AssertException(constantMsg); + } + public static void assertFalse(boolean condition, Supplier msg) { if (!condition) { return; @@ -99,6 +128,19 @@ public static void assertFalse(boolean condition) { throw new AssertException("condition expected to be false"); } + /** + * If you use this method, make sure that the message is a constant string + * + * @param condition the condition that must be false + * @param constantMsg make sure this is a constant message + */ + public static void assertFalse(boolean condition, String constantMsg) { + if (!condition) { + return; + } + throw new AssertException(constantMsg); + } + private static final String invalidNameErrorMessage = "Name must be non-null, non-empty and match [_A-Za-z][_0-9A-Za-z]* - was '%s'"; private static final Pattern validNamePattern = Pattern.compile("[_A-Za-z][_0-9A-Za-z]*"); diff --git a/src/main/java/graphql/GraphqlErrorBuilder.java b/src/main/java/graphql/GraphqlErrorBuilder.java index 4cef5beabe..c7281a5f53 100644 --- a/src/main/java/graphql/GraphqlErrorBuilder.java +++ b/src/main/java/graphql/GraphqlErrorBuilder.java @@ -10,6 +10,7 @@ import java.util.List; import java.util.Map; +import static graphql.Assert.assertNotNull; import static graphql.Assert.assertNotNull; /** @@ -128,7 +129,7 @@ public B extensions(@Nullable Map extensions) { * @return a newly built GraphqlError */ public GraphQLError build() { - assertNotNull(message, () -> "You must provide error message"); + Assert.assertNotNull(message,"You must provide error message"); return new GraphqlErrorImpl(message, locations, errorType, path, extensions); } diff --git a/src/main/java/graphql/execution/Async.java b/src/main/java/graphql/execution/Async.java index 56f7a2f9be..cbdd45d96b 100644 --- a/src/main/java/graphql/execution/Async.java +++ b/src/main/java/graphql/execution/Async.java @@ -58,7 +58,7 @@ public void add(CompletableFuture completableFuture) { @Override public CompletableFuture> await() { - Assert.assertTrue(ix == 0, () -> "expected size was " + 0 + " got " + ix); + Assert.assertTrue(ix == 0, "expected size was 0"); return typedEmpty(); } @@ -109,7 +109,7 @@ public void add(CompletableFuture completableFuture) { @Override public CompletableFuture> await() { - Assert.assertTrue(ix == array.length, () -> "expected size was " + array.length + " got " + ix); + Assert.assertTrue(ix == array.length, "expected size was less than the array length"); CompletableFuture> overallResult = new CompletableFuture<>(); CompletableFuture.allOf(array) @@ -135,7 +135,7 @@ public static CompletableFuture> each(Collection list, Functio CompletableFuture cf; try { cf = cfFactory.apply(t); - Assert.assertNotNull(cf, () -> "cfFactory must return a non null value"); + Assert.assertNotNull(cf, "cfFactory must return a non null value"); } catch (Exception e) { cf = new CompletableFuture<>(); // Async.each makes sure that it is not a CompletionException inside a CompletionException @@ -160,7 +160,7 @@ private static void eachSequentiallyImpl(Iterator iterator, BiFunction CompletableFuture cf; try { cf = cfFactory.apply(iterator.next(), tmpResult); - Assert.assertNotNull(cf, () -> "cfFactory must return a non null value"); + Assert.assertNotNull(cf,"cfFactory must return a non null value"); } catch (Exception e) { cf = new CompletableFuture<>(); cf.completeExceptionally(new CompletionException(e)); diff --git a/src/main/java/graphql/execution/ExecutionStepInfo.java b/src/main/java/graphql/execution/ExecutionStepInfo.java index 08836d977f..42e0b01ebb 100644 --- a/src/main/java/graphql/execution/ExecutionStepInfo.java +++ b/src/main/java/graphql/execution/ExecutionStepInfo.java @@ -1,5 +1,6 @@ package graphql.execution; +import graphql.Assert; import graphql.PublicApi; import graphql.collect.ImmutableMapWithNullValues; import graphql.schema.GraphQLFieldDefinition; @@ -72,7 +73,7 @@ private ExecutionStepInfo(Builder builder) { this.field = builder.field; this.path = builder.path; this.parent = builder.parentInfo; - this.type = assertNotNull(builder.type, () -> "you must provide a graphql type"); + this.type = Assert.assertNotNull(builder.type, "you must provide a graphql type"); this.arguments = builder.arguments; this.fieldContainer = builder.fieldContainer; } @@ -202,7 +203,7 @@ public boolean hasParent() { * @return a new type info with the same */ public ExecutionStepInfo changeTypeWithPreservedNonNull(GraphQLOutputType newType) { - assertTrue(!GraphQLTypeUtil.isNonNull(newType), () -> "newType can't be non null"); + Assert.assertTrue(!GraphQLTypeUtil.isNonNull(newType), "newType can't be non null"); if (isNonNullType()) { return newExecutionStepInfo(this).type(GraphQLNonNull.nonNull(newType)).build(); } else { diff --git a/src/main/java/graphql/execution/ExecutionStrategyParameters.java b/src/main/java/graphql/execution/ExecutionStrategyParameters.java index b40fa781c5..4df2b73605 100644 --- a/src/main/java/graphql/execution/ExecutionStrategyParameters.java +++ b/src/main/java/graphql/execution/ExecutionStrategyParameters.java @@ -33,9 +33,9 @@ private ExecutionStrategyParameters(ExecutionStepInfo executionStepInfo, ExecutionStrategyParameters parent, DeferredCallContext deferredCallContext) { - this.executionStepInfo = assertNotNull(executionStepInfo, () -> "executionStepInfo is null"); + this.executionStepInfo = Assert.assertNotNull(executionStepInfo, "executionStepInfo is null"); this.localContext = localContext; - this.fields = assertNotNull(fields, () -> "fields is null"); + this.fields = Assert.assertNotNull(fields, "fields is null"); this.source = source; this.nonNullableFieldValidator = nonNullableFieldValidator; this.path = path; @@ -168,7 +168,7 @@ public Builder localContext(Object localContext) { } public Builder nonNullFieldValidator(NonNullableFieldValidator nonNullableFieldValidator) { - this.nonNullableFieldValidator = Assert.assertNotNull(nonNullableFieldValidator, () -> "requires a NonNullValidator"); + this.nonNullableFieldValidator = Assert.assertNotNull(nonNullableFieldValidator, "requires a NonNullValidator"); return this; } diff --git a/src/main/java/graphql/execution/FieldCollectorParameters.java b/src/main/java/graphql/execution/FieldCollectorParameters.java index c0a23404a7..75dafc1eff 100644 --- a/src/main/java/graphql/execution/FieldCollectorParameters.java +++ b/src/main/java/graphql/execution/FieldCollectorParameters.java @@ -92,7 +92,7 @@ public Builder variables(Map variables) { } public FieldCollectorParameters build() { - Assert.assertNotNull(graphQLSchema, () -> "You must provide a schema"); + Assert.assertNotNull(graphQLSchema, "You must provide a schema"); return new FieldCollectorParameters(this); } diff --git a/src/main/java/graphql/execution/FieldValueInfo.java b/src/main/java/graphql/execution/FieldValueInfo.java index c13e915936..a170ecb6e3 100644 --- a/src/main/java/graphql/execution/FieldValueInfo.java +++ b/src/main/java/graphql/execution/FieldValueInfo.java @@ -1,6 +1,7 @@ package graphql.execution; import com.google.common.collect.ImmutableList; +import graphql.Assert; import graphql.ExecutionResult; import graphql.ExecutionResultImpl; import graphql.PublicApi; @@ -31,7 +32,7 @@ public enum CompleteValueType { } FieldValueInfo(CompleteValueType completeValueType, CompletableFuture fieldValue, List fieldValueInfos) { - assertNotNull(fieldValueInfos, () -> "fieldValueInfos can't be null"); + Assert.assertNotNull(fieldValueInfos, "fieldValueInfos can't be null"); this.completeValueType = completeValueType; this.fieldValue = fieldValue; this.fieldValueInfos = fieldValueInfos; diff --git a/src/main/java/graphql/execution/ResultPath.java b/src/main/java/graphql/execution/ResultPath.java index 7f65379ade..9544ad57c7 100644 --- a/src/main/java/graphql/execution/ResultPath.java +++ b/src/main/java/graphql/execution/ResultPath.java @@ -12,6 +12,7 @@ import java.util.Objects; import java.util.StringTokenizer; +import static graphql.Assert.assertNotNull; import static graphql.Assert.assertNotNull; import static graphql.Assert.assertTrue; import static java.lang.String.format; @@ -46,12 +47,12 @@ private ResultPath() { } private ResultPath(ResultPath parent, String segment) { - this.parent = assertNotNull(parent, () -> "Must provide a parent path"); - this.segment = assertNotNull(segment, () -> "Must provide a sub path"); + this.parent = Assert.assertNotNull(parent, "Must provide a parent path"); + this.segment = Assert.assertNotNull(segment, "Must provide a sub path"); } private ResultPath(ResultPath parent, int segment) { - this.parent = assertNotNull(parent, () -> "Must provide a parent path"); + this.parent = Assert.assertNotNull(parent, "Must provide a parent path"); this.segment = segment; } @@ -199,7 +200,7 @@ public ResultPath dropSegment() { * @return a new path with the last segment replaced */ public ResultPath replaceSegment(int segment) { - Assert.assertTrue(!ROOT_PATH.equals(this), () -> "You MUST not call this with the root path"); + Assert.assertTrue(!ROOT_PATH.equals(this), "You MUST not call this with the root path"); return new ResultPath(parent, segment); } @@ -211,7 +212,7 @@ public ResultPath replaceSegment(int segment) { * @return a new path with the last segment replaced */ public ResultPath replaceSegment(String segment) { - Assert.assertTrue(!ROOT_PATH.equals(this), () -> "You MUST not call this with the root path"); + Assert.assertTrue(!ROOT_PATH.equals(this), "You MUST not call this with the root path"); return new ResultPath(parent, segment); } @@ -237,12 +238,12 @@ public ResultPath append(ResultPath path) { public ResultPath sibling(String siblingField) { - Assert.assertTrue(!ROOT_PATH.equals(this), () -> "You MUST not call this with the root path"); + Assert.assertTrue(!ROOT_PATH.equals(this), "You MUST not call this with the root path"); return new ResultPath(this.parent, siblingField); } public ResultPath sibling(int siblingField) { - Assert.assertTrue(!ROOT_PATH.equals(this), () -> "You MUST not call this with the root path"); + Assert.assertTrue(!ROOT_PATH.equals(this), "You MUST not call this with the root path"); return new ResultPath(this.parent, siblingField); } diff --git a/src/main/java/graphql/execution/SubscriptionExecutionStrategy.java b/src/main/java/graphql/execution/SubscriptionExecutionStrategy.java index f735b0dc6a..16a91a942b 100644 --- a/src/main/java/graphql/execution/SubscriptionExecutionStrategy.java +++ b/src/main/java/graphql/execution/SubscriptionExecutionStrategy.java @@ -1,5 +1,6 @@ package graphql.execution; +import graphql.Assert; import graphql.ExecutionResult; import graphql.ExecutionResultImpl; import graphql.PublicApi; @@ -97,7 +98,7 @@ private CompletableFuture> createSourceEventStream(ExecutionCo return fieldFetched.thenApply(fetchedValue -> { Object publisher = fetchedValue.getFetchedValue(); if (publisher != null) { - assertTrue(publisher instanceof Publisher, () -> "Your data fetcher must return a Publisher of events when using graphql subscriptions"); + Assert.assertTrue(publisher instanceof Publisher, "Your data fetcher must return a Publisher of events when using graphql subscriptions"); } //noinspection unchecked return (Publisher) publisher; diff --git a/src/main/java/graphql/execution/ValuesResolverConversion.java b/src/main/java/graphql/execution/ValuesResolverConversion.java index 29c3edaf2d..a80a0a7358 100644 --- a/src/main/java/graphql/execution/ValuesResolverConversion.java +++ b/src/main/java/graphql/execution/ValuesResolverConversion.java @@ -1,6 +1,7 @@ package graphql.execution; import com.google.common.collect.ImmutableList; +import graphql.Assert; import graphql.GraphQLContext; import graphql.Internal; import graphql.execution.values.InputInterceptor; @@ -38,6 +39,7 @@ import static graphql.Assert.assertShouldNeverHappen; import static graphql.Assert.assertTrue; +import static graphql.Assert.assertTrue; import static graphql.collect.ImmutableKit.emptyList; import static graphql.collect.ImmutableKit.map; import static graphql.execution.ValuesResolver.ValueMode.NORMALIZED; @@ -281,7 +283,7 @@ private static Object externalValueToLiteralForObject( GraphQLContext graphqlContext, Locale locale ) { - assertTrue(inputValue instanceof Map, () -> "Expect Map as input"); + Assert.assertTrue(inputValue instanceof Map, "Expect Map as input"); Map inputMap = (Map) inputValue; List fieldDefinitions = fieldVisibility.getFieldDefinitions(inputObjectType); diff --git a/src/main/java/graphql/execution/reactive/NonBlockingMutexExecutor.java b/src/main/java/graphql/execution/reactive/NonBlockingMutexExecutor.java index 49f6fde6ee..449e65f5d3 100644 --- a/src/main/java/graphql/execution/reactive/NonBlockingMutexExecutor.java +++ b/src/main/java/graphql/execution/reactive/NonBlockingMutexExecutor.java @@ -1,6 +1,7 @@ package graphql.execution.reactive; +import graphql.Assert; import graphql.Internal; import java.util.concurrent.Executor; @@ -37,7 +38,7 @@ class NonBlockingMutexExecutor implements Executor { @Override public void execute(final Runnable command) { - final RunNode newNode = new RunNode(assertNotNull(command, () -> "Runnable must not be null")); + final RunNode newNode = new RunNode(Assert.assertNotNull(command, "Runnable must not be null")); final RunNode prevLast = last.getAndSet(newNode); if (prevLast != null) prevLast.lazySet(newNode); diff --git a/src/main/java/graphql/extensions/ExtensionsBuilder.java b/src/main/java/graphql/extensions/ExtensionsBuilder.java index fe37c64743..8f731cf33b 100644 --- a/src/main/java/graphql/extensions/ExtensionsBuilder.java +++ b/src/main/java/graphql/extensions/ExtensionsBuilder.java @@ -1,6 +1,7 @@ package graphql.extensions; import com.google.common.collect.ImmutableMap; +import graphql.Assert; import graphql.ExecutionResult; import graphql.PublicApi; import org.jetbrains.annotations.NotNull; @@ -12,6 +13,7 @@ import java.util.Map; import java.util.concurrent.CopyOnWriteArrayList; +import static graphql.Assert.assertNotNull; import static graphql.Assert.assertNotNull; /** @@ -106,7 +108,7 @@ public Map buildExtensions() { Map outMap = new LinkedHashMap<>(firstChange); for (int i = 1; i < changes.size(); i++) { Map newMap = extensionsMerger.merge(outMap, changes.get(i)); - assertNotNull(outMap, () -> "You MUST provide a non null Map from ExtensionsMerger.merge()"); + Assert.assertNotNull(outMap, "You MUST provide a non null Map from ExtensionsMerger.merge()"); outMap = newMap; } return outMap; diff --git a/src/main/java/graphql/language/AbstractNode.java b/src/main/java/graphql/language/AbstractNode.java index f097c9b491..736d23a98f 100644 --- a/src/main/java/graphql/language/AbstractNode.java +++ b/src/main/java/graphql/language/AbstractNode.java @@ -25,9 +25,9 @@ public AbstractNode(SourceLocation sourceLocation, List comments, Ignor } public AbstractNode(SourceLocation sourceLocation, List comments, IgnoredChars ignoredChars, Map additionalData) { - Assert.assertNotNull(comments, () -> "comments can't be null"); - Assert.assertNotNull(ignoredChars, () -> "ignoredChars can't be null"); - Assert.assertNotNull(additionalData, () -> "additionalData can't be null"); + Assert.assertNotNull(comments, "comments can't be null"); + Assert.assertNotNull(ignoredChars, "ignoredChars can't be null"); + Assert.assertNotNull(additionalData, "additionalData can't be null"); this.sourceLocation = sourceLocation; this.additionalData = ImmutableMap.copyOf(additionalData); diff --git a/src/main/java/graphql/language/AstPrinter.java b/src/main/java/graphql/language/AstPrinter.java index 3deff35b25..df2c1f71ea 100644 --- a/src/main/java/graphql/language/AstPrinter.java +++ b/src/main/java/graphql/language/AstPrinter.java @@ -1,5 +1,6 @@ package graphql.language; +import graphql.Assert; import graphql.AssertException; import graphql.PublicApi; import graphql.collect.ImmutableKit; @@ -455,7 +456,7 @@ private String node(Node node) { private String node(Node node, Class startClass) { if (startClass != null) { - assertTrue(startClass.isInstance(node), () -> "The starting class must be in the inherit tree"); + Assert.assertTrue(startClass.isInstance(node), "The starting class must be in the inherit tree"); } StringBuilder builder = new StringBuilder(); NodePrinter printer = _findPrinter(node, startClass); diff --git a/src/main/java/graphql/language/NodeParentTree.java b/src/main/java/graphql/language/NodeParentTree.java index fc78ea093d..616efadacc 100644 --- a/src/main/java/graphql/language/NodeParentTree.java +++ b/src/main/java/graphql/language/NodeParentTree.java @@ -1,6 +1,7 @@ package graphql.language; import com.google.common.collect.ImmutableList; +import graphql.Assert; import graphql.Internal; import graphql.PublicApi; @@ -28,8 +29,8 @@ public class NodeParentTree { @Internal public NodeParentTree(Deque nodeStack) { - assertNotNull(nodeStack, () -> "You MUST have a non null stack of nodes"); - assertTrue(!nodeStack.isEmpty(), () -> "You MUST have a non empty stack of nodes"); + Assert.assertNotNull(nodeStack, "You MUST have a non null stack of nodes"); + Assert.assertTrue(!nodeStack.isEmpty(), "You MUST have a non empty stack of nodes"); Deque copy = new ArrayDeque<>(nodeStack); path = mkPath(copy); diff --git a/src/main/java/graphql/language/PrettyAstPrinter.java b/src/main/java/graphql/language/PrettyAstPrinter.java index 8d29f3864d..2a73b6cd8b 100644 --- a/src/main/java/graphql/language/PrettyAstPrinter.java +++ b/src/main/java/graphql/language/PrettyAstPrinter.java @@ -1,5 +1,6 @@ package graphql.language; +import graphql.Assert; import graphql.ExperimentalApi; import graphql.collect.ImmutableKit; import graphql.parser.CommentParser; @@ -220,7 +221,7 @@ private NodePrinter unionTypeDefinition(String nodeName) { private String node(Node node, Class startClass) { if (startClass != null) { - assertTrue(startClass.isInstance(node), () -> "The starting class must be in the inherit tree"); + Assert.assertTrue(startClass.isInstance(node), "The starting class must be in the inherit tree"); } StringBuilder builder = new StringBuilder(); diff --git a/src/main/java/graphql/normalized/ExecutableNormalizedField.java b/src/main/java/graphql/normalized/ExecutableNormalizedField.java index 47f8151229..ebc5a21727 100644 --- a/src/main/java/graphql/normalized/ExecutableNormalizedField.java +++ b/src/main/java/graphql/normalized/ExecutableNormalizedField.java @@ -184,7 +184,7 @@ public boolean hasChildren() { public GraphQLOutputType getType(GraphQLSchema schema) { List fieldDefinitions = getFieldDefinitions(schema); Set fieldTypes = fieldDefinitions.stream().map(fd -> simplePrint(fd.getType())).collect(toSet()); - Assert.assertTrue(fieldTypes.size() == 1, () -> "More than one type ... use getTypes"); + Assert.assertTrue(fieldTypes.size() == 1, "More than one type ... use getTypes"); return fieldDefinitions.get(0).getType(); } @@ -438,7 +438,7 @@ public List getChildrenWithSameResultKey(String resul public List getChildren(int includingRelativeLevel) { List result = new ArrayList<>(); - assertTrue(includingRelativeLevel >= 1, () -> "relative level must be >= 1"); + Assert.assertTrue(includingRelativeLevel >= 1, "relative level must be >= 1"); this.getChildren().forEach(child -> { traverseImpl(child, result::add, 1, includingRelativeLevel); diff --git a/src/main/java/graphql/normalized/NormalizedInputValue.java b/src/main/java/graphql/normalized/NormalizedInputValue.java index 390bac032a..a73d90c4a9 100644 --- a/src/main/java/graphql/normalized/NormalizedInputValue.java +++ b/src/main/java/graphql/normalized/NormalizedInputValue.java @@ -1,5 +1,6 @@ package graphql.normalized; +import graphql.Assert; import graphql.PublicApi; import graphql.language.Value; @@ -7,6 +8,7 @@ import static graphql.Assert.assertNotNull; import static graphql.Assert.assertTrue; +import static graphql.Assert.assertTrue; import static graphql.Assert.assertValidName; import static graphql.language.AstPrinter.printAst; @@ -99,7 +101,7 @@ private boolean isListOnly(String typeName) { private String unwrapOne(String typeName) { assertNotNull(typeName); - assertTrue(typeName.trim().length() > 0, () -> "We have an empty type name unwrapped"); + Assert.assertTrue(typeName.trim().length() > 0, "We have an empty type name unwrapped"); if (typeName.endsWith("!")) { return typeName.substring(0, typeName.length() - 1); } diff --git a/src/main/java/graphql/schema/AsyncDataFetcher.java b/src/main/java/graphql/schema/AsyncDataFetcher.java index b07aa80ddb..fc405fe4b6 100644 --- a/src/main/java/graphql/schema/AsyncDataFetcher.java +++ b/src/main/java/graphql/schema/AsyncDataFetcher.java @@ -1,5 +1,6 @@ package graphql.schema; +import graphql.Assert; import graphql.PublicApi; import java.util.concurrent.CompletableFuture; @@ -67,8 +68,8 @@ public AsyncDataFetcher(DataFetcher wrappedDataFetcher) { } public AsyncDataFetcher(DataFetcher wrappedDataFetcher, Executor executor) { - this.wrappedDataFetcher = assertNotNull(wrappedDataFetcher, () -> "wrappedDataFetcher can't be null"); - this.executor = assertNotNull(executor, () -> "executor can't be null"); + this.wrappedDataFetcher = Assert.assertNotNull(wrappedDataFetcher, "wrappedDataFetcher can't be null"); + this.executor = Assert.assertNotNull(executor, "executor can't be null"); } @Override diff --git a/src/main/java/graphql/schema/DefaultGraphqlTypeComparatorRegistry.java b/src/main/java/graphql/schema/DefaultGraphqlTypeComparatorRegistry.java index a7c3c42f11..c945ddf3c9 100644 --- a/src/main/java/graphql/schema/DefaultGraphqlTypeComparatorRegistry.java +++ b/src/main/java/graphql/schema/DefaultGraphqlTypeComparatorRegistry.java @@ -1,6 +1,7 @@ package graphql.schema; import com.google.common.collect.ImmutableMap; +import graphql.Assert; import graphql.PublicApi; import java.util.Comparator; @@ -9,6 +10,7 @@ import java.util.Objects; import java.util.function.UnaryOperator; +import static graphql.Assert.assertNotNull; import static graphql.Assert.assertNotNull; import static graphql.schema.GraphQLTypeUtil.unwrapAll; import static graphql.schema.GraphqlTypeComparatorEnvironment.newEnvironment; @@ -33,6 +35,7 @@ public class DefaultGraphqlTypeComparatorRegistry implements GraphqlTypeComparat /** * This orders the schema into a sensible grouped order + * * @return a comparator that allows for sensible grouped order */ public static Comparator sensibleGroupedOrder() { @@ -124,9 +127,9 @@ public static class Builder { * @return The {@code Builder} instance to allow chaining. */ public Builder addComparator(GraphqlTypeComparatorEnvironment environment, Class comparatorClass, Comparator comparator) { - assertNotNull(environment, () -> "environment can't be null"); - assertNotNull(comparatorClass, () -> "comparatorClass can't be null"); - assertNotNull(comparator, () -> "comparator can't be null"); + Assert.assertNotNull(environment, "environment can't be null"); + Assert.assertNotNull(comparatorClass, "comparatorClass can't be null"); + Assert.assertNotNull(comparator, "comparator can't be null"); registry.put(environment, comparator); return this; } diff --git a/src/main/java/graphql/schema/GraphQLTypeUtil.java b/src/main/java/graphql/schema/GraphQLTypeUtil.java index de5cd5e8b7..f33164937b 100644 --- a/src/main/java/graphql/schema/GraphQLTypeUtil.java +++ b/src/main/java/graphql/schema/GraphQLTypeUtil.java @@ -26,7 +26,7 @@ public class GraphQLTypeUtil { * @return the type in graphql SDL format, eg [typeName!]! */ public static String simplePrint(GraphQLType type) { - Assert.assertNotNull(type, () -> "type can't be null"); + Assert.assertNotNull(type, "type can't be null"); if (isNonNull(type)) { return simplePrint(unwrapOne(type)) + "!"; } else if (isList(type)) { diff --git a/src/main/java/graphql/schema/InputValueWithState.java b/src/main/java/graphql/schema/InputValueWithState.java index 33ef45d062..0a8ed45f0a 100644 --- a/src/main/java/graphql/schema/InputValueWithState.java +++ b/src/main/java/graphql/schema/InputValueWithState.java @@ -1,5 +1,6 @@ package graphql.schema; +import graphql.Assert; import graphql.PublicApi; import graphql.language.Value; import org.jetbrains.annotations.NotNull; @@ -45,7 +46,7 @@ private InputValueWithState(State state, Object value) { public static final InputValueWithState NOT_SET = new InputValueWithState(State.NOT_SET, null); public static InputValueWithState newLiteralValue(@NotNull Value value) { - assertNotNull(value, () -> "value literal can't be null"); + Assert.assertNotNull(value, "value literal can't be null"); return new InputValueWithState(State.LITERAL, value); } diff --git a/src/test/groovy/graphql/AssertTest.groovy b/src/test/groovy/graphql/AssertTest.groovy index 074c8b9b67..13a4bae364 100644 --- a/src/test/groovy/graphql/AssertTest.groovy +++ b/src/test/groovy/graphql/AssertTest.groovy @@ -3,7 +3,7 @@ package graphql import spock.lang.Specification class AssertTest extends Specification { - def "assertNull should not throw on none null value"() { + def "assertNotNull should not throw on none null value"() { when: Assert.assertNotNull("some object") @@ -11,7 +11,7 @@ class AssertTest extends Specification { noExceptionThrown() } - def "assertNull should throw on null value"() { + def "assertNotNull should throw on null value"() { when: Assert.assertNotNull(null) @@ -19,15 +19,24 @@ class AssertTest extends Specification { thrown(AssertException) } - def "assertNull with error message should not throw on none null value"() { + def "assertNotNull constant message should throw on null value"() { when: - Assert.assertNotNull("some object", { -> "error message"}) + Assert.assertNotNull(null, "constant message") + + then: + def error = thrown(AssertException) + error.message == "constant message" + } + + def "assertNotNull with error message should not throw on none null value"() { + when: + Assert.assertNotNull("some object", { -> "error message" }) then: noExceptionThrown() } - def "assertNull with error message should throw on null value with formatted message"() { + def "assertNotNull with error message should throw on null value with formatted message"() { when: Assert.assertNotNull(value, { -> String.format(format, arg) }) @@ -91,7 +100,7 @@ class AssertTest extends Specification { def "assertNotEmpty should not throw on none empty collection"() { when: - Assert.assertNotEmpty(["some object"], { -> "error message"}) + Assert.assertNotEmpty(["some object"], { -> "error message" }) then: noExceptionThrown() @@ -99,7 +108,7 @@ class AssertTest extends Specification { def "assertTrue should not throw on true value"() { when: - Assert.assertTrue(true, { ->"error message"}) + Assert.assertTrue(true, { -> "error message" }) then: noExceptionThrown() @@ -120,6 +129,47 @@ class AssertTest extends Specification { "code" | null || "code" } + def "assertTrue constant message should throw with message"() { + when: + Assert.assertTrue(false, "constant message") + + then: + def error = thrown(AssertException) + error.message == "constant message" + } + + def "assertFalse should throw"() { + when: + Assert.assertFalse(true) + + then: + thrown(AssertException) + } + + def "assertFalse constant message should throw with message"() { + when: + Assert.assertFalse(true, "constant message") + + then: + def error = thrown(AssertException) + error.message == "constant message" + } + + def "assertFalse with error message should throw on false value with formatted message"() { + when: + Assert.assertFalse(true, { -> String.format(format, arg) }) + + then: + def error = thrown(AssertException) + error.message == expectedMessage + + where: + format | arg || expectedMessage + "error %s" | "msg" || "error msg" + "code %d" | 1 || "code 1" + "code" | null || "code" + } + def "assertValidName should not throw on valid names"() { when: Assert.assertValidName(name) From b0a362e494d3bf3e76c950a21ca713a6b8b52b56 Mon Sep 17 00:00:00 2001 From: Brad Baker Date: Thu, 7 Mar 2024 21:24:49 +1100 Subject: [PATCH 2/6] Cheaper assertions with N arity functions --- src/main/java/graphql/Assert.java | 131 ++++++++++++------ .../analysis/NodeVisitorWithTypeTracking.java | 5 +- src/main/java/graphql/execution/Async.java | 6 +- .../execution/ValuesResolverLegacy.java | 2 +- .../ValuesResolverOneOfValidation.java | 4 +- .../conditional/ConditionalNodes.java | 2 +- .../incremental/IncrementalUtils.java | 7 +- .../persisted/PersistedQuerySupport.java | 2 +- .../graphql/introspection/Introspection.java | 4 +- .../java/graphql/language/AstPrinter.java | 3 +- .../normalized/ExecutableNormalizedField.java | 4 +- .../normalized/NormalizedInputValue.java | 4 +- .../graphql/schema/CodeRegistryVisitor.java | 4 +- .../graphql/schema/GraphQLCodeRegistry.java | 4 +- .../java/graphql/schema/GraphQLNonNull.java | 4 +- .../java/graphql/schema/GraphQLSchema.java | 6 +- .../schema/GraphQLTypeResolvingVisitor.java | 2 +- .../graphql/schema/SchemaTransformer.java | 4 +- .../graphql/schema/diffing/SchemaGraph.java | 16 +-- .../java/graphql/schema/diffing/Vertex.java | 2 +- .../idl/SchemaGeneratorDirectiveHelper.java | 2 +- .../schema/idl/SchemaGeneratorHelper.java | 2 +- src/main/java/graphql/util/Anonymizer.java | 11 +- .../java/graphql/util/TraverserState.java | 4 +- .../graphql/util/TreeParallelTransformer.java | 2 +- .../graphql/util/TreeParallelTraverser.java | 2 +- src/test/groovy/graphql/AssertTest.groovy | 93 +++++++++++++ src/test/java/benchmark/AssertBenchmark.java | 90 ++++++++++++ 28 files changed, 326 insertions(+), 96 deletions(-) create mode 100644 src/test/java/benchmark/AssertBenchmark.java diff --git a/src/main/java/graphql/Assert.java b/src/main/java/graphql/Assert.java index d780e419f3..872e19c05e 100644 --- a/src/main/java/graphql/Assert.java +++ b/src/main/java/graphql/Assert.java @@ -10,79 +10,92 @@ @Internal public class Assert { - public static T assertNotNull(T object, Supplier msg) { + public static T assertNotNullWithNPE(T object, Supplier msg) { if (object != null) { return object; } - throw new AssertException(msg.get()); + throw new NullPointerException(msg.get()); } - public static T assertNotNullWithNPE(T object, Supplier msg) { + public static T assertNotNull(T object) { if (object != null) { return object; } - throw new NullPointerException(msg.get()); + return throwAssert("Object required to be not null"); } - public static T assertNotNull(T object) { + public static T assertNotNull(T object, Supplier msg) { if (object != null) { return object; } - throw new AssertException("Object required to be not null"); + return throwAssert(msg.get()); } - /** - * If you use this method, make sure that the message is a constant string - * - * @param object the object to check for null - * @param constantMsg make sure this is a constant message - * @param for two - * - * @return the object - */ public static T assertNotNull(T object, String constantMsg) { if (object != null) { return object; } - throw new AssertException(constantMsg); + return throwAssert(constantMsg); + } + + public static T assertNotNull(T object, String msgFmt, Object arg1) { + if (object != null) { + return object; + } + return throwAssert(msgFmt, arg1); } + public static T assertNotNull(T object, String msgFmt, Object arg1, Object arg2) { + if (object != null) { + return object; + } + return throwAssert(msgFmt, arg1, arg2); + } + + public static T assertNotNull(T object, String msgFmt, Object arg1, Object arg2, Object arg3) { + if (object != null) { + return object; + } + return throwAssert(msgFmt, arg1, arg2, arg3); + } + + public static void assertNull(T object, Supplier msg) { if (object == null) { return; } - throw new AssertException(msg.get()); + throwAssert(msg.get()); } public static void assertNull(T object) { if (object == null) { return; } - throw new AssertException("Object required to be null"); + throwAssert("Object required to be null"); } public static T assertNeverCalled() { - throw new AssertException("Should never been called"); + return throwAssert("Should never been called"); } public static T assertShouldNeverHappen(String format, Object... args) { - throw new AssertException("Internal error: should never happen: " + format(format, args)); + return throwAssert("Internal error: should never happen: %s", format(format, args)); } public static T assertShouldNeverHappen() { - throw new AssertException("Internal error: should never happen"); + return throwAssert("Internal error: should never happen"); } public static Collection assertNotEmpty(Collection collection) { if (collection == null || collection.isEmpty()) { - throw new AssertException("collection must be not null and not empty"); + throwAssert("collection must be not null and not empty"); } return collection; } public static Collection assertNotEmpty(Collection collection, Supplier msg) { if (collection == null || collection.isEmpty()) { - throw new AssertException(msg.get()); + throwAssert(msg.get()); } return collection; } @@ -91,54 +104,84 @@ public static void assertTrue(boolean condition, Supplier msg) { if (condition) { return; } - throw new AssertException(msg.get()); + throwAssert(msg.get()); } public static void assertTrue(boolean condition) { if (condition) { return; } - throw new AssertException("condition expected to be true"); + throwAssert("condition expected to be true"); } - /** - * If you use this method, make sure that the message is a constant string - * - * @param condition the condition that must be true - * @param constantMsg make sure this is a constant message - */ public static void assertTrue(boolean condition, String constantMsg) { if (condition) { return; } - throw new AssertException(constantMsg); + throwAssert(constantMsg); + } + + public static void assertTrue(boolean condition, String msgFmt, Object arg1) { + if (condition) { + return; + } + throwAssert(msgFmt, arg1); + } + + public static void assertTrue(boolean condition, String msgFmt, Object arg1, Object arg2) { + if (condition) { + return; + } + throwAssert(msgFmt, arg1, arg2); + } + + public static void assertTrue(boolean condition, String msgFmt, Object arg1, Object arg2, Object arg3) { + if (condition) { + return; + } + throwAssert(msgFmt, arg1, arg2, arg3); } public static void assertFalse(boolean condition, Supplier msg) { if (!condition) { return; } - throw new AssertException(msg.get()); + throwAssert(msg.get()); } public static void assertFalse(boolean condition) { if (!condition) { return; } - throw new AssertException("condition expected to be false"); + throwAssert("condition expected to be false"); } - /** - * If you use this method, make sure that the message is a constant string - * - * @param condition the condition that must be false - * @param constantMsg make sure this is a constant message - */ public static void assertFalse(boolean condition, String constantMsg) { if (!condition) { return; } - throw new AssertException(constantMsg); + throwAssert(constantMsg); + } + + public static void assertFalse(boolean condition, String msgFmt, Object arg1) { + if (!condition) { + return; + } + throwAssert(msgFmt, arg1); + } + + public static void assertFalse(boolean condition, String msgFmt, Object arg1, Object arg2) { + if (!condition) { + return; + } + throwAssert(msgFmt, arg1, arg2); + } + + public static void assertFalse(boolean condition, String msgFmt, Object arg1, Object arg2, Object arg3) { + if (!condition) { + return; + } + throwAssert(msgFmt, arg1, arg2, arg3); } private static final String invalidNameErrorMessage = "Name must be non-null, non-empty and match [_A-Za-z][_0-9A-Za-z]* - was '%s'"; @@ -150,13 +193,17 @@ public static void assertFalse(boolean condition, String constantMsg) { * currently non null, non empty, * * @param name - the name to be validated. + * * @return the name if valid, or AssertException if invalid. */ public static String assertValidName(String name) { if (name != null && !name.isEmpty() && validNamePattern.matcher(name).matches()) { return name; } - throw new AssertException(String.format(invalidNameErrorMessage, name)); + return throwAssert(invalidNameErrorMessage, name); } + private static T throwAssert(String format, Object... args) { + throw new AssertException(format(format, args)); + } } diff --git a/src/main/java/graphql/analysis/NodeVisitorWithTypeTracking.java b/src/main/java/graphql/analysis/NodeVisitorWithTypeTracking.java index 972a6f8e9c..683138e387 100644 --- a/src/main/java/graphql/analysis/NodeVisitorWithTypeTracking.java +++ b/src/main/java/graphql/analysis/NodeVisitorWithTypeTracking.java @@ -36,7 +36,6 @@ import static graphql.Assert.assertNotNull; import static graphql.schema.GraphQLTypeUtil.unwrapAll; import static graphql.util.TraverserContext.Phase.LEAVE; -import static java.lang.String.format; /** * Internally used node visitor which delegates to a {@link QueryVisitor} with type @@ -142,8 +141,8 @@ public TraversalControl visitFragmentSpread(FragmentSpread fragmentSpread, Trave GraphQLCompositeType typeCondition = (GraphQLCompositeType) schema.getType(fragmentDefinition.getTypeCondition().getName()); assertNotNull(typeCondition, - () -> format("Invalid type condition '%s' in fragment '%s'", fragmentDefinition.getTypeCondition().getName(), - fragmentDefinition.getName())); + "Invalid type condition '%s' in fragment '%s'", fragmentDefinition.getTypeCondition().getName(), + fragmentDefinition.getName()); context.setVar(QueryTraversalContext.class, new QueryTraversalContext(typeCondition, parentEnv.getEnvironment(), fragmentDefinition, graphQLContext)); return TraversalControl.CONTINUE; } diff --git a/src/main/java/graphql/execution/Async.java b/src/main/java/graphql/execution/Async.java index cbdd45d96b..f3eebc9e3e 100644 --- a/src/main/java/graphql/execution/Async.java +++ b/src/main/java/graphql/execution/Async.java @@ -86,7 +86,7 @@ public void add(CompletableFuture completableFuture) { @Override public CompletableFuture> await() { - Assert.assertTrue(ix == 1, () -> "expected size was " + 1 + " got " + ix); + Assert.assertTrue(ix == 1, "expected size was 1 got %d", ix); return completableFuture.thenApply(Collections::singletonList); } } @@ -109,7 +109,7 @@ public void add(CompletableFuture completableFuture) { @Override public CompletableFuture> await() { - Assert.assertTrue(ix == array.length, "expected size was less than the array length"); + Assert.assertTrue(ix == array.length, "expected size was %d got %d", array.length, ix); CompletableFuture> overallResult = new CompletableFuture<>(); CompletableFuture.allOf(array) @@ -160,7 +160,7 @@ private static void eachSequentiallyImpl(Iterator iterator, BiFunction CompletableFuture cf; try { cf = cfFactory.apply(iterator.next(), tmpResult); - Assert.assertNotNull(cf,"cfFactory must return a non null value"); + Assert.assertNotNull(cf, "cfFactory must return a non null value"); } catch (Exception e) { cf = new CompletableFuture<>(); cf.completeExceptionally(new CompletionException(e)); diff --git a/src/main/java/graphql/execution/ValuesResolverLegacy.java b/src/main/java/graphql/execution/ValuesResolverLegacy.java index 81bc80ccdc..d5e58f4656 100644 --- a/src/main/java/graphql/execution/ValuesResolverLegacy.java +++ b/src/main/java/graphql/execution/ValuesResolverLegacy.java @@ -49,7 +49,7 @@ class ValuesResolverLegacy { */ @VisibleForTesting static Value valueToLiteralLegacy(Object value, GraphQLType type, GraphQLContext graphqlContext, Locale locale) { - assertTrue(!(value instanceof Value), () -> "Unexpected literal " + value); + assertTrue(!(value instanceof Value), "Unexpected literal %s", value); if (value == null) { return null; } diff --git a/src/main/java/graphql/execution/ValuesResolverOneOfValidation.java b/src/main/java/graphql/execution/ValuesResolverOneOfValidation.java index c0f98f5523..9955ce050d 100644 --- a/src/main/java/graphql/execution/ValuesResolverOneOfValidation.java +++ b/src/main/java/graphql/execution/ValuesResolverOneOfValidation.java @@ -42,7 +42,7 @@ static void validateOneOfInputTypes(GraphQLType type, Object inputValue, Value String.format("The coerced argument %s GraphQLInputObjectType is unexpectedly not a map", argumentName)); + Assert.assertTrue(inputValue instanceof Map, "The coerced argument %s GraphQLInputObjectType is unexpectedly not a map", argumentName); Map objectMap = (Map) inputValue; GraphQLInputObjectType inputObjectType = (GraphQLInputObjectType) unwrappedNonNullType; @@ -63,7 +63,7 @@ static void validateOneOfInputTypes(GraphQLType type, Object inputValue, Value 1) { - Assert.assertShouldNeverHappen(String.format("argument %s has %s object fields with the same name: '%s'. A maximum of 1 is expected", argumentName, values.size(), childFieldName)); + Assert.assertShouldNeverHappen("argument %s has %s object fields with the same name: '%s'. A maximum of 1 is expected", argumentName, values.size(), childFieldName); } else if (!values.isEmpty()) { validateOneOfInputTypes(childFieldType, childFieldInputValue, values.get(0), argumentName, locale); } diff --git a/src/main/java/graphql/execution/conditional/ConditionalNodes.java b/src/main/java/graphql/execution/conditional/ConditionalNodes.java index 9c90deead0..7013c53bf3 100644 --- a/src/main/java/graphql/execution/conditional/ConditionalNodes.java +++ b/src/main/java/graphql/execution/conditional/ConditionalNodes.java @@ -93,7 +93,7 @@ private boolean getDirectiveResult(Map variables, List argumentValues = ValuesResolver.getArgumentValues(SkipDirective.getArguments(), foundDirective.getArguments(), CoercedVariables.of(variables), GraphQLContext.getDefault(), Locale.getDefault()); Object flag = argumentValues.get("if"); - Assert.assertTrue(flag instanceof Boolean, () -> String.format("The '%s' directive MUST have a value for the 'if' argument", directiveName)); + Assert.assertTrue(flag instanceof Boolean, "The '%s' directive MUST have a value for the 'if' argument", directiveName); return (Boolean) flag; } return defaultValue; diff --git a/src/main/java/graphql/execution/incremental/IncrementalUtils.java b/src/main/java/graphql/execution/incremental/IncrementalUtils.java index 78d4655027..2a89ade3fd 100644 --- a/src/main/java/graphql/execution/incremental/IncrementalUtils.java +++ b/src/main/java/graphql/execution/incremental/IncrementalUtils.java @@ -17,7 +17,8 @@ @Internal public class IncrementalUtils { - private IncrementalUtils() {} + private IncrementalUtils() { + } public static T createDeferredExecution( Map variables, @@ -30,7 +31,7 @@ public static T createDeferredExecution( Map argumentValues = ValuesResolver.getArgumentValues(DeferDirective.getArguments(), deferDirective.getArguments(), CoercedVariables.of(variables), GraphQLContext.getDefault(), Locale.getDefault()); Object flag = argumentValues.get("if"); - Assert.assertTrue(flag instanceof Boolean, () -> String.format("The '%s' directive MUST have a value for the 'if' argument", DeferDirective.getName())); + Assert.assertTrue(flag instanceof Boolean, "The '%s' directive MUST have a value for the 'if' argument", DeferDirective.getName()); if (!((Boolean) flag)) { return null; @@ -42,7 +43,7 @@ public static T createDeferredExecution( return builderFunction.apply(null); } - Assert.assertTrue(label instanceof String, () -> String.format("The 'label' argument from the '%s' directive MUST contain a String value", DeferDirective.getName())); + Assert.assertTrue(label instanceof String, "The 'label' argument from the '%s' directive MUST contain a String value", DeferDirective.getName()); return builderFunction.apply((String) label); } diff --git a/src/main/java/graphql/execution/preparsed/persisted/PersistedQuerySupport.java b/src/main/java/graphql/execution/preparsed/persisted/PersistedQuerySupport.java index af2edef379..dc5357b448 100644 --- a/src/main/java/graphql/execution/preparsed/persisted/PersistedQuerySupport.java +++ b/src/main/java/graphql/execution/preparsed/persisted/PersistedQuerySupport.java @@ -38,7 +38,7 @@ public PersistedQuerySupport(PersistedQueryCache persistedQueryCache) { @Override public PreparsedDocumentEntry getDocument(ExecutionInput executionInput, Function parseAndValidateFunction) { Optional queryIdOption = getPersistedQueryId(executionInput); - assertNotNull(queryIdOption, () -> String.format("The class %s MUST return a non null optional query id", this.getClass().getName())); + assertNotNull(queryIdOption, "The class %s MUST return a non null optional query id", this.getClass().getName()); try { if (queryIdOption.isPresent()) { diff --git a/src/main/java/graphql/introspection/Introspection.java b/src/main/java/graphql/introspection/Introspection.java index d496e03702..d1a6d97124 100644 --- a/src/main/java/graphql/introspection/Introspection.java +++ b/src/main/java/graphql/introspection/Introspection.java @@ -714,10 +714,10 @@ public static GraphQLFieldDefinition getFieldDef(GraphQLSchema schema, GraphQLCo return schema.getIntrospectionTypenameFieldDefinition(); } - assertTrue(parentType instanceof GraphQLFieldsContainer, () -> String.format("should not happen : parent type must be an object or interface %s", parentType)); + assertTrue(parentType instanceof GraphQLFieldsContainer, "Should not happen : parent type must be an object or interface %s", parentType); GraphQLFieldsContainer fieldsContainer = (GraphQLFieldsContainer) parentType; GraphQLFieldDefinition fieldDefinition = schema.getCodeRegistry().getFieldVisibility().getFieldDefinition(fieldsContainer, fieldName); - assertTrue(fieldDefinition != null, () -> String.format("Unknown field '%s' for type %s", fieldName, fieldsContainer.getName())); + assertTrue(fieldDefinition != null, "Unknown field '%s' for type %s", fieldName, fieldsContainer.getName()); return fieldDefinition; } } diff --git a/src/main/java/graphql/language/AstPrinter.java b/src/main/java/graphql/language/AstPrinter.java index df2c1f71ea..e032c52c3b 100644 --- a/src/main/java/graphql/language/AstPrinter.java +++ b/src/main/java/graphql/language/AstPrinter.java @@ -13,6 +13,7 @@ import java.util.Map; import java.util.StringJoiner; +import static graphql.Assert.assertShouldNeverHappen; import static graphql.Assert.assertTrue; import static graphql.util.EscapeUtil.escapeJsonString; import static java.lang.String.valueOf; @@ -483,7 +484,7 @@ NodePrinter _findPrinter(Node node, Class startClass) { } clazz = clazz.getSuperclass(); } - throw new AssertException(String.format("We have a missing printer implementation for %s : report a bug!", clazz)); + return assertShouldNeverHappen("We have a missing printer implementation for %s : report a bug!", clazz); } private boolean isEmpty(List list) { diff --git a/src/main/java/graphql/normalized/ExecutableNormalizedField.java b/src/main/java/graphql/normalized/ExecutableNormalizedField.java index ebc5a21727..01178b8160 100644 --- a/src/main/java/graphql/normalized/ExecutableNormalizedField.java +++ b/src/main/java/graphql/normalized/ExecutableNormalizedField.java @@ -201,7 +201,7 @@ public void forEachFieldDefinition(GraphQLSchema schema, Consumer String.format("No field %s found for type %s", fieldName, objectTypeName))); + consumer.accept(assertNotNull(type.getField(fieldName), "No field %s found for type %s", fieldName, objectTypeName)); } } @@ -224,7 +224,7 @@ private GraphQLFieldDefinition getOneFieldDefinition(GraphQLSchema schema) { String objectTypeName = objectTypeNames.iterator().next(); GraphQLObjectType type = (GraphQLObjectType) assertNotNull(schema.getType(objectTypeName)); - return assertNotNull(type.getField(fieldName), () -> String.format("No field %s found for type %s", fieldName, objectTypeName)); + return assertNotNull(type.getField(fieldName), "No field %s found for type %s", fieldName, objectTypeName); } private static GraphQLFieldDefinition resolveIntrospectionField(GraphQLSchema schema, Set objectTypeNames, String fieldName) { diff --git a/src/main/java/graphql/normalized/NormalizedInputValue.java b/src/main/java/graphql/normalized/NormalizedInputValue.java index a73d90c4a9..181e8594b1 100644 --- a/src/main/java/graphql/normalized/NormalizedInputValue.java +++ b/src/main/java/graphql/normalized/NormalizedInputValue.java @@ -107,8 +107,8 @@ private String unwrapOne(String typeName) { } if (isListOnly(typeName)) { // nominally this will never be true - but better to be safe than sorry - assertTrue(typeName.startsWith("["), () -> String.format("We have a unbalanced list type string '%s'", typeName)); - assertTrue(typeName.endsWith("]"), () -> String.format("We have a unbalanced list type string '%s'", typeName)); + assertTrue(typeName.startsWith("["), "We have a unbalanced list type string '%s'", typeName); + assertTrue(typeName.endsWith("]"), "We have a unbalanced list type string '%s'", typeName); return typeName.substring(1, typeName.length() - 1); } diff --git a/src/main/java/graphql/schema/CodeRegistryVisitor.java b/src/main/java/graphql/schema/CodeRegistryVisitor.java index 50cfa2a492..166ed6239c 100644 --- a/src/main/java/graphql/schema/CodeRegistryVisitor.java +++ b/src/main/java/graphql/schema/CodeRegistryVisitor.java @@ -40,7 +40,7 @@ public TraversalControl visitGraphQLInterfaceType(GraphQLInterfaceType node, Tra codeRegistry.typeResolverIfAbsent(node, typeResolver); } assertTrue(codeRegistry.getTypeResolver(node) != null, - () -> String.format("You MUST provide a type resolver for the interface type '%s'", node.getName())); + "You MUST provide a type resolver for the interface type '%s'", node.getName()); return CONTINUE; } @@ -51,7 +51,7 @@ public TraversalControl visitGraphQLUnionType(GraphQLUnionType node, TraverserCo codeRegistry.typeResolverIfAbsent(node, typeResolver); } assertTrue(codeRegistry.getTypeResolver(node) != null, - () -> String.format("You MUST provide a type resolver for the union type '%s'", node.getName())); + "You MUST provide a type resolver for the union type '%s'", node.getName()); return CONTINUE; } } diff --git a/src/main/java/graphql/schema/GraphQLCodeRegistry.java b/src/main/java/graphql/schema/GraphQLCodeRegistry.java index d1b0f94d4c..e72fadf080 100644 --- a/src/main/java/graphql/schema/GraphQLCodeRegistry.java +++ b/src/main/java/graphql/schema/GraphQLCodeRegistry.java @@ -157,7 +157,7 @@ private static TypeResolver getTypeResolverForInterface(GraphQLInterfaceType par if (typeResolver == null) { typeResolver = parentType.getTypeResolver(); } - return assertNotNull(typeResolver, () -> "There must be a type resolver for interface " + parentType.getName()); + return assertNotNull(typeResolver, "There must be a type resolver for interface %s", parentType.getName()); } private static TypeResolver getTypeResolverForUnion(GraphQLUnionType parentType, Map typeResolverMap) { @@ -166,7 +166,7 @@ private static TypeResolver getTypeResolverForUnion(GraphQLUnionType parentType, if (typeResolver == null) { typeResolver = parentType.getTypeResolver(); } - return assertNotNull(typeResolver, () -> "There must be a type resolver for union " + parentType.getName()); + return assertNotNull(typeResolver, "There must be a type resolver for union %s",parentType.getName()); } /** diff --git a/src/main/java/graphql/schema/GraphQLNonNull.java b/src/main/java/graphql/schema/GraphQLNonNull.java index 6de1ba61d3..12f131b428 100644 --- a/src/main/java/graphql/schema/GraphQLNonNull.java +++ b/src/main/java/graphql/schema/GraphQLNonNull.java @@ -46,8 +46,8 @@ public GraphQLNonNull(GraphQLType wrappedType) { } private void assertNonNullWrapping(GraphQLType wrappedType) { - assertTrue(!GraphQLTypeUtil.isNonNull(wrappedType), () -> - String.format("A non null type cannot wrap an existing non null type '%s'", GraphQLTypeUtil.simplePrint(wrappedType))); + assertTrue(!GraphQLTypeUtil.isNonNull(wrappedType), + "A non null type cannot wrap an existing non null type '%s'", GraphQLTypeUtil.simplePrint(wrappedType)); } @Override diff --git a/src/main/java/graphql/schema/GraphQLSchema.java b/src/main/java/graphql/schema/GraphQLSchema.java index e7026ad6c3..af95dd7258 100644 --- a/src/main/java/graphql/schema/GraphQLSchema.java +++ b/src/main/java/graphql/schema/GraphQLSchema.java @@ -248,7 +248,7 @@ public Set getAdditionalTypes() { public List getTypes(Collection typeNames) { ImmutableList.Builder builder = ImmutableList.builder(); for (String typeName : typeNames) { - builder.add((T) assertNotNull(typeMap.get(typeName), () -> String.format("No type found for name %s", typeName))); + builder.add((T) assertNotNull(typeMap.get(typeName), "No type found for name %s", typeName)); } return builder.build(); } @@ -292,7 +292,7 @@ public GraphQLObjectType getObjectType(String typeName) { GraphQLType graphQLType = typeMap.get(typeName); if (graphQLType != null) { assertTrue(graphQLType instanceof GraphQLObjectType, - () -> String.format("You have asked for named object type '%s' but it's not an object type but rather a '%s'", typeName, graphQLType.getClass().getName())); + "You have asked for named object type '%s' but it's not an object type but rather a '%s'", typeName, graphQLType.getClass().getName()); } return (GraphQLObjectType) graphQLType; } @@ -323,7 +323,7 @@ public GraphQLFieldDefinition getFieldDefinition(FieldCoordinates fieldCoordinat GraphQLType graphQLType = getType(typeName); if (graphQLType != null) { assertTrue(graphQLType instanceof GraphQLFieldsContainer, - () -> String.format("You have asked for named type '%s' but it's not GraphQLFieldsContainer but rather a '%s'", typeName, graphQLType.getClass().getName())); + "You have asked for named type '%s' but it's not GraphQLFieldsContainer but rather a '%s'", typeName, graphQLType.getClass().getName()); return ((GraphQLFieldsContainer) graphQLType).getFieldDefinition(fieldName); } return null; diff --git a/src/main/java/graphql/schema/GraphQLTypeResolvingVisitor.java b/src/main/java/graphql/schema/GraphQLTypeResolvingVisitor.java index 0380ac5cd1..37a57f8933 100644 --- a/src/main/java/graphql/schema/GraphQLTypeResolvingVisitor.java +++ b/src/main/java/graphql/schema/GraphQLTypeResolvingVisitor.java @@ -46,7 +46,7 @@ public TraversalControl visitGraphQLTypeReference(GraphQLTypeReference node, Tra public TraversalControl handleTypeReference(GraphQLTypeReference node, TraverserContext context) { final GraphQLType resolvedType = typeMap.get(node.getName()); - assertNotNull(resolvedType, () -> String.format("type %s not found in schema", node.getName())); + assertNotNull(resolvedType, "type %s not found in schema", node.getName()); context.getParentContext().thisNode().accept(context, new TypeRefResolvingVisitor(resolvedType)); return CONTINUE; } diff --git a/src/main/java/graphql/schema/SchemaTransformer.java b/src/main/java/graphql/schema/SchemaTransformer.java index 06b6cfe08b..31846b6f85 100644 --- a/src/main/java/graphql/schema/SchemaTransformer.java +++ b/src/main/java/graphql/schema/SchemaTransformer.java @@ -322,7 +322,7 @@ public void updateZipper(NodeZipper currentZipper, List>> currentBreadcrumbs = breadcrumbsByZipper.get(currentZipper); - assertNotNull(currentBreadcrumbs, () -> format("No breadcrumbs found for zipper %s", currentZipper)); + assertNotNull(currentBreadcrumbs, "No breadcrumbs found for zipper %s", currentZipper); for (List> breadcrumbs : currentBreadcrumbs) { GraphQLSchemaElement parent = breadcrumbs.get(0).getNode(); zipperByParent.remove(parent, currentZipper); @@ -392,7 +392,7 @@ private boolean zipUpToDummyRoot(List> zippers, } NodeZipper curZipperForElement = nodeToZipper.get(element); - assertNotNull(curZipperForElement, () -> format("curZipperForElement is null for parentNode %s", element)); + assertNotNull(curZipperForElement, "curZipperForElement is null for parentNode %s", element); relevantZippers.updateZipper(curZipperForElement, newZipper); } diff --git a/src/main/java/graphql/schema/diffing/SchemaGraph.java b/src/main/java/graphql/schema/diffing/SchemaGraph.java index edaebe5284..c12c109fb7 100644 --- a/src/main/java/graphql/schema/diffing/SchemaGraph.java +++ b/src/main/java/graphql/schema/diffing/SchemaGraph.java @@ -230,31 +230,31 @@ public List addIsolatedVertices(int count, String debugPrefix) { public Vertex getFieldOrDirectiveForArgument(Vertex argument) { List adjacentVertices = getAdjacentVerticesInverse(argument); - assertTrue(adjacentVertices.size() == 1, () -> format("No field or directive found for %s", argument)); + assertTrue(adjacentVertices.size() == 1, "No field or directive found for %s", argument); return adjacentVertices.get(0); } public Vertex getFieldsContainerForField(Vertex field) { List adjacentVertices = getAdjacentVerticesInverse(field); - assertTrue(adjacentVertices.size() == 1, () -> format("No fields container found for %s", field)); + assertTrue(adjacentVertices.size() == 1, "No fields container found for %s", field); return adjacentVertices.get(0); } public Vertex getInputObjectForInputField(Vertex inputField) { List adjacentVertices = this.getAdjacentVerticesInverse(inputField); - assertTrue(adjacentVertices.size() == 1, () -> format("No input object found for %s", inputField)); + assertTrue(adjacentVertices.size() == 1, "No input object found for %s", inputField); return adjacentVertices.get(0); } public Vertex getAppliedDirectiveForAppliedArgument(Vertex appliedArgument) { List adjacentVertices = this.getAdjacentVerticesInverse(appliedArgument); - assertTrue(adjacentVertices.size() == 1, () -> format("No applied directive found for %s", appliedArgument)); + assertTrue(adjacentVertices.size() == 1, "No applied directive found for %s", appliedArgument); return adjacentVertices.get(0); } public Vertex getAppliedDirectiveContainerForAppliedDirective(Vertex appliedDirective) { List adjacentVertices = this.getAdjacentVerticesInverse(appliedDirective); - assertTrue(adjacentVertices.size() == 1, () -> format("No applied directive container found for %s", appliedDirective)); + assertTrue(adjacentVertices.size() == 1, "No applied directive container found for %s", appliedDirective); return adjacentVertices.get(0); } @@ -266,19 +266,19 @@ public Vertex getAppliedDirectiveContainerForAppliedDirective(Vertex appliedDire */ public Vertex getSingleAdjacentInverseVertex(Vertex input) { Collection adjacentVertices = this.getAdjacentEdgesInverseNonCopy(input); - assertTrue(adjacentVertices.size() == 1, () -> format("No parent found for %s", input)); + assertTrue(adjacentVertices.size() == 1, "No parent found for %s", input); return adjacentVertices.iterator().next().getFrom(); } public int getAppliedDirectiveIndex(Vertex appliedDirective) { List adjacentEdges = this.getAdjacentEdgesInverseCopied(appliedDirective); - assertTrue(adjacentEdges.size() == 1, () -> format("No applied directive container found for %s", appliedDirective)); + assertTrue(adjacentEdges.size() == 1, "No applied directive container found for %s", appliedDirective); return Integer.parseInt(adjacentEdges.get(0).getLabel()); } public Vertex getEnumForEnumValue(Vertex enumValue) { List adjacentVertices = this.getAdjacentVerticesInverse(enumValue); - assertTrue(adjacentVertices.size() == 1, () -> format("No enum found for %s", enumValue)); + assertTrue(adjacentVertices.size() == 1, "No enum found for %s", enumValue); return adjacentVertices.get(0); } diff --git a/src/main/java/graphql/schema/diffing/Vertex.java b/src/main/java/graphql/schema/diffing/Vertex.java index 3a39a34e62..4b408a37c1 100644 --- a/src/main/java/graphql/schema/diffing/Vertex.java +++ b/src/main/java/graphql/schema/diffing/Vertex.java @@ -62,7 +62,7 @@ public T getProperty(String name) { } public String getName() { - return (String) Assert.assertNotNull(properties.get("name"), () -> String.format("should not call getName on %s", this)); + return (String) Assert.assertNotNull(properties.get("name"), "should not call getName on %s", this); } public Map getProperties() { diff --git a/src/main/java/graphql/schema/idl/SchemaGeneratorDirectiveHelper.java b/src/main/java/graphql/schema/idl/SchemaGeneratorDirectiveHelper.java index 8de58a983b..824ee17f41 100644 --- a/src/main/java/graphql/schema/idl/SchemaGeneratorDirectiveHelper.java +++ b/src/main/java/graphql/schema/idl/SchemaGeneratorDirectiveHelper.java @@ -464,7 +464,7 @@ private T wireDirectives( private T invokeWiring(T element, EnvInvoker invoker, SchemaDirectiveWiring schemaDirectiveWiring, SchemaDirectiveWiringEnvironment env) { T newElement = invoker.apply(schemaDirectiveWiring, env); - assertNotNull(newElement, () -> "The SchemaDirectiveWiring MUST return a non null return value for element '" + element.getName() + "'"); + assertNotNull(newElement, "The SchemaDirectiveWiring MUST return a non null return value for element '%s'",element.getName()); return newElement; } diff --git a/src/main/java/graphql/schema/idl/SchemaGeneratorHelper.java b/src/main/java/graphql/schema/idl/SchemaGeneratorHelper.java index 317828aaf8..aa4878efb4 100644 --- a/src/main/java/graphql/schema/idl/SchemaGeneratorHelper.java +++ b/src/main/java/graphql/schema/idl/SchemaGeneratorHelper.java @@ -401,7 +401,7 @@ private GraphQLEnumValueDefinition buildEnumValue(BuildContext buildCtx, if (enumValuesProvider != null) { value = enumValuesProvider.getValue(evd.getName()); assertNotNull(value, - () -> format("EnumValuesProvider for %s returned null for %s", typeDefinition.getName(), evd.getName())); + "EnumValuesProvider for %s returned null for %s", typeDefinition.getName(), evd.getName()); } else { value = evd.getName(); } diff --git a/src/main/java/graphql/util/Anonymizer.java b/src/main/java/graphql/util/Anonymizer.java index 34553d8777..2d0497eca6 100644 --- a/src/main/java/graphql/util/Anonymizer.java +++ b/src/main/java/graphql/util/Anonymizer.java @@ -100,7 +100,6 @@ import static graphql.schema.idl.SchemaGenerator.createdMockedSchema; import static graphql.util.TraversalControl.CONTINUE; import static graphql.util.TreeTransformerUtil.changeNode; -import static java.lang.String.format; /** * Util class which converts schemas and optionally queries @@ -735,14 +734,14 @@ public void visitField(QueryVisitorFieldEnvironment env) { List directives = field.getDirectives(); for (Directive directive : directives) { // this is a directive definition - GraphQLDirective directiveDefinition = assertNotNull(schema.getDirective(directive.getName()), () -> format("%s directive definition not found ", directive.getName())); + GraphQLDirective directiveDefinition = assertNotNull(schema.getDirective(directive.getName()), "%s directive definition not found ", directive.getName()); String directiveName = directiveDefinition.getName(); - String newDirectiveName = assertNotNull(newNames.get(directiveDefinition), () -> format("No new name found for directive %s", directiveName)); + String newDirectiveName = assertNotNull(newNames.get(directiveDefinition), "No new name found for directive %s", directiveName); astNodeToNewName.put(directive, newDirectiveName); for (Argument argument : directive.getArguments()) { GraphQLArgument argumentDefinition = directiveDefinition.getArgument(argument.getName()); - String newArgumentName = assertNotNull(newNames.get(argumentDefinition), () -> format("No new name found for directive %s argument %s", directiveName, argument.getName())); + String newArgumentName = assertNotNull(newNames.get(argumentDefinition), "No new name found for directive %s argument %s", directiveName, argument.getName()); astNodeToNewName.put(argument, newArgumentName); visitDirectiveArgumentValues(directive, argument.getValue()); } @@ -865,7 +864,7 @@ public TraversalControl visitVariableDefinition(VariableDefinition node, Travers @Override public TraversalControl visitVariableReference(VariableReference node, TraverserContext context) { - String newName = assertNotNull(variableNames.get(node.getName()), () -> format("No new variable name found for %s", node.getName())); + String newName = assertNotNull(variableNames.get(node.getName()), "No new variable name found for %s", node.getName()); return changeNode(context, node.transform(builder -> builder.name(newName))); } @@ -916,7 +915,7 @@ private static GraphQLType fromTypeToGraphQLType(Type type, GraphQLSchema schema if (type instanceof TypeName) { String typeName = ((TypeName) type).getName(); GraphQLType graphQLType = schema.getType(typeName); - graphql.Assert.assertNotNull(graphQLType, () -> "Schema must contain type " + typeName); + graphql.Assert.assertNotNull(graphQLType, "Schema must contain type %s", typeName); return graphQLType; } else if (type instanceof NonNullType) { return GraphQLNonNull.nonNull(fromTypeToGraphQLType(TypeUtil.unwrapOne(type), schema)); diff --git a/src/main/java/graphql/util/TraverserState.java b/src/main/java/graphql/util/TraverserState.java index e44058f914..e478e05a0b 100644 --- a/src/main/java/graphql/util/TraverserState.java +++ b/src/main/java/graphql/util/TraverserState.java @@ -44,7 +44,7 @@ public void pushAll(TraverserContext traverserContext, Function { List children = childrenMap.get(key); for (int i = children.size() - 1; i >= 0; i--) { - U child = assertNotNull(children.get(i), () -> "null child for key " + key); + U child = assertNotNull(children.get(i), "null child for key %s",key); NodeLocation nodeLocation = new NodeLocation(key, i); DefaultTraverserContext context = super.newContext(child, traverserContext, nodeLocation); super.state.push(context); @@ -72,7 +72,7 @@ public void pushAll(TraverserContext traverserContext, Function { List children = childrenMap.get(key); for (int i = 0; i < children.size(); i++) { - U child = assertNotNull(children.get(i), () -> "null child for key " + key); + U child = assertNotNull(children.get(i), "null child for key %s",key); NodeLocation nodeLocation = new NodeLocation(key, i); DefaultTraverserContext context = super.newContext(child, traverserContext, nodeLocation); childrenContextMap.computeIfAbsent(key, notUsed -> new ArrayList<>()); diff --git a/src/main/java/graphql/util/TreeParallelTransformer.java b/src/main/java/graphql/util/TreeParallelTransformer.java index 0102af2eff..3003ac3772 100644 --- a/src/main/java/graphql/util/TreeParallelTransformer.java +++ b/src/main/java/graphql/util/TreeParallelTransformer.java @@ -223,7 +223,7 @@ private List pushAll(TraverserContext traverserConte childrenMap.keySet().forEach(key -> { List children = childrenMap.get(key); for (int i = children.size() - 1; i >= 0; i--) { - T child = assertNotNull(children.get(i), () -> String.format("null child for key %s", key)); + T child = assertNotNull(children.get(i), "null child for key %s", key); NodeLocation nodeLocation = new NodeLocation(key, i); DefaultTraverserContext context = newContext(child, traverserContext, nodeLocation); contexts.push(context); diff --git a/src/main/java/graphql/util/TreeParallelTraverser.java b/src/main/java/graphql/util/TreeParallelTraverser.java index db7f27dbaa..75a903be13 100644 --- a/src/main/java/graphql/util/TreeParallelTraverser.java +++ b/src/main/java/graphql/util/TreeParallelTraverser.java @@ -162,7 +162,7 @@ private List pushAll(TraverserContext traverserConte childrenMap.keySet().forEach(key -> { List children = childrenMap.get(key); for (int i = children.size() - 1; i >= 0; i--) { - T child = assertNotNull(children.get(i), () -> String.format("null child for key %s", key)); + T child = assertNotNull(children.get(i), "null child for key %s", key); NodeLocation nodeLocation = new NodeLocation(key, i); DefaultTraverserContext context = newContext(child, traverserContext, nodeLocation); contexts.push(context); diff --git a/src/test/groovy/graphql/AssertTest.groovy b/src/test/groovy/graphql/AssertTest.groovy index 13a4bae364..5d6901ba23 100644 --- a/src/test/groovy/graphql/AssertTest.groovy +++ b/src/test/groovy/graphql/AssertTest.groovy @@ -51,6 +51,35 @@ class AssertTest extends Specification { null | "code" | null || "code" } + def "assertNotNull with different number of error args throws assertions"() { + when: + toRun.run() + + then: + def error = thrown(AssertException) + error.message == expectedMessage + + where: + toRun | expectedMessage + runnable({ Assert.assertNotNull(null, "error %s", "arg1") }) | "error arg1" + runnable({ Assert.assertNotNull(null, "error %s %s", "arg1", "arg2") }) | "error arg1 arg2" + runnable({ Assert.assertNotNull(null, "error %s %s %s", "arg1", "arg2", "arg3") }) | "error arg1 arg2 arg3" + } + + def "assertNotNull with different number of error args with non null does not throw assertions"() { + when: + toRun.run() + + then: + noExceptionThrown() + + where: + toRun | expectedMessage + runnable({ Assert.assertNotNull("x", "error %s", "arg1") }) | "error arg1" + runnable({ Assert.assertNotNull("x", "error %s %s", "arg1", "arg2") }) | "error arg1 arg2" + runnable({ Assert.assertNotNull("x", "error %s %s %s", "arg1", "arg2", "arg3") }) | "error arg1 arg2 arg3" + } + def "assertNeverCalled should always throw"() { when: Assert.assertNeverCalled() @@ -138,6 +167,35 @@ class AssertTest extends Specification { error.message == "constant message" } + def "assertTrue with different number of error args throws assertions"() { + when: + toRun.run() + + then: + def error = thrown(AssertException) + error.message == expectedMessage + + where: + toRun | expectedMessage + runnable({ Assert.assertTrue(false, "error %s", "arg1") }) | "error arg1" + runnable({ Assert.assertTrue(false, "error %s %s", "arg1", "arg2") }) | "error arg1 arg2" + runnable({ Assert.assertTrue(false, "error %s %s %s", "arg1", "arg2", "arg3") }) | "error arg1 arg2 arg3" + } + + def "assertTrue with different number of error args but false does not throw assertions"() { + when: + toRun.run() + + then: + noExceptionThrown() + + where: + toRun | expectedMessage + runnable({ Assert.assertTrue(true, "error %s", "arg1") }) | "error arg1" + runnable({ Assert.assertTrue(true, "error %s %s", "arg1", "arg2") }) | "error arg1 arg2" + runnable({ Assert.assertTrue(true, "error %s %s %s", "arg1", "arg2", "arg3") }) | "error arg1 arg2 arg3" + } + def "assertFalse should throw"() { when: Assert.assertFalse(true) @@ -170,6 +228,35 @@ class AssertTest extends Specification { "code" | null || "code" } + def "assertFalse with different number of error args throws assertions"() { + when: + toRun.run() + + then: + def error = thrown(AssertException) + error.message == expectedMessage + + where: + toRun | expectedMessage + runnable({ Assert.assertFalse(true, "error %s", "arg1") }) | "error arg1" + runnable({ Assert.assertFalse(true, "error %s %s", "arg1", "arg2") }) | "error arg1 arg2" + runnable({ Assert.assertFalse(true, "error %s %s %s", "arg1", "arg2", "arg3") }) | "error arg1 arg2 arg3" + } + + def "assertFalse with different number of error args but false does not throw assertions"() { + when: + toRun.run() + + then: + noExceptionThrown() + + where: + toRun | expectedMessage + runnable({ Assert.assertFalse(false, "error %s", "arg1") }) | "error arg1" + runnable({ Assert.assertFalse(false, "error %s %s", "arg1", "arg2") }) | "error arg1 arg2" + runnable({ Assert.assertFalse(false, "error %s %s %s", "arg1", "arg2", "arg3") }) | "error arg1 arg2 arg3" + } + def "assertValidName should not throw on valid names"() { when: Assert.assertValidName(name) @@ -200,4 +287,10 @@ class AssertTest extends Specification { "���" | _ "_()" | _ } + + // Spock data tables cant cope with { x } syntax but it cna do this + Runnable runnable(Runnable r) { + return r + } + } diff --git a/src/test/java/benchmark/AssertBenchmark.java b/src/test/java/benchmark/AssertBenchmark.java new file mode 100644 index 0000000000..192b267280 --- /dev/null +++ b/src/test/java/benchmark/AssertBenchmark.java @@ -0,0 +1,90 @@ +package benchmark; + +import graphql.Assert; +import org.openjdk.jmh.annotations.Benchmark; +import org.openjdk.jmh.annotations.BenchmarkMode; +import org.openjdk.jmh.annotations.Fork; +import org.openjdk.jmh.annotations.Measurement; +import org.openjdk.jmh.annotations.Mode; +import org.openjdk.jmh.annotations.OutputTimeUnit; +import org.openjdk.jmh.annotations.Warmup; +import org.openjdk.jmh.runner.Runner; +import org.openjdk.jmh.runner.RunnerException; +import org.openjdk.jmh.runner.options.Options; +import org.openjdk.jmh.runner.options.OptionsBuilder; + +import java.util.Random; +import java.util.concurrent.TimeUnit; + +@Warmup(iterations = 2, time = 5, batchSize = 50) +@Measurement(iterations = 3, batchSize = 50) +@Fork(3) +public class AssertBenchmark { + + private static final int LOOPS = 100; + private static final Random random = new Random(); + + @Benchmark + @BenchmarkMode(Mode.Throughput) + @OutputTimeUnit(TimeUnit.MILLISECONDS) + public void benchMarkAssertWithString() { + for (int i = 0; i < LOOPS; i++) { + Assert.assertTrue(jitTrue(), "This string is constant"); + } + } + + @Benchmark + @BenchmarkMode(Mode.Throughput) + @OutputTimeUnit(TimeUnit.MILLISECONDS) + public void benchMarkAssertWithStringSupplier() { + for (int i = 0; i < LOOPS; i++) { + Assert.assertTrue(jitTrue(), () -> "This string is constant"); + } + } + + @Benchmark + @BenchmarkMode(Mode.Throughput) + @OutputTimeUnit(TimeUnit.MILLISECONDS) + public void benchMarkAssertWithStringSupplierFormatted() { + for (int i = 0; i < LOOPS; i++) { + final int captured = i; + Assert.assertTrue(jitTrue(), () -> String.format("This string is not constant %d", captured)); + } + } + + @Benchmark + @BenchmarkMode(Mode.Throughput) + @OutputTimeUnit(TimeUnit.MILLISECONDS) + public void benchMarkAssertWithStringFormatted() { + for (int i = 0; i < LOOPS; i++) { + Assert.assertTrue(jitTrue(), "This string is not constant %d", i); + } + } + + private boolean jitTrue() { + int i = random.nextInt(); + // can you jit this away, Mr JIT?? + return i % 10 < -1 || i * 2 + 1 < -2; + } + + public static void main(String[] args) throws RunnerException { + runAtStartup(); + Options opt = new OptionsBuilder() + .include("benchmark.AssertBenchmark") + .build(); + + new Runner(opt).run(); + } + + private static void runAtStartup() { + AssertBenchmark benchMark = new AssertBenchmark(); + BenchmarkUtils.runInToolingForSomeTimeThenExit( + () -> { + }, + benchMark::benchMarkAssertWithStringSupplier, + () -> { + } + + ); + } +} From 34adae60b104e98295051c6842fcfed663641d01 Mon Sep 17 00:00:00 2001 From: Brad Baker Date: Thu, 7 Mar 2024 21:46:52 +1100 Subject: [PATCH 3/6] put () -> back --- src/main/java/graphql/GraphqlErrorBuilder.java | 2 +- src/main/java/graphql/execution/Async.java | 6 +++--- src/main/java/graphql/execution/ExecutionStepInfo.java | 6 ++---- .../graphql/execution/ExecutionStrategyParameters.java | 6 +++--- .../graphql/execution/FieldCollectorParameters.java | 2 +- src/main/java/graphql/execution/FieldValueInfo.java | 2 +- src/main/java/graphql/execution/ResultPath.java | 10 +++++----- .../execution/SubscriptionExecutionStrategy.java | 2 +- .../graphql/execution/ValuesResolverConversion.java | 2 +- .../execution/reactive/NonBlockingMutexExecutor.java | 2 +- .../java/graphql/extensions/ExtensionsBuilder.java | 2 +- src/main/java/graphql/language/AbstractNode.java | 6 +++--- src/main/java/graphql/language/AstPrinter.java | 2 +- src/main/java/graphql/language/NodeParentTree.java | 4 ++-- src/main/java/graphql/language/PrettyAstPrinter.java | 2 +- .../graphql/normalized/ExecutableNormalizedField.java | 4 ++-- .../java/graphql/normalized/NormalizedInputValue.java | 2 +- src/main/java/graphql/schema/AsyncDataFetcher.java | 4 ++-- .../schema/DefaultGraphqlTypeComparatorRegistry.java | 6 +++--- src/main/java/graphql/schema/GraphQLTypeUtil.java | 2 +- src/main/java/graphql/schema/InputValueWithState.java | 2 +- 21 files changed, 37 insertions(+), 39 deletions(-) diff --git a/src/main/java/graphql/GraphqlErrorBuilder.java b/src/main/java/graphql/GraphqlErrorBuilder.java index c7281a5f53..ae4ad5095f 100644 --- a/src/main/java/graphql/GraphqlErrorBuilder.java +++ b/src/main/java/graphql/GraphqlErrorBuilder.java @@ -129,7 +129,7 @@ public B extensions(@Nullable Map extensions) { * @return a newly built GraphqlError */ public GraphQLError build() { - Assert.assertNotNull(message,"You must provide error message"); + Assert.assertNotNull(message,() -> "You must provide error message"); return new GraphqlErrorImpl(message, locations, errorType, path, extensions); } diff --git a/src/main/java/graphql/execution/Async.java b/src/main/java/graphql/execution/Async.java index f3eebc9e3e..498acaba6f 100644 --- a/src/main/java/graphql/execution/Async.java +++ b/src/main/java/graphql/execution/Async.java @@ -58,7 +58,7 @@ public void add(CompletableFuture completableFuture) { @Override public CompletableFuture> await() { - Assert.assertTrue(ix == 0, "expected size was 0"); + Assert.assertTrue(ix == 0, () -> "expected size was 0"); return typedEmpty(); } @@ -135,7 +135,7 @@ public static CompletableFuture> each(Collection list, Functio CompletableFuture cf; try { cf = cfFactory.apply(t); - Assert.assertNotNull(cf, "cfFactory must return a non null value"); + Assert.assertNotNull(cf, () -> "cfFactory must return a non null value"); } catch (Exception e) { cf = new CompletableFuture<>(); // Async.each makes sure that it is not a CompletionException inside a CompletionException @@ -160,7 +160,7 @@ private static void eachSequentiallyImpl(Iterator iterator, BiFunction CompletableFuture cf; try { cf = cfFactory.apply(iterator.next(), tmpResult); - Assert.assertNotNull(cf, "cfFactory must return a non null value"); + Assert.assertNotNull(cf, () -> "cfFactory must return a non null value"); } catch (Exception e) { cf = new CompletableFuture<>(); cf.completeExceptionally(new CompletionException(e)); diff --git a/src/main/java/graphql/execution/ExecutionStepInfo.java b/src/main/java/graphql/execution/ExecutionStepInfo.java index 42e0b01ebb..c7074a5c7f 100644 --- a/src/main/java/graphql/execution/ExecutionStepInfo.java +++ b/src/main/java/graphql/execution/ExecutionStepInfo.java @@ -14,8 +14,6 @@ import java.util.function.Consumer; import java.util.function.Supplier; -import static graphql.Assert.assertNotNull; -import static graphql.Assert.assertTrue; import static graphql.schema.GraphQLTypeUtil.isList; /** @@ -73,7 +71,7 @@ private ExecutionStepInfo(Builder builder) { this.field = builder.field; this.path = builder.path; this.parent = builder.parentInfo; - this.type = Assert.assertNotNull(builder.type, "you must provide a graphql type"); + this.type = Assert.assertNotNull(builder.type, () -> "you must provide a graphql type"); this.arguments = builder.arguments; this.fieldContainer = builder.fieldContainer; } @@ -203,7 +201,7 @@ public boolean hasParent() { * @return a new type info with the same */ public ExecutionStepInfo changeTypeWithPreservedNonNull(GraphQLOutputType newType) { - Assert.assertTrue(!GraphQLTypeUtil.isNonNull(newType), "newType can't be non null"); + Assert.assertTrue(!GraphQLTypeUtil.isNonNull(newType), () -> "newType can't be non null"); if (isNonNullType()) { return newExecutionStepInfo(this).type(GraphQLNonNull.nonNull(newType)).build(); } else { diff --git a/src/main/java/graphql/execution/ExecutionStrategyParameters.java b/src/main/java/graphql/execution/ExecutionStrategyParameters.java index 4df2b73605..915a4cfab6 100644 --- a/src/main/java/graphql/execution/ExecutionStrategyParameters.java +++ b/src/main/java/graphql/execution/ExecutionStrategyParameters.java @@ -33,9 +33,9 @@ private ExecutionStrategyParameters(ExecutionStepInfo executionStepInfo, ExecutionStrategyParameters parent, DeferredCallContext deferredCallContext) { - this.executionStepInfo = Assert.assertNotNull(executionStepInfo, "executionStepInfo is null"); + this.executionStepInfo = Assert.assertNotNull(executionStepInfo, () -> "executionStepInfo is null"); this.localContext = localContext; - this.fields = Assert.assertNotNull(fields, "fields is null"); + this.fields = Assert.assertNotNull(fields, () -> "fields is null"); this.source = source; this.nonNullableFieldValidator = nonNullableFieldValidator; this.path = path; @@ -168,7 +168,7 @@ public Builder localContext(Object localContext) { } public Builder nonNullFieldValidator(NonNullableFieldValidator nonNullableFieldValidator) { - this.nonNullableFieldValidator = Assert.assertNotNull(nonNullableFieldValidator, "requires a NonNullValidator"); + this.nonNullableFieldValidator = Assert.assertNotNull(nonNullableFieldValidator, () -> "requires a NonNullValidator"); return this; } diff --git a/src/main/java/graphql/execution/FieldCollectorParameters.java b/src/main/java/graphql/execution/FieldCollectorParameters.java index 75dafc1eff..c0a23404a7 100644 --- a/src/main/java/graphql/execution/FieldCollectorParameters.java +++ b/src/main/java/graphql/execution/FieldCollectorParameters.java @@ -92,7 +92,7 @@ public Builder variables(Map variables) { } public FieldCollectorParameters build() { - Assert.assertNotNull(graphQLSchema, "You must provide a schema"); + Assert.assertNotNull(graphQLSchema, () -> "You must provide a schema"); return new FieldCollectorParameters(this); } diff --git a/src/main/java/graphql/execution/FieldValueInfo.java b/src/main/java/graphql/execution/FieldValueInfo.java index a170ecb6e3..969ff4fd49 100644 --- a/src/main/java/graphql/execution/FieldValueInfo.java +++ b/src/main/java/graphql/execution/FieldValueInfo.java @@ -32,7 +32,7 @@ public enum CompleteValueType { } FieldValueInfo(CompleteValueType completeValueType, CompletableFuture fieldValue, List fieldValueInfos) { - Assert.assertNotNull(fieldValueInfos, "fieldValueInfos can't be null"); + Assert.assertNotNull(fieldValueInfos, () -> "fieldValueInfos can't be null"); this.completeValueType = completeValueType; this.fieldValue = fieldValue; this.fieldValueInfos = fieldValueInfos; diff --git a/src/main/java/graphql/execution/ResultPath.java b/src/main/java/graphql/execution/ResultPath.java index 9544ad57c7..28c37fcade 100644 --- a/src/main/java/graphql/execution/ResultPath.java +++ b/src/main/java/graphql/execution/ResultPath.java @@ -47,12 +47,12 @@ private ResultPath() { } private ResultPath(ResultPath parent, String segment) { - this.parent = Assert.assertNotNull(parent, "Must provide a parent path"); - this.segment = Assert.assertNotNull(segment, "Must provide a sub path"); + this.parent = Assert.assertNotNull(parent, () -> "Must provide a parent path"); + this.segment = Assert.assertNotNull(segment, () -> "Must provide a sub path"); } private ResultPath(ResultPath parent, int segment) { - this.parent = Assert.assertNotNull(parent, "Must provide a parent path"); + this.parent = Assert.assertNotNull(parent, () -> "Must provide a parent path"); this.segment = segment; } @@ -200,7 +200,7 @@ public ResultPath dropSegment() { * @return a new path with the last segment replaced */ public ResultPath replaceSegment(int segment) { - Assert.assertTrue(!ROOT_PATH.equals(this), "You MUST not call this with the root path"); + Assert.assertTrue(!ROOT_PATH.equals(this), () -> "You MUST not call this with the root path"); return new ResultPath(parent, segment); } @@ -212,7 +212,7 @@ public ResultPath replaceSegment(int segment) { * @return a new path with the last segment replaced */ public ResultPath replaceSegment(String segment) { - Assert.assertTrue(!ROOT_PATH.equals(this), "You MUST not call this with the root path"); + Assert.assertTrue(!ROOT_PATH.equals(this), () -> "You MUST not call this with the root path"); return new ResultPath(parent, segment); } diff --git a/src/main/java/graphql/execution/SubscriptionExecutionStrategy.java b/src/main/java/graphql/execution/SubscriptionExecutionStrategy.java index 16a91a942b..9385bab8c3 100644 --- a/src/main/java/graphql/execution/SubscriptionExecutionStrategy.java +++ b/src/main/java/graphql/execution/SubscriptionExecutionStrategy.java @@ -98,7 +98,7 @@ private CompletableFuture> createSourceEventStream(ExecutionCo return fieldFetched.thenApply(fetchedValue -> { Object publisher = fetchedValue.getFetchedValue(); if (publisher != null) { - Assert.assertTrue(publisher instanceof Publisher, "Your data fetcher must return a Publisher of events when using graphql subscriptions"); + Assert.assertTrue(publisher instanceof Publisher, () -> "Your data fetcher must return a Publisher of events when using graphql subscriptions"); } //noinspection unchecked return (Publisher) publisher; diff --git a/src/main/java/graphql/execution/ValuesResolverConversion.java b/src/main/java/graphql/execution/ValuesResolverConversion.java index a80a0a7358..3dbd929efc 100644 --- a/src/main/java/graphql/execution/ValuesResolverConversion.java +++ b/src/main/java/graphql/execution/ValuesResolverConversion.java @@ -283,7 +283,7 @@ private static Object externalValueToLiteralForObject( GraphQLContext graphqlContext, Locale locale ) { - Assert.assertTrue(inputValue instanceof Map, "Expect Map as input"); + Assert.assertTrue(inputValue instanceof Map, () -> "Expect Map as input"); Map inputMap = (Map) inputValue; List fieldDefinitions = fieldVisibility.getFieldDefinitions(inputObjectType); diff --git a/src/main/java/graphql/execution/reactive/NonBlockingMutexExecutor.java b/src/main/java/graphql/execution/reactive/NonBlockingMutexExecutor.java index 449e65f5d3..8f321d431a 100644 --- a/src/main/java/graphql/execution/reactive/NonBlockingMutexExecutor.java +++ b/src/main/java/graphql/execution/reactive/NonBlockingMutexExecutor.java @@ -38,7 +38,7 @@ class NonBlockingMutexExecutor implements Executor { @Override public void execute(final Runnable command) { - final RunNode newNode = new RunNode(Assert.assertNotNull(command, "Runnable must not be null")); + final RunNode newNode = new RunNode(Assert.assertNotNull(command, () -> "Runnable must not be null")); final RunNode prevLast = last.getAndSet(newNode); if (prevLast != null) prevLast.lazySet(newNode); diff --git a/src/main/java/graphql/extensions/ExtensionsBuilder.java b/src/main/java/graphql/extensions/ExtensionsBuilder.java index 8f731cf33b..bb28bf7472 100644 --- a/src/main/java/graphql/extensions/ExtensionsBuilder.java +++ b/src/main/java/graphql/extensions/ExtensionsBuilder.java @@ -108,7 +108,7 @@ public Map buildExtensions() { Map outMap = new LinkedHashMap<>(firstChange); for (int i = 1; i < changes.size(); i++) { Map newMap = extensionsMerger.merge(outMap, changes.get(i)); - Assert.assertNotNull(outMap, "You MUST provide a non null Map from ExtensionsMerger.merge()"); + Assert.assertNotNull(outMap, () -> "You MUST provide a non null Map from ExtensionsMerger.merge()"); outMap = newMap; } return outMap; diff --git a/src/main/java/graphql/language/AbstractNode.java b/src/main/java/graphql/language/AbstractNode.java index 736d23a98f..f097c9b491 100644 --- a/src/main/java/graphql/language/AbstractNode.java +++ b/src/main/java/graphql/language/AbstractNode.java @@ -25,9 +25,9 @@ public AbstractNode(SourceLocation sourceLocation, List comments, Ignor } public AbstractNode(SourceLocation sourceLocation, List comments, IgnoredChars ignoredChars, Map additionalData) { - Assert.assertNotNull(comments, "comments can't be null"); - Assert.assertNotNull(ignoredChars, "ignoredChars can't be null"); - Assert.assertNotNull(additionalData, "additionalData can't be null"); + Assert.assertNotNull(comments, () -> "comments can't be null"); + Assert.assertNotNull(ignoredChars, () -> "ignoredChars can't be null"); + Assert.assertNotNull(additionalData, () -> "additionalData can't be null"); this.sourceLocation = sourceLocation; this.additionalData = ImmutableMap.copyOf(additionalData); diff --git a/src/main/java/graphql/language/AstPrinter.java b/src/main/java/graphql/language/AstPrinter.java index e032c52c3b..1d1e717b73 100644 --- a/src/main/java/graphql/language/AstPrinter.java +++ b/src/main/java/graphql/language/AstPrinter.java @@ -457,7 +457,7 @@ private String node(Node node) { private String node(Node node, Class startClass) { if (startClass != null) { - Assert.assertTrue(startClass.isInstance(node), "The starting class must be in the inherit tree"); + Assert.assertTrue(startClass.isInstance(node), () -> "The starting class must be in the inherit tree"); } StringBuilder builder = new StringBuilder(); NodePrinter printer = _findPrinter(node, startClass); diff --git a/src/main/java/graphql/language/NodeParentTree.java b/src/main/java/graphql/language/NodeParentTree.java index 616efadacc..da2dbfaacc 100644 --- a/src/main/java/graphql/language/NodeParentTree.java +++ b/src/main/java/graphql/language/NodeParentTree.java @@ -29,8 +29,8 @@ public class NodeParentTree { @Internal public NodeParentTree(Deque nodeStack) { - Assert.assertNotNull(nodeStack, "You MUST have a non null stack of nodes"); - Assert.assertTrue(!nodeStack.isEmpty(), "You MUST have a non empty stack of nodes"); + Assert.assertNotNull(nodeStack, () -> "You MUST have a non null stack of nodes"); + Assert.assertTrue(!nodeStack.isEmpty(), () -> "You MUST have a non empty stack of nodes"); Deque copy = new ArrayDeque<>(nodeStack); path = mkPath(copy); diff --git a/src/main/java/graphql/language/PrettyAstPrinter.java b/src/main/java/graphql/language/PrettyAstPrinter.java index 2a73b6cd8b..6ba895be98 100644 --- a/src/main/java/graphql/language/PrettyAstPrinter.java +++ b/src/main/java/graphql/language/PrettyAstPrinter.java @@ -221,7 +221,7 @@ private NodePrinter unionTypeDefinition(String nodeName) { private String node(Node node, Class startClass) { if (startClass != null) { - Assert.assertTrue(startClass.isInstance(node), "The starting class must be in the inherit tree"); + Assert.assertTrue(startClass.isInstance(node), () -> "The starting class must be in the inherit tree"); } StringBuilder builder = new StringBuilder(); diff --git a/src/main/java/graphql/normalized/ExecutableNormalizedField.java b/src/main/java/graphql/normalized/ExecutableNormalizedField.java index 01178b8160..c979d4a649 100644 --- a/src/main/java/graphql/normalized/ExecutableNormalizedField.java +++ b/src/main/java/graphql/normalized/ExecutableNormalizedField.java @@ -184,7 +184,7 @@ public boolean hasChildren() { public GraphQLOutputType getType(GraphQLSchema schema) { List fieldDefinitions = getFieldDefinitions(schema); Set fieldTypes = fieldDefinitions.stream().map(fd -> simplePrint(fd.getType())).collect(toSet()); - Assert.assertTrue(fieldTypes.size() == 1, "More than one type ... use getTypes"); + Assert.assertTrue(fieldTypes.size() == 1, () -> "More than one type ... use getTypes"); return fieldDefinitions.get(0).getType(); } @@ -438,7 +438,7 @@ public List getChildrenWithSameResultKey(String resul public List getChildren(int includingRelativeLevel) { List result = new ArrayList<>(); - Assert.assertTrue(includingRelativeLevel >= 1, "relative level must be >= 1"); + Assert.assertTrue(includingRelativeLevel >= 1, () -> "relative level must be >= 1"); this.getChildren().forEach(child -> { traverseImpl(child, result::add, 1, includingRelativeLevel); diff --git a/src/main/java/graphql/normalized/NormalizedInputValue.java b/src/main/java/graphql/normalized/NormalizedInputValue.java index 181e8594b1..1593d07135 100644 --- a/src/main/java/graphql/normalized/NormalizedInputValue.java +++ b/src/main/java/graphql/normalized/NormalizedInputValue.java @@ -101,7 +101,7 @@ private boolean isListOnly(String typeName) { private String unwrapOne(String typeName) { assertNotNull(typeName); - Assert.assertTrue(typeName.trim().length() > 0, "We have an empty type name unwrapped"); + Assert.assertTrue(typeName.trim().length() > 0, () -> "We have an empty type name unwrapped"); if (typeName.endsWith("!")) { return typeName.substring(0, typeName.length() - 1); } diff --git a/src/main/java/graphql/schema/AsyncDataFetcher.java b/src/main/java/graphql/schema/AsyncDataFetcher.java index fc405fe4b6..6fd93fae42 100644 --- a/src/main/java/graphql/schema/AsyncDataFetcher.java +++ b/src/main/java/graphql/schema/AsyncDataFetcher.java @@ -68,8 +68,8 @@ public AsyncDataFetcher(DataFetcher wrappedDataFetcher) { } public AsyncDataFetcher(DataFetcher wrappedDataFetcher, Executor executor) { - this.wrappedDataFetcher = Assert.assertNotNull(wrappedDataFetcher, "wrappedDataFetcher can't be null"); - this.executor = Assert.assertNotNull(executor, "executor can't be null"); + this.wrappedDataFetcher = Assert.assertNotNull(wrappedDataFetcher, () -> "wrappedDataFetcher can't be null"); + this.executor = Assert.assertNotNull(executor, () -> "executor can't be null"); } @Override diff --git a/src/main/java/graphql/schema/DefaultGraphqlTypeComparatorRegistry.java b/src/main/java/graphql/schema/DefaultGraphqlTypeComparatorRegistry.java index c945ddf3c9..86b832109b 100644 --- a/src/main/java/graphql/schema/DefaultGraphqlTypeComparatorRegistry.java +++ b/src/main/java/graphql/schema/DefaultGraphqlTypeComparatorRegistry.java @@ -127,9 +127,9 @@ public static class Builder { * @return The {@code Builder} instance to allow chaining. */ public Builder addComparator(GraphqlTypeComparatorEnvironment environment, Class comparatorClass, Comparator comparator) { - Assert.assertNotNull(environment, "environment can't be null"); - Assert.assertNotNull(comparatorClass, "comparatorClass can't be null"); - Assert.assertNotNull(comparator, "comparator can't be null"); + Assert.assertNotNull(environment, () -> "environment can't be null"); + Assert.assertNotNull(comparatorClass, () -> "comparatorClass can't be null"); + Assert.assertNotNull(comparator, () -> "comparator can't be null"); registry.put(environment, comparator); return this; } diff --git a/src/main/java/graphql/schema/GraphQLTypeUtil.java b/src/main/java/graphql/schema/GraphQLTypeUtil.java index f33164937b..de5cd5e8b7 100644 --- a/src/main/java/graphql/schema/GraphQLTypeUtil.java +++ b/src/main/java/graphql/schema/GraphQLTypeUtil.java @@ -26,7 +26,7 @@ public class GraphQLTypeUtil { * @return the type in graphql SDL format, eg [typeName!]! */ public static String simplePrint(GraphQLType type) { - Assert.assertNotNull(type, "type can't be null"); + Assert.assertNotNull(type, () -> "type can't be null"); if (isNonNull(type)) { return simplePrint(unwrapOne(type)) + "!"; } else if (isList(type)) { diff --git a/src/main/java/graphql/schema/InputValueWithState.java b/src/main/java/graphql/schema/InputValueWithState.java index 0a8ed45f0a..46ed2ec134 100644 --- a/src/main/java/graphql/schema/InputValueWithState.java +++ b/src/main/java/graphql/schema/InputValueWithState.java @@ -46,7 +46,7 @@ private InputValueWithState(State state, Object value) { public static final InputValueWithState NOT_SET = new InputValueWithState(State.NOT_SET, null); public static InputValueWithState newLiteralValue(@NotNull Value value) { - Assert.assertNotNull(value, "value literal can't be null"); + Assert.assertNotNull(value, () -> "value literal can't be null"); return new InputValueWithState(State.LITERAL, value); } From 0449ee9c211f38f58de3b59fe3c439348d0e29cc Mon Sep 17 00:00:00 2001 From: Brad Baker Date: Thu, 7 Mar 2024 22:16:44 +1100 Subject: [PATCH 4/6] reverting back to pre code change --- .../java/graphql/GraphqlErrorBuilder.java | 3 +-- src/main/java/graphql/execution/Async.java | 8 +++--- .../graphql/execution/ExecutionStepInfo.java | 7 +++--- .../ExecutionStrategyParameters.java | 7 +++--- .../graphql/execution/FieldValueInfo.java | 3 +-- .../java/graphql/execution/ResultPath.java | 23 ++++++++++------- .../SubscriptionExecutionStrategy.java | 3 +-- .../execution/ValuesResolverConversion.java | 6 ++--- .../reactive/NonBlockingMutexExecutor.java | 25 ++++++++++--------- .../graphql/extensions/ExtensionsBuilder.java | 4 +-- .../java/graphql/language/AstPrinter.java | 11 ++++---- .../java/graphql/language/NodeParentTree.java | 5 ++-- .../graphql/language/PrettyAstPrinter.java | 7 +++--- .../normalized/ExecutableNormalizedField.java | 6 ++--- .../java/graphql/schema/AsyncDataFetcher.java | 5 ++-- .../DefaultGraphqlTypeComparatorRegistry.java | 8 +++--- .../graphql/schema/InputValueWithState.java | 3 +-- 17 files changed, 65 insertions(+), 69 deletions(-) diff --git a/src/main/java/graphql/GraphqlErrorBuilder.java b/src/main/java/graphql/GraphqlErrorBuilder.java index ae4ad5095f..4cef5beabe 100644 --- a/src/main/java/graphql/GraphqlErrorBuilder.java +++ b/src/main/java/graphql/GraphqlErrorBuilder.java @@ -10,7 +10,6 @@ import java.util.List; import java.util.Map; -import static graphql.Assert.assertNotNull; import static graphql.Assert.assertNotNull; /** @@ -129,7 +128,7 @@ public B extensions(@Nullable Map extensions) { * @return a newly built GraphqlError */ public GraphQLError build() { - Assert.assertNotNull(message,() -> "You must provide error message"); + assertNotNull(message, () -> "You must provide error message"); return new GraphqlErrorImpl(message, locations, errorType, path, extensions); } diff --git a/src/main/java/graphql/execution/Async.java b/src/main/java/graphql/execution/Async.java index 498acaba6f..b1f1dc36d4 100644 --- a/src/main/java/graphql/execution/Async.java +++ b/src/main/java/graphql/execution/Async.java @@ -17,6 +17,8 @@ import java.util.function.Function; import java.util.function.Supplier; +import static graphql.Assert.assertTrue; + @Internal @SuppressWarnings("FutureReturnValueIgnored") public class Async { @@ -58,7 +60,7 @@ public void add(CompletableFuture completableFuture) { @Override public CompletableFuture> await() { - Assert.assertTrue(ix == 0, () -> "expected size was 0"); + assertTrue(ix == 0, "expected size was 0 got %d", ix); return typedEmpty(); } @@ -86,7 +88,7 @@ public void add(CompletableFuture completableFuture) { @Override public CompletableFuture> await() { - Assert.assertTrue(ix == 1, "expected size was 1 got %d", ix); + assertTrue(ix == 1, "expected size was 1 got %d", ix); return completableFuture.thenApply(Collections::singletonList); } } @@ -109,7 +111,7 @@ public void add(CompletableFuture completableFuture) { @Override public CompletableFuture> await() { - Assert.assertTrue(ix == array.length, "expected size was %d got %d", array.length, ix); + assertTrue(ix == array.length, "expected size was %d got %d", array.length, ix); CompletableFuture> overallResult = new CompletableFuture<>(); CompletableFuture.allOf(array) diff --git a/src/main/java/graphql/execution/ExecutionStepInfo.java b/src/main/java/graphql/execution/ExecutionStepInfo.java index c7074a5c7f..08836d977f 100644 --- a/src/main/java/graphql/execution/ExecutionStepInfo.java +++ b/src/main/java/graphql/execution/ExecutionStepInfo.java @@ -1,6 +1,5 @@ package graphql.execution; -import graphql.Assert; import graphql.PublicApi; import graphql.collect.ImmutableMapWithNullValues; import graphql.schema.GraphQLFieldDefinition; @@ -14,6 +13,8 @@ import java.util.function.Consumer; import java.util.function.Supplier; +import static graphql.Assert.assertNotNull; +import static graphql.Assert.assertTrue; import static graphql.schema.GraphQLTypeUtil.isList; /** @@ -71,7 +72,7 @@ private ExecutionStepInfo(Builder builder) { this.field = builder.field; this.path = builder.path; this.parent = builder.parentInfo; - this.type = Assert.assertNotNull(builder.type, () -> "you must provide a graphql type"); + this.type = assertNotNull(builder.type, () -> "you must provide a graphql type"); this.arguments = builder.arguments; this.fieldContainer = builder.fieldContainer; } @@ -201,7 +202,7 @@ public boolean hasParent() { * @return a new type info with the same */ public ExecutionStepInfo changeTypeWithPreservedNonNull(GraphQLOutputType newType) { - Assert.assertTrue(!GraphQLTypeUtil.isNonNull(newType), () -> "newType can't be non null"); + assertTrue(!GraphQLTypeUtil.isNonNull(newType), () -> "newType can't be non null"); if (isNonNullType()) { return newExecutionStepInfo(this).type(GraphQLNonNull.nonNull(newType)).build(); } else { diff --git a/src/main/java/graphql/execution/ExecutionStrategyParameters.java b/src/main/java/graphql/execution/ExecutionStrategyParameters.java index 915a4cfab6..e1df7bb363 100644 --- a/src/main/java/graphql/execution/ExecutionStrategyParameters.java +++ b/src/main/java/graphql/execution/ExecutionStrategyParameters.java @@ -1,6 +1,5 @@ package graphql.execution; -import graphql.Assert; import graphql.PublicApi; import graphql.execution.incremental.DeferredCallContext; @@ -33,9 +32,9 @@ private ExecutionStrategyParameters(ExecutionStepInfo executionStepInfo, ExecutionStrategyParameters parent, DeferredCallContext deferredCallContext) { - this.executionStepInfo = Assert.assertNotNull(executionStepInfo, () -> "executionStepInfo is null"); + this.executionStepInfo = assertNotNull(executionStepInfo, () -> "executionStepInfo is null"); this.localContext = localContext; - this.fields = Assert.assertNotNull(fields, () -> "fields is null"); + this.fields = assertNotNull(fields, () -> "fields is null"); this.source = source; this.nonNullableFieldValidator = nonNullableFieldValidator; this.path = path; @@ -168,7 +167,7 @@ public Builder localContext(Object localContext) { } public Builder nonNullFieldValidator(NonNullableFieldValidator nonNullableFieldValidator) { - this.nonNullableFieldValidator = Assert.assertNotNull(nonNullableFieldValidator, () -> "requires a NonNullValidator"); + this.nonNullableFieldValidator = assertNotNull(nonNullableFieldValidator, () -> "requires a NonNullValidator"); return this; } diff --git a/src/main/java/graphql/execution/FieldValueInfo.java b/src/main/java/graphql/execution/FieldValueInfo.java index 969ff4fd49..c13e915936 100644 --- a/src/main/java/graphql/execution/FieldValueInfo.java +++ b/src/main/java/graphql/execution/FieldValueInfo.java @@ -1,7 +1,6 @@ package graphql.execution; import com.google.common.collect.ImmutableList; -import graphql.Assert; import graphql.ExecutionResult; import graphql.ExecutionResultImpl; import graphql.PublicApi; @@ -32,7 +31,7 @@ public enum CompleteValueType { } FieldValueInfo(CompleteValueType completeValueType, CompletableFuture fieldValue, List fieldValueInfos) { - Assert.assertNotNull(fieldValueInfos, () -> "fieldValueInfos can't be null"); + assertNotNull(fieldValueInfos, () -> "fieldValueInfos can't be null"); this.completeValueType = completeValueType; this.fieldValue = fieldValue; this.fieldValueInfos = fieldValueInfos; diff --git a/src/main/java/graphql/execution/ResultPath.java b/src/main/java/graphql/execution/ResultPath.java index 28c37fcade..472b5fa529 100644 --- a/src/main/java/graphql/execution/ResultPath.java +++ b/src/main/java/graphql/execution/ResultPath.java @@ -1,7 +1,6 @@ package graphql.execution; import com.google.common.collect.ImmutableList; -import graphql.Assert; import graphql.AssertException; import graphql.PublicApi; import graphql.collect.ImmutableKit; @@ -12,7 +11,6 @@ import java.util.Objects; import java.util.StringTokenizer; -import static graphql.Assert.assertNotNull; import static graphql.Assert.assertNotNull; import static graphql.Assert.assertTrue; import static java.lang.String.format; @@ -47,12 +45,12 @@ private ResultPath() { } private ResultPath(ResultPath parent, String segment) { - this.parent = Assert.assertNotNull(parent, () -> "Must provide a parent path"); - this.segment = Assert.assertNotNull(segment, () -> "Must provide a sub path"); + this.parent = assertNotNull(parent, () -> "Must provide a parent path"); + this.segment = assertNotNull(segment, () -> "Must provide a sub path"); } private ResultPath(ResultPath parent, int segment) { - this.parent = Assert.assertNotNull(parent, () -> "Must provide a parent path"); + this.parent = assertNotNull(parent, () -> "Must provide a parent path"); this.segment = segment; } @@ -113,6 +111,7 @@ public ResultPath getParent() { * Parses an execution path from the provided path string in the format /segment1/segment2[index]/segmentN * * @param pathString the path string + * * @return a parsed execution path */ public static ResultPath parse(String pathString) { @@ -141,6 +140,7 @@ public static ResultPath parse(String pathString) { * This will create an execution path from the list of objects * * @param objects the path objects + * * @return a new execution path */ public static ResultPath fromList(List objects) { @@ -164,6 +164,7 @@ private static String mkErrMsg() { * Takes the current path and adds a new segment to it, returning a new path * * @param segment the string path segment to add + * * @return a new path containing that segment */ public ResultPath segment(String segment) { @@ -174,6 +175,7 @@ public ResultPath segment(String segment) { * Takes the current path and adds a new segment to it, returning a new path * * @param segment the int path segment to add + * * @return a new path containing that segment */ public ResultPath segment(int segment) { @@ -197,10 +199,11 @@ public ResultPath dropSegment() { * equals "/a/b[9]" * * @param segment the integer segment to use + * * @return a new path with the last segment replaced */ public ResultPath replaceSegment(int segment) { - Assert.assertTrue(!ROOT_PATH.equals(this), () -> "You MUST not call this with the root path"); + assertTrue(!ROOT_PATH.equals(this), () -> "You MUST not call this with the root path"); return new ResultPath(parent, segment); } @@ -209,10 +212,11 @@ public ResultPath replaceSegment(int segment) { * equals "/a/b/x" * * @param segment the string segment to use + * * @return a new path with the last segment replaced */ public ResultPath replaceSegment(String segment) { - Assert.assertTrue(!ROOT_PATH.equals(this), () -> "You MUST not call this with the root path"); + assertTrue(!ROOT_PATH.equals(this), () -> "You MUST not call this with the root path"); return new ResultPath(parent, segment); } @@ -228,6 +232,7 @@ public boolean isRootPath() { * Appends the provided path to the current one * * @param path the path to append + * * @return a new path */ public ResultPath append(ResultPath path) { @@ -238,12 +243,12 @@ public ResultPath append(ResultPath path) { public ResultPath sibling(String siblingField) { - Assert.assertTrue(!ROOT_PATH.equals(this), "You MUST not call this with the root path"); + assertTrue(!ROOT_PATH.equals(this), "You MUST not call this with the root path"); return new ResultPath(this.parent, siblingField); } public ResultPath sibling(int siblingField) { - Assert.assertTrue(!ROOT_PATH.equals(this), "You MUST not call this with the root path"); + assertTrue(!ROOT_PATH.equals(this), "You MUST not call this with the root path"); return new ResultPath(this.parent, siblingField); } diff --git a/src/main/java/graphql/execution/SubscriptionExecutionStrategy.java b/src/main/java/graphql/execution/SubscriptionExecutionStrategy.java index 9385bab8c3..f735b0dc6a 100644 --- a/src/main/java/graphql/execution/SubscriptionExecutionStrategy.java +++ b/src/main/java/graphql/execution/SubscriptionExecutionStrategy.java @@ -1,6 +1,5 @@ package graphql.execution; -import graphql.Assert; import graphql.ExecutionResult; import graphql.ExecutionResultImpl; import graphql.PublicApi; @@ -98,7 +97,7 @@ private CompletableFuture> createSourceEventStream(ExecutionCo return fieldFetched.thenApply(fetchedValue -> { Object publisher = fetchedValue.getFetchedValue(); if (publisher != null) { - Assert.assertTrue(publisher instanceof Publisher, () -> "Your data fetcher must return a Publisher of events when using graphql subscriptions"); + assertTrue(publisher instanceof Publisher, () -> "Your data fetcher must return a Publisher of events when using graphql subscriptions"); } //noinspection unchecked return (Publisher) publisher; diff --git a/src/main/java/graphql/execution/ValuesResolverConversion.java b/src/main/java/graphql/execution/ValuesResolverConversion.java index 3dbd929efc..eff4d1cef1 100644 --- a/src/main/java/graphql/execution/ValuesResolverConversion.java +++ b/src/main/java/graphql/execution/ValuesResolverConversion.java @@ -1,7 +1,6 @@ package graphql.execution; import com.google.common.collect.ImmutableList; -import graphql.Assert; import graphql.GraphQLContext; import graphql.Internal; import graphql.execution.values.InputInterceptor; @@ -39,7 +38,6 @@ import static graphql.Assert.assertShouldNeverHappen; import static graphql.Assert.assertTrue; -import static graphql.Assert.assertTrue; import static graphql.collect.ImmutableKit.emptyList; import static graphql.collect.ImmutableKit.map; import static graphql.execution.ValuesResolver.ValueMode.NORMALIZED; @@ -106,6 +104,7 @@ static Object valueToLiteralImpl(GraphqlFieldVisibility fieldVisibility, * @param type the type of input value * @param graphqlContext the GraphqlContext to use * @param locale the Locale to use + * * @return a value converted to an internal value */ static Object externalValueToInternalValue(GraphqlFieldVisibility fieldVisibility, @@ -283,7 +282,7 @@ private static Object externalValueToLiteralForObject( GraphQLContext graphqlContext, Locale locale ) { - Assert.assertTrue(inputValue instanceof Map, () -> "Expect Map as input"); + assertTrue(inputValue instanceof Map, () -> "Expect Map as input"); Map inputMap = (Map) inputValue; List fieldDefinitions = fieldVisibility.getFieldDefinitions(inputObjectType); @@ -597,6 +596,7 @@ private static List externalValueToInternalValueForList( * @param coercedVariables the coerced variable values * @param graphqlContext the GraphqlContext to use * @param locale the Locale to use + * * @return literal converted to an internal value */ static Object literalToInternalValue( diff --git a/src/main/java/graphql/execution/reactive/NonBlockingMutexExecutor.java b/src/main/java/graphql/execution/reactive/NonBlockingMutexExecutor.java index 8f321d431a..38c6eb1238 100644 --- a/src/main/java/graphql/execution/reactive/NonBlockingMutexExecutor.java +++ b/src/main/java/graphql/execution/reactive/NonBlockingMutexExecutor.java @@ -1,8 +1,8 @@ package graphql.execution.reactive; -import graphql.Assert; import graphql.Internal; +import org.jetbrains.annotations.NotNull; import java.util.concurrent.Executor; import java.util.concurrent.atomic.AtomicReference; @@ -13,37 +13,38 @@ /** * Executor that provides mutual exclusion between the operations submitted to it, * without blocking. - * + *

* If an operation is submitted to this executor while no other operation is * running, it will run immediately. - * + *

* If an operation is submitted to this executor while another operation is * running, it will be added to a queue of operations to run, and the executor will * return. The thread currently running an operation will end up running the * operation just submitted. - * + *

* Operations submitted to this executor should run fast, as they can end up running * on other threads and interfere with the operation of other threads. - * + *

* This executor can also be used to address infinite recursion problems, as * operations submitted recursively will run sequentially. + *

* - * - * Inspired by Public Domain CC0 code at h - * https://github.com/jroper/reactive-streams-servlet/tree/master/reactive-streams-servlet/src/main/java/org/reactivestreams/servlet + * Inspired by Public Domain CC0 code at + * ... */ @Internal class NonBlockingMutexExecutor implements Executor { private final AtomicReference last = new AtomicReference<>(); @Override - public void execute(final Runnable command) { - final RunNode newNode = new RunNode(Assert.assertNotNull(command, () -> "Runnable must not be null")); + public void execute(final @NotNull Runnable command) { + final RunNode newNode = new RunNode(assertNotNull(command, () -> "Runnable must not be null")); final RunNode prevLast = last.getAndSet(newNode); - if (prevLast != null) + if (prevLast != null) { prevLast.lazySet(newNode); - else + } else { runAll(newNode); + } } private void reportFailure(final Thread runner, final Throwable thrown) { diff --git a/src/main/java/graphql/extensions/ExtensionsBuilder.java b/src/main/java/graphql/extensions/ExtensionsBuilder.java index bb28bf7472..fe37c64743 100644 --- a/src/main/java/graphql/extensions/ExtensionsBuilder.java +++ b/src/main/java/graphql/extensions/ExtensionsBuilder.java @@ -1,7 +1,6 @@ package graphql.extensions; import com.google.common.collect.ImmutableMap; -import graphql.Assert; import graphql.ExecutionResult; import graphql.PublicApi; import org.jetbrains.annotations.NotNull; @@ -13,7 +12,6 @@ import java.util.Map; import java.util.concurrent.CopyOnWriteArrayList; -import static graphql.Assert.assertNotNull; import static graphql.Assert.assertNotNull; /** @@ -108,7 +106,7 @@ public Map buildExtensions() { Map outMap = new LinkedHashMap<>(firstChange); for (int i = 1; i < changes.size(); i++) { Map newMap = extensionsMerger.merge(outMap, changes.get(i)); - Assert.assertNotNull(outMap, () -> "You MUST provide a non null Map from ExtensionsMerger.merge()"); + assertNotNull(outMap, () -> "You MUST provide a non null Map from ExtensionsMerger.merge()"); outMap = newMap; } return outMap; diff --git a/src/main/java/graphql/language/AstPrinter.java b/src/main/java/graphql/language/AstPrinter.java index 1d1e717b73..b48fa2075d 100644 --- a/src/main/java/graphql/language/AstPrinter.java +++ b/src/main/java/graphql/language/AstPrinter.java @@ -1,7 +1,5 @@ package graphql.language; -import graphql.Assert; -import graphql.AssertException; import graphql.PublicApi; import graphql.collect.ImmutableKit; @@ -457,7 +455,7 @@ private String node(Node node) { private String node(Node node, Class startClass) { if (startClass != null) { - Assert.assertTrue(startClass.isInstance(node), () -> "The starting class must be in the inherit tree"); + assertTrue(startClass.isInstance(node), () -> "The starting class must be in the inherit tree"); } StringBuilder builder = new StringBuilder(); NodePrinter printer = _findPrinter(node, startClass); @@ -543,7 +541,7 @@ private String description(Node node) { } private String directives(List directives) { - return join(nvl(directives), compactMode? "" : " "); + return join(nvl(directives), compactMode ? "" : " "); } private String join(List nodes, String delim) { @@ -566,7 +564,7 @@ private String joinTight(List nodes, String delim, String pr first = false; } else { boolean canButtTogether = lastNodeText.endsWith("}"); - if (! canButtTogether) { + if (!canButtTogether) { joined.append(delim); } } @@ -706,7 +704,8 @@ interface NodePrinter { /** * Allow subclasses to replace a printer for a specific {@link Node} - * @param nodeClass the class of the {@link Node} + * + * @param nodeClass the class of the {@link Node} * @param nodePrinter the custom {@link NodePrinter} */ void replacePrinter(Class nodeClass, NodePrinter nodePrinter) { diff --git a/src/main/java/graphql/language/NodeParentTree.java b/src/main/java/graphql/language/NodeParentTree.java index da2dbfaacc..fc78ea093d 100644 --- a/src/main/java/graphql/language/NodeParentTree.java +++ b/src/main/java/graphql/language/NodeParentTree.java @@ -1,7 +1,6 @@ package graphql.language; import com.google.common.collect.ImmutableList; -import graphql.Assert; import graphql.Internal; import graphql.PublicApi; @@ -29,8 +28,8 @@ public class NodeParentTree { @Internal public NodeParentTree(Deque nodeStack) { - Assert.assertNotNull(nodeStack, () -> "You MUST have a non null stack of nodes"); - Assert.assertTrue(!nodeStack.isEmpty(), () -> "You MUST have a non empty stack of nodes"); + assertNotNull(nodeStack, () -> "You MUST have a non null stack of nodes"); + assertTrue(!nodeStack.isEmpty(), () -> "You MUST have a non empty stack of nodes"); Deque copy = new ArrayDeque<>(nodeStack); path = mkPath(copy); diff --git a/src/main/java/graphql/language/PrettyAstPrinter.java b/src/main/java/graphql/language/PrettyAstPrinter.java index 6ba895be98..c763a7b93d 100644 --- a/src/main/java/graphql/language/PrettyAstPrinter.java +++ b/src/main/java/graphql/language/PrettyAstPrinter.java @@ -1,6 +1,5 @@ package graphql.language; -import graphql.Assert; import graphql.ExperimentalApi; import graphql.collect.ImmutableKit; import graphql.parser.CommentParser; @@ -221,7 +220,7 @@ private NodePrinter unionTypeDefinition(String nodeName) { private String node(Node node, Class startClass) { if (startClass != null) { - Assert.assertTrue(startClass.isInstance(node), () -> "The starting class must be in the inherit tree"); + assertTrue(startClass.isInstance(node), () -> "The starting class must be in the inherit tree"); } StringBuilder builder = new StringBuilder(); @@ -409,10 +408,10 @@ public static class PrettyPrinterOptions { private static final PrettyPrinterOptions defaultOptions = new PrettyPrinterOptions(IndentType.SPACE, 2); private PrettyPrinterOptions(IndentType indentType, int indentWidth) { - this.indentText = String.join("", Collections.nCopies(indentWidth, indentType.character)); + this.indentText = String.join("", Collections.nCopies(indentWidth, indentType.character)); } - public static PrettyPrinterOptions defaultOptions() { + public static PrettyPrinterOptions defaultOptions() { return defaultOptions; } diff --git a/src/main/java/graphql/normalized/ExecutableNormalizedField.java b/src/main/java/graphql/normalized/ExecutableNormalizedField.java index c979d4a649..f9db04b00a 100644 --- a/src/main/java/graphql/normalized/ExecutableNormalizedField.java +++ b/src/main/java/graphql/normalized/ExecutableNormalizedField.java @@ -2,7 +2,6 @@ import com.google.common.collect.ImmutableList; import com.google.common.collect.ImmutableMap; -import graphql.Assert; import graphql.ExperimentalApi; import graphql.Internal; import graphql.Mutable; @@ -184,7 +183,7 @@ public boolean hasChildren() { public GraphQLOutputType getType(GraphQLSchema schema) { List fieldDefinitions = getFieldDefinitions(schema); Set fieldTypes = fieldDefinitions.stream().map(fd -> simplePrint(fd.getType())).collect(toSet()); - Assert.assertTrue(fieldTypes.size() == 1, () -> "More than one type ... use getTypes"); + assertTrue(fieldTypes.size() == 1, () -> "More than one type ... use getTypes"); return fieldDefinitions.get(0).getType(); } @@ -438,7 +437,7 @@ public List getChildrenWithSameResultKey(String resul public List getChildren(int includingRelativeLevel) { List result = new ArrayList<>(); - Assert.assertTrue(includingRelativeLevel >= 1, () -> "relative level must be >= 1"); + assertTrue(includingRelativeLevel >= 1, () -> "relative level must be >= 1"); this.getChildren().forEach(child -> { traverseImpl(child, result::add, 1, includingRelativeLevel); @@ -478,6 +477,7 @@ public ExecutableNormalizedField getParent() { /** * @return the {@link NormalizedDeferredExecution}s associated with this {@link ExecutableNormalizedField}. + * * @see NormalizedDeferredExecution */ @ExperimentalApi diff --git a/src/main/java/graphql/schema/AsyncDataFetcher.java b/src/main/java/graphql/schema/AsyncDataFetcher.java index 6fd93fae42..b07aa80ddb 100644 --- a/src/main/java/graphql/schema/AsyncDataFetcher.java +++ b/src/main/java/graphql/schema/AsyncDataFetcher.java @@ -1,6 +1,5 @@ package graphql.schema; -import graphql.Assert; import graphql.PublicApi; import java.util.concurrent.CompletableFuture; @@ -68,8 +67,8 @@ public AsyncDataFetcher(DataFetcher wrappedDataFetcher) { } public AsyncDataFetcher(DataFetcher wrappedDataFetcher, Executor executor) { - this.wrappedDataFetcher = Assert.assertNotNull(wrappedDataFetcher, () -> "wrappedDataFetcher can't be null"); - this.executor = Assert.assertNotNull(executor, () -> "executor can't be null"); + this.wrappedDataFetcher = assertNotNull(wrappedDataFetcher, () -> "wrappedDataFetcher can't be null"); + this.executor = assertNotNull(executor, () -> "executor can't be null"); } @Override diff --git a/src/main/java/graphql/schema/DefaultGraphqlTypeComparatorRegistry.java b/src/main/java/graphql/schema/DefaultGraphqlTypeComparatorRegistry.java index 86b832109b..3474dd5b7c 100644 --- a/src/main/java/graphql/schema/DefaultGraphqlTypeComparatorRegistry.java +++ b/src/main/java/graphql/schema/DefaultGraphqlTypeComparatorRegistry.java @@ -1,7 +1,6 @@ package graphql.schema; import com.google.common.collect.ImmutableMap; -import graphql.Assert; import graphql.PublicApi; import java.util.Comparator; @@ -10,7 +9,6 @@ import java.util.Objects; import java.util.function.UnaryOperator; -import static graphql.Assert.assertNotNull; import static graphql.Assert.assertNotNull; import static graphql.schema.GraphQLTypeUtil.unwrapAll; import static graphql.schema.GraphqlTypeComparatorEnvironment.newEnvironment; @@ -127,9 +125,9 @@ public static class Builder { * @return The {@code Builder} instance to allow chaining. */ public Builder addComparator(GraphqlTypeComparatorEnvironment environment, Class comparatorClass, Comparator comparator) { - Assert.assertNotNull(environment, () -> "environment can't be null"); - Assert.assertNotNull(comparatorClass, () -> "comparatorClass can't be null"); - Assert.assertNotNull(comparator, () -> "comparator can't be null"); + assertNotNull(environment, () -> "environment can't be null"); + assertNotNull(comparatorClass, () -> "comparatorClass can't be null"); + assertNotNull(comparator, () -> "comparator can't be null"); registry.put(environment, comparator); return this; } diff --git a/src/main/java/graphql/schema/InputValueWithState.java b/src/main/java/graphql/schema/InputValueWithState.java index 46ed2ec134..33ef45d062 100644 --- a/src/main/java/graphql/schema/InputValueWithState.java +++ b/src/main/java/graphql/schema/InputValueWithState.java @@ -1,6 +1,5 @@ package graphql.schema; -import graphql.Assert; import graphql.PublicApi; import graphql.language.Value; import org.jetbrains.annotations.NotNull; @@ -46,7 +45,7 @@ private InputValueWithState(State state, Object value) { public static final InputValueWithState NOT_SET = new InputValueWithState(State.NOT_SET, null); public static InputValueWithState newLiteralValue(@NotNull Value value) { - Assert.assertNotNull(value, () -> "value literal can't be null"); + assertNotNull(value, () -> "value literal can't be null"); return new InputValueWithState(State.LITERAL, value); } From 254009cce4c2f3a26d8b9ef3fd7acb5de3e0ae62 Mon Sep 17 00:00:00 2001 From: Brad Baker Date: Thu, 7 Mar 2024 22:27:30 +1100 Subject: [PATCH 5/6] better import --- src/test/groovy/graphql/AssertTest.groovy | 74 ++++++++++++----------- 1 file changed, 38 insertions(+), 36 deletions(-) diff --git a/src/test/groovy/graphql/AssertTest.groovy b/src/test/groovy/graphql/AssertTest.groovy index 5d6901ba23..fe6d818f1f 100644 --- a/src/test/groovy/graphql/AssertTest.groovy +++ b/src/test/groovy/graphql/AssertTest.groovy @@ -2,10 +2,12 @@ package graphql import spock.lang.Specification +import static graphql.Assert.* + class AssertTest extends Specification { def "assertNotNull should not throw on none null value"() { when: - Assert.assertNotNull("some object") + assertNotNull("some object") then: noExceptionThrown() @@ -13,7 +15,7 @@ class AssertTest extends Specification { def "assertNotNull should throw on null value"() { when: - Assert.assertNotNull(null) + assertNotNull(null) then: thrown(AssertException) @@ -21,7 +23,7 @@ class AssertTest extends Specification { def "assertNotNull constant message should throw on null value"() { when: - Assert.assertNotNull(null, "constant message") + assertNotNull(null, "constant message") then: def error = thrown(AssertException) @@ -30,7 +32,7 @@ class AssertTest extends Specification { def "assertNotNull with error message should not throw on none null value"() { when: - Assert.assertNotNull("some object", { -> "error message" }) + assertNotNull("some object", { -> "error message" }) then: noExceptionThrown() @@ -38,7 +40,7 @@ class AssertTest extends Specification { def "assertNotNull with error message should throw on null value with formatted message"() { when: - Assert.assertNotNull(value, { -> String.format(format, arg) }) + assertNotNull(value, { -> String.format(format, arg) }) then: def error = thrown(AssertException) @@ -61,9 +63,9 @@ class AssertTest extends Specification { where: toRun | expectedMessage - runnable({ Assert.assertNotNull(null, "error %s", "arg1") }) | "error arg1" - runnable({ Assert.assertNotNull(null, "error %s %s", "arg1", "arg2") }) | "error arg1 arg2" - runnable({ Assert.assertNotNull(null, "error %s %s %s", "arg1", "arg2", "arg3") }) | "error arg1 arg2 arg3" + runnable({ assertNotNull(null, "error %s", "arg1") }) | "error arg1" + runnable({ assertNotNull(null, "error %s %s", "arg1", "arg2") }) | "error arg1 arg2" + runnable({ assertNotNull(null, "error %s %s %s", "arg1", "arg2", "arg3") }) | "error arg1 arg2 arg3" } def "assertNotNull with different number of error args with non null does not throw assertions"() { @@ -75,14 +77,14 @@ class AssertTest extends Specification { where: toRun | expectedMessage - runnable({ Assert.assertNotNull("x", "error %s", "arg1") }) | "error arg1" - runnable({ Assert.assertNotNull("x", "error %s %s", "arg1", "arg2") }) | "error arg1 arg2" - runnable({ Assert.assertNotNull("x", "error %s %s %s", "arg1", "arg2", "arg3") }) | "error arg1 arg2 arg3" + runnable({ assertNotNull("x", "error %s", "arg1") }) | "error arg1" + runnable({ assertNotNull("x", "error %s %s", "arg1", "arg2") }) | "error arg1 arg2" + runnable({ assertNotNull("x", "error %s %s %s", "arg1", "arg2", "arg3") }) | "error arg1 arg2 arg3" } def "assertNeverCalled should always throw"() { when: - Assert.assertNeverCalled() + assertNeverCalled() then: def e = thrown(AssertException) @@ -91,7 +93,7 @@ class AssertTest extends Specification { def "assertShouldNeverHappen should always throw"() { when: - Assert.assertShouldNeverHappen() + assertShouldNeverHappen() then: def e = thrown(AssertException) @@ -100,7 +102,7 @@ class AssertTest extends Specification { def "assertShouldNeverHappen should always throw with formatted message"() { when: - Assert.assertShouldNeverHappen(format, arg) + assertShouldNeverHappen(format, arg) then: def error = thrown(AssertException) @@ -115,7 +117,7 @@ class AssertTest extends Specification { def "assertNotEmpty collection should throw on null or empty"() { when: - Assert.assertNotEmpty(value, { -> String.format(format, arg) }) + assertNotEmpty(value, { -> String.format(format, arg) }) then: def error = thrown(AssertException) @@ -129,7 +131,7 @@ class AssertTest extends Specification { def "assertNotEmpty should not throw on none empty collection"() { when: - Assert.assertNotEmpty(["some object"], { -> "error message" }) + assertNotEmpty(["some object"], { -> "error message" }) then: noExceptionThrown() @@ -137,7 +139,7 @@ class AssertTest extends Specification { def "assertTrue should not throw on true value"() { when: - Assert.assertTrue(true, { -> "error message" }) + assertTrue(true, { -> "error message" }) then: noExceptionThrown() @@ -145,7 +147,7 @@ class AssertTest extends Specification { def "assertTrue with error message should throw on false value with formatted message"() { when: - Assert.assertTrue(false, { -> String.format(format, arg) }) + assertTrue(false, { -> String.format(format, arg) }) then: def error = thrown(AssertException) @@ -160,7 +162,7 @@ class AssertTest extends Specification { def "assertTrue constant message should throw with message"() { when: - Assert.assertTrue(false, "constant message") + assertTrue(false, "constant message") then: def error = thrown(AssertException) @@ -177,9 +179,9 @@ class AssertTest extends Specification { where: toRun | expectedMessage - runnable({ Assert.assertTrue(false, "error %s", "arg1") }) | "error arg1" - runnable({ Assert.assertTrue(false, "error %s %s", "arg1", "arg2") }) | "error arg1 arg2" - runnable({ Assert.assertTrue(false, "error %s %s %s", "arg1", "arg2", "arg3") }) | "error arg1 arg2 arg3" + runnable({ assertTrue(false, "error %s", "arg1") }) | "error arg1" + runnable({ assertTrue(false, "error %s %s", "arg1", "arg2") }) | "error arg1 arg2" + runnable({ assertTrue(false, "error %s %s %s", "arg1", "arg2", "arg3") }) | "error arg1 arg2 arg3" } def "assertTrue with different number of error args but false does not throw assertions"() { @@ -191,14 +193,14 @@ class AssertTest extends Specification { where: toRun | expectedMessage - runnable({ Assert.assertTrue(true, "error %s", "arg1") }) | "error arg1" - runnable({ Assert.assertTrue(true, "error %s %s", "arg1", "arg2") }) | "error arg1 arg2" - runnable({ Assert.assertTrue(true, "error %s %s %s", "arg1", "arg2", "arg3") }) | "error arg1 arg2 arg3" + runnable({ assertTrue(true, "error %s", "arg1") }) | "error arg1" + runnable({ assertTrue(true, "error %s %s", "arg1", "arg2") }) | "error arg1 arg2" + runnable({ assertTrue(true, "error %s %s %s", "arg1", "arg2", "arg3") }) | "error arg1 arg2 arg3" } def "assertFalse should throw"() { when: - Assert.assertFalse(true) + assertFalse(true) then: thrown(AssertException) @@ -206,7 +208,7 @@ class AssertTest extends Specification { def "assertFalse constant message should throw with message"() { when: - Assert.assertFalse(true, "constant message") + assertFalse(true, "constant message") then: def error = thrown(AssertException) @@ -215,7 +217,7 @@ class AssertTest extends Specification { def "assertFalse with error message should throw on false value with formatted message"() { when: - Assert.assertFalse(true, { -> String.format(format, arg) }) + assertFalse(true, { -> String.format(format, arg) }) then: def error = thrown(AssertException) @@ -238,9 +240,9 @@ class AssertTest extends Specification { where: toRun | expectedMessage - runnable({ Assert.assertFalse(true, "error %s", "arg1") }) | "error arg1" - runnable({ Assert.assertFalse(true, "error %s %s", "arg1", "arg2") }) | "error arg1 arg2" - runnable({ Assert.assertFalse(true, "error %s %s %s", "arg1", "arg2", "arg3") }) | "error arg1 arg2 arg3" + runnable({ assertFalse(true, "error %s", "arg1") }) | "error arg1" + runnable({ assertFalse(true, "error %s %s", "arg1", "arg2") }) | "error arg1 arg2" + runnable({ assertFalse(true, "error %s %s %s", "arg1", "arg2", "arg3") }) | "error arg1 arg2 arg3" } def "assertFalse with different number of error args but false does not throw assertions"() { @@ -252,14 +254,14 @@ class AssertTest extends Specification { where: toRun | expectedMessage - runnable({ Assert.assertFalse(false, "error %s", "arg1") }) | "error arg1" - runnable({ Assert.assertFalse(false, "error %s %s", "arg1", "arg2") }) | "error arg1 arg2" - runnable({ Assert.assertFalse(false, "error %s %s %s", "arg1", "arg2", "arg3") }) | "error arg1 arg2 arg3" + runnable({ assertFalse(false, "error %s", "arg1") }) | "error arg1" + runnable({ assertFalse(false, "error %s %s", "arg1", "arg2") }) | "error arg1 arg2" + runnable({ assertFalse(false, "error %s %s %s", "arg1", "arg2", "arg3") }) | "error arg1 arg2 arg3" } def "assertValidName should not throw on valid names"() { when: - Assert.assertValidName(name) + assertValidName(name) then: noExceptionThrown() @@ -275,7 +277,7 @@ class AssertTest extends Specification { def "assertValidName should throw on invalid names"() { when: - Assert.assertValidName(name) + assertValidName(name) then: def error = thrown(AssertException) From bae150f8473a1625fd23696f2cf176de8ed26603 Mon Sep 17 00:00:00 2001 From: Brad Baker Date: Thu, 7 Mar 2024 23:15:33 +1100 Subject: [PATCH 6/6] re-wrote i18n corcing asserts --- src/main/java/graphql/scalar/GraphqlBooleanCoercing.java | 6 ++++-- src/main/java/graphql/scalar/GraphqlFloatCoercing.java | 7 +++++-- src/main/java/graphql/scalar/GraphqlIDCoercing.java | 6 +++++- src/main/java/graphql/scalar/GraphqlIntCoercing.java | 7 +++++-- src/main/java/graphql/schema/GraphQLEnumType.java | 4 +++- src/test/java/benchmark/AssertBenchmark.java | 6 +++--- 6 files changed, 25 insertions(+), 11 deletions(-) diff --git a/src/main/java/graphql/scalar/GraphqlBooleanCoercing.java b/src/main/java/graphql/scalar/GraphqlBooleanCoercing.java index 266e64c4a5..ceddc7558c 100644 --- a/src/main/java/graphql/scalar/GraphqlBooleanCoercing.java +++ b/src/main/java/graphql/scalar/GraphqlBooleanCoercing.java @@ -15,7 +15,6 @@ import java.math.BigDecimal; import java.util.Locale; -import static graphql.Assert.assertNotNull; import static graphql.Assert.assertShouldNeverHappen; import static graphql.scalar.CoercingUtil.i18nMsg; import static graphql.scalar.CoercingUtil.isNumberIsh; @@ -88,7 +87,10 @@ private static boolean parseLiteralImpl(@NotNull Object input, @NotNull Locale l @NotNull private BooleanValue valueToLiteralImpl(@NotNull Object input, @NotNull Locale locale) { - Boolean result = assertNotNull(convertImpl(input), () -> i18nMsg(locale, "Boolean.notBoolean", typeName(input))); + Boolean result = convertImpl(input); + if (result == null) { + assertShouldNeverHappen(i18nMsg(locale, "Boolean.notBoolean", typeName(input))); + } return BooleanValue.newBooleanValue(result).build(); } diff --git a/src/main/java/graphql/scalar/GraphqlFloatCoercing.java b/src/main/java/graphql/scalar/GraphqlFloatCoercing.java index 58329e56f3..e5e3526e8a 100644 --- a/src/main/java/graphql/scalar/GraphqlFloatCoercing.java +++ b/src/main/java/graphql/scalar/GraphqlFloatCoercing.java @@ -16,7 +16,7 @@ import java.math.BigDecimal; import java.util.Locale; -import static graphql.Assert.assertNotNull; +import static graphql.Assert.assertShouldNeverHappen; import static graphql.scalar.CoercingUtil.i18nMsg; import static graphql.scalar.CoercingUtil.isNumberIsh; import static graphql.scalar.CoercingUtil.typeName; @@ -89,7 +89,10 @@ private static double parseLiteralImpl(@NotNull Object input, @NotNull Locale lo @NotNull private FloatValue valueToLiteralImpl(Object input, @NotNull Locale locale) { - Double result = assertNotNull(convertImpl(input), () -> i18nMsg(locale, "Float.notFloat", typeName(input))); + Double result = convertImpl(input); + if (result == null) { + assertShouldNeverHappen(i18nMsg(locale, "Float.notFloat", typeName(input))); + } return FloatValue.newFloatValue(BigDecimal.valueOf(result)).build(); } diff --git a/src/main/java/graphql/scalar/GraphqlIDCoercing.java b/src/main/java/graphql/scalar/GraphqlIDCoercing.java index 76e78e7917..4631c93c5d 100644 --- a/src/main/java/graphql/scalar/GraphqlIDCoercing.java +++ b/src/main/java/graphql/scalar/GraphqlIDCoercing.java @@ -18,6 +18,7 @@ import java.util.UUID; import static graphql.Assert.assertNotNull; +import static graphql.Assert.assertShouldNeverHappen; import static graphql.scalar.CoercingUtil.i18nMsg; import static graphql.scalar.CoercingUtil.typeName; @@ -84,7 +85,10 @@ private String parseLiteralImpl(Object input, @NotNull Locale locale) { @NotNull private StringValue valueToLiteralImpl(Object input, @NotNull Locale locale) { - String result = assertNotNull(convertImpl(input), () -> i18nMsg(locale, "ID.notId", typeName(input))); + String result = convertImpl(input); + if (result == null) { + assertShouldNeverHappen(i18nMsg(locale, "ID.notId", typeName(input))); + } return StringValue.newStringValue(result).build(); } diff --git a/src/main/java/graphql/scalar/GraphqlIntCoercing.java b/src/main/java/graphql/scalar/GraphqlIntCoercing.java index 99e9eeb84b..0a4a055883 100644 --- a/src/main/java/graphql/scalar/GraphqlIntCoercing.java +++ b/src/main/java/graphql/scalar/GraphqlIntCoercing.java @@ -16,7 +16,7 @@ import java.math.BigInteger; import java.util.Locale; -import static graphql.Assert.assertNotNull; +import static graphql.Assert.assertShouldNeverHappen; import static graphql.scalar.CoercingUtil.i18nMsg; import static graphql.scalar.CoercingUtil.isNumberIsh; import static graphql.scalar.CoercingUtil.typeName; @@ -91,7 +91,10 @@ private static int parseLiteralImpl(Object input, @NotNull Locale locale) { } private IntValue valueToLiteralImpl(Object input, @NotNull Locale locale) { - Integer result = assertNotNull(convertImpl(input),() -> i18nMsg(locale, "Int.notInt", typeName(input))); + Integer result = convertImpl(input); + if (result == null) { + assertShouldNeverHappen(i18nMsg(locale, "Int.notInt", typeName(input))); + } return IntValue.newIntValue(BigInteger.valueOf(result)).build(); } diff --git a/src/main/java/graphql/schema/GraphQLEnumType.java b/src/main/java/graphql/schema/GraphQLEnumType.java index 00ced1b451..f6aa8d49ed 100644 --- a/src/main/java/graphql/schema/GraphQLEnumType.java +++ b/src/main/java/graphql/schema/GraphQLEnumType.java @@ -130,7 +130,9 @@ public Value valueToLiteral(Object input) { @Internal public Value valueToLiteral(Object input, GraphQLContext graphQLContext, Locale locale) { GraphQLEnumValueDefinition enumValueDefinition = valueDefinitionMap.get(input.toString()); - assertNotNull(enumValueDefinition, () -> i18nMsg(locale, "Enum.badName", name, input.toString())); + if (enumValueDefinition == null) { + assertShouldNeverHappen(i18nMsg(locale, "Enum.badName", name, input.toString())); + }; return EnumValue.newEnumValue(enumValueDefinition.getName()).build(); } diff --git a/src/test/java/benchmark/AssertBenchmark.java b/src/test/java/benchmark/AssertBenchmark.java index 192b267280..04a11c03b2 100644 --- a/src/test/java/benchmark/AssertBenchmark.java +++ b/src/test/java/benchmark/AssertBenchmark.java @@ -22,7 +22,7 @@ public class AssertBenchmark { private static final int LOOPS = 100; - private static final Random random = new Random(); + private static final boolean BOOL = new Random().nextBoolean(); @Benchmark @BenchmarkMode(Mode.Throughput) @@ -62,9 +62,9 @@ public void benchMarkAssertWithStringFormatted() { } private boolean jitTrue() { - int i = random.nextInt(); // can you jit this away, Mr JIT?? - return i % 10 < -1 || i * 2 + 1 < -2; + //noinspection ConstantValue,SimplifiableConditionalExpression + return BOOL ? BOOL : !BOOL; } public static void main(String[] args) throws RunnerException {