From ac7872b226110a94e2531096e348fd0c9315e34d Mon Sep 17 00:00:00 2001 From: Stephan Schnabel Date: Thu, 27 Apr 2023 09:32:01 +0200 Subject: [PATCH] Refactor test: remove micrometer mocking, micrometer is testable --- pom.xml | 2 +- .../metrics/event/MetricsEventListener.java | 10 ++- .../event/MetricsEventListenerFactory.java | 7 +-- .../stats/MetricsStatsFactoryImpl.java | 6 +- .../metrics/stats/MetricsStatsTask.java | 14 ++--- .../event/MetricsEventListenerTest.java | 63 +++++-------------- .../metrics/junit/AbstractMockitoTest.java | 29 +++++++++ 7 files changed, 61 insertions(+), 70 deletions(-) create mode 100644 src/test/java/io/kokuwa/keycloak/metrics/junit/AbstractMockitoTest.java diff --git a/pom.xml b/pom.xml index 559384d..3a77f1a 100644 --- a/pom.xml +++ b/pom.xml @@ -159,7 +159,7 @@ com.openshift openshift-restclient-java - + org.keycloak keycloak-admin-ui diff --git a/src/main/java/io/kokuwa/keycloak/metrics/event/MetricsEventListener.java b/src/main/java/io/kokuwa/keycloak/metrics/event/MetricsEventListener.java index 38bb7d0..4ba5144 100644 --- a/src/main/java/io/kokuwa/keycloak/metrics/event/MetricsEventListener.java +++ b/src/main/java/io/kokuwa/keycloak/metrics/event/MetricsEventListener.java @@ -10,7 +10,7 @@ import org.keycloak.models.KeycloakContext; import org.keycloak.models.KeycloakSession; import org.keycloak.models.RealmModel; -import io.micrometer.core.instrument.MeterRegistry; +import io.micrometer.core.instrument.Metrics; /** * Listener for {@link Event} and {@link AdminEvent}. @@ -20,19 +20,17 @@ import io.micrometer.core.instrument.MeterRegistry; public class MetricsEventListener implements EventListenerProvider, AutoCloseable { private static final Logger log = Logger.getLogger(MetricsEventListener.class); - private final MeterRegistry registry; private final boolean replaceIds; private final KeycloakSession session; - MetricsEventListener(MeterRegistry registry, boolean replaceIds, KeycloakSession session) { - this.registry = registry; + MetricsEventListener(boolean replaceIds, KeycloakSession session) { this.replaceIds = replaceIds; this.session = session; } @Override public void onEvent(Event event) { - registry.counter("keycloak_event_user", + Metrics.counter("keycloak_event_user", "realm", toBlank(replaceIds ? getRealmName(event.getRealmId()) : event.getRealmId()), "type", toBlank(event.getType()), "client", toBlank(event.getClientId()), @@ -42,7 +40,7 @@ public class MetricsEventListener implements EventListenerProvider, AutoCloseabl @Override public void onEvent(AdminEvent event, boolean includeRepresentation) { - registry.counter("keycloak_event_admin", + Metrics.counter("keycloak_event_admin", "realm", toBlank(replaceIds ? getRealmName(event.getRealmId()) : event.getRealmId()), "resource", toBlank(event.getResourceType()), "operation", toBlank(event.getOperationType()), diff --git a/src/main/java/io/kokuwa/keycloak/metrics/event/MetricsEventListenerFactory.java b/src/main/java/io/kokuwa/keycloak/metrics/event/MetricsEventListenerFactory.java index 1cd62d4..1580be7 100644 --- a/src/main/java/io/kokuwa/keycloak/metrics/event/MetricsEventListenerFactory.java +++ b/src/main/java/io/kokuwa/keycloak/metrics/event/MetricsEventListenerFactory.java @@ -7,11 +7,8 @@ import org.keycloak.events.EventListenerProviderFactory; import org.keycloak.models.KeycloakSession; import org.keycloak.models.KeycloakSessionFactory; -import io.micrometer.core.instrument.MeterRegistry; -import io.micrometer.core.instrument.Metrics; - /** - * Factory for {@link MetricsEventListener}, uses {@link MeterRegistry} from CDI. + * Factory for {@link MetricsEventListener}. * * @author Stephan Schnabel */ @@ -36,7 +33,7 @@ public class MetricsEventListenerFactory implements EventListenerProviderFactory @Override public EventListenerProvider create(KeycloakSession session) { - return new MetricsEventListener(Metrics.globalRegistry, replaceIds, session); + return new MetricsEventListener(replaceIds, session); } @Override diff --git a/src/main/java/io/kokuwa/keycloak/metrics/stats/MetricsStatsFactoryImpl.java b/src/main/java/io/kokuwa/keycloak/metrics/stats/MetricsStatsFactoryImpl.java index 46cb9f5..03d8f78 100644 --- a/src/main/java/io/kokuwa/keycloak/metrics/stats/MetricsStatsFactoryImpl.java +++ b/src/main/java/io/kokuwa/keycloak/metrics/stats/MetricsStatsFactoryImpl.java @@ -10,8 +10,6 @@ import org.keycloak.models.KeycloakSessionFactory; import org.keycloak.models.utils.KeycloakModelUtils; import org.keycloak.timer.TimerProvider; -import io.micrometer.core.instrument.Metrics; - /** * Implementation of {@link MetricsStatsFactory}. * @@ -53,10 +51,10 @@ public class MetricsStatsFactoryImpl implements MetricsStatsFactory { intervalDuration, infoThreshold, warnThreshold); var interval = intervalDuration.toMillis(); - var task = new MetricsStatsTask(Metrics.globalRegistry, intervalDuration, infoThreshold, warnThreshold); + var task = new MetricsStatsTask(intervalDuration, infoThreshold, warnThreshold); KeycloakModelUtils.runJobInTransaction(factory, session -> session .getProvider(TimerProvider.class) - .schedule(() -> KeycloakModelUtils.runJobInTransaction(factory, task), interval, "metrics")); + .scheduleTask(task, interval, "metrics")); } @Override diff --git a/src/main/java/io/kokuwa/keycloak/metrics/stats/MetricsStatsTask.java b/src/main/java/io/kokuwa/keycloak/metrics/stats/MetricsStatsTask.java index a0767a8..22f7c3c 100644 --- a/src/main/java/io/kokuwa/keycloak/metrics/stats/MetricsStatsTask.java +++ b/src/main/java/io/kokuwa/keycloak/metrics/stats/MetricsStatsTask.java @@ -9,10 +9,10 @@ import java.util.concurrent.atomic.AtomicLong; import org.jboss.logging.Logger; import org.keycloak.models.KeycloakSession; -import org.keycloak.models.KeycloakSessionTask; import org.keycloak.provider.Provider; +import org.keycloak.timer.ScheduledTask; -import io.micrometer.core.instrument.MeterRegistry; +import io.micrometer.core.instrument.Metrics; import io.micrometer.core.instrument.Tag; /** @@ -20,17 +20,15 @@ import io.micrometer.core.instrument.Tag; * * @author Stephan Schnabel */ -public class MetricsStatsTask implements Provider, KeycloakSessionTask { +public class MetricsStatsTask implements Provider, ScheduledTask { private static final Logger log = Logger.getLogger(MetricsStatsTask.class); - private final Map values = new HashMap<>(); - private final MeterRegistry registry; + private static final Map values = new HashMap<>(); private final Duration interval; private final Duration infoThreshold; private final Duration warnThreshold; - MetricsStatsTask(MeterRegistry registry, Duration interval, Duration infoThreshold, Duration warnThreshold) { - this.registry = registry; + MetricsStatsTask(Duration interval, Duration infoThreshold, Duration warnThreshold) { this.interval = interval; this.infoThreshold = infoThreshold; this.warnThreshold = warnThreshold; @@ -84,6 +82,6 @@ public class MetricsStatsTask implements Provider, KeycloakSessionTask { } private void gauge(String name, Set tags, long value) { - values.computeIfAbsent(name + tags, s -> registry.gauge(name, tags, new AtomicLong())).set(value); + values.computeIfAbsent(name + tags, s -> Metrics.gauge(name, tags, new AtomicLong())).set(value); } } diff --git a/src/test/java/io/kokuwa/keycloak/metrics/event/MetricsEventListenerTest.java b/src/test/java/io/kokuwa/keycloak/metrics/event/MetricsEventListenerTest.java index f1bd995..16684c5 100644 --- a/src/test/java/io/kokuwa/keycloak/metrics/event/MetricsEventListenerTest.java +++ b/src/test/java/io/kokuwa/keycloak/metrics/event/MetricsEventListenerTest.java @@ -1,22 +1,13 @@ package io.kokuwa.keycloak.metrics.event; import static org.junit.jupiter.api.Assertions.assertEquals; -import static org.mockito.ArgumentMatchers.any; -import static org.mockito.ArgumentMatchers.anyString; -import static org.mockito.Mockito.verify; import static org.mockito.Mockito.when; -import java.util.Map; -import java.util.TreeMap; import java.util.UUID; -import java.util.stream.Collectors; -import java.util.stream.IntStream; -import org.junit.jupiter.api.BeforeEach; import org.junit.jupiter.api.DisplayName; import org.junit.jupiter.api.Nested; import org.junit.jupiter.api.Test; -import org.junit.jupiter.api.extension.ExtendWith; import org.keycloak.events.Event; import org.keycloak.events.EventType; import org.keycloak.events.admin.AdminEvent; @@ -26,21 +17,18 @@ import org.keycloak.models.KeycloakContext; import org.keycloak.models.KeycloakSession; import org.keycloak.models.RealmModel; import org.keycloak.models.RealmProvider; -import org.mockito.ArgumentCaptor; -import org.mockito.Captor; import org.mockito.Mock; -import org.mockito.junit.jupiter.MockitoExtension; -import io.micrometer.core.instrument.Counter; -import io.micrometer.core.instrument.MeterRegistry; +import io.kokuwa.keycloak.metrics.junit.AbstractMockitoTest; +import io.micrometer.core.instrument.Metrics; /** * Test for {@link MetricsEventListener} with Mockito. * * @author Stephan Schnabel */ -@ExtendWith(MockitoExtension.class) -public class MetricsEventListenerTest { +@DisplayName("events: listener") +public class MetricsEventListenerTest extends AbstractMockitoTest { @Mock KeycloakSession session; @@ -50,19 +38,6 @@ public class MetricsEventListenerTest { RealmProvider realmProvider; @Mock KeycloakContext context; - @Mock - MeterRegistry registry; - @Mock - Counter counter; - @Captor - ArgumentCaptor metricCaptor; - @Captor - ArgumentCaptor tagsCaptor; - - @BeforeEach - void setup() { - when(registry.counter(metricCaptor.capture(), tagsCaptor.capture())).thenReturn(counter); - } @DisplayName("onEvent(true)") @Nested @@ -213,11 +188,11 @@ public class MetricsEventListenerTest { } private void assertEvent(String realm, String client, String type, String error) { - assertCounter("keycloak_event_user", Map.of( + assertCounter("keycloak_event_user", "realm", realm, "client", client, "type", type, - "error", error)); + "error", error); } } @@ -370,29 +345,25 @@ public class MetricsEventListenerTest { } private void assertAdminEvent(String realm, String resource, String operation, String error) { - assertCounter("keycloak_event_admin", Map.of( + assertCounter("keycloak_event_admin", "realm", realm, "resource", resource, "operation", operation, - "error", error)); + "error", error); } } private MetricsEventListener listener(boolean replace) { - return new MetricsEventListener(registry, replace, session); + return new MetricsEventListener(replace, session); } - private void assertCounter(String metric, Map tags) { - verify(registry).counter(anyString(), any(String[].class)); - verify(counter).increment(); - assertEquals(metric, metricCaptor.getValue(), "metric"); - var expectedTags = new TreeMap<>(tags); - var actualTags = IntStream - .range(0, tagsCaptor.getValue().length / 2).mapToObj(i -> i * 2) - .collect(Collectors.toMap( - i -> tagsCaptor.getValue()[i], - i -> tagsCaptor.getValue()[i + 1], - (i, j) -> i, TreeMap::new)); - assertEquals(expectedTags, actualTags, "tags"); + private static void assertCounter(String metric, String... tags) { + var counter = Metrics.globalRegistry.counter(metric, tags); + assertEquals(1D, counter.count(), "micrometer.counter.count"); + assertEquals(0, Metrics.globalRegistry + .getMeters().stream() + .filter(meter -> meter != counter) + .count(), + "other meter found"); } } diff --git a/src/test/java/io/kokuwa/keycloak/metrics/junit/AbstractMockitoTest.java b/src/test/java/io/kokuwa/keycloak/metrics/junit/AbstractMockitoTest.java new file mode 100644 index 0000000..e42de7c --- /dev/null +++ b/src/test/java/io/kokuwa/keycloak/metrics/junit/AbstractMockitoTest.java @@ -0,0 +1,29 @@ +package io.kokuwa.keycloak.metrics.junit; + +import org.junit.jupiter.api.BeforeEach; +import org.junit.jupiter.api.ClassOrderer; +import org.junit.jupiter.api.MethodOrderer; +import org.junit.jupiter.api.TestClassOrder; +import org.junit.jupiter.api.TestMethodOrder; +import org.junit.jupiter.api.extension.ExtendWith; +import org.mockito.junit.jupiter.MockitoExtension; + +import io.micrometer.core.instrument.Metrics; +import io.micrometer.core.instrument.simple.SimpleMeterRegistry; + +/** + * Mockito base class with configured logging. + * + * @author Stephan Schnabel + */ +@ExtendWith(MockitoExtension.class) +@TestClassOrder(ClassOrderer.DisplayName.class) +@TestMethodOrder(MethodOrderer.DisplayName.class) +public abstract class AbstractMockitoTest { + + @BeforeEach + void reset() { + Metrics.globalRegistry.clear(); + Metrics.addRegistry(new SimpleMeterRegistry()); + } +}