From 38bfb35e2cfe15bf9594df6df3dd0e9f020c818f Mon Sep 17 00:00:00 2001 From: Matthew Chen Date: Fri, 25 Oct 2019 13:20:39 -0300 Subject: [PATCH] Cache avatar data(s) on SignalAccount. --- SignalMessaging/contacts/OWSContactsManager.m | 1 + SignalServiceKit/Resources/schema.sql | 1 + .../src/Contacts/SignalAccount+SDS.swift | 30 +++++++--- SignalServiceKit/src/Contacts/SignalAccount.h | 26 ++++++++- SignalServiceKit/src/Contacts/SignalAccount.m | 55 ++++++++++++++++++- .../src/Devices/OWSContactsOutputStream.m | 16 +----- .../Storage/Database/GRDBSchemaMigrator.swift | 23 +++++++- SignalServiceKit/src/Util/NSData+Image.h | 2 +- SignalServiceKit/src/Util/NSData+Image.m | 2 +- 9 files changed, 129 insertions(+), 27 deletions(-) diff --git a/SignalMessaging/contacts/OWSContactsManager.m b/SignalMessaging/contacts/OWSContactsManager.m index a7d90a9b87..0ae7276cdd 100644 --- a/SignalMessaging/contacts/OWSContactsManager.m +++ b/SignalMessaging/contacts/OWSContactsManager.m @@ -582,6 +582,7 @@ NSString *const OWSContactsManagerKeyNextFullIntersectionDate = @"OWSContactsMan signalAccount.multipleAccountLabelText = [[self class] accountLabelForContact:contact address:signalRecipient.address]; } + [signalAccount tryToCacheContactAvatarData]; [signalAccounts addObject:signalAccount]; } } diff --git a/SignalServiceKit/Resources/schema.sql b/SignalServiceKit/Resources/schema.sql index 6b9111a130..60a54ae7b0 100644 --- a/SignalServiceKit/Resources/schema.sql +++ b/SignalServiceKit/Resources/schema.sql @@ -332,6 +332,7 @@ CREATE ,"multipleAccountLabelText" TEXT NOT NULL ,"recipientPhoneNumber" TEXT ,"recipientUUID" TEXT + ,"contactAvatarPngData" BLOB ) ; diff --git a/SignalServiceKit/src/Contacts/SignalAccount+SDS.swift b/SignalServiceKit/src/Contacts/SignalAccount+SDS.swift index eeac597b3b..382e95d7e3 100644 --- a/SignalServiceKit/src/Contacts/SignalAccount+SDS.swift +++ b/SignalServiceKit/src/Contacts/SignalAccount+SDS.swift @@ -29,6 +29,8 @@ public struct SignalAccountRecord: SDSRecord { // Base class properties public let contact: Data? + public let contactAvatarData: Data? + public let contactAvatarPngData: Data? public let multipleAccountLabelText: String public let recipientPhoneNumber: String? public let recipientUUID: String? @@ -38,6 +40,8 @@ public struct SignalAccountRecord: SDSRecord { case recordType case uniqueId case contact + case contactAvatarData + case contactAvatarPngData case multipleAccountLabelText case recipientPhoneNumber case recipientUUID @@ -68,9 +72,11 @@ public extension SignalAccountRecord { recordType = row[1] uniqueId = row[2] contact = row[3] - multipleAccountLabelText = row[4] - recipientPhoneNumber = row[5] - recipientUUID = row[6] + contactAvatarData = row[4] + contactAvatarPngData = row[5] + multipleAccountLabelText = row[6] + recipientPhoneNumber = row[7] + recipientUUID = row[8] } } @@ -104,6 +110,8 @@ extension SignalAccount { let uniqueId: String = record.uniqueId let contactSerialized: Data? = record.contact let contact: Contact? = try SDSDeserialization.optionalUnarchive(contactSerialized, name: "contact") + let contactAvatarData: Data? = SDSDeserialization.optionalData(record.contactAvatarData, name: "contactAvatarData") + let contactAvatarPngData: Data? = SDSDeserialization.optionalData(record.contactAvatarPngData, name: "contactAvatarPngData") let multipleAccountLabelText: String = record.multipleAccountLabelText let recipientPhoneNumber: String? = record.recipientPhoneNumber let recipientUUID: String? = record.recipientUUID @@ -111,6 +119,8 @@ extension SignalAccount { return SignalAccount(grdbId: recordId, uniqueId: uniqueId, contact: contact, + contactAvatarData: contactAvatarData, + contactAvatarPngData: contactAvatarPngData, multipleAccountLabelText: multipleAccountLabelText, recipientPhoneNumber: recipientPhoneNumber, recipientUUID: recipientUUID) @@ -159,9 +169,11 @@ extension SignalAccountSerializer { static let uniqueIdColumn = SDSColumnMetadata(columnName: "uniqueId", columnType: .unicodeString, isUnique: true, columnIndex: 2) // Base class properties static let contactColumn = SDSColumnMetadata(columnName: "contact", columnType: .blob, isOptional: true, columnIndex: 3) - static let multipleAccountLabelTextColumn = SDSColumnMetadata(columnName: "multipleAccountLabelText", columnType: .unicodeString, columnIndex: 4) - static let recipientPhoneNumberColumn = SDSColumnMetadata(columnName: "recipientPhoneNumber", columnType: .unicodeString, isOptional: true, columnIndex: 5) - static let recipientUUIDColumn = SDSColumnMetadata(columnName: "recipientUUID", columnType: .unicodeString, isOptional: true, columnIndex: 6) + static let contactAvatarDataColumn = SDSColumnMetadata(columnName: "contactAvatarData", columnType: .blob, isOptional: true, columnIndex: 4) + static let contactAvatarPngDataColumn = SDSColumnMetadata(columnName: "contactAvatarPngData", columnType: .blob, isOptional: true, columnIndex: 5) + static let multipleAccountLabelTextColumn = SDSColumnMetadata(columnName: "multipleAccountLabelText", columnType: .unicodeString, columnIndex: 6) + static let recipientPhoneNumberColumn = SDSColumnMetadata(columnName: "recipientPhoneNumber", columnType: .unicodeString, isOptional: true, columnIndex: 7) + static let recipientUUIDColumn = SDSColumnMetadata(columnName: "recipientUUID", columnType: .unicodeString, isOptional: true, columnIndex: 8) // TODO: We should decide on a naming convention for // tables that store models. @@ -172,6 +184,8 @@ extension SignalAccountSerializer { recordTypeColumn, uniqueIdColumn, contactColumn, + contactAvatarDataColumn, + contactAvatarPngDataColumn, multipleAccountLabelTextColumn, recipientPhoneNumberColumn, recipientUUIDColumn @@ -585,10 +599,12 @@ class SignalAccountSerializer: SDSSerializer { // Base class properties let contact: Data? = optionalArchive(model.contact) + let contactAvatarData: Data? = model.contactAvatarData + let contactAvatarPngData: Data? = model.contactAvatarPngData let multipleAccountLabelText: String = model.multipleAccountLabelText let recipientPhoneNumber: String? = model.recipientPhoneNumber let recipientUUID: String? = model.recipientUUID - return SignalAccountRecord(delegate: model, id: id, recordType: recordType, uniqueId: uniqueId, contact: contact, multipleAccountLabelText: multipleAccountLabelText, recipientPhoneNumber: recipientPhoneNumber, recipientUUID: recipientUUID) + return SignalAccountRecord(delegate: model, id: id, recordType: recordType, uniqueId: uniqueId, contact: contact, contactAvatarData: contactAvatarData, contactAvatarPngData: contactAvatarPngData, multipleAccountLabelText: multipleAccountLabelText, recipientPhoneNumber: recipientPhoneNumber, recipientUUID: recipientUUID) } } diff --git a/SignalServiceKit/src/Contacts/SignalAccount.h b/SignalServiceKit/src/Contacts/SignalAccount.h index a542e8fe22..9507ae21d9 100644 --- a/SignalServiceKit/src/Contacts/SignalAccount.h +++ b/SignalServiceKit/src/Contacts/SignalAccount.h @@ -31,6 +31,26 @@ NS_ASSUME_NONNULL_BEGIN // non-contact account. @property (nonatomic, nullable) Contact *contact; +// We cache the contact avatar data on this class. +// +// contactAvatarData is the original avatar data +// (if any) from the system contact. It might not be +// a PNG. We use it for change detection. When the +// underlying contact changes, we need to update +// the SignalAccount. +// +// contactAvatarPngData contains the data we'll sync +// to Desktop. We only want to send valid PNGs. +// Converting the avatars to PNGs isn't deterministic +// and our contact sync de-bouncing logic is based +// on the actual data sent over the wire, so we need +// to cache this as well. +// +// This property is optional and will not be set for +// non-contact account. +@property (nonatomic, nullable) NSData *contactAvatarData; +@property (nonatomic, nullable) NSData *contactAvatarPngData; + // For contacts with more than one signal account, // this is a label for the account. @property (nonatomic) NSString *multipleAccountLabelText; @@ -53,10 +73,12 @@ NS_ASSUME_NONNULL_BEGIN - (instancetype)initWithGrdbId:(int64_t)grdbId uniqueId:(NSString *)uniqueId contact:(nullable Contact *)contact + contactAvatarData:(nullable NSData *)contactAvatarData + contactAvatarPngData:(nullable NSData *)contactAvatarPngData multipleAccountLabelText:(NSString *)multipleAccountLabelText recipientPhoneNumber:(nullable NSString *)recipientPhoneNumber recipientUUID:(nullable NSString *)recipientUUID -NS_SWIFT_NAME(init(grdbId:uniqueId:contact:multipleAccountLabelText:recipientPhoneNumber:recipientUUID:)); +NS_SWIFT_NAME(init(grdbId:uniqueId:contact:contactAvatarData:contactAvatarPngData:multipleAccountLabelText:recipientPhoneNumber:recipientUUID:)); // clang-format on @@ -64,6 +86,8 @@ NS_SWIFT_NAME(init(grdbId:uniqueId:contact:multipleAccountLabelText:recipientPho - (BOOL)hasSameContent:(SignalAccount *)other; +- (void)tryToCacheContactAvatarData; + @end NS_ASSUME_NONNULL_END diff --git a/SignalServiceKit/src/Contacts/SignalAccount.m b/SignalServiceKit/src/Contacts/SignalAccount.m index d1f88864af..0d1244345f 100644 --- a/SignalServiceKit/src/Contacts/SignalAccount.m +++ b/SignalServiceKit/src/Contacts/SignalAccount.m @@ -4,6 +4,9 @@ #import "SignalAccount.h" #import "Contact.h" +#import "ContactsManagerProtocol.h" +#import "NSData+Image.h" +#import "SSKEnvironment.h" #import "SignalRecipient.h" #import #import @@ -22,6 +25,15 @@ NSUInteger const SignalAccountSchemaVersion = 1; @implementation SignalAccount +#pragma mark - Dependencies + +- (id)contactsManager +{ + return SSKEnvironment.shared.contactsManager; +} + +#pragma mark - + + (BOOL)shouldBeIndexedForFTS { return YES; @@ -75,6 +87,8 @@ NSUInteger const SignalAccountSchemaVersion = 1; - (instancetype)initWithGrdbId:(int64_t)grdbId uniqueId:(NSString *)uniqueId contact:(nullable Contact *)contact + contactAvatarData:(nullable NSData *)contactAvatarData + contactAvatarPngData:(nullable NSData *)contactAvatarPngData multipleAccountLabelText:(NSString *)multipleAccountLabelText recipientPhoneNumber:(nullable NSString *)recipientPhoneNumber recipientUUID:(nullable NSString *)recipientUUID @@ -87,6 +101,8 @@ NSUInteger const SignalAccountSchemaVersion = 1; } _contact = contact; + _contactAvatarData = contactAvatarData; + _contactAvatarPngData = contactAvatarPngData; _multipleAccountLabelText = multipleAccountLabelText; _recipientPhoneNumber = recipientPhoneNumber; _recipientUUID = recipientUUID; @@ -118,10 +134,47 @@ NSUInteger const SignalAccountSchemaVersion = 1; { OWSAssertDebug(other != nil); + // NOTE: We don't want to compare contactAvatarPngData, which + // is derived from contactAvatarData. return ([NSObject isNullableObject:self.recipientPhoneNumber equalTo:other.recipientPhoneNumber] && [NSObject isNullableObject:self.recipientUUID equalTo:other.recipientUUID] && [NSObject isNullableObject:self.contact equalTo:other.contact] && - [NSObject isNullableObject:self.multipleAccountLabelText equalTo:other.multipleAccountLabelText]); + [NSObject isNullableObject:self.multipleAccountLabelText equalTo:other.multipleAccountLabelText] && + [NSObject isNullableObject:self.contactAvatarData equalTo:other.contactAvatarData]); +} + +- (void)tryToCacheContactAvatarData +{ + OWSAssertDebug(self.contactAvatarData == nil); + OWSAssertDebug(self.contactAvatarPngData == nil); + + if (self.contact == nil) { + OWSFailDebug(@"Missing contact."); + return; + } + + self.contactAvatarData = [self.contactsManager avatarDataForCNContactId:self.contact.cnContactId]; + if (self.contactAvatarData == nil) { + return; + } + + if (self.contactAvatarData.ows_isValidPng) { + self.contactAvatarPngData = self.contactAvatarData; + return; + } + + // TODO: Avoid this work by caching the avatarPngData on SignalAccount. + UIImage *_Nullable avatarImage = [UIImage imageWithData:self.contactAvatarData]; + if (avatarImage == nil) { + OWSFailDebug(@"Could not load avatar."); + return; + } + self.contactAvatarPngData = UIImagePNGRepresentation(avatarImage); + if (self.contactAvatarPngData == nil) { + OWSFailDebug(@"Could not convert avatar to PNG."); + return; + } + OWSLogVerbose(@"Converted avatar to PNG."); } @end diff --git a/SignalServiceKit/src/Devices/OWSContactsOutputStream.m b/SignalServiceKit/src/Devices/OWSContactsOutputStream.m index 07adfc7183..c0ffdd1d8c 100644 --- a/SignalServiceKit/src/Devices/OWSContactsOutputStream.m +++ b/SignalServiceKit/src/Devices/OWSContactsOutputStream.m @@ -50,21 +50,7 @@ disappearingMessagesConfiguration:(nullable OWSDisappearingMessagesConfiguration contactBuilder.verified = verified; } - NSData *_Nullable rawAvatarData = [contactsManager avatarDataForCNContactId:signalAccount.contact.cnContactId]; - NSData *_Nullable avatarPngData; - if (rawAvatarData != nil) { - if (rawAvatarData.ows_isValidPng) { - avatarPngData = rawAvatarData; - } else { - OWSLogVerbose(@"Converting avatar to PNG."); - // TODO: Avoid this work by caching the avatarPngData on SignalAccount. - UIImage *_Nullable avatarImage = [UIImage imageWithData:rawAvatarData]; - if (avatarImage != nil) { - avatarPngData = UIImagePNGRepresentation(avatarImage); - } - } - } - + NSData *_Nullable avatarPngData = signalAccount.contactAvatarPngData; if (avatarPngData != nil) { SSKProtoContactDetailsAvatarBuilder *avatarBuilder = [SSKProtoContactDetailsAvatar builder]; [avatarBuilder setContentType:OWSMimeTypeImagePng]; diff --git a/SignalServiceKit/src/Storage/Database/GRDBSchemaMigrator.swift b/SignalServiceKit/src/Storage/Database/GRDBSchemaMigrator.swift index e65334b22c..042470caac 100644 --- a/SignalServiceKit/src/Storage/Database/GRDBSchemaMigrator.swift +++ b/SignalServiceKit/src/Storage/Database/GRDBSchemaMigrator.swift @@ -26,6 +26,8 @@ public class GRDBSchemaMigrator: NSObject { private enum MigrationId: String, CaseIterable { case createInitialSchema + case signalAccount_add_contactAvatarPngData + case signalAccount_add_contactAvatarData } // For new users, we import the latest schema with the first migration @@ -57,10 +59,27 @@ public class GRDBSchemaMigrator: NSObject { // to the latest. private lazy var incrementalMigrator: DatabaseMigrator = { var migrator = DatabaseMigrator() - migrator.registerMigration(MigrationId.createInitialSchema.rawValue) { db in + migrator.registerMigration(MigrationId.createInitialSchema.rawValue) { _ in owsFail("This migration should have already been run by the last YapDB migration.") // try createV1Schema(db: db) } + migrator.registerMigration(MigrationId.signalAccount_add_contactAvatarPngData.rawValue) { database in + let sql = """ + ALTER TABLE \(SignalAccountRecord.databaseTableName) + ADD COLUMN \(signalAccountColumn: .contactAvatarPngData) BLOB + """ + let statement = try database.makeUpdateStatement(sql: sql) + try statement.execute() + } + migrator.registerMigration(MigrationId.signalAccount_add_contactAvatarData.rawValue) { database in + let sql = """ + ALTER TABLE \(SignalAccountRecord.databaseTableName) + ADD COLUMN \(signalAccountColumn: .contactAvatarData) BLOB + """ + let statement = try database.makeUpdateStatement(sql: sql) + try statement.execute() + } + return migrator }() } @@ -388,6 +407,8 @@ private func createV1Schema(db: Database) throws { .notNull() table.column("recipientPhoneNumber", .text) table.column("recipientUUID", .text) + table.column("contactAvatarPngData", .blob) + table.column("contactAvatarData", .blob) } try db.create(index: "index_model_SignalAccount_on_uniqueId", on: "model_SignalAccount", columns: ["uniqueId"]) diff --git a/SignalServiceKit/src/Util/NSData+Image.h b/SignalServiceKit/src/Util/NSData+Image.h index df9476e40b..84d41f3abe 100644 --- a/SignalServiceKit/src/Util/NSData+Image.h +++ b/SignalServiceKit/src/Util/NSData+Image.h @@ -23,7 +23,7 @@ NS_ASSUME_NONNULL_BEGIN - (nullable UIImage *)stillForWebpData; -- (BOOL)ows_isValidGroupAvatarPng; +- (BOOL)ows_isValidPng; @end diff --git a/SignalServiceKit/src/Util/NSData+Image.m b/SignalServiceKit/src/Util/NSData+Image.m index cf81c4f106..c449269a91 100644 --- a/SignalServiceKit/src/Util/NSData+Image.m +++ b/SignalServiceKit/src/Util/NSData+Image.m @@ -555,7 +555,7 @@ NSString *NSStringForImageFormat(ImageFormat value) return uiImage; } -- (BOOL)ows_isValidGroupAvatarPng +- (BOOL)ows_isValidPng { ImageFormat imageFormat = [self ows_guessImageFormat]; if (imageFormat != ImageFormat_Png) {