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

Respond to CR.

Browse Source
This commit is contained in:
Matthew Chen 2020-08-11 17:25:47 -03:00
parent 4fab1f7e09
commit e2c2f37e60
2 changed files with 122 additions and 105 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

166
SignalServiceKit/src/Contacts/SignalServiceAddress.swift
View File

@ -129,80 +129,6 @@ public class SignalServiceAddress: NSObject, NSCopying, NSSecureCoding, Codable
observeMappingChanges()
}
@objc
private class AddressMappingObserver: NSObject {
private let uuid: UUID
private let unfairLock = UnfairLock()
// There might be multiple instances in this aray that represent the same address.
//
// weakAddresses should only be accessed within unfairLock.
private var weakAddresses = NSHashTable<SignalServiceAddress>(options: [
.weakMemory,
.objectPointerPersonality
])
init(uuid: UUID) {
self.uuid = uuid
super.init()
registerForMappingChangeNotification()
}
func add(address: SignalServiceAddress) {
unfairLock.withLock {
weakAddresses.add(address)
}
}
private func registerForMappingChangeNotification() {
NotificationCenter.default.addObserver(
self, selector:
#selector(mappingDidChange),
name: SignalServiceAddressCache.mappingDidChangeNotification,
object: nil
)
}
@objc
func mappingDidChange(notification: Notification) {
guard let updatedUuid = notification.userInfo?[SignalServiceAddressCache.mappingDidChangeNotificationUUIDKey] as? UUID,
uuid == updatedUuid
else { return }
let addresses = unfairLock.withLock {
weakAddresses.allObjects
}
for address in addresses {
address.backingPhoneNumber.set(SignalServiceAddress.cache.phoneNumber(forUuid: uuid))
}
}
}
private var mappingObserver = AtomicOptional<AddressMappingObserver>(nil)
private static let mappingObserverCache = NSCache<NSUUID, AddressMappingObserver>()
private func observeMappingChanges() {
guard let uuid = backingUuid.get() else {
return
}
guard mappingObserver.get() == nil else {
owsFailDebug("There's shouldn't be an existing observer.")
return
}
let observer: AddressMappingObserver
if let cachedObserver = Self.mappingObserverCache.object(forKey: uuid as NSUUID) {
observer = cachedObserver
} else {
observer = AddressMappingObserver(uuid: uuid)
Self.mappingObserverCache.setObject(observer, forKey: uuid as NSUUID)
}
observer.add(address: self)
// We could race in this method, but in practice it should never happen.
// If it did, it wouldn't have any adverse side effects.
owsAssertDebug(mappingObserver.get() == nil)
mappingObserver.set(observer)
}
// MARK: - Codable
private enum CodingKeys: String, CodingKey {
@ -371,8 +297,91 @@ public class SignalServiceAddress: NSObject, NSCopying, NSSecureCoding, Codable
override public var description: String {
return "<SignalServiceAddress phoneNumber: \(phoneNumber ?? "nil"), uuid: \(uuid?.uuidString ?? "nil")>"
}
// MARK: - Mapping Changes
// The "observer" is a weak array of addresses with the
// same UUID observing changes to that UUID.
private typealias AddressMappingObserver = NSHashTable<SignalServiceAddress>
// Every address retains a strong reference to its observer.
private var mappingObserver = AtomicOptional<AddressMappingObserver>(nil)
private static let unfairLock = UnfairLock()
private static let mappingObserverCache = NSMapTable<NSUUID, AddressMappingObserver>(keyOptions: .strongMemory,
valueOptions: .weakMemory)
private func observeMappingChanges() {
guard let uuid = backingUuid.get() else {
return
}
guard mappingObserver.get() == nil else {
owsFailDebug("There's shouldn't be an existing observer.")
return
}
let observer = Self.unfairLock.withLock { () -> AddressMappingObserver in
let observer = { () -> AddressMappingObserver in
if let observer = Self.mappingObserverCache.object(forKey: uuid as NSUUID) {
return observer
} else {
// * Use weak references to addresses.
// * Use .objectPointerPersonality; this NSHashTable will contain
// a list of addresses that are all "equal".
let observer = NSHashTable<SignalServiceAddress>(options: [
.weakMemory,
.objectPointerPersonality
])
Self.mappingObserverCache.setObject(observer, forKey: uuid as NSUUID)
return observer
}
}()
observer.add(self)
return observer
}
// We could race in this method, but in practice it should never happen.
// If it did, it wouldn't have any adverse side effects.
owsAssertDebug(mappingObserver.get() == nil)
mappingObserver.set(observer)
}
fileprivate static func notifyMappingDidChange(forUuid uuid: UUID) {
guard let addresses = (Self.unfairLock.withLock { () -> [SignalServiceAddress]? in
guard let observer = Self.mappingObserverCache.object(forKey: uuid as NSUUID) else {
return nil
}
return observer.allObjects
}) else {
return
}
for address in addresses {
address.mappingDidChange(uuid: uuid)
}
}
fileprivate func mappingDidChange(uuid: UUID) {
owsAssertDebug(uuid == self.uuid)
backingPhoneNumber.set(SignalServiceAddress.cache.phoneNumber(forUuid: uuid))
}
}
// MARK: -
#if TESTABLE_BUILD
extension SignalServiceAddress {
var unresolvedUuid: UUID? {
backingUuid.get()
}
var unresolvedPhoneNumber: String? {
backingPhoneNumber.get()
}
}
#endif
// MARK: -
@objc
public class SignalServiceAddressCache: NSObject {
private let serialQueue = DispatchQueue(label: "SignalServiceAddressCache")
@ -462,9 +471,6 @@ public class SignalServiceAddressCache: NSObject {
return serialQueue.sync { uuidToPhoneNumberCache[uuid] }
}
static let mappingDidChangeNotification = Notification.Name("SignalServiceAddressCacheMappingDidChange")
static let mappingDidChangeNotificationUUIDKey = "mappingDidChangeNotificationUUIDKey"
@objc
func updateMapping(uuid: UUID, phoneNumber: String?) {
serialQueue.sync {
@ -505,10 +511,6 @@ public class SignalServiceAddressCache: NSObject {
}
// Notify any existing address objects to update their backing phone number
NotificationCenter.default.post(
name: SignalServiceAddressCache.mappingDidChangeNotification,
object: nil,
userInfo: [SignalServiceAddressCache.mappingDidChangeNotificationUUIDKey: uuid]
)
SignalServiceAddress.notifyMappingDidChange(forUuid: uuid)
}
}

61
SignalServiceKit/tests/Messages/SignalServiceAddressTest.swift
View File

@ -85,37 +85,52 @@ class SignalServiceAddressTest: SSKBaseTestSwift {
autoreleasepool {
let address1a = SignalServiceAddress(uuid: uuid1, phoneNumber: nil)
let address1b = SignalServiceAddress(uuid: uuid1, phoneNumber: nil)
let address1c = SignalServiceAddress(uuid: uuid1, phoneNumber: nil)
let address1c = SignalServiceAddress(uuid: uuid1, phoneNumber: phoneNumber1)
let address2a = SignalServiceAddress(uuid: uuid2, phoneNumber: nil)
let address2b = SignalServiceAddress(uuid: uuid2, phoneNumber: phoneNumber2)
XCTAssertEqual(address1a.uuid, uuid1)
XCTAssertNil(address1a.phoneNumber)
XCTAssertEqual(address1b.uuid, uuid1)
XCTAssertNil(address1b.phoneNumber)
XCTAssertEqual(address1c.uuid, uuid1)
XCTAssertNil(address1c.phoneNumber)
XCTAssertEqual(address2a.uuid, uuid2)
XCTAssertNil(address2a.phoneNumber)
XCTAssertEqual(address2b.uuid, uuid2)
XCTAssertEqual(address2b.uuid, uuid2)
// We use the "unresolved" accessors unresolvedUuid(), unresolvedPhoneNumber()
// to avoid filling in the backing values.
XCTAssertEqual(address1a.unresolvedUuid, uuid1)
XCTAssertNil(address1a.unresolvedPhoneNumber)
XCTAssertEqual(address1b.unresolvedUuid, uuid1)
XCTAssertNil(address1b.unresolvedPhoneNumber)
XCTAssertEqual(address1c.unresolvedUuid, uuid1)
XCTAssertEqual(address1c.unresolvedPhoneNumber, phoneNumber1)
XCTAssertEqual(address2a.unresolvedUuid, uuid2)
XCTAssertNil(address2a.unresolvedPhoneNumber)
XCTAssertEqual(address2b.unresolvedUuid, uuid2)
XCTAssertEqual(address2b.unresolvedPhoneNumber, phoneNumber2)
SSKEnvironment.shared.signalServiceAddressCache.updateMapping(uuid: uuid1, phoneNumber: phoneNumber1)
XCTAssertEqual(address1a.uuid, uuid1)
XCTAssertEqual(address1a.phoneNumber, phoneNumber1)
XCTAssertEqual(address1b.uuid, uuid1)
// Consulting address1b.phoneNumber will fill in the missing value.
XCTAssertEqual(address1b.phoneNumber, phoneNumber1)
XCTAssertEqual(address1c.uuid, uuid1)
// Do not consult address1c.phoneNumber.
XCTAssertEqual(address2a.uuid, uuid2)
XCTAssertNil(address2a.phoneNumber)
XCTAssertEqual(address2b.uuid, uuid2)
XCTAssertEqual(address2b.uuid, uuid2)
XCTAssertEqual(address1a.unresolvedUuid, uuid1)
XCTAssertEqual(address1a.unresolvedPhoneNumber, phoneNumber1)
XCTAssertEqual(address1b.unresolvedUuid, uuid1)
XCTAssertEqual(address1b.unresolvedPhoneNumber, phoneNumber1)
XCTAssertEqual(address1c.unresolvedUuid, uuid1)
XCTAssertEqual(address1c.unresolvedPhoneNumber, phoneNumber1)
XCTAssertEqual(address2a.unresolvedUuid, uuid2)
XCTAssertNil(address2a.unresolvedPhoneNumber)
XCTAssertEqual(address2b.unresolvedUuid, uuid2)
XCTAssertEqual(address2b.unresolvedPhoneNumber, phoneNumber2)
SSKEnvironment.shared.signalServiceAddressCache.updateMapping(uuid: uuid1, phoneNumber: phoneNumber3)
XCTAssertEqual(address1a.unresolvedUuid, uuid1)
XCTAssertEqual(address1a.unresolvedPhoneNumber, phoneNumber3)
XCTAssertEqual(address1b.unresolvedUuid, uuid1)
XCTAssertEqual(address1b.unresolvedPhoneNumber, phoneNumber3)
XCTAssertEqual(address1c.unresolvedUuid, uuid1)
XCTAssertEqual(address1c.unresolvedPhoneNumber, phoneNumber3)
XCTAssertEqual(address2a.unresolvedUuid, uuid2)
XCTAssertNil(address2a.unresolvedPhoneNumber)
XCTAssertEqual(address2b.unresolvedUuid, uuid2)
XCTAssertEqual(address2b.unresolvedPhoneNumber, phoneNumber2)
// MARK: - Resolved values
XCTAssertEqual(address1a.uuid, uuid1)
XCTAssertEqual(address1a.phoneNumber, phoneNumber3)
XCTAssertEqual(address1b.uuid, uuid1)
@ -125,7 +140,7 @@ class SignalServiceAddressTest: SSKBaseTestSwift {
XCTAssertEqual(address2a.uuid, uuid2)
XCTAssertNil(address2a.phoneNumber)
XCTAssertEqual(address2b.uuid, uuid2)
XCTAssertEqual(address2b.uuid, uuid2)
XCTAssertEqual(address2b.phoneNumber, phoneNumber2)
}
}
}