SpringBoot RoutingDataSourceHealthContributor should heed Maps with null values in keys

https://github.com/spring-projects/spring-boot/blob/b65cc4d62f4a818e36be8b99b98eba01a28f6f2a/spring-boot-project/spring-boot-actuator-autoconfigure/src/main/java/org/springframework/boot/actuate/autoconfigure/jdbc/DataSourceHealthContributorAutoConfiguration.java#L130

I just upgraded our project from springboot 2.4.1 to 2.5.3. I am super exited, that there is support for AbstractRoutingDataSource now. Thank' and kudos for that! Due to the update the health check now run's into an NPE which kills the whole application. It happens because the entryset 'routingDataSource.getResolvedDataSources().entrySet()' contains null as a key.

We create our datasource like this:

@Bean
@Primary
public DataSource dataSource() {
  final RoutingDataSource routingDataSource = new RoutingDataSource();

  final DataSource primaryDataSource = buildDataSource("PrimaryHikariPool", primaryDataSourceProperties());
  final DataSource replicaDataSource = buildDataSource("ReplicaHikariPool", replicaDataSourceProperties());

  final Map<Object, Object> targetDataSources = new HashMap<>();
  // HERE WE PUT IN OUR EVIL null KEY
  targetDataSources.put(null, primaryDataSource);
  targetDataSources.put(RoutingDataSource.Route.REPLICA, replicaDataSource);

  routingDataSource.setTargetDataSources(targetDataSources);
  routingDataSource.setDefaultTargetDataSource(primaryDataSource);

  return routingDataSource;
}

It might be strange or even stupid to put 'null' as a key into a map, but the RoutingDataSourceHealthContributor shouldn't crash because of that anyway.

Comment From: thegeekyasian

@philwebb, a PR has been created to fix the null pointer by filtering the entries with the non-null only. Can you please review and share your thoughts?

Comment From: philwebb

Thanks @thegeekyasian, I've added a comment to the pull-request.

Comment From: timmalich

Thanks @thegeekyasian, I've added a comment to the pull-request.

@philwebb, a PR has been created to fix the null pointer by filtering the entries with the non-null only. Can you please review and share your thoughts?

Thank you both very much.

Wouldn't that filter just drop an otherwise valid configuration? Wouldn't it be better to check null within the next line? Like: e.getKey()==null?null:e.getKey().toString()

Option 2) Would it be possible to at least log a warning?

Comment From: philwebb

@timmalich Let's carry on the discussion in the PR. I'll copy your comment there.

Comment From: philwebb

Closing in favor of PR #27698