Remove PNI repair code

This commit is contained in:
Jon Chambers 2022-02-28 17:33:54 -05:00 committed by Jon Chambers
parent 179b4a69eb
commit fe86e15d80
2 changed files with 6 additions and 82 deletions

View File

@ -520,7 +520,7 @@ public class Accounts extends AbstractDynamoDbStore {
return Optional.ofNullable(response.item())
.map(item -> item.get(KEY_ACCOUNT_UUID))
.map(this::accountByUuid)
.map(this::fromItem);
.map(Accounts::fromItem);
});
}
@ -535,7 +535,7 @@ public class Accounts extends AbstractDynamoDbStore {
return Optional.ofNullable(response.item())
.map(item -> item.get(KEY_ACCOUNT_UUID))
.map(this::accountByUuid)
.map(this::fromItem);
.map(Accounts::fromItem);
});
}
@ -550,7 +550,7 @@ public class Accounts extends AbstractDynamoDbStore {
return Optional.ofNullable(response.item())
.map(item -> item.get(KEY_ACCOUNT_UUID))
.map(this::accountByUuid)
.map(this::fromItem);
.map(Accounts::fromItem);
});
}
@ -566,7 +566,7 @@ public class Accounts extends AbstractDynamoDbStore {
public Optional<Account> getByAccountIdentifier(UUID uuid) {
return GET_BY_UUID_TIMER.record(() ->
Optional.ofNullable(accountByUuid(AttributeValues.fromUUID(uuid)))
.map(this::fromItem));
.map(Accounts::fromItem));
}
public void delete(UUID uuid) {
@ -633,7 +633,7 @@ public class Accounts extends AbstractDynamoDbStore {
final List<Account> accounts = timer.record(() -> scan(scanRequestBuilder.build(), maxCount)
.stream()
.map(this::fromItem)
.map(Accounts::fromItem)
.collect(Collectors.toList()));
return new AccountCrawlChunk(accounts, accounts.size() > 0 ? accounts.get(accounts.size() - 1).getUuid() : null);
@ -645,9 +645,8 @@ public class Accounts extends AbstractDynamoDbStore {
.collect(Collectors.joining(", "));
}
// TODO Make this static once PNI repairs are complete and the call to #update(Account) is no longer needed
@VisibleForTesting
Account fromItem(Map<String, AttributeValue> item) {
static Account fromItem(Map<String, AttributeValue> item) {
if (!item.containsKey(ATTR_ACCOUNT_DATA) ||
!item.containsKey(ATTR_ACCOUNT_E164) ||
// TODO: eventually require ATTR_CANONICALLY_DISCOVERABLE
@ -667,27 +666,12 @@ public class Accounts extends AbstractDynamoDbStore {
accountIdentifier, account.getPhoneNumberIdentifier(), phoneNumberIdentifierFromAttribute);
}
boolean shouldResetPni = false;
if (account.getPhoneNumberIdentifier() == null && phoneNumberIdentifierFromAttribute != null) {
log.info("Account {} has a PNI in its attributes, but not JSON blob; will repair", accountIdentifier);
shouldResetPni = true;
}
account.setNumber(item.get(ATTR_ACCOUNT_E164).s(), phoneNumberIdentifierFromAttribute);
account.setUuid(accountIdentifier);
account.setUsername(AttributeValues.getString(item, ATTR_USERNAME, null));
account.setVersion(Integer.parseInt(item.get(ATTR_VERSION).n()));
account.setCanonicallyDiscoverable(Optional.ofNullable(item.get(ATTR_CANONICALLY_DISCOVERABLE)).map(av -> av.bool()).orElse(false));
if (shouldResetPni) {
try {
update(account);
} catch (final Exception e) {
log.warn("Failed to reset PNI for account {}", accountIdentifier, e);
}
}
return account;
} catch (IOException e) {

View File

@ -769,66 +769,6 @@ class AccountsTest {
assertThat(account.getUsername()).hasValueSatisfying(u -> assertThat(u).isEqualTo(username));
}
// TODO Remove when PNI repair pass is complete
@Test
void testRepairMissingPni() throws JsonProcessingException {
final String number = "+18005551234";
final UUID accountIdentifier = UUID.randomUUID();
final UUID phoneNumberIdentifier = UUID.randomUUID();
{
final Account account = generateAccount(number, accountIdentifier, phoneNumberIdentifier);
accounts.create(account);
// Artificially inject data with a null PNI in the JSON blob into the backing table
account.setNumber(number, null);
dynamoDbExtension.getDynamoDbClient().updateItem(UpdateItemRequest.builder()
.tableName(ACCOUNTS_TABLE_NAME)
.key(Map.of(Accounts.KEY_ACCOUNT_UUID, AttributeValues.fromUUID(accountIdentifier)))
.updateExpression("SET #data = :data")
.expressionAttributeNames(Map.of("#data", Accounts.ATTR_ACCOUNT_DATA))
.expressionAttributeValues(
Map.of(":data", AttributeValues.fromByteArray(SystemMapper.getMapper().writeValueAsBytes(account))))
.build());
}
{
final GetItemResponse response = dynamoDbExtension.getDynamoDbClient().getItem(GetItemRequest.builder()
.tableName(ACCOUNTS_TABLE_NAME)
.key(Map.of(Accounts.KEY_ACCOUNT_UUID, AttributeValues.fromUUID(accountIdentifier)))
.consistentRead(true)
.build());
final String accountJson = new String(
AttributeValues.getByteArray(response.item(), Accounts.ATTR_ACCOUNT_DATA, null), StandardCharsets.UTF_8);
final Account accountFromJson = SystemMapper.getMapper().readValue(accountJson, Account.class);
assertThat(accountFromJson.getPhoneNumberIdentifier()).isNull();
}
{
// Loading the account should auto-repair the missing PNI
final Account account = accounts.getByAccountIdentifier(accountIdentifier).orElseThrow();
assertThat(account.getPhoneNumberIdentifier()).isEqualTo(phoneNumberIdentifier);
assertThat(account.isStale()).isFalse();
}
{
final GetItemResponse response = dynamoDbExtension.getDynamoDbClient().getItem(GetItemRequest.builder()
.tableName(ACCOUNTS_TABLE_NAME)
.key(Map.of(Accounts.KEY_ACCOUNT_UUID, AttributeValues.fromUUID(accountIdentifier)))
.consistentRead(true)
.build());
final String accountJson = new String(
AttributeValues.getByteArray(response.item(), Accounts.ATTR_ACCOUNT_DATA, null), StandardCharsets.UTF_8);
final Account accountFromJson = SystemMapper.getMapper().readValue(accountJson, Account.class);
assertThat(accountFromJson.getPhoneNumberIdentifier()).isEqualTo(phoneNumberIdentifier);
}
}
private Device generateDevice(long id) {
Random random = new Random(System.currentTimeMillis());
SignedPreKey signedPreKey = new SignedPreKey(random.nextInt(), "testPublicKey-" + random.nextInt(),