Skip to content

Commit 7e5c9f4

Browse files
mskacelikmskacelik
committed
Added better exception messages for dynamic client name validation
1 parent 55f2c53 commit 7e5c9f4

File tree

5 files changed

+129
-44
lines changed

5 files changed

+129
-44
lines changed

‎client/api/src/main/java/io/smallrye/graphql/client/core/VariableType.java

+4-4
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,7 @@
11
package io.smallrye.graphql.client.core;
22

33
import static io.smallrye.graphql.client.core.utils.ServiceUtils.getNewInstanceFromFactory;
4-
import static io.smallrye.graphql.client.core.utils.validation.NameValidation.validateName;
4+
import static io.smallrye.graphql.client.core.utils.validation.NameValidation.validateTypeName;
55

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

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

17-
varType.setName(validateName(objectTypeName));
17+
varType.setName(validateTypeName(objectTypeName));
1818
varType.setNonNull(false);
1919
varType.setChild(null);
2020

@@ -47,7 +47,7 @@ static VariableType nonNull(ScalarType scalarType) {
4747
static VariableType nonNull(String objectTypeName) {
4848
VariableType varType = getNewInstanceFromFactory(VariableTypeFactory.class);
4949

50-
varType.setName(validateName(objectTypeName));
50+
varType.setName(validateTypeName(objectTypeName));
5151
varType.setNonNull(true);
5252
varType.setChild(null);
5353

@@ -75,7 +75,7 @@ static VariableType list(ScalarType scalarType) {
7575
static VariableType list(String objectTypeName) {
7676
VariableType varType = getNewInstanceFromFactory(VariableTypeFactory.class);
7777

78-
varType.setName("list(" + validateName(objectTypeName) + ")");
78+
varType.setName("list(" + validateTypeName(objectTypeName) + ")");
7979
varType.setNonNull(false);
8080
varType.setChild(varType(objectTypeName));
8181

‎client/api/src/main/java/io/smallrye/graphql/client/core/utils/validation/NameValidation.java

+36-6
Original file line numberDiff line numberDiff line change
@@ -30,7 +30,9 @@ public static String validateNameAllowEmpty(String name) {
3030
if (name == null || name.isEmpty()) {
3131
return "";
3232
} else if (!nameMatchesPattern(name, NAME_PATTERN)) {
33-
throw new IllegalArgumentException("Invalid name: " + name);
33+
throw new IllegalArgumentException(
34+
"Invalid name: '%s'. Name does not match the regex pattern %s, please check the GraphQL specification %s"
35+
.formatted(name, _NAME_REGEX, "https://spec.graphql.org/draft/#Name"));
3436
}
3537
return name;
3638
}
@@ -48,9 +50,11 @@ public static String validateNameAllowEmpty(String name) {
4850
*/
4951
public static String validateFragmentName(String name) {
5052
if (name == null || !nameMatchesPattern(name, NAME_PATTERN)) {
51-
throw new IllegalArgumentException("Invalid fragment name: " + name);
53+
throw new IllegalArgumentException(
54+
"Invalid fragment name '%s'. Fragment name does not match the regex pattern %s, please check the GraphQL specification %s"
55+
.formatted(name, _NAME_REGEX, "https://spec.graphql.org/draft/#sec-Language.Fragments"));
5256
} else if (name.equals("on")) {
53-
throw new IllegalArgumentException("Fragment name cannot be 'on'");
57+
throw new IllegalArgumentException("Invalid fragment name. Fragment name cannot be 'on'");
5458
}
5559
return name;
5660
}
@@ -65,7 +69,9 @@ public static String validateFragmentName(String name) {
6569
*/
6670
public static String validateName(String name) {
6771
if (name == null || !nameMatchesPattern(name, NAME_PATTERN)) {
68-
throw new IllegalArgumentException("Invalid name: " + name);
72+
throw new IllegalArgumentException(
73+
"Invalid name: '%s'. Name does not match the regex pattern %s, please check the GraphQL specification %s"
74+
.formatted(name, _NAME_REGEX, "https://spec.graphql.org/draft/#Name"));
6975
}
7076
return name;
7177
}
@@ -83,13 +89,37 @@ public static String validateName(String name) {
8389
*/
8490
public static String validateFieldName(String fieldName) {
8591
if (fieldName == null || !nameMatchesPattern(fieldName, FIELD_NAME_PATTERN)) {
86-
throw new IllegalArgumentException("Invalid field name: " + fieldName);
92+
throw new IllegalArgumentException(
93+
"Invalid field name: '%s'. Field name does not match the regex pattern %s, please check the GraphQL specification %s"
94+
.formatted(fieldName, _FIELD_NAME_REGEX, "https://spec.graphql.org/draft/#sec-Language.Fields"));
8795
}
8896
return fieldName;
8997
}
9098

99+
public static String validateTypeName(String typeName) {
100+
if (typeName == null) {
101+
throw new IllegalArgumentException(
102+
"Invalid type name. Type name cannot be null");
103+
}
104+
if (typeName.contains("]") || typeName.contains("[")) {
105+
throw new IllegalArgumentException(
106+
"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"
107+
.formatted(typeName));
108+
}
109+
if (typeName.contains("!")) {
110+
throw new IllegalArgumentException(
111+
"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"
112+
.formatted(typeName));
113+
}
114+
if (!nameMatchesPattern(typeName, NAME_PATTERN)) {
115+
throw new IllegalArgumentException(
116+
"Invalid type name: '%s'. Type name does not match the regex pattern %s, please check the GraphQL specification %s"
117+
.formatted(typeName, _NAME_REGEX, "https://spec.graphql.org/draft/#sec-Type-References"));
118+
}
119+
return typeName;
120+
}
121+
91122
private static boolean nameMatchesPattern(String name, Pattern pattern) {
92123
return pattern.matcher(name).matches();
93124
}
94-
95125
}

‎client/tck/src/main/java/tck/graphql/dynamic/core/OperationsTest.java

+22-9
Original file line numberDiff line numberDiff line change
@@ -4,12 +4,17 @@
44
import static org.junit.jupiter.api.Assertions.assertDoesNotThrow;
55
import static org.junit.jupiter.api.Assertions.assertEquals;
66
import static org.junit.jupiter.api.Assertions.assertThrows;
7+
import static org.junit.jupiter.api.Assertions.assertTrue;
78

89
import org.junit.jupiter.api.Test;
10+
import org.junit.jupiter.api.function.Executable;
911

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

1214
public class OperationsTest {
15+
16+
private static final String NAME_URL = "https://spec.graphql.org/draft/#Name";
17+
1318
@Test
1419
public void operationsShouldNotThrowExceptionForValidNameTest() {
1520
assertDoesNotThrow(() -> operation("myOperation"));
@@ -29,14 +34,22 @@ public void operationsShouldNotThrowExceptionForValidNameTest() {
2934

3035
@Test
3136
public void operationsShouldThrowExceptionForInvalidNameTest() {
32-
assertThrows(IllegalArgumentException.class, () -> operation("Invalid Name"));
33-
assertThrows(IllegalArgumentException.class, () -> operation(":InvalidName"));
34-
assertThrows(IllegalArgumentException.class, () -> operation("InvalidName:"));
35-
assertThrows(IllegalArgumentException.class, () -> operation("::InvalidName"));
36-
assertThrows(IllegalArgumentException.class, () -> operation("InvalidName::"));
37-
assertThrows(IllegalArgumentException.class, () -> operation("@InvalidName"));
38-
assertThrows(IllegalArgumentException.class, () -> operation("my.Operation"));
39-
assertThrows(IllegalArgumentException.class, () -> operation("my,Operation"));
40-
assertThrows(IllegalArgumentException.class, () -> operation("my-Operation"));
37+
assertThrowsWithUrlMessage(() -> operation("Invalid Name"));
38+
assertThrowsWithUrlMessage(() -> operation(":InvalidName"));
39+
assertThrowsWithUrlMessage(() -> operation("InvalidName:"));
40+
assertThrowsWithUrlMessage(() -> operation("::InvalidName"));
41+
assertThrowsWithUrlMessage(() -> operation("InvalidName::"));
42+
assertThrowsWithUrlMessage(() -> operation("@InvalidName"));
43+
assertThrowsWithUrlMessage(() -> operation("my.Operation"));
44+
assertThrowsWithUrlMessage(() -> operation("my,Operation"));
45+
assertThrowsWithUrlMessage(() -> operation("my-Operation"));
46+
assertThrowsWithUrlMessage(() -> operation("[myOperation]"));
47+
assertThrowsWithUrlMessage(() -> operation("myOperation!"));
48+
}
49+
50+
private void assertThrowsWithUrlMessage(Executable lambda) {
51+
IllegalArgumentException ex = assertThrows(IllegalArgumentException.class, lambda);
52+
assertTrue(ex.getMessage().contains(NAME_URL),
53+
"Expected the '%s' message to contain '%s'".formatted(ex.getMessage(), NAME_URL));
4154
}
4255
}

‎client/tck/src/main/java/tck/graphql/dynamic/core/VariableTypesTest.java

+48-18
Original file line numberDiff line numberDiff line change
@@ -5,10 +5,16 @@
55
import static io.smallrye.graphql.client.core.VariableType.varType;
66
import static org.junit.jupiter.api.Assertions.assertDoesNotThrow;
77
import static org.junit.jupiter.api.Assertions.assertThrows;
8+
import static org.junit.jupiter.api.Assertions.assertTrue;
89

910
import org.junit.jupiter.api.Test;
11+
import org.junit.jupiter.api.function.Executable;
1012

1113
public class VariableTypesTest {
14+
private static final String TYPE_NAME_URL = "https://spec.graphql.org/draft/#sec-Type-References";
15+
private static final String LIST = "io.smallrye.graphql.client.core.VariableType.list";
16+
private static final String NON_NULL = "io.smallrye.graphql.client.core.VariableType.nonNull";
17+
1218
@Test
1319
public void varTypesShouldNotThrowExceptionForValidName() {
1420
assertDoesNotThrow(() -> varType("ValidName"));
@@ -24,12 +30,14 @@ public void varTypesShouldNotThrowExceptionForValidName() {
2430

2531
@Test
2632
public void varTypesShouldThrowExceptionForInvalidName() {
27-
assertThrows(IllegalArgumentException.class, () -> varType("Invalid:Name"));
28-
assertThrows(IllegalArgumentException.class, () -> varType("Invalid,Name"));
29-
assertThrows(IllegalArgumentException.class, () -> varType("123InvalidName"));
30-
assertThrows(IllegalArgumentException.class, () -> varType("invalid-Name"));
31-
assertThrows(IllegalArgumentException.class, () -> varType("invalid name"));
32-
assertThrows(IllegalArgumentException.class, () -> varType("@invalidName"));
33+
assertThrowsWithUrlMessage(() -> varType("Invalid:Name"));
34+
assertThrowsWithUrlMessage(() -> varType("Invalid,Name"));
35+
assertThrowsWithUrlMessage(() -> varType("123InvalidName"));
36+
assertThrowsWithUrlMessage(() -> varType("invalid-Name"));
37+
assertThrowsWithUrlMessage(() -> varType("invalid name"));
38+
assertThrowsWithUrlMessage(() -> varType("@invalidName"));
39+
assertThrowsWithListMessage(() -> varType("[invalidName]"));
40+
assertThrowsWithNonNullMessage(() -> varType("invalidName!"));
3341
}
3442

3543
@Test
@@ -47,12 +55,14 @@ public void nonNullsShouldNotThrowExceptionForValidName() {
4755

4856
@Test
4957
public void nonNullsShouldThrowExceptionForInvalidName() {
50-
assertThrows(IllegalArgumentException.class, () -> nonNull("Invalid:Name"));
51-
assertThrows(IllegalArgumentException.class, () -> nonNull("Invalid,Name"));
52-
assertThrows(IllegalArgumentException.class, () -> nonNull("123InvalidName"));
53-
assertThrows(IllegalArgumentException.class, () -> nonNull("invalid-Name"));
54-
assertThrows(IllegalArgumentException.class, () -> nonNull("invalid name"));
55-
assertThrows(IllegalArgumentException.class, () -> nonNull("@invalidName"));
58+
assertThrowsWithUrlMessage(() -> nonNull("Invalid:Name"));
59+
assertThrowsWithUrlMessage(() -> nonNull("Invalid,Name"));
60+
assertThrowsWithUrlMessage(() -> nonNull("123InvalidName"));
61+
assertThrowsWithUrlMessage(() -> nonNull("invalid-Name"));
62+
assertThrowsWithUrlMessage(() -> nonNull("invalid name"));
63+
assertThrowsWithUrlMessage(() -> nonNull("@invalidName"));
64+
assertThrowsWithListMessage(() -> nonNull("[invalidName]"));
65+
assertThrowsWithNonNullMessage(() -> nonNull("invalidName!"));
5666
}
5767

5868
@Test
@@ -70,11 +80,31 @@ public void listsShouldNotThrowExceptionForValidName() {
7080

7181
@Test
7282
public void listsShouldThrowExceptionForInvalidName() {
73-
assertThrows(IllegalArgumentException.class, () -> list("Invalid:Name"));
74-
assertThrows(IllegalArgumentException.class, () -> list("Invalid,Name"));
75-
assertThrows(IllegalArgumentException.class, () -> list("123InvalidName"));
76-
assertThrows(IllegalArgumentException.class, () -> list("invalidName-"));
77-
assertThrows(IllegalArgumentException.class, () -> list("invalid name"));
78-
assertThrows(IllegalArgumentException.class, () -> list("@invalidName"));
83+
assertThrowsWithUrlMessage(() -> list("Invalid:Name"));
84+
assertThrowsWithUrlMessage(() -> list("Invalid,Name"));
85+
assertThrowsWithUrlMessage(() -> list("123InvalidName"));
86+
assertThrowsWithUrlMessage(() -> list("invalid-Name"));
87+
assertThrowsWithUrlMessage(() -> list("invalid name"));
88+
assertThrowsWithUrlMessage(() -> list("@invalidName"));
89+
assertThrowsWithListMessage(() -> list("[invalidName]"));
90+
assertThrowsWithNonNullMessage(() -> list("invalidName!"));
91+
}
92+
93+
private void assertThrowsWithUrlMessage(Executable lambda) {
94+
IllegalArgumentException ex = assertThrows(IllegalArgumentException.class, lambda);
95+
assertTrue(ex.getMessage().contains(TYPE_NAME_URL),
96+
"Expected the '%s' message to contain '%s'".formatted(ex.getMessage(), TYPE_NAME_URL));
97+
}
98+
99+
private void assertThrowsWithListMessage(Executable lambda) {
100+
IllegalArgumentException ex = assertThrows(IllegalArgumentException.class, lambda);
101+
assertTrue(ex.getMessage().contains(LIST),
102+
"Expected the '%s' message to contain '%s'".formatted(ex.getMessage(), LIST));
103+
}
104+
105+
private void assertThrowsWithNonNullMessage(Executable lambda) {
106+
IllegalArgumentException ex = assertThrows(IllegalArgumentException.class, lambda);
107+
assertTrue(ex.getMessage().contains(NON_NULL),
108+
"Expected the '%s' message to contain '%s'".formatted(ex.getMessage(), NON_NULL));
79109
}
80110
}

‎client/tck/src/main/java/tck/graphql/dynamic/core/VariablesTest.java

+19-7
Original file line numberDiff line numberDiff line change
@@ -20,11 +20,13 @@
2020
import static io.smallrye.graphql.client.core.VariableType.nonNull;
2121
import static org.junit.jupiter.api.Assertions.assertDoesNotThrow;
2222
import static org.junit.jupiter.api.Assertions.assertThrows;
23+
import static org.junit.jupiter.api.Assertions.assertTrue;
2324

2425
import java.io.IOException;
2526
import java.net.URISyntaxException;
2627

2728
import org.junit.jupiter.api.Test;
29+
import org.junit.jupiter.api.function.Executable;
2830

2931
import io.smallrye.graphql.client.core.Document;
3032
import io.smallrye.graphql.client.core.ScalarType;
@@ -34,6 +36,8 @@
3436

3537
public class VariablesTest {
3638

39+
private static final String NAME_URL = "https://spec.graphql.org/draft/#Name";
40+
3741
@Test
3842
public void variablesDefaultValueTest() throws IOException, URISyntaxException {
3943
String expectedRequest = Utils.getResourceFileContent("core/variablesDefaultValue.graphql");
@@ -188,13 +192,21 @@ public void variablesShouldNotThrowExceptionForValidNameTest() {
188192
@Test
189193
public void variablesShouldThrowExceptionForInvalidNameTest() {
190194
ScalarType scalarType = GQL_INT;
191-
assertThrows(IllegalArgumentException.class, () -> var("123", scalarType));
192-
assertThrows(IllegalArgumentException.class, () -> var("my_var$", scalarType));
193-
assertThrows(IllegalArgumentException.class, () -> var("va:r", scalarType));
194-
assertThrows(IllegalArgumentException.class, () -> var("", scalarType));
195-
assertThrows(IllegalArgumentException.class, () -> var(":var", scalarType));
196-
assertThrows(IllegalArgumentException.class, () -> var("va:r:", scalarType));
197-
assertThrows(IllegalArgumentException.class, () -> var(null, scalarType));
195+
assertThrowsWithUrlMessage(() -> var("123", scalarType));
196+
assertThrowsWithUrlMessage(() -> var("my_var$", scalarType));
197+
assertThrowsWithUrlMessage(() -> var("va:r", scalarType));
198+
assertThrowsWithUrlMessage(() -> var("", scalarType));
199+
assertThrowsWithUrlMessage(() -> var(":var", scalarType));
200+
assertThrowsWithUrlMessage(() -> var("va:r:", scalarType));
201+
assertThrowsWithUrlMessage(() -> var(null, scalarType));
202+
assertThrowsWithUrlMessage(() -> var("[var]", scalarType));
203+
assertThrowsWithUrlMessage(() -> var("var!", scalarType));
204+
}
205+
206+
private void assertThrowsWithUrlMessage(Executable lambda) {
207+
IllegalArgumentException ex = assertThrows(IllegalArgumentException.class, lambda);
208+
assertTrue(ex.getMessage().contains(NAME_URL),
209+
"Expected the '%s' message to contain '%s'".formatted(ex.getMessage(), NAME_URL));
198210
}
199211

200212
}

0 commit comments

Comments
 (0)