signalapp/Signal-iOS
signalapp/Signal-iOS
1
0
Fork 0
Code Issues Pull Requests Actions 1 Packages Projects Releases 4 Wiki Activity

Filter storage service contacts more consistently

Browse Source
This commit is contained in:
Max Radermacher 2023-02-13 10:48:54 -08:00 committed by GitHub
parent 1a0efe6fbb
commit db2678e86b
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
7 changed files with 149 additions and 44 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

4
Signal.xcodeproj/project.pbxproj
View File

@ -764,6 +764,7 @@
5049FA3028BEAABE00D6E099 /* ContactDiscoveryV2Connection.swift in Sources */ = {isa = PBXBuildFile; fileRef = 5049FA2E28BEAABE00D6E099 /* ContactDiscoveryV2Connection.swift */; };
5049FA3228BEAAD800D6E099 /* cdsi.pb.swift in Sources */ = {isa = PBXBuildFile; fileRef = 5049FA3128BEAAD800D6E099 /* cdsi.pb.swift */; };
505C2ED029937B5B00C23FB2 /* Data+KeyVersionByte.swift in Sources */ = {isa = PBXBuildFile; fileRef = 505C2ECF29937B5B00C23FB2 /* Data+KeyVersionByte.swift */; };
505C2EDB29974D2000C23FB2 /* StorageServiceContactTest.swift in Sources */ = {isa = PBXBuildFile; fileRef = 505C2EDA29974D2000C23FB2 /* StorageServiceContactTest.swift */; };
506A924F28FA232100A2EFC2 /* ContactDiscoveryManager.swift in Sources */ = {isa = PBXBuildFile; fileRef = 506A924E28FA232100A2EFC2 /* ContactDiscoveryManager.swift */; };
506A92512900888900A2EFC2 /* RecipientPickerContainerViewController.swift in Sources */ = {isa = PBXBuildFile; fileRef = 506A92502900888900A2EFC2 /* RecipientPickerContainerViewController.swift */; };
506A9252290326A600A2EFC2 /* TextHelper.swift in Sources */ = {isa = PBXBuildFile; fileRef = 4521C3BF1F59F3BA00B4C582 /* TextHelper.swift */; };
@ -3164,6 +3165,7 @@
5049FA2E28BEAABE00D6E099 /* ContactDiscoveryV2Connection.swift */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.swift; path = ContactDiscoveryV2Connection.swift; sourceTree = "<group>"; };
5049FA3128BEAAD800D6E099 /* cdsi.pb.swift */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.swift; path = cdsi.pb.swift; sourceTree = "<group>"; };
505C2ECF29937B5B00C23FB2 /* Data+KeyVersionByte.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = "Data+KeyVersionByte.swift"; sourceTree = "<group>"; };
505C2EDA29974D2000C23FB2 /* StorageServiceContactTest.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = StorageServiceContactTest.swift; sourceTree = "<group>"; };
506A924E28FA232100A2EFC2 /* ContactDiscoveryManager.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = ContactDiscoveryManager.swift; sourceTree = "<group>"; };
506A92502900888900A2EFC2 /* RecipientPickerContainerViewController.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = RecipientPickerContainerViewController.swift; sourceTree = "<group>"; };
508F0345296F72F4001D88D0 /* CustomCellBackgroundColor.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = CustomCellBackgroundColor.swift; sourceTree = "<group>"; };
@ -8931,6 +8933,7 @@
isa = PBXGroup;
children = (
F9C612B3284E466B00B2199A /* CGPointExtensionsTest.swift */,
505C2EDA29974D2000C23FB2 /* StorageServiceContactTest.swift */,
F992ACC328F8C9D900906038 /* StripeTest.swift */,
503BDDB3296F3E2C00FED3B2 /* SystemContactsDataProviderTest.swift */,
);
@ -11224,6 +11227,7 @@
34843B2421432293004DED45 /* SignalBaseTest.m in Sources */,
F9844C492867936400B16DD4 /* SignalMeTest.swift in Sources */,
4C3EF802210918740007EBF7 /* SSKProtoEnvelopeTest.swift in Sources */,
505C2EDB29974D2000C23FB2 /* StorageServiceContactTest.swift in Sources */,
8803FF6628EF89B50023574A /* StorySharingTests.swift in Sources */,
452D1AF12081059C00A67F7F /* StringAdditionsTest.swift in Sources */,
F992ACC428F8C9D900906038 /* StripeTest.swift in Sources */,

48
SignalMessaging/Storage Service/StorageServiceManager.swift
View File

@ -1030,15 +1030,17 @@ class StorageServiceOperation: OWSOperation {
}
var orphanedAccountCount = 0
let currentDate = Date()
for (accountId, identifier) in mutableState.accountIdToIdentifierMap where !allManifestItems.contains(identifier) {
// Only consider registered recipients as orphaned, if another client removes
// an unregistered recipient allow it.
// Only consider registered recipients as orphaned. If another client
// removes an unregistered recipient, allow it.
guard
let address = OWSAccountIdFinder.address(forAccountId: accountId, transaction: transaction),
let recipient = AnySignalRecipientFinder().signalRecipient(for: address, transaction: transaction),
recipient.isRegistered
else { continue }
let storageServiceContact = StorageServiceContact.fetch(for: accountId, transaction: transaction),
storageServiceContact.shouldBeInStorageService(currentDate: currentDate),
storageServiceContact.registrationStatus(currentDate: currentDate) == .registered
else {
continue
}
mutableState.accountIdChangeMap[accountId] = .updated
orphanedAccountCount += 1
}
@ -1254,26 +1256,24 @@ class StorageServiceOperation: OWSOperation {
}
private func cleanUpOrphanedAccounts(transaction: SDSAnyWriteTransaction) {
// We don't keep unregistered accounts in storage service after a certain amount of time.
// We may also have records for accounts that no longer exists, e.g. that SignalRecipient
// was merged with another recipient. We try to proactively delete these records from storage
// service, but there was a period of time we didn't and we need to cleanup after ourselves.
let orphanedAccountIds = State.current(transaction: transaction)
.accountIdToIdentifierMap
.keys
.filter { accountId in
guard let address = OWSAccountIdFinder.address(
forAccountId: accountId,
transaction: transaction
) else { return true }
// We don't keep unregistered accounts in storage service after a certain
// amount of time. We may also have records for accounts that no longer
// exist, e.g. that SignalRecipient was merged with another recipient. We
// try to proactively delete these records from storage service, but there
// was a period of time we didn't, and we need to cleanup after ourselves.
guard let recipient = AnySignalRecipientFinder().signalRecipient(
for: address,
transaction: transaction
) else { return true }
let currentDate = Date()
return !recipient.shouldBeRepresentedInStorageService
func shouldRecipientBeInStorageService(accountId: AccountId) -> Bool {
guard let storageServiceContact = StorageServiceContact.fetch(for: accountId, transaction: transaction) else {
return false
}
return storageServiceContact.shouldBeInStorageService(currentDate: currentDate)
}
let orphanedAccountIds = State.current(transaction: transaction).accountIdToIdentifierMap.keys.filter {
!shouldRecipientBeInStorageService(accountId: $0)
}
guard !orphanedAccountIds.isEmpty else { return }

70
SignalMessaging/Storage Service/StorageServiceProto+Sync.swift
View File

@ -69,6 +69,63 @@ enum StorageServiceMergeResult<IdType> {
// MARK: - Contact Record
struct StorageServiceContact {
private enum Constant {
static let storageServiceUnregisteredThreshold = kMonthInterval
}
/// Contact records may be unregistered.
var unregisteredAtTimestamp: UInt64?
init(unregisteredAtTimestamp: UInt64?) {
self.unregisteredAtTimestamp = unregisteredAtTimestamp
}
enum RegistrationStatus {
case registered
case unregisteredRecently
case unregisteredMoreThanOneMonthAgo
}
func registrationStatus(currentDate: Date) -> RegistrationStatus {
switch unregisteredAtTimestamp {
case .none:
return .registered
case .some(let timestamp) where currentDate.timeIntervalSince(Date(millisecondsSince1970: timestamp)) <= Constant.storageServiceUnregisteredThreshold:
return .unregisteredRecently
case .some:
return .unregisteredMoreThanOneMonthAgo
}
}
static func fetch(for accountId: AccountId, transaction: SDSAnyReadTransaction) -> Self? {
SignalRecipient.anyFetch(uniqueId: accountId, transaction: transaction).flatMap { Self($0) }
}
fileprivate init?(_ signalRecipient: SignalRecipient) {
let unregisteredAtTimestamp: UInt64?
if signalRecipient.isRegistered {
unregisteredAtTimestamp = nil
} else {
unregisteredAtTimestamp = (
signalRecipient.unregisteredAtTimestamp?.uint64Value ?? SignalRecipientDistantPastUnregisteredTimestamp
)
}
self.init(unregisteredAtTimestamp: unregisteredAtTimestamp)
}
func shouldBeInStorageService(currentDate: Date) -> Bool {
switch registrationStatus(currentDate: currentDate) {
case .registered, .unregisteredRecently:
return true
case .unregisteredMoreThanOneMonthAgo:
return false
}
}
}
class StorageServiceContactRecordUpdater: StorageServiceRecordUpdater {
typealias IdType = AccountId
typealias RecordType = StorageServiceProtoContactRecord
@ -103,19 +160,22 @@ class StorageServiceContactRecordUpdater: StorageServiceRecordUpdater {
unknownFields: UnknownStorage?,
transaction: SDSAnyReadTransaction
) -> StorageServiceProtoContactRecord? {
guard
let address = OWSAccountIdFinder.address(forAccountId: accountId, transaction: transaction),
let recipient = AnySignalRecipientFinder().signalRecipient(for: address, transaction: transaction)
else {
guard let recipient = SignalRecipient.anyFetch(uniqueId: accountId, transaction: transaction) else {
return nil
}
guard let contact = StorageServiceContact(recipient), contact.shouldBeInStorageService(currentDate: Date()) else {
return nil
}
var builder = StorageServiceProtoContactRecord.builder()
if !recipient.isRegistered, let unregisteredAtTimestamp = recipient.unregisteredAtTimestamp?.uint64Value {
if let unregisteredAtTimestamp = contact.unregisteredAtTimestamp {
builder.setUnregisteredAtTimestamp(unregisteredAtTimestamp)
}
let address = recipient.address
if let phoneNumber = address.phoneNumber {
if PhoneNumber.resemblesE164(phoneNumber) {
builder.setServiceE164(phoneNumber)

50
SignalMessaging/test/StorageServiceContactTest.swift Normal file
View File

@ -0,0 +1,50 @@
//
// Copyright 2023 Signal Messenger, LLC
// SPDX-License-Identifier: AGPL-3.0-only
//
import XCTest
@testable import SignalMessaging
class StorageServiceContactTest: XCTestCase {
func testRegistrationStatus() {
let now = Date()
let nowMs = now.ows_millisecondsSince1970
let testCases: [(UInt64?, StorageServiceContact.RegistrationStatus)] = [
(nil, .registered),
(nowMs, .unregisteredRecently),
(nowMs, .unregisteredRecently),
(nowMs - 29 * kDayInMs, .unregisteredRecently),
(nowMs - 32 * kDayInMs, .unregisteredMoreThanOneMonthAgo),
(nowMs + 100 * kDayInMs, .unregisteredRecently)
]
for (unregisteredAtTimestamp, expectedValue) in testCases {
let storageServiceContact = StorageServiceContact(unregisteredAtTimestamp: unregisteredAtTimestamp)
let actualValue = storageServiceContact.registrationStatus(currentDate: now)
XCTAssertEqual(actualValue, expectedValue, String(describing: unregisteredAtTimestamp))
}
}
func testShouldBeInStorageService() {
let now = Date()
let nowMs = now.ows_millisecondsSince1970
let testCases: [(UInt64?, Bool)] = [
(nil, true),
(nowMs, true),
(nowMs, true),
(nowMs - 29 * kDayInMs, true),
(nowMs + 100 * kDayInMs, true),
(nowMs - 32 * kDayInMs, false)
]
for (unregisteredAtTimestamp, expectedValue) in testCases {
let storageServiceContact = StorageServiceContact(unregisteredAtTimestamp: unregisteredAtTimestamp)
let actualValue = storageServiceContact.shouldBeInStorageService(currentDate: now)
XCTAssertEqual(actualValue, expectedValue, String(describing: unregisteredAtTimestamp))
}
}
}

2
SignalServiceKit/src/Contacts/SignalRecipient.h
View File

@ -21,6 +21,8 @@ typedef NS_CLOSED_ENUM(NSUInteger, SignalRecipientSource) {
SignalRecipientSourceStorageService,
};
extern const uint64_t SignalRecipientDistantPastUnregisteredTimestamp;
/// SignalRecipient serves two purposes:
///
/// a) It serves as a cache of "known" Signal accounts. When the service indicates

6
SignalServiceKit/src/Contacts/SignalRecipient.m
View File

@ -14,6 +14,8 @@ NS_ASSUME_NONNULL_BEGIN
const NSUInteger SignalRecipientSchemaVersion = 1;
const uint64_t SignalRecipientDistantPastUnregisteredTimestamp = 1;
@interface SignalRecipient ()
@property (nonatomic) NSOrderedSet<NSNumber *> *devices;
@ -37,7 +39,7 @@ const NSUInteger SignalRecipientSchemaVersion = 1;
_recipientPhoneNumber = nil;
_recipientSchemaVersion = SignalRecipientSchemaVersion;
// New recipients start out as "unregistered in the distant past"
_unregisteredAtTimestamp = @1;
_unregisteredAtTimestamp = @(SignalRecipientDistantPastUnregisteredTimestamp);
_devices = [NSOrderedSet orderedSet];
return self;
@ -55,7 +57,7 @@ const NSUInteger SignalRecipientSchemaVersion = 1;
_recipientPhoneNumber = address.phoneNumber;
_recipientSchemaVersion = SignalRecipientSchemaVersion;
// New recipients start out as "unregistered in the distant past"
_unregisteredAtTimestamp = @1;
_unregisteredAtTimestamp = @(SignalRecipientDistantPastUnregisteredTimestamp);
_devices = [NSOrderedSet orderedSet];
return self;

13
SignalServiceKit/src/Contacts/SignalRecipient.swift
View File

@ -16,19 +16,6 @@ extension SignalRecipient {
(devices.array as? [NSNumber])?.map { $0.uint32Value }
}
private static let storageServiceUnregisteredThreshold = kMonthInterval
@objc
public var shouldBeRepresentedInStorageService: Bool {
guard !isRegistered else { return true }
guard let unregisteredAtTimestamp = unregisteredAtTimestamp?.uint64Value else {
return false
}
return Date().timeIntervalSince(Date(millisecondsSince1970: unregisteredAtTimestamp)) <= Self.storageServiceUnregisteredThreshold
}
// MARK: -
public func markAsUnregistered(at timestamp: UInt64? = nil, source: SignalRecipientSource = .local, transaction: SDSAnyWriteTransaction) {