From fce255695ebf5641e0c12ded00649298b1a08411 Mon Sep 17 00:00:00 2001 From: Andy Wilkinson Date: Fri, 30 Aug 2019 16:57:21 +0100 Subject: [PATCH] Polish "Improve diagnostics for null configuration" See gh-583 --- .../RestDocumentationResultHandler.java | 13 +++++-------- ...kMvcRestDocumentationIntegrationTests.java | 13 +++++-------- .../restassured3/RestDocumentationFilter.java | 11 +++++------ ...uredRestDocumentationIntegrationTests.java | 16 +++++++--------- .../WebTestClientRestDocumentation.java | 19 ++++++------------- ...TestClientRestDocumentationConfigurer.java | 6 +++++- ...lientRestDocumentationConfigurerTests.java | 4 +++- ...ientRestDocumentationIntegrationTests.java | 10 +++------- 8 files changed, 39 insertions(+), 53 deletions(-) diff --git a/spring-restdocs-mockmvc/src/main/java/org/springframework/restdocs/mockmvc/RestDocumentationResultHandler.java b/spring-restdocs-mockmvc/src/main/java/org/springframework/restdocs/mockmvc/RestDocumentationResultHandler.java index 944940b3..3111b7b6 100644 --- a/spring-restdocs-mockmvc/src/main/java/org/springframework/restdocs/mockmvc/RestDocumentationResultHandler.java +++ b/spring-restdocs-mockmvc/src/main/java/org/springframework/restdocs/mockmvc/RestDocumentationResultHandler.java @@ -49,7 +49,7 @@ public class RestDocumentationResultHandler implements ResultHandler { @Override public void handle(MvcResult result) throws Exception { - this.delegate.handle(result.getRequest(), result.getResponse(), getRequiredConfiguration(result)); + this.delegate.handle(result.getRequest(), result.getResponse(), retrieveConfiguration(result)); } /** @@ -72,7 +72,7 @@ public class RestDocumentationResultHandler implements ResultHandler { @Override public void handle(MvcResult result) throws Exception { - Map configuration = new HashMap<>(getRequiredConfiguration(result)); + Map configuration = new HashMap<>(retrieveConfiguration(result)); configuration.remove(RestDocumentationGenerator.ATTRIBUTE_NAME_DEFAULT_SNIPPETS); getDelegate().handle(result.getRequest(), result.getResponse(), configuration); } @@ -88,15 +88,12 @@ public class RestDocumentationResultHandler implements ResultHandler { return this.delegate; } - private static Map getRequiredConfiguration(MvcResult result) { + private Map retrieveConfiguration(MvcResult result) { @SuppressWarnings("unchecked") Map configuration = (Map) result.getRequest() .getAttribute(ATTRIBUTE_NAME_CONFIGURATION); - Assert.state(configuration != null, - () -> String.format( - "There is no REST Docs configuration. Looks like " - + "'%s' was not invoked. Please check your configuration.", - MockMvcRestDocumentationConfigurer.class.getName())); + Assert.state(configuration != null, () -> "REST Docs configuration not found. Did you forget to apply a " + + MockMvcRestDocumentationConfigurer.class.getSimpleName() + " when building the MockMvc instance?"); return configuration; } diff --git a/spring-restdocs-mockmvc/src/test/java/org/springframework/restdocs/mockmvc/MockMvcRestDocumentationIntegrationTests.java b/spring-restdocs-mockmvc/src/test/java/org/springframework/restdocs/mockmvc/MockMvcRestDocumentationIntegrationTests.java index 38ebdfa4..129952b7 100644 --- a/spring-restdocs-mockmvc/src/test/java/org/springframework/restdocs/mockmvc/MockMvcRestDocumentationIntegrationTests.java +++ b/spring-restdocs-mockmvc/src/test/java/org/springframework/restdocs/mockmvc/MockMvcRestDocumentationIntegrationTests.java @@ -496,7 +496,8 @@ public class MockMvcRestDocumentationIntegrationTests { public void exceptionShouldBeThrownWhenCallDocumentMockMvcNotConfigured() { MockMvc mockMvc = MockMvcBuilders.webAppContextSetup(this.context).build(); assertThatThrownBy(() -> mockMvc.perform(get("/").accept(MediaType.APPLICATION_JSON)).andDo(document("basic"))) - .isInstanceOf(IllegalStateException.class).hasMessageContaining(missingConfigurationMessage()); + .isInstanceOf(IllegalStateException.class).hasMessage("REST Docs configuration not found. Did you " + + "forget to apply a MockMvcRestDocumentationConfigurer when building the MockMvc instance?"); } @@ -506,13 +507,9 @@ public class MockMvcRestDocumentationIntegrationTests { MockMvc mockMvc = MockMvcBuilders.webAppContextSetup(this.context).build(); assertThatThrownBy(() -> mockMvc.perform(get("/").accept(MediaType.APPLICATION_JSON)) .andDo(documentation.document(responseHeaders(headerWithName("a").description("one"))))) - .isInstanceOf(IllegalStateException.class).hasMessageContaining(missingConfigurationMessage()); - } - - private String missingConfigurationMessage() { - return "There is no REST Docs configuration. Looks like " - + "'org.springframework.restdocs.mockmvc.MockMvcRestDocumentationConfigurer' " - + "was not invoked. Please check your configuration."; + .isInstanceOf(IllegalStateException.class) + .hasMessage("REST Docs configuration not found. Did you forget to apply a " + + "MockMvcRestDocumentationConfigurer when building the MockMvc instance?"); } @Test diff --git a/spring-restdocs-restassured/src/main/java/org/springframework/restdocs/restassured3/RestDocumentationFilter.java b/spring-restdocs-restassured/src/main/java/org/springframework/restdocs/restassured3/RestDocumentationFilter.java index eb629917..342d69eb 100644 --- a/spring-restdocs-restassured/src/main/java/org/springframework/restdocs/restassured3/RestDocumentationFilter.java +++ b/spring-restdocs-restassured/src/main/java/org/springframework/restdocs/restassured3/RestDocumentationFilter.java @@ -67,7 +67,7 @@ public class RestDocumentationFilter implements Filter { * @return the configuration */ protected Map getConfiguration(FilterableRequestSpecification requestSpec, FilterContext context) { - Map configuration = new HashMap<>(getRequiredConfiguration(context)); + Map configuration = new HashMap<>(retrieveConfiguration(context)); configuration.put(RestDocumentationContext.class.getName(), context.getValue(RestDocumentationContext.class.getName())); configuration.put(RestDocumentationGenerator.ATTRIBUTE_NAME_URL_TEMPLATE, requestSpec.getUserDefinedPath()); @@ -96,13 +96,12 @@ public class RestDocumentationFilter implements Filter { }; } - private static Map getRequiredConfiguration(FilterContext context) { + private static Map retrieveConfiguration(FilterContext context) { Map configuration = context.getValue(CONTEXT_KEY_CONFIGURATION); Assert.state(configuration != null, - () -> String.format( - "There is no REST Docs configuration. Looks like " - + "'%s' was not invoked. Please check your configuration.", - RestDocumentationFilter.class.getName())); + () -> "REST Docs configuration not found. Did you forget to add a " + + RestAssuredRestDocumentationConfigurer.class.getSimpleName() + + " as a filter when building the RequestSpecification?"); return configuration; } diff --git a/spring-restdocs-restassured/src/test/java/org/springframework/restdocs/restassured3/RestAssuredRestDocumentationIntegrationTests.java b/spring-restdocs-restassured/src/test/java/org/springframework/restdocs/restassured3/RestAssuredRestDocumentationIntegrationTests.java index c3b91f6b..492e71b2 100644 --- a/spring-restdocs-restassured/src/test/java/org/springframework/restdocs/restassured3/RestAssuredRestDocumentationIntegrationTests.java +++ b/spring-restdocs-restassured/src/test/java/org/springframework/restdocs/restassured3/RestAssuredRestDocumentationIntegrationTests.java @@ -343,7 +343,9 @@ public class RestAssuredRestDocumentationIntegrationTests { @Test public void exceptionShouldBeThrownWhenCallDocumentRequestSpecificationNotConfigured() { assertThatThrownBy(() -> given().port(tomcat.getPort()).filter(document("default")).get("/")) - .isInstanceOf(IllegalStateException.class).hasMessageContaining(messingConfigurationMessage()); + .isInstanceOf(IllegalStateException.class) + .hasMessage("REST Docs configuration not found. Did you forget to add a " + + "RestAssuredRestDocumentationConfigurer as a filter when building the RequestSpecification?"); } @Test @@ -351,14 +353,10 @@ public class RestAssuredRestDocumentationIntegrationTests { RestDocumentationFilter documentation = document("{method-name}-{step}"); assertThatThrownBy(() -> given().port(tomcat.getPort()) .filter(documentation.document(responseHeaders(headerWithName("a").description("one")))).get("/")) - .isInstanceOf(IllegalStateException.class).hasMessageContaining(messingConfigurationMessage()); - } - - private String messingConfigurationMessage() { - return "There is no REST Docs configuration. Looks like 'org.springframework." - + "restdocs.restassured3.RestDocumentationFilter' was not invoked." - + " Please check your configuration."; - + .isInstanceOf(IllegalStateException.class) + .hasMessage("REST Docs configuration not found. Did you forget to add a " + + "RestAssuredRestDocumentationConfigurer as a filter when building the " + + "RequestSpecification?"); } private void assertExpectedSnippetFilesExist(File directory, String... snippets) { diff --git a/spring-restdocs-webtestclient/src/main/java/org/springframework/restdocs/webtestclient/WebTestClientRestDocumentation.java b/spring-restdocs-webtestclient/src/main/java/org/springframework/restdocs/webtestclient/WebTestClientRestDocumentation.java index 8ed49c24..ce66df5f 100644 --- a/spring-restdocs-webtestclient/src/main/java/org/springframework/restdocs/webtestclient/WebTestClientRestDocumentation.java +++ b/spring-restdocs-webtestclient/src/main/java/org/springframework/restdocs/webtestclient/WebTestClientRestDocumentation.java @@ -16,7 +16,6 @@ package org.springframework.restdocs.webtestclient; -import java.util.HashMap; import java.util.Map; import java.util.function.Consumer; @@ -30,7 +29,6 @@ import org.springframework.test.web.reactive.server.WebTestClient; import org.springframework.test.web.reactive.server.WebTestClient.BodyContentSpec; import org.springframework.test.web.reactive.server.WebTestClient.BodySpec; import org.springframework.test.web.reactive.server.WebTestClient.Builder; -import org.springframework.util.Assert; import org.springframework.web.reactive.function.client.ExchangeFilterFunction; /** @@ -76,7 +74,7 @@ public abstract class WebTestClientRestDocumentation { */ public static Consumer document(String identifier, Snippet... snippets) { return (result) -> new RestDocumentationGenerator<>(identifier, REQUEST_CONVERTER, RESPONSE_CONVERTER, snippets) - .handle(result, result, getRequiredConfiguration(result)); + .handle(result, result, retrieveConfiguration(result)); } /** @@ -93,7 +91,7 @@ public abstract class WebTestClientRestDocumentation { public static Consumer document(String identifier, OperationRequestPreprocessor requestPreprocessor, Snippet... snippets) { return (result) -> new RestDocumentationGenerator<>(identifier, REQUEST_CONVERTER, RESPONSE_CONVERTER, - requestPreprocessor, snippets).handle(result, result, getRequiredConfiguration(result)); + requestPreprocessor, snippets).handle(result, result, retrieveConfiguration(result)); } /** @@ -110,7 +108,7 @@ public abstract class WebTestClientRestDocumentation { public static Consumer document(String identifier, OperationResponsePreprocessor responsePreprocessor, Snippet... snippets) { return (result) -> new RestDocumentationGenerator<>(identifier, REQUEST_CONVERTER, RESPONSE_CONVERTER, - responsePreprocessor, snippets).handle(result, result, getRequiredConfiguration(result)); + responsePreprocessor, snippets).handle(result, result, retrieveConfiguration(result)); } /** @@ -131,17 +129,12 @@ public abstract class WebTestClientRestDocumentation { Snippet... snippets) { return (result) -> new RestDocumentationGenerator<>(identifier, REQUEST_CONVERTER, RESPONSE_CONVERTER, requestPreprocessor, responsePreprocessor, snippets).handle(result, result, - getRequiredConfiguration(result)); + retrieveConfiguration(result)); } - private static Map getRequiredConfiguration(ExchangeResult result) { - Map config = WebTestClientRestDocumentationConfigurer + private static Map retrieveConfiguration(ExchangeResult result) { + Map configuration = WebTestClientRestDocumentationConfigurer .retrieveConfiguration(result.getRequestHeaders()); - Assert.state(config != null, - () -> String.format("There is no REST Docs configuration. Looks like '%s' " - + "was not invoked or configuration has already been removed. Please check your configuration.", - WebTestClientRestDocumentationConfigurer.class.getName())); - Map configuration = new HashMap<>(config); configuration.put(RestDocumentationGenerator.ATTRIBUTE_NAME_URL_TEMPLATE, result.getUriTemplate()); return configuration; } diff --git a/spring-restdocs-webtestclient/src/main/java/org/springframework/restdocs/webtestclient/WebTestClientRestDocumentationConfigurer.java b/spring-restdocs-webtestclient/src/main/java/org/springframework/restdocs/webtestclient/WebTestClientRestDocumentationConfigurer.java index f210d3ed..e6d5299d 100644 --- a/spring-restdocs-webtestclient/src/main/java/org/springframework/restdocs/webtestclient/WebTestClientRestDocumentationConfigurer.java +++ b/spring-restdocs-webtestclient/src/main/java/org/springframework/restdocs/webtestclient/WebTestClientRestDocumentationConfigurer.java @@ -29,6 +29,7 @@ import org.springframework.restdocs.RestDocumentationContext; import org.springframework.restdocs.RestDocumentationContextProvider; import org.springframework.restdocs.config.RestDocumentationConfigurer; import org.springframework.test.web.reactive.server.WebTestClient; +import org.springframework.util.Assert; import org.springframework.util.StringUtils; import org.springframework.web.reactive.function.client.ClientRequest; import org.springframework.web.reactive.function.client.ClientResponse; @@ -78,7 +79,10 @@ public class WebTestClientRestDocumentationConfigurer extends static Map retrieveConfiguration(HttpHeaders headers) { String requestId = headers.getFirst(WebTestClient.WEBTESTCLIENT_REQUEST_ID); - return configurations.remove(requestId); + Map configuration = configurations.remove(requestId); + Assert.state(configuration != null, () -> "REST Docs configuration not found. Did you forget to register a " + + WebTestClientRestDocumentationConfigurer.class.getSimpleName() + " as a filter?"); + return configuration; } @Override diff --git a/spring-restdocs-webtestclient/src/test/java/org/springframework/restdocs/webtestclient/WebTestClientRestDocumentationConfigurerTests.java b/spring-restdocs-webtestclient/src/test/java/org/springframework/restdocs/webtestclient/WebTestClientRestDocumentationConfigurerTests.java index 920b8582..74a3a4aa 100644 --- a/spring-restdocs-webtestclient/src/test/java/org/springframework/restdocs/webtestclient/WebTestClientRestDocumentationConfigurerTests.java +++ b/spring-restdocs-webtestclient/src/test/java/org/springframework/restdocs/webtestclient/WebTestClientRestDocumentationConfigurerTests.java @@ -29,6 +29,7 @@ import org.springframework.web.reactive.function.client.ClientRequest; import org.springframework.web.reactive.function.client.ExchangeFunction; import static org.assertj.core.api.Assertions.assertThat; +import static org.assertj.core.api.Assertions.assertThatIllegalStateException; import static org.mockito.Mockito.mock; import static org.mockito.Mockito.verify; @@ -51,7 +52,8 @@ public class WebTestClientRestDocumentationConfigurerTests { .header(WebTestClient.WEBTESTCLIENT_REQUEST_ID, "1").build(); this.configurer.filter(request, mock(ExchangeFunction.class)); assertThat(WebTestClientRestDocumentationConfigurer.retrieveConfiguration(request.headers())).isNotNull(); - assertThat(WebTestClientRestDocumentationConfigurer.retrieveConfiguration(request.headers())).isNull(); + assertThatIllegalStateException() + .isThrownBy(() -> WebTestClientRestDocumentationConfigurer.retrieveConfiguration(request.headers())); } @Test diff --git a/spring-restdocs-webtestclient/src/test/java/org/springframework/restdocs/webtestclient/WebTestClientRestDocumentationIntegrationTests.java b/spring-restdocs-webtestclient/src/test/java/org/springframework/restdocs/webtestclient/WebTestClientRestDocumentationIntegrationTests.java index e09633fe..06199f8c 100644 --- a/spring-restdocs-webtestclient/src/test/java/org/springframework/restdocs/webtestclient/WebTestClientRestDocumentationIntegrationTests.java +++ b/spring-restdocs-webtestclient/src/test/java/org/springframework/restdocs/webtestclient/WebTestClientRestDocumentationIntegrationTests.java @@ -180,13 +180,9 @@ public class WebTestClientRestDocumentationIntegrationTests { assertThatThrownBy(() -> this.webTestClient .mutateWith((builder, httpHandlerBuilder, connector) -> builder.filters(List::clear).build()).get() .uri("/").exchange().expectBody().consumeWith(document("default-snippets"))) - .isInstanceOf(IllegalStateException.class).hasMessageContaining(missingConfiguration()); - } - - private String missingConfiguration() { - return "There is no REST Docs configuration. Looks like " - + "'org.springframework.restdocs.webtestclient.WebTestClientRestDocumentationConfigurer' " - + "was not invoked or configuration has already been removed. Please check your configuration."; + .isInstanceOf(IllegalStateException.class) + .hasMessage("REST Docs configuration not found. Did you forget to register a " + + "WebTestClientRestDocumentationConfigurer as a filter?"); } private void assertExpectedSnippetFilesExist(File directory, String... snippets) {