Remove PNP flag checks in some areas.

This commit is contained in:
Greyson Parrelli 2023-09-21 16:08:39 -04:00 committed by Alex Hart
parent eb6394eb6a
commit 588a6cf74f
6 changed files with 32 additions and 73 deletions

View file

@ -793,11 +793,7 @@ open class RecipientTable(context: Context, databaseHelper: SignalDatabase) : Da
val recipientId: RecipientId
if (id < 0) {
Log.w(TAG, "[applyStorageSyncContactInsert] Failed to insert. Possibly merging.")
if (FeatureFlags.phoneNumberPrivacy()) {
recipientId = getAndPossiblyMergePnpVerified(insert.aci.orNull(), insert.pni.orNull(), insert.number.orNull())
} else {
recipientId = getAndPossiblyMerge(insert.aci.orNull(), insert.number.orNull())
}
recipientId = getAndPossiblyMergePnpVerified(insert.aci.orNull(), insert.pni.orNull(), insert.number.orNull())
db.update(TABLE_NAME, values, ID_WHERE, SqlUtil.buildArgs(recipientId))
} else {
recipientId = RecipientId.from(id)
@ -834,11 +830,7 @@ open class RecipientTable(context: Context, databaseHelper: SignalDatabase) : Da
var recipientId = getByColumn(STORAGE_SERVICE_ID, Base64.encodeBytes(update.old.id.raw)).get()
Log.w(TAG, "[applyStorageSyncContactUpdate] Found user $recipientId. Possibly merging.")
if (FeatureFlags.phoneNumberPrivacy()) {
recipientId = getAndPossiblyMergePnpVerified(update.new.aci.orElse(null), update.new.pni.orElse(null), update.new.number.orElse(null))
} else {
recipientId = getAndPossiblyMerge(update.new.aci.orElse(null), update.new.number.orElse(null))
}
recipientId = getAndPossiblyMergePnpVerified(update.new.aci.orElse(null), update.new.pni.orElse(null), update.new.number.orElse(null))
Log.w(TAG, "[applyStorageSyncContactUpdate] Merged into $recipientId")
db.update(TABLE_NAME, values, ID_WHERE, SqlUtil.buildArgs(recipientId))
@ -2158,14 +2150,10 @@ open class RecipientTable(context: Context, databaseHelper: SignalDatabase) : Da
}
fun markUnregistered(id: RecipientId) {
if (FeatureFlags.phoneNumberPrivacy()) {
val record = getRecord(id)
val record = getRecord(id)
if (record.aci != null && record.pni != null) {
markUnregisteredAndSplit(id, record)
} else {
markUnregisteredWithoutSplit(id)
}
if (record.aci != null && record.pni != null) {
markUnregisteredAndSplit(id, record)
} else {
markUnregisteredWithoutSplit(id)
}
@ -2216,8 +2204,6 @@ open class RecipientTable(context: Context, databaseHelper: SignalDatabase) : Da
* Done so we can match a split contact during storage sync.
*/
fun splitForStorageSync(storageId: ByteArray) {
check(FeatureFlags.phoneNumberPrivacy())
val record = getByStorageId(storageId)!!
check(record.aci != null && record.pni != null)
@ -3874,11 +3860,7 @@ open class RecipientTable(context: Context, databaseHelper: SignalDatabase) : Da
val username = contact.username.orElse(null)
put(ACI_COLUMN, contact.aci.orElse(null)?.toString())
if (FeatureFlags.phoneNumberPrivacy()) {
put(PNI_COLUMN, contact.pni.orElse(null)?.toString())
}
put(PNI_COLUMN, contact.pni.orElse(null)?.toString())
put(E164, contact.number.orElse(null))
put(PROFILE_GIVEN_NAME, profileName.givenName)
put(PROFILE_FAMILY_NAME, profileName.familyName)

View file

@ -76,10 +76,6 @@ class PnpInitializeDevicesJob private constructor(parameters: Parameters) : Base
throw IllegalStateException("This should only be run if you have the capability!")
}
if (!FeatureFlags.phoneNumberPrivacy()) {
throw IllegalStateException("This should only be running if PNP is enabled!")
}
if (!SignalStore.account().isRegistered || SignalStore.account().aci == null) {
Log.w(TAG, "Not registered! Skipping, as it wouldn't do anything.")
return

View file

@ -117,7 +117,7 @@ public class AccountRecordProcessor extends DefaultStorageRecordProcessor<Signal
boolean preferContactAvatars = remote.isPreferContactAvatars();
int universalExpireTimer = remote.getUniversalExpireTimer();
boolean primarySendsSms = SignalStore.account().isPrimaryDevice() ? local.isPrimarySendsSms() : remote.isPrimarySendsSms();
String e164 = SignalStore.account().isPrimaryDevice() && (!FeatureFlags.phoneNumberPrivacy() || !self.getPnpCapability().isSupported()) ? local.getE164() : remote.getE164();
String e164 = SignalStore.account().isPrimaryDevice() && !self.getPnpCapability().isSupported() ? local.getE164() : remote.getE164();
List<String> defaultReactions = remote.getDefaultReactions().size() > 0 ? remote.getDefaultReactions() : local.getDefaultReactions();
boolean displayBadgesOnProfile = remote.isDisplayBadgesOnProfile();
boolean subscriptionManuallyCancelled = remote.isSubscriptionManuallyCancelled();
@ -169,7 +169,7 @@ public class AccountRecordProcessor extends DefaultStorageRecordProcessor<Signal
.setUsername(username)
.setUsernameLink(usernameLink);
if (!FeatureFlags.phoneNumberPrivacy() || !self.getPnpCapability().isSupported()) {
if (!self.getPnpCapability().isSupported()) {
builder.setE164(e164);
}

View file

@ -67,11 +67,6 @@ public class ContactRecordProcessor extends DefaultStorageRecordProcessor<Signal
*/
@Override
public void process(@NonNull Collection<SignalContactRecord> remoteRecords, @NonNull StorageKeyGenerator keyGenerator) throws IOException {
if (!FeatureFlags.phoneNumberPrivacy()) {
super.process(remoteRecords, keyGenerator);
return;
}
List<SignalContactRecord> unregisteredAciOnly = new ArrayList<>();
List<SignalContactRecord> pniE164Only = new ArrayList<>();
@ -149,10 +144,6 @@ public class ContactRecordProcessor extends DefaultStorageRecordProcessor<Signal
@Override
@NonNull Optional<SignalContactRecord> getMatching(@NonNull SignalContactRecord remote, @NonNull StorageKeyGenerator keyGenerator) {
if (!FeatureFlags.phoneNumberPrivacy()) {
remote = remote.withoutPni();
}
Optional<RecipientId> found = remote.getAci().isPresent() ? recipientTable.getByAci(remote.getAci().get()) : Optional.empty();
if (found.isEmpty() && remote.getNumber().isPresent()) {
@ -180,11 +171,6 @@ public class ContactRecordProcessor extends DefaultStorageRecordProcessor<Signal
@Override
@NonNull SignalContactRecord merge(@NonNull SignalContactRecord remote, @NonNull SignalContactRecord local, @NonNull StorageKeyGenerator keyGenerator) {
if (!FeatureFlags.phoneNumberPrivacy()) {
local = local.withoutPni();
remote = remote.withoutPni();
}
String profileGivenName;
String profileFamilyName;
@ -217,35 +203,30 @@ public class ContactRecordProcessor extends DefaultStorageRecordProcessor<Signal
PNI pni;
String e164;
if (FeatureFlags.phoneNumberPrivacy()) {
boolean e164sMatchButPnisDont = local.getNumber().isPresent() &&
local.getNumber().get().equals(remote.getNumber().orElse(null)) &&
local.getPni().isPresent() &&
remote.getPni().isPresent() &&
!local.getPni().get().equals(remote.getPni().get());
boolean e164sMatchButPnisDont = local.getNumber().isPresent() &&
local.getNumber().get().equals(remote.getNumber().orElse(null)) &&
local.getPni().isPresent() &&
remote.getPni().isPresent() &&
!local.getPni().get().equals(remote.getPni().get());
boolean pnisMatchButE164sDont = local.getPni().isPresent() &&
local.getPni().get().equals(remote.getPni().orElse(null)) &&
local.getNumber().isPresent() &&
remote.getNumber().isPresent() &&
!local.getNumber().get().equals(remote.getNumber().get());
boolean pnisMatchButE164sDont = local.getPni().isPresent() &&
local.getPni().get().equals(remote.getPni().orElse(null)) &&
local.getNumber().isPresent() &&
remote.getNumber().isPresent() &&
!local.getNumber().get().equals(remote.getNumber().get());
if (e164sMatchButPnisDont) {
Log.w(TAG, "Matching E164s, but the PNIs differ! Trusting our local pair.");
// TODO [pnp] Schedule CDS fetch?
pni = local.getPni().get();
e164 = local.getNumber().get();
} else if (pnisMatchButE164sDont) {
Log.w(TAG, "Matching PNIs, but the E164s differ! Trusting our local pair.");
// TODO [pnp] Schedule CDS fetch?
pni = local.getPni().get();
e164 = local.getNumber().get();
} else {
pni = OptionalUtil.or(remote.getPni(), local.getPni()).orElse(null);
e164 = OptionalUtil.or(remote.getNumber(), local.getNumber()).orElse(null);
}
if (e164sMatchButPnisDont) {
Log.w(TAG, "Matching E164s, but the PNIs differ! Trusting our local pair.");
// TODO [pnp] Schedule CDS fetch?
pni = local.getPni().get();
e164 = local.getNumber().get();
} else if (pnisMatchButE164sDont) {
Log.w(TAG, "Matching PNIs, but the E164s differ! Trusting our local pair.");
// TODO [pnp] Schedule CDS fetch?
pni = local.getPni().get();
e164 = local.getNumber().get();
} else {
pni = null;
pni = OptionalUtil.or(remote.getPni(), local.getPni()).orElse(null);
e164 = OptionalUtil.or(remote.getNumber(), local.getNumber()).orElse(null);
}

View file

@ -160,7 +160,7 @@ public final class StorageSyncHelper {
.setHasSeenGroupStoryEducationSheet(SignalStore.storyValues().getUserHasSeenGroupStoryEducationSheet())
.setUsername(self.getUsername().orElse(null));
if (!FeatureFlags.phoneNumberPrivacy() || !self.getPnpCapability().isSupported()) {
if (!self.getPnpCapability().isSupported()) {
account.setE164(self.requireE164());
}

View file

@ -381,7 +381,7 @@ class ContactRecordProcessorTest {
}
@Test
fun `merge, pnpDisabled, pniDropped`() {
fun `merge, pnpDisabled, pniNotDropped`() {
// GIVEN
val subject = ContactRecordProcessor(ACI_A, PNI_A, E164_A, recipientTable)
@ -411,7 +411,7 @@ class ContactRecordProcessorTest {
// THEN
assertEquals(remote.aci, result.aci)
assertEquals(remote.number.get(), result.number.get())
assertEquals(false, result.pni.isPresent)
assertEquals(true, result.pni.isPresent)
}
private fun buildRecord(id: StorageId = STORAGE_ID_A, record: ContactRecord): SignalContactRecord {