Skip to content

Commit

Permalink
Improve naming conventions federation RPC calls (#1511)
Browse files Browse the repository at this point in the history
* Improve naming conventions of brig's federation API

* galley: Follow brig's federator naming convention

* Adapt test names to new federation convention
  • Loading branch information
pcapriotti authored May 19, 2021
1 parent a1ee909 commit 2e5ead6
Show file tree
Hide file tree
Showing 8 changed files with 43 additions and 50 deletions.
22 changes: 8 additions & 14 deletions libs/wire-api-federation/src/Wire/API/Federation/API/Brig.hs
Original file line number Diff line number Diff line change
Expand Up @@ -47,43 +47,37 @@ data Api routes = Api
{ getUserByHandle ::
routes
:- "federation"
:> "users"
:> "by-handle"
:> "get-user-by-handle"
:> ReqBody '[JSON] Handle
:> Post '[JSON] (Maybe UserProfile),
getUsersByIds ::
routes
:- "federation"
:> "users"
:> "get-by-ids"
:> "get-users-by-ids"
:> ReqBody '[JSON] [UserId]
:> Post '[JSON] [UserProfile],
claimPrekey ::
routes
:- "federation"
:> "users"
:> "prekey"
:> "claim-prekey"
:> ReqBody '[JSON] (UserId, ClientId)
:> Post '[JSON] (Maybe ClientPrekey),
getPrekeyBundle ::
claimPrekeyBundle ::
routes
:- "federation"
:> "users"
:> "prekey-bundle"
:> "claim-prekey-bundle"
:> ReqBody '[JSON] UserId
:> Post '[JSON] PrekeyBundle,
getMultiPrekeyBundle ::
claimMultiPrekeyBundle ::
routes
:- "federation"
:> "users"
:> "multi-prekey-bundle"
:> "claim-multi-prekey-bundle"
:> ReqBody '[JSON] UserClients
:> Post '[JSON] (UserClientMap (Maybe Prekey)),
searchUsers ::
routes
:- "federation"
:> "search"
:> "users"
:> "search-users"
-- FUTUREWORK(federation): do we want to perform some type-level validation like length checks?
-- (handles can be up to 256 chars currently)
:> ReqBody '[JSON] SearchRequest
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -35,17 +35,15 @@ data Api routes = Api
{ getConversations ::
routes
:- "federation"
:> "conversations"
:> "get-by-ids"
:> "get-conversations"
:> ReqBody '[JSON] GetConversationsRequest
:> Post '[JSON] GetConversationsResponse,
-- used by backend that owns the conversation to inform the backend about
-- add/removal of its users to the conversation
updateConversationMemberships ::
routes
:- "federation"
:> "conversations"
:> "update-membership"
:> "update-conversation-memberships"
:> ReqBody '[JSON] ConversationMemberUpdate
:> Post '[JSON] ()
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -59,7 +59,7 @@ spec = do
withMockFederator stateRef (mkSuccessResponse expectedResponse) $
Brig.getUserByHandle Brig.clientRoutes handle

sentRequests `shouldBe` [FederatedRequest "target.example.com" (Just $ Request Brig "/federation/users/by-handle" (LBS.toStrict (Aeson.encode handle)) "origin.example.com")]
sentRequests `shouldBe` [FederatedRequest "target.example.com" (Just $ Request Brig "/federation/get-user-by-handle" (LBS.toStrict (Aeson.encode handle)) "origin.example.com")]
actualResponse `shouldBe` Right expectedResponse

it "should parse failure response correctly" $ do
Expand Down
27 changes: 14 additions & 13 deletions services/brig/src/Brig/API/Federation.hs
Original file line number Diff line number Diff line change
Expand Up @@ -30,22 +30,23 @@ import Servant (ServerT)
import Servant.API.Generic (ToServantApi)
import Servant.Server.Generic (genericServerT)
import Wire.API.Federation.API.Brig (SearchRequest (SearchRequest))
import qualified Wire.API.Federation.API.Brig as FederationAPIBrig
import qualified Wire.API.Federation.API.Brig as Federated
import Wire.API.Message (UserClientMap, UserClients)
import Wire.API.User (UserProfile)
import Wire.API.User.Client.Prekey (ClientPrekey)
import Wire.API.User.Search

federationSitemap :: ServerT (ToServantApi FederationAPIBrig.Api) Handler
federationSitemap :: ServerT (ToServantApi Federated.Api) Handler
federationSitemap =
genericServerT $
FederationAPIBrig.Api
getUserByHandle
getUsersByIds
claimPrekey
getPrekeyBundle
getMultiPrekeyBundle
searchUsers
Federated.Api
{ Federated.getUserByHandle = getUserByHandle,
Federated.getUsersByIds = getUsersByIds,
Federated.claimPrekey = claimPrekey,
Federated.claimPrekeyBundle = claimPrekeyBundle,
Federated.claimMultiPrekeyBundle = claimMultiPrekeyBundle,
Federated.searchUsers = searchUsers
}

getUserByHandle :: Handle -> Handler (Maybe UserProfile)
getUserByHandle handle = lift $ do
Expand All @@ -63,11 +64,11 @@ getUsersByIds uids =
claimPrekey :: (UserId, ClientId) -> Handler (Maybe ClientPrekey)
claimPrekey (user, client) = lift (Data.claimPrekey user client)

getPrekeyBundle :: UserId -> Handler PrekeyBundle
getPrekeyBundle user = lift (API.claimLocalPrekeyBundle user)
claimPrekeyBundle :: UserId -> Handler PrekeyBundle
claimPrekeyBundle user = lift (API.claimLocalPrekeyBundle user)

getMultiPrekeyBundle :: UserClients -> Handler (UserClientMap (Maybe Prekey))
getMultiPrekeyBundle uc = lift (API.claimLocalMultiPrekeyBundles uc)
claimMultiPrekeyBundle :: UserClients -> Handler (UserClientMap (Maybe Prekey))
claimMultiPrekeyBundle uc = lift (API.claimLocalMultiPrekeyBundles uc)

-- | Searching for federated users on a remote backend should
-- only search by exact handle search, not in elasticsearch.
Expand Down
4 changes: 2 additions & 2 deletions services/brig/src/Brig/Federation/Client.hs
Original file line number Diff line number Diff line change
Expand Up @@ -61,15 +61,15 @@ claimPrekey (Qualified user domain) client = do
claimPrekeyBundle :: Qualified UserId -> FederationAppIO PrekeyBundle
claimPrekeyBundle (Qualified user domain) = do
Log.info $ Log.msg @Text "Brig-federation: claiming remote prekey bundle"
executeFederated domain $ FederatedBrig.getPrekeyBundle clientRoutes user
executeFederated domain $ FederatedBrig.claimPrekeyBundle clientRoutes user

claimMultiPrekeyBundle ::
Domain ->
UserClients ->
FederationAppIO (UserClientMap (Maybe Prekey))
claimMultiPrekeyBundle domain uc = do
Log.info $ Log.msg @Text "Brig-federation: claiming remote multi-user prekey bundle"
executeFederated domain $ FederatedBrig.getMultiPrekeyBundle clientRoutes uc
executeFederated domain $ FederatedBrig.claimMultiPrekeyBundle clientRoutes uc

-- FUTUREWORK(federation): rework error handling and FUTUREWORK from getUserHandleInfo and search:
-- decoding error should not throw a 404 most likely
Expand Down
26 changes: 13 additions & 13 deletions services/brig/test/integration/API/Federation.hs
Original file line number Diff line number Diff line change
Expand Up @@ -45,17 +45,17 @@ tests m brig fedBrigClient =
return $
testGroup
"federation"
[ test m "GET /federation/search/users : Found" (testSearchSuccess brig fedBrigClient),
test m "GET /federation/search/users : NotFound" (testSearchNotFound fedBrigClient),
test m "GET /federation/search/users : Empty Input - NotFound" (testSearchNotFoundEmpty fedBrigClient),
test m "GET /federation/users/by-handle : Found" (testGetUserByHandleSuccess brig fedBrigClient),
test m "GET /federation/users/by-handle : NotFound" (testGetUserByHandleNotFound fedBrigClient),
test m "GET /federation/users/get-by-id : 200 all found" (testGetUsersByIdsSuccess brig fedBrigClient),
test m "GET /federation/users/get-by-id : 200 partially found" (testGetUsersByIdsPartial brig fedBrigClient),
test m "GET /federation/users/get-by-id : 200 none found" (testGetUsersByIdsNoneFound fedBrigClient),
test m "GET /federation/users/prekey : 200" (testClaimPrekeySuccess brig fedBrigClient),
test m "GET /federation/users/prekey-bundle : 200" (testClaimPrekeyBundleSuccess brig fedBrigClient),
test m "POST /federation/users/multi-prekey-bundle : 200" (testClaimMultiPrekeyBundleSuccess brig fedBrigClient)
[ test m "GET /federation/search-users : Found" (testSearchSuccess brig fedBrigClient),
test m "GET /federation/search-users : NotFound" (testSearchNotFound fedBrigClient),
test m "GET /federation/search-users : Empty Input - NotFound" (testSearchNotFoundEmpty fedBrigClient),
test m "GET /federation/get-user-by-handle : Found" (testGetUserByHandleSuccess brig fedBrigClient),
test m "GET /federation/get-user-by-handle : NotFound" (testGetUserByHandleNotFound fedBrigClient),
test m "GET /federation/get-users-by-ids : 200 all found" (testGetUsersByIdsSuccess brig fedBrigClient),
test m "GET /federation/get-users-by-ids : 200 partially found" (testGetUsersByIdsPartial brig fedBrigClient),
test m "GET /federation/get-users-by-ids : 200 none found" (testGetUsersByIdsNoneFound fedBrigClient),
test m "GET /federation/claim-prekey : 200" (testClaimPrekeySuccess brig fedBrigClient),
test m "GET /federation/claim-prekey-bundle : 200" (testClaimPrekeyBundleSuccess brig fedBrigClient),
test m "POST /federation/claim-multi-prekey-bundle : 200" (testClaimMultiPrekeyBundleSuccess brig fedBrigClient)
]

testSearchSuccess :: Brig -> FedBrigClient -> Http ()
Expand Down Expand Up @@ -158,7 +158,7 @@ testClaimPrekeyBundleSuccess brig fedBrigClient = do
let prekeys = take 5 (zip somePrekeys someLastPrekeys)
(quid, clients) <- generateClientPrekeys brig prekeys
let sortClients = sortBy (compare `on` prekeyClient)
bundle <- FedBrig.getPrekeyBundle fedBrigClient (qUnqualified quid)
bundle <- FedBrig.claimPrekeyBundle fedBrigClient (qUnqualified quid)
liftIO $
assertEqual
"bundle should contain the clients"
Expand All @@ -176,7 +176,7 @@ testClaimMultiPrekeyBundleSuccess brig fedBrigClient = do
c2 <- first qUnqualified <$> generateClientPrekeys brig prekeys2
let uc = UserClients (Map.fromList [mkClients <$> c1, mkClients <$> c2])
ucm = UserClientMap (Map.fromList [mkClientMap <$> c1, mkClientMap <$> c2])
ucmResponse <- FedBrig.getMultiPrekeyBundle fedBrigClient uc
ucmResponse <- FedBrig.claimMultiPrekeyBundle fedBrigClient uc
liftIO $
assertEqual
"should return the UserClientMap"
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -78,7 +78,7 @@ spec env =
c <- case client of
Left err -> liftIO $ assertFailure (show err)
Right cli -> pure cli
let brigCall = Request Brig "federation/users/by-handle" (LBS.toStrict (encode hdl)) "foo.example.com"
let brigCall = Request Brig "federation/get-user-by-handle" (LBS.toStrict (encode hdl)) "foo.example.com"
res <- liftIO $ gRpcCall @'MsgProtoBuf @Inward @"Inward" @"call" c brigCall

liftIO $ case res of
Expand Down
4 changes: 2 additions & 2 deletions services/galley/test/integration/API/Federation.hs
Original file line number Diff line number Diff line change
Expand Up @@ -38,8 +38,8 @@ tests :: IO TestSetup -> TestTree
tests s =
testGroup
"federation"
[ test s "getConversations: All Found" getConversationsAllFound,
test s "getConversations: Conversations user is not a part of are excluded from result" getConversationsNotPartOf
[ test s "GET /federation/get-conversations : All Found" getConversationsAllFound,
test s "GET /federation/get-conversations : Conversations user is not a part of are excluded from result" getConversationsNotPartOf
]

getConversationsAllFound :: TestM ()
Expand Down

0 comments on commit 2e5ead6

Please sign in to comment.