Dima/matrix-rust-sdk
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

feat(sdk): Remove hacks for Sliding Sync ranges

Browse Source 
feat(sdk): Remove hacks for Sliding Sync ranges
This commit is contained in:
Ivan Enderlin
2023-04-27 14:35:05 +02:00
committed by GitHub
parent 6ca6a9a84a 3a8b6696f7
commit cbfa134087
3 changed files with 89 additions and 115 deletions
Download Patch File Download Diff File Expand all files Collapse all files
.github/workflows/ci.yml
+1 -1
View File
@@ -436,7 +436,7 @@ jobs:
# run sliding sync and point it at the postgres container and synapse container.
# the postgres container needs to be above this to make sure it has started prior to this service.
slidingsync:
image: "ghcr.io/matrix-org/sliding-sync:v0.99.0"
image: "ghcr.io/matrix-org/sliding-sync:v0.99.2"
env:
SYNCV3_SERVER: "http://synapse:8008"
SYNCV3_SECRET: "SUPER_CI_SECRET"
crates/matrix-sdk/src/sliding_sync/list/mod.rs
+87 -113
View File
@@ -615,7 +615,7 @@ fn apply_sync_operations(
// > knew about entries in this range.
v4::SlidingOp::Sync => {
// Extract `start` and `end` from the operation's range.
let (start, mut end) = operation
let (start, end) = operation
.range
.ok_or_else(|| {
Error::BadResponse(
@@ -629,23 +629,6 @@ fn apply_sync_operations(
)
})?;
// The `end` bound of the range might not be correct… At the time of writing,
// there is a bug in the Sliding Sync Proxy that can return
// ranges greater than the `room_list` size.
//
// For example, if the client asks for a range `0..=9`, and there is only one
// room, the server will reply with one `room_id` (which is correct) but with
// the range `0..=9` instead of `0..=0`.
//
// So, a safe workaround is to take the minimum between `end` and the
// `room_list`'s length.
//
// The “safety” is ensured by the fact we also compare the size of the new range
// with the size of the `operation.room_ids` length later on.
//
// See https://github.com/matrix-org/sliding-sync/issues/52.
end = min(end, room_list.len());
// Range is invalid.
if start > end {
return Err(Error::BadResponse(format!(
@@ -654,7 +637,16 @@ fn apply_sync_operations(
)));
}
let mut room_entry_range = start..end;
// Range is too big.
if end > room_list.len() {
return Err(Error::BadResponse(format!(
"`range` is out of the `rooms_list`'s bounds ({} > {})",
end,
room_list.len(),
)));
}
let room_entry_range = start..end;
// `room_ids` is absent.
if operation.room_ids.is_empty() {
@@ -667,18 +659,6 @@ fn apply_sync_operations(
// Mismatch between the `range` and `room_ids`.
if room_entry_range.len() != room_ids.len() {
// Because of https://github.com/matrix-org/sliding-sync/issues/52, we
// can't trust the `range` returned by the server. That's a
// problem. Let's try to work around
// that.
//
// Let's pretend the `start` bound of the range is… correct.
room_entry_range = start..room_ids.len();
// Once the bug is fixed on the Sliding Sync Proxy side, we
// can remove this code, and uncomment
// the code below.
/*
return Err(Error::BadResponse(
format!(
"There is a mismatch between the number of items in `range` and `room_ids` ({} != {})",
@@ -686,7 +666,6 @@ fn apply_sync_operations(
room_ids.len(),
)
));
*/
}
// Update parts `room_list`.
@@ -783,7 +762,7 @@ fn apply_sync_operations(
// > arrive from the server.
v4::SlidingOp::Invalidate => {
// Extract `start` and `end` from the operation's range.
let (start, mut end) = operation
let (start, end) = operation
.range
.ok_or_else(|| {
Error::BadResponse(
@@ -797,23 +776,6 @@ fn apply_sync_operations(
)
})?;
// The `end` bound of the range might not be correct… At the time of writing,
// there is a bug in the Sliding Sync Proxy that can return
// ranges greater than the `room_list` size.
//
// For example, if the client asks for a range `0..=9`, and there is only one
// room, the server will reply with one `room_id` (which is correct) but with
// the range `0..=9` instead of `0..=0`.
//
// So, a safe workaround is to take the minimum between `end` and the
// `room_list`'s length.
//
// The “safety” is ensured by the fact we also compare the size of the new range
// with the size of the `operation.room_ids` length later on.
//
// See https://github.com/matrix-org/sliding-sync/issues/52.
end = min(end, room_list.len());
// Range is invalid.
if start > end {
return Err(Error::BadResponse(format!(
@@ -822,6 +784,15 @@ fn apply_sync_operations(
)));
}
// Range is too big.
if end > room_list.len() {
return Err(Error::BadResponse(format!(
"`range` is out of the `room_list`' bounds ({} > {})",
end,
room_list.len(),
)));
}
let room_entry_range = start..end;
// Invalidate parts of `room_list`.
@@ -1781,6 +1752,7 @@ mod tests {
macro_rules! assert_sync_operations {
(
$assert_description:literal :
room_list = [ $( $room_list_entries:tt )* ],
sync_operations = [
$(
@@ -1817,8 +1789,13 @@ mod tests {
let result = apply_sync_operations(operations, &mut room_list, &mut rooms_that_have_received_an_update);
assert!(result.$result());
assert_eq!(*room_list, entries![ $( $expected_room_list_entries )* ]);
assert!(result.$result(), "{}; assert the `Result`", $assert_description);
assert_eq!(
*room_list,
entries![ $( $expected_room_list_entries )* ],
"{}; asserting the `room_list`",
$assert_description,
);
$(
#[allow(unused_mut)]
@@ -1830,15 +1807,20 @@ mod tests {
)*
}
assert_eq!(rooms_that_have_received_an_update, expected_rooms_that_have_received_an_update);
assert_eq!(
rooms_that_have_received_an_update,
expected_rooms_that_have_received_an_update,
"{}; asserting the rooms that have received an update",
$assert_description,
);
)?
};
}
#[test]
fn test_sync_operations_sync() {
// All room list is updated.
assert_sync_operations! {
"All room list is updated":
room_list = [E, E, E, F("!r3:x.y")],
sync_operations = [
{
@@ -1854,8 +1836,8 @@ mod tests {
rooms = ["!r3:x.y"],
};
// Partial update.
assert_sync_operations! {
"Partial update":
room_list = [E, E, E],
sync_operations = [
{
@@ -1868,7 +1850,9 @@ mod tests {
result = is_ok,
room_list = [F("!r0:x.y"), F("!r1:x.y"), E],
};
assert_sync_operations! {
"Partial update":
room_list = [E, E, E],
sync_operations = [
{
@@ -1882,26 +1866,23 @@ mod tests {
room_list = [E, F("!r1:x.y"), F("!r2:x.y")],
};
// The range returned by the server is too large compared to the `room_ids` but
// we can fix it on-the-fly.
//
// See https://github.com/matrix-org/sliding-sync/issues/52.
assert_sync_operations! {
"The range returned by the server is too large compared to the `room_ids`":
room_list = [E],
sync_operations = [
{
"op": SlidingOp::Sync,
"range": [0, 9], // <- it should be [0, 0]
"range": [0, 2], // <- it should be [0, 0]
"room_ids": ["!r0:x.y"],
}
]
=>
result = is_ok, // <- because we have fixed it
room_list = [F("!r0:x.y")],
result = is_err,
room_list = [E],
};
// Missing `range`.
assert_sync_operations! {
"Missing `range`":
room_list = [E, E, E],
sync_operations = [
{
@@ -1914,8 +1895,8 @@ mod tests {
room_list = [E, E, E],
};
// Invalid `range`.
assert_sync_operations! {
"Invalid `range`":
room_list = [E, E, E],
sync_operations = [
{
@@ -1929,8 +1910,8 @@ mod tests {
room_list = [E, E, E],
};
// Missing `room_ids`.
assert_sync_operations! {
"Missing `room_ids`":
room_list = [E, E, E],
sync_operations = [
{
@@ -1943,9 +1924,9 @@ mod tests {
room_list = [E, E, E],
};
// Out of bounds operation.
assert_sync_operations! {
room_list = [E, E, E],
"Out of bounds operation":
room_list = [E, F("!r1:x.y"), E],
sync_operations = [
{
"op": SlidingOp::Sync,
@@ -1954,19 +1935,12 @@ mod tests {
}
]
=>
// As soon https://github.com/matrix-org/sliding-sync/issues/52
// is fixed, let's uncomment the real test.
result = is_ok,
room_list = [E, E, E],
/*
result = is_err,
room_list = [E, E, E],
*/
room_list = [E, F("!r1:x.y"), E],
};
// The server replies with a particular range, but some room IDs are
// missing.
assert_sync_operations! {
"The server replies with a particular range, but some room IDs are missing":
room_list = [E, E, E],
sync_operations = [
{
@@ -1976,19 +1950,12 @@ mod tests {
}
]
=>
// As soon https://github.com/matrix-org/sliding-sync/issues/52
// is fixed, let's uncomment the real test.
result = is_ok,
room_list = [F("!r0:x.y"), E, E],
/*
result = is_err,
room_list = [E, E, E],
*/
};
// The server replies with a particular range, but there is too much
// room IDs.
assert_sync_operations! {
"The server replies with a particular range, but there is too much room IDs":
room_list = [E, E, E],
sync_operations = [
{
@@ -1998,21 +1965,15 @@ mod tests {
}
]
=>
// As soon https://github.com/matrix-org/sliding-sync/issues/52
// is fixed, let's uncomment the real test.
result = is_ok,
room_list = [F("!r0:x.y"), F("!r1:x.y"), F("!extra:x.y")],
/*
result = is_err,
room_list = [E, E, E],
*/
};
}
#[test]
fn test_sync_operations_delete() {
// Delete a room entry in the middle.
assert_sync_operations! {
"Delete a room entry in the middle":
room_list = [F("!r0:x.y"), F("!r1:x.y"), F("!r2:x.y")],
sync_operations = [
{
@@ -2027,8 +1988,8 @@ mod tests {
rooms = ["!r0:x.y"],
};
// Delete a room entry at the beginning.
assert_sync_operations! {
"Delete a room entry at the beginning":
room_list = [F("!r0:x.y"), F("!r1:x.y"), F("!r2:x.y")],
sync_operations = [
{
@@ -2041,8 +2002,8 @@ mod tests {
room_list = [F("!r1:x.y"), F("!r2:x.y")],
};
// Delete a room entry at the end.
assert_sync_operations! {
"Delete a room entry at the end":
room_list = [F("!r0:x.y"), F("!r1:x.y"), F("!r2:x.y")],
sync_operations = [
{
@@ -2055,8 +2016,8 @@ mod tests {
room_list = [F("!r0:x.y"), F("!r1:x.y")],
};
// Delete an out of bounds room entry.
assert_sync_operations! {
"Delete an out of bounds room entry":
room_list = [F("!r0:x.y"), F("!r1:x.y"), F("!r2:x.y")],
sync_operations = [
{
@@ -2072,8 +2033,8 @@ mod tests {
#[test]
fn test_sync_operations_insert() {
// Insert a room entry in the middle.
assert_sync_operations! {
"Insert a room entry in the middle":
room_list = [E, E, E],
sync_operations = [
{
@@ -2089,8 +2050,8 @@ mod tests {
rooms = ["!r0:x.y"],
};
// Insert a room entry at the beginning.
assert_sync_operations! {
"Insert a room entry at the beginning":
room_list = [E, E, E],
sync_operations = [
{
@@ -2104,8 +2065,8 @@ mod tests {
room_list = [F("!r0:x.y"), E, E, E],
};
// Insert a room entry at the end
assert_sync_operations! {
"Insert a room entry at the end":
room_list = [E, E, E],
sync_operations = [
{
@@ -2119,8 +2080,8 @@ mod tests {
room_list = [E, E, E, F("!r3:x.y")],
};
// Insert an out of bounds room entry.
assert_sync_operations! {
"Insert an out of bounds room entry":
room_list = [E, F("!r1:x.y"), E],
sync_operations = [
{
@@ -2137,8 +2098,8 @@ mod tests {
#[test]
fn test_sync_operations_invalidate() {
// Invalidating an empty room.
assert_sync_operations! {
"Invalidating an empty room":
room_list = [E, F("!r1:x.y")],
sync_operations = [
{
@@ -2153,8 +2114,8 @@ mod tests {
rooms = ["!r1:x.y"],
};
// Invalidating a filled room.
assert_sync_operations! {
"Invalidating a filled room":
room_list = [F("!r0:x.y"), F("!r1:x.y")],
sync_operations = [
{
@@ -2169,8 +2130,8 @@ mod tests {
rooms = ["!r1:x.y"],
};
// Invalidating an invalidated room.
assert_sync_operations! {
"Invalidating an invalidated room":
room_list = [I("!r0:x.y"), F("!r1:x.y")],
sync_operations = [
{
@@ -2185,8 +2146,8 @@ mod tests {
rooms = ["!r1:x.y"],
};
// Partial update.
assert_sync_operations! {
"Partial update from the beginning":
room_list = [F("!r0:x.y"), F("!r1:x.y"), F("!r2:x.y")],
sync_operations = [
{
@@ -2198,7 +2159,9 @@ mod tests {
result = is_ok,
room_list = [I("!r0:x.y"), I("!r1:x.y"), F("!r2:x.y")],
};
assert_sync_operations! {
"Partial update from the end":
room_list = [F("!r0:x.y"), F("!r1:x.y"), F("!r2:x.y")],
sync_operations = [
{
@@ -2211,8 +2174,8 @@ mod tests {
room_list = [F("!r0:x.y"), I("!r1:x.y"), I("!r2:x.y")],
};
// Full update.
assert_sync_operations! {
"Full update":
room_list = [F("!r0:x.y"), F("!r1:x.y"), F("!r2:x.y")],
sync_operations = [
{
@@ -2225,11 +2188,8 @@ mod tests {
room_list = [I("!r0:x.y"), I("!r1:x.y"), I("!r2:x.y")],
};
// The range returned by the server is too large compared to the `room_lists`
// but we can fix it on-the-fly.
//
// See https://github.com/matrix-org/sliding-sync/issues/52.
assert_sync_operations! {
"The range returned by the server is too large compared to the `room_lists`":
room_list = [F("!r0:x.y")],
sync_operations = [
{
@@ -2238,16 +2198,16 @@ mod tests {
}
]
=>
result = is_ok, // <- because we have fixed it
room_list = [I("!r0:x.y")],
result = is_err,
room_list = [F("!r0:x.y")],
};
// Missing `range`.
assert_sync_operations! {
"Missing `range`":
room_list = [F("!r0:x.y"), F("!r1:x.y"), F("!r2:x.y")],
sync_operations = [
{
"op": SlidingOp::Invalidate,
"op": SlidingOp::Delete,
}
]
=>
@@ -2255,12 +2215,12 @@ mod tests {
room_list = [F("!r0:x.y"), F("!r1:x.y"), F("!r2:x.y")],
};
// Invalid `range`.
assert_sync_operations! {
"Invalid `range`":
room_list = [F("!r0:x.y"), F("!r1:x.y"), F("!r2:x.y")],
sync_operations = [
{
"op": SlidingOp::Invalidate,
"op": SlidingOp::Delete,
"range": [12, 0],
}
]
@@ -2268,5 +2228,19 @@ mod tests {
result = is_err,
room_list = [F("!r0:x.y"), F("!r1:x.y"), F("!r2:x.y")],
};
assert_sync_operations! {
"Out of bounds operation":
room_list = [F("!r0:x.y"), F("!r1:x.y"), F("!r2:x.y")],
sync_operations = [
{
"op": SlidingOp::Delete,
"range": [2, 3],
}
]
=>
result = is_err,
room_list = [F("!r0:x.y"), F("!r1:x.y"), F("!r2:x.y")],
};
}
}
testing/sliding-sync-integration-test/assets/docker-compose.yml
+1 -1
View File
@@ -28,7 +28,7 @@ services:
- ./data/db:/var/lib/postgresql/data
sliding-sync-proxy:
image: ghcr.io/matrix-org/sliding-sync:v0.99.1
image: ghcr.io/matrix-org/sliding-sync:v0.99.2
depends_on:
postgres:
condition: service_healthy