Skip to content

Commit

Permalink
Added better exception messages for dynamic client name validation
Browse files Browse the repository at this point in the history
  • Loading branch information
@mskacelik @jmartisk
mskacelik authored and jmartisk committed Jan 27, 2025
1 parent 55f2c53 commit 464bd6b
Show file tree
Hide file tree
Showing 5 changed files with 129 additions and 44 deletions.
8 changes: 4 additions & 4 deletions client/api/src/main/java/io/smallrye/graphql/client/core/VariableType.java
View file
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
package io.smallrye.graphql.client.core;

import static io.smallrye.graphql.client.core.utils.ServiceUtils.getNewInstanceFromFactory;
import static io.smallrye.graphql.client.core.utils.validation.NameValidation.validateName;
import static io.smallrye.graphql.client.core.utils.validation.NameValidation.validateTypeName;

import io.smallrye.graphql.client.core.factory.VariableTypeFactory;

Expand All @@ -14,7 +14,7 @@ public interface VariableType extends Buildable {
static VariableType varType(String objectTypeName) {
VariableType varType = getNewInstanceFromFactory(VariableTypeFactory.class);

varType.setName(validateName(objectTypeName));
varType.setName(validateTypeName(objectTypeName));
varType.setNonNull(false);
varType.setChild(null);

Expand Down Expand Up @@ -47,7 +47,7 @@ static VariableType nonNull(ScalarType scalarType) {
static VariableType nonNull(String objectTypeName) {
VariableType varType = getNewInstanceFromFactory(VariableTypeFactory.class);

varType.setName(validateName(objectTypeName));
varType.setName(validateTypeName(objectTypeName));
varType.setNonNull(true);
varType.setChild(null);

Expand Down Expand Up @@ -75,7 +75,7 @@ static VariableType list(ScalarType scalarType) {
static VariableType list(String objectTypeName) {
VariableType varType = getNewInstanceFromFactory(VariableTypeFactory.class);

varType.setName("list(" + validateName(objectTypeName) + ")");
varType.setName("list(" + validateTypeName(objectTypeName) + ")");
varType.setNonNull(false);
varType.setChild(varType(objectTypeName));

Expand Down
42 changes: 36 additions & 6 deletions ...nt/api/src/main/java/io/smallrye/graphql/client/core/utils/validation/NameValidation.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,9 @@ public static String validateNameAllowEmpty(String name) {
if (name == null || name.isEmpty()) {
return "";
} else if (!nameMatchesPattern(name, NAME_PATTERN)) {
throw new IllegalArgumentException("Invalid name: " + name);
throw new IllegalArgumentException(
"Invalid name: '%s'. Name does not match the regex pattern %s, please check the GraphQL specification %s"
.formatted(name, _NAME_REGEX, "https://spec.graphql.org/draft/#Name"));
}
return name;
}
Expand All @@ -48,9 +50,11 @@ public static String validateNameAllowEmpty(String name) {
*/
public static String validateFragmentName(String name) {
if (name == null || !nameMatchesPattern(name, NAME_PATTERN)) {
throw new IllegalArgumentException("Invalid fragment name: " + name);
throw new IllegalArgumentException(
"Invalid fragment name '%s'. Fragment name does not match the regex pattern %s, please check the GraphQL specification %s"
.formatted(name, _NAME_REGEX, "https://spec.graphql.org/draft/#sec-Language.Fragments"));
} else if (name.equals("on")) {
throw new IllegalArgumentException("Fragment name cannot be 'on'");
throw new IllegalArgumentException("Invalid fragment name. Fragment name cannot be 'on'");
}
return name;
}
Expand All @@ -65,7 +69,9 @@ public static String validateFragmentName(String name) {
*/
public static String validateName(String name) {
if (name == null || !nameMatchesPattern(name, NAME_PATTERN)) {
throw new IllegalArgumentException("Invalid name: " + name);
throw new IllegalArgumentException(
"Invalid name: '%s'. Name does not match the regex pattern %s, please check the GraphQL specification %s"
.formatted(name, _NAME_REGEX, "https://spec.graphql.org/draft/#Name"));
}
return name;
}
Expand All @@ -83,13 +89,37 @@ public static String validateName(String name) {
*/
public static String validateFieldName(String fieldName) {
if (fieldName == null || !nameMatchesPattern(fieldName, FIELD_NAME_PATTERN)) {
throw new IllegalArgumentException("Invalid field name: " + fieldName);
throw new IllegalArgumentException(
"Invalid field name: '%s'. Field name does not match the regex pattern %s, please check the GraphQL specification %s"
.formatted(fieldName, _FIELD_NAME_REGEX, "https://spec.graphql.org/draft/#sec-Language.Fields"));
}
return fieldName;
}

public static String validateTypeName(String typeName) {
if (typeName == null) {
throw new IllegalArgumentException(
"Invalid type name. Type name cannot be null");
}
if (typeName.contains("]") || typeName.contains("[")) {
throw new IllegalArgumentException(
"Invalid type name: '%s'. Type name cannot contain '[' or ']', instead use the io.smallrye.graphql.client.core.VariableType.list method to wrap your type into a list type"
.formatted(typeName));
}
if (typeName.contains("!")) {
throw new IllegalArgumentException(
"Invalid type name: '%s'. Type name cannot contain '!', instead use the io.smallrye.graphql.client.core.VariableType.nonNull method to wrap your type into a non-null type"
.formatted(typeName));
}
if (!nameMatchesPattern(typeName, NAME_PATTERN)) {
throw new IllegalArgumentException(
"Invalid type name: '%s'. Type name does not match the regex pattern %s, please check the GraphQL specification %s"
.formatted(typeName, _NAME_REGEX, "https://spec.graphql.org/draft/#sec-Type-References"));
}
return typeName;
}

private static boolean nameMatchesPattern(String name, Pattern pattern) {
return pattern.matcher(name).matches();
}

}
31 changes: 22 additions & 9 deletions client/tck/src/main/java/tck/graphql/dynamic/core/OperationsTest.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -4,12 +4,17 @@
import static org.junit.jupiter.api.Assertions.assertDoesNotThrow;
import static org.junit.jupiter.api.Assertions.assertEquals;
import static org.junit.jupiter.api.Assertions.assertThrows;
import static org.junit.jupiter.api.Assertions.assertTrue;

import org.junit.jupiter.api.Test;
import org.junit.jupiter.api.function.Executable;

import io.smallrye.graphql.client.core.Operation;

public class OperationsTest {

private static final String NAME_URL = "https://spec.graphql.org/draft/#Name";

@Test
public void operationsShouldNotThrowExceptionForValidNameTest() {
assertDoesNotThrow(() -> operation("myOperation"));
Expand All @@ -29,14 +34,22 @@ public void operationsShouldNotThrowExceptionForValidNameTest() {

@Test
public void operationsShouldThrowExceptionForInvalidNameTest() {
assertThrows(IllegalArgumentException.class, () -> operation("Invalid Name"));
assertThrows(IllegalArgumentException.class, () -> operation(":InvalidName"));
assertThrows(IllegalArgumentException.class, () -> operation("InvalidName:"));
assertThrows(IllegalArgumentException.class, () -> operation("::InvalidName"));
assertThrows(IllegalArgumentException.class, () -> operation("InvalidName::"));
assertThrows(IllegalArgumentException.class, () -> operation("@InvalidName"));
assertThrows(IllegalArgumentException.class, () -> operation("my.Operation"));
assertThrows(IllegalArgumentException.class, () -> operation("my,Operation"));
assertThrows(IllegalArgumentException.class, () -> operation("my-Operation"));
assertThrowsWithUrlMessage(() -> operation("Invalid Name"));
assertThrowsWithUrlMessage(() -> operation(":InvalidName"));
assertThrowsWithUrlMessage(() -> operation("InvalidName:"));
assertThrowsWithUrlMessage(() -> operation("::InvalidName"));
assertThrowsWithUrlMessage(() -> operation("InvalidName::"));
assertThrowsWithUrlMessage(() -> operation("@InvalidName"));
assertThrowsWithUrlMessage(() -> operation("my.Operation"));
assertThrowsWithUrlMessage(() -> operation("my,Operation"));
assertThrowsWithUrlMessage(() -> operation("my-Operation"));
assertThrowsWithUrlMessage(() -> operation("[myOperation]"));
assertThrowsWithUrlMessage(() -> operation("myOperation!"));
}

private void assertThrowsWithUrlMessage(Executable lambda) {
IllegalArgumentException ex = assertThrows(IllegalArgumentException.class, lambda);
assertTrue(ex.getMessage().contains(NAME_URL),
"Expected the '%s' message to contain '%s'".formatted(ex.getMessage(), NAME_URL));
}
}
66 changes: 48 additions & 18 deletions client/tck/src/main/java/tck/graphql/dynamic/core/VariableTypesTest.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -5,10 +5,16 @@
import static io.smallrye.graphql.client.core.VariableType.varType;
import static org.junit.jupiter.api.Assertions.assertDoesNotThrow;
import static org.junit.jupiter.api.Assertions.assertThrows;
import static org.junit.jupiter.api.Assertions.assertTrue;

import org.junit.jupiter.api.Test;
import org.junit.jupiter.api.function.Executable;

public class VariableTypesTest {
private static final String TYPE_NAME_URL = "https://spec.graphql.org/draft/#sec-Type-References";
private static final String LIST = "io.smallrye.graphql.client.core.VariableType.list";
private static final String NON_NULL = "io.smallrye.graphql.client.core.VariableType.nonNull";

@Test
public void varTypesShouldNotThrowExceptionForValidName() {
assertDoesNotThrow(() -> varType("ValidName"));
Expand All @@ -24,12 +30,14 @@ public void varTypesShouldNotThrowExceptionForValidName() {

@Test
public void varTypesShouldThrowExceptionForInvalidName() {
assertThrows(IllegalArgumentException.class, () -> varType("Invalid:Name"));
assertThrows(IllegalArgumentException.class, () -> varType("Invalid,Name"));
assertThrows(IllegalArgumentException.class, () -> varType("123InvalidName"));
assertThrows(IllegalArgumentException.class, () -> varType("invalid-Name"));
assertThrows(IllegalArgumentException.class, () -> varType("invalid name"));
assertThrows(IllegalArgumentException.class, () -> varType("@invalidName"));
assertThrowsWithUrlMessage(() -> varType("Invalid:Name"));
assertThrowsWithUrlMessage(() -> varType("Invalid,Name"));
assertThrowsWithUrlMessage(() -> varType("123InvalidName"));
assertThrowsWithUrlMessage(() -> varType("invalid-Name"));
assertThrowsWithUrlMessage(() -> varType("invalid name"));
assertThrowsWithUrlMessage(() -> varType("@invalidName"));
assertThrowsWithListMessage(() -> varType("[invalidName]"));
assertThrowsWithNonNullMessage(() -> varType("invalidName!"));
}

@Test
Expand All @@ -47,12 +55,14 @@ public void nonNullsShouldNotThrowExceptionForValidName() {

@Test
public void nonNullsShouldThrowExceptionForInvalidName() {
assertThrows(IllegalArgumentException.class, () -> nonNull("Invalid:Name"));
assertThrows(IllegalArgumentException.class, () -> nonNull("Invalid,Name"));
assertThrows(IllegalArgumentException.class, () -> nonNull("123InvalidName"));
assertThrows(IllegalArgumentException.class, () -> nonNull("invalid-Name"));
assertThrows(IllegalArgumentException.class, () -> nonNull("invalid name"));
assertThrows(IllegalArgumentException.class, () -> nonNull("@invalidName"));
assertThrowsWithUrlMessage(() -> nonNull("Invalid:Name"));
assertThrowsWithUrlMessage(() -> nonNull("Invalid,Name"));
assertThrowsWithUrlMessage(() -> nonNull("123InvalidName"));
assertThrowsWithUrlMessage(() -> nonNull("invalid-Name"));
assertThrowsWithUrlMessage(() -> nonNull("invalid name"));
assertThrowsWithUrlMessage(() -> nonNull("@invalidName"));
assertThrowsWithListMessage(() -> nonNull("[invalidName]"));
assertThrowsWithNonNullMessage(() -> nonNull("invalidName!"));
}

@Test
Expand All @@ -70,11 +80,31 @@ public void listsShouldNotThrowExceptionForValidName() {

@Test
public void listsShouldThrowExceptionForInvalidName() {
assertThrows(IllegalArgumentException.class, () -> list("Invalid:Name"));
assertThrows(IllegalArgumentException.class, () -> list("Invalid,Name"));
assertThrows(IllegalArgumentException.class, () -> list("123InvalidName"));
assertThrows(IllegalArgumentException.class, () -> list("invalidName-"));
assertThrows(IllegalArgumentException.class, () -> list("invalid name"));
assertThrows(IllegalArgumentException.class, () -> list("@invalidName"));
assertThrowsWithUrlMessage(() -> list("Invalid:Name"));
assertThrowsWithUrlMessage(() -> list("Invalid,Name"));
assertThrowsWithUrlMessage(() -> list("123InvalidName"));
assertThrowsWithUrlMessage(() -> list("invalid-Name"));
assertThrowsWithUrlMessage(() -> list("invalid name"));
assertThrowsWithUrlMessage(() -> list("@invalidName"));
assertThrowsWithListMessage(() -> list("[invalidName]"));
assertThrowsWithNonNullMessage(() -> list("invalidName!"));
}

private void assertThrowsWithUrlMessage(Executable lambda) {
IllegalArgumentException ex = assertThrows(IllegalArgumentException.class, lambda);
assertTrue(ex.getMessage().contains(TYPE_NAME_URL),
"Expected the '%s' message to contain '%s'".formatted(ex.getMessage(), TYPE_NAME_URL));
}

private void assertThrowsWithListMessage(Executable lambda) {
IllegalArgumentException ex = assertThrows(IllegalArgumentException.class, lambda);
assertTrue(ex.getMessage().contains(LIST),
"Expected the '%s' message to contain '%s'".formatted(ex.getMessage(), LIST));
}

private void assertThrowsWithNonNullMessage(Executable lambda) {
IllegalArgumentException ex = assertThrows(IllegalArgumentException.class, lambda);
assertTrue(ex.getMessage().contains(NON_NULL),
"Expected the '%s' message to contain '%s'".formatted(ex.getMessage(), NON_NULL));
}
}
26 changes: 19 additions & 7 deletions client/tck/src/main/java/tck/graphql/dynamic/core/VariablesTest.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -20,11 +20,13 @@
import static io.smallrye.graphql.client.core.VariableType.nonNull;
import static org.junit.jupiter.api.Assertions.assertDoesNotThrow;
import static org.junit.jupiter.api.Assertions.assertThrows;
import static org.junit.jupiter.api.Assertions.assertTrue;

import java.io.IOException;
import java.net.URISyntaxException;

import org.junit.jupiter.api.Test;
import org.junit.jupiter.api.function.Executable;

import io.smallrye.graphql.client.core.Document;
import io.smallrye.graphql.client.core.ScalarType;
Expand All @@ -34,6 +36,8 @@

public class VariablesTest {

private static final String NAME_URL = "https://spec.graphql.org/draft/#Name";

@Test
public void variablesDefaultValueTest() throws IOException, URISyntaxException {
String expectedRequest = Utils.getResourceFileContent("core/variablesDefaultValue.graphql");
Expand Down Expand Up @@ -188,13 +192,21 @@ public void variablesShouldNotThrowExceptionForValidNameTest() {
@Test
public void variablesShouldThrowExceptionForInvalidNameTest() {
ScalarType scalarType = GQL_INT;
assertThrows(IllegalArgumentException.class, () -> var("123", scalarType));
assertThrows(IllegalArgumentException.class, () -> var("my_var$", scalarType));
assertThrows(IllegalArgumentException.class, () -> var("va:r", scalarType));
assertThrows(IllegalArgumentException.class, () -> var("", scalarType));
assertThrows(IllegalArgumentException.class, () -> var(":var", scalarType));
assertThrows(IllegalArgumentException.class, () -> var("va:r:", scalarType));
assertThrows(IllegalArgumentException.class, () -> var(null, scalarType));
assertThrowsWithUrlMessage(() -> var("123", scalarType));
assertThrowsWithUrlMessage(() -> var("my_var$", scalarType));
assertThrowsWithUrlMessage(() -> var("va:r", scalarType));
assertThrowsWithUrlMessage(() -> var("", scalarType));
assertThrowsWithUrlMessage(() -> var(":var", scalarType));
assertThrowsWithUrlMessage(() -> var("va:r:", scalarType));
assertThrowsWithUrlMessage(() -> var(null, scalarType));
assertThrowsWithUrlMessage(() -> var("[var]", scalarType));
assertThrowsWithUrlMessage(() -> var("var!", scalarType));
}

private void assertThrowsWithUrlMessage(Executable lambda) {
IllegalArgumentException ex = assertThrows(IllegalArgumentException.class, lambda);
assertTrue(ex.getMessage().contains(NAME_URL),
"Expected the '%s' message to contain '%s'".formatted(ex.getMessage(), NAME_URL));
}

}

0 comments on commit 464bd6b

Please sign in to comment.