Skip to content

Commit

Permalink
[WPB-2789] Use EmptyResponse consistently across Galley federation en…
Browse files Browse the repository at this point in the history
…dpoints (#3365)

* Use EmptyResponse consistently for galley fed calls

* Amended doc for empty response.
  • Loading branch information
elland committed Jul 11, 2023
1 parent 8563623 commit c28a592
Show file tree
Hide file tree
Showing 5 changed files with 31 additions and 28 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,8 @@ import Test.QuickCheck
import Wire.Arbitrary

-- | This is equivalent to '()', but JSONifies to an empty object instead of an
-- empty array.
-- empty array. Returning an empty object gives us more flexibility, allowing
-- us to expand the response without breaking compatibility.
data EmptyResponse = EmptyResponse
deriving stock (Eq, Show, Generic)
deriving (Arbitrary) via (GenericUniform EmptyResponse)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -54,14 +54,14 @@ type GalleyApi =
-- that are part of a conversation at creation time. Since MLS conversations
-- are always created empty (i.e. they only contain the creator), this RPC is
-- never invoked for such conversations.
FedEndpoint "on-conversation-created" (ConversationCreated ConvId) ()
FedEndpoint "on-conversation-created" (ConversationCreated ConvId) EmptyResponse
-- This endpoint is called the first time a user from this backend is
-- added to a remote conversation.
:<|> FedEndpoint "on-new-remote-conversation" NewRemoteConversation EmptyResponse
:<|> FedEndpoint "get-conversations" GetConversationsRequest GetConversationsResponse
-- used by the backend that owns a conversation to inform this backend of
-- changes to the conversation
:<|> FedEndpoint "on-conversation-updated" ConversationUpdate ()
:<|> FedEndpoint "on-conversation-updated" ConversationUpdate EmptyResponse
:<|> FedEndpointWithMods
'[ MakesFederatedCall 'Galley "on-conversation-updated",
MakesFederatedCall 'Galley "on-mls-message-sent",
Expand Down
10 changes: 6 additions & 4 deletions services/galley/src/Galley/API/Federation.hs
Original file line number Diff line number Diff line change
Expand Up @@ -155,7 +155,7 @@ onConversationCreated ::
) =>
Domain ->
F.ConversationCreated ConvId ->
Sem r ()
Sem r EmptyResponse
onConversationCreated domain rc = do
let qrc = fmap (toRemoteUnsafe domain) rc
loc <- qualifyLocal ()
Expand Down Expand Up @@ -188,6 +188,7 @@ onConversationCreated domain rc = do
(F.ccTime qrcConnected)
(EdConversation c)
pushConversationEvent Nothing event (qualifyAs loc [qUnqualified . Public.memId $ mem]) []
pure EmptyResponse

onNewRemoteConversation ::
Member ConversationStore r =>
Expand Down Expand Up @@ -226,10 +227,11 @@ onConversationUpdated ::
) =>
Domain ->
F.ConversationUpdate ->
Sem r ()
onConversationUpdated requestingDomain cu =
Sem r EmptyResponse
onConversationUpdated requestingDomain cu = do
let rcu = toRemoteUnsafe requestingDomain cu
in void $ updateLocalStateOfRemoteConv rcu Nothing
void $ updateLocalStateOfRemoteConv rcu Nothing
pure EmptyResponse

-- as of now this will not generate the necessary events on the leaver's domain
leaveConversation ::
Expand Down
22 changes: 11 additions & 11 deletions services/galley/test/integration/API.hs
Original file line number Diff line number Diff line change
Expand Up @@ -2159,7 +2159,7 @@ paginateConvListIds = do
F.cuAlreadyPresentUsers = [],
F.cuAction = SomeConversationAction (sing @'ConversationJoinTag) (ConversationJoin (pure qAlice) roleNameWireMember)
}
runFedClient @"on-conversation-updated" fedGalleyClient chadDomain cu
void $ runFedClient @"on-conversation-updated" fedGalleyClient chadDomain cu

remoteDee <- randomId
let deeDomain = Domain "dee.example.com"
Expand All @@ -2175,7 +2175,7 @@ paginateConvListIds = do
F.cuAlreadyPresentUsers = [],
F.cuAction = SomeConversationAction (sing @'ConversationJoinTag) (ConversationJoin (pure qAlice) roleNameWireMember)
}
runFedClient @"on-conversation-updated" fedGalleyClient deeDomain cu
void $ runFedClient @"on-conversation-updated" fedGalleyClient deeDomain cu

-- 1 Proteus self conv + 1 MLS self conv + 2 convs with bob and eve + 196
-- local convs + 25 convs on chad.example.com + 31 on dee.example = 256 convs.
Expand Down Expand Up @@ -2220,7 +2220,7 @@ paginateConvListIdsPageEndingAtLocalsAndDomain = do
F.cuAlreadyPresentUsers = [],
F.cuAction = SomeConversationAction (sing @'ConversationJoinTag) (ConversationJoin (pure qAlice) roleNameWireMember)
}
runFedClient @"on-conversation-updated" fedGalleyClient chadDomain cu
void $ runFedClient @"on-conversation-updated" fedGalleyClient chadDomain cu

remoteDee <- randomId
let deeDomain = Domain "dee.example.com"
Expand All @@ -2238,7 +2238,7 @@ paginateConvListIdsPageEndingAtLocalsAndDomain = do
F.cuAlreadyPresentUsers = [],
F.cuAction = SomeConversationAction (sing @'ConversationJoinTag) (ConversationJoin (pure qAlice) roleNameWireMember)
}
runFedClient @"on-conversation-updated" fedGalleyClient deeDomain cu
void $ runFedClient @"on-conversation-updated" fedGalleyClient deeDomain cu

foldM_ (getChunkedConvs 16 0 alice) Nothing [4, 3, 2, 1, 0 :: Int]

Expand Down Expand Up @@ -3897,7 +3897,7 @@ putRemoteConvMemberOk update = do
cuAction =
SomeConversationAction (sing @'ConversationJoinTag) (ConversationJoin (pure qalice) roleNameWireMember)
}
runFedClient @"on-conversation-updated" fedGalleyClient remoteDomain cu
void $ runFedClient @"on-conversation-updated" fedGalleyClient remoteDomain cu

-- Expected member state
let memberAlice =
Expand Down Expand Up @@ -4042,7 +4042,7 @@ putRemoteReceiptModeOk = do
cuAction =
SomeConversationAction (sing @'ConversationJoinTag) (ConversationJoin (pure qalice) roleNameWireAdmin)
}
runFedClient @"on-conversation-updated" fedGalleyClient remoteDomain cuAddAlice
void $ runFedClient @"on-conversation-updated" fedGalleyClient remoteDomain cuAddAlice

-- add another user adam as member
qadam <- randomQualifiedUser
Expand All @@ -4057,7 +4057,7 @@ putRemoteReceiptModeOk = do
cuAction =
SomeConversationAction (sing @'ConversationJoinTag) (ConversationJoin (pure qadam) roleNameWireMember)
}
runFedClient @"on-conversation-updated" fedGalleyClient remoteDomain cuAddAdam
void $ runFedClient @"on-conversation-updated" fedGalleyClient remoteDomain cuAddAdam

let newReceiptMode = ReceiptMode 42
let action = ConversationReceiptModeUpdate newReceiptMode
Expand Down Expand Up @@ -4373,10 +4373,10 @@ removeUser = do
F.ccReceiptMode = Nothing,
F.ccProtocol = ProtocolProteus
}
runFedClient @"on-conversation-created" fedGalleyClient bDomain $ nc convB1 bart [alice, alexDel]
runFedClient @"on-conversation-created" fedGalleyClient bDomain $ nc convB2 bart [alexDel]
runFedClient @"on-conversation-created" fedGalleyClient cDomain $ nc convC1 carl [alexDel]
runFedClient @"on-conversation-created" fedGalleyClient dDomain $ nc convD1 dory [alexDel]
void $ runFedClient @"on-conversation-created" fedGalleyClient bDomain $ nc convB1 bart [alice, alexDel]
void $ runFedClient @"on-conversation-created" fedGalleyClient bDomain $ nc convB2 bart [alexDel]
void $ runFedClient @"on-conversation-created" fedGalleyClient cDomain $ nc convC1 carl [alexDel]
void $ runFedClient @"on-conversation-created" fedGalleyClient dDomain $ nc convD1 dory [alexDel]

WS.bracketR3 c alice' alexDel' amy' $ \(wsAlice, wsAlexDel, wsAmy) -> do
let handler = do
Expand Down
20 changes: 10 additions & 10 deletions services/galley/test/integration/API/Federation.hs
Original file line number Diff line number Diff line change
Expand Up @@ -247,7 +247,7 @@ addLocalUser = do
SomeConversationAction (sing @'ConversationJoinTag) (ConversationJoin (qalice :| [qdee]) roleNameWireMember)
}
WS.bracketRN c [alice, charlie, dee] $ \[wsA, wsC, wsD] -> do
runFedClient @"on-conversation-updated" fedGalleyClient remoteDomain cu
void $ runFedClient @"on-conversation-updated" fedGalleyClient remoteDomain cu
liftIO $ do
WS.assertMatch_ (5 # Second) wsA $
wsAssertMemberJoinWithRole qconv qbob [qalice] roleNameWireMember
Expand Down Expand Up @@ -301,7 +301,7 @@ addUnconnectedUsersOnly = do
SomeConversationAction (sing @'ConversationJoinTag) (ConversationJoin (qCharlie :| []) roleNameWireMember)
}
-- Alice receives no notifications from this
runFedClient @"on-conversation-updated" fedGalleyClient remoteDomain cu
void $ runFedClient @"on-conversation-updated" fedGalleyClient remoteDomain cu
WS.assertNoEvent (5 # Second) [wsA]

-- | This test invokes the federation endpoint:
Expand Down Expand Up @@ -346,9 +346,9 @@ removeLocalUser = do

connectWithRemoteUser alice qBob
WS.bracketR c alice $ \ws -> do
runFedClient @"on-conversation-updated" fedGalleyClient remoteDomain cuAdd
void $ runFedClient @"on-conversation-updated" fedGalleyClient remoteDomain cuAdd
afterAddition <- listRemoteConvs remoteDomain alice
runFedClient @"on-conversation-updated" fedGalleyClient remoteDomain cuRemove
void $ runFedClient @"on-conversation-updated" fedGalleyClient remoteDomain cuRemove
liftIO $ do
void . WS.assertMatch (3 # Second) ws $
wsAssertMemberJoinWithRole qconv qBob [qAlice] roleNameWireMember
Expand Down Expand Up @@ -409,21 +409,21 @@ removeRemoteUser = do
}

WS.bracketRN c [alice, charlie, dee, flo] $ \[wsA, wsC, wsD, wsF] -> do
runFedClient @"on-conversation-updated" fedGalleyClient remoteDomain (cuRemove qEve)
void $ runFedClient @"on-conversation-updated" fedGalleyClient remoteDomain (cuRemove qEve)
liftIO $ do
WS.assertMatchN_ (3 # Second) [wsA, wsD] $
wsAssertMembersLeave qconv qBob [qEve]
WS.assertNoEvent (1 # Second) [wsC, wsF]

WS.bracketRN c [alice, charlie, dee, flo] $ \[wsA, wsC, wsD, wsF] -> do
runFedClient @"on-conversation-updated" fedGalleyClient remoteDomain (cuRemove qDee)
void $ runFedClient @"on-conversation-updated" fedGalleyClient remoteDomain (cuRemove qDee)
liftIO $ do
WS.assertMatchN_ (3 # Second) [wsA, wsD] $
wsAssertMembersLeave qconv qBob [qDee]
WS.assertNoEvent (1 # Second) [wsC, wsF]

WS.bracketRN c [alice, charlie, dee, flo] $ \[wsA, wsC, wsD, wsF] -> do
runFedClient @"on-conversation-updated" fedGalleyClient remoteDomain (cuRemove qFlo)
void $ runFedClient @"on-conversation-updated" fedGalleyClient remoteDomain (cuRemove qFlo)
liftIO $ do
WS.assertMatchN_ (3 # Second) [wsA] $
wsAssertMembersLeave qconv qBob [qFlo]
Expand Down Expand Up @@ -460,7 +460,7 @@ notifyUpdate extras action etype edata = do
FedGalley.cuAction = action
}
WS.bracketR2 c alice charlie $ \(wsA, wsC) -> do
runFedClient @"on-conversation-updated" fedGalleyClient bdom cu
void $ runFedClient @"on-conversation-updated" fedGalleyClient bdom cu
liftIO $ do
WS.assertMatch_ (5 # Second) wsA $ \n -> do
let e = List1.head (WS.unpackPayload n)
Expand Down Expand Up @@ -637,7 +637,7 @@ notifyDeletedConversation = do
FedGalley.cuAlreadyPresentUsers = [alice],
FedGalley.cuAction = SomeConversationAction (sing @'ConversationDeleteTag) ()
}
runFedClient @"on-conversation-updated" fedGalleyClient bobDomain cu
void $ runFedClient @"on-conversation-updated" fedGalleyClient bobDomain cu

liftIO $ do
WS.assertMatch_ (5 # Second) wsAlice $ \n -> do
Expand Down Expand Up @@ -695,7 +695,7 @@ addRemoteUser = do
SomeConversationAction (sing @'ConversationJoinTag) (ConversationJoin (qdee :| [qeve, qflo]) roleNameWireMember)
}
WS.bracketRN c (map qUnqualified [qalice, qcharlie, qdee, qflo]) $ \[wsA, wsC, wsD, wsF] -> do
runFedClient @"on-conversation-updated" fedGalleyClient bdom cu
void $ runFedClient @"on-conversation-updated" fedGalleyClient bdom cu
void . liftIO $ do
WS.assertMatchN_ (5 # Second) [wsA, wsD] $
wsAssertMemberJoinWithRole qconv qbob [qeve, qdee] roleNameWireMember
Expand Down

0 comments on commit c28a592

Please sign in to comment.