From 35972d176aae3d86bf9f5b25b154f42e21c8a00a Mon Sep 17 00:00:00 2001 From: Morgan Pretty Date: Wed, 12 Feb 2025 17:27:24 +1100 Subject: [PATCH] Fixed broken message sending for TestFlight build MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit • Fixed broken unit tests • Fixed some incorrect logic for validating a message --- Session.xcodeproj/project.pbxproj | 4 ++-- .../ClosedGroupControlMessage.swift | 6 +++--- .../DataExtractionNotification.swift | 4 ++-- .../Control Messages/ReadReceipt.swift | 4 ++-- .../Control Messages/TypingIndicator.swift | 4 ++-- .../Control Messages/UnsendRequest.swift | 4 ++-- .../Messages/LibSessionMessage.swift | 2 +- SessionMessagingKit/Messages/Message.swift | 20 +++++++++++++++---- .../VisibleMessage+Attachment.swift | 2 +- .../VisibleMessage+LinkPreview.swift | 2 +- .../VisibleMessage+Quote.swift | 2 +- .../VisibleMessage+Reaction.swift | 2 +- .../Visible Messages/VisibleMessage.swift | 4 ++-- .../Sending & Receiving/MessageReceiver.swift | 2 +- .../Sending & Receiving/MessageSender.swift | 2 +- .../MessageSenderSpec.swift | 5 +++++ .../_TestUtilities/MockPoller.swift | 2 ++ .../_TestUtilities/MockSwarmPoller.swift | 2 ++ 18 files changed, 47 insertions(+), 26 deletions(-) diff --git a/Session.xcodeproj/project.pbxproj b/Session.xcodeproj/project.pbxproj index 26413c08a..271648df5 100644 --- a/Session.xcodeproj/project.pbxproj +++ b/Session.xcodeproj/project.pbxproj @@ -7935,7 +7935,7 @@ CLANG_WARN__ARC_BRIDGE_CAST_NONARC = YES; CLANG_WARN__DUPLICATE_METHOD_MATCH = YES; CODE_SIGN_IDENTITY = "iPhone Developer"; - CURRENT_PROJECT_VERSION = 539; + CURRENT_PROJECT_VERSION = 540; ENABLE_BITCODE = NO; ENABLE_STRICT_OBJC_MSGSEND = YES; ENABLE_TESTABILITY = YES; @@ -8011,7 +8011,7 @@ CLANG_WARN__ARC_BRIDGE_CAST_NONARC = YES; CLANG_WARN__DUPLICATE_METHOD_MATCH = YES; CODE_SIGN_IDENTITY = "iPhone Distribution"; - CURRENT_PROJECT_VERSION = 539; + CURRENT_PROJECT_VERSION = 540; ENABLE_BITCODE = NO; ENABLE_MODULE_VERIFIER = YES; ENABLE_STRICT_OBJC_MSGSEND = YES; diff --git a/SessionMessagingKit/Messages/Control Messages/ClosedGroupControlMessage.swift b/SessionMessagingKit/Messages/Control Messages/ClosedGroupControlMessage.swift index c2395e9ee..40d273528 100644 --- a/SessionMessagingKit/Messages/Control Messages/ClosedGroupControlMessage.swift +++ b/SessionMessagingKit/Messages/Control Messages/ClosedGroupControlMessage.swift @@ -157,7 +157,7 @@ public final class ClosedGroupControlMessage: ControlMessage { public var publicKey: String? public var encryptedKeyPair: Data? - public func isValid(using dependencies: Dependencies) -> Bool { publicKey != nil && encryptedKeyPair != nil } + public func isValid(isSending: Bool) -> Bool { publicKey != nil && encryptedKeyPair != nil } public init(publicKey: String, encryptedKeyPair: Data) { self.publicKey = publicKey @@ -192,8 +192,8 @@ public final class ClosedGroupControlMessage: ControlMessage { // MARK: - Validation - public override func isValid(using dependencies: Dependencies) -> Bool { - guard super.isValid(using: dependencies), let kind = kind else { return false } + public override func isValid(isSending: Bool) -> Bool { + guard super.isValid(isSending: isSending), let kind = kind else { return false } switch kind { case .new(let publicKey, let name, let encryptionKeyPair, let members, let admins, _): diff --git a/SessionMessagingKit/Messages/Control Messages/DataExtractionNotification.swift b/SessionMessagingKit/Messages/Control Messages/DataExtractionNotification.swift index 3d8c07b04..08e18e3fc 100644 --- a/SessionMessagingKit/Messages/Control Messages/DataExtractionNotification.swift +++ b/SessionMessagingKit/Messages/Control Messages/DataExtractionNotification.swift @@ -42,8 +42,8 @@ public final class DataExtractionNotification: ControlMessage { // MARK: - Validation - public override func isValid(using dependencies: Dependencies) -> Bool { - guard super.isValid(using: dependencies), let kind = kind else { return false } + public override func isValid(isSending: Bool) -> Bool { + guard super.isValid(isSending: isSending), let kind = kind else { return false } switch kind { case .screenshot: return true diff --git a/SessionMessagingKit/Messages/Control Messages/ReadReceipt.swift b/SessionMessagingKit/Messages/Control Messages/ReadReceipt.swift index 79735010a..978c424e2 100644 --- a/SessionMessagingKit/Messages/Control Messages/ReadReceipt.swift +++ b/SessionMessagingKit/Messages/Control Messages/ReadReceipt.swift @@ -21,8 +21,8 @@ public final class ReadReceipt: ControlMessage { // MARK: - Validation - public override func isValid(using dependencies: Dependencies) -> Bool { - guard super.isValid(using: dependencies) else { return false } + public override func isValid(isSending: Bool) -> Bool { + guard super.isValid(isSending: isSending) else { return false } if let timestamps = timestamps, !timestamps.isEmpty { return true } return false } diff --git a/SessionMessagingKit/Messages/Control Messages/TypingIndicator.swift b/SessionMessagingKit/Messages/Control Messages/TypingIndicator.swift index 25480ef9f..15d3da97d 100644 --- a/SessionMessagingKit/Messages/Control Messages/TypingIndicator.swift +++ b/SessionMessagingKit/Messages/Control Messages/TypingIndicator.swift @@ -43,8 +43,8 @@ public final class TypingIndicator: ControlMessage { // MARK: - Validation - public override func isValid(using dependencies: Dependencies) -> Bool { - guard super.isValid(using: dependencies) else { return false } + public override func isValid(isSending: Bool) -> Bool { + guard super.isValid(isSending: isSending) else { return false } return kind != nil } diff --git a/SessionMessagingKit/Messages/Control Messages/UnsendRequest.swift b/SessionMessagingKit/Messages/Control Messages/UnsendRequest.swift index 433806869..acf4fbc97 100644 --- a/SessionMessagingKit/Messages/Control Messages/UnsendRequest.swift +++ b/SessionMessagingKit/Messages/Control Messages/UnsendRequest.swift @@ -17,8 +17,8 @@ public final class UnsendRequest: ControlMessage { // MARK: - Validation - public override func isValid(using dependencies: Dependencies) -> Bool { - guard super.isValid(using: dependencies) else { return false } + public override func isValid(isSending: Bool) -> Bool { + guard super.isValid(isSending: isSending) else { return false } return timestamp != nil && author != nil } diff --git a/SessionMessagingKit/Messages/LibSessionMessage.swift b/SessionMessagingKit/Messages/LibSessionMessage.swift index 409c71993..0ad28ca8d 100644 --- a/SessionMessagingKit/Messages/LibSessionMessage.swift +++ b/SessionMessagingKit/Messages/LibSessionMessage.swift @@ -13,7 +13,7 @@ public final class LibSessionMessage: Message, NotProtoConvertible { // MARK: - Validation - public override func isValid(using dependencies: Dependencies) -> Bool { + public override func isValid(isSending: Bool) -> Bool { return !ciphertext.isEmpty } diff --git a/SessionMessagingKit/Messages/Message.swift b/SessionMessagingKit/Messages/Message.swift index 56fd5c6e4..4374bab03 100644 --- a/SessionMessagingKit/Messages/Message.swift +++ b/SessionMessagingKit/Messages/Message.swift @@ -44,10 +44,22 @@ public class Message: Codable { // MARK: - Validation - public func isValid(using dependencies: Dependencies) -> Bool { - if let sentTimestampMs = sentTimestampMs { guard sentTimestampMs > 0 else { return false } } - if let receivedTimestampMs = receivedTimestampMs { guard receivedTimestampMs > 0 else { return false } } - return sender != nil + public func isValid(isSending: Bool) -> Bool { + guard + let sentTimestampMs: UInt64 = sentTimestampMs, + sentTimestampMs > 0, + sender != nil + else { return false } + + /// If this is an incoming message then ensure we also have a received timestamp + if !isSending { + guard + let receivedTimestampMs: UInt64 = receivedTimestampMs, + receivedTimestampMs > 0 + else { return false } + } + + return true } // MARK: - Initialization diff --git a/SessionMessagingKit/Messages/Visible Messages/VisibleMessage+Attachment.swift b/SessionMessagingKit/Messages/Visible Messages/VisibleMessage+Attachment.swift index c5484a35b..00e8327e1 100644 --- a/SessionMessagingKit/Messages/Visible Messages/VisibleMessage+Attachment.swift +++ b/SessionMessagingKit/Messages/Visible Messages/VisibleMessage+Attachment.swift @@ -21,7 +21,7 @@ public extension VisibleMessage { public var sizeInBytes: UInt? public var url: String? - public func isValid(using dependencies: Dependencies) -> Bool { + public func isValid(isSending: Bool) -> Bool { // key and digest can be nil for open group attachments contentType != nil && kind != nil && size != nil && sizeInBytes != nil && url != nil } diff --git a/SessionMessagingKit/Messages/Visible Messages/VisibleMessage+LinkPreview.swift b/SessionMessagingKit/Messages/Visible Messages/VisibleMessage+LinkPreview.swift index 184e1fae4..6dc958c40 100644 --- a/SessionMessagingKit/Messages/Visible Messages/VisibleMessage+LinkPreview.swift +++ b/SessionMessagingKit/Messages/Visible Messages/VisibleMessage+LinkPreview.swift @@ -10,7 +10,7 @@ public extension VisibleMessage { public let url: String? public let attachmentId: String? - public func isValid(using dependencies: Dependencies) -> Bool { title != nil && url != nil && attachmentId != nil } + public func isValid(isSending: Bool) -> Bool { title != nil && url != nil && attachmentId != nil } // MARK: - Initialization diff --git a/SessionMessagingKit/Messages/Visible Messages/VisibleMessage+Quote.swift b/SessionMessagingKit/Messages/Visible Messages/VisibleMessage+Quote.swift index 6321bcb9b..865717d40 100644 --- a/SessionMessagingKit/Messages/Visible Messages/VisibleMessage+Quote.swift +++ b/SessionMessagingKit/Messages/Visible Messages/VisibleMessage+Quote.swift @@ -12,7 +12,7 @@ public extension VisibleMessage { public let text: String? public let attachmentId: String? - public func isValid(using dependencies: Dependencies) -> Bool { timestamp != nil && publicKey != nil } + public func isValid(isSending: Bool) -> Bool { timestamp != nil && publicKey != nil } // MARK: - Initialization diff --git a/SessionMessagingKit/Messages/Visible Messages/VisibleMessage+Reaction.swift b/SessionMessagingKit/Messages/Visible Messages/VisibleMessage+Reaction.swift index fda5f3ca9..60c42833a 100644 --- a/SessionMessagingKit/Messages/Visible Messages/VisibleMessage+Reaction.swift +++ b/SessionMessagingKit/Messages/Visible Messages/VisibleMessage+Reaction.swift @@ -18,7 +18,7 @@ public extension VisibleMessage { /// This is the behaviour for the reaction public var kind: Kind - public func isValid(using dependencies: Dependencies) -> Bool { true } + public func isValid(isSending: Bool) -> Bool { true } // MARK: - Kind diff --git a/SessionMessagingKit/Messages/Visible Messages/VisibleMessage.swift b/SessionMessagingKit/Messages/Visible Messages/VisibleMessage.swift index c15175a12..113238b97 100644 --- a/SessionMessagingKit/Messages/Visible Messages/VisibleMessage.swift +++ b/SessionMessagingKit/Messages/Visible Messages/VisibleMessage.swift @@ -34,8 +34,8 @@ public final class VisibleMessage: Message { // MARK: - Validation - public override func isValid(using dependencies: Dependencies) -> Bool { - guard super.isValid(using: dependencies) else { return false } + public override func isValid(isSending: Bool) -> Bool { + guard super.isValid(isSending: isSending) else { return false } if !attachmentIds.isEmpty || dataMessageHasAttachments == true { return true } if openGroupInvitation != nil { return true } if reaction != nil { return true } diff --git a/SessionMessagingKit/Sending & Receiving/MessageReceiver.swift b/SessionMessagingKit/Sending & Receiving/MessageReceiver.swift index af7330162..66e087a8c 100644 --- a/SessionMessagingKit/Sending & Receiving/MessageReceiver.swift +++ b/SessionMessagingKit/Sending & Receiving/MessageReceiver.swift @@ -269,7 +269,7 @@ public enum MessageReceiver { } // Validate - guard message.isValid(using: dependencies) else { + guard message.isValid(isSending: false) else { throw MessageReceiverError.invalidMessage } diff --git a/SessionMessagingKit/Sending & Receiving/MessageSender.swift b/SessionMessagingKit/Sending & Receiving/MessageSender.swift index ed2544ff2..b11ab48ee 100644 --- a/SessionMessagingKit/Sending & Receiving/MessageSender.swift +++ b/SessionMessagingKit/Sending & Receiving/MessageSender.swift @@ -568,7 +568,7 @@ public final class MessageSender { using dependencies: Dependencies ) throws { /// Check the message itself is valid - guard message.isValid(using: dependencies) else { throw MessageSenderError.invalidMessage } + guard message.isValid(isSending: true) else { throw MessageSenderError.invalidMessage } /// We now allow the creation of message data without validating it's attachments have finished uploading first, this is here to /// ensure we don't send a message which should have uploaded files diff --git a/SessionMessagingKitTests/Sending & Receiving/MessageSenderSpec.swift b/SessionMessagingKitTests/Sending & Receiving/MessageSenderSpec.swift index a0eba4ef7..16e811db3 100644 --- a/SessionMessagingKitTests/Sending & Receiving/MessageSenderSpec.swift +++ b/SessionMessagingKitTests/Sending & Receiving/MessageSenderSpec.swift @@ -37,6 +37,11 @@ class MessageSenderSpec: QuickSpec { .thenReturn(Array(Data(base64Encoded: "pbTUizreT0sqJ2R2LloseQDyVL2RYztD")!)) } ) + @TestState(cache: .general, in: dependencies) var mockGeneralCache: MockGeneralCache! = MockGeneralCache( + initialSetup: { cache in + cache.when { $0.sessionId }.thenReturn(SessionId(.standard, hex: TestConstants.publicKey)) + } + ) // MARK: - a MessageSender describe("a MessageSender") { diff --git a/SessionMessagingKitTests/_TestUtilities/MockPoller.swift b/SessionMessagingKitTests/_TestUtilities/MockPoller.swift index 11e760530..0918751fa 100644 --- a/SessionMessagingKitTests/_TestUtilities/MockPoller.swift +++ b/SessionMessagingKitTests/_TestUtilities/MockPoller.swift @@ -74,6 +74,8 @@ class MockPoller: Mock, PollerType { func startIfNeeded() { mockNoReturn() } func stop() { mockNoReturn() } + + func pollerDidStart() { mockNoReturn() } func poll(forceSynchronousProcessing: Bool) -> AnyPublisher { mock(args: [forceSynchronousProcessing]) } func nextPollDelay() -> TimeInterval { mock() } func handlePollError(_ error: Error, _ lastError: Error?) -> PollerErrorResponse { mock(args: [error, lastError]) } diff --git a/SessionMessagingKitTests/_TestUtilities/MockSwarmPoller.swift b/SessionMessagingKitTests/_TestUtilities/MockSwarmPoller.swift index 72633e913..ca43abdf2 100644 --- a/SessionMessagingKitTests/_TestUtilities/MockSwarmPoller.swift +++ b/SessionMessagingKitTests/_TestUtilities/MockSwarmPoller.swift @@ -70,6 +70,8 @@ class MockSwarmPoller: Mock, SwarmPollerType & Pol func startIfNeeded() { mockNoReturn() } func stop() { mockNoReturn() } + + func pollerDidStart() { mockNoReturn() } func poll(forceSynchronousProcessing: Bool) -> AnyPublisher { mock(args: [forceSynchronousProcessing]) } func nextPollDelay() -> TimeInterval { mock() } func handlePollError(_ error: Error, _ lastError: Error?) -> PollerErrorResponse { mock(args: [error, lastError]) }