From 9e5a1b32c84e61b319c137404a76f7aa75b1bcd3 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Edd=C3=BA=20Mel=C3=A9ndez?= Date: Tue, 10 Oct 2017 18:52:00 -0500 Subject: [PATCH 1/2] Provide informative reason when rejecting request with invalid level Previously, bad request with no reason was included in the response. This commit introduces the reason when invalid log level is sent in the request. Fixes gh-10588 --- .../endpoint/mvc/LoggersMvcEndpoint.java | 17 ++++++++++++++++- .../endpoint/mvc/LoggersMvcEndpointTests.java | 4 +++- 2 files changed, 19 insertions(+), 2 deletions(-) diff --git a/spring-boot-actuator/src/main/java/org/springframework/boot/actuate/endpoint/mvc/LoggersMvcEndpoint.java b/spring-boot-actuator/src/main/java/org/springframework/boot/actuate/endpoint/mvc/LoggersMvcEndpoint.java index 0ede3e749f1..77b1d400db1 100644 --- a/spring-boot-actuator/src/main/java/org/springframework/boot/actuate/endpoint/mvc/LoggersMvcEndpoint.java +++ b/spring-boot-actuator/src/main/java/org/springframework/boot/actuate/endpoint/mvc/LoggersMvcEndpoint.java @@ -22,10 +22,12 @@ import org.springframework.boot.actuate.endpoint.LoggersEndpoint; import org.springframework.boot.actuate.endpoint.LoggersEndpoint.LoggerLevels; import org.springframework.boot.context.properties.ConfigurationProperties; import org.springframework.boot.logging.LogLevel; +import org.springframework.http.HttpStatus; import org.springframework.http.ResponseEntity; import org.springframework.web.bind.annotation.PathVariable; import org.springframework.web.bind.annotation.RequestBody; import org.springframework.web.bind.annotation.ResponseBody; +import org.springframework.web.bind.annotation.ResponseStatus; /** * Adapter to expose {@link LoggersEndpoint} as an {@link MvcEndpoint}. @@ -74,7 +76,7 @@ public class LoggersMvcEndpoint extends EndpointMvcAdapter { return ResponseEntity.ok().build(); } catch (IllegalArgumentException ex) { - return ResponseEntity.badRequest().build(); + throw new InvalidLogLevelException("No such log level " + configuration.get("configuredLevel")); } } @@ -83,4 +85,17 @@ public class LoggersMvcEndpoint extends EndpointMvcAdapter { return (level == null ? null : LogLevel.valueOf(level.toUpperCase())); } + /** + * Exception thrown when the specified log level cannot be found. + */ + @SuppressWarnings("serial") + @ResponseStatus(value = HttpStatus.BAD_REQUEST, reason = "No such log level") + public static class InvalidLogLevelException extends RuntimeException { + + public InvalidLogLevelException(String string) { + super(string); + } + + } + } diff --git a/spring-boot-actuator/src/test/java/org/springframework/boot/actuate/endpoint/mvc/LoggersMvcEndpointTests.java b/spring-boot-actuator/src/test/java/org/springframework/boot/actuate/endpoint/mvc/LoggersMvcEndpointTests.java index 499062e2062..a6234dddc74 100644 --- a/spring-boot-actuator/src/test/java/org/springframework/boot/actuate/endpoint/mvc/LoggersMvcEndpointTests.java +++ b/spring-boot-actuator/src/test/java/org/springframework/boot/actuate/endpoint/mvc/LoggersMvcEndpointTests.java @@ -49,6 +49,7 @@ import org.springframework.test.web.servlet.setup.MockMvcBuilders; import org.springframework.web.context.WebApplicationContext; import static org.hamcrest.Matchers.equalTo; +import static org.hamcrest.Matchers.is; import static org.mockito.BDDMockito.given; import static org.mockito.Mockito.mock; import static org.mockito.Mockito.verify; @@ -174,7 +175,8 @@ public class LoggersMvcEndpointTests { public void setLoggerWithWrongLogLevel() throws Exception { this.mvc.perform(post("/loggers/ROOT").contentType(MediaType.APPLICATION_JSON) .content("{\"configuredLevel\":\"other\"}")) - .andExpect(status().is4xxClientError()); + .andExpect(status().is4xxClientError()) + .andExpect(status().reason(is("No such log level"))); verifyZeroInteractions(this.loggingSystem); } From 43aa7dbaf1a041e282d5ff90cdc03a034d7b9a63 Mon Sep 17 00:00:00 2001 From: Andy Wilkinson Date: Fri, 13 Oct 2017 10:55:35 +0100 Subject: [PATCH 2/2] Polish "Provide informative reason when rejecting request with invalid level" See gh-10588 --- .../endpoint/mvc/LoggersMvcEndpoint.java | 22 +++++++++---------- 1 file changed, 11 insertions(+), 11 deletions(-) diff --git a/spring-boot-actuator/src/main/java/org/springframework/boot/actuate/endpoint/mvc/LoggersMvcEndpoint.java b/spring-boot-actuator/src/main/java/org/springframework/boot/actuate/endpoint/mvc/LoggersMvcEndpoint.java index 77b1d400db1..ea91b48572b 100644 --- a/spring-boot-actuator/src/main/java/org/springframework/boot/actuate/endpoint/mvc/LoggersMvcEndpoint.java +++ b/spring-boot-actuator/src/main/java/org/springframework/boot/actuate/endpoint/mvc/LoggersMvcEndpoint.java @@ -70,19 +70,19 @@ public class LoggersMvcEndpoint extends EndpointMvcAdapter { // disabled return getDisabledResponse(); } - try { - LogLevel logLevel = getLogLevel(configuration); - this.delegate.setLogLevel(name, logLevel); - return ResponseEntity.ok().build(); - } - catch (IllegalArgumentException ex) { - throw new InvalidLogLevelException("No such log level " + configuration.get("configuredLevel")); - } + LogLevel logLevel = getLogLevel(configuration); + this.delegate.setLogLevel(name, logLevel); + return ResponseEntity.ok().build(); } private LogLevel getLogLevel(Map configuration) { String level = configuration.get("configuredLevel"); - return (level == null ? null : LogLevel.valueOf(level.toUpperCase())); + try { + return (level == null ? null : LogLevel.valueOf(level.toUpperCase())); + } + catch (IllegalArgumentException ex) { + throw new InvalidLogLevelException(level); + } } /** @@ -92,8 +92,8 @@ public class LoggersMvcEndpoint extends EndpointMvcAdapter { @ResponseStatus(value = HttpStatus.BAD_REQUEST, reason = "No such log level") public static class InvalidLogLevelException extends RuntimeException { - public InvalidLogLevelException(String string) { - super(string); + public InvalidLogLevelException(String level) { + super("Log level '" + level + "' is invalid"); } }