From 11f6db21cfe4b392591bb4ed284bbbb7261ff978 Mon Sep 17 00:00:00 2001 From: Jordan Rose Date: Mon, 28 Mar 2022 17:29:04 -0700 Subject: [PATCH] ConversationView: Remove improper cache of DisplayableTexts The main effect of this is that mentions should now get their names properly updated when the corresponding profile is fetched, fixing the situation where someone just added to a group shows up as "Unknown" or as their phone number until some point in the future. (The "Unknown" can still flicker in here, but at least it will get fixed, just like the top line with the sender name.) This does remove a layer of caching, but as noted in the now-deleted code, there's already a *higher* level of caching for an entire CVComponentState, and I verified that we're not actually hitting the deleted cache more than once while a conversation is open. The higher-level cache already knows that some kinds of reloads require rebuilding the displayed content, so removing this cache was sufficient to get things to work. --- .../CV/CVComponentState.swift | 101 +++++------------- 1 file changed, 29 insertions(+), 72 deletions(-) diff --git a/Signal/src/ViewControllers/ConversationView/CV/CVComponentState.swift b/Signal/src/ViewControllers/ConversationView/CV/CVComponentState.swift index 66084162e8..7e069219cc 100644 --- a/Signal/src/ViewControllers/ConversationView/CV/CVComponentState.swift +++ b/Signal/src/ViewControllers/ConversationView/CV/CVComponentState.swift @@ -1121,15 +1121,10 @@ public extension CVComponentState { ranges: MessageBodyRanges?, interaction: TSInteraction, transaction: SDSAnyReadTransaction) -> DisplayableText { - - let cacheKey = "body-\(interaction.uniqueId)" - let mentionStyle: Mention.Style = (interaction as? TSOutgoingMessage != nil) ? .outgoing : .incoming - return Self.displayableText(cacheKey: cacheKey, - mentionStyle: mentionStyle, - transaction: transaction) { - MessageBody(text: text, ranges: ranges ?? .empty) - } + return DisplayableText.displayableText(withMessageBody: MessageBody(text: text, ranges: ranges ?? .empty), + mentionStyle: mentionStyle, + transaction: transaction) } static func displayableBodyText(oversizeTextAttachment attachmentStream: TSAttachmentStream, @@ -1137,31 +1132,28 @@ public extension CVComponentState { interaction: TSInteraction, transaction: SDSAnyReadTransaction) -> DisplayableText { - let cacheKey = "oversize-body-\(interaction.uniqueId)" - - let mentionStyle: Mention.Style = (interaction as? TSOutgoingMessage != nil) ? .outgoing : .incoming - return Self.displayableText(cacheKey: cacheKey, - mentionStyle: mentionStyle, - transaction: transaction) { - let text = { () -> String in - do { - guard let url = attachmentStream.originalMediaURL else { - owsFailDebug("Missing originalMediaURL.") - return "" - } - let data = try Data(contentsOf: url) - guard let string = String(data: data, encoding: .utf8) else { - owsFailDebug("Couldn't parse oversize text.") - return "" - } - return string - } catch { - owsFailDebug("Couldn't load oversize text: \(error).") + let text = { () -> String in + do { + guard let url = attachmentStream.originalMediaURL else { + owsFailDebug("Missing originalMediaURL.") return "" } - }() - return MessageBody(text: text, ranges: ranges ?? .empty) - } + let data = try Data(contentsOf: url) + guard let string = String(data: data, encoding: .utf8) else { + owsFailDebug("Couldn't parse oversize text.") + return "" + } + return string + } catch { + owsFailDebug("Couldn't load oversize text: \(error).") + return "" + } + }() + + let mentionStyle: Mention.Style = (interaction as? TSOutgoingMessage != nil) ? .outgoing : .incoming + return DisplayableText.displayableText(withMessageBody: MessageBody(text: text, ranges: ranges ?? .empty), + mentionStyle: mentionStyle, + transaction: transaction) } } @@ -1173,52 +1165,17 @@ fileprivate extension CVComponentState { ranges: MessageBodyRanges?, interaction: TSInteraction, transaction: SDSAnyReadTransaction) -> DisplayableText { - - let cacheKey = "quoted-\(interaction.uniqueId)" - - let mentionStyle: Mention.Style = .quotedReply - return Self.displayableText(cacheKey: cacheKey, - mentionStyle: mentionStyle, - transaction: transaction) { - MessageBody(text: text, ranges: ranges ?? .empty) - } + return DisplayableText.displayableText(withMessageBody: MessageBody(text: text, ranges: ranges ?? .empty), + mentionStyle: .quotedReply, + transaction: transaction) } static func displayableCaption(text: String, attachmentId: String, transaction: SDSAnyReadTransaction) -> DisplayableText { - - let cacheKey = "attachment-caption-\(attachmentId)" - - let mentionStyle: Mention.Style = .incoming - return CVComponentState.displayableText(cacheKey: cacheKey, - mentionStyle: mentionStyle, - transaction: transaction) { - MessageBody(text: text, ranges: .empty) - } - } - - // TODO: Now that we're caching the displayable text on the view items, - // I'm not sure if we still need this cache. - // - // Cache the results for up to 1,000 messages. - private static let displayableTextCache = LRUCache(maxSize: 1000) - - static func displayableText(cacheKey: String, - mentionStyle: Mention.Style, - transaction: SDSAnyReadTransaction, - messageBodyBlock: () -> MessageBody) -> DisplayableText { - owsAssertDebug(!cacheKey.isEmpty) - - if let displayableText = displayableTextCache.object(forKey: cacheKey) { - return displayableText - } - let messageBody = messageBodyBlock() - let displayableText = DisplayableText.displayableText(withMessageBody: messageBody, - mentionStyle: mentionStyle, - transaction: transaction) - displayableTextCache.setObject(displayableText, forKey: cacheKey) - return displayableText + return DisplayableText.displayableText(withMessageBody: MessageBody(text: text, ranges: .empty), + mentionStyle: .incoming, + transaction: transaction) } }