Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Cheaper assertions with constant strings #3507

Merged
merged 6 commits into from Mar 22, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
123 changes: 106 additions & 17 deletions src/main/java/graphql/Assert.java
Expand Up @@ -10,63 +10,92 @@
@Internal
public class Assert {

public static <T> T assertNotNullWithNPE(T object, Supplier<String> msg) {
if (object != null) {
return object;
}
throw new NullPointerException(msg.get());
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

moved up to top - explains diff

}

public static <T> T assertNotNull(T object) {
if (object != null) {
return object;
}
return throwAssert("Object required to be not null");
}

public static <T> T assertNotNull(T object, Supplier<String> msg) {
if (object != null) {
return object;
}
throw new AssertException(msg.get());
return throwAssert(msg.get());
}

public static <T> T assertNotNullWithNPE(T object, Supplier<String> msg) {
public static <T> T assertNotNull(T object, String constantMsg) {
if (object != null) {
return object;
}
throw new NullPointerException(msg.get());
return throwAssert(constantMsg);
}

public static <T> T assertNotNull(T object) {
public static <T> T assertNotNull(T object, String msgFmt, Object arg1) {
if (object != null) {
return object;
}
throw new AssertException("Object required to be not null");
return throwAssert(msgFmt, arg1);
}

public static <T> T assertNotNull(T object, String msgFmt, Object arg1, Object arg2) {
if (object != null) {
return object;
}
return throwAssert(msgFmt, arg1, arg2);
}

public static <T> 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 <T> void assertNull(T object, Supplier<String> msg) {
if (object == null) {
return;
}
throw new AssertException(msg.get());
throwAssert(msg.get());
}

public static <T> 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> T assertNeverCalled() {
throw new AssertException("Should never been called");
return throwAssert("Should never been called");
}

public static <T> 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> T assertShouldNeverHappen() {
throw new AssertException("Internal error: should never happen");
return throwAssert("Internal error: should never happen");
}

public static <T> Collection<T> assertNotEmpty(Collection<T> 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 <T> Collection<T> assertNotEmpty(Collection<T> collection, Supplier<String> msg) {
if (collection == null || collection.isEmpty()) {
throw new AssertException(msg.get());
throwAssert(msg.get());
}
return collection;
}
Expand All @@ -75,28 +104,84 @@ public static void assertTrue(boolean condition, Supplier<String> 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");
}

public static void assertTrue(boolean condition, String constantMsg) {
if (condition) {
return;
}
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<String> 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");
}

public static void assertFalse(boolean condition, String constantMsg) {
if (!condition) {
return;
}
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'";
Expand All @@ -108,13 +193,17 @@ public static void assertFalse(boolean condition) {
* 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> T throwAssert(String format, Object... args) {
throw new AssertException(format(format, args));
}
}
Expand Up @@ -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
Expand Down Expand Up @@ -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;
}
Expand Down
8 changes: 5 additions & 3 deletions src/main/java/graphql/execution/Async.java
Expand Up @@ -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 {
Expand Down Expand Up @@ -58,7 +60,7 @@ public void add(CompletableFuture<T> completableFuture) {

@Override
public CompletableFuture<List<T>> await() {
Assert.assertTrue(ix == 0, () -> "expected size was " + 0 + " got " + ix);
assertTrue(ix == 0, "expected size was 0 got %d", ix);
return typedEmpty();
}

Expand Down Expand Up @@ -86,7 +88,7 @@ public void add(CompletableFuture<T> completableFuture) {

@Override
public CompletableFuture<List<T>> await() {
Assert.assertTrue(ix == 1, () -> "expected size was " + 1 + " got " + ix);
assertTrue(ix == 1, "expected size was 1 got %d", ix);
return completableFuture.thenApply(Collections::singletonList);
}
}
Expand All @@ -109,7 +111,7 @@ public void add(CompletableFuture<T> completableFuture) {

@Override
public CompletableFuture<List<T>> await() {
Assert.assertTrue(ix == array.length, () -> "expected size was " + array.length + " got " + ix);
assertTrue(ix == array.length, "expected size was %d got %d", array.length, ix);

CompletableFuture<List<T>> overallResult = new CompletableFuture<>();
CompletableFuture.allOf(array)
Expand Down
@@ -1,6 +1,5 @@
package graphql.execution;

import graphql.Assert;
import graphql.PublicApi;
import graphql.execution.incremental.DeferredCallContext;

Expand Down Expand Up @@ -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;
}

Expand Down
16 changes: 11 additions & 5 deletions 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;
Expand Down Expand Up @@ -112,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) {
Expand Down Expand Up @@ -140,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) {
Expand All @@ -163,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) {
Expand All @@ -173,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) {
Expand All @@ -196,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);
}

Expand All @@ -208,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);
}

Expand All @@ -227,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) {
Expand All @@ -237,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);
}

Expand Down
2 changes: 2 additions & 0 deletions src/main/java/graphql/execution/ValuesResolverConversion.java
Expand Up @@ -104,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,
Expand Down Expand Up @@ -595,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(
Expand Down
2 changes: 1 addition & 1 deletion src/main/java/graphql/execution/ValuesResolverLegacy.java
Expand Up @@ -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;
}
Expand Down