Fixed a few bugs uncovered by QA

Fixed a bug where the ConfigurationMessage was getting generated before the contact state was persisted to the database in the message request flow causing odd behaviours (now generating the ConfigurationMessage within the same transaction)
Fixed a bug where sending a message to an existing message request thread once the message requests item has been hidden would show the message requests notification and trigger the section to re-appear on the home screen
Fixed a bug where blocked contacts weren't getting excluded from the contacts list in the configuration message
This commit is contained in:
Morgan Pretty 2022-02-23 17:12:57 +11:00
parent 2d6dad67eb
commit 4c89c165d8
9 changed files with 83 additions and 39 deletions

View File

@ -1138,7 +1138,7 @@ extension ConversationVC {
// Send a sync message with the details of the contact
if let appDelegate = UIApplication.shared.delegate as? AppDelegate {
appDelegate.forceSyncConfigurationNowIfNeeded().retainUntilComplete()
appDelegate.forceSyncConfigurationNowIfNeeded(with: transaction).retainUntilComplete()
}
}
}

View File

@ -273,24 +273,28 @@ final class HomeVC : BaseVC, UITableViewDataSource, UITableViewDelegate, NewConv
// Separate out the changes for new message requests and the inbox (so we can avoid updating for
// new messages within an existing message request)
let messageRequestInserts = rowChanges
let messageRequestChanges = rowChanges
.compactMap { $0 as? YapDatabaseViewRowChange }
.filter { $0.finalGroup == TSMessageRequestGroup && $0.type == .insert }
.filter { $0.originalGroup == TSMessageRequestGroup || $0.finalGroup == TSMessageRequestGroup }
let inboxRowChanges = rowChanges
.filter { ($0 as? YapDatabaseViewRowChange)?.finalGroup != TSMessageRequestGroup }
.compactMap { $0 as? YapDatabaseViewRowChange }
.filter { $0.originalGroup == TSInboxGroup || $0.finalGroup == TSInboxGroup }
guard sectionChanges.count > 0 || inboxRowChanges.count > 0 || messageRequestInserts.count > 0 else { return }
guard sectionChanges.count > 0 || inboxRowChanges.count > 0 || messageRequestChanges.count > 0 else { return }
tableView.beginUpdates()
// If we need to unhide the message request row and then re-insert it
if !messageRequestInserts.isEmpty && (CurrentAppContext().appUserDefaults()[.hasHiddenMessageRequests] || tableView.numberOfRows(inSection: 0) == 0) {
CurrentAppContext().appUserDefaults()[.hasHiddenMessageRequests] = false
tableView.insertRows(at: [IndexPath(row: 0, section: 0)], with: .automatic)
if !messageRequestChanges.isEmpty {
if tableView.numberOfRows(inSection: 0) == 1 && Int(messageRequestCount) <= 0 {
tableView.deleteRows(at: [IndexPath(row: 0, section: 0)], with: .automatic)
}
else if tableView.numberOfRows(inSection: 0) == 0 && Int(messageRequestCount) > 0 && !CurrentAppContext().appUserDefaults()[.hasHiddenMessageRequests] {
tableView.insertRows(at: [IndexPath(row: 0, section: 0)], with: .automatic)
}
}
inboxRowChanges.forEach { rowChange in
let rowChange = rowChange as! YapDatabaseViewRowChange
let key = rowChange.collectionKey.key
threadViewModelCache[key] = nil
@ -310,12 +314,9 @@ final class HomeVC : BaseVC, UITableViewDataSource, UITableViewDelegate, NewConv
// an insert as the change won't be defined correctly)
if rowChange.originalGroup == TSMessageRequestGroup && rowChange.finalGroup == TSInboxGroup {
tableView.insertRows(at: [ rowChange.newIndexPath! ], with: .automatic)
// If that was the last message request then we need to also remove the message request
// row to prevent a crash
if messageRequestCount == 0 {
tableView.deleteRows(at: [ IndexPath(row: 0, section: 0) ], with: .automatic)
}
}
else if rowChange.originalGroup == TSInboxGroup && rowChange.finalGroup == TSMessageRequestGroup {
tableView.deleteRows(at: [ rowChange.indexPath! ], with: .automatic)
}
default: break
@ -336,7 +337,7 @@ final class HomeVC : BaseVC, UITableViewDataSource, UITableViewDelegate, NewConv
case .move:
// Since we are custom handling this specific movement in the above 'updates' call we need
// to avoid trying to handle it here
if rowChange.originalGroup == TSMessageRequestGroup && rowChange.finalGroup == TSInboxGroup {
if rowChange.originalGroup == TSMessageRequestGroup || rowChange.finalGroup == TSMessageRequestGroup {
return
}

View File

@ -22,9 +22,11 @@ extension AppDelegate {
}
}
func forceSyncConfigurationNowIfNeeded() -> Promise<Void> {
guard Storage.shared.getUser()?.name != nil,
let configurationMessage = ConfigurationMessage.getCurrent() else { return Promise.value(()) }
func forceSyncConfigurationNowIfNeeded(with transaction: YapDatabaseReadWriteTransaction? = nil) -> Promise<Void> {
guard Storage.shared.getUser()?.name != nil, let configurationMessage = ConfigurationMessage.getCurrent(with: transaction) else {
return Promise.value(())
}
let destination = Message.Destination.contact(publicKey: getUserHexEncodedPublicKey())
let (promise, seal) = Promise<Void>.pending()
Storage.writeSync { transaction in

View File

@ -182,6 +182,9 @@ public class NotificationPresenter: NSObject, NotificationsProtocol {
guard numMessageRequests == 0 else { return }
}
else if thread.isMessageRequest() && CurrentAppContext().appUserDefaults()[.hasHiddenMessageRequests] {
// If there are other interactions on this thread already then don't show the notification
if thread.numberOfInteractions() > 1 { return }
CurrentAppContext().appUserDefaults()[.hasHiddenMessageRequests] = false
}

View File

@ -55,10 +55,16 @@ extension Storage {
@objc public func getAllContacts() -> Set<Contact> {
var result: Set<Contact> = []
Storage.read { transaction in
transaction.enumerateRows(inCollection: Storage.contactCollection) { _, object, _, _ in
guard let contact = object as? Contact else { return }
result.insert(contact)
}
result = self.getAllContacts(with: transaction)
}
return result
}
@objc public func getAllContacts(with transaction: YapDatabaseReadTransaction) -> Set<Contact> {
var result: Set<Contact> = []
transaction.enumerateRows(inCollection: Storage.contactCollection) { _, object, _, _ in
guard let contact = object as? Contact else { return }
result.insert(contact)
}
return result
}

View File

@ -1,7 +1,7 @@
extension ConfigurationMessage {
public static func getCurrent() -> ConfigurationMessage? {
public static func getCurrent(with transaction: YapDatabaseReadWriteTransaction? = nil) -> ConfigurationMessage? {
let storage = Storage.shared
guard let user = storage.getUser() else { return nil }
@ -13,7 +13,7 @@ extension ConfigurationMessage {
var contacts: Set<Contact> = []
var contactCount = 0
Storage.read { transaction in
let populateDataClosure: (YapDatabaseReadTransaction) -> () = { transaction in
TSGroupThread.enumerateCollectionObjects(with: transaction) { object, _ in
guard let thread = object as? TSGroupThread else { return }
@ -47,7 +47,8 @@ extension ConfigurationMessage {
}
}
var truncatedContacts = storage.getAllContacts()
let currentUserPublicKey: String = getUserHexEncodedPublicKey()
var truncatedContacts = storage.getAllContacts(with: transaction)
if truncatedContacts.count > 200 {
truncatedContacts = Set(Array(truncatedContacts)[0..<200])
@ -57,10 +58,12 @@ extension ConfigurationMessage {
let publicKey = contact.sessionID
let threadID = TSContactThread.threadID(fromContactSessionID: publicKey)
// Want to sync contacts for visible threads and blocked contacts between devices
guard
let thread = TSContactThread.fetch(uniqueId: threadID, transaction: transaction),
thread.shouldBeVisible &&
!SSKEnvironment.shared.blockingManager.isRecipientIdBlocked(publicKey)
publicKey != currentUserPublicKey && (
TSContactThread.fetch(uniqueId: threadID, transaction: transaction)?.shouldBeVisible == true ||
SSKEnvironment.shared.blockingManager.isRecipientIdBlocked(publicKey)
)
else {
return
}
@ -87,6 +90,15 @@ extension ConfigurationMessage {
}
}
// If we are provided with a transaction then read the data based on the state of the database
// from within the transaction rather than the state in disk
if let transaction: YapDatabaseReadWriteTransaction = transaction {
populateDataClosure(transaction)
}
else {
Storage.read { transaction in populateDataClosure(transaction) }
}
return ConfigurationMessage(
displayName: displayName,
profilePictureURL: profilePictureURL,

View File

@ -223,24 +223,40 @@ extension MessageReceiver {
if contactInfo.hasDidApproveMe { contact.didApproveMe = contactInfo.didApproveMe }
Storage.shared.setContact(contact, using: transaction)
let thread = TSContactThread.getOrCreateThread(withContactSessionID: sessionID, transaction: transaction)
thread.shouldBeVisible = true
thread.save(with: transaction)
// If the contact is blocked
if contactInfo.hasIsBlocked && contactInfo.isBlocked {
// If this message changed them to the blocked state and there is an existing thread
// associated with them that is a message request thread then delete it (assume
// that the current user had deleted that message request)
if
contactInfo.isBlocked != OWSBlockingManager.shared().isRecipientIdBlocked(sessionID),
let thread: TSContactThread = TSContactThread.getWithContactSessionID(sessionID, transaction: transaction),
thread.isMessageRequest(using: transaction)
{
thread.removeAllThreadInteractions(with: transaction)
thread.remove(with: transaction)
}
}
else {
// Otherwise create and save the thread
let thread = TSContactThread.getOrCreateThread(withContactSessionID: sessionID, transaction: transaction)
thread.shouldBeVisible = true
thread.save(with: transaction)
}
}
// Contacts blocked state
// FIXME: 'OWSBlockingManager' manages it's own dbConnection and transactions so we have to dispatch this to prevent deadlocks
DispatchQueue.global(qos: .background).async {
DispatchQueue.global().async {
for contactInfo in message.contacts {
let sessionID = contactInfo.publicKey!
let contact = Contact(sessionID: sessionID)
if contact.isBlocked != OWSBlockingManager.shared().isRecipientIdBlocked(contact.sessionID) {
if contact.isBlocked {
OWSBlockingManager.shared().addBlockedPhoneNumber(contact.sessionID)
if contactInfo.hasIsBlocked && contactInfo.isBlocked != OWSBlockingManager.shared().isRecipientIdBlocked(sessionID) {
if contactInfo.isBlocked {
OWSBlockingManager.shared().addBlockedPhoneNumber(sessionID)
}
else {
OWSBlockingManager.shared().removeBlockedPhoneNumber(contact.sessionID)
OWSBlockingManager.shared().removeBlockedPhoneNumber(sessionID)
}
}
}

View File

@ -18,6 +18,7 @@ public protocol SessionMessagingKitStorageProtocol {
func getUserED25519KeyPair() -> Box.KeyPair?
func getUser() -> Contact?
func getAllContacts() -> Set<Contact>
func getAllContacts(with transaction: YapDatabaseReadTransaction) -> Set<Contact>
// MARK: - Closed Groups

View File

@ -88,6 +88,9 @@ public final class NotificationServiceExtension : UNNotificationServiceExtension
guard numMessageRequests == 0 else { return self.completeSilenty() }
}
else if thread.isMessageRequest() && CurrentAppContext().appUserDefaults()[.hasHiddenMessageRequests] {
// If there are other interactions on this thread already then don't show the notification
if thread.numberOfInteractions() > 1 { return }
CurrentAppContext().appUserDefaults()[.hasHiddenMessageRequests] = false
}