`MemoryStore` allows duplicate `ChunkIdentifier`'s

[mgoldenberg]

MemoryStore currently relies on RelationalLinkedChunk::apply_updates to manage Update's to its internal data. This function, in turn, relies on RelationalLinkedChunk::insert_chunk when adding new chunks, which does not check whether the given ChunkIdentifier exists already.

To confirm this behavior, one can add the following integration test to EventCacheStoreIntegrationTests.

impl EventCacheStoreIntegrationTests for DynEventCacheStore {
    async fn test_linked_chunk_identifiers_are_unique(&self) {
        self.handle_linked_chunk_updates(
            LinkedChunkId::Room(room_id!("!r0:matrix.org")),
            vec![
                Update::NewItemsChunk { previous: None, new: ChunkIdentifier::new(0), next: None },
                Update::NewItemsChunk { previous: None, new: ChunkIdentifier::new(0), next: None },
            ],
        )
        .await
        .unwrap_err();
    }

    // -- snip --
}

My guess is that this behavior is not desirable. It also happens to be inconsistent with other implementations of EventCacheStore.

[Hywan]

Yes, if we could forbid this behaviour, it could help detecting potential bugs in the code, since this store is used in the test suites.