From efdc19647cb890994bfc65e7f83489b5b2e5d121 Mon Sep 17 00:00:00 2001 From: Safeer A Date: Tue, 17 Aug 2021 01:49:43 +0400 Subject: [PATCH 1/2] Avoid NPE when routing DataSource has target with null key See gh-27698 --- .../jdbc/DataSourceHealthContributorAutoConfiguration.java | 7 ++++++- .../DataSourceHealthContributorAutoConfigurationTests.java | 5 ++++- 2 files changed, 10 insertions(+), 2 deletions(-) diff --git a/spring-boot-project/spring-boot-actuator-autoconfigure/src/main/java/org/springframework/boot/actuate/autoconfigure/jdbc/DataSourceHealthContributorAutoConfiguration.java b/spring-boot-project/spring-boot-actuator-autoconfigure/src/main/java/org/springframework/boot/actuate/autoconfigure/jdbc/DataSourceHealthContributorAutoConfiguration.java index be84c6efa7..aeb0785800 100644 --- a/spring-boot-project/spring-boot-actuator-autoconfigure/src/main/java/org/springframework/boot/actuate/autoconfigure/jdbc/DataSourceHealthContributorAutoConfiguration.java +++ b/spring-boot-project/spring-boot-actuator-autoconfigure/src/main/java/org/springframework/boot/actuate/autoconfigure/jdbc/DataSourceHealthContributorAutoConfiguration.java @@ -19,6 +19,7 @@ package org.springframework.boot.actuate.autoconfigure.jdbc; import java.util.Collection; import java.util.Iterator; import java.util.Map; +import java.util.Optional; import java.util.function.Function; import java.util.stream.Collectors; @@ -57,6 +58,7 @@ import org.springframework.util.Assert; * @author Stephane Nicoll * @author Arthur Kalimullin * @author Julio Gomez + * @author Safeer Ansari * @since 2.0.0 */ @Configuration(proxyBeanMethods = false) @@ -124,10 +126,13 @@ public class DataSourceHealthContributorAutoConfiguration implements Initializin private final CompositeHealthContributor delegate; + private static final String UNNAMED_DATASOURCE_KEY = "unnamed"; + RoutingDataSourceHealthContributor(AbstractRoutingDataSource routingDataSource, Function contributorFunction) { Map routedDataSources = routingDataSource.getResolvedDataSources().entrySet().stream() - .collect(Collectors.toMap((e) -> e.getKey().toString(), Map.Entry::getValue)); + .collect(Collectors.toMap((e) -> Optional.ofNullable(e.getKey()).map(Object::toString) + .orElse(UNNAMED_DATASOURCE_KEY), Map.Entry::getValue)); this.delegate = CompositeHealthContributor.fromMap(routedDataSources, contributorFunction); } diff --git a/spring-boot-project/spring-boot-actuator-autoconfigure/src/test/java/org/springframework/boot/actuate/autoconfigure/jdbc/DataSourceHealthContributorAutoConfigurationTests.java b/spring-boot-project/spring-boot-actuator-autoconfigure/src/test/java/org/springframework/boot/actuate/autoconfigure/jdbc/DataSourceHealthContributorAutoConfigurationTests.java index 91d7eac8e5..2d86bf1f98 100644 --- a/spring-boot-project/spring-boot-actuator-autoconfigure/src/test/java/org/springframework/boot/actuate/autoconfigure/jdbc/DataSourceHealthContributorAutoConfigurationTests.java +++ b/spring-boot-project/spring-boot-actuator-autoconfigure/src/test/java/org/springframework/boot/actuate/autoconfigure/jdbc/DataSourceHealthContributorAutoConfigurationTests.java @@ -49,6 +49,7 @@ import static org.mockito.Mockito.mock; * * @author Phillip Webb * @author Julio Gomez + * @author Safeer Ansari */ class DataSourceHealthContributorAutoConfigurationTests { @@ -102,10 +103,11 @@ class DataSourceHealthContributorAutoConfigurationTests { assertThat(context).hasSingleBean(RoutingDataSourceHealthContributor.class); RoutingDataSourceHealthContributor routingHealthContributor = context .getBean(RoutingDataSourceHealthContributor.class); + assertThat(routingHealthContributor.getContributor("unnamed")).isInstanceOf(DataSourceHealthIndicator.class); assertThat(routingHealthContributor.getContributor("one")).isInstanceOf(DataSourceHealthIndicator.class); assertThat(routingHealthContributor.getContributor("two")).isInstanceOf(DataSourceHealthIndicator.class); assertThat(routingHealthContributor.iterator()).toIterable().extracting("name") - .containsExactlyInAnyOrder("one", "two"); + .containsExactlyInAnyOrder("unnamed", "one", "two"); }); } @@ -155,6 +157,7 @@ class DataSourceHealthContributorAutoConfigurationTests { @Bean AbstractRoutingDataSource routingDataSource() { Map dataSources = new HashMap<>(); + dataSources.put(null, mock(DataSource.class)); dataSources.put("one", mock(DataSource.class)); dataSources.put("two", mock(DataSource.class)); AbstractRoutingDataSource routingDataSource = mock(AbstractRoutingDataSource.class); From 24c88ab80b6e0f2188af1d2f0322db6dba60aa23 Mon Sep 17 00:00:00 2001 From: Andy Wilkinson Date: Mon, 23 Aug 2021 13:53:34 +0100 Subject: [PATCH 2/2] Polish "Avoid NPE when routing DataSource has target with null key" See gh-27698 --- ...rceHealthContributorAutoConfiguration.java | 6 ++-- ...althContributorAutoConfigurationTests.java | 33 +++++++++++++++++-- 2 files changed, 33 insertions(+), 6 deletions(-) diff --git a/spring-boot-project/spring-boot-actuator-autoconfigure/src/main/java/org/springframework/boot/actuate/autoconfigure/jdbc/DataSourceHealthContributorAutoConfiguration.java b/spring-boot-project/spring-boot-actuator-autoconfigure/src/main/java/org/springframework/boot/actuate/autoconfigure/jdbc/DataSourceHealthContributorAutoConfiguration.java index aeb0785800..26d2ba4681 100644 --- a/spring-boot-project/spring-boot-actuator-autoconfigure/src/main/java/org/springframework/boot/actuate/autoconfigure/jdbc/DataSourceHealthContributorAutoConfiguration.java +++ b/spring-boot-project/spring-boot-actuator-autoconfigure/src/main/java/org/springframework/boot/actuate/autoconfigure/jdbc/DataSourceHealthContributorAutoConfiguration.java @@ -19,7 +19,7 @@ package org.springframework.boot.actuate.autoconfigure.jdbc; import java.util.Collection; import java.util.Iterator; import java.util.Map; -import java.util.Optional; +import java.util.Objects; import java.util.function.Function; import java.util.stream.Collectors; @@ -131,8 +131,8 @@ public class DataSourceHealthContributorAutoConfiguration implements Initializin RoutingDataSourceHealthContributor(AbstractRoutingDataSource routingDataSource, Function contributorFunction) { Map routedDataSources = routingDataSource.getResolvedDataSources().entrySet().stream() - .collect(Collectors.toMap((e) -> Optional.ofNullable(e.getKey()).map(Object::toString) - .orElse(UNNAMED_DATASOURCE_KEY), Map.Entry::getValue)); + .collect(Collectors.toMap((e) -> Objects.toString(e.getKey(), UNNAMED_DATASOURCE_KEY), + Map.Entry::getValue)); this.delegate = CompositeHealthContributor.fromMap(routedDataSources, contributorFunction); } diff --git a/spring-boot-project/spring-boot-actuator-autoconfigure/src/test/java/org/springframework/boot/actuate/autoconfigure/jdbc/DataSourceHealthContributorAutoConfigurationTests.java b/spring-boot-project/spring-boot-actuator-autoconfigure/src/test/java/org/springframework/boot/actuate/autoconfigure/jdbc/DataSourceHealthContributorAutoConfigurationTests.java index 2d86bf1f98..8316f0acd9 100644 --- a/spring-boot-project/spring-boot-actuator-autoconfigure/src/test/java/org/springframework/boot/actuate/autoconfigure/jdbc/DataSourceHealthContributorAutoConfigurationTests.java +++ b/spring-boot-project/spring-boot-actuator-autoconfigure/src/test/java/org/springframework/boot/actuate/autoconfigure/jdbc/DataSourceHealthContributorAutoConfigurationTests.java @@ -103,11 +103,10 @@ class DataSourceHealthContributorAutoConfigurationTests { assertThat(context).hasSingleBean(RoutingDataSourceHealthContributor.class); RoutingDataSourceHealthContributor routingHealthContributor = context .getBean(RoutingDataSourceHealthContributor.class); - assertThat(routingHealthContributor.getContributor("unnamed")).isInstanceOf(DataSourceHealthIndicator.class); assertThat(routingHealthContributor.getContributor("one")).isInstanceOf(DataSourceHealthIndicator.class); assertThat(routingHealthContributor.getContributor("two")).isInstanceOf(DataSourceHealthIndicator.class); assertThat(routingHealthContributor.iterator()).toIterable().extracting("name") - .containsExactlyInAnyOrder("unnamed", "one", "two"); + .containsExactlyInAnyOrder("one", "two"); }); } @@ -138,6 +137,20 @@ class DataSourceHealthContributorAutoConfigurationTests { .doesNotHaveBean(CompositeHealthContributor.class)); } + @Test + void runWhenDataSourceHasNullRoutingKeyShouldProduceUnnamedComposedIndicator() { + this.contextRunner.withUserConfiguration(NullKeyRoutingDataSourceConfig.class).run((context) -> { + assertThat(context).hasSingleBean(RoutingDataSourceHealthContributor.class); + RoutingDataSourceHealthContributor routingHealthContributor = context + .getBean(RoutingDataSourceHealthContributor.class); + assertThat(routingHealthContributor.getContributor("unnamed")) + .isInstanceOf(DataSourceHealthIndicator.class); + assertThat(routingHealthContributor.getContributor("one")).isInstanceOf(DataSourceHealthIndicator.class); + assertThat(routingHealthContributor.iterator()).toIterable().extracting("name") + .containsExactlyInAnyOrder("unnamed", "one"); + }); + } + @Configuration(proxyBeanMethods = false) @EnableConfigurationProperties static class DataSourceConfig { @@ -157,7 +170,6 @@ class DataSourceHealthContributorAutoConfigurationTests { @Bean AbstractRoutingDataSource routingDataSource() { Map dataSources = new HashMap<>(); - dataSources.put(null, mock(DataSource.class)); dataSources.put("one", mock(DataSource.class)); dataSources.put("two", mock(DataSource.class)); AbstractRoutingDataSource routingDataSource = mock(AbstractRoutingDataSource.class); @@ -167,4 +179,19 @@ class DataSourceHealthContributorAutoConfigurationTests { } + @Configuration(proxyBeanMethods = false) + static class NullKeyRoutingDataSourceConfig { + + @Bean + AbstractRoutingDataSource routingDataSource() { + Map dataSources = new HashMap<>(); + dataSources.put(null, mock(DataSource.class)); + dataSources.put("one", mock(DataSource.class)); + AbstractRoutingDataSource routingDataSource = mock(AbstractRoutingDataSource.class); + given(routingDataSource.getResolvedDataSources()).willReturn(dataSources); + return routingDataSource; + } + + } + }