This patch updates `TimelineMetadata` to work on the _remotes_ region
only, excluding the _start_ and the _locals_ regions. It helps to reduce
the risk of inserting items in an incorrect regions.
This patch also removes on more `rfind_event_by_id` usage, which is
nice.
This patch updates `ReadReceiptTimelineUpdate` to work on the _remotes_
region only, excluding the _start_ and the _lcoals_ regions. It helps
to reduce the risk of inserting a `ReadMarker` inside the _start_ or the
_locals_ regions.
This patch updates `DateDividerAdjuster` to work on _remotes_ and
_locals_ regions only, excluding the _start_ region. It helps to reduce
the risk of inserting a `DateDivider` inside the _start_ region.
This patch also uses the new `push_date_divider` method, which provides
a couple of invariants.
This patch defines a new concept in the `Timeline`: Regions.
The `ObservableItems` holds all the invariants about the _position_ of the
items. It defines three regions where items can live:
1. the _start_ region, which can only contain a single `TimelineStart`,
2. the _remotes_ region, which can only contain many `Remote` timeline
items with their decorations (only `DateDivider`s and `ReadMarker`s),
3. the _locals_ region, which can only contain many `Local` timeline items
with their decorations (only `DateDivider`s).
The `iter_all_regions` method allows to iterate over all regions.
`iter_remotes_region` will restrict the iterator over the _remotes_
region, and so on. These iterators provide the absolute indices of the
items, so that it's harder to make mistakes when manipulating the indices of
items with operations like `insert`, `remove`, `replace` etc.
Other methods like `push_local` or `push_date_divider` insert the items
in the correct region, and check a couple of invariants.
This patch implements the `has_local` method on
`ObservableItemsTransaction`, which is way faster than the previous the
previous solution which was to iterate over all items to find at least
one local timeline item.
This patch adds the `push_local` method on `ObservableItemsTransaction`
to add semantics and hardcode the invariant in a single place for the
different timeline items.
This patch adds the `push_timeline_start_if_missing` method on
`ObservableItemsTransaction` to add semantics and hardcode the
invariant in a single place for the different timeline items.
This patch adds the `--generate-link-to-definition`
argument to `rustdoc` for `docs.rs`. This is using
https://github.com/rust-lang/rust/pull/84176 to add links in the source
code page.
If we already have cross-signing details for the owner of the device at the
point we receive the to-device message, we should store that rather than just
the device info.
create a new method `SenderData::from_device` which does the last few steps of
`SenderDataFinder`: turns out we want it elsewhere. Add some tests to test that
functionality in isolation.
- Doc comment for the SQLite-based state store incorrectly referred to
it as a "cryptostore".
- Consistent capitalisation of SQLite.
- Consistent use of indefinite article "an" before SQLite.
- Fix line length.
Imagine we have the following events:
| event_id | room_id | chunk_id | position |
|----------|---------|----------|----------|
| $ev0 | !r0 | 42 | 0 |
| $ev1 | !r0 | 42 | 1 |
| $ev2 | !r0 | 42 | 2 |
| $ev3 | !r0 | 42 | 3 |
| $ev4 | !r0 | 42 | 4 |
`$ev2` has been removed, then we end up in this state:
| event_id | room_id | chunk_id | position |
|----------|---------|----------|----------|
| $ev0 | !r0 | 42 | 0 |
| $ev1 | !r0 | 42 | 1 |
| | | | | <- no more `$ev2`
| $ev3 | !r0 | 42 | 3 |
| $ev4 | !r0 | 42 | 4 |
We need to shift the `position` of `$ev3` and `$ev4` to `position - 1`,
like so:
| event_id | room_id | chunk_id | position |
|----------|---------|----------|----------|
| $ev0 | !r0 | 42 | 0 |
| $ev1 | !r0 | 42 | 1 |
| $ev3 | !r0 | 42 | 2 |
| $ev4 | !r0 | 42 | 3 |
Usually, it boils down to run the following query:
```sql
UPDATE event_chunks
SET position = position - 1
WHERE position > 2 AND …
```
Okay. But `UPDATE` runs on rows in no particular order. It means that
it can update `$ev4` before `$ev3` for example. What happens in this
particular case? The `position` of `$ev4` becomes `3`, however `$ev3`
already has `position = 3`. Because there is a `UNIQUE` constraint
on `(room_id, chunk_id, position)`, it will result in a constraint
violation.
There is **no way** to control the execution order of `UPDATE` in
SQLite. To persuade yourself, try:
```sql
UPDATE event_chunks
SET position = position - 1
FROM (
SELECT event_id
FROM event_chunks
WHERE position > 2 AND …
ORDER BY position ASC
) as ordered
WHERE event_chunks.event_id = ordered.event_id
```
It will fail the same way.
Thus, we have 2 solutions:
1. Remove the `UNIQUE` constraint,
2. Be creative.
The `UNIQUE` constraint is a safe belt. Normally, we have
`event_cache::Deduplicator` that is responsible to ensure there is no
duplicated event. However, relying on this is “fragile” in the sense it
can contain bugs. Relying on the `UNIQUE` constraint from SQLite is more
robust. It's “braces and belt” as we say here.
So. We need to be creative.
Many solutions exist. Amongst the most popular, we see _dropping and
re-creating the index_, which is no-go for us, it's too expensive. I
(@hywan) have adopted the following one:
- Do `position = position - 1` but in the negative space, so
`position = -(position - 1)`. A position cannot be negative; we are
sure it is unique!
- Once all candidate rows are updated, do `position = -position` to move
back to the positive space.
'told you it's gonna be creative.
This solution is a hack, **but** it is a small number of operations, and
we can keep the `UNIQUE` constraint in place.
This patch updates the `test_linked_chunk_remove_item` to handle
6 events. On _my_ system, with _my_ SQLite version, it triggers the
`UNIQUE` constraint violation without the bug fix.
