Skip to content

Commit 85cb5a9

Browse files
mdimjasevicpcapriottiakshaymankar
authored
[WPB-3664] Bug fix: Notify remote backends of their users removed from conversation when reachable again (#3537)
* Formatting * Test utilities for changing a conv name * Add a test confirming the bug report * An action to enqueue notifications concurrently * Enqueue member removal notification for remotes * Add a changelog * Test case formatting * Migrate test roleUpdateWithRemotesUnavailable * Migrate test putReceiptModeWithRemotesOk * Migrate test putReceiptModeWithRemotesUnavailable * Migrate test testRoleUpdateWithRemotesOk * Migrate test roleUpdateRemoteMember * Migrate test putQualifiedConvRenameWithRemotesUnavailable This one is already covered by testSynchroniseUserRemovalNotification * Migrate test putQualifiedConvRenameWithRemotesOk * Migrate test deleteLocalMemberConvLocalQualifiedOk * Migrate test deleteRemoteMemberConvLocalQualifiedOk * Migrate test deleteUnavailableRemoteMemberConvLocalQualifiedOk * Add the copyright header to a test module * Move a test utility (allPreds) * Test utility: create a team with members * Migrate test testAccessUpdateGuestRemoved * Migrate test messageTimerChangeWithRemotes * Migrate test messageTimerUnavailableRemotes * Migrate test testAccessUpdateGuestRemovedRemotesUnavailable * Migrate test accessUpdateWithRemotes * Migrate test testAddRemoteMember * Migrate test testDeleteTeamConversationWithRemoteMembers * Migrate test testDeleteTeamConversationWithUnavailableRemoteMembers * Move a test utility (assertLeaveNotification) * Migrate test "POST /federation/leave-conversation : Success" * Migrate test "POST /federation/on-user-deleted-conversations : Remove deleted remote user from local conversations" * Migrate test updateConversationByRemoteAdmin * Tests: support giving a role when adding * Use cannon API for notifications when possible * Use startDynamicBackends when possible * Fix assertion * Migrate test testAddRemoteUsersToLocalConv * Test add member endpoint at version 1 * Add return value to enqueueNotification * Use cannon assertions in offline backends test * Check that remote notifications are received * Test removal of users from unreachable backends * Use correct domains for default backends Taking the domains in the `backendA` and `backendB` resources only works locally. * fixup! Use cannon assertions in offline backends test --------- Co-authored-by: Paolo Capriotti <[email protected]> Co-authored-by: Akshay Mankar <[email protected]>
1 parent 3f88513 commit 85cb5a9

File tree

32 files changed

+1074
-1523
lines changed

32 files changed

+1074
-1523
lines changed
Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1 @@
1+
This fixes a bug where a remote member is removed from a conversation while their backend is unreachable, and the backend does not receive the removal notification once it is reachable again.

integration/default.nix

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -19,6 +19,7 @@
1919
, cql-io
2020
, cryptonite
2121
, data-default
22+
, data-timeout
2223
, directory
2324
, errors
2425
, exceptions
@@ -86,6 +87,7 @@ mkDerivation {
8687
cql-io
8788
cryptonite
8889
data-default
90+
data-timeout
8991
directory
9092
errors
9193
exceptions

integration/integration.cabal

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -100,6 +100,7 @@ library
100100
Notifications
101101
RunAllTests
102102
SetupHelpers
103+
Test.AccessUpdate
103104
Test.AssetDownload
104105
Test.B2B
105106
Test.Brig
@@ -108,8 +109,10 @@ library
108109
Test.Demo
109110
Test.Federation
110111
Test.Federator
112+
Test.MessageTimer
111113
Test.Notifications
112114
Test.Presence
115+
Test.Roles
113116
Test.User
114117
Testlib.App
115118
Testlib.Assertions
@@ -146,6 +149,7 @@ library
146149
, cql-io
147150
, cryptonite
148151
, data-default
152+
, data-timeout
149153
, directory
150154
, errors
151155
, exceptions

integration/test/API/Galley.hs

Lines changed: 134 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,7 @@ module API.Galley where
55
import Control.Lens hiding ((.=))
66
import Control.Monad.Reader
77
import Data.Aeson qualified as Aeson
8+
import Data.Aeson.Types qualified as Aeson
89
import Data.ByteString.Lazy qualified as LBS
910
import Data.ProtoLens qualified as Proto
1011
import Data.ProtoLens.Labels ()
@@ -80,6 +81,21 @@ postConversation user cc = do
8081
ccv <- make cc
8182
submit "POST" $ req & addJSON ccv
8283

84+
deleteTeamConversation ::
85+
( HasCallStack,
86+
MakesValue user,
87+
MakesValue conv
88+
) =>
89+
String ->
90+
conv ->
91+
user ->
92+
App Response
93+
deleteTeamConversation tid qcnv user = do
94+
cnv <- snd <$> objQid qcnv
95+
let path = joinHttpPath ["teams", tid, "conversations", cnv]
96+
req <- baseRequest user Galley Versioned path
97+
submit "DELETE" req
98+
8399
putConversationProtocol ::
84100
( HasCallStack,
85101
MakesValue user,
@@ -217,12 +233,39 @@ getGroupInfo user conv = do
217233
req <- baseRequest user Galley Versioned path
218234
submit "GET" req
219235

220-
addMembers :: (HasCallStack, MakesValue user, MakesValue conv) => user -> conv -> [Value] -> App Response
221-
addMembers usr qcnv newMembers = do
236+
data AddMembers = AddMembers
237+
{ users :: [Value],
238+
role :: Maybe String,
239+
version :: Maybe Int
240+
}
241+
242+
instance Default AddMembers where
243+
def = AddMembers {users = [], role = Nothing, version = Nothing}
244+
245+
addMembers ::
246+
(HasCallStack, MakesValue user, MakesValue conv) =>
247+
user ->
248+
conv ->
249+
AddMembers ->
250+
App Response
251+
addMembers usr qcnv opts = do
222252
(convDomain, convId) <- objQid qcnv
223-
qUsers <- mapM objQidObject newMembers
224-
req <- baseRequest usr Galley Versioned (joinHttpPath ["conversations", convDomain, convId, "members"])
225-
submit "POST" (req & addJSONObject ["qualified_users" .= qUsers])
253+
qUsers <- mapM objQidObject opts.users
254+
let path = case opts.version of
255+
Just v | v <= 1 -> ["conversations", convId, "members", "v2"]
256+
_ -> ["conversations", convDomain, convId, "members"]
257+
req <-
258+
baseRequest
259+
usr
260+
Galley
261+
(maybe Versioned ExplicitVersion opts.version)
262+
(joinHttpPath path)
263+
submit "POST" $
264+
req
265+
& addJSONObject
266+
( ["qualified_users" .= qUsers]
267+
<> ["conversation_role" .= r | r <- toList opts.role]
268+
)
226269

227270
removeMember :: (HasCallStack, MakesValue remover, MakesValue conv, MakesValue removed) => remover -> conv -> removed -> App Response
228271
removeMember remover qcnv removed = do
@@ -263,3 +306,89 @@ getConversationCode user conv mbZHost = do
263306
& addQueryParams [("cnv", convId)]
264307
& maybe id zHost mbZHost
265308
)
309+
310+
changeConversationName ::
311+
(HasCallStack, MakesValue user, MakesValue conv, MakesValue name) =>
312+
user ->
313+
conv ->
314+
name ->
315+
App Response
316+
changeConversationName user qcnv name = do
317+
(convDomain, convId) <- objQid qcnv
318+
let path = joinHttpPath ["conversations", convDomain, convId, "name"]
319+
nameReq <- make name
320+
req <- baseRequest user Galley Versioned path
321+
submit "PUT" (req & addJSONObject ["name" .= nameReq])
322+
323+
updateRole ::
324+
( HasCallStack,
325+
MakesValue callerUser,
326+
MakesValue targetUser,
327+
MakesValue roleUpdate,
328+
MakesValue qcnv
329+
) =>
330+
callerUser ->
331+
targetUser ->
332+
roleUpdate ->
333+
qcnv ->
334+
App Response
335+
updateRole caller target role qcnv = do
336+
(cnvDomain, cnvId) <- objQid qcnv
337+
(tarDomain, tarId) <- objQid target
338+
roleReq <- make role
339+
req <-
340+
baseRequest
341+
caller
342+
Galley
343+
Versioned
344+
( joinHttpPath ["conversations", cnvDomain, cnvId, "members", tarDomain, tarId]
345+
)
346+
submit "PUT" (req & addJSONObject ["conversation_role" .= roleReq])
347+
348+
updateReceiptMode ::
349+
( HasCallStack,
350+
MakesValue user,
351+
MakesValue conv,
352+
MakesValue mode
353+
) =>
354+
user ->
355+
conv ->
356+
mode ->
357+
App Response
358+
updateReceiptMode user qcnv mode = do
359+
(cnvDomain, cnvId) <- objQid qcnv
360+
modeReq <- make mode
361+
let path = joinHttpPath ["conversations", cnvDomain, cnvId, "receipt-mode"]
362+
req <- baseRequest user Galley Versioned path
363+
submit "PUT" (req & addJSONObject ["receipt_mode" .= modeReq])
364+
365+
updateAccess ::
366+
( HasCallStack,
367+
MakesValue user,
368+
MakesValue conv
369+
) =>
370+
user ->
371+
conv ->
372+
[Aeson.Pair] ->
373+
App Response
374+
updateAccess user qcnv update = do
375+
(cnvDomain, cnvId) <- objQid qcnv
376+
let path = joinHttpPath ["conversations", cnvDomain, cnvId, "access"]
377+
req <- baseRequest user Galley Versioned path
378+
submit "PUT" (req & addJSONObject update)
379+
380+
updateMessageTimer ::
381+
( HasCallStack,
382+
MakesValue user,
383+
MakesValue conv
384+
) =>
385+
user ->
386+
conv ->
387+
Word64 ->
388+
App Response
389+
updateMessageTimer user qcnv update = do
390+
(cnvDomain, cnvId) <- objQid qcnv
391+
updateReq <- make update
392+
let path = joinHttpPath ["conversations", cnvDomain, cnvId, "message-timer"]
393+
req <- baseRequest user Galley Versioned path
394+
submit "PUT" (addJSONObject ["message_timer" .= updateReq] req)

integration/test/Notifications.hs

Lines changed: 57 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -65,8 +65,63 @@ isMemberJoinNotif n = fieldEquals n "payload.0.type" "conversation.member-join"
6565
isConvLeaveNotif :: MakesValue a => a -> App Bool
6666
isConvLeaveNotif n = fieldEquals n "payload.0.type" "conversation.member-leave"
6767

68-
isNotifConv :: (MakesValue conv, MakesValue a) => conv -> a -> App Bool
68+
isNotifConv :: (MakesValue conv, MakesValue a, HasCallStack) => conv -> a -> App Bool
6969
isNotifConv conv n = fieldEquals n "payload.0.qualified_conversation" (objQidObject conv)
7070

71-
isNotifForUser :: (MakesValue user, MakesValue a) => user -> a -> App Bool
71+
isNotifForUser :: (MakesValue user, MakesValue a, HasCallStack) => user -> a -> App Bool
7272
isNotifForUser user n = fieldEquals n "payload.0.data.qualified_user_ids.0" (objQidObject user)
73+
74+
isNotifFromUser :: (MakesValue user, MakesValue a, HasCallStack) => user -> a -> App Bool
75+
isNotifFromUser user n = fieldEquals n "payload.0.qualified_from" (objQidObject user)
76+
77+
isConvNameChangeNotif :: (HasCallStack, MakesValue a) => a -> App Bool
78+
isConvNameChangeNotif n = fieldEquals n "payload.0.type" "conversation.rename"
79+
80+
isMemberUpdateNotif :: (HasCallStack, MakesValue n) => n -> App Bool
81+
isMemberUpdateNotif n = fieldEquals n "payload.0.type" "conversation.member-update"
82+
83+
isReceiptModeUpdateNotif :: (HasCallStack, MakesValue n) => n -> App Bool
84+
isReceiptModeUpdateNotif n =
85+
fieldEquals n "payload.0.type" "conversation.receipt-mode-update"
86+
87+
isConvMsgTimerUpdateNotif :: (HasCallStack, MakesValue n) => n -> App Bool
88+
isConvMsgTimerUpdateNotif n =
89+
fieldEquals n "payload.0.type" "conversation.message-timer-update"
90+
91+
isConvAccessUpdateNotif :: (HasCallStack, MakesValue n) => n -> App Bool
92+
isConvAccessUpdateNotif n =
93+
fieldEquals n "payload.0.type" "conversation.access-update"
94+
95+
isConvCreateNotif :: MakesValue a => a -> App Bool
96+
isConvCreateNotif n = fieldEquals n "payload.0.type" "conversation.create"
97+
98+
isConvDeleteNotif :: MakesValue a => a -> App Bool
99+
isConvDeleteNotif n = fieldEquals n "payload.0.type" "conversation.delete"
100+
101+
assertLeaveNotification ::
102+
( HasCallStack,
103+
MakesValue fromUser,
104+
MakesValue conv,
105+
MakesValue user,
106+
MakesValue kickedUser
107+
) =>
108+
fromUser ->
109+
conv ->
110+
user ->
111+
String ->
112+
kickedUser ->
113+
App ()
114+
assertLeaveNotification fromUser conv user client leaver =
115+
void $
116+
awaitNotification
117+
user
118+
client
119+
noValue
120+
2
121+
( allPreds
122+
[ isConvLeaveNotif,
123+
isNotifConv conv,
124+
isNotifForUser leaver,
125+
isNotifFromUser fromUser
126+
]
127+
)

integration/test/SetupHelpers.hs

Lines changed: 37 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,7 @@ module SetupHelpers where
44

55
import API.Brig qualified as Brig
66
import API.BrigInternal qualified as Internal
7+
import API.Common
78
import API.Galley
89
import Control.Concurrent (threadDelay)
910
import Control.Monad.Reader
@@ -32,15 +33,43 @@ deleteUser user = bindResponse (Brig.deleteUser user) $ \resp -> do
3233
resp.status `shouldMatchInt` 200
3334

3435
-- | returns (user, team id)
35-
createTeam :: (HasCallStack, MakesValue domain) => domain -> App (Value, String)
36-
createTeam domain = do
36+
createTeam :: (HasCallStack, MakesValue domain) => domain -> Int -> App (Value, String, [Value])
37+
createTeam domain memberCount = do
3738
res <- Internal.createUser domain def {Internal.team = True}
38-
user <- res.json
39-
tid <- user %. "team" & asString
40-
-- TODO
41-
-- SQS.assertTeamActivate "create team" tid
42-
-- refreshIndex
43-
pure (user, tid)
39+
owner <- res.json
40+
tid <- owner %. "team" & asString
41+
members <- for [2 .. memberCount] $ \_ -> createTeamMember owner tid
42+
pure (owner, tid, members)
43+
44+
createTeamMember ::
45+
(HasCallStack, MakesValue inviter) =>
46+
inviter ->
47+
String ->
48+
App Value
49+
createTeamMember inviter tid = do
50+
newUserEmail <- randomEmail
51+
let invitationJSON = ["role" .= "member", "email" .= newUserEmail]
52+
invitationReq <-
53+
baseRequest inviter Brig Versioned $
54+
joinHttpPath ["teams", tid, "invitations"]
55+
invitation <- getJSON 201 =<< submit "POST" (addJSONObject invitationJSON invitationReq)
56+
invitationId <- objId invitation
57+
invitationCodeReq <-
58+
rawBaseRequest inviter Brig Unversioned "/i/teams/invitation-code"
59+
<&> addQueryParams [("team", tid), ("invitation_id", invitationId)]
60+
invitationCode <- bindResponse (submit "GET" invitationCodeReq) $ \res -> do
61+
res.status `shouldMatchInt` 200
62+
res.json %. "code" & asString
63+
let registerJSON =
64+
[ "name" .= newUserEmail,
65+
"email" .= newUserEmail,
66+
"password" .= defPassword,
67+
"team_code" .= invitationCode
68+
]
69+
registerReq <-
70+
rawBaseRequest inviter Brig Versioned "/register"
71+
<&> addJSONObject registerJSON
72+
getJSON 201 =<< submit "POST" registerReq
4473

4574
connectUsers ::
4675
( HasCallStack,

0 commit comments

Comments
 (0)