fix: address PR review findings — double-counting, finally safety, JavaDoc, unit tests

- Remove duplicate recordError(INVALID_TOKEN) call from executeValidationRequest;
  outer catch (TurnstileValidationException) is the single correct recording site
- Wrap metrics.recordResponseTime() in try/catch inside finally block so a metrics
  failure cannot suppress the original validation exception
- Add log.debug to TurnstileValidationException catch for symmetry with other handlers
- Improve broad catch to log exception class name for better diagnostics
- Add complete JavaDoc to TurnstileMetrics interface (recordValidation, recordSuccess,
  recordResponseTime) including call-sequence contract and timing semantics
- Add class and constructor JavaDoc to MicrometerTurnstileMetrics; add
  Objects.requireNonNull guard on registry parameter
- Fix TurnstileValidationService class-level doc (no longer says "when metrics enabled")
- Fix TurnstileHealthConfiguration comment to mention @ConditionalOnEnabledHealthIndicator
- Fix noOpTurnstileMetrics comment to say "no TurnstileMetrics bean registered"
  rather than "Micrometer not on classpath"
- Add MicrometerTurnstileMetricsTest (9 tests) covering all counter dispatch paths,
  timer recording, null-registry rejection, and aggregate/sub-counter consistency

Author: devondragon

src/main/java/com/digitalsanctuary/cf/turnstile/TurnstileConfiguration.java

@@ -47,7 +47,9 @@ static class TurnstileMetricsConfiguration {
 
     /**
      * Health indicator configuration for Turnstile.
-     * Only imported if Spring Actuator's HealthIndicator is available on the classpath.
+     * Only imported if Spring Actuator's {@code HealthIndicator} class is on the classpath
+     * and the turnstile health indicator has not been disabled via
+     * {@code management.health.turnstile.enabled=false}.
      */
     @Configuration
     @ConditionalOnEnabledHealthIndicator("turnstile")

src/main/java/com/digitalsanctuary/cf/turnstile/config/TurnstileServiceConfig.java

@@ -27,8 +27,10 @@ public class TurnstileServiceConfig {
     private final TurnstileConfigProperties properties;
 
     /**
-     * Provides a no-op TurnstileMetrics bean when no other implementation is registered.
-     * This is the fallback when Micrometer is not on the classpath.
+     * Provides a no-op {@link TurnstileMetrics} bean when no other implementation is registered.
+     * This fallback is active when Micrometer is absent from the classpath, when metrics are
+     * disabled via {@code ds.cf.turnstile.metrics.enabled=false}, or when no custom
+     * {@code TurnstileMetrics} bean has been supplied by the consuming application.
      *
      * @return a no-op TurnstileMetrics instance
      */

src/main/java/com/digitalsanctuary/cf/turnstile/metrics/MicrometerTurnstileMetrics.java

@@ -4,12 +4,24 @@
 import io.micrometer.core.instrument.Counter;
 import io.micrometer.core.instrument.MeterRegistry;
 import io.micrometer.core.instrument.Timer;
+import java.util.Objects;
 import java.util.concurrent.TimeUnit;
 import lombok.extern.slf4j.Slf4j;
 
 /**
- * Micrometer-backed implementation of TurnstileMetrics.
- * This class is only instantiated when Micrometer is present on the classpath.
+ * Micrometer-backed implementation of {@link TurnstileMetrics}.
+ * <p>
+ * This class is only instantiated when Micrometer is present on the classpath, via
+ * {@code TurnstileMetricsConfig} which is guarded by {@code @ConditionalOnClass(MeterRegistry.class)}.
+ * </p>
+ * <p>
+ * All eight meters ({@code errorCounter} is the aggregate; the four type-specific counters are
+ * sub-categories whose sum equals the aggregate) are eagerly registered at construction time so
+ * they appear in monitoring dashboards before the first validation event occurs. A single instance
+ * should be registered per application context to avoid duplicate meter registration errors.
+ * </p>
+ *
+ * @see NoOpTurnstileMetrics
  */
 @Slf4j
 public class MicrometerTurnstileMetrics implements TurnstileMetrics {
@@ -23,7 +35,24 @@ public class MicrometerTurnstileMetrics implements TurnstileMetrics {
     private final Counter inputErrorCounter;
     private final Timer responseTimer;
 
+    /**
+     * Creates a {@code MicrometerTurnstileMetrics} instance and eagerly registers all Turnstile
+     * meters with the provided registry. The registered meters are:
+     * <ul>
+     *   <li>{@code turnstile.validation.requests} — total attempts</li>
+     *   <li>{@code turnstile.validation.success} — successful validations</li>
+     *   <li>{@code turnstile.validation.errors} — all errors (aggregate)</li>
+     *   <li>{@code turnstile.validation.errors.network} — network errors</li>
+     *   <li>{@code turnstile.validation.errors.config} — configuration errors</li>
+     *   <li>{@code turnstile.validation.errors.token} — invalid token errors</li>
+     *   <li>{@code turnstile.validation.errors.input} — input validation errors</li>
+     *   <li>{@code turnstile.validation.response.time} — response time timer</li>
+     * </ul>
+     *
+     * @param registry the Micrometer {@link MeterRegistry} to register meters with; must not be null
+     */
     public MicrometerTurnstileMetrics(MeterRegistry registry) {
+        Objects.requireNonNull(registry, "registry must not be null");
         log.info("Initializing Turnstile metrics with MeterRegistry");
         validationCounter = Counter.builder("turnstile.validation.requests")
                 .description("Total number of Turnstile validation requests").register(registry);

src/main/java/com/digitalsanctuary/cf/turnstile/metrics/TurnstileMetrics.java

@@ -4,10 +4,30 @@
 
 /**
  * Abstraction for recording Turnstile validation metrics.
- * Implementations may be no-op (when Micrometer is absent) or Micrometer-backed.
+ * <p>
+ * Implementations may be no-op (when Micrometer is absent from the classpath or metrics are
+ * disabled) or Micrometer-backed. Consumers may also supply a custom implementation as a Spring
+ * bean to integrate with their own metrics infrastructure.
+ * </p>
+ * <p>
+ * Expected call sequence per validation attempt:
+ * {@link #recordValidation()} is always called first, followed by exactly one of
+ * {@link #recordSuccess()} or {@link #recordError(ValidationResultType)}, and then
+ * {@link #recordResponseTime(long)} for any attempt that reached the network (input and
+ * configuration errors do not record a response time).
+ * </p>
  */
 public interface TurnstileMetrics {
+
+    /**
+     * Records a validation attempt. Called once per invocation of
+     * {@code validateTurnstileResponseDetailed}, regardless of outcome.
+     */
     void recordValidation();
+
+    /**
+     * Records a successful validation. Called only when Cloudflare returns a success response.
+     */
     void recordSuccess();
 
     /**
@@ -22,5 +42,12 @@ public interface TurnstileMetrics {
      */
     void recordError(ValidationResultType type);
 
+    /**
+     * Records the elapsed wall-clock time for a validation attempt that reached the network.
+     * Not called for input or configuration errors that short-circuit before the HTTP request.
+     *
+     * @param milliseconds elapsed time in milliseconds from the start of the validation call
+     *                     to its completion (success or failure)
+     */
     void recordResponseTime(long milliseconds);
 }

src/main/java/com/digitalsanctuary/cf/turnstile/service/TurnstileValidationService.java

@@ -29,8 +29,8 @@
  * Service for validating responses from Cloudflare's Turnstile API.
  * <p>
  * This service provides methods to validate Turnstile tokens with the Cloudflare API, handling various error scenarios with appropriate exceptions
- * and detailed validation results. It also collects metrics on validation attempts, success/failure rates, and response times when metrics are
- * enabled.
+ * and detailed validation results. It maintains internal counters for validation attempts, success/failure rates, and response times, and delegates
+ * to a {@link com.digitalsanctuary.cf.turnstile.metrics.TurnstileMetrics} implementation (Micrometer-backed or no-op) for external metric recording.
  * </p>
  */
 @Slf4j
@@ -207,18 +207,23 @@ public ValidationResult validateTurnstileResponseDetailed(String token, String r
             recordError(ValidationResultType.NETWORK_ERROR);
             throw new TurnstileNetworkException("Network error: " + e.getMessage(), e);
         } catch (TurnstileValidationException e) {
+            log.debug("Turnstile token rejected by Cloudflare: {}", e.getMessage());
             recordError(ValidationResultType.INVALID_TOKEN);
             throw e;
         } catch (Exception e) {
-            log.error("Unexpected error during Turnstile validation: {}", e.getMessage(), e);
+            log.error("Unexpected {} during Turnstile validation: {}", e.getClass().getSimpleName(), e.getMessage(), e);
             recordError(ValidationResultType.NETWORK_ERROR);
             throw new TurnstileNetworkException("Unexpected error: " + e.getMessage(), e);
         } finally {
             long elapsed = System.currentTimeMillis() - startTime;
             lastResponseTime.set(elapsed);
             totalResponseTime.addAndGet(elapsed);
             responseCount.incrementAndGet();
-            metrics.recordResponseTime(elapsed);
+            try {
+                metrics.recordResponseTime(elapsed);
+            } catch (Exception metricsEx) {
+                log.warn("Failed to record response time metric; validation result is unaffected: {}", metricsEx.getMessage(), metricsEx);
+            }
         }
     }
 
@@ -241,7 +246,6 @@ private ValidationResult executeValidationRequest(Map<String, String> requestBod
             return ValidationResult.success();
         } else {
             log.warn("Turnstile validation failed with error codes: {}", response.getErrorCodes());
-            recordError(ValidationResultType.INVALID_TOKEN);
             throw new TurnstileValidationException("Token validation failed", response.getErrorCodes());
         }
     }

src/test/java/com/digitalsanctuary/cf/test/turnstile/MicrometerTurnstileMetricsTest.java

@@ -0,0 +1,98 @@
+package com.digitalsanctuary.cf.test.turnstile;
+
+import static org.junit.jupiter.api.Assertions.assertEquals;
+import static org.junit.jupiter.api.Assertions.assertThrows;
+import io.micrometer.core.instrument.Counter;
+import io.micrometer.core.instrument.simple.SimpleMeterRegistry;
+import org.junit.jupiter.api.BeforeEach;
+import org.junit.jupiter.api.Test;
+import com.digitalsanctuary.cf.turnstile.dto.ValidationResult.ValidationResultType;
+import com.digitalsanctuary.cf.turnstile.metrics.MicrometerTurnstileMetrics;
+
+/**
+ * Unit tests for MicrometerTurnstileMetrics using an in-memory SimpleMeterRegistry.
+ * Verifies that each method routes to the correct named Micrometer meter.
+ */
+class MicrometerTurnstileMetricsTest {
+
+    private SimpleMeterRegistry registry;
+    private MicrometerTurnstileMetrics metrics;
+
+    @BeforeEach
+    void setUp() {
+        registry = new SimpleMeterRegistry();
+        metrics = new MicrometerTurnstileMetrics(registry);
+    }
+
+    @Test
+    void recordValidation_incrementsRequestsCounter() {
+        metrics.recordValidation();
+        metrics.recordValidation();
+        assertEquals(2.0, registry.counter("turnstile.validation.requests").count());
+    }
+
+    @Test
+    void recordSuccess_incrementsSuccessCounter() {
+        metrics.recordSuccess();
+        assertEquals(1.0, registry.counter("turnstile.validation.success").count());
+    }
+
+    @Test
+    void recordError_networkError_incrementsAggregateAndSubcounter() {
+        metrics.recordError(ValidationResultType.NETWORK_ERROR);
+        assertEquals(1.0, registry.counter("turnstile.validation.errors").count());
+        assertEquals(1.0, registry.counter("turnstile.validation.errors.network").count());
+        assertEquals(0.0, registry.counter("turnstile.validation.errors.config").count());
+        assertEquals(0.0, registry.counter("turnstile.validation.errors.token").count());
+        assertEquals(0.0, registry.counter("turnstile.validation.errors.input").count());
+    }
+
+    @Test
+    void recordError_configurationError_incrementsAggregateAndSubcounter() {
+        metrics.recordError(ValidationResultType.CONFIGURATION_ERROR);
+        assertEquals(1.0, registry.counter("turnstile.validation.errors").count());
+        assertEquals(0.0, registry.counter("turnstile.validation.errors.network").count());
+        assertEquals(1.0, registry.counter("turnstile.validation.errors.config").count());
+    }
+
+    @Test
+    void recordError_invalidToken_incrementsAggregateAndSubcounter() {
+        metrics.recordError(ValidationResultType.INVALID_TOKEN);
+        assertEquals(1.0, registry.counter("turnstile.validation.errors").count());
+        assertEquals(1.0, registry.counter("turnstile.validation.errors.token").count());
+    }
+
+    @Test
+    void recordError_inputError_incrementsAggregateAndSubcounter() {
+        metrics.recordError(ValidationResultType.INPUT_ERROR);
+        assertEquals(1.0, registry.counter("turnstile.validation.errors").count());
+        assertEquals(1.0, registry.counter("turnstile.validation.errors.input").count());
+    }
+
+    @Test
+    void recordResponseTime_recordsToTimer() {
+        metrics.recordResponseTime(150L);
+        metrics.recordResponseTime(250L);
+        assertEquals(2L, registry.timer("turnstile.validation.response.time").count());
+    }
+
+    @Test
+    void constructor_rejectsNullRegistry() {
+        assertThrows(NullPointerException.class, () -> new MicrometerTurnstileMetrics(null));
+    }
+
+    @Test
+    void multipleErrors_aggregateCountEqualsSum() {
+        metrics.recordError(ValidationResultType.NETWORK_ERROR);
+        metrics.recordError(ValidationResultType.INVALID_TOKEN);
+        metrics.recordError(ValidationResultType.INPUT_ERROR);
+
+        Counter aggregate = registry.counter("turnstile.validation.errors");
+        double subTotal = registry.counter("turnstile.validation.errors.network").count()
+                + registry.counter("turnstile.validation.errors.config").count()
+                + registry.counter("turnstile.validation.errors.token").count()
+                + registry.counter("turnstile.validation.errors.input").count();
+
+        assertEquals(aggregate.count(), subTotal, "Aggregate error count must equal sum of sub-counters");
+    }
+}