Skip to content

ErrorType#name must be CamelCase #13

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.

Sign up for GitHub

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

Merged
merged 2 commits into from
Jun 30, 2017
Merged

ErrorType#name must be CamelCase #13

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
22590a9
ErrorType#name must be CamelCase
Jun 30, 2017
021b408
Add more test cases
Jun 30, 2017
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
30 changes: 18 additions & 12 deletions errors/src/main/java/com/palantir/remoting/api/errors/ErrorType.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@

package com.palantir.remoting.api.errors;

import java.util.regex.Pattern;
import org.immutables.value.Value;

/**
Expand All @@ -26,6 +27,8 @@
@ImmutablesStyle
public abstract class ErrorType {

private static final Pattern UPPER_CAMEL_CASE = Pattern.compile("([A-Z][a-z]+)+");

public enum Code {
UNKNOWN(500),
PERMISSION_DENIED(403),
Expand All @@ -41,11 +44,11 @@ public enum Code {
}
}

public static final ErrorType UNKNOWN = create(Code.UNKNOWN);
public static final ErrorType PERMISSION_DENIED = create(Code.PERMISSION_DENIED);
public static final ErrorType INVALID_ARGUMENT = create(Code.INVALID_ARGUMENT);
public static final ErrorType FAILED_PRECONDITION = create(Code.FAILED_PRECONDITION);
public static final ErrorType INTERNAL = create(Code.INTERNAL);
public static final ErrorType UNKNOWN = createInternal(Code.UNKNOWN, "Unknown");
public static final ErrorType PERMISSION_DENIED = createInternal(Code.PERMISSION_DENIED, "PermissionDenied");
public static final ErrorType INVALID_ARGUMENT = createInternal(Code.INVALID_ARGUMENT, "InvalidArgument");
public static final ErrorType FAILED_PRECONDITION = createInternal(Code.FAILED_PRECONDITION, "FailedPrecondition");
public static final ErrorType INTERNAL = createInternal(Code.INTERNAL, "Internal");

/** The {@link Code} of this error. */
public abstract Code code();
Expand All @@ -59,6 +62,13 @@ public enum Code {
/** The HTTP error code used to convey this error to HTTP clients. */
public abstract int httpErrorCode();

@Value.Check
final void check() {
if (!UPPER_CAMEL_CASE.matcher(name()).matches()) {
throw new IllegalArgumentException("ErrorType names must be UpperCamelCase: " + name());
}
}

/**
* Creates a new error type with the given name and HTTP error code, and error type{@link Code#CUSTOM}.
* Allowed error codes are {@code 400 BAD REQUEST} and {@code 500 INTERNAL SERVER ERROR}.
Expand All @@ -82,17 +92,13 @@ public static ErrorType of(Code code, String name) {
if (code == Code.CUSTOM) {
throw new IllegalArgumentException("Use the custom() method to construct ErrorTypes with code CUSTOM");
}
return ImmutableErrorType.builder()
.code(code)
.name(name)
.httpErrorCode(code.httpErrorCode)
.build();
return createInternal(code, name);
}

private static ErrorType create(Code code) {
private static ErrorType createInternal(Code code, String name) {
return ImmutableErrorType.builder()
.code(code)
.name(code.name())
.name(name)
.httpErrorCode(code.httpErrorCode)
.build();
}
Expand Down
33 changes: 25 additions & 8 deletions errors/src/test/java/com/palantir/remoting/api/errors/ErrorTypeTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,23 @@

public final class ErrorTypeTest {

@Test
public void testNameMustBeCamelCase() throws Exception {
assertThatThrownBy(() -> ErrorType.of(ErrorType.Code.FAILED_PRECONDITION, "foo"))
.isInstanceOf(IllegalArgumentException.class)
.hasMessageStartingWith("ErrorType names must be UpperCamelCase: foo");

assertThatThrownBy(() -> ErrorType.custom("foo", 400))
.isInstanceOf(IllegalArgumentException.class)
.hasMessageStartingWith("ErrorType names must be UpperCamelCase: foo");
assertThatThrownBy(() -> ErrorType.custom("fooBar", 400))
.isInstanceOf(IllegalArgumentException.class)
.hasMessageStartingWith("ErrorType names must be UpperCamelCase: fooBar");
assertThatThrownBy(() -> ErrorType.custom("", 400))
.isInstanceOf(IllegalArgumentException.class)
.hasMessageStartingWith("ErrorType names must be UpperCamelCase: ");
}

@Test
public void testDefaultErrorTypeHttpErrorCodes() throws Exception {
assertThat(ErrorType.UNKNOWN.httpErrorCode()).isEqualTo(500);
Expand All @@ -34,29 +51,29 @@ public void testDefaultErrorTypeHttpErrorCodes() throws Exception {

@Test
public void testCustomErrors() throws Exception {
ErrorType custom400 = ErrorType.custom("myDesc", 400);
ErrorType custom400 = ErrorType.custom("MyDesc", 400);
assertThat(custom400.code()).isEqualTo(ErrorType.Code.CUSTOM);
assertThat(custom400.httpErrorCode()).isEqualTo(400);
assertThat(custom400.name()).isEqualTo("myDesc");
assertThat(custom400.name()).isEqualTo("MyDesc");

ErrorType custom500 = ErrorType.custom("myDesc", 500);
ErrorType custom500 = ErrorType.custom("MyDesc", 500);
assertThat(custom500.code()).isEqualTo(ErrorType.Code.CUSTOM);
assertThat(custom500.httpErrorCode()).isEqualTo(500);
assertThat(custom500.name()).isEqualTo("myDesc");
assertThat(custom500.name()).isEqualTo("MyDesc");

assertThatThrownBy(() -> ErrorType.custom("myDesc", 403))
assertThatThrownBy(() -> ErrorType.custom("MyDesc", 403))
.isInstanceOf(IllegalArgumentException.class)
.hasMessage("CUSTOM ErrorTypes must have HTTP error code 400 or 500");
}

@Test
public void testCanCreateNewErrorTypes() throws Exception {
ErrorType error = ErrorType.of(ErrorType.Code.FAILED_PRECONDITION, "myDesc");
ErrorType error = ErrorType.of(ErrorType.Code.FAILED_PRECONDITION, "MyDesc");
assertThat(error.code()).isEqualTo(ErrorType.Code.FAILED_PRECONDITION);
assertThat(error.httpErrorCode()).isEqualTo(400);
assertThat(error.name()).isEqualTo("myDesc");
assertThat(error.name()).isEqualTo("MyDesc");

assertThatThrownBy(() -> ErrorType.of(ErrorType.Code.CUSTOM, "myDesc"))
assertThatThrownBy(() -> ErrorType.of(ErrorType.Code.CUSTOM, "MyDesc"))
.isInstanceOf(IllegalArgumentException.class)
.hasMessage("Use the custom() method to construct ErrorTypes with code CUSTOM");
}
Expand Down
4 changes: 2 additions & 2 deletions errors/src/test/java/com/palantir/remoting/api/errors/ServiceExceptionTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -26,8 +26,8 @@

public final class ServiceExceptionTest {

private static final ErrorType ERROR = ErrorType.custom("myDesc", 400);
private static final String EXPECTED_ERROR_MSG = "ServiceException: CUSTOM (myDesc)";
private static final ErrorType ERROR = ErrorType.custom("MyDesc", 400);
private static final String EXPECTED_ERROR_MSG = "ServiceException: CUSTOM (MyDesc)";

@Test
public void testExceptionMessagesContainsSafeArgsOnly() {
Expand Down