diff --git a/src/matrix/room/timeline/persistence/GapWriter.js b/src/matrix/room/timeline/persistence/GapWriter.js index d51f4a18da..b040e3a182 100644 --- a/src/matrix/room/timeline/persistence/GapWriter.js +++ b/src/matrix/room/timeline/persistence/GapWriter.js @@ -26,47 +26,28 @@ export class GapWriter { this._fragmentIdComparer = fragmentIdComparer; this._relationWriter = relationWriter; } + + async _pickNeighbourFragment(txn, duplcateEventIds) { + const duplicateEventId = duplcateEventIds[0]; + const duplicateEvent = await txn.timelineEvents.getByEventId(this._roomId, duplicateEventId); + return duplicateEvent.fragmentId; + } + // events is in reverse-chronological order (last event comes at index 0) if backwards async _findOverlappingEvents(fragmentEntry, events, txn, log) { - let expectedOverlappingEventId; - if (fragmentEntry.hasLinkedFragment) { - expectedOverlappingEventId = await this._findExpectedOverlappingEventId(fragmentEntry, txn); - } - let remainingEvents = events; - let nonOverlappingEvents = []; + const eventIds = events.map(e => e.event_id); + const duplicateEventIds = await txn.timelineEvents.findOccurringEventIds(this._roomId, eventIds); + const nonOverlappingEvents = events.filter(e => !duplicateEventIds.includes(e.event_id)); let neighbourFragmentEntry; - while (remainingEvents && remainingEvents.length) { - const eventIds = remainingEvents.map(e => e.event_id); - const duplicateEventId = await txn.timelineEvents.findFirstOccurringEventId(this._roomId, eventIds); - if (duplicateEventId) { - const duplicateEventIndex = remainingEvents.findIndex(e => e.event_id === duplicateEventId); - // should never happen, just being defensive as this *can't* go wrong - if (duplicateEventIndex === -1) { - throw new Error(`findFirstOccurringEventId returned ${duplicateEventIndex} which wasn't ` + - `in [${eventIds.join(",")}] in ${this._roomId}`); - } - nonOverlappingEvents.push(...remainingEvents.slice(0, duplicateEventIndex)); - if (!expectedOverlappingEventId || duplicateEventId === expectedOverlappingEventId) { - // TODO: check here that the neighbourEvent is at the correct edge of it's fragment - // get neighbour fragment to link it up later on - const neighbourEvent = await txn.timelineEvents.getByEventId(this._roomId, duplicateEventId); - if (neighbourEvent.fragmentId === fragmentEntry.fragmentId) { - log.log("hit #160, prevent fragment linking to itself", log.level.Warn); - } else { - const neighbourFragment = await txn.timelineFragments.get(this._roomId, neighbourEvent.fragmentId); - neighbourFragmentEntry = fragmentEntry.createNeighbourEntry(neighbourFragment); - } - // trim overlapping events - remainingEvents = null; - } else { - // we've hit https://github.com/matrix-org/synapse/issues/7164, - // e.g. the event id we found is already in our store but it is not - // the adjacent fragment id. Ignore the event, but keep processing the ones after. - remainingEvents = remainingEvents.slice(duplicateEventIndex + 1); - } + if (eventIds.length && eventIds.length === duplicateEventIds.length) { + // TODO: check here that the neighbourEvent is at the correct edge of it's fragment + // get neighbour fragment to link it up later on + const neighbourFragmentId = await this._pickNeighbourFragment(txn, duplicateEventIds); + if (neighbourFragmentId === fragmentEntry.fragmentId) { + log.log("hit #160, prevent fragment linking to itself", log.level.Warn); } else { - nonOverlappingEvents.push(...remainingEvents); - remainingEvents = null; + const neighbourFragment = await txn.timelineFragments.get(this._roomId, neighbourFragmentId); + neighbourFragmentEntry = fragmentEntry.createNeighbourEntry(neighbourFragment); } } return {nonOverlappingEvents, neighbourFragmentEntry}; @@ -330,6 +311,10 @@ export function tests() { return txn.timelineFragments.get(roomId, fragmentId); } + async function updatedFragmentEntry(mock, fragmentEntry) { + return fragmentEntry.withUpdatedFragment(await fetchFragment(mock, fragmentEntry.fragmentId)); + } + function assertFilledLink(assert, fragment1, fragment2) { assert.equal(fragment1.nextId, fragment2.id); assert.equal(fragment2.previousId, fragment1.id); @@ -361,7 +346,9 @@ export function tests() { const {syncResponse, fragmentEntry: firstFragmentEntry} = await syncAndWrite(mocks, { limit: 10 }); timelineMock.append(15); const {fragmentEntry: secondFragmentEntry} = await syncAndWrite(mocks, { previous: syncResponse, limit: 10 }); + // Only the second backfill (in which all events overlap) fills the gap. await backfillAndWrite(mocks, secondFragmentEntry, 10); + await backfillAndWrite(mocks, await updatedFragmentEntry(mocks, secondFragmentEntry), 10); const firstFragment = await fetchFragment(mocks, firstFragmentEntry.fragmentId); const secondFragment = await fetchFragment(mocks, secondFragmentEntry.fragmentId); @@ -413,7 +400,9 @@ export function tests() { timelineMock.append(11); const {fragmentEntry: secondFragmentEntry} = await syncAndWrite(mocks, { previous: syncResponse, limit: 10 }); timelineMock.insertAfter(eventId(9), 5); + // Only the second backfill (in which all events overlap) fills the gap. await backfillAndWrite(mocks, secondFragmentEntry, 10); + await backfillAndWrite(mocks, await updatedFragmentEntry(mocks, secondFragmentEntry), 10); const firstEvents = await allFragmentEvents(mocks, firstFragmentEntry.fragmentId); assert.deepEqual(firstEvents.map(e => e.event_id), eventIds(0, 10)); diff --git a/src/matrix/storage/idb/stores/TimelineEventStore.ts b/src/matrix/storage/idb/stores/TimelineEventStore.ts index 3d27e8a78c..34165d9e12 100644 --- a/src/matrix/storage/idb/stores/TimelineEventStore.ts +++ b/src/matrix/storage/idb/stores/TimelineEventStore.ts @@ -234,31 +234,23 @@ export class TimelineEventStore { // In that case we could avoid running over all eventIds, as the reported order by findExistingKeys // would match the order of eventIds. That's why findLast is also passed as backwards to keysExist. // also passing them in chronological order makes sense as that's how we'll receive them almost always. - async findFirstOccurringEventId(roomId: string, eventIds: string[]): Promise { + async findOccurringEventIds(roomId: string, eventIds: string[]): Promise { const byEventId = this._timelineStore.index("byEventId"); const keys = eventIds.map(eventId => encodeEventIdKey(roomId, eventId)); const results = new Array(keys.length); - let firstFoundKey: string | undefined; - - // find first result that is found and has no undefined results before it - function firstFoundAndPrecedingResolved(): string | undefined { - for(let i = 0; i < results.length; ++i) { - if (results[i] === undefined) { - return; - } else if(results[i] === true) { - return keys[i]; - } - } - } + const occuringEventIds: string[] = []; await byEventId.findExistingKeys(keys, false, (key, found) => { // T[].search(T, number), but we want T[].search(R, number), so cast const index = (keys as IDBValidKey[]).indexOf(key); results[index] = found; - firstFoundKey = firstFoundAndPrecedingResolved(); - return !!firstFoundKey; + return false; }); - return firstFoundKey && decodeEventIdKey(firstFoundKey).eventId; + for(let i = 0; i < results.length; ++i) { + if (!results[i]) continue; + occuringEventIds.push(eventIds[i]); + } + return occuringEventIds; } /** Inserts a new entry into the store.