Merge pull request #31676 from terminux

* pr/31676: Polish "Call the value adapter during NamedContributorsMapAdapter construction" Call the value adapter during NamedContributorsMapAdapter construction Closes gh-31676
spring-projects · Jul 26, 2022 · 1e886bd · 1e886bd
2 parents 5243cb8 + 59c9a9c
commit 1e886bd
Show file tree

Hide file tree

Showing 2 changed files with 38 additions and 18 deletions.
diff --git a/...or/src/main/java/org/springframework/boot/actuate/health/NamedContributorsMapAdapter.java b/...or/src/main/java/org/springframework/boot/actuate/health/NamedContributorsMapAdapter.java
@@ -1,5 +1,5 @@
 /*
- * Copyright 2012-2021 the original author or authors.
+ * Copyright 2012-2022 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.
@@ -18,10 +18,10 @@
 
 import java.util.Collections;
 import java.util.Iterator;
-import java.util.LinkedHashMap;
 import java.util.Map;
 import java.util.Map.Entry;
 import java.util.function.Function;
+import java.util.stream.Collectors;
 
 import org.springframework.util.Assert;
 
@@ -31,34 +31,36 @@
  * @param <V> the value type
  * @param <C> the contributor type
  * @author Phillip Webb
+ * @author Guirong Hu
  * @see CompositeHealthContributorMapAdapter
  * @see CompositeReactiveHealthContributorMapAdapter
  */
 abstract class NamedContributorsMapAdapter<V, C> implements NamedContributors<C> {
 
-	private final Map<String, V> map;
-
-	private final Function<V, ? extends C> valueAdapter;
+	private final Map<String, C> map;
 
 	NamedContributorsMapAdapter(Map<String, V> map, Function<V, ? extends C> valueAdapter) {
 		Assert.notNull(map, "Map must not be null");
 		Assert.notNull(valueAdapter, "ValueAdapter must not be null");
 		map.keySet().forEach(this::validateKey);
-		map.values().stream().map(valueAdapter)
-				.forEach((value) -> Assert.notNull(value, "Map must not contain null values"));
-		this.map = Collections.unmodifiableMap(new LinkedHashMap<>(map));
-		this.valueAdapter = valueAdapter;
+		this.map = Collections.unmodifiableMap(map.entrySet().stream()
+				.collect(Collectors.toMap(Entry::getKey, (entry) -> adapt(entry.getValue(), valueAdapter))));
 	}
 
 	private void validateKey(String value) {
 		Assert.notNull(value, "Map must not contain null keys");
 		Assert.isTrue(!value.contains("/"), "Map keys must not contain a '/'");
+	}
 
+	private C adapt(V value, Function<V, ? extends C> valueAdapter) {
+		C contributor = (value != null) ? valueAdapter.apply(value) : null;
+		Assert.notNull(contributor, "Map must not contain null values");
+		return contributor;
 	}
 
 	@Override
 	public Iterator<NamedContributor<C>> iterator() {
-		Iterator<Entry<String, V>> iterator = this.map.entrySet().iterator();
+		Iterator<Entry<String, C>> iterator = this.map.entrySet().iterator();
 		return new Iterator<NamedContributor<C>>() {
 
 			@Override
@@ -68,20 +70,16 @@ public boolean hasNext() {
 
 			@Override
 			public NamedContributor<C> next() {
-				Entry<String, V> entry = iterator.next();
-				return NamedContributor.of(entry.getKey(), adapt(entry.getValue()));
+				Entry<String, C> entry = iterator.next();
+				return NamedContributor.of(entry.getKey(), entry.getValue());
 			}
 
 		};
 	}
 
 	@Override
 	public C getContributor(String name) {
-		return adapt(this.map.get(name));
-	}
-
-	private C adapt(V value) {
-		return (value != null) ? this.valueAdapter.apply(value) : null;
+		return this.map.get(name);
 	}
 
 }
diff --git a/...c/test/java/org/springframework/boot/actuate/health/NamedContributorsMapAdapterTests.java b/...c/test/java/org/springframework/boot/actuate/health/NamedContributorsMapAdapterTests.java
@@ -1,5 +1,5 @@
 /*
- * Copyright 2012-2021 the original author or authors.
+ * Copyright 2012-2022 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.
@@ -20,6 +20,7 @@
 import java.util.Iterator;
 import java.util.LinkedHashMap;
 import java.util.Map;
+import java.util.concurrent.atomic.AtomicInteger;
 import java.util.function.Function;
 
 import org.junit.jupiter.api.Test;
@@ -31,6 +32,7 @@
  * Tests for {@link NamedContributorsMapAdapter}.
  *
  * @author Phillip Webb
+ * @author Guirong Hu
  */
 class NamedContributorsMapAdapterTests {
 
@@ -92,6 +94,21 @@ void getContributorReturnsAdaptedEntry() {
 		assertThat(adapter.getContributor("two")).isEqualTo("owt");
 	}
 
+	@Test
+	void getContributorCallsAdaptersOnlyOnce() {
+		Map<String, String> map = new LinkedHashMap<>();
+		map.put("one", "one");
+		map.put("two", "two");
+		int callCount = map.size();
+		AtomicInteger counter = new AtomicInteger(0);
+		TestNamedContributorsMapAdapter<String> adapter = new TestNamedContributorsMapAdapter<>(map,
+				(name) -> count(name, counter));
+		assertThat(adapter.getContributor("one")).isEqualTo("eno");
+		assertThat(counter.get()).isEqualTo(callCount);
+		assertThat(adapter.getContributor("two")).isEqualTo("owt");
+		assertThat(counter.get()).isEqualTo(callCount);
+	}
+
 	@Test
 	void getContributorWhenNotInMapReturnsNull() {
 		TestNamedContributorsMapAdapter<String> adapter = createAdapter();
@@ -106,6 +123,11 @@ private TestNamedContributorsMapAdapter<String> createAdapter() {
 		return adapter;
 	}
 
+	private String count(CharSequence charSequence, AtomicInteger counter) {
+		counter.incrementAndGet();
+		return reverse(charSequence);
+	}
+
 	private String reverse(CharSequence charSequence) {
 		return new StringBuilder(charSequence).reverse().toString();
 	}