diff --git a/service/src/main/java/org/whispersystems/textsecuregcm/limits/PushChallengeManager.java b/service/src/main/java/org/whispersystems/textsecuregcm/limits/PushChallengeManager.java index 1f2b592c1..460fb4672 100644 --- a/service/src/main/java/org/whispersystems/textsecuregcm/limits/PushChallengeManager.java +++ b/service/src/main/java/org/whispersystems/textsecuregcm/limits/PushChallengeManager.java @@ -74,7 +74,7 @@ public class PushChallengeManager { Metrics.counter(CHALLENGE_REQUESTED_COUNTER_NAME, PLATFORM_TAG_NAME, platform, - SOURCE_COUNTRY_TAG_NAME, Util.getCountryCode(account.getNumber()), + SOURCE_COUNTRY_TAG_NAME, Util.getCountryCode(account), SENT_TAG_NAME, String.valueOf(sent)).increment(); } @@ -98,7 +98,7 @@ public class PushChallengeManager { Metrics.counter(CHALLENGE_ANSWERED_COUNTER_NAME, PLATFORM_TAG_NAME, platform, - SOURCE_COUNTRY_TAG_NAME, Util.getCountryCode(account.getNumber()), + SOURCE_COUNTRY_TAG_NAME, Util.getCountryCode(account), SUCCESS_TAG_NAME, String.valueOf(success)).increment(); return success; diff --git a/service/src/main/java/org/whispersystems/textsecuregcm/limits/RateLimitChallengeManager.java b/service/src/main/java/org/whispersystems/textsecuregcm/limits/RateLimitChallengeManager.java index 5647eb032..07e1894f8 100644 --- a/service/src/main/java/org/whispersystems/textsecuregcm/limits/RateLimitChallengeManager.java +++ b/service/src/main/java/org/whispersystems/textsecuregcm/limits/RateLimitChallengeManager.java @@ -81,7 +81,7 @@ public class RateLimitChallengeManager { final boolean challengeSuccess = assessmentResult.isValid(scoreThreshold); final Tags tags = Tags.of( - Tag.of(SOURCE_COUNTRY_TAG_NAME, Util.getCountryCode(account.getNumber())), + Tag.of(SOURCE_COUNTRY_TAG_NAME, Util.getCountryCode(account)), Tag.of(SUCCESS_TAG_NAME, String.valueOf(challengeSuccess)), UserAgentTagUtil.getPlatformTag(userAgent) ); @@ -90,7 +90,7 @@ public class RateLimitChallengeManager { CaptchaMetrics.measureCaptchaOutcome(assessmentResult.getNormalizedIntScore(), challengeSuccess, - Util.getRegion(account.getNumber()), + Util.getRegion(account), // Note: currently all challenges are for message-sending, but if we add more use cases, we'll need to make the // accept a context from callers rather than hard-coding it here "sendMessage"); @@ -107,7 +107,7 @@ public class RateLimitChallengeManager { rateLimiters.getRateLimitResetLimiter().validate(account.getAccountIdentifier()); } catch (final RateLimitExceededException e) { Metrics.counter(RESET_RATE_LIMIT_EXCEEDED_COUNTER_NAME, - SOURCE_COUNTRY_TAG_NAME, Util.getCountryCode(account.getNumber())).increment(); + SOURCE_COUNTRY_TAG_NAME, Util.getCountryCode(account)).increment(); throw e; } diff --git a/service/src/main/java/org/whispersystems/textsecuregcm/metrics/ReportedMessageMetricsListener.java b/service/src/main/java/org/whispersystems/textsecuregcm/metrics/ReportedMessageMetricsListener.java index ddb8a0a11..cbfe322b2 100644 --- a/service/src/main/java/org/whispersystems/textsecuregcm/metrics/ReportedMessageMetricsListener.java +++ b/service/src/main/java/org/whispersystems/textsecuregcm/metrics/ReportedMessageMetricsListener.java @@ -43,9 +43,7 @@ public class ReportedMessageMetricsListener implements ReportedMessageListener { Metrics.counter(REPORTED_COUNTER_NAME, COUNTRY_CODE_TAG_NAME, sourceCountryCode).increment(); accountsManager.getByAccountIdentifier(reporterUuid).ifPresent(reporter -> { - final String destinationCountryCode = reporter.getNumberOptional() - .map(Util::getCountryCode) - .orElse("n/a"); + final String destinationCountryCode = Util.getCountryCode(reporter); logger.info(Markers.appendEntries(Map.of( "sourceCountry", sourceCountryCode, diff --git a/service/src/main/java/org/whispersystems/textsecuregcm/storage/AccountsManager.java b/service/src/main/java/org/whispersystems/textsecuregcm/storage/AccountsManager.java index 288554a64..120867c7c 100644 --- a/service/src/main/java/org/whispersystems/textsecuregcm/storage/AccountsManager.java +++ b/service/src/main/java/org/whispersystems/textsecuregcm/storage/AccountsManager.java @@ -1188,7 +1188,7 @@ public class AccountsManager extends RedisPubSubAdapter implemen }, accountLockExecutor); Metrics.counter(DELETE_COUNTER_NAME, - COUNTRY_CODE_TAG_NAME, Util.getCountryCode(account.getNumber()), + COUNTRY_CODE_TAG_NAME, Util.getCountryCode(account), DELETION_REASON_TAG_NAME, deletionReason.tagValue) .increment(); } catch (final RuntimeException e) { diff --git a/service/src/main/java/org/whispersystems/textsecuregcm/util/Util.java b/service/src/main/java/org/whispersystems/textsecuregcm/util/Util.java index ed03adace..d203ec9c4 100644 --- a/service/src/main/java/org/whispersystems/textsecuregcm/util/Util.java +++ b/service/src/main/java/org/whispersystems/textsecuregcm/util/Util.java @@ -14,25 +14,21 @@ import java.time.Clock; import java.time.Duration; import java.util.ArrayList; import java.util.Collection; -import java.util.Collections; import java.util.Comparator; -import java.util.HashSet; import java.util.List; import java.util.Locale; import java.util.Locale.LanguageRange; import java.util.Optional; -import java.util.Random; import java.util.Set; import java.util.concurrent.TimeUnit; import java.util.function.Function; -import java.util.random.RandomGenerator; import java.util.stream.Collectors; import org.apache.commons.lang3.StringUtils; +import org.apache.commons.lang3.Strings; +import org.whispersystems.textsecuregcm.storage.Account; public class Util { - private static final RandomGenerator RANDOM_GENERATOR = new Random(); - private static final PhoneNumberUtil PHONE_NUMBER_UTIL = PhoneNumberUtil.getInstance(); public static final Runnable NOOP = () -> {}; @@ -87,7 +83,11 @@ public class Util { } } - public static String getCountryCode(String number) { + public static String getCountryCode(final Account account) { + return account.getNumberOptional().map(Util::getCountryCode).orElse("n/a"); + } + + public static String getCountryCode(final String number) { try { return String.valueOf(PHONE_NUMBER_UTIL.parse(number, null).getCountryCode()); } catch (final NumberParseException e) { @@ -95,6 +95,10 @@ public class Util { } } + public static String getRegion(final Account account) { + return account.getNumberOptional().map(Util::getRegion).orElse("n/a"); + } + public static String getRegion(final String number) { try { final PhoneNumber phoneNumber = PHONE_NUMBER_UTIL.parse(number, null); @@ -132,7 +136,7 @@ public class Util { if (nationalSignificantNumber.length() == 10) { // This is a new-format number; we can get the old-format version by stripping the leading "01" from the // national number - alternateE164 = "+229" + StringUtils.removeStart(nationalSignificantNumber, "01"); + alternateE164 = "+229" + Strings.CS.removeStart(nationalSignificantNumber, "01"); } else { // This is an old-format number; we can get the new-format version by adding a "01" prefix to the national // number @@ -177,7 +181,7 @@ public class Util { if (regions.contains("BJ")) { // Benin changed phone number formats from +229 XXXXXXXX to +229 01XXXXXXXX on November 30, 2024 // We prefer the longest form for long-term stability - return e164s.stream().sorted(Comparator.comparingInt(String::length).reversed()).findFirst(); + return e164s.stream().max(Comparator.comparingInt(String::length)); } // No matching country; fall back to something that's at least stable return e164s.stream().sorted().findFirst(); @@ -272,18 +276,6 @@ public class Util { return Optional.ofNullable(Locale.lookupTag(priorityList, supportedLocales)); } - /** - * Map ints to non-negative ints. - *
- * Unlike Math.abs this method handles Integer.MIN_VALUE correctly. - * - * @param n any int value - * @return an int value guaranteed to be non-negative - */ - public static int ensureNonNegativeInt(int n) { - return n == Integer.MIN_VALUE ? 0 : Math.abs(n); - } - /** * Map longs to non-negative longs. *
@@ -295,45 +287,4 @@ public class Util { public static long ensureNonNegativeLong(long n) { return n == Long.MIN_VALUE ? 0 : Math.abs(n); } - - /** - * Chooses min(values.size(), n) random values in shuffled order. - *
- * Copies the input Array - use for small lists only or for when n/values.size() is near 1. - */ - public static List randomNOfShuffled(List values, int n) { - if(values == null || values.isEmpty()) { - return Collections.emptyList(); - } - - List result = new ArrayList<>(values); - Collections.shuffle(result); - - return result.stream().limit(n).toList(); - } - - /** - * Chooses min(values.size(), n) random values. Return value is in stable order from input values. - * Not uniform random, but good enough. - *
- * Does NOT copy the input Array. - */ - public static List randomNOfStable(List values, int n) { - if(values == null || values.isEmpty()) { - return Collections.emptyList(); - } - if(n >= values.size()) { - return values; - } - - Set indices = new HashSet<>(RANDOM_GENERATOR.ints(0, values.size()).distinct().limit(n).boxed().toList()); - List result = new ArrayList<>(n); - for(int i = 0; i < values.size() && result.size() < n; i++) { - if(indices.contains(i)) { - result.add(values.get(i)); - } - } - - return result; - } }