From a92158ef16d775f3da2aad3bb30a8d0d08a421e2 Mon Sep 17 00:00:00 2001 From: Michael Kirk Date: Tue, 21 Feb 2017 19:21:09 -0500 Subject: [PATCH] CR: fix register `async` where specified * fix thread test * add IncomingMessageFinder test * use constants * clearer comments // FREEBIE --- .../TSKitiOSTestApp.xcodeproj/project.pbxproj | 4 + src/Devices/OWSDevice.h | 2 + src/Devices/OWSDevice.m | 2 +- src/Messages/Interactions/TSIncomingMessage.h | 4 +- src/Messages/TSMessagesManager.m | 3 + src/Storage/OWSIncomingMessageFinder.m | 9 +- tests/Contacts/TSThreadTest.m | 26 ++--- tests/Messages/OWSIncomingMessageFinderTest.m | 105 ++++++++++++++++++ tests/Storage/OWSOrphanedDataCleanerTest.m | 10 +- 9 files changed, 143 insertions(+), 22 deletions(-) create mode 100644 tests/Messages/OWSIncomingMessageFinderTest.m diff --git a/Example/TSKitiOSTestApp/TSKitiOSTestApp.xcodeproj/project.pbxproj b/Example/TSKitiOSTestApp/TSKitiOSTestApp.xcodeproj/project.pbxproj index 3ac6a8eab..46206806b 100644 --- a/Example/TSKitiOSTestApp/TSKitiOSTestApp.xcodeproj/project.pbxproj +++ b/Example/TSKitiOSTestApp/TSKitiOSTestApp.xcodeproj/project.pbxproj @@ -37,6 +37,7 @@ 45B840211D988DA100F9E938 /* OWSReadReceiptTest.m in Sources */ = {isa = PBXBuildFile; fileRef = 45B840201D988DA100F9E938 /* OWSReadReceiptTest.m */; }; 45C6A09A1D2F029B007D8AC0 /* TSMessageTest.m in Sources */ = {isa = PBXBuildFile; fileRef = 45C6A0991D2F029B007D8AC0 /* TSMessageTest.m */; }; 45D7243F1D67899F00E0CA54 /* OWSDeviceProvisionerTest.m in Sources */ = {isa = PBXBuildFile; fileRef = 45D7243E1D67899F00E0CA54 /* OWSDeviceProvisionerTest.m */; }; + 45E741B61E5D14E800735842 /* OWSIncomingMessageFinderTest.m in Sources */ = {isa = PBXBuildFile; fileRef = 45E741B51E5D14E800735842 /* OWSIncomingMessageFinderTest.m */; }; 51520592F83F2440F2DE4D67 /* libPods-TSKitiOSTestApp.a in Frameworks */ = {isa = PBXBuildFile; fileRef = B8362AB8E280E0F64352F08A /* libPods-TSKitiOSTestApp.a */; }; 6323E1F7730289398452E5C5 /* OWSFingerprintTest.m in Sources */ = {isa = PBXBuildFile; fileRef = 6323E02A33682A8838FE3F27 /* OWSFingerprintTest.m */; }; 6323E339D5B8F4CB77EB3ED4 /* SignalRecipientTest.m in Sources */ = {isa = PBXBuildFile; fileRef = 6323E3E540CF763D71DACB59 /* SignalRecipientTest.m */; }; @@ -99,6 +100,7 @@ 45B840201D988DA100F9E938 /* OWSReadReceiptTest.m */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.c.objc; name = OWSReadReceiptTest.m; path = ../../../tests/Devices/OWSReadReceiptTest.m; sourceTree = ""; }; 45C6A0991D2F029B007D8AC0 /* TSMessageTest.m */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.c.objc; name = TSMessageTest.m; path = ../../../tests/Messages/Interactions/TSMessageTest.m; sourceTree = ""; }; 45D7243E1D67899F00E0CA54 /* OWSDeviceProvisionerTest.m */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.c.objc; name = OWSDeviceProvisionerTest.m; path = ../../../tests/Devices/OWSDeviceProvisionerTest.m; sourceTree = ""; }; + 45E741B51E5D14E800735842 /* OWSIncomingMessageFinderTest.m */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.c.objc; name = OWSIncomingMessageFinderTest.m; path = ../../../tests/Messages/OWSIncomingMessageFinderTest.m; sourceTree = ""; }; 6323E02A33682A8838FE3F27 /* OWSFingerprintTest.m */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.c.objc; name = OWSFingerprintTest.m; path = ../../../tests/Security/OWSFingerprintTest.m; sourceTree = ""; }; 6323E3E540CF763D71DACB59 /* SignalRecipientTest.m */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.c.objc; name = SignalRecipientTest.m; path = ../../tests/Contacts/SignalRecipientTest.m; sourceTree = ""; }; B6273DD11C13A2E500738558 /* TSKitiOSTestApp.app */ = {isa = PBXFileReference; explicitFileType = wrapper.application; includeInIndex = 0; path = TSKitiOSTestApp.app; sourceTree = BUILT_PRODUCTS_DIR; }; @@ -237,6 +239,7 @@ 45046FDF1D95A6130015EFF2 /* TSMessagesManagerTest.m */, 454021EC1D960ABF00F2126D /* OWSDisappearingMessageFinderTest.m */, 453E1FCE1DA8313100DDD7B7 /* OWSMessageSenderTest.m */, + 45E741B51E5D14E800735842 /* OWSIncomingMessageFinderTest.m */, ); name = Messages; sourceTree = ""; @@ -561,6 +564,7 @@ 45D7243F1D67899F00E0CA54 /* OWSDeviceProvisionerTest.m in Sources */, 4516E3E81DD153CC00DC4206 /* TSGroupThreadTest.m in Sources */, 45458B791CC342B600A02153 /* TSStoragePreKeyStoreTests.m in Sources */, + 45E741B61E5D14E800735842 /* OWSIncomingMessageFinderTest.m in Sources */, 452EE6D51D4AC43300E934BA /* OWSOrphanedDataCleanerTest.m in Sources */, 450E3C9A1D96DD2600BF4EB6 /* OWSDisappearingMessagesJobTest.m in Sources */, 452EE6CF1D4A754C00E934BA /* TSThreadTest.m in Sources */, diff --git a/src/Devices/OWSDevice.h b/src/Devices/OWSDevice.h index 54209673b..5caf78e4b 100644 --- a/src/Devices/OWSDevice.h +++ b/src/Devices/OWSDevice.h @@ -7,6 +7,8 @@ NS_ASSUME_NONNULL_BEGIN +extern uint32_t const OWSDevicePrimaryDeviceId; + @interface OWSDevice : TSYapDatabaseObject @property (nonatomic, readonly) NSInteger deviceId; diff --git a/src/Devices/OWSDevice.m b/src/Devices/OWSDevice.m index 14842667e..3f3e9ba6b 100644 --- a/src/Devices/OWSDevice.m +++ b/src/Devices/OWSDevice.m @@ -12,7 +12,7 @@ NS_ASSUME_NONNULL_BEGIN static MTLValueTransformer *_millisecondTimestampToDateTransformer; -static uint32_t const OWSDevicePrimaryDeviceId = 1; +uint32_t const OWSDevicePrimaryDeviceId = 1; @interface OWSDevice () diff --git a/src/Messages/Interactions/TSIncomingMessage.h b/src/Messages/Interactions/TSIncomingMessage.h index d9062f199..cb1b43d83 100644 --- a/src/Messages/Interactions/TSIncomingMessage.h +++ b/src/Messages/Interactions/TSIncomingMessage.h @@ -69,7 +69,7 @@ extern NSString *const TSIncomingMessageWasReadOnThisDeviceNotification; /** * For sake of a smaller API, and simplifying assumptions elsewhere, you must specify an author id for *all* incoming - * messages, even though we technically could infer the author id for a contact threads. + * messages, even though we technically could infer the author id for a contact thread. */ - (instancetype)initWithTimestamp:(uint64_t)timestamp NS_UNAVAILABLE; - (instancetype)initWithTimestamp:(uint64_t)timestamp inThread:(nullable TSThread *)thread NS_UNAVAILABLE; @@ -104,6 +104,8 @@ extern NSString *const TSIncomingMessageWasReadOnThisDeviceNotification; + (nullable instancetype)findMessageWithAuthorId:(NSString *)authorId timestamp:(uint64_t)timestamp; @property (nonatomic, readonly) NSString *authorId; + +// This will be 0 for messages created before we were tracking sourceDeviceId @property (nonatomic, readonly) UInt32 sourceDeviceId; @property (nonatomic, readonly, getter=wasRead) BOOL read; diff --git a/src/Messages/TSMessagesManager.m b/src/Messages/TSMessagesManager.m index 7eddd0124..3daad6d44 100644 --- a/src/Messages/TSMessagesManager.m +++ b/src/Messages/TSMessagesManager.m @@ -284,6 +284,9 @@ NS_ASSUME_NONNULL_BEGIN - (void)handleEnvelope:(OWSSignalServiceProtosEnvelope *)envelope plaintextData:(NSData *)plaintextData { OWSAssert([NSThread isMainThread]); + OWSAssert(envelope.hasTimestamp && envelope.timestamp > 0); + OWSAssert(envelope.hasSource && envelope.source.length > 0); + OWSAssert(envelope.hasSourceDevice && envelope.sourceDevice > 0); BOOL duplicateEnvelope = [self.incomingMessageFinder existsMessageWithTimestamp:envelope.timestamp sourceId:envelope.source diff --git a/src/Storage/OWSIncomingMessageFinder.m b/src/Storage/OWSIncomingMessageFinder.m index 3c6161474..af28990de 100644 --- a/src/Storage/OWSIncomingMessageFinder.m +++ b/src/Storage/OWSIncomingMessageFinder.m @@ -20,7 +20,6 @@ NSString *const OWSIncomingMessageFinderColumnSourceDeviceId = @"OWSIncomingMess @property (nonatomic, readonly) YapDatabase *database; @property (nonatomic, readonly) YapDatabaseConnection *dbConnection; -@property (nonatomic, readonly) BOOL isExtensionRegistered; @end @@ -93,12 +92,16 @@ NSString *const OWSIncomingMessageFinderColumnSourceDeviceId = @"OWSIncomingMess - (void)asyncRegisterExtension { DDLogInfo(@"%@ registering async.", self.tag); - [self.database registerExtension:self.indexExtension withName:OWSIncomingMessageFinderExtensionName]; + [self.database asyncRegisterExtension:self.indexExtension + withName:OWSIncomingMessageFinderExtensionName + completionBlock:^(BOOL ready) { + DDLogInfo(@"%@ finished registering async.", self.tag); + }]; } +// We should not normally hit this, as we should have prefer registering async, but it is useful for testing. - (void)registerExtension { - OWSAssert(NO); DDLogError(@"%@ registering SYNC. We should prefer async when possible.", self.tag); [self.database registerExtension:self.indexExtension withName:OWSIncomingMessageFinderExtensionName]; } diff --git a/tests/Contacts/TSThreadTest.m b/tests/Contacts/TSThreadTest.m index 117e30ba6..4f21d770d 100644 --- a/tests/Contacts/TSThreadTest.m +++ b/tests/Contacts/TSThreadTest.m @@ -2,6 +2,7 @@ // Copyright (c) 2017 Open Whisper Systems. All rights reserved. // +#import "OWSDevice.h" #import "TSAttachmentStream.h" #import "TSContactThread.h" #import "TSIncomingMessage.h" @@ -40,7 +41,7 @@ TSIncomingMessage *incomingMessage = [[TSIncomingMessage alloc] initWithTimestamp:10000 inThread:thread authorId:@"fake-author-id" - sourceDeviceId:1 + sourceDeviceId:OWSDevicePrimaryDeviceId messageBody:@"Incoming message body"]; [incomingMessage save]; @@ -73,13 +74,13 @@ BOOL incomingFileWasCreated = [[NSFileManager defaultManager] fileExistsAtPath:[incomingAttachment filePath]]; XCTAssert(incomingFileWasCreated); - TSIncomingMessage *incomingMessage = - [[TSIncomingMessage alloc] initWithTimestamp:10000 - inThread:thread - authorId:@"fake-author-id" - sourceDeviceId:1 - messageBody:@"incoming message body" - attachmentIds:[NSMutableArray arrayWithObject:incomingAttachment.uniqueId]]; + TSIncomingMessage *incomingMessage = [[TSIncomingMessage alloc] initWithTimestamp:10000 + inThread:thread + authorId:@"fake-author-id" + sourceDeviceId:OWSDevicePrimaryDeviceId + messageBody:@"incoming message body" + attachmentIds:@[ incomingAttachment.uniqueId ] + expiresInSeconds:0]; [incomingMessage save]; TSAttachmentStream *outgoingAttachment = [[TSAttachmentStream alloc] initWithContentType:@"image/jpeg"]; @@ -90,11 +91,10 @@ BOOL outgoingFileWasCreated = [[NSFileManager defaultManager] fileExistsAtPath:[outgoingAttachment filePath]]; XCTAssert(outgoingFileWasCreated); - TSOutgoingMessage *outgoingMessage = - [[TSOutgoingMessage alloc] initWithTimestamp:10000 - inThread:thread - messageBody:@"outgoing message body" - attachmentIds:[NSMutableArray arrayWithObject:outgoingAttachment.uniqueId]]; + TSOutgoingMessage *outgoingMessage = [[TSOutgoingMessage alloc] initWithTimestamp:10000 + inThread:thread + messageBody:@"outgoing message body" + attachmentIds:@[ outgoingAttachment.uniqueId ]]; [outgoingMessage save]; // Sanity check diff --git a/tests/Messages/OWSIncomingMessageFinderTest.m b/tests/Messages/OWSIncomingMessageFinderTest.m new file mode 100644 index 000000000..ba859ad95 --- /dev/null +++ b/tests/Messages/OWSIncomingMessageFinderTest.m @@ -0,0 +1,105 @@ +// +// Copyright (c) 2017 Open Whisper Systems. All rights reserved. +// + +#import "OWSDevice.h" +#import "OWSIncomingMessageFinder.h" +#import "TSContactThread.h" +#import "TSIncomingMessage.h" +#import + +NS_ASSUME_NONNULL_BEGIN + +@interface OWSIncomingMessageFinder (Testing) + +- (void)registerExtension; + +@end + +@interface OWSIncomingMessageFinderTest : XCTestCase + +@property (nonatomic) NSString *sourceId; +@property (nonatomic) TSThread *thread; +@property (nonatomic) OWSIncomingMessageFinder *finder; + +@end + +@implementation OWSIncomingMessageFinderTest + +- (void)setUp +{ + [super setUp]; + self.sourceId = @"some-source-id"; + self.thread = [TSContactThread getOrCreateThreadWithContactId:self.sourceId]; + self.finder = [OWSIncomingMessageFinder new]; + [self.finder registerExtension]; +} + +- (void)tearDown +{ + // Put teardown code here. This method is called after the invocation of each test method in the class. + [super tearDown]; +} + +- (void)testExistingMessages +{ + + uint64_t timestamp = 1234; + BOOL result = [self.finder existsMessageWithTimestamp:timestamp + sourceId:self.sourceId + sourceDeviceId:OWSDevicePrimaryDeviceId]; + + // Sanity check. + XCTAssertFalse(result); + + // Different timestamp + [[[TSIncomingMessage alloc] initWithTimestamp:timestamp + 1 + inThread:self.thread + authorId:self.sourceId + sourceDeviceId:OWSDevicePrimaryDeviceId + messageBody:@"foo"] save]; + result = [self.finder existsMessageWithTimestamp:timestamp + sourceId:self.sourceId + sourceDeviceId:OWSDevicePrimaryDeviceId]; + XCTAssertFalse(result); + + // Different authorId + [[[TSIncomingMessage alloc] initWithTimestamp:timestamp + inThread:self.thread + authorId:@"some-other-author-id" + sourceDeviceId:OWSDevicePrimaryDeviceId + messageBody:@"foo"] save]; + + result = [self.finder existsMessageWithTimestamp:timestamp + sourceId:self.sourceId + sourceDeviceId:OWSDevicePrimaryDeviceId]; + XCTAssertFalse(result); + + // Different device + [[[TSIncomingMessage alloc] initWithTimestamp:timestamp + inThread:self.thread + authorId:self.sourceId + sourceDeviceId:OWSDevicePrimaryDeviceId + 1 + messageBody:@"foo"] save]; + + result = [self.finder existsMessageWithTimestamp:timestamp + sourceId:self.sourceId + sourceDeviceId:OWSDevicePrimaryDeviceId]; + XCTAssertFalse(result); + + // The real deal... + [[[TSIncomingMessage alloc] initWithTimestamp:timestamp + inThread:self.thread + authorId:self.sourceId + sourceDeviceId:OWSDevicePrimaryDeviceId + messageBody:@"foo"] save]; + + result = [self.finder existsMessageWithTimestamp:timestamp + sourceId:self.sourceId + sourceDeviceId:OWSDevicePrimaryDeviceId]; + XCTAssertTrue(result); +} + +@end + +NS_ASSUME_NONNULL_END diff --git a/tests/Storage/OWSOrphanedDataCleanerTest.m b/tests/Storage/OWSOrphanedDataCleanerTest.m index 85c674fcc..432d9e1ea 100644 --- a/tests/Storage/OWSOrphanedDataCleanerTest.m +++ b/tests/Storage/OWSOrphanedDataCleanerTest.m @@ -2,6 +2,7 @@ // Copyright (c) 2017 Open Whisper Systems. All rights reserved. // +#import "OWSDevice.h" #import "OWSOrphanedDataCleaner.h" #import "TSAttachmentStream.h" #import "TSContactThread.h" @@ -47,7 +48,7 @@ TSIncomingMessage *incomingMessage = [[TSIncomingMessage alloc] initWithTimestamp:1 inThread:unsavedThread authorId:@"fake-author-id" - sourceDeviceId:1 + sourceDeviceId:OWSDevicePrimaryDeviceId messageBody:@"footch"]; [incomingMessage save]; XCTAssertEqual(1, [TSIncomingMessage numberOfKeysInCollection]); @@ -64,7 +65,7 @@ TSIncomingMessage *incomingMessage = [[TSIncomingMessage alloc] initWithTimestamp:1 inThread:savedThread authorId:@"fake-author-id" - sourceDeviceId:1 + sourceDeviceId:OWSDevicePrimaryDeviceId messageBody:@"footch"]; [incomingMessage save]; XCTAssertEqual(1, [TSIncomingMessage numberOfKeysInCollection]); @@ -103,9 +104,10 @@ TSIncomingMessage *incomingMessage = [[TSIncomingMessage alloc] initWithTimestamp:1 inThread:savedThread authorId:@"fake-author-id" - sourceDeviceId:1 + sourceDeviceId:OWSDevicePrimaryDeviceId messageBody:@"footch" - attachmentIds:@[ attachmentStream.uniqueId ]]; + attachmentIds:@[ attachmentStream.uniqueId ] + expiresInSeconds:0]; [incomingMessage save]; NSString *attachmentFilePath = [attachmentStream filePath];