GH-1531: fixed NPE in MicrometerHolder during app shutdown (#1532)

* fixed NPE issue in MicrometerHolder during app shutdown (#1531)

* fixed build to be compatible with Java 8

* fixed checkstyle issues

Co-authored-by: vsarzhynskyi <[email protected]>

Author: vasiliy-sarzhynskyi

spring-kafka/src/main/java/org/springframework/kafka/support/micrometer/MicrometerHolder.java

@@ -36,6 +36,8 @@
  */
 public final class MicrometerHolder {
 
+	private static final String NONE_EXCEPTION_METERS_KEY = "none";
+
 	private final Map<String, Timer> meters = new ConcurrentHashMap<>();
 
 	private final MeterRegistry registry;
@@ -69,7 +71,7 @@ public MicrometerHolder(@Nullable ApplicationContext context, String name,
 		this.tags = tags;
 		if (registries.size() == 1) {
 			this.registry = registries.values().iterator().next();
-			buildTimer("none");
+			buildTimer(NONE_EXCEPTION_METERS_KEY);
 		}
 		else {
 			throw new IllegalStateException("No micrometer registry present (or more than one)");
@@ -86,11 +88,14 @@ public Object start() {
 
 	/**
 	 * Record success.
-	 * @param sample ths sample.
+	 * @param sample the sample.
 	 * @see #start()
 	 */
 	public void success(Object sample) {
-		((Sample) sample).stop(this.meters.get("none"));
+		Timer timer = this.meters.get(NONE_EXCEPTION_METERS_KEY);
+		if (timer != null) {
+			((Sample) sample).stop(timer);
+		}
 	}
 
 	/**
@@ -111,10 +116,10 @@ private Timer buildTimer(String exception) {
 		Builder builder = Timer.builder(this.timerName)
 			.description(this.timerDesc)
 			.tag("name", this.name)
-			.tag("result", exception.equals("none") ? "success" : "failure")
+			.tag("result", exception.equals(NONE_EXCEPTION_METERS_KEY) ? "success" : "failure")
 			.tag("exception", exception);
 		if (this.tags != null && !this.tags.isEmpty()) {
-			this.tags.forEach((key, value) -> builder.tag(key, value));
+			this.tags.forEach(builder::tag);
 		}
 		Timer registeredTimer = builder.register(this.registry);
 		this.meters.put(exception, registeredTimer);

spring-kafka/src/test/java/org/springframework/kafka/support/micrometer/MicrometerHolderTests.java

@@ -0,0 +1,71 @@
+/*
+ * Copyright 2020 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.
+ * You may obtain a copy of the License at
+ *
+ *      https://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.springframework.kafka.support.micrometer;
+
+import static org.assertj.core.api.Assertions.assertThat;
+import static org.mockito.ArgumentMatchers.any;
+import static org.mockito.ArgumentMatchers.anyBoolean;
+import static org.mockito.BDDMockito.given;
+import static org.mockito.Mockito.mock;
+import static org.mockito.Mockito.times;
+import static org.mockito.Mockito.verify;
+import static org.mockito.Mockito.verifyNoMoreInteractions;
+
+import java.util.Collections;
+import java.util.Map;
+
+import org.junit.jupiter.api.Test;
+
+import org.springframework.context.ApplicationContext;
+import org.springframework.test.util.ReflectionTestUtils;
+
+import io.micrometer.core.instrument.MeterRegistry;
+import io.micrometer.core.instrument.Timer;
+import io.micrometer.core.instrument.simple.SimpleMeterRegistry;
+
+/**
+ * @author Vasyl Sarzhynskyi
+ */
+public class MicrometerHolderTests {
+
+	@SuppressWarnings({ "unchecked" })
+	@Test
+	public void testMicrometerHolderRecordSuccessWorksGracefullyAfterDestroy() {
+		MeterRegistry meterRegistry = new SimpleMeterRegistry();
+		ApplicationContext ctx = mock(ApplicationContext.class);
+		Timer.Sample sample = mock(Timer.Sample.class);
+		given(ctx.getBeansOfType(any(), anyBoolean(), anyBoolean())).willReturn(Collections.singletonMap("registry", meterRegistry));
+
+		MicrometerHolder micrometerHolder = new MicrometerHolder(ctx, "holderName",
+				"timerName", "timerDesc", Collections.emptyMap());
+		Map<String, Timer> meters = (Map<String, Timer>) ReflectionTestUtils.getField(micrometerHolder, "meters");
+		assertThat(meters).hasSize(1);
+
+		micrometerHolder.success(sample);
+		micrometerHolder.destroy();
+
+		meters = (Map<String, Timer>) ReflectionTestUtils.getField(micrometerHolder, "meters");
+		assertThat(meters).hasSize(0);
+
+		micrometerHolder.success(sample);
+
+		verify(ctx, times(1)).getBeansOfType(any(), anyBoolean(), anyBoolean());
+		verify(sample, times(1)).stop(any());
+		verifyNoMoreInteractions(ctx, sample);
+	}
+
+}