Fix operator order in metric calculation

This commit is contained in:
Ravi Khadiwala 2022-09-15 13:08:21 -05:00 committed by ravi-signal
parent 61b3cecd17
commit 2e497b5834
2 changed files with 25 additions and 4 deletions

View File

@ -23,10 +23,13 @@ import java.util.Objects;
import javax.annotation.Nonnull; import javax.annotation.Nonnull;
import javax.ws.rs.BadRequestException; import javax.ws.rs.BadRequestException;
import org.apache.commons.lang3.StringUtils; 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.configuration.dynamic.DynamicConfiguration;
import org.whispersystems.textsecuregcm.storage.DynamicConfigurationManager; import org.whispersystems.textsecuregcm.storage.DynamicConfigurationManager;
public class RecaptchaClient { public class RecaptchaClient {
private static final Logger log = LoggerFactory.getLogger(RecaptchaClient.class);
@VisibleForTesting @VisibleForTesting
static final String SEPARATOR = "."; static final String SEPARATOR = ".";
@ -97,8 +100,9 @@ public class RecaptchaClient {
* recaptcha enterprise scores are from [0.0, 1.0] in increments of .1 * recaptcha enterprise scores are from [0.0, 1.0] in increments of .1
* map to [0, 100] for easier interpretation * map to [0, 100] for easier interpretation
*/ */
private static String scoreString(final float score) { @VisibleForTesting
return Integer.toString((int) score * 100); static String scoreString(final float score) {
return Integer.toString((int) (score * 100));
} }
@ -128,14 +132,15 @@ public class RecaptchaClient {
if (assessment.getTokenProperties().getValid()) { 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()) { for (RiskAnalysis.ClassificationReason reason : assessment.getRiskAnalysis().getReasonsList()) {
Metrics.counter(ASSESSMENT_REASON_COUNTER_NAME, Metrics.counter(ASSESSMENT_REASON_COUNTER_NAME,
"action", String.valueOf(expectedAction), "action", String.valueOf(expectedAction),
"score", scoreString(assessment.getRiskAnalysis().getScore()), "score", scoreString(score),
"reason", reason.name()) "reason", reason.name())
.increment(); .increment();
} }
final float score = assessment.getRiskAnalysis().getScore();
return new AssessmentResult( return new AssessmentResult(
score >= score >=
dynamicConfigurationManager.getConfiguration().getCaptchaConfiguration().getScoreFloor().floatValue(), dynamicConfigurationManager.getConfiguration().getCaptchaConfiguration().getScoreFloor().floatValue(),

View File

@ -5,6 +5,7 @@
package org.whispersystems.textsecuregcm.recaptcha; 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.assertEquals;
import static org.junit.jupiter.api.Assertions.assertThrows; import static org.junit.jupiter.api.Assertions.assertThrows;
import static org.whispersystems.textsecuregcm.recaptcha.RecaptchaClient.SEPARATOR; 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() { static Stream<Arguments> parseInputToken() {
return Stream.of( return Stream.of(
Arguments.of( Arguments.of(