Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix converting deleted database objects #3497

Open
wants to merge 14 commits into
base: develop
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,8 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/).
### 🐞 Fixed
- Fix connection not resuming after guest user goes to background [#3483](https://github.com/GetStream/stream-chat-swift/pull/3483)
- Fix empty channel list if the channel list filter contains OR statement with only custom filtering keys [#3482](https://github.com/GetStream/stream-chat-swift/pull/3482)
- Fix not returning models with empty properties when the underlying database model was deleted [#3497](https://github.com/GetStream/stream-chat-swift/pull/3497)
- Fix an issue where deleting current user in the local database cleared member data in channels [#3497](https://github.com/GetStream/stream-chat-swift/pull/3497)
- Fix rare crashes when accessing the current user object [#3500](https://github.com/GetStream/stream-chat-swift/pull/3500)
### ⚡ Performance
- Avoid creating `CurrentChatUserController` for reading user privacy settings which is more expensive than just reading the data from the local database [#3502](https://github.com/GetStream/stream-chat-swift/pull/3502)
Expand Down
2 changes: 2 additions & 0 deletions Sources/StreamChat/Database/DTOs/AttachmentDTO.swift
Original file line number Diff line number Diff line change
Expand Up @@ -261,6 +261,8 @@ private extension AttachmentDTO {
extension AttachmentDTO {
/// Snapshots the current state of `AttachmentDTO` and returns an immutable model object from it.
func asAnyModel() -> AnyChatMessageAttachment? {
guard !isDeleted else { return nil }

guard let id = attachmentID else {
log.debug("Attachment failed to be converted to model because ID is invalid.")
return nil
Expand Down
3 changes: 2 additions & 1 deletion Sources/StreamChat/Database/DTOs/ChannelConfigDTO.swift
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,8 @@ final class ChannelConfigDTO: NSManagedObject {
@NSManaged var commands: NSOrderedSet

func asModel() throws -> ChannelConfig {
.init(
try isNotDeleted()
return .init(
reactionsEnabled: reactionsEnabled,
typingEventsEnabled: typingEventsEnabled,
readEventsEnabled: readEventsEnabled,
Expand Down
1 change: 1 addition & 0 deletions Sources/StreamChat/Database/DTOs/ChannelDTO.swift
Original file line number Diff line number Diff line change
Expand Up @@ -470,6 +470,7 @@ extension ChatChannel {
guard StreamRuntimeCheck._canFetchRelationship(currentDepth: depth) else {
throw RecursionLimitError()
}
try dto.isNotDeleted()
guard let cid = try? ChannelId(cid: dto.cid), let context = dto.managedObjectContext else {
throw InvalidModel(dto)
}
Expand Down
8 changes: 6 additions & 2 deletions Sources/StreamChat/Database/DTOs/ChannelReadDTO.swift
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,10 @@ class ChannelReadDTO: NSManagedObject {

override func willSave() {
super.willSave()


guard !isDeleted else {
return
}
laevandus marked this conversation as resolved.
Show resolved Hide resolved
// When the read is updated, we need to propagate this change up to holding channel
if hasPersistentChangedValues, !channel.hasChanges, !channel.isDeleted {
// this will not change object, but mark it as dirty, triggering updates
Expand Down Expand Up @@ -201,7 +204,8 @@ extension ChannelReadDTO {

extension ChatChannelRead {
fileprivate static func create(fromDTO dto: ChannelReadDTO) throws -> ChatChannelRead {
try .init(
try dto.isNotDeleted()
return try .init(
lastReadAt: dto.lastReadAt.bridgeDate,
lastReadMessageId: dto.lastReadMessageId,
unreadMessagesCount: Int(dto.unreadMessageCount),
Expand Down
3 changes: 2 additions & 1 deletion Sources/StreamChat/Database/DTOs/CommandDTO.swift
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,8 @@ final class CommandDTO: NSManagedObject {
@NSManaged var args: String

func asModel() throws -> Command {
.init(
try isNotDeleted()
return .init(
name: name,
description: desc,
set: set,
Expand Down
2 changes: 2 additions & 0 deletions Sources/StreamChat/Database/DTOs/CurrentUserDTO.swift
Original file line number Diff line number Diff line change
Expand Up @@ -215,6 +215,8 @@ extension CurrentUserDTO {

extension CurrentChatUser {
fileprivate static func create(fromDTO dto: CurrentUserDTO) throws -> CurrentChatUser {
try dto.isNotDeleted()

let user = dto.user

var extraData = [String: RawJSON]()
Expand Down
3 changes: 2 additions & 1 deletion Sources/StreamChat/Database/DTOs/DeviceDTO.swift
Original file line number Diff line number Diff line change
Expand Up @@ -45,6 +45,7 @@ extension DeviceDTO {

extension DeviceDTO {
func asModel() throws -> Device {
Device(id: id, createdAt: createdAt?.bridgeDate)
try isNotDeleted()
return Device(id: id, createdAt: createdAt?.bridgeDate)
}
}
2 changes: 2 additions & 0 deletions Sources/StreamChat/Database/DTOs/MemberModelDTO.swift
Original file line number Diff line number Diff line change
Expand Up @@ -180,6 +180,8 @@ extension MemberDTO {

extension ChatChannelMember {
fileprivate static func create(fromDTO dto: MemberDTO) throws -> ChatChannelMember {
try dto.isNotDeleted()

let extraData: [String: RawJSON]
do {
extraData = try JSONDecoder.default.decode([String: RawJSON].self, from: dto.user.extraData)
Expand Down
1 change: 1 addition & 0 deletions Sources/StreamChat/Database/DTOs/MessageDTO.swift
Original file line number Diff line number Diff line change
Expand Up @@ -1339,6 +1339,7 @@ private extension ChatMessage {
guard let context = dto.managedObjectContext else {
throw InvalidModel(dto)
}
try dto.isNotDeleted()

id = dto.id
cid = try? dto.cid.map { try ChannelId(cid: $0) }
Expand Down
2 changes: 2 additions & 0 deletions Sources/StreamChat/Database/DTOs/MessageReactionDTO.swift
Original file line number Diff line number Diff line change
Expand Up @@ -205,6 +205,8 @@ extension MessageReactionDTO {

/// Snapshots the current state of `MessageReactionDTO` and returns an immutable model object from it.
func asModel() throws -> ChatMessageReaction {
try isNotDeleted()

let decodedExtraData: [String: RawJSON]

if let extraData = self.extraData, !extraData.isEmpty {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -50,14 +50,16 @@ extension Set where Element == MessageReactionGroupDTO {

func asModel() -> [MessageReactionType: ChatMessageReactionGroup] {
reduce(into: [:]) { partialResult, groupDTO in
partialResult[MessageReactionType(rawValue: groupDTO.type)] = groupDTO.asModel()
guard let model = try? groupDTO.asModel() else { return }
partialResult[MessageReactionType(rawValue: groupDTO.type)] = model
}
}
}

extension MessageReactionGroupDTO {
func asModel() -> ChatMessageReactionGroup {
.init(
func asModel() throws -> ChatMessageReactionGroup {
try isNotDeleted()
return .init(
type: MessageReactionType(rawValue: type),
sumScores: Int(sumScores),
count: Int(count),
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,11 @@ extension NSManagedObject {
}
return true
}

func isNotDeleted() throws {
guard isDeleted else { return }
throw DeletedModel(self)
}
}

struct InvalidModel: LocalizedError {
Expand All @@ -31,4 +36,18 @@ struct InvalidModel: LocalizedError {
}
}

struct DeletedModel: LocalizedError {
let id: NSManagedObjectID
let entityName: String?

init(_ model: NSManagedObject) {
id = model.objectID
entityName = model.entity.name
}

var errorDescription: String? {
"\(entityName ?? "Unknown") object with ID \(id) is deleted"
}
}

struct RecursionLimitError: LocalizedError {}
6 changes: 6 additions & 0 deletions Sources/StreamChat/Database/DTOs/PollDTO.swift
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,10 @@ class PollDTO: NSManagedObject {
override func willSave() {
super.willSave()

guard !isDeleted else {
return
}

// When the poll is updated, trigger message FRC update.
if let message = self.message, hasPersistentChangedValues, !message.hasChanges, !message.isDeleted {
message.id = message.id
Expand Down Expand Up @@ -82,6 +86,8 @@ extension PollDTO {

extension PollDTO {
func asModel() throws -> Poll {
try isNotDeleted()

var extraData: [String: RawJSON] = [:]
if let custom,
!custom.isEmpty,
Expand Down
2 changes: 2 additions & 0 deletions Sources/StreamChat/Database/DTOs/PollOptionDTO.swift
Original file line number Diff line number Diff line change
Expand Up @@ -58,6 +58,8 @@ extension PollOptionDTO {

extension PollOptionDTO {
func asModel() throws -> PollOption {
try isNotDeleted()

var extraData: [String: RawJSON] = [:]
if let custom,
!custom.isEmpty,
Expand Down
7 changes: 6 additions & 1 deletion Sources/StreamChat/Database/DTOs/PollVoteDTO.swift
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,10 @@ class PollVoteDTO: NSManagedObject {
override func willSave() {
super.willSave()

guard !isDeleted else {
return
}

// When the poll is updated, trigger message FRC update.
if let message = poll?.message, hasPersistentChangedValues, !message.hasChanges, !message.isDeleted {
message.id = message.id
Expand Down Expand Up @@ -75,7 +79,8 @@ extension PollVoteDTO {

extension PollVoteDTO {
func asModel() throws -> PollVote {
try PollVote(
try isNotDeleted()
return try PollVote(
id: id,
createdAt: createdAt.bridgeDate,
updatedAt: updatedAt.bridgeDate,
Expand Down
2 changes: 2 additions & 0 deletions Sources/StreamChat/Database/DTOs/ThreadDTO.swift
Original file line number Diff line number Diff line change
Expand Up @@ -143,6 +143,8 @@ extension ThreadDTO {

extension ThreadDTO {
func asModel() throws -> ChatThread {
try isNotDeleted()

let extraData: [String: RawJSON]
do {
extraData = try JSONDecoder.default.decode([String: RawJSON].self, from: self.extraData)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -46,7 +46,8 @@ extension ThreadParticipantDTO {

extension ThreadParticipantDTO {
func asModel() throws -> ThreadParticipant {
try .init(
try isNotDeleted()
return try .init(
user: user.asModel(),
threadId: threadId,
createdAt: createdAt.bridgeDate,
Expand Down
7 changes: 6 additions & 1 deletion Sources/StreamChat/Database/DTOs/ThreadReadDTO.swift
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,10 @@ class ThreadReadDTO: NSManagedObject {
override func willSave() {
super.willSave()

guard !isDeleted else {
return
}

// When the read is updated, we need to propagate this change up to holding thread
if hasPersistentChangedValues, !thread.hasChanges, !thread.isDeleted {
// this will not change object, but mark it as dirty, triggering updates
Expand Down Expand Up @@ -70,7 +74,8 @@ extension ThreadReadDTO {

extension ThreadReadDTO {
func asModel() throws -> ThreadRead {
try .init(
try isNotDeleted()
return try .init(
user: user.asModel(),
lastReadAt: lastReadAt?.bridgeDate,
unreadMessagesCount: Int(unreadMessagesCount)
Expand Down
6 changes: 6 additions & 0 deletions Sources/StreamChat/Database/DTOs/UserDTO.swift
Original file line number Diff line number Diff line change
Expand Up @@ -40,6 +40,10 @@ class UserDTO: NSManagedObject {
override func willSave() {
super.willSave()

guard !isDeleted else {
return
}

// We need to propagate fake changes to other models so that it triggers FRC
// updates for other entities. We also need to check that these models
// don't have changes already, otherwise it creates an infinite loop.
Expand Down Expand Up @@ -227,6 +231,8 @@ extension UserDTO {

extension ChatUser {
fileprivate static func create(fromDTO dto: UserDTO) throws -> ChatUser {
try dto.isNotDeleted()

let extraData: [String: RawJSON]
do {
extraData = try JSONDecoder.default.decode([String: RawJSON].self, from: dto.extraData)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -152,7 +152,7 @@
<relationship name="devices" optional="YES" toMany="YES" deletionRule="Nullify" destinationEntity="DeviceDTO" inverseName="user" inverseEntity="DeviceDTO"/>
<relationship name="flaggedMessages" optional="YES" toMany="YES" deletionRule="Nullify" destinationEntity="MessageDTO" inverseName="flaggedBy" inverseEntity="MessageDTO"/>
<relationship name="flaggedUsers" optional="YES" toMany="YES" deletionRule="Nullify" destinationEntity="UserDTO" inverseName="flaggedBy" inverseEntity="UserDTO"/>
<relationship name="mutedUsers" toMany="YES" deletionRule="Cascade" destinationEntity="UserDTO" inverseName="mutedBy" inverseEntity="UserDTO"/>
<relationship name="mutedUsers" toMany="YES" deletionRule="Nullify" destinationEntity="UserDTO" inverseName="mutedBy" inverseEntity="UserDTO"/>
<relationship name="user" optional="YES" maxCount="1" deletionRule="Nullify" destinationEntity="UserDTO" inverseName="currentUser" inverseEntity="UserDTO"/>
<uniquenessConstraints>
<uniquenessConstraint>
Expand Down
18 changes: 12 additions & 6 deletions Sources/StreamChat/Repositories/PollsRepository.swift
Original file line number Diff line number Diff line change
Expand Up @@ -144,12 +144,18 @@ class PollsRepository {
voteId: String,
completion: ((Error?) -> Void)? = nil
) {
var pollVote: PollVote?
var exists = false
var answerText: String?
var optionId: String?
var userId: UserId?
var filterHash: String?
database.write { session in
let voteDto = try session.removePollVote(with: voteId, pollId: pollId)
exists = voteDto != nil
filterHash = voteDto?.queries?.first?.filterHash
pollVote = try voteDto?.asModel()
answerText = voteDto?.answerText
optionId = voteDto?.optionId
userId = voteDto?.user?.id
nuno-vieira marked this conversation as resolved.
Show resolved Hide resolved
} completion: { [weak self] error in
if error == nil {
self?.apiClient.request(
Expand All @@ -159,14 +165,14 @@ class PollsRepository {
voteId: voteId
)
) {
if $0.error != nil, $0.error?.isBackendNotFound404StatusCode == false, let pollVote {
if $0.error != nil, $0.error?.isBackendNotFound404StatusCode == false, exists {
self?.database.write { session in
_ = try session.savePollVote(
voteId: voteId,
pollId: pollId,
optionId: pollVote.optionId,
answerText: pollVote.answerText,
userId: pollVote.user?.id,
optionId: optionId,
answerText: answerText,
userId: userId,
query: nil
)
try? session.linkVote(with: voteId, in: pollId, to: filterHash)
Expand Down
43 changes: 43 additions & 0 deletions Tests/StreamChatTests/Database/DTOs/CurrentUserDTO_Tests.swift
Original file line number Diff line number Diff line change
Expand Up @@ -289,4 +289,47 @@ final class CurrentUserModelDTO_Tests: XCTestCase {
XCTAssertEqual(true, loadedCurrentUser.privacySettings.readReceipts?.enabled)
XCTAssertEqual(true, loadedCurrentUser.privacySettings.typingIndicators?.enabled)
}

func test_deletingCurrentUser() throws {
laevandus marked this conversation as resolved.
Show resolved Hide resolved
let currentUserId = "current_user_id"
let mutedUserId = "muted_user_id"
let cid = ChannelId.unique
try database.writeSynchronously { session in
try session.saveCurrentUser(
payload: .dummy(
userId: currentUserId,
role: .admin,
mutedUsers: [.dummy(
userId: mutedUserId
)]
)
)
try session.saveChannel(
payload: .dummy(
channel: .dummy(cid: cid),
members: [
.dummy(user: .dummy(userId: currentUserId)),
.dummy(user: .dummy(userId: mutedUserId))
]
)
)
}
try database.readSynchronously { session in
guard let channelDTO = session.channel(cid: cid) else { throw ClientError.ChannelDoesNotExist(cid: cid) }
let channel = try channelDTO.asModel()
let memberIds = channel.lastActiveMembers.map(\.id).sorted()
XCTAssertEqual([currentUserId, mutedUserId].sorted(), memberIds)
}
// Delete current user which should not clear member ids of the channel
try database.writeSynchronously { session in
guard let currentUserDTO = session.currentUser else { throw ClientError.CurrentUserDoesNotExist() }
(session as! NSManagedObjectContext).delete(currentUserDTO)
}
try database.writeSynchronously { session in
guard let channelDTO = session.channel(cid: cid) else { throw ClientError.ChannelDoesNotExist(cid: cid) }
let channel = try channelDTO.asModel()
let memberIds = channel.lastActiveMembers.map(\.id).sorted()
XCTAssertEqual([currentUserId, mutedUserId].sorted(), memberIds)
}
}
}
Loading
Loading