From 00feb14b10353191f0a76fd8590456516eb3277b Mon Sep 17 00:00:00 2001 From: Matthew Chen Date: Wed, 15 Nov 2017 13:15:48 -0500 Subject: [PATCH] Respond to CR. // FREEBIE --- ...ExperienceUpgradesPageViewController.swift | 6 +- .../MessageDetailViewController.swift | 12 +- .../environment/ExperienceUpgradeFinder.swift | 2 +- .../ExperienceUpgrade.swift | 2 +- .../OWSDisappearingMessagesConfiguration.m | 2 +- SignalServiceKit/src/Contacts/TSThread.h | 24 +--- SignalServiceKit/src/Contacts/TSThread.m | 28 +--- .../src/Messages/Interactions/TSMessage.h | 24 +--- .../src/Messages/Interactions/TSMessage.m | 27 +--- .../Messages/Interactions/TSOutgoingMessage.h | 24 +--- .../Messages/Interactions/TSOutgoingMessage.m | 120 ++++++++---------- .../src/Messages/OWSBatchMessageProcessor.m | 2 +- .../src/Messages/OWSMessageReceiver.m | 2 +- .../src/Security/OWSRecipientIdentity.h | 2 +- .../src/Storage/TSYapDatabaseObject.h | 46 ++++++- .../src/Storage/TSYapDatabaseObject.m | 37 +++++- 16 files changed, 163 insertions(+), 197 deletions(-) diff --git a/Signal/src/ViewControllers/ExperienceUpgradesPageViewController.swift b/Signal/src/ViewControllers/ExperienceUpgradesPageViewController.swift index e41f8f8aa..dc9d2c1d5 100644 --- a/Signal/src/ViewControllers/ExperienceUpgradesPageViewController.swift +++ b/Signal/src/ViewControllers/ExperienceUpgradesPageViewController.swift @@ -526,7 +526,11 @@ class ExperienceUpgradesPageViewController: OWSViewController, UIPageViewControl } public func addViewController(experienceUpgrade: ExperienceUpgrade) { - guard let identifier = ExperienceUpgradeId(rawValue: experienceUpgrade.uniqueId) else { + guard let uniqueId = experienceUpgrade.uniqueId else { + Logger.error("\(self.TAG) experienceUpgrade is missing uniqueId.") + return + } + guard let identifier = ExperienceUpgradeId(rawValue: uniqueId) else { owsFail("\(TAG) unknown experience upgrade. skipping") return } diff --git a/Signal/src/ViewControllers/MessageDetailViewController.swift b/Signal/src/ViewControllers/MessageDetailViewController.swift index 455e637a3..bac831bcd 100644 --- a/Signal/src/ViewControllers/MessageDetailViewController.swift +++ b/Signal/src/ViewControllers/MessageDetailViewController.swift @@ -551,7 +551,11 @@ class MessageDetailViewController: OWSViewController, UIScrollViewDelegate { AssertIsOnMainThread() self.databaseConnection.read { transaction in - guard let newMessage = TSInteraction.fetch(uniqueId: self.message.uniqueId, transaction: transaction) as? TSMessage else { + guard let uniqueId = self.message.uniqueId else { + Logger.error("\(self.TAG) Message is missing uniqueId.") + return + } + guard let newMessage = TSInteraction.fetch(uniqueId: uniqueId, transaction: transaction) as? TSMessage else { Logger.error("\(self.TAG) Couldn't reload message.") return } @@ -564,7 +568,11 @@ class MessageDetailViewController: OWSViewController, UIScrollViewDelegate { let notifications = self.databaseConnection.beginLongLivedReadTransaction() - guard self.databaseConnection.hasChange(forKey: message.uniqueId, + guard let uniqueId = self.message.uniqueId else { + Logger.error("\(self.TAG) Message is missing uniqueId.") + return + } + guard self.databaseConnection.hasChange(forKey: uniqueId, inCollection: TSInteraction.collection(), in: notifications) else { Logger.debug("\(TAG) No relevant changes.") diff --git a/Signal/src/environment/ExperienceUpgradeFinder.swift b/Signal/src/environment/ExperienceUpgradeFinder.swift index 537ca2844..0641b960e 100644 --- a/Signal/src/environment/ExperienceUpgradeFinder.swift +++ b/Signal/src/environment/ExperienceUpgradeFinder.swift @@ -59,7 +59,7 @@ class ExperienceUpgradeFinder: NSObject { // MARK: - Instance Methods public func allUnseen(transaction: YapDatabaseReadTransaction) -> [ExperienceUpgrade] { - return allExperienceUpgrades.filter { ExperienceUpgrade.fetch(uniqueId: $0.uniqueId, transaction: transaction) == nil } + return allExperienceUpgrades.filter { ExperienceUpgrade.fetch(uniqueId: $0.uniqueId!, transaction: transaction) == nil } } public func markAllAsSeen(transaction: YapDatabaseReadWriteTransaction) { diff --git a/Signal/src/environment/ExperienceUpgrades/ExperienceUpgrade.swift b/Signal/src/environment/ExperienceUpgrades/ExperienceUpgrade.swift index 699a4e225..2e86facd7 100644 --- a/Signal/src/environment/ExperienceUpgrades/ExperienceUpgrade.swift +++ b/Signal/src/environment/ExperienceUpgrades/ExperienceUpgrade.swift @@ -16,7 +16,7 @@ class ExperienceUpgrade: TSYapDatabaseObject { super.init(uniqueId: uniqueId) } - override required init(uniqueId: String) { + override required init(uniqueId: String?) { // This is the unfortunate seam between strict swift and fast-and-loose objc // we can't leave these properties nil, since we really "don't know" that the superclass // will assign them. diff --git a/SignalServiceKit/src/Contacts/OWSDisappearingMessagesConfiguration.m b/SignalServiceKit/src/Contacts/OWSDisappearingMessagesConfiguration.m index 484420ea3..f3242de2f 100644 --- a/SignalServiceKit/src/Contacts/OWSDisappearingMessagesConfiguration.m +++ b/SignalServiceKit/src/Contacts/OWSDisappearingMessagesConfiguration.m @@ -27,7 +27,7 @@ const uint32_t OWSDisappearingMessagesConfigurationDefaultExpirationDuration = k durationSeconds:OWSDisappearingMessagesConfigurationDefaultExpirationDuration]; } -- (instancetype)initWithCoder:(NSCoder *)coder +- (nullable instancetype)initWithCoder:(NSCoder *)coder { self = [super initWithCoder:coder]; diff --git a/SignalServiceKit/src/Contacts/TSThread.h b/SignalServiceKit/src/Contacts/TSThread.h index 4ed06aa29..e0308edae 100644 --- a/SignalServiceKit/src/Contacts/TSThread.h +++ b/SignalServiceKit/src/Contacts/TSThread.h @@ -156,28 +156,8 @@ NS_ASSUME_NONNULL_BEGIN @property (atomic, readonly) BOOL isMuted; @property (atomic, readonly, nullable) NSDate *mutedUntilDate; -// This model may be updated from many threads. We don't want to save -// our local copy (this instance) since it may be out of date. We also -// want to avoid re-saving a model that has been deleted. Therefore, we -// use these "updateWith..." methods to: -// -// a) Update a property of this instance. -// b) If a copy of this model exists in the database, load an up-to-date copy, -// and update and save that copy. -// b) If a copy of this model _DOES NOT_ exist in the database, do _NOT_ save -// this local instance. -// -// After "updateWith...": -// -// a) An copy of this model in the database will have been updated. -// b) The local property on this instance will always have been updated. -// c) Other properties on this instance may be out of date. -// -// All mutable properties of this class have been made read-only to -// prevent accidentally modifying them directly. -// -// This isn't a perfect arrangement, but in practice this will prevent -// data loss and will resolve all known issues. +#pragma mark - Update With... Methods + - (void)updateWithMutedUntilDate:(NSDate *)mutedUntilDate; @end diff --git a/SignalServiceKit/src/Contacts/TSThread.m b/SignalServiceKit/src/Contacts/TSThread.m index 292a570bc..4fa1b9b8f 100644 --- a/SignalServiceKit/src/Contacts/TSThread.m +++ b/SignalServiceKit/src/Contacts/TSThread.m @@ -34,7 +34,8 @@ NS_ASSUME_NONNULL_BEGIN return @"TSThread"; } -- (instancetype)initWithUniqueId:(NSString *)uniqueId { +- (instancetype)initWithUniqueId:(NSString *_Nullable)uniqueId +{ self = [super initWithUniqueId:uniqueId]; if (self) { @@ -382,30 +383,15 @@ NS_ASSUME_NONNULL_BEGIN [mutedUntilDate timeIntervalSinceDate:now] > 0); } -// This method does the work for the "updateWith..." methods. Please see -// the header for a discussion of those methods. -- (void)applyChangeToSelfAndLatestThread:(YapDatabaseReadWriteTransaction *)transaction - changeBlock:(void (^)(TSThread *))changeBlock -{ - OWSAssert(transaction); - - changeBlock(self); - - NSString *collection = [[self class] collection]; - TSThread *latestInstance = [transaction objectForKey:self.uniqueId inCollection:collection]; - if (latestInstance) { - changeBlock(latestInstance); - [latestInstance saveWithTransaction:transaction]; - } -} +#pragma mark - Update With... Methods - (void)updateWithMutedUntilDate:(NSDate *)mutedUntilDate { [self.dbReadWriteConnection readWriteWithBlock:^(YapDatabaseReadWriteTransaction *transaction) { - [self applyChangeToSelfAndLatestThread:transaction - changeBlock:^(TSThread *thread) { - [thread setMutedUntilDate:mutedUntilDate]; - }]; + [self applyChangeToSelfAndLatestCopy:transaction + changeBlock:^(TSThread *thread) { + [thread setMutedUntilDate:mutedUntilDate]; + }]; }]; } diff --git a/SignalServiceKit/src/Messages/Interactions/TSMessage.h b/SignalServiceKit/src/Messages/Interactions/TSMessage.h index 068f9c28a..e00caa19b 100644 --- a/SignalServiceKit/src/Messages/Interactions/TSMessage.h +++ b/SignalServiceKit/src/Messages/Interactions/TSMessage.h @@ -56,30 +56,8 @@ NS_ASSUME_NONNULL_BEGIN - (BOOL)shouldStartExpireTimer; - (BOOL)shouldStartExpireTimer:(YapDatabaseReadTransaction *)transaction; -#pragma mark - Update Methods +#pragma mark - Update With... Methods -// This model may be updated from many threads. We don't want to save -// our local copy (this instance) since it may be out of date. We also -// want to avoid re-saving a model that has been deleted. Therefore, we -// use these "updateWith..." methods to: -// -// a) Update a property of this instance. -// b) If a copy of this model exists in the database, load an up-to-date copy, -// and update and save that copy. -// b) If a copy of this model _DOES NOT_ exist in the database, do _NOT_ save -// this local instance. -// -// After "updateWith...": -// -// a) An copy of this model in the database will have been updated. -// b) The local property on this instance will always have been updated. -// c) Other properties on this instance may be out of date. -// -// All mutable properties of this class have been made read-only to -// prevent accidentally modifying them directly. -// -// This isn't a perfect arrangement, but in practice this will prevent -// data loss and will resolve all known issues. - (void)updateWithExpireStartedAt:(uint64_t)expireStartedAt transaction:(YapDatabaseReadWriteTransaction *)transaction; @end diff --git a/SignalServiceKit/src/Messages/Interactions/TSMessage.m b/SignalServiceKit/src/Messages/Interactions/TSMessage.m index 74a8b7282..8207da26b 100644 --- a/SignalServiceKit/src/Messages/Interactions/TSMessage.m +++ b/SignalServiceKit/src/Messages/Interactions/TSMessage.m @@ -281,33 +281,16 @@ static const NSUInteger OWSMessageSchemaVersion = 3; return YES; } -#pragma mark - Update Methods - -// This method does the work for the "updateWith..." methods. Please see -// the header for a discussion of those methods. -- (void)applyChangeToSelfAndLatestMessage:(YapDatabaseReadWriteTransaction *)transaction - changeBlock:(void (^)(TSMessage *))changeBlock -{ - OWSAssert(transaction); - - changeBlock(self); - - NSString *collection = [[self class] collection]; - TSMessage *latestMessage = [transaction objectForKey:self.uniqueId inCollection:collection]; - if (latestMessage) { - changeBlock(latestMessage); - [latestMessage saveWithTransaction:transaction]; - } -} +#pragma mark - Update With... Methods - (void)updateWithExpireStartedAt:(uint64_t)expireStartedAt transaction:(YapDatabaseReadWriteTransaction *)transaction { OWSAssert(expireStartedAt > 0); - [self applyChangeToSelfAndLatestMessage:transaction - changeBlock:^(TSMessage *message) { - [message setExpireStartedAt:expireStartedAt]; - }]; + [self applyChangeToSelfAndLatestCopy:transaction + changeBlock:^(TSMessage *message) { + [message setExpireStartedAt:expireStartedAt]; + }]; } @end diff --git a/SignalServiceKit/src/Messages/Interactions/TSOutgoingMessage.h b/SignalServiceKit/src/Messages/Interactions/TSOutgoingMessage.h index abb39aa49..842bf9574 100644 --- a/SignalServiceKit/src/Messages/Interactions/TSOutgoingMessage.h +++ b/SignalServiceKit/src/Messages/Interactions/TSOutgoingMessage.h @@ -148,28 +148,8 @@ typedef NS_ENUM(NSInteger, TSGroupMetaMessage) { - (OWSSignalServiceProtosAttachmentPointer *)buildAttachmentProtoForAttachmentId:(NSString *)attachmentId filename:(nullable NSString *)filename; -// This model may be updated from many threads. We don't want to save -// our local copy (this instance) since it may be out of date. We also -// want to avoid re-saving a model that has been deleted. Therefore, we -// use these "updateWith..." methods to: -// -// a) Update a property of this instance. -// b) If a copy of this model exists in the database, load an up-to-date copy, -// and update and save that copy. -// b) If a copy of this model _DOES NOT_ exist in the database, do _NOT_ save -// this local instance. -// -// After "updateWith...": -// -// a) An copy of this model in the database will have been updated. -// b) The local property on this instance will always have been updated. -// c) Other properties on this instance may be out of date. -// -// All mutable properties of this class have been made read-only to -// prevent accidentally modifying them directly. -// -// This isn't a perfect arrangement, but in practice this will prevent -// data loss and will resolve all known issues. +#pragma mark - Update With... Methods + - (void)updateWithMessageState:(TSOutgoingMessageState)messageState; - (void)updateWithMessageState:(TSOutgoingMessageState)messageState transaction:(YapDatabaseReadWriteTransaction *)transaction; diff --git a/SignalServiceKit/src/Messages/Interactions/TSOutgoingMessage.m b/SignalServiceKit/src/Messages/Interactions/TSOutgoingMessage.m index c130ee9e2..04a0fc70e 100644 --- a/SignalServiceKit/src/Messages/Interactions/TSOutgoingMessage.m +++ b/SignalServiceKit/src/Messages/Interactions/TSOutgoingMessage.m @@ -238,35 +238,18 @@ NSString *const kTSOutgoingMessageSentRecipientAll = @"kTSOutgoingMessageSentRec return OWSInteractionType_OutgoingMessage; } -#pragma mark - Update Methods - -// This method does the work for the "updateWith..." methods. Please see -// the header for a discussion of those methods. -- (void)applyChangeToSelfAndLatestOutgoingMessage:(YapDatabaseReadWriteTransaction *)transaction - changeBlock:(void (^)(TSOutgoingMessage *))changeBlock -{ - OWSAssert(transaction); - - changeBlock(self); - - NSString *collection = [[self class] collection]; - TSOutgoingMessage *latestMessage = [transaction objectForKey:self.uniqueId inCollection:collection]; - if (latestMessage) { - changeBlock(latestMessage); - [latestMessage saveWithTransaction:transaction]; - } -} +#pragma mark - Update With... Methods - (void)updateWithSendingError:(NSError *)error { OWSAssert(error); [self.dbReadWriteConnection readWriteWithBlock:^(YapDatabaseReadWriteTransaction *transaction) { - [self applyChangeToSelfAndLatestOutgoingMessage:transaction - changeBlock:^(TSOutgoingMessage *message) { - [message setMessageState:TSOutgoingMessageStateUnsent]; - [message setMostRecentFailureText:error.localizedDescription]; - }]; + [self applyChangeToSelfAndLatestCopy:transaction + changeBlock:^(TSOutgoingMessage *message) { + [message setMessageState:TSOutgoingMessageStateUnsent]; + [message setMostRecentFailureText:error.localizedDescription]; + }]; }]; } @@ -282,19 +265,19 @@ NSString *const kTSOutgoingMessageSentRecipientAll = @"kTSOutgoingMessageSentRec { OWSAssert(transaction); - [self applyChangeToSelfAndLatestOutgoingMessage:transaction - changeBlock:^(TSOutgoingMessage *message) { - [message setMessageState:messageState]; - }]; + [self applyChangeToSelfAndLatestCopy:transaction + changeBlock:^(TSOutgoingMessage *message) { + [message setMessageState:messageState]; + }]; } - (void)updateWithHasSyncedTranscript:(BOOL)hasSyncedTranscript transaction:(YapDatabaseReadWriteTransaction *)transaction { - [self applyChangeToSelfAndLatestOutgoingMessage:transaction - changeBlock:^(TSOutgoingMessage *message) { - [message setHasSyncedTranscript:hasSyncedTranscript]; - }]; + [self applyChangeToSelfAndLatestCopy:transaction + changeBlock:^(TSOutgoingMessage *message) { + [message setHasSyncedTranscript:hasSyncedTranscript]; + }]; } - (void)updateWithCustomMessage:(NSString *)customMessage transaction:(YapDatabaseReadWriteTransaction *)transaction @@ -302,10 +285,10 @@ NSString *const kTSOutgoingMessageSentRecipientAll = @"kTSOutgoingMessageSentRec OWSAssert(customMessage); OWSAssert(transaction); - [self applyChangeToSelfAndLatestOutgoingMessage:transaction - changeBlock:^(TSOutgoingMessage *message) { - [message setCustomMessage:customMessage]; - }]; + [self applyChangeToSelfAndLatestCopy:transaction + changeBlock:^(TSOutgoingMessage *message) { + [message setCustomMessage:customMessage]; + }]; } - (void)updateWithCustomMessage:(NSString *)customMessage @@ -322,32 +305,31 @@ NSString *const kTSOutgoingMessageSentRecipientAll = @"kTSOutgoingMessageSentRec OWSAssert(recipientId.length > 0); OWSAssert(transaction); - [self applyChangeToSelfAndLatestOutgoingMessage:transaction - changeBlock:^(TSOutgoingMessage *message) { + [self applyChangeToSelfAndLatestCopy:transaction + changeBlock:^(TSOutgoingMessage *message) { - if (deliveryTimestamp) { - NSMutableDictionary *recipientDeliveryMap - = (message.recipientDeliveryMap - ? [message.recipientDeliveryMap mutableCopy] - : [NSMutableDictionary new]); - recipientDeliveryMap[recipientId] = deliveryTimestamp; - message.recipientDeliveryMap = [recipientDeliveryMap copy]; - } + if (deliveryTimestamp) { + NSMutableDictionary *recipientDeliveryMap + = (message.recipientDeliveryMap ? [message.recipientDeliveryMap mutableCopy] + : [NSMutableDictionary new]); + recipientDeliveryMap[recipientId] = deliveryTimestamp; + message.recipientDeliveryMap = [recipientDeliveryMap copy]; + } - [message setWasDelivered:YES]; - }]; + [message setWasDelivered:YES]; + }]; } - (void)updateWithWasSentFromLinkedDeviceWithTransaction:(YapDatabaseReadWriteTransaction *)transaction { OWSAssert(transaction); - [self applyChangeToSelfAndLatestOutgoingMessage:transaction - changeBlock:^(TSOutgoingMessage *message) { - [message setMessageState:TSOutgoingMessageStateSentToService]; - [message setWasDelivered:YES]; - [message setIsFromLinkedDevice:YES]; - }]; + [self applyChangeToSelfAndLatestCopy:transaction + changeBlock:^(TSOutgoingMessage *message) { + [message setMessageState:TSOutgoingMessageStateSentToService]; + [message setWasDelivered:YES]; + [message setIsFromLinkedDevice:YES]; + }]; } - (void)updateWithSingleGroupRecipient:(NSString *)singleGroupRecipient @@ -356,10 +338,10 @@ NSString *const kTSOutgoingMessageSentRecipientAll = @"kTSOutgoingMessageSentRec OWSAssert(transaction); OWSAssert(singleGroupRecipient.length > 0); - [self applyChangeToSelfAndLatestOutgoingMessage:transaction - changeBlock:^(TSOutgoingMessage *message) { - [message setSingleGroupRecipient:singleGroupRecipient]; - }]; + [self applyChangeToSelfAndLatestCopy:transaction + changeBlock:^(TSOutgoingMessage *message) { + [message setSingleGroupRecipient:singleGroupRecipient]; + }]; } #pragma mark - Sent Recipients @@ -411,10 +393,10 @@ NSString *const kTSOutgoingMessageSentRecipientAll = @"kTSOutgoingMessageSentRec - (void)updateWithSentRecipient:(NSString *)contactId transaction:(YapDatabaseReadWriteTransaction *)transaction { OWSAssert(transaction); - [self applyChangeToSelfAndLatestOutgoingMessage:transaction - changeBlock:^(TSOutgoingMessage *message) { - [message addSentRecipient:contactId]; - }]; + [self applyChangeToSelfAndLatestCopy:transaction + changeBlock:^(TSOutgoingMessage *message) { + [message addSentRecipient:contactId]; + }]; } - (void)updateWithReadRecipientId:(NSString *)recipientId @@ -424,14 +406,14 @@ NSString *const kTSOutgoingMessageSentRecipientAll = @"kTSOutgoingMessageSentRec OWSAssert(recipientId.length > 0); OWSAssert(transaction); - [self applyChangeToSelfAndLatestOutgoingMessage:transaction - changeBlock:^(TSOutgoingMessage *message) { - NSMutableDictionary *recipientReadMap - = (message.recipientReadMap ? [message.recipientReadMap mutableCopy] - : [NSMutableDictionary new]); - recipientReadMap[recipientId] = @(readTimestamp); - message.recipientReadMap = [recipientReadMap copy]; - }]; + [self applyChangeToSelfAndLatestCopy:transaction + changeBlock:^(TSOutgoingMessage *message) { + NSMutableDictionary *recipientReadMap + = (message.recipientReadMap ? [message.recipientReadMap mutableCopy] + : [NSMutableDictionary new]); + recipientReadMap[recipientId] = @(readTimestamp); + message.recipientReadMap = [recipientReadMap copy]; + }]; } - (nullable NSNumber *)firstRecipientReadTimestamp diff --git a/SignalServiceKit/src/Messages/OWSBatchMessageProcessor.m b/SignalServiceKit/src/Messages/OWSBatchMessageProcessor.m index ccef73244..f894f83d1 100644 --- a/SignalServiceKit/src/Messages/OWSBatchMessageProcessor.m +++ b/SignalServiceKit/src/Messages/OWSBatchMessageProcessor.m @@ -30,7 +30,7 @@ NS_ASSUME_NONNULL_BEGIN - (instancetype)initWithEnvelopeData:(NSData *)envelopeData plaintextData:(NSData *_Nullable)plaintextData NS_DESIGNATED_INITIALIZER; - (nullable instancetype)initWithCoder:(NSCoder *)coder NS_DESIGNATED_INITIALIZER; -- (instancetype)initWithUniqueId:(NSString *)uniqueId NS_UNAVAILABLE; +- (instancetype)initWithUniqueId:(NSString *_Nullable)uniqueId NS_UNAVAILABLE; - (OWSSignalServiceProtosEnvelope *)envelopeProto; @end diff --git a/SignalServiceKit/src/Messages/OWSMessageReceiver.m b/SignalServiceKit/src/Messages/OWSMessageReceiver.m index 33197ef31..6831f7da3 100644 --- a/SignalServiceKit/src/Messages/OWSMessageReceiver.m +++ b/SignalServiceKit/src/Messages/OWSMessageReceiver.m @@ -25,7 +25,7 @@ NS_ASSUME_NONNULL_BEGIN - (instancetype)initWithEnvelope:(OWSSignalServiceProtosEnvelope *)envelope NS_DESIGNATED_INITIALIZER; - (nullable instancetype)initWithCoder:(NSCoder *)coder NS_DESIGNATED_INITIALIZER; -- (instancetype)initWithUniqueId:(NSString *)uniqueId NS_UNAVAILABLE; +- (instancetype)initWithUniqueId:(NSString *_Nullable)uniqueId NS_UNAVAILABLE; - (OWSSignalServiceProtosEnvelope *)envelopeProto; @end diff --git a/SignalServiceKit/src/Security/OWSRecipientIdentity.h b/SignalServiceKit/src/Security/OWSRecipientIdentity.h index 7b5c82608..667f2f9e8 100644 --- a/SignalServiceKit/src/Security/OWSRecipientIdentity.h +++ b/SignalServiceKit/src/Security/OWSRecipientIdentity.h @@ -31,7 +31,7 @@ OWSSignalServiceProtosVerifiedState OWSVerificationStateToProtoState(OWSVerifica #pragma mark - Initializers -- (instancetype)initWithUniqueId:(NSString *)uniqueId NS_UNAVAILABLE; +- (instancetype)initWithUniqueId:(NSString *_Nullable)uniqueId NS_UNAVAILABLE; - (instancetype)initWithCoder:(NSCoder *)coder NS_DESIGNATED_INITIALIZER; diff --git a/SignalServiceKit/src/Storage/TSYapDatabaseObject.h b/SignalServiceKit/src/Storage/TSYapDatabaseObject.h index cb68f9496..1e47a8ca7 100644 --- a/SignalServiceKit/src/Storage/TSYapDatabaseObject.h +++ b/SignalServiceKit/src/Storage/TSYapDatabaseObject.h @@ -4,6 +4,8 @@ #import +NS_ASSUME_NONNULL_BEGIN + @class TSStorageManager; @class YapDatabaseConnection; @class YapDatabaseReadTransaction; @@ -18,8 +20,8 @@ * * @return Initialized object */ -- (instancetype)initWithUniqueId:(NSString *)uniqueId NS_DESIGNATED_INITIALIZER; -- (instancetype)initWithCoder:(NSCoder *)coder NS_DESIGNATED_INITIALIZER; +- (instancetype)initWithUniqueId:(NSString *_Nullable)uniqueId NS_DESIGNATED_INITIALIZER; +- (nullable instancetype)initWithCoder:(NSCoder *)coder NS_DESIGNATED_INITIALIZER; /** * Returns the collection to which the object belongs. @@ -77,8 +79,11 @@ * * @return Instance of the object or nil if non-existent */ -+ (instancetype)fetchObjectWithUniqueID:(NSString *)uniqueID transaction:(YapDatabaseReadTransaction *)transaction NS_SWIFT_NAME(fetch(uniqueId:transaction:)); -+ (instancetype)fetchObjectWithUniqueID:(NSString *)uniqueID NS_SWIFT_NAME(fetch(uniqueId:)); ++ (nullable instancetype)fetchObjectWithUniqueID:(NSString *)uniqueID + transaction:(YapDatabaseReadTransaction *)transaction + NS_SWIFT_NAME(fetch(uniqueId:transaction +:)); ++ (nullable instancetype)fetchObjectWithUniqueID:(NSString *)uniqueID NS_SWIFT_NAME(fetch(uniqueId:)); /** * Saves the object with the shared readWrite connection. @@ -113,9 +118,40 @@ /** * The unique identifier of the stored object */ -@property (nonatomic) NSString *uniqueId; +@property (nonatomic, nullable) NSString *uniqueId; - (void)removeWithTransaction:(YapDatabaseReadWriteTransaction *)transaction; - (void)remove; +#pragma mark Update With... + +// This method is used by "updateWith..." methods. +// +// This model may be updated from many threads. We don't want to save +// our local copy (this instance) since it may be out of date. We also +// want to avoid re-saving a model that has been deleted. Therefore, we +// use "updateWith..." methods to: +// +// a) Update a property of this instance. +// b) If a copy of this model exists in the database, load an up-to-date copy, +// and update and save that copy. +// b) If a copy of this model _DOES NOT_ exist in the database, do _NOT_ save +// this local instance. +// +// After "updateWith...": +// +// a) Any copy of this model in the database will have been updated. +// b) The local property on this instance will always have been updated. +// c) Other properties on this instance may be out of date. +// +// All mutable properties of this class have been made read-only to +// prevent accidentally modifying them directly. +// +// This isn't a perfect arrangement, but in practice this will prevent +// data loss and will resolve all known issues. +- (void)applyChangeToSelfAndLatestCopy:(YapDatabaseReadWriteTransaction *)transaction + changeBlock:(void (^)(id))changeBlock; + @end + +NS_ASSUME_NONNULL_END diff --git a/SignalServiceKit/src/Storage/TSYapDatabaseObject.m b/SignalServiceKit/src/Storage/TSYapDatabaseObject.m index ec392ccde..22c637e08 100644 --- a/SignalServiceKit/src/Storage/TSYapDatabaseObject.m +++ b/SignalServiceKit/src/Storage/TSYapDatabaseObject.m @@ -6,6 +6,8 @@ #import "TSStorageManager.h" #import +NS_ASSUME_NONNULL_BEGIN + @implementation TSYapDatabaseObject - (instancetype)init @@ -13,7 +15,7 @@ return [self initWithUniqueId:[[NSUUID UUID] UUIDString]]; } -- (instancetype)initWithUniqueId:(NSString *)aUniqueId +- (instancetype)initWithUniqueId:(NSString *_Nullable)aUniqueId { self = [super init]; if (!self) { @@ -25,6 +27,11 @@ return self; } +- (nullable instancetype)initWithCoder:(NSCoder *)coder +{ + return [super initWithCoder:coder]; +} + - (void)saveWithTransaction:(YapDatabaseReadWriteTransaction *)transaction { [transaction setObject:self forKey:self.uniqueId inCollection:[[self class] collection]]; @@ -176,18 +183,40 @@ }]; } -+ (instancetype)fetchObjectWithUniqueID:(NSString *)uniqueID transaction:(YapDatabaseReadTransaction *)transaction ++ (nullable instancetype)fetchObjectWithUniqueID:(NSString *)uniqueID + transaction:(YapDatabaseReadTransaction *)transaction { return [transaction objectForKey:uniqueID inCollection:[self collection]]; } -+ (instancetype)fetchObjectWithUniqueID:(NSString *)uniqueID ++ (nullable instancetype)fetchObjectWithUniqueID:(NSString *)uniqueID { - __block id object; + __block id _Nullable object = nil; [[self dbReadConnection] readWithBlock:^(YapDatabaseReadTransaction *transaction) { object = [transaction objectForKey:uniqueID inCollection:[self collection]]; }]; return object; } +#pragma mark Update With... + +// This method does the work for the "updateWith..." methods. Please see +// the header for a discussion of those methods. +- (void)applyChangeToSelfAndLatestCopy:(YapDatabaseReadWriteTransaction *)transaction + changeBlock:(void (^)(id))changeBlock +{ + OWSAssert(transaction); + + changeBlock(self); + + NSString *collection = [[self class] collection]; + id latestInstance = [transaction objectForKey:self.uniqueId inCollection:collection]; + if (latestInstance) { + changeBlock(latestInstance); + [latestInstance saveWithTransaction:transaction]; + } +} + @end + +NS_ASSUME_NONNULL_END