From 2e497b583495322e9e454ccc9428921f5368d6b3 Mon Sep 17 00:00:00 2001
From: Ravi Khadiwala <ravi@signal.org>
Date: Thu, 15 Sep 2022 13:08:21 -0500
Subject: [PATCH] Fix operator order in metric calculation

---
 .../textsecuregcm/recaptcha/RecaptchaClient.java | 13 +++++++++----
 .../recaptcha/RecaptchaClientTest.java           | 16 ++++++++++++++++
 2 files changed, 25 insertions(+), 4 deletions(-)

diff --git a/service/src/main/java/org/whispersystems/textsecuregcm/recaptcha/RecaptchaClient.java b/service/src/main/java/org/whispersystems/textsecuregcm/recaptcha/RecaptchaClient.java
index dc53b70c2..c62fe5e64 100644
--- a/service/src/main/java/org/whispersystems/textsecuregcm/recaptcha/RecaptchaClient.java
+++ b/service/src/main/java/org/whispersystems/textsecuregcm/recaptcha/RecaptchaClient.java
@@ -23,10 +23,13 @@ import java.util.Objects;
 import javax.annotation.Nonnull;
 import javax.ws.rs.BadRequestException;
 import org.apache.commons.lang3.StringUtils;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
 import org.whispersystems.textsecuregcm.configuration.dynamic.DynamicConfiguration;
 import org.whispersystems.textsecuregcm.storage.DynamicConfigurationManager;
 
 public class RecaptchaClient {
+  private static final Logger log = LoggerFactory.getLogger(RecaptchaClient.class);
 
   @VisibleForTesting
   static final String SEPARATOR = ".";
@@ -97,8 +100,9 @@ public class RecaptchaClient {
    * recaptcha enterprise scores are from [0.0, 1.0] in increments of .1
    * map to [0, 100] for easier interpretation
    */
-  private static String scoreString(final float score) {
-    return Integer.toString((int) score * 100);
+  @VisibleForTesting
+  static String scoreString(final float score) {
+    return Integer.toString((int) (score * 100));
   }
 
 
@@ -128,14 +132,15 @@ public class RecaptchaClient {
 
 
     if (assessment.getTokenProperties().getValid()) {
+      final float score = assessment.getRiskAnalysis().getScore();
+      log.debug("assessment for {} was valid, score: {}", expectedAction, score);
       for (RiskAnalysis.ClassificationReason reason : assessment.getRiskAnalysis().getReasonsList()) {
         Metrics.counter(ASSESSMENT_REASON_COUNTER_NAME,
             "action", String.valueOf(expectedAction),
-            "score", scoreString(assessment.getRiskAnalysis().getScore()),
+            "score", scoreString(score),
             "reason", reason.name())
             .increment();
       }
-      final float score = assessment.getRiskAnalysis().getScore();
       return new AssessmentResult(
           score >=
               dynamicConfigurationManager.getConfiguration().getCaptchaConfiguration().getScoreFloor().floatValue(),
diff --git a/service/src/test/java/org/whispersystems/textsecuregcm/recaptcha/RecaptchaClientTest.java b/service/src/test/java/org/whispersystems/textsecuregcm/recaptcha/RecaptchaClientTest.java
index 8b188a304..c50702c6b 100644
--- a/service/src/test/java/org/whispersystems/textsecuregcm/recaptcha/RecaptchaClientTest.java
+++ b/service/src/test/java/org/whispersystems/textsecuregcm/recaptcha/RecaptchaClientTest.java
@@ -5,6 +5,7 @@
 
 package org.whispersystems.textsecuregcm.recaptcha;
 
+import static org.assertj.core.api.Assertions.assertThat;
 import static org.junit.jupiter.api.Assertions.assertEquals;
 import static org.junit.jupiter.api.Assertions.assertThrows;
 import static org.whispersystems.textsecuregcm.recaptcha.RecaptchaClient.SEPARATOR;
@@ -43,6 +44,21 @@ class RecaptchaClientTest {
     });
   }
 
+  @ParameterizedTest
+  @MethodSource
+  void scoreString(float score, String expected) {
+    assertThat(RecaptchaClient.scoreString(score)).isEqualTo(expected);
+  }
+
+  static Stream<Arguments> scoreString() {
+    return Stream.of(
+        Arguments.of(0.3f, "30"),
+        Arguments.of(0.0f, "0"),
+        Arguments.of(0.333f, "33"),
+        Arguments.of(Float.NaN, "0")
+    );
+  }
+
   static Stream<Arguments> parseInputToken() {
     return Stream.of(
         Arguments.of(