From 9237c1343851da791cc3ad975d954f3533dd97c6 Mon Sep 17 00:00:00 2001 From: Jeroen Leenarts Date: Mon, 15 Nov 2021 13:18:31 +0100 Subject: [PATCH] Marked model objects as (non)optional in line with the Core Data model - Uses sensible defaults when possible - Unit tests are also adjusted - Populate `ownReactions` and `latestReactions` on MessageDTO when creating a new object, they are non-optinal on the Core Data Model - Mark `MessageReactionDTO.id` and `MessageDTO.quotedMessage` and `ChannelDto.typeRawValue as non-optional - Make localStateRaw non optional on both the MessageReactionDTO and the AttachmentDTO. --- .../Database/DTOs/AttachmentDTO.swift | 14 ++++-- .../Database/DTOs/AttachmentDTO_Tests.swift | 8 ++-- .../Database/DTOs/ChannelMuteDTO.swift | 2 +- .../Database/DTOs/ChannelReadDTO.swift | 8 +++- .../StreamChat/Database/DTOs/MessageDTO.swift | 46 ++++++++++++------- .../Database/DTOs/MessageDTO_Tests.swift | 6 +-- .../Database/DTOs/MessageReactionDTO.swift | 29 ++++++------ .../Database/DTOs/MessageSearchQueryDTO.swift | 2 +- .../StreamChatModel.xcdatamodel/contents | 44 +++++++++--------- .../Models/Attachments/AttachmentTypes.swift | 2 + Sources/StreamChat/Models/ChatMessage.swift | 3 ++ Sources/StreamChat/Models/MuteDetails.swift | 2 +- .../ChannelReadUpdaterMiddleware.swift | 2 +- .../Workers/MessageUpdater_Tests.swift | 2 +- 14 files changed, 98 insertions(+), 72 deletions(-) diff --git a/Sources/StreamChat/Database/DTOs/AttachmentDTO.swift b/Sources/StreamChat/Database/DTOs/AttachmentDTO.swift index c84816d3225..7382ca504e6 100644 --- a/Sources/StreamChat/Database/DTOs/AttachmentDTO.swift +++ b/Sources/StreamChat/Database/DTOs/AttachmentDTO.swift @@ -15,9 +15,9 @@ class AttachmentDTO: NSManagedObject { } /// An attachment type. - @NSManaged private var type: String + @NSManaged private var type: String? var attachmentType: AttachmentType { - get { .init(rawValue: type) } + get { AttachmentType(rawValue: type ?? AttachmentType.unknown.rawValue) } set { type = newValue.rawValue } } @@ -25,7 +25,9 @@ class AttachmentDTO: NSManagedObject { @NSManaged private var localStateRaw: String @NSManaged private var localProgress: Double var localState: LocalAttachmentState? { - get { LocalAttachmentState(rawValue: localStateRaw, progress: localProgress) } + get { + LocalAttachmentState(rawValue: localStateRaw, progress: localProgress) + } set { localStateRaw = newValue?.rawValue ?? "" localProgress = newValue?.progress ?? 0 @@ -221,6 +223,8 @@ extension AttachmentDTO { extension LocalAttachmentState { var rawValue: String { switch self { + case .unknown: + return "" case .pendingUpload: return "pendingUpload" case .uploading: @@ -251,8 +255,10 @@ extension LocalAttachmentState { self = .uploadingFailed case LocalAttachmentState.uploaded.rawValue: self = .uploaded + case LocalAttachmentState.unknown.rawValue: + self = .unknown default: - return nil + self = .unknown } } } diff --git a/Sources/StreamChat/Database/DTOs/AttachmentDTO_Tests.swift b/Sources/StreamChat/Database/DTOs/AttachmentDTO_Tests.swift index bfba03fe080..ab33f2d48d0 100644 --- a/Sources/StreamChat/Database/DTOs/AttachmentDTO_Tests.swift +++ b/Sources/StreamChat/Database/DTOs/AttachmentDTO_Tests.swift @@ -37,7 +37,7 @@ class AttachmentDTO_Tests: XCTestCase { // Assert attachment has correct values. XCTAssertEqual(loadedAttachment.attachmentID, attachmentId) - XCTAssertEqual(loadedAttachment.localState, nil) + XCTAssertEqual(loadedAttachment.localState, .unknown) XCTAssertEqual(loadedAttachment.attachmentType, attachment.type) XCTAssertEqual(loadedAttachment.message.id, messageId) XCTAssertEqual(loadedAttachment.channel.cid, cid.rawValue) @@ -72,7 +72,7 @@ class AttachmentDTO_Tests: XCTestCase { // Assert attachment has correct values. XCTAssertEqual(loadedAttachment.attachmentID, attachmentId) - XCTAssertEqual(loadedAttachment.localState, nil) + XCTAssertEqual(loadedAttachment.localState, .unknown) XCTAssertEqual(loadedAttachment.attachmentType, attachment.type) XCTAssertEqual(loadedAttachment.message.id, messageId) XCTAssertEqual(loadedAttachment.channel.cid, cid.rawValue) @@ -107,7 +107,7 @@ class AttachmentDTO_Tests: XCTestCase { // Assert attachment has correct values. XCTAssertEqual(loadedAttachment.attachmentID, attachmentId) - XCTAssertEqual(loadedAttachment.localState, nil) + XCTAssertEqual(loadedAttachment.localState, .unknown) XCTAssertEqual(loadedAttachment.attachmentType, attachment.type) XCTAssertEqual(loadedAttachment.message.id, messageId) XCTAssertEqual(loadedAttachment.channel.cid, cid.rawValue) @@ -270,7 +270,7 @@ class AttachmentDTO_Tests: XCTestCase { // Assert attachment local file URL and state are nil. XCTAssertNil(loadedAttachment?.localURL) - XCTAssertNil(loadedAttachment?.localState) + XCTAssertEqual(loadedAttachment?.localState, .unknown) } func test_attachmentChange_triggerMessageUpdate() throws { diff --git a/Sources/StreamChat/Database/DTOs/ChannelMuteDTO.swift b/Sources/StreamChat/Database/DTOs/ChannelMuteDTO.swift index ff6de2bde9a..0e78daec603 100644 --- a/Sources/StreamChat/Database/DTOs/ChannelMuteDTO.swift +++ b/Sources/StreamChat/Database/DTOs/ChannelMuteDTO.swift @@ -8,7 +8,7 @@ import Foundation @objc(ChannelMuteDTO) final class ChannelMuteDTO: NSManagedObject { @NSManaged var createdAt: Date - @NSManaged var updatedAt: Date + @NSManaged var updatedAt: Date? @NSManaged var channel: ChannelDTO @NSManaged var user: UserDTO diff --git a/Sources/StreamChat/Database/DTOs/ChannelReadDTO.swift b/Sources/StreamChat/Database/DTOs/ChannelReadDTO.swift index f0558f9e30f..98a35c1ec79 100644 --- a/Sources/StreamChat/Database/DTOs/ChannelReadDTO.swift +++ b/Sources/StreamChat/Database/DTOs/ChannelReadDTO.swift @@ -7,7 +7,7 @@ import Foundation @objc(ChannelReadDTO) class ChannelReadDTO: NSManagedObject { - @NSManaged var lastReadAt: Date + @NSManaged var lastReadAt: Date? @NSManaged var unreadMessageCount: Int32 // MARK: - Relationships @@ -124,6 +124,10 @@ extension NSManagedObjectContext { extension ChatChannelRead { fileprivate static func create(fromDTO dto: ChannelReadDTO) -> ChatChannelRead { - .init(lastReadAt: dto.lastReadAt, unreadMessagesCount: Int(dto.unreadMessageCount), user: dto.user.asModel()) + .init( + lastReadAt: dto.lastReadAt ?? Date.distantPast, + unreadMessagesCount: Int(dto.unreadMessageCount), + user: dto.user.asModel() + ) } } diff --git a/Sources/StreamChat/Database/DTOs/MessageDTO.swift b/Sources/StreamChat/Database/DTOs/MessageDTO.swift index 5e41058dfe4..a4750447814 100644 --- a/Sources/StreamChat/Database/DTOs/MessageDTO.swift +++ b/Sources/StreamChat/Database/DTOs/MessageDTO.swift @@ -20,7 +20,7 @@ class MessageDTO: NSManagedObject { @NSManaged var parentMessageId: MessageId? @NSManaged var showReplyInChannel: Bool @NSManaged var replyCount: Int32 - @NSManaged var extraData: Data + @NSManaged var extraData: Data? @NSManaged var isSilent: Bool @NSManaged var isShadowed: Bool @NSManaged var reactionScores: [String: Int] @@ -280,6 +280,8 @@ class MessageDTO: NSManagedObject { let new = NSEntityDescription.insertNewObject(forEntityName: entityName, into: context) as! Self new.id = id + new.latestReactions = [] + new.ownReactions = [] return new } @@ -667,15 +669,20 @@ extension MessageDTO { /// Snapshots the current state of `MessageDTO` and returns its representation for the use in API calls. func asRequestBody() -> MessageRequestBody { - var extraData: [String: RawJSON] - do { - extraData = try JSONDecoder.default.decode([String: RawJSON].self, from: self.extraData) - } catch { - log.assertionFailure( - "Failed decoding saved extra data with error: \(error). This should never happen because" - + "the extra data must be a valid JSON to be saved." - ) - extraData = [:] + var decodedExtraData: [String: RawJSON] + + if let extraData = self.extraData { + do { + decodedExtraData = try JSONDecoder.default.decode([String: RawJSON].self, from: extraData) + } catch { + log.assertionFailure( + "Failed decoding saved extra data with error: \(error). This should never happen because" + + "the extra data must be a valid JSON to be saved." + ) + decodedExtraData = [:] + } + } else { + decodedExtraData = [:] } return .init( @@ -694,7 +701,7 @@ extension MessageDTO { mentionedUserIds: mentionedUsers.map(\.id), pinned: pinned, pinExpires: pinExpires, - extraData: extraData + extraData: decodedExtraData ) } } @@ -720,11 +727,18 @@ private extension ChatMessage { isShadowed = dto.isShadowed reactionScores = dto.reactionScores.mapKeys { MessageReactionType(rawValue: $0) } reactionCounts = dto.reactionCounts.mapKeys { MessageReactionType(rawValue: $0) } - - do { - extraData = try JSONDecoder.default.decode([String: RawJSON].self, from: dto.extraData) - } catch { - log.error("Failed to decode extra data for Message with id: <\(dto.id)>, using default value instead. Error: \(error)") + + if let extraData = dto.extraData, !extraData.isEmpty { + do { + self.extraData = try JSONDecoder.default.decode([String: RawJSON].self, from: extraData) + } catch { + log + .error( + "Failed to decode extra data for Message with id: <\(dto.id)>, using default value instead. Error: \(error)" + ) + self.extraData = [:] + } + } else { extraData = [:] } diff --git a/Sources/StreamChat/Database/DTOs/MessageDTO_Tests.swift b/Sources/StreamChat/Database/DTOs/MessageDTO_Tests.swift index 0bbde86d82c..8b721ea4831 100644 --- a/Sources/StreamChat/Database/DTOs/MessageDTO_Tests.swift +++ b/Sources/StreamChat/Database/DTOs/MessageDTO_Tests.swift @@ -20,7 +20,7 @@ class MessageDTO_Tests: XCTestCase { super.tearDown() } - func test_messagePayload_isStoredAndLoadedFromDB() { + func test_messagePayload_isStoredAndLoadedFromDB() throws { let userId: UserId = .unique let messageId: MessageId = .unique let channelId: ChannelId = .unique @@ -143,7 +143,7 @@ class MessageDTO_Tests: XCTestCase { ) XCTAssertEqual(Int32(messagePayload.replyCount), loadedMessage?.replyCount) XCTAssertEqual(messagePayload.extraData, loadedMessage.map { - try? JSONDecoder.default.decode([String: RawJSON].self, from: $0.extraData) + try? JSONDecoder.default.decode([String: RawJSON].self, from: $0.extraData!) }) XCTAssertEqual(messagePayload.reactionScores, loadedMessage?.reactionScores.mapKeys { reaction in MessageReactionType(rawValue: reaction) @@ -228,7 +228,7 @@ class MessageDTO_Tests: XCTestCase { ) Assert.willBeEqual(Int32(messagePayload.replyCount), loadedMessage?.replyCount) Assert.willBeEqual(messagePayload.extraData, loadedMessage.map { - try? JSONDecoder.default.decode([String: RawJSON].self, from: $0.extraData) + try? JSONDecoder.default.decode([String: RawJSON].self, from: $0.extraData!) }) Assert.willBeEqual(messagePayload.reactionScores, loadedMessage?.reactionScores.mapKeys { reaction in MessageReactionType(rawValue: reaction) diff --git a/Sources/StreamChat/Database/DTOs/MessageReactionDTO.swift b/Sources/StreamChat/Database/DTOs/MessageReactionDTO.swift index c38e3b3f46d..94df69c10b1 100644 --- a/Sources/StreamChat/Database/DTOs/MessageReactionDTO.swift +++ b/Sources/StreamChat/Database/DTOs/MessageReactionDTO.swift @@ -10,12 +10,12 @@ final class MessageReactionDTO: NSManagedObject { @NSManaged private(set) var id: String // holds the rawValue of LocalReactionState - @NSManaged fileprivate var localStateRaw: String? + @NSManaged fileprivate var localStateRaw: String @NSManaged var type: String @NSManaged var score: Int64 @NSManaged var createdAt: Date? @NSManaged var updatedAt: Date? - @NSManaged var extraData: Data + @NSManaged var extraData: Data? @NSManaged var message: MessageDTO @NSManaged var user: UserDTO @@ -47,7 +47,7 @@ extension MessageReactionDTO { static let notLocallyDeletedPredicates: NSPredicate = { NSCompoundPredicate(orPredicateWithSubpredicates: [ - NSPredicate(format: "localStateRaw == nil"), + NSPredicate(format: "localStateRaw == %@", LocalReactionState.unknown.rawValue), NSPredicate(format: "localStateRaw == %@", LocalReactionState.sending.rawValue), NSPredicate(format: "localStateRaw == %@", LocalReactionState.pendingSend.rawValue) ]) @@ -125,29 +125,26 @@ extension NSManagedObjectContext { extension MessageReactionDTO { var localState: LocalReactionState? { get { - guard let state = localStateRaw else { - return nil - } - return LocalReactionState(rawValue: state) + LocalReactionState(rawValue: localStateRaw) } set(state) { - localStateRaw = state?.rawValue + localStateRaw = state?.rawValue ?? LocalReactionState.unknown.rawValue } } /// Snapshots the current state of `MessageReactionDTO` and returns an immutable model object from it. func asModel() -> ChatMessageReaction { - let extraData: [String: RawJSON] - - if self.extraData.isEmpty { - extraData = [:] - } else { + let decodedExtraData: [String: RawJSON] + + if let extraData = self.extraData, !extraData.isEmpty { do { - extraData = try JSONDecoder.default.decode([String: RawJSON].self, from: self.extraData) + decodedExtraData = try JSONDecoder.default.decode([String: RawJSON].self, from: extraData) } catch { log.error("Failed decoding saved extra data with error: \(error)") - extraData = [:] + decodedExtraData = [:] } + } else { + decodedExtraData = [:] } return .init( @@ -156,7 +153,7 @@ extension MessageReactionDTO { createdAt: createdAt ?? .init(), updatedAt: updatedAt ?? .init(), author: user.asModel(), - extraData: extraData + extraData: decodedExtraData ) } } diff --git a/Sources/StreamChat/Database/DTOs/MessageSearchQueryDTO.swift b/Sources/StreamChat/Database/DTOs/MessageSearchQueryDTO.swift index 4a5baaa6ce1..633e923850a 100644 --- a/Sources/StreamChat/Database/DTOs/MessageSearchQueryDTO.swift +++ b/Sources/StreamChat/Database/DTOs/MessageSearchQueryDTO.swift @@ -7,7 +7,7 @@ import CoreData @objc(MessageSearchQueryDTO) class MessageSearchQueryDTO: NSManagedObject { /// Unique identifier of the query/ - @NSManaged var filterHash: String + @NSManaged var filterHash: String? @NSManaged var messages: Set diff --git a/Sources/StreamChat/Database/StreamChatModel.xcdatamodeld/StreamChatModel.xcdatamodel/contents b/Sources/StreamChat/Database/StreamChatModel.xcdatamodeld/StreamChatModel.xcdatamodel/contents index 4429edcc43c..eb7084e06b1 100644 --- a/Sources/StreamChat/Database/StreamChatModel.xcdatamodeld/StreamChatModel.xcdatamodel/contents +++ b/Sources/StreamChat/Database/StreamChatModel.xcdatamodeld/StreamChatModel.xcdatamodel/contents @@ -1,10 +1,10 @@ - + - - + + @@ -18,7 +18,7 @@ - + @@ -32,9 +32,9 @@ - + - + @@ -92,22 +92,22 @@ - + - + - - + + @@ -122,7 +122,7 @@ - + @@ -138,7 +138,7 @@ - + @@ -165,10 +165,10 @@ - + - + @@ -210,8 +210,8 @@ - - + + @@ -243,7 +243,7 @@ - + @@ -290,19 +290,19 @@ - - + + - + - + - + \ No newline at end of file diff --git a/Sources/StreamChat/Models/Attachments/AttachmentTypes.swift b/Sources/StreamChat/Models/Attachments/AttachmentTypes.swift index 8a367622ec9..5126fed8d44 100644 --- a/Sources/StreamChat/Models/Attachments/AttachmentTypes.swift +++ b/Sources/StreamChat/Models/Attachments/AttachmentTypes.swift @@ -23,6 +23,8 @@ enum AttachmentCodingKeys: String, CodingKey, CaseIterable { /// A local state of the attachment. Applies only for attachments linked to the new messages sent from current device. public enum LocalAttachmentState: Hashable { + /// The current state is unknown + case unknown /// The attachment is waiting to be uploaded. case pendingUpload /// The attachment is currently being uploaded. The progress in [0, 1] range. diff --git a/Sources/StreamChat/Models/ChatMessage.swift b/Sources/StreamChat/Models/ChatMessage.swift index e28af74b86e..dee1b5fe8da 100644 --- a/Sources/StreamChat/Models/ChatMessage.swift +++ b/Sources/StreamChat/Models/ChatMessage.swift @@ -358,6 +358,9 @@ public enum LocalMessageState: String { } public enum LocalReactionState: String { + /// The reaction state is unknown + case unknown = "" + /// The reaction is waiting to be sent to the server case pendingSend diff --git a/Sources/StreamChat/Models/MuteDetails.swift b/Sources/StreamChat/Models/MuteDetails.swift index c449ea08cd6..d3fb923c775 100644 --- a/Sources/StreamChat/Models/MuteDetails.swift +++ b/Sources/StreamChat/Models/MuteDetails.swift @@ -9,5 +9,5 @@ public struct MuteDetails: Equatable { /// The time when the mute action was taken. public let createdAt: Date /// The time when the mute was updated. - public let updatedAt: Date + public let updatedAt: Date? } diff --git a/Sources/StreamChat/WebSocketClient/EventMiddlewares/ChannelReadUpdaterMiddleware.swift b/Sources/StreamChat/WebSocketClient/EventMiddlewares/ChannelReadUpdaterMiddleware.swift index f5338e17648..3634be94192 100644 --- a/Sources/StreamChat/WebSocketClient/EventMiddlewares/ChannelReadUpdaterMiddleware.swift +++ b/Sources/StreamChat/WebSocketClient/EventMiddlewares/ChannelReadUpdaterMiddleware.swift @@ -72,7 +72,7 @@ struct ChannelReadUpdaterMiddleware: EventMiddleware { // Try to get the existing channel read for the current user if let read = session.loadChannelRead(cid: cid, userId: currentUserId) { - if message.createdAt > read.lastReadAt { + if message.createdAt > read.lastReadAt ?? Date.distantPast { read.unreadMessageCount += 1 } } else { diff --git a/Sources/StreamChat/Workers/MessageUpdater_Tests.swift b/Sources/StreamChat/Workers/MessageUpdater_Tests.swift index 8ace5b7c465..b683caeee90 100644 --- a/Sources/StreamChat/Workers/MessageUpdater_Tests.swift +++ b/Sources/StreamChat/Workers/MessageUpdater_Tests.swift @@ -1098,7 +1098,7 @@ final class MessageUpdater_Tests: XCTestCase { return } - XCTAssertEqual(reactionReloaded.localState, nil) + XCTAssertEqual(reactionReloaded.localState, .unknown) } // MARK: - Pinning message