Skip to content

Commit 184896d

Browse files
pkoenig10pkoenig10
authored
Relax ErrorType namespace restrictions (#1056)
1 parent 6a8d68d commit 184896d

File tree

3 files changed

+36
-56
lines changed

3 files changed

+36
-56
lines changed

‎changelog/@unreleased/pr-1056.v2.yml‎

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,6 @@
1+
type: fix
2+
fix:
3+
description: '`ErrorType.create` no longer prevents creating `ErrorType` values
4+
in the `Default` namespace.'
5+
links:
6+
- https://github.com/palantir/conjure-java-runtime-api/pull/1056

‎errors/src/main/java/com/palantir/conjure/java/api/errors/ErrorType.java‎

Lines changed: 14 additions & 33 deletions
Original file line numberDiff line numberDiff line change
@@ -16,7 +16,8 @@
1616

1717
package com.palantir.conjure.java.api.errors;
1818

19-
import java.util.regex.Matcher;
19+
import com.palantir.logsafe.SafeArg;
20+
import com.palantir.logsafe.exceptions.SafeIllegalArgumentException;
2021
import java.util.regex.Pattern;
2122
import org.immutables.value.Value;
2223

@@ -55,18 +56,16 @@ public enum Code {
5556
}
5657
}
5758

58-
public static final ErrorType UNAUTHORIZED = createInternal(Code.UNAUTHORIZED, "Default:Unauthorized");
59-
public static final ErrorType PERMISSION_DENIED =
60-
createInternal(Code.PERMISSION_DENIED, "Default:PermissionDenied");
61-
public static final ErrorType INVALID_ARGUMENT = createInternal(Code.INVALID_ARGUMENT, "Default:InvalidArgument");
59+
public static final ErrorType UNAUTHORIZED = create(Code.UNAUTHORIZED, "Default:Unauthorized");
60+
public static final ErrorType PERMISSION_DENIED = create(Code.PERMISSION_DENIED, "Default:PermissionDenied");
61+
public static final ErrorType INVALID_ARGUMENT = create(Code.INVALID_ARGUMENT, "Default:InvalidArgument");
6262
public static final ErrorType REQUEST_ENTITY_TOO_LARGE =
63-
createInternal(Code.REQUEST_ENTITY_TOO_LARGE, "Default:RequestEntityTooLarge");
64-
public static final ErrorType NOT_FOUND = createInternal(Code.NOT_FOUND, "Default:NotFound");
65-
public static final ErrorType CONFLICT = createInternal(Code.CONFLICT, "Default:Conflict");
66-
public static final ErrorType FAILED_PRECONDITION =
67-
createInternal(Code.FAILED_PRECONDITION, "Default:FailedPrecondition");
68-
public static final ErrorType INTERNAL = createInternal(Code.INTERNAL, "Default:Internal");
69-
public static final ErrorType TIMEOUT = createInternal(Code.TIMEOUT, "Default:Timeout");
63+
create(Code.REQUEST_ENTITY_TOO_LARGE, "Default:RequestEntityTooLarge");
64+
public static final ErrorType NOT_FOUND = create(Code.NOT_FOUND, "Default:NotFound");
65+
public static final ErrorType CONFLICT = create(Code.CONFLICT, "Default:Conflict");
66+
public static final ErrorType FAILED_PRECONDITION = create(Code.FAILED_PRECONDITION, "Default:FailedPrecondition");
67+
public static final ErrorType INTERNAL = create(Code.INTERNAL, "Default:Internal");
68+
public static final ErrorType TIMEOUT = create(Code.TIMEOUT, "Default:Timeout");
7069

7170
/** The {@link Code} of this error. */
7271
public abstract Code code();
@@ -83,32 +82,14 @@ public enum Code {
8382
@Value.Check
8483
final void check() {
8584
if (!ERROR_NAME_PATTERN.matcher(name()).matches()) {
86-
throw new IllegalArgumentException(
87-
"ErrorType names must be of the form 'UpperCamelNamespace:UpperCamelName': " + name());
85+
throw new SafeIllegalArgumentException(
86+
"ErrorType names must be of the form 'UpperCamelNamespace:UpperCamelName'",
87+
SafeArg.of("name", name()));
8888
}
8989
}
9090

9191
/** Constructs an {@link ErrorType} with the given error {@link Code} and name. */
9292
public static ErrorType create(Code code, String name) {
93-
return createAndCheckNamespaceIsNotDefault(code, name);
94-
}
95-
96-
private static ErrorType createAndCheckNamespaceIsNotDefault(Code code, String name) {
97-
ErrorType error = createInternal(code, name);
98-
Matcher matcher = ERROR_NAME_PATTERN.matcher(name);
99-
if (!matcher.matches()) {
100-
throw new IllegalArgumentException("Expected ERROR_NAME_PATTERN to match, this is a bug: " + name);
101-
}
102-
103-
String namespace = matcher.group(1);
104-
if (namespace.equals("Default")) {
105-
throw new IllegalArgumentException("Namespace must not be 'Default' in ErrorType name: " + name);
106-
}
107-
108-
return error;
109-
}
110-
111-
private static ErrorType createInternal(Code code, String name) {
11293
return ImmutableErrorType.builder()
11394
.code(code)
11495
.name(name)

‎errors/src/test/java/com/palantir/conjure/java/api/errors/ErrorTypeTest.java‎

Lines changed: 16 additions & 23 deletions
Original file line numberDiff line numberDiff line change
@@ -16,9 +16,12 @@
1616

1717
package com.palantir.conjure.java.api.errors;
1818

19+
import static com.palantir.logsafe.testing.Assertions.assertThatLoggableExceptionThrownBy;
1920
import static org.assertj.core.api.Assertions.assertThat;
20-
import static org.assertj.core.api.Assertions.assertThatThrownBy;
2121

22+
import com.palantir.conjure.java.api.errors.ErrorType.Code;
23+
import com.palantir.logsafe.SafeArg;
24+
import com.palantir.logsafe.exceptions.SafeIllegalArgumentException;
2225
import org.junit.jupiter.api.Test;
2326

2427
public final class ErrorTypeTest {
@@ -27,15 +30,18 @@ public final class ErrorTypeTest {
2730
public void testNameMustBeCamelCaseWithOptionalNameSpace() throws Exception {
2831
String[] badNames = new String[] {":", "foo:Bar", ":Bar", "Bar:", "foo:bar", "Foo:bar", "Foo:2Bar"};
2932
for (String name : badNames) {
30-
assertThatThrownBy(() -> ErrorType.create(ErrorType.Code.CUSTOM_CLIENT, name))
31-
.isInstanceOf(IllegalArgumentException.class)
32-
.hasMessage("ErrorType names must be of the form 'UpperCamelNamespace:UpperCamelName': %s", name);
33-
assertThatThrownBy(() -> ErrorType.create(ErrorType.Code.CUSTOM_SERVER, name))
34-
.isInstanceOf(IllegalArgumentException.class)
35-
.hasMessage("ErrorType names must be of the form 'UpperCamelNamespace:UpperCamelName': %s", name);
36-
assertThatThrownBy(() -> ErrorType.create(ErrorType.Code.FAILED_PRECONDITION, name))
37-
.isInstanceOf(IllegalArgumentException.class)
38-
.hasMessage("ErrorType names must be of the form 'UpperCamelNamespace:UpperCamelName': %s", name);
33+
assertThatLoggableExceptionThrownBy(() -> ErrorType.create(Code.CUSTOM_CLIENT, name))
34+
.isInstanceOf(SafeIllegalArgumentException.class)
35+
.hasLogMessage("ErrorType names must be of the form 'UpperCamelNamespace:UpperCamelName'")
36+
.hasExactlyArgs(SafeArg.of("name", name));
37+
assertThatLoggableExceptionThrownBy(() -> ErrorType.create(ErrorType.Code.CUSTOM_SERVER, name))
38+
.isInstanceOf(SafeIllegalArgumentException.class)
39+
.hasLogMessage("ErrorType names must be of the form 'UpperCamelNamespace:UpperCamelName'")
40+
.hasExactlyArgs(SafeArg.of("name", name));
41+
assertThatLoggableExceptionThrownBy(() -> ErrorType.create(ErrorType.Code.FAILED_PRECONDITION, name))
42+
.isInstanceOf(SafeIllegalArgumentException.class)
43+
.hasLogMessage("ErrorType names must be of the form 'UpperCamelNamespace:UpperCamelName'")
44+
.hasExactlyArgs(SafeArg.of("name", name));
3945
}
4046

4147
String[] goodNames = new String[] {"Foo:Bar", "FooBar:Baz", "FooBar:BoomBang", "Foo:Bar2Baz3"};
@@ -46,19 +52,6 @@ public void testNameMustBeCamelCaseWithOptionalNameSpace() throws Exception {
4652
}
4753
}
4854

49-
@Test
50-
public void testNamespaceMustNotBeDefault() throws Exception {
51-
assertThatThrownBy(() -> ErrorType.create(ErrorType.Code.CUSTOM_SERVER, "Default:Foo"))
52-
.isInstanceOf(IllegalArgumentException.class)
53-
.hasMessage("Namespace must not be 'Default' in ErrorType name: Default:Foo");
54-
assertThatThrownBy(() -> ErrorType.create(ErrorType.Code.CUSTOM_SERVER, "Default:Foo"))
55-
.isInstanceOf(IllegalArgumentException.class)
56-
.hasMessage("Namespace must not be 'Default' in ErrorType name: Default:Foo");
57-
assertThatThrownBy(() -> ErrorType.create(ErrorType.Code.INVALID_ARGUMENT, "Default:Foo"))
58-
.isInstanceOf(IllegalArgumentException.class)
59-
.hasMessage("Namespace must not be 'Default' in ErrorType name: Default:Foo");
60-
}
61-
6255
@Test
6356
public void testDefaultErrorTypeHttpErrorCodes() throws Exception {
6457
assertThat(ErrorType.UNAUTHORIZED.httpErrorCode()).isEqualTo(401);

0 commit comments

Comments
 (0)