From 6b0af4dc95dd707043fb74164a0f5c46aa8bccd4 Mon Sep 17 00:00:00 2001 From: Erik Johnston Date: Tue, 28 Jan 2020 17:39:28 +0000 Subject: [PATCH 1/3] Test that fetching device correctly resyncs --- tests/50federation/40devicelists.pl | 91 +++++++++++++++++++++++++++++ 1 file changed, 91 insertions(+) diff --git a/tests/50federation/40devicelists.pl b/tests/50federation/40devicelists.pl index 13d740efc..1b4d33165 100644 --- a/tests/50federation/40devicelists.pl +++ b/tests/50federation/40devicelists.pl @@ -201,6 +201,97 @@ }; +test "Server correctly resyncs when client query keys and there is no remote cache", + requires => [ local_user_fixture(), + $main::INBOUND_SERVER, $main::OUTBOUND_CLIENT, + federation_user_id_fixture(), + room_alias_name_fixture() ], + + check => sub { + my ( $user, $inbound_server, $outbound_client, $creator_id, $room_alias_name ) = @_; + + my ( $room_id ); + + my $remote_server_name = $inbound_server->server_name; + my $datastore = $inbound_server->datastore; + + my $room_alias = "#$room_alias_name:$remote_server_name"; + + # We return two devices, as there was a bug in synapse which correctly + # handled returning one device but not two. + my $device_id1 = "random_device_id1"; + my $device_id2 = "random_device_id2"; + + + # We set up a situation where sytest joins a room with a user without + # relaying any device keys, and then a client of synapse requests the keys + # for that user. This should cause synapse to do a resync and cache those + # keys correctly. + my $room = $datastore->create_room( + creator => $creator_id, + alias => $room_alias, + ); + + do_request_json_for( $user, + method => "POST", + uri => "/r0/join/$room_alias", + + content => {}, + )->then( sub { + Future->needs_all( + $inbound_server->await_request_user_devices( $creator_id ) + ->then( sub { + my ( $req, undef ) = @_; + + assert_eq( $req->method, "GET", 'request method' ); + + $req->respond_json( { + user_id => $creator_id, + stream_id => 1, + devices => [ { + device_id => $device_id1, + + keys => { + device_keys => {} + } + }, { + device_id => $device_id2, + + keys => { + device_keys => {} + } + } ] + } ); + Future->done(1); + }), + do_request_json_for( $user, + method => "POST", + uri => "/unstable/keys/query", + content => { + device_keys => { + $creator_id => [], + } + } + ) + ) + })->then( sub { + my ( $first, $content ) = @_; + + log_if_fail "query response", $content; + + assert_json_keys( $content, "device_keys" ); + + my $device_keys = $content->{device_keys}; + assert_json_keys( $device_keys, $creator_id ); + + my $alice_keys = $device_keys->{ $creator_id }; + assert_json_keys( $alice_keys, ( $device_id1, $device_id2 ) ); + + Future->done( 1 ) + }); + }; + + test "Local device key changes get to remote servers with correct prev_id", requires => [ local_user_fixtures( 2 ), $main::INBOUND_SERVER, federation_user_id_fixture(), room_alias_name_fixture() ], From 44481a0e097fdc970863582ea11fd1c90dbe2956 Mon Sep 17 00:00:00 2001 From: Erik Johnston Date: Wed, 29 Jan 2020 10:09:27 +0000 Subject: [PATCH 2/3] Fixup --- tests/50federation/40devicelists.pl | 95 +++++++++++------------------ 1 file changed, 35 insertions(+), 60 deletions(-) diff --git a/tests/50federation/40devicelists.pl b/tests/50federation/40devicelists.pl index 1b4d33165..d49afb087 100644 --- a/tests/50federation/40devicelists.pl +++ b/tests/50federation/40devicelists.pl @@ -131,7 +131,7 @@ })->then( sub { do_request_json_for( $user, method => "POST", - uri => "/unstable/keys/query", + uri => "/r0/keys/query", content => { device_keys => { $creator_id => [ "random_device_id" ], @@ -169,7 +169,7 @@ })->then( sub { do_request_json_for( $user, method => "POST", - uri => "/unstable/keys/query", + uri => "/r0/keys/query", content => { device_keys => { $creator_id => [ "random_device_id" ], @@ -202,20 +202,10 @@ test "Server correctly resyncs when client query keys and there is no remote cache", - requires => [ local_user_fixture(), - $main::INBOUND_SERVER, $main::OUTBOUND_CLIENT, - federation_user_id_fixture(), - room_alias_name_fixture() ], + requires => [ $main::INBOUND_SERVER, federated_rooms_fixture() ], check => sub { - my ( $user, $inbound_server, $outbound_client, $creator_id, $room_alias_name ) = @_; - - my ( $room_id ); - - my $remote_server_name = $inbound_server->server_name; - my $datastore = $inbound_server->datastore; - - my $room_alias = "#$room_alias_name:$remote_server_name"; + my ( $inbound_server, $user, $federated_user_id ) = @_; # We return two devices, as there was a bug in synapse which correctly # handled returning one device but not two. @@ -227,54 +217,39 @@ # relaying any device keys, and then a client of synapse requests the keys # for that user. This should cause synapse to do a resync and cache those # keys correctly. - my $room = $datastore->create_room( - creator => $creator_id, - alias => $room_alias, - ); - - do_request_json_for( $user, - method => "POST", - uri => "/r0/join/$room_alias", - - content => {}, - )->then( sub { - Future->needs_all( - $inbound_server->await_request_user_devices( $creator_id ) - ->then( sub { - my ( $req, undef ) = @_; - - assert_eq( $req->method, "GET", 'request method' ); - - $req->respond_json( { - user_id => $creator_id, - stream_id => 1, - devices => [ { + Future->needs_all( + $inbound_server->await_request_user_devices( $federated_user_id ) + ->then( sub { + my ( $req, undef ) = @_; + + assert_eq( $req->method, "GET", 'request method' ); + + $req->respond_json( { + user_id => $federated_user_id, + stream_id => 1, + devices => [ + { device_id => $device_id1, - - keys => { - device_keys => {} - } - }, { + keys => { device_keys => {} }, + }, + { device_id => $device_id2, - - keys => { - device_keys => {} - } - } ] - } ); - Future->done(1); - }), - do_request_json_for( $user, - method => "POST", - uri => "/unstable/keys/query", - content => { - device_keys => { - $creator_id => [], + keys => { device_keys => {} }, } + ] + } ); + Future->done(1); + }), + do_request_json_for( $user, + method => "POST", + uri => "/r0/keys/query", + content => { + device_keys => { + $federated_user_id => [], } - ) + } ) - })->then( sub { + )->then( sub { my ( $first, $content ) = @_; log_if_fail "query response", $content; @@ -282,9 +257,9 @@ assert_json_keys( $content, "device_keys" ); my $device_keys = $content->{device_keys}; - assert_json_keys( $device_keys, $creator_id ); + assert_json_keys( $device_keys, $federated_user_id ); - my $alice_keys = $device_keys->{ $creator_id }; + my $alice_keys = $device_keys->{ $federated_user_id }; assert_json_keys( $alice_keys, ( $device_id1, $device_id2 ) ); Future->done( 1 ) @@ -660,7 +635,7 @@ })->then( sub { do_request_json_for( $user, method => "POST", - uri => "/unstable/keys/query", + uri => "/r0/keys/query", content => { device_keys => { $creator_id => [ $device_id ], From 0630ee6f833a18fec151c2d9976fab8d2d8ce0b7 Mon Sep 17 00:00:00 2001 From: Erik Johnston Date: Wed, 29 Jan 2020 11:10:06 +0000 Subject: [PATCH 3/3] Apply suggestions from code review Co-Authored-By: Richard van der Hoff <1389908+richvdh@users.noreply.github.com> --- tests/50federation/40devicelists.pl | 13 ++++++------- 1 file changed, 6 insertions(+), 7 deletions(-) diff --git a/tests/50federation/40devicelists.pl b/tests/50federation/40devicelists.pl index d49afb087..0bd3d1718 100644 --- a/tests/50federation/40devicelists.pl +++ b/tests/50federation/40devicelists.pl @@ -205,14 +205,13 @@ requires => [ $main::INBOUND_SERVER, federated_rooms_fixture() ], check => sub { - my ( $inbound_server, $user, $federated_user_id ) = @_; + my ( $inbound_server, $user, $federated_user_id, undef) = @_; # We return two devices, as there was a bug in synapse which correctly # handled returning one device but not two. my $device_id1 = "random_device_id1"; my $device_id2 = "random_device_id2"; - # We set up a situation where sytest joins a room with a user without # relaying any device keys, and then a client of synapse requests the keys # for that user. This should cause synapse to do a resync and cache those @@ -235,8 +234,8 @@ { device_id => $device_id2, keys => { device_keys => {} }, - } - ] + }, + ], } ); Future->done(1); }), @@ -246,9 +245,9 @@ content => { device_keys => { $federated_user_id => [], - } - } - ) + }, + }, + ), )->then( sub { my ( $first, $content ) = @_;