From d5ae59dad70439a501ea18060464e45dd53af586 Mon Sep 17 00:00:00 2001 From: Brian Clozel Date: Tue, 8 Jan 2019 18:58:48 +0100 Subject: [PATCH] Polish Closes gh-15594 --- ...faultRestTemplateExchangeTagsProvider.java | 2 +- .../web/client/RestTemplateExchangeTags.java | 44 +++++++++-------- .../DefaultWebClientExchangeTagsProvider.java | 2 +- .../client/WebClientExchangeTags.java | 17 ++++--- .../client/RestTemplateExchangeTagsTests.java | 49 ++++++++++++++----- ...ultWebClientExchangeTagsProviderTests.java | 2 +- .../client/WebClientExchangeTagsTests.java | 9 +++- 7 files changed, 81 insertions(+), 44 deletions(-) diff --git a/spring-boot-project/spring-boot-actuator/src/main/java/org/springframework/boot/actuate/metrics/web/client/DefaultRestTemplateExchangeTagsProvider.java b/spring-boot-project/spring-boot-actuator/src/main/java/org/springframework/boot/actuate/metrics/web/client/DefaultRestTemplateExchangeTagsProvider.java index d0635fa140..f0cd542e79 100644 --- a/spring-boot-project/spring-boot-actuator/src/main/java/org/springframework/boot/actuate/metrics/web/client/DefaultRestTemplateExchangeTagsProvider.java +++ b/spring-boot-project/spring-boot-actuator/src/main/java/org/springframework/boot/actuate/metrics/web/client/DefaultRestTemplateExchangeTagsProvider.java @@ -1,5 +1,5 @@ /* - * Copyright 2012-2018 the original author or authors. + * Copyright 2012-2019 the original author or authors. * * Licensed under the Apache License, Version 2.0 (the "License"); * you may not use this file except in compliance with the License. diff --git a/spring-boot-project/spring-boot-actuator/src/main/java/org/springframework/boot/actuate/metrics/web/client/RestTemplateExchangeTags.java b/spring-boot-project/spring-boot-actuator/src/main/java/org/springframework/boot/actuate/metrics/web/client/RestTemplateExchangeTags.java index 4c647a97d2..33d4310576 100644 --- a/spring-boot-project/spring-boot-actuator/src/main/java/org/springframework/boot/actuate/metrics/web/client/RestTemplateExchangeTags.java +++ b/spring-boot-project/spring-boot-actuator/src/main/java/org/springframework/boot/actuate/metrics/web/client/RestTemplateExchangeTags.java @@ -1,5 +1,5 @@ /* - * Copyright 2012-2018 the original author or authors. + * Copyright 2012-2019 the original author or authors. * * Licensed under the Apache License, Version 2.0 (the "License"); * you may not use this file except in compliance with the License. @@ -35,6 +35,7 @@ import org.springframework.web.client.RestTemplate; * @author Andy Wilkinson * @author Jon Schneider * @author Nishant Raut + * @author Brian Clozel * @since 2.0.0 */ public final class RestTemplateExchangeTags { @@ -137,36 +138,37 @@ public final class RestTemplateExchangeTags { * @since 2.2.0 */ public static Tag outcome(ClientHttpResponse response) { - if (response != null) { - HttpStatus status = extractStatus(response); - if (status != null) { - if (status.is1xxInformational()) { - return OUTCOME_INFORMATIONAL; - } - if (status.is2xxSuccessful()) { - return OUTCOME_SUCCESS; - } - if (status.is3xxRedirection()) { - return OUTCOME_REDIRECTION; - } - if (status.is4xxClientError()) { - return OUTCOME_CLIENT_ERROR; - } + HttpStatus status = extractStatus(response); + if (status != null) { + if (status.is1xxInformational()) { + return OUTCOME_INFORMATIONAL; + } + if (status.is2xxSuccessful()) { + return OUTCOME_SUCCESS; + } + if (status.is3xxRedirection()) { + return OUTCOME_REDIRECTION; + } + if (status.is4xxClientError()) { + return OUTCOME_CLIENT_ERROR; + } + if (status.is5xxServerError()) { + return OUTCOME_SERVER_ERROR; } - return OUTCOME_SERVER_ERROR; } return OUTCOME_UNKNOWN; } private static HttpStatus extractStatus(ClientHttpResponse response) { - try { - return response.getStatusCode(); + if (response != null) { + return response.getStatusCode(); + } + return null; } - catch (IOException ex) { + catch (IOException | IllegalArgumentException exc) { return null; } - } } diff --git a/spring-boot-project/spring-boot-actuator/src/main/java/org/springframework/boot/actuate/metrics/web/reactive/client/DefaultWebClientExchangeTagsProvider.java b/spring-boot-project/spring-boot-actuator/src/main/java/org/springframework/boot/actuate/metrics/web/reactive/client/DefaultWebClientExchangeTagsProvider.java index 1c012caa49..1ed63883b7 100644 --- a/spring-boot-project/spring-boot-actuator/src/main/java/org/springframework/boot/actuate/metrics/web/reactive/client/DefaultWebClientExchangeTagsProvider.java +++ b/spring-boot-project/spring-boot-actuator/src/main/java/org/springframework/boot/actuate/metrics/web/reactive/client/DefaultWebClientExchangeTagsProvider.java @@ -1,5 +1,5 @@ /* - * Copyright 2012-2018 the original author or authors. + * Copyright 2012-2019 the original author or authors. * * Licensed under the Apache License, Version 2.0 (the "License"); * you may not use this file except in compliance with the License. diff --git a/spring-boot-project/spring-boot-actuator/src/main/java/org/springframework/boot/actuate/metrics/web/reactive/client/WebClientExchangeTags.java b/spring-boot-project/spring-boot-actuator/src/main/java/org/springframework/boot/actuate/metrics/web/reactive/client/WebClientExchangeTags.java index a28ef9a0c2..8d488c25ae 100644 --- a/spring-boot-project/spring-boot-actuator/src/main/java/org/springframework/boot/actuate/metrics/web/reactive/client/WebClientExchangeTags.java +++ b/spring-boot-project/spring-boot-actuator/src/main/java/org/springframework/boot/actuate/metrics/web/reactive/client/WebClientExchangeTags.java @@ -1,5 +1,5 @@ /* - * Copyright 2012-2018 the original author or authors. + * Copyright 2012-2019 the original author or authors. * * Licensed under the Apache License, Version 2.0 (the "License"); * you may not use this file except in compliance with the License. @@ -133,9 +133,9 @@ public final class WebClientExchangeTags { * @since 2.2.0 */ public static Tag outcome(ClientResponse response) { - if (response != null) { - HttpStatus status = response.statusCode(); - if (status != null) { + try { + if (response != null) { + HttpStatus status = response.statusCode(); if (status.is1xxInformational()) { return OUTCOME_INFORMATIONAL; } @@ -148,10 +148,15 @@ public final class WebClientExchangeTags { if (status.is4xxClientError()) { return OUTCOME_CLIENT_ERROR; } + if (status.is5xxServerError()) { + return OUTCOME_SERVER_ERROR; + } } - return OUTCOME_SERVER_ERROR; + return OUTCOME_UNKNOWN; + } + catch (IllegalArgumentException exc) { + return OUTCOME_UNKNOWN; } - return OUTCOME_UNKNOWN; } } diff --git a/spring-boot-project/spring-boot-actuator/src/test/java/org/springframework/boot/actuate/metrics/web/client/RestTemplateExchangeTagsTests.java b/spring-boot-project/spring-boot-actuator/src/test/java/org/springframework/boot/actuate/metrics/web/client/RestTemplateExchangeTagsTests.java index cfc7dba306..14614b53e6 100644 --- a/spring-boot-project/spring-boot-actuator/src/test/java/org/springframework/boot/actuate/metrics/web/client/RestTemplateExchangeTagsTests.java +++ b/spring-boot-project/spring-boot-actuator/src/test/java/org/springframework/boot/actuate/metrics/web/client/RestTemplateExchangeTagsTests.java @@ -1,5 +1,5 @@ /* - * Copyright 2012-2018 the original author or authors. + * Copyright 2012-2019 the original author or authors. * * Licensed under the Apache License, Version 2.0 (the "License"); * you may not use this file except in compliance with the License. @@ -16,23 +16,28 @@ package org.springframework.boot.actuate.metrics.web.client; +import java.io.IOException; + import io.micrometer.core.instrument.Tag; import org.junit.Test; import org.springframework.http.HttpStatus; +import org.springframework.http.client.ClientHttpResponse; import org.springframework.mock.http.client.MockClientHttpResponse; import static org.assertj.core.api.Assertions.assertThat; +import static org.mockito.BDDMockito.given; +import static org.mockito.Mockito.mock; /** * Tests for {@link RestTemplateExchangeTags}. * * @author Nishant Raut + * @author Brian Clozel + * @author Brian Clozel */ public class RestTemplateExchangeTagsTests { - private MockClientHttpResponse response; - @Test public void outcomeTagIsUnknownWhenResponseStatusIsNull() { Tag tag = RestTemplateExchangeTags.outcome(null); @@ -41,40 +46,58 @@ public class RestTemplateExchangeTagsTests { @Test public void outcomeTagIsInformationalWhenResponseIs1xx() { - this.response = new MockClientHttpResponse("foo".getBytes(), HttpStatus.CONTINUE); - Tag tag = RestTemplateExchangeTags.outcome(this.response); + ClientHttpResponse response = new MockClientHttpResponse("foo".getBytes(), + HttpStatus.CONTINUE); + Tag tag = RestTemplateExchangeTags.outcome(response); assertThat(tag.getValue()).isEqualTo("INFORMATIONAL"); } @Test public void outcomeTagIsSuccessWhenResponseIs2xx() { - this.response = new MockClientHttpResponse("foo".getBytes(), HttpStatus.OK); - Tag tag = RestTemplateExchangeTags.outcome(this.response); + ClientHttpResponse response = new MockClientHttpResponse("foo".getBytes(), + HttpStatus.OK); + Tag tag = RestTemplateExchangeTags.outcome(response); assertThat(tag.getValue()).isEqualTo("SUCCESS"); } @Test public void outcomeTagIsRedirectionWhenResponseIs3xx() { - this.response = new MockClientHttpResponse("foo".getBytes(), + ClientHttpResponse response = new MockClientHttpResponse("foo".getBytes(), HttpStatus.MOVED_PERMANENTLY); - Tag tag = RestTemplateExchangeTags.outcome(this.response); + Tag tag = RestTemplateExchangeTags.outcome(response); assertThat(tag.getValue()).isEqualTo("REDIRECTION"); } @Test public void outcomeTagIsClientErrorWhenResponseIs4xx() { - this.response = new MockClientHttpResponse("foo".getBytes(), + ClientHttpResponse response = new MockClientHttpResponse("foo".getBytes(), HttpStatus.BAD_REQUEST); - Tag tag = RestTemplateExchangeTags.outcome(this.response); + Tag tag = RestTemplateExchangeTags.outcome(response); assertThat(tag.getValue()).isEqualTo("CLIENT_ERROR"); } @Test public void outcomeTagIsServerErrorWhenResponseIs5xx() { - this.response = new MockClientHttpResponse("foo".getBytes(), + ClientHttpResponse response = new MockClientHttpResponse("foo".getBytes(), HttpStatus.BAD_GATEWAY); - Tag tag = RestTemplateExchangeTags.outcome(this.response); + Tag tag = RestTemplateExchangeTags.outcome(response); assertThat(tag.getValue()).isEqualTo("SERVER_ERROR"); } + @Test + public void outcomeTagIsUnknownWhenResponseThrowsIOException() throws Exception { + ClientHttpResponse response = mock(ClientHttpResponse.class); + given(response.getStatusCode()).willThrow(IOException.class); + Tag tag = RestTemplateExchangeTags.outcome(response); + assertThat(tag.getValue()).isEqualTo("UNKNOWN"); + } + + @Test + public void outcomeTagIsUnknownForCustomResponseStatus() throws Exception { + ClientHttpResponse response = mock(ClientHttpResponse.class); + given(response.getStatusCode()).willThrow(IllegalArgumentException.class); + Tag tag = RestTemplateExchangeTags.outcome(response); + assertThat(tag.getValue()).isEqualTo("UNKNOWN"); + } + } diff --git a/spring-boot-project/spring-boot-actuator/src/test/java/org/springframework/boot/actuate/metrics/web/reactive/client/DefaultWebClientExchangeTagsProviderTests.java b/spring-boot-project/spring-boot-actuator/src/test/java/org/springframework/boot/actuate/metrics/web/reactive/client/DefaultWebClientExchangeTagsProviderTests.java index 377808da66..be906a5cf1 100644 --- a/spring-boot-project/spring-boot-actuator/src/test/java/org/springframework/boot/actuate/metrics/web/reactive/client/DefaultWebClientExchangeTagsProviderTests.java +++ b/spring-boot-project/spring-boot-actuator/src/test/java/org/springframework/boot/actuate/metrics/web/reactive/client/DefaultWebClientExchangeTagsProviderTests.java @@ -1,5 +1,5 @@ /* - * Copyright 2012-2018 the original author or authors. + * Copyright 2012-2019 the original author or authors. * * Licensed under the Apache License, Version 2.0 (the "License"); * you may not use this file except in compliance with the License. diff --git a/spring-boot-project/spring-boot-actuator/src/test/java/org/springframework/boot/actuate/metrics/web/reactive/client/WebClientExchangeTagsTests.java b/spring-boot-project/spring-boot-actuator/src/test/java/org/springframework/boot/actuate/metrics/web/reactive/client/WebClientExchangeTagsTests.java index 24c5ee9d5e..686db658a5 100644 --- a/spring-boot-project/spring-boot-actuator/src/test/java/org/springframework/boot/actuate/metrics/web/reactive/client/WebClientExchangeTagsTests.java +++ b/spring-boot-project/spring-boot-actuator/src/test/java/org/springframework/boot/actuate/metrics/web/reactive/client/WebClientExchangeTagsTests.java @@ -1,5 +1,5 @@ /* - * Copyright 2012-2018 the original author or authors. + * Copyright 2012-2019 the original author or authors. * * Licensed under the Apache License, Version 2.0 (the "License"); * you may not use this file except in compliance with the License. @@ -155,4 +155,11 @@ public class WebClientExchangeTagsTests { assertThat(tag.getValue()).isEqualTo("SERVER_ERROR"); } + @Test + public void outcomeTagIsUknownWhenResponseStatusIsUknown() { + given(this.response.statusCode()).willThrow(IllegalArgumentException.class); + Tag tag = WebClientExchangeTags.outcome(this.response); + assertThat(tag.getValue()).isEqualTo("UNKNOWN"); + } + }