Drop the non-normalized account crawler
This commit is contained in:
		
							parent
							
								
									09f6d60ae9
								
							
						
					
					
						commit
						0e0cb4d422
					
				|  | @ -1,119 +0,0 @@ | ||||||
| /* |  | ||||||
|  * Copyright 2013-2021 Signal Messenger, LLC |  | ||||||
|  * SPDX-License-Identifier: AGPL-3.0-only |  | ||||||
|  */ |  | ||||||
| 
 |  | ||||||
| package org.whispersystems.textsecuregcm.storage; |  | ||||||
| 
 |  | ||||||
| import com.google.common.annotations.VisibleForTesting; |  | ||||||
| import com.google.i18n.phonenumbers.NumberParseException; |  | ||||||
| import com.google.i18n.phonenumbers.PhoneNumberUtil; |  | ||||||
| import com.google.i18n.phonenumbers.PhoneNumberUtil.PhoneNumberFormat; |  | ||||||
| import com.google.i18n.phonenumbers.Phonenumber.PhoneNumber; |  | ||||||
| import org.slf4j.Logger; |  | ||||||
| import org.slf4j.LoggerFactory; |  | ||||||
| import org.whispersystems.textsecuregcm.redis.FaultTolerantRedisCluster; |  | ||||||
| import java.util.List; |  | ||||||
| import java.util.Optional; |  | ||||||
| import java.util.UUID; |  | ||||||
| 
 |  | ||||||
| public class NonNormalizedAccountCrawlerListener extends AccountDatabaseCrawlerListener { |  | ||||||
| 
 |  | ||||||
|   private final AccountsManager accountsManager; |  | ||||||
|   private final FaultTolerantRedisCluster metricsCluster; |  | ||||||
| 
 |  | ||||||
|   private static final String NORMALIZED_NUMBER_COUNT_KEY = "NonNormalizedAccountCrawlerListener::normalized"; |  | ||||||
|   private static final String NON_NORMALIZED_NUMBER_COUNT_KEY = "NonNormalizedAccountCrawlerListener::nonNormalized"; |  | ||||||
|   private static final String CONFLICTING_NUMBER_COUNT_KEY = "NonNormalizedAccountCrawlerListener::conflicts"; |  | ||||||
| 
 |  | ||||||
|   private static final PhoneNumberUtil PHONE_NUMBER_UTIL = PhoneNumberUtil.getInstance(); |  | ||||||
| 
 |  | ||||||
|   private static final Logger log = LoggerFactory.getLogger(NonNormalizedAccountCrawlerListener.class); |  | ||||||
| 
 |  | ||||||
|   public NonNormalizedAccountCrawlerListener( |  | ||||||
|       final AccountsManager accountsManager, |  | ||||||
|       final FaultTolerantRedisCluster metricsCluster) { |  | ||||||
| 
 |  | ||||||
|     this.accountsManager = accountsManager; |  | ||||||
|     this.metricsCluster = metricsCluster; |  | ||||||
|   } |  | ||||||
| 
 |  | ||||||
|   @Override |  | ||||||
|   public void onCrawlStart() { |  | ||||||
|     metricsCluster.useCluster(connection -> |  | ||||||
|         connection.sync().del(NORMALIZED_NUMBER_COUNT_KEY, NON_NORMALIZED_NUMBER_COUNT_KEY, CONFLICTING_NUMBER_COUNT_KEY)); |  | ||||||
|   } |  | ||||||
| 
 |  | ||||||
|   @Override |  | ||||||
|   protected void onCrawlChunk(final Optional<UUID> fromUuid, final List<Account> chunkAccounts) { |  | ||||||
| 
 |  | ||||||
|     final int normalizedNumbers; |  | ||||||
|     final int nonNormalizedNumbers; |  | ||||||
|     final int conflictingNumbers; |  | ||||||
|     { |  | ||||||
|       int workingNormalizedNumbers = 0; |  | ||||||
|       int workingNonNormalizedNumbers = 0; |  | ||||||
|       int workingConflictingNumbers = 0; |  | ||||||
| 
 |  | ||||||
|       for (final Account account : chunkAccounts) { |  | ||||||
|         if (hasNumberNormalized(account)) { |  | ||||||
|           workingNormalizedNumbers++; |  | ||||||
|         } else { |  | ||||||
|           workingNonNormalizedNumbers++; |  | ||||||
| 
 |  | ||||||
|           try { |  | ||||||
|             final Optional<Account> maybeConflictingAccount = accountsManager.getByE164(getNormalizedNumber(account)); |  | ||||||
| 
 |  | ||||||
|             if (maybeConflictingAccount.isPresent()) { |  | ||||||
|               workingConflictingNumbers++; |  | ||||||
|               log.info("Normalized form of number for account {} conflicts with number for account {}", |  | ||||||
|                   account.getUuid(), maybeConflictingAccount.get().getUuid()); |  | ||||||
|             } |  | ||||||
|           } catch (final NumberParseException e) { |  | ||||||
|             log.warn("Failed to parse phone number for account {}", account.getUuid(), e); |  | ||||||
|           } |  | ||||||
|         } |  | ||||||
|       } |  | ||||||
| 
 |  | ||||||
|       normalizedNumbers = workingNormalizedNumbers; |  | ||||||
|       nonNormalizedNumbers = workingNonNormalizedNumbers; |  | ||||||
|       conflictingNumbers = workingConflictingNumbers; |  | ||||||
|     } |  | ||||||
| 
 |  | ||||||
|     metricsCluster.useCluster(connection -> { |  | ||||||
|       connection.sync().incrby(NORMALIZED_NUMBER_COUNT_KEY, normalizedNumbers); |  | ||||||
|       connection.sync().incrby(NON_NORMALIZED_NUMBER_COUNT_KEY, nonNormalizedNumbers); |  | ||||||
|       connection.sync().incrby(CONFLICTING_NUMBER_COUNT_KEY, conflictingNumbers); |  | ||||||
|     }); |  | ||||||
|   } |  | ||||||
| 
 |  | ||||||
|   @Override |  | ||||||
|   public void onCrawlEnd() { |  | ||||||
|     final int normalizedNumbers = metricsCluster.withCluster(connection -> |  | ||||||
|         Integer.parseInt(connection.sync().get(NORMALIZED_NUMBER_COUNT_KEY))); |  | ||||||
| 
 |  | ||||||
|     final int nonNormalizedNumbers = metricsCluster.withCluster(connection -> |  | ||||||
|         Integer.parseInt(connection.sync().get(NON_NORMALIZED_NUMBER_COUNT_KEY))); |  | ||||||
| 
 |  | ||||||
|     final int conflictingNumbers = metricsCluster.withCluster(connection -> |  | ||||||
|         Integer.parseInt(connection.sync().get(CONFLICTING_NUMBER_COUNT_KEY))); |  | ||||||
| 
 |  | ||||||
|     log.info("Crawl completed. Normalized numbers: {}; non-normalized numbers: {}; conflicting numbers: {}", |  | ||||||
|         normalizedNumbers, nonNormalizedNumbers, conflictingNumbers); |  | ||||||
|   } |  | ||||||
| 
 |  | ||||||
|   @VisibleForTesting |  | ||||||
|   static boolean hasNumberNormalized(final Account account) { |  | ||||||
|     try { |  | ||||||
|       return account.getNumber().equals(getNormalizedNumber(account)); |  | ||||||
|     } catch (final NumberParseException e) { |  | ||||||
|       log.warn("Failed to parse phone number for account {}", account.getUuid(), e); |  | ||||||
|       return false; |  | ||||||
|     } |  | ||||||
|   } |  | ||||||
| 
 |  | ||||||
|   private static String getNormalizedNumber(final Account account) throws NumberParseException { |  | ||||||
|     final PhoneNumber phoneNumber = PHONE_NUMBER_UTIL.parse(account.getNumber(), null); |  | ||||||
|     return PHONE_NUMBER_UTIL.format(phoneNumber, PhoneNumberFormat.E164); |  | ||||||
|   } |  | ||||||
| } |  | ||||||
|  | @ -31,7 +31,6 @@ import org.whispersystems.textsecuregcm.storage.AccountDatabaseCrawlerCache; | ||||||
| import org.whispersystems.textsecuregcm.storage.AccountDatabaseCrawlerListener; | import org.whispersystems.textsecuregcm.storage.AccountDatabaseCrawlerListener; | ||||||
| import org.whispersystems.textsecuregcm.storage.AccountsManager; | import org.whispersystems.textsecuregcm.storage.AccountsManager; | ||||||
| import org.whispersystems.textsecuregcm.storage.DynamicConfigurationManager; | import org.whispersystems.textsecuregcm.storage.DynamicConfigurationManager; | ||||||
| import org.whispersystems.textsecuregcm.storage.NonNormalizedAccountCrawlerListener; |  | ||||||
| import org.whispersystems.textsecuregcm.storage.PushFeedbackProcessor; | import org.whispersystems.textsecuregcm.storage.PushFeedbackProcessor; | ||||||
| import org.whispersystems.textsecuregcm.util.logging.UncaughtExceptionHandler; | import org.whispersystems.textsecuregcm.util.logging.UncaughtExceptionHandler; | ||||||
| 
 | 
 | ||||||
|  | @ -112,7 +111,6 @@ public class CrawlAccountsCommand extends EnvironmentCommand<WhisperServerConfig | ||||||
| 
 | 
 | ||||||
|         // TODO listeners must be ordered so that ones that directly update accounts come last, so that read-only ones are not working with stale data |         // TODO listeners must be ordered so that ones that directly update accounts come last, so that read-only ones are not working with stale data | ||||||
|         final List<AccountDatabaseCrawlerListener> accountDatabaseCrawlerListeners = List.of( |         final List<AccountDatabaseCrawlerListener> accountDatabaseCrawlerListeners = List.of( | ||||||
|             new NonNormalizedAccountCrawlerListener(accountsManager, metricsCluster), |  | ||||||
|             // PushFeedbackProcessor may update device properties |             // PushFeedbackProcessor may update device properties | ||||||
|             new PushFeedbackProcessor(accountsManager, pushFeedbackUpdateExecutor)); |             new PushFeedbackProcessor(accountsManager, pushFeedbackUpdateExecutor)); | ||||||
| 
 | 
 | ||||||
|  |  | ||||||
|  | @ -1,39 +0,0 @@ | ||||||
| /* |  | ||||||
|  * Copyright 2013-2021 Signal Messenger, LLC |  | ||||||
|  * SPDX-License-Identifier: AGPL-3.0-only |  | ||||||
|  */ |  | ||||||
| 
 |  | ||||||
| package org.whispersystems.textsecuregcm.storage; |  | ||||||
| 
 |  | ||||||
| import org.junit.jupiter.params.ParameterizedTest; |  | ||||||
| import org.junit.jupiter.params.provider.Arguments; |  | ||||||
| import org.junit.jupiter.params.provider.MethodSource; |  | ||||||
| 
 |  | ||||||
| import java.util.UUID; |  | ||||||
| import java.util.stream.Stream; |  | ||||||
| 
 |  | ||||||
| import static org.junit.jupiter.api.Assertions.*; |  | ||||||
| import static org.mockito.Mockito.mock; |  | ||||||
| import static org.mockito.Mockito.when; |  | ||||||
| 
 |  | ||||||
| class NonNormalizedAccountCrawlerListenerTest { |  | ||||||
| 
 |  | ||||||
|   @ParameterizedTest |  | ||||||
|   @MethodSource |  | ||||||
|   void hasNumberNormalized(final String number, final boolean expectNormalized) { |  | ||||||
|     final Account account = mock(Account.class); |  | ||||||
|     when(account.getUuid()).thenReturn(UUID.randomUUID()); |  | ||||||
|     when(account.getNumber()).thenReturn(number); |  | ||||||
| 
 |  | ||||||
|     assertEquals(expectNormalized, NonNormalizedAccountCrawlerListener.hasNumberNormalized(account)); |  | ||||||
|   } |  | ||||||
| 
 |  | ||||||
|   private static Stream<Arguments> hasNumberNormalized() { |  | ||||||
|     return Stream.of( |  | ||||||
|         Arguments.of("+447700900111", true), |  | ||||||
|         Arguments.of("+4407700900111", false), |  | ||||||
|         Arguments.of("Not a real phone number", false), |  | ||||||
|         Arguments.of(null, false) |  | ||||||
|     ); |  | ||||||
|   } |  | ||||||
| } |  | ||||||
		Loading…
	
		Reference in New Issue
	
	 Jon Chambers
						Jon Chambers