519c1ca53c
If the load result is contiguous with the already loaded items, we append
rather than replace.
background:
We have 4 load flavors:
- load older (scrolling up)
- load more recent (scrolling down after unload)
- load most recent (initial page load or scroll down button)
- load around (arbitrary jump from search results or tapping quoted reply)
Previously only "load earlier" and "load later" would append their results to
the existing loaded items, trimming if necessary.
"load most recent" and "load around" would not append, instead they'd replace
the existing loaded items, since the loaded items might not be contiguous with
the existing loaded items.
An optimization we have is that when given a load request, we compare to the
already-loaded items to determine which "unfetched" items from the request
actually need to be fetched.
The error: we consulted the already-loaded items for "load most recent" and
"load around" modes, which discard the previously-loaded items to avoid
introducing a discontinuity in case the fetched items are far away in message
history.
Pseudo code example:
// given these already loaded items
alreadyLoaded = [1, 2, 3, 4, 5]
// if we have a search result for message 5, we'll request to "load around" 5
requestLoadedAround(5) ->
// we'll generate a request set around 5
requestSet = (3..<8)
// The problem is here, when we remove already-loaded items
unfetched = requestSet - alreadyLoaded // == [6, 7, 8]
// Since "load around" replaced the loaded set, rather than appending to
// it, we inadverently lost some important items (3, 4, 5) from our
// request set.
alreadyLoaded = unfetched
A simple solution would be to not consider alreadyLoaded when replacing rather than appending -
for "load around" or "load most recent" but not for "load older" and "load more recent".
alreadyLoaded = [1, 2, 3, 4, 5]
requestLoadedAround(5) ->
requestSet = (3..<8)
// fetch everything rather than worry about removing the
// already-fetched-items
unfetched = requestSet
alreadyLoaded = unfetched
When we're making large discontiguous jumps in the conversation history this
behavior of replacing rather than appending is unavoidable, but when doing
short hops it's wasteful to lose the already loaded items.
So now, whether the load be via "load older", "load more recent", "load most
recent", or "load around" - whenever the fetched items are contiguous with the
already loaded items, we append rather than replace.
245 lines
11 KiB
Swift
245 lines
11 KiB
Swift
//
|
|
// Copyright (c) 2020 Open Whisper Systems. All rights reserved.
|
|
//
|
|
|
|
import XCTest
|
|
@testable import Signal
|
|
|
|
class ConversationMessageMappingTest: SignalBaseTest {
|
|
|
|
var thread: TSThread!
|
|
var mapping: ConversationMessageMapping!
|
|
var messageFactory: IncomingMessageFactory!
|
|
|
|
override func setUp() {
|
|
super.setUp()
|
|
let thread = ContactThreadFactory().create()
|
|
self.thread = thread
|
|
self.mapping = ConversationMessageMapping(thread: thread)
|
|
self.messageFactory = IncomingMessageFactory()
|
|
messageFactory.threadCreator = { _ in return thread }
|
|
}
|
|
|
|
func test_loadInitialMessagePage_empty() throws {
|
|
try read { transaction in
|
|
try self.mapping.loadInitialMessagePage(focusMessageId: nil, transaction: transaction)
|
|
}
|
|
|
|
XCTAssertEqual([], mapping.loadedInteractions)
|
|
}
|
|
|
|
func test_loadInitialMessagePage_nonempty() throws {
|
|
let initialMessages = messageFactory.create(count: 5)
|
|
try read { transaction in
|
|
try self.mapping.loadInitialMessagePage(focusMessageId: nil, transaction: transaction)
|
|
}
|
|
XCTAssertEqual(5, mapping.loadedInteractions.count)
|
|
XCTAssertEqual(initialMessages.map { $0.uniqueId }, mapping.loadedInteractions.map { $0.uniqueId })
|
|
}
|
|
|
|
func test_updateAndCalculateDiff_deletes() throws {
|
|
let initialMessages = messageFactory.create(count: 5)
|
|
try read { transaction in
|
|
try self.mapping.loadInitialMessagePage(focusMessageId: nil, transaction: transaction)
|
|
}
|
|
|
|
let removedMessages = [initialMessages[0], initialMessages[2]]
|
|
write { transaction in
|
|
for message in removedMessages {
|
|
message.anyRemove(transaction: transaction)
|
|
}
|
|
}
|
|
XCTAssertEqual(initialMessages.map { $0.uniqueId }, mapping.loadedInteractions.map { $0.uniqueId })
|
|
|
|
let remainingMessages = [initialMessages[1], initialMessages[3], initialMessages[4]]
|
|
let deletedInteractionIds: Set<String> = Set(removedMessages.map { $0.uniqueId })
|
|
let diff: ConversationMessageMapping.ConversationMessageMappingDiff = try read { transaction in
|
|
return try self.mapping.updateAndCalculateDiff(updatedInteractionIds: deletedInteractionIds, transaction: transaction)
|
|
}
|
|
XCTAssertEqual(remainingMessages.map { $0.uniqueId }, mapping.loadedInteractions.map { $0.uniqueId })
|
|
|
|
XCTAssertEqual([], diff.addedItemIds)
|
|
XCTAssertEqual([], diff.updatedItemIds)
|
|
XCTAssertEqual(deletedInteractionIds, diff.removedItemIds)
|
|
}
|
|
|
|
func test_updateAndCalculateDiff_inserts() throws {
|
|
let initialMessages = messageFactory.create(count: 2)
|
|
try read { transaction in
|
|
try self.mapping.loadInitialMessagePage(focusMessageId: nil, transaction: transaction)
|
|
}
|
|
|
|
let insertedMessages = messageFactory.create(count: 3)
|
|
let insertedIds = Set(insertedMessages.map { $0.uniqueId})
|
|
let diff: ConversationMessageMapping.ConversationMessageMappingDiff = try read { transaction in
|
|
return try self.mapping.updateAndCalculateDiff(updatedInteractionIds: insertedIds,
|
|
transaction: transaction)
|
|
}
|
|
XCTAssertEqual((initialMessages + insertedMessages).map { $0.uniqueId }, mapping.loadedInteractions.map { $0.uniqueId })
|
|
|
|
XCTAssertEqual(insertedIds, diff.addedItemIds)
|
|
XCTAssertEqual([], diff.updatedItemIds)
|
|
XCTAssertEqual([], diff.removedItemIds)
|
|
}
|
|
|
|
func test_updateAndCalculateDiff_updates() throws {
|
|
let initialMessages = messageFactory.create(count: 5)
|
|
try read { transaction in
|
|
try self.mapping.loadInitialMessagePage(focusMessageId: nil, transaction: transaction)
|
|
}
|
|
|
|
let updatedMessages = [initialMessages[1], initialMessages[2]]
|
|
|
|
write { transaction in
|
|
for message in updatedMessages {
|
|
// This write is actually not necessary for the test to succeed, since we're manually
|
|
// passing in `updatedInteractionIds` rather than observing db changes in this unit test,
|
|
// "marking as read" here only documents an example of what we mean by "updated".
|
|
message.debugonly_markAsReadNow(transaction: transaction)
|
|
}
|
|
}
|
|
|
|
let updatedIds = Set(updatedMessages.map { $0.uniqueId })
|
|
let diff: ConversationMessageMapping.ConversationMessageMappingDiff = try read { transaction in
|
|
return try self.mapping.updateAndCalculateDiff(updatedInteractionIds: updatedIds,
|
|
transaction: transaction)
|
|
}
|
|
XCTAssertEqual(initialMessages.map { $0.uniqueId }, mapping.loadedInteractions.map { $0.uniqueId })
|
|
|
|
XCTAssertEqual([], diff.addedItemIds)
|
|
XCTAssertEqual(updatedIds, diff.updatedItemIds)
|
|
XCTAssertEqual([], diff.removedItemIds)
|
|
}
|
|
|
|
func test_updateAndCalculateDiff_mixed_updates() throws {
|
|
let initialMessages = messageFactory.create(count: 4)
|
|
try read { transaction in
|
|
try self.mapping.loadInitialMessagePage(focusMessageId: nil, transaction: transaction)
|
|
}
|
|
|
|
let removedMessages = [initialMessages[0], initialMessages[3]]
|
|
let updatedMessages = [initialMessages[1], initialMessages[2]]
|
|
let insertedMessage: TSIncomingMessage = write { transaction in
|
|
for message in removedMessages {
|
|
message.anyRemove(transaction: transaction)
|
|
}
|
|
for message in updatedMessages {
|
|
// This write is actually not necessary for the test to succeed, since we're manually
|
|
// passing in `updatedInteractionIds` rather than observing db changes in this unit test,
|
|
// "marking as read" here only documents an example of what we mean by "updated".
|
|
message.debugonly_markAsReadNow(transaction: transaction)
|
|
}
|
|
return self.messageFactory.create(transaction: transaction)
|
|
}
|
|
|
|
let updatedIds = Set((updatedMessages + removedMessages + [insertedMessage]).map { $0.uniqueId })
|
|
let diff: ConversationMessageMapping.ConversationMessageMappingDiff = try read { transaction in
|
|
return try self.mapping.updateAndCalculateDiff(updatedInteractionIds: updatedIds,
|
|
transaction: transaction)
|
|
}
|
|
|
|
let remainingMessages = [initialMessages[1], initialMessages[2], insertedMessage]
|
|
XCTAssertEqual(remainingMessages.map { $0.uniqueId }, mapping.loadedInteractions.map { $0.uniqueId })
|
|
|
|
XCTAssertEqual([insertedMessage.uniqueId], diff.addedItemIds)
|
|
XCTAssertEqual(Set(updatedMessages.map { $0.uniqueId }), diff.updatedItemIds)
|
|
XCTAssertEqual(Set(removedMessages.map { $0.uniqueId }), diff.removedItemIds)
|
|
}
|
|
|
|
func test_updateAndCalculateDiff_removeAll() throws {
|
|
let initialMessages = messageFactory.create(count: 5)
|
|
try read { transaction in
|
|
try self.mapping.loadInitialMessagePage(focusMessageId: nil, transaction: transaction)
|
|
}
|
|
|
|
XCTAssert(!mapping.canLoadNewer)
|
|
XCTAssert(!mapping.canLoadOlder)
|
|
|
|
write { transaction in
|
|
for message in initialMessages {
|
|
message.anyRemove(transaction: transaction)
|
|
}
|
|
}
|
|
|
|
let removedIds = Set(initialMessages.map { $0.uniqueId })
|
|
let diff: ConversationMessageMapping.ConversationMessageMappingDiff = try read { transaction in
|
|
return try self.mapping.updateAndCalculateDiff(updatedInteractionIds: removedIds,
|
|
transaction: transaction)
|
|
}
|
|
|
|
XCTAssertEqual([], mapping.loadedInteractions)
|
|
|
|
XCTAssertEqual([], diff.addedItemIds)
|
|
XCTAssertEqual([], diff.updatedItemIds)
|
|
XCTAssertEqual(removedIds, diff.removedItemIds)
|
|
}
|
|
|
|
func test_updateAndCalculateDiff_fix_crash() throws {
|
|
let initialLoadCount = mapping.initialLoadCount
|
|
|
|
let initialMessages: [TSIncomingMessage] = write { transaction in
|
|
// create more messages than the initial load window can fit
|
|
let createdMessages = self.messageFactory.create(count: UInt(initialLoadCount + 1), transaction: transaction)
|
|
|
|
// mark as read so that we initially load the bottom of the conversation
|
|
for message in createdMessages {
|
|
message.debugonly_markAsReadNow(transaction: transaction)
|
|
}
|
|
|
|
return createdMessages
|
|
}
|
|
|
|
try read { transaction in
|
|
try self.mapping.loadInitialMessagePage(focusMessageId: nil, transaction: transaction)
|
|
}
|
|
let initiallyLoadedInteractions = mapping.loadedInteractions
|
|
XCTAssertEqual(initialLoadCount, initiallyLoadedInteractions.count)
|
|
XCTAssert(!mapping.canLoadNewer)
|
|
XCTAssert(mapping.canLoadOlder)
|
|
|
|
write { transaction in
|
|
for message in initialMessages {
|
|
message.anyRemove(transaction: transaction)
|
|
}
|
|
}
|
|
|
|
let removedIds = Set(initialMessages.map { $0.uniqueId })
|
|
let diff: ConversationMessageMapping.ConversationMessageMappingDiff = try read { transaction in
|
|
return try self.mapping.updateAndCalculateDiff(updatedInteractionIds: removedIds,
|
|
transaction: transaction)
|
|
}
|
|
|
|
XCTAssertEqual([], mapping.loadedInteractions)
|
|
|
|
XCTAssertEqual([], diff.addedItemIds)
|
|
XCTAssertEqual([], diff.updatedItemIds)
|
|
XCTAssertEqual(Set(initiallyLoadedInteractions.map { $0.uniqueId }), diff.removedItemIds)
|
|
}
|
|
|
|
func test_loadAroundEdge() throws {
|
|
let initialMessages: [TSIncomingMessage] = write { transaction in
|
|
// create more messages than the initial load window can fit
|
|
let createdMessages = self.messageFactory.create(count: UInt(self.mapping.initialLoadCount * 2), transaction: transaction)
|
|
|
|
// mark as read so that we initially load the bottom of the conversation
|
|
for message in createdMessages {
|
|
message.debugonly_markAsReadNow(transaction: transaction)
|
|
}
|
|
|
|
return createdMessages
|
|
}
|
|
|
|
for message in initialMessages {
|
|
self.mapping = ConversationMessageMapping(thread: thread)
|
|
try read { transaction in
|
|
try self.mapping.loadInitialMessagePage(focusMessageId: nil, transaction: transaction)
|
|
try self.mapping.loadMessagePage(aroundInteractionId: message.uniqueId, transaction: transaction)
|
|
}
|
|
guard (mapping.loadedInteractions.map { $0.uniqueId }.contains(message.uniqueId)) else {
|
|
XCTFail("message not loaded: \(message)")
|
|
return
|
|
}
|
|
}
|
|
}
|
|
}
|