Skip to content

Commit

Permalink
brig: Remove unnecessary List1 Event when pushing notifications (#4289)
Browse files Browse the repository at this point in the history
* brig: Remove unnecesssary List1 Event when pushing

Brig only ever pushes 1 notification at a time

* NotificationSubsystem: Make PushNotificationsAsync only push 1 notif

We only ever use it for 1 notification
  • Loading branch information
akshaymankar authored Oct 10, 2024
1 parent 1941f53 commit 05cffed
Show file tree
Hide file tree
Showing 5 changed files with 21 additions and 23 deletions.
2 changes: 1 addition & 1 deletion libs/wire-subsystems/src/Wire/NotificationSubsystem.hs
Original file line number Diff line number Diff line change
Expand Up @@ -45,7 +45,7 @@ data NotificationSubsystem m a where
-- send notifications is not critical.
--
-- See 'Polysemy.Async' to know more about the 'Maybe'
PushNotificationsAsync :: [Push] -> NotificationSubsystem m (Async (Maybe ()))
PushNotificationAsync :: Push -> NotificationSubsystem m (Async (Maybe ()))
CleanupUser :: UserId -> NotificationSubsystem m ()
UnregisterPushClient :: UserId -> ClientId -> NotificationSubsystem m ()
GetPushTokens :: UserId -> NotificationSubsystem m [PushToken]
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -42,7 +42,7 @@ runNotificationSubsystemGundeck ::
runNotificationSubsystemGundeck cfg = interpret $ \case
PushNotifications ps -> runInputConst cfg $ pushImpl ps
PushNotificationsSlowly ps -> runInputConst cfg $ pushSlowlyImpl ps
PushNotificationsAsync ps -> runInputConst cfg $ pushAsyncImpl ps
PushNotificationAsync ps -> runInputConst cfg $ pushAsyncImpl ps
CleanupUser uid -> GundeckAPIAccess.userDeleted uid
UnregisterPushClient uid cid -> GundeckAPIAccess.unregisterPushClient uid cid
GetPushTokens uid -> GundeckAPIAccess.getPushTokens uid
Expand Down Expand Up @@ -75,11 +75,11 @@ pushAsyncImpl ::
Member (Final IO) r,
Member P.TinyLog r
) =>
[Push] ->
Push ->
Sem r (Async (Maybe ()))
pushAsyncImpl ps = async $ do
pushAsyncImpl p = async $ do
reqId <- inputs requestId
errorToIOFinal @SomeException (fromExceptionSem @SomeException $ pushImpl ps) >>= \case
errorToIOFinal @SomeException (fromExceptionSem @SomeException $ pushImpl [p]) >>= \case
Left e ->
P.err $
Log.msg (Log.val "Error while pushing notifications")
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -228,9 +228,8 @@ spec = describe "NotificationSubsystem.Interpreter" do
pushJson = payload1,
_pushApsData = Nothing
}
pushes = [push1]
(_, attemptedPushes, logs) <- runMiniStackAsync mockConfig $ do
thread <- pushAsyncImpl pushes
thread <- pushAsyncImpl push1
await thread

attemptedPushes `shouldBe` [[toV2Push push1]]
Expand Down
2 changes: 1 addition & 1 deletion services/brig/src/Brig/API/Federation.hs
Original file line number Diff line number Diff line change
Expand Up @@ -287,7 +287,7 @@ onUserDeleted ::
onUserDeleted origDomain udcn = lift $ do
let deletedUser = toRemoteUnsafe origDomain udcn.user
connections = udcn.connections
event = pure . UserEvent $ UserDeleted (tUntagged deletedUser)
event = UserEvent $ UserDeleted (tUntagged deletedUser)
acceptedLocals <-
map csv2From
. filter (\x -> csv2Status x == Accepted)
Expand Down
29 changes: 14 additions & 15 deletions services/brig/src/Brig/IO/Intra.hs
Original file line number Diff line number Diff line change
Expand Up @@ -72,7 +72,6 @@ import Data.ByteString.Lazy qualified as BL
import Data.Id
import Data.Json.Util
import Data.List.NonEmpty (NonEmpty (..))
import Data.List1 (List1, singleton)
import Data.Proxy
import Data.Qualified
import Data.Range
Expand Down Expand Up @@ -150,7 +149,7 @@ onConnectionEvent ::
onConnectionEvent orig conn evt = do
let from = ucFrom (ucConn evt)
notify
(singleton $ ConnectionEvent evt)
(ConnectionEvent evt)
orig
V2.RouteAny
conn
Expand All @@ -166,7 +165,7 @@ onPropertyEvent ::
Sem r ()
onPropertyEvent orig conn e =
notify
(singleton $ PropertyEvent e)
(PropertyEvent e)
orig
V2.RouteDirect
(Just conn)
Expand Down Expand Up @@ -245,7 +244,7 @@ dispatchNotifications orig conn e = case e of
notifyUserDeletionLocals orig conn event
notifyUserDeletionRemotes orig
where
event = singleton $ UserEvent e
event = UserEvent e

notifyUserDeletionLocals ::
forall r.
Expand All @@ -256,7 +255,7 @@ notifyUserDeletionLocals ::
) =>
UserId ->
Maybe ConnId ->
List1 Event ->
Event ->
Sem r ()
notifyUserDeletionLocals deleted conn event = do
luid <- qualifyLocal' deleted
Expand Down Expand Up @@ -344,7 +343,7 @@ notifyUserDeletionRemotes deleted = do
-- | (Asynchronously) notifies other users of events.
notify ::
(Member NotificationSubsystem r) =>
List1 Event ->
Event ->
-- | Origin user, TODO: Delete
UserId ->
-- | Push routing strategy.
Expand All @@ -354,44 +353,44 @@ notify ::
-- | Users to notify.
Sem r (NonEmpty UserId) ->
Sem r ()
notify (toList -> events) orig route conn recipients = do
notify event orig route conn recipients = do
rs <- (\u -> Recipient u RecipientClientsAll) <$$> recipients
let pushes = flip map events $ \event ->
let push =
newPush1 (Just orig) (toJSONObject event) rs
& pushConn .~ conn
& pushRoute .~ route
& pushApsData .~ toApsData event
void $ pushNotificationsAsync pushes
void $ pushNotificationAsync push

notifySelf ::
(Member NotificationSubsystem r) =>
List1 Event ->
Event ->
-- | Origin user.
UserId ->
-- | Push routing strategy.
V2.Route ->
-- | Origin device connection, if any.
Maybe ConnId ->
Sem r ()
notifySelf events orig route conn =
notify events orig route conn (pure (orig :| []))
notifySelf event orig route conn =
notify event orig route conn (pure (orig :| []))

notifyContacts ::
forall r.
( Member (Embed HttpClientIO) r,
Member NotificationSubsystem r,
Member TinyLog r
) =>
List1 Event ->
Event ->
-- | Origin user.
UserId ->
-- | Push routing strategy.
V2.Route ->
-- | Origin device connection, if any.
Maybe ConnId ->
Sem r ()
notifyContacts events orig route conn = do
notify events orig route conn $
notifyContacts event orig route conn = do
notify event orig route conn $
(:|) orig <$> liftA2 (++) contacts teamContacts
where
contacts :: Sem r [UserId]
Expand Down

0 comments on commit 05cffed

Please sign in to comment.