From c1c55ca70032f6b55ae0ea9c07cdb97887d40539 Mon Sep 17 00:00:00 2001 From: Efim Poberezkin <8711996+efim-poberezkin@users.noreply.github.com> Date: Sun, 13 Feb 2022 13:19:24 +0400 Subject: [PATCH] deduplicate contact requests (#287) * deprecate XContact * XInfoId * xInfoId tests * merging * saving on connection * connectByAddress * remove old connect * deduplicate contact requests * check on contact acceptance * test * rename response * reuse CRContactRequestAlreadyAccepted * Update src/Simplex/Chat.hs * createConnReqConnection * simplify controller logic * store methods + profile change * index * more indices * unXInfoId * simplify * XInfo with ID -> XContact * sync reply to Connect when contact already exists * update view for sync CRContactAlreadyExists command response Co-authored-by: Evgeny Poberezkin <2769109+epoberezkin@users.noreply.github.com> --- .gitignore | 14 +- ...2022-02-10-deduplicate-contact-requests.md | 19 ++ simplex-chat.cabal | 1 + src/Simplex/Chat.hs | 54 +++-- src/Simplex/Chat/Controller.hs | 2 + .../M20220210_deduplicate_contact_requests.hs | 23 ++ src/Simplex/Chat/Protocol.hs | 8 +- src/Simplex/Chat/Store.hs | 204 +++++++++++++++--- src/Simplex/Chat/Types.hs | 42 +++- src/Simplex/Chat/View.hs | 6 +- tests/ChatTests.hs | 121 ++++++++++- tests/ProtocolTests.hs | 9 +- 12 files changed, 426 insertions(+), 77 deletions(-) create mode 100644 rfcs/2022-02-10-deduplicate-contact-requests.md create mode 100644 src/Simplex/Chat/Migrations/M20220210_deduplicate_contact_requests.hs diff --git a/.gitignore b/.gitignore index 8b4cc543b..e645225e9 100644 --- a/.gitignore +++ b/.gitignore @@ -5,12 +5,6 @@ *.so *.dylib -# Test binary, built with `go test -c` -*.test - -# Output of the go coverage tool, specifically when used with LiteIDE -*.out - # Dependency directories (remove the comment below to include it) # vendor/ @@ -42,9 +36,9 @@ cabal.project.local~ .ghc.environment.* stack.yaml.lock -# Idris -*.ibc - -# chat database +# Chat database *.db *.db.bak + +# Temporary test files +tests/tmp diff --git a/rfcs/2022-02-10-deduplicate-contact-requests.md b/rfcs/2022-02-10-deduplicate-contact-requests.md new file mode 100644 index 000000000..3e513296d --- /dev/null +++ b/rfcs/2022-02-10-deduplicate-contact-requests.md @@ -0,0 +1,19 @@ +# Deduplicate contact requests + +1. add nullable fields `via_contact_uri_hash` and `xcontact_id` to `connections` +2. when joining (Connect -> SCMContact) + - generate and save random `xcontact_id` + - save hash of `AConnectionRequestUri` when joining via contact uri + (AConnectionRequestUri -> ConnectionRequestUri -> CRContactUri) + - send random identifier in `XContact` as `Maybe XContactId` + - check for repeat join - if connection with such `via_contact_uri_hash` has contact notify user + - check for repeat join - check in connections if such contact uri exists, if yes use same identifier; the rest of request can (should) be regenerated, e.g. new server, profile + can be required +3. add nullable field `xcontact_id` to `contact_requests` and to `contacts` (* for auto-acceptance) +4. on contact request (processUserContactRequest) + - save identifier + - \* check if `xcontact_id` is in `contacts` - then notify this contact already exists + - when saving check if contact request with such identifier exists, if yes update `contact_request` + (`invId`, new profile) + - ? remove old invitation - probably not necessarily, to be done in scope of connection expiration + - return from Store whether request is new or updated (Bool?), new chat response for update or same response diff --git a/simplex-chat.cabal b/simplex-chat.cabal index 72c24eea3..f773fc489 100644 --- a/simplex-chat.cabal +++ b/simplex-chat.cabal @@ -27,6 +27,7 @@ library Simplex.Chat.Migrations.M20220101_initial Simplex.Chat.Migrations.M20220122_v1_1 Simplex.Chat.Migrations.M20220205_chat_item_status + Simplex.Chat.Migrations.M20220210_deduplicate_contact_requests Simplex.Chat.Mobile Simplex.Chat.Options Simplex.Chat.Protocol diff --git a/src/Simplex/Chat.hs b/src/Simplex/Chat.hs index a055f9434..1130fb594 100644 --- a/src/Simplex/Chat.hs +++ b/src/Simplex/Chat.hs @@ -179,12 +179,12 @@ processChatCommand = \case gs -> throwChatError $ CEContactGroups ct gs CTGroup -> pure $ chatCmdError "not implemented" CTContactRequest -> pure $ chatCmdError "not supported" - APIAcceptContact connReqId -> withUser $ \User {userId, profile} -> do - UserContactRequest {agentInvitationId = AgentInvId invId, localDisplayName = cName, profileId, profile = p} <- withStore $ \st -> - getContactRequest st userId connReqId - withChatLock . procCmd $ do + APIAcceptContact connReqId -> withUser $ \User {userId, profile} -> withChatLock $ do + UserContactRequest {agentInvitationId = AgentInvId invId, localDisplayName = cName, profileId, profile = p, xContactId} <- + withStore $ \st -> getContactRequest st userId connReqId + procCmd $ do connId <- withAgent $ \a -> acceptContact a invId . directMessage $ XInfo profile - acceptedContact <- withStore $ \st -> createAcceptedContact st userId connId cName profileId p + acceptedContact <- withStore $ \st -> createAcceptedContact st userId connId cName profileId p xContactId pure $ CRAcceptingContactRequest acceptedContact APIRejectContact connReqId -> withUser $ \User {userId} -> withChatLock $ do cReq@UserContactRequest {agentContactConnId = AgentConnId connId, agentInvitationId = AgentInvId invId} <- @@ -200,15 +200,14 @@ processChatCommand = \case withStore $ \st -> createDirectConnection st userId connId pure $ CRInvitation cReq Connect (Just (ACR SCMInvitation cReq)) -> withUser $ \User {userId, profile} -> withChatLock . procCmd $ do - connect userId cReq $ XInfo profile + connId <- withAgent $ \a -> joinConnection a cReq . directMessage $ XInfo profile + withStore $ \st -> createDirectConnection st userId connId pure CRSentConfirmation - Connect (Just (ACR SCMContact cReq)) -> withUser $ \User {userId, profile} -> withChatLock . procCmd $ do - connect userId cReq $ XContact profile Nothing - pure CRSentInvitation + Connect (Just (ACR SCMContact cReq)) -> withUser $ \User {userId, profile} -> + connectViaContact userId cReq profile Connect Nothing -> throwChatError CEInvalidConnReq - ConnectAdmin -> withUser $ \User {userId, profile} -> withChatLock . procCmd $ do - connect userId adminContactReq $ XContact profile Nothing - pure CRSentInvitation + ConnectAdmin -> withUser $ \User {userId, profile} -> + connectViaContact userId adminContactReq profile DeleteContact cName -> withUser $ \User {userId} -> do contactId <- withStore $ \st -> getContactIdByName st userId cName processChatCommand $ APIDeleteChat CTDirect contactId @@ -395,10 +394,17 @@ processChatCommand = \case -- use function below to make commands "synchronous" -- procCmd :: m ChatResponse -> m ChatResponse -- procCmd = id - connect :: UserId -> ConnectionRequestUri c -> ChatMsgEvent -> m () - connect userId cReq msg = do - connId <- withAgent $ \a -> joinConnection a cReq $ directMessage msg - withStore $ \st -> createDirectConnection st userId connId + connectViaContact :: UserId -> ConnectionRequestUri 'CMContact -> Profile -> m ChatResponse + connectViaContact userId cReq profile = withChatLock $ do + let cReqHash = ConnReqUriHash . C.sha256Hash $ strEncode cReq + withStore (\st -> getConnReqContactXContactId st userId cReqHash) >>= \case + (Just contact, _) -> pure $ CRContactAlreadyExists contact + (_, xContactId_) -> procCmd $ do + let randomXContactId = XContactId <$> (asks idsDrg >>= liftIO . (`randomBytes` 16)) + xContactId <- maybe randomXContactId pure xContactId_ + connId <- withAgent $ \a -> joinConnection a cReq $ directMessage (XContact profile $ Just xContactId) + withStore $ \st -> createConnReqConnection st userId connId cReqHash xContactId + pure CRSentInvitation contactMember :: Contact -> [GroupMember] -> Maybe GroupMember contactMember Contact {contactId} = find $ \GroupMember {memberContactId = cId, memberStatus = s} -> @@ -812,8 +818,8 @@ processAgentMessage (Just user@User {userId, profile}) agentConnId agentMessage REQ invId connInfo -> do ChatMessage {chatMsgEvent} <- liftEither $ parseChatMessage connInfo case chatMsgEvent of - XContact p _ -> profileContactRequest invId p - XInfo p -> profileContactRequest invId p + XContact p xContactId_ -> profileContactRequest invId p xContactId_ + XInfo p -> profileContactRequest invId p Nothing -- TODO show/log error, other events in contact request _ -> pure () -- TODO print errors @@ -822,11 +828,13 @@ processAgentMessage (Just user@User {userId, profile}) agentConnId agentMessage -- TODO add debugging output _ -> pure () where - profileContactRequest :: InvitationId -> Profile -> m () - profileContactRequest invId p = do - cReq@UserContactRequest {localDisplayName} <- withStore $ \st -> createContactRequest st userId userContactLinkId invId p - toView $ CRReceivedContactRequest cReq - showToast (localDisplayName <> "> ") "wants to connect to you" + profileContactRequest :: InvitationId -> Profile -> Maybe XContactId -> m () + profileContactRequest invId p xContactId_ = do + withStore (\st -> createOrUpdateContactRequest st userId userContactLinkId invId p xContactId_) >>= \case + Left contact -> toView $ CRContactRequestAlreadyAccepted contact + Right cReq@UserContactRequest {localDisplayName} -> do + toView $ CRReceivedContactRequest cReq + showToast (localDisplayName <> "> ") "wants to connect to you" withAckMessage :: ConnId -> MsgMeta -> m () -> m () withAckMessage cId MsgMeta {recipient = (msgId, _)} action = diff --git a/src/Simplex/Chat/Controller.hs b/src/Simplex/Chat/Controller.hs index 4bf983d20..9fcabae5e 100644 --- a/src/Simplex/Chat/Controller.hs +++ b/src/Simplex/Chat/Controller.hs @@ -162,6 +162,8 @@ data ChatResponse | CRUserContactLinkDeleted | CRReceivedContactRequest {contactRequest :: UserContactRequest} | CRAcceptingContactRequest {contact :: Contact} + | CRContactAlreadyExists {contact :: Contact} + | CRContactRequestAlreadyAccepted {contact :: Contact} | CRLeftMemberUser {groupInfo :: GroupInfo} | CRGroupDeletedUser {groupInfo :: GroupInfo} | CRRcvFileAccepted {fileTransfer :: RcvFileTransfer, filePath :: FilePath} diff --git a/src/Simplex/Chat/Migrations/M20220210_deduplicate_contact_requests.hs b/src/Simplex/Chat/Migrations/M20220210_deduplicate_contact_requests.hs new file mode 100644 index 000000000..0f55c9553 --- /dev/null +++ b/src/Simplex/Chat/Migrations/M20220210_deduplicate_contact_requests.hs @@ -0,0 +1,23 @@ +{-# LANGUAGE QuasiQuotes #-} + +module Simplex.Chat.Migrations.M20220210_deduplicate_contact_requests where + +import Database.SQLite.Simple (Query) +import Database.SQLite.Simple.QQ (sql) + +m20220210_deduplicate_contact_requests :: Query +m20220210_deduplicate_contact_requests = + [sql| +-- hash of contact address uri used by contact request sender to connect, +-- null for contact request recipient and for both parties when using one-off invitation +ALTER TABLE connections ADD COLUMN via_contact_uri_hash BLOB; +CREATE INDEX idx_connections_via_contact_uri_hash ON connections (via_contact_uri_hash); + +ALTER TABLE connections ADD COLUMN xcontact_id BLOB; + +ALTER TABLE contact_requests ADD COLUMN xcontact_id BLOB; +CREATE INDEX idx_contact_requests_xcontact_id ON contact_requests (xcontact_id); + +ALTER TABLE contacts ADD COLUMN xcontact_id BLOB; +CREATE INDEX idx_contacts_xcontact_id ON contacts (xcontact_id); +|] diff --git a/src/Simplex/Chat/Protocol.hs b/src/Simplex/Chat/Protocol.hs index 3b49f3ad7..c1d35e307 100644 --- a/src/Simplex/Chat/Protocol.hs +++ b/src/Simplex/Chat/Protocol.hs @@ -70,7 +70,7 @@ data ChatMsgEvent | XFile FileInvitation | XFileAcpt String | XInfo Profile - | XContact Profile (Maybe MsgContent) + | XContact Profile (Maybe XContactId) | XGrpInv GroupInvitation | XGrpAcpt MemberId | XGrpMemNew MemberInfo @@ -264,7 +264,7 @@ appToChatMessage AppMessage {event, params} = do XFile_ -> XFile <$> p "file" XFileAcpt_ -> XFileAcpt <$> p "fileName" XInfo_ -> XInfo <$> p "profile" - XContact_ -> XContact <$> p "profile" <*> JT.parseEither (.:? "content") params + XContact_ -> XContact <$> p "profile" <*> JT.parseEither (.:? "contactReqId") params XGrpInv_ -> XGrpInv <$> p "groupInvitation" XGrpAcpt_ -> XGrpAcpt <$> p "memberId" XGrpMemNew_ -> XGrpMemNew <$> p "memberInfo" @@ -292,8 +292,8 @@ chatToAppMessage ChatMessage {chatMsgEvent} = AppMessage {event, params} XMsgNew content -> o ["content" .= content] XFile fileInv -> o ["file" .= fileInv] XFileAcpt fileName -> o ["fileName" .= fileName] - XInfo profile -> o ["profile" .= profile] - XContact profile content -> o $ maybe id ((:) . ("content" .=)) content ["profile" .= profile] + XInfo profile -> o $ ["profile" .= profile] + XContact profile xContactId -> o $ maybe id ((:) . ("contactReqId" .=)) xContactId ["profile" .= profile] XGrpInv groupInv -> o ["groupInvitation" .= groupInv] XGrpAcpt memId -> o ["memberId" .= memId] XGrpMemNew memInfo -> o ["memberInfo" .= memInfo] diff --git a/src/Simplex/Chat/Store.hs b/src/Simplex/Chat/Store.hs index 2d625d7eb..b857dd9f6 100644 --- a/src/Simplex/Chat/Store.hs +++ b/src/Simplex/Chat/Store.hs @@ -25,6 +25,8 @@ module Simplex.Chat.Store getUsers, setActiveUser, createDirectConnection, + createConnReqConnection, + getConnReqContactXContactId, createDirectContact, getContactGroupNames, deleteContact, @@ -38,7 +40,7 @@ module Simplex.Chat.Store getUserContactLinkConnections, deleteUserContactLink, getUserContactLink, - createContactRequest, + createOrUpdateContactRequest, getContactRequest, getContactRequestIdByName, deleteContactRequest, @@ -153,6 +155,7 @@ import Simplex.Chat.Messages import Simplex.Chat.Migrations.M20220101_initial import Simplex.Chat.Migrations.M20220122_v1_1 import Simplex.Chat.Migrations.M20220205_chat_item_status +import Simplex.Chat.Migrations.M20220210_deduplicate_contact_requests import Simplex.Chat.Protocol import Simplex.Chat.Types import Simplex.Chat.Util (eitherToMaybe) @@ -169,7 +172,8 @@ schemaMigrations :: [(String, Query)] schemaMigrations = [ ("20220101_initial", m20220101_initial), ("20220122_v1_1", m20220122_v1_1), - ("20220205_chat_item_status", m20220205_chat_item_status) + ("20220205_chat_item_status", m20220205_chat_item_status), + ("20220210_deduplicate_contact_requests", m20220210_deduplicate_contact_requests) ] -- | The list of migrations in ascending order by date @@ -247,6 +251,55 @@ setActiveUser st userId = do DB.execute_ db "UPDATE users SET active_user = 0" DB.execute db "UPDATE users SET active_user = 1 WHERE user_id = ?" (Only userId) +createConnReqConnection :: MonadUnliftIO m => SQLiteStore -> UserId -> ConnId -> ConnReqUriHash -> XContactId -> m () +createConnReqConnection st userId acId cReqHash xContactId = do + liftIO . withTransaction st $ \db -> do + currentTs <- getCurrentTime + DB.execute + db + [sql| + INSERT INTO connections ( + user_id, agent_conn_id, conn_status, conn_type, + created_at, updated_at, via_contact_uri_hash, xcontact_id + ) VALUES (?,?,?,?,?,?,?,?) + |] + (userId, acId, ConnNew, ConnContact, currentTs, currentTs, cReqHash, xContactId) + +getConnReqContactXContactId :: MonadUnliftIO m => SQLiteStore -> UserId -> ConnReqUriHash -> m (Maybe Contact, Maybe XContactId) +getConnReqContactXContactId st userId cReqHash = do + liftIO . withTransaction st $ \db -> + getContact' db >>= \case + c@(Just _) -> pure (c, Nothing) + Nothing -> (Nothing,) <$> getXContactId db + where + getContact' :: DB.Connection -> IO (Maybe Contact) + getContact' db = + fmap toContact . listToMaybe + <$> DB.query + db + [sql| + SELECT + -- Contact + ct.contact_id, ct.local_display_name, ct.via_group, cp.display_name, cp.full_name, ct.created_at, + -- Connection + c.connection_id, c.agent_conn_id, c.conn_level, c.via_contact, c.conn_status, c.conn_type, + c.contact_id, c.group_member_id, c.snd_file_id, c.rcv_file_id, c.user_contact_link_id, c.created_at + FROM contacts ct + JOIN contact_profiles cp ON ct.contact_profile_id = cp.contact_profile_id + JOIN connections c ON c.contact_id = ct.contact_id + WHERE ct.user_id = ? AND c.via_contact_uri_hash = ? + ORDER BY c.connection_id DESC + LIMIT 1 + |] + (userId, cReqHash) + getXContactId :: DB.Connection -> IO (Maybe XContactId) + getXContactId db = + fmap fromOnly . listToMaybe + <$> DB.query + db + "SELECT xcontact_id FROM connections WHERE user_id = ? AND via_contact_uri_hash = ? LIMIT 1" + (userId, cReqHash) + createDirectConnection :: MonadUnliftIO m => SQLiteStore -> UserId -> ConnId -> m () createDirectConnection st userId agentConnId = liftIO . withTransaction st $ \db -> do @@ -254,7 +307,7 @@ createDirectConnection st userId agentConnId = void $ createContactConnection_ db userId agentConnId Nothing 0 currentTs createContactConnection_ :: DB.Connection -> UserId -> ConnId -> Maybe Int64 -> Int -> UTCTime -> IO Connection -createContactConnection_ db userId = do createConnection_ db userId ConnContact Nothing +createContactConnection_ db userId = createConnection_ db userId ConnContact Nothing createConnection_ :: DB.Connection -> UserId -> ConnType -> Maybe Int64 -> ConnId -> Maybe Int64 -> Int -> UTCTime -> IO Connection createConnection_ db userId connType entityId acId viaContact connLevel currentTs = do @@ -519,28 +572,117 @@ getUserContactLink st userId = connReq [Only cReq] = Right cReq connReq _ = Left SEUserContactLinkNotFound -createContactRequest :: StoreMonad m => SQLiteStore -> UserId -> Int64 -> InvitationId -> Profile -> m UserContactRequest -createContactRequest st userId userContactId invId Profile {displayName, fullName} = +createOrUpdateContactRequest :: StoreMonad m => SQLiteStore -> UserId -> Int64 -> InvitationId -> Profile -> Maybe XContactId -> m (Either Contact UserContactRequest) +createOrUpdateContactRequest st userId userContactLinkId invId profile xContactId_ = liftIOEither . withTransaction st $ \db -> - join <$> withLocalDisplayName db userId displayName (createContactRequest' db) + createOrUpdateContactRequest_ db userId userContactLinkId invId profile xContactId_ + +createOrUpdateContactRequest_ :: DB.Connection -> UserId -> Int64 -> InvitationId -> Profile -> Maybe XContactId -> IO (Either StoreError (Either Contact UserContactRequest)) +createOrUpdateContactRequest_ db userId userContactLinkId invId Profile {displayName, fullName} xContactId_ = + maybeM getContact' xContactId_ >>= \case + Just contact -> pure . Right $ Left contact + Nothing -> Right <$$> createOrUpdate_ where - createContactRequest' db ldn = do + maybeM = maybe (pure Nothing) + createOrUpdate_ :: IO (Either StoreError UserContactRequest) + createOrUpdate_ = + maybeM getContactRequest' xContactId_ >>= \case + Nothing -> createContactRequest + Just UserContactRequest {contactRequestId, profile = oldProfile} -> + updateContactRequest contactRequestId oldProfile + createContactRequest :: IO (Either StoreError UserContactRequest) + createContactRequest = do currentTs <- getCurrentTime - DB.execute - db - "INSERT INTO contact_profiles (display_name, full_name, created_at, updated_at) VALUES (?,?,?,?)" - (displayName, fullName, currentTs, currentTs) - profileId <- insertedRowId db - DB.execute - db - [sql| - INSERT INTO contact_requests - (user_contact_link_id, agent_invitation_id, contact_profile_id, local_display_name, user_id, created_at, updated_at) - VALUES (?,?,?,?,?,?,?) - |] - (userContactId, invId, profileId, ldn, userId, currentTs, currentTs) - contactRequestId <- insertedRowId db - getContactRequest_ db userId contactRequestId + join <$> withLocalDisplayName db userId displayName (createContactRequest_ currentTs) + where + createContactRequest_ currentTs ldn = do + DB.execute + db + "INSERT INTO contact_profiles (display_name, full_name, created_at, updated_at) VALUES (?,?,?,?)" + (displayName, fullName, currentTs, currentTs) + profileId <- insertedRowId db + DB.execute + db + [sql| + INSERT INTO contact_requests + (user_contact_link_id, agent_invitation_id, contact_profile_id, local_display_name, user_id, created_at, updated_at, xcontact_id) + VALUES (?,?,?,?,?,?,?,?) + |] + (userContactLinkId, invId, profileId, ldn, userId, currentTs, currentTs, xContactId_) + contactRequestId <- insertedRowId db + getContactRequest_ db userId contactRequestId + getContact' :: XContactId -> IO (Maybe Contact) + getContact' xContactId = + fmap toContact . listToMaybe + <$> DB.query + db + [sql| + SELECT + -- Contact + ct.contact_id, ct.local_display_name, ct.via_group, cp.display_name, cp.full_name, ct.created_at, + -- Connection + c.connection_id, c.agent_conn_id, c.conn_level, c.via_contact, c.conn_status, c.conn_type, + c.contact_id, c.group_member_id, c.snd_file_id, c.rcv_file_id, c.user_contact_link_id, c.created_at + FROM contacts ct + JOIN contact_profiles cp ON ct.contact_profile_id = cp.contact_profile_id + LEFT JOIN connections c ON c.contact_id = ct.contact_id + WHERE ct.user_id = ? AND ct.xcontact_id = ? + ORDER BY c.connection_id DESC + LIMIT 1 + |] + (userId, xContactId) + getContactRequest' :: XContactId -> IO (Maybe UserContactRequest) + getContactRequest' xContactId = + fmap toContactRequest . listToMaybe + <$> DB.query + db + [sql| + SELECT + cr.contact_request_id, cr.local_display_name, cr.agent_invitation_id, cr.user_contact_link_id, + c.agent_conn_id, cr.contact_profile_id, p.display_name, p.full_name, cr.created_at, cr.xcontact_id + FROM contact_requests cr + JOIN connections c USING (user_contact_link_id) + JOIN contact_profiles p USING (contact_profile_id) + WHERE cr.user_id = ? + AND cr.xcontact_id = ? + LIMIT 1 + |] + (userId, xContactId) + updateContactRequest :: Int64 -> Profile -> IO (Either StoreError UserContactRequest) + updateContactRequest cReqId Profile {displayName = oldDisplayName} = do + currentTs <- liftIO getCurrentTime + if displayName == oldDisplayName + then updateContactRequest_ currentTs displayName + else join <$> withLocalDisplayName db userId displayName (updateContactRequest_ currentTs) + where + updateContactRequest_ updatedAt ldn = do + DB.execute + db + [sql| + UPDATE contact_profiles + SET display_name = ?, + full_name = ?, + updated_at = ? + WHERE contact_profile_id IN ( + SELECT contact_profile_id + FROM contact_requests + WHERE user_id = ? + AND contact_request_id = ? + ) + |] + (ldn, fullName, updatedAt, userId, cReqId) + DB.execute + db + [sql| + UPDATE contact_requests + SET agent_invitation_id = ?, + local_display_name = ?, + updated_at = ? + WHERE user_id = ? + AND contact_request_id = ? + |] + (invId, ldn, updatedAt, userId, cReqId) + getContactRequest_ db userId cReqId getContactRequest :: StoreMonad m => SQLiteStore -> UserId -> Int64 -> m UserContactRequest getContactRequest st userId contactRequestId = @@ -555,7 +697,7 @@ getContactRequest_ db userId contactRequestId = [sql| SELECT cr.contact_request_id, cr.local_display_name, cr.agent_invitation_id, cr.user_contact_link_id, - c.agent_conn_id, cr.contact_profile_id, p.display_name, p.full_name, cr.created_at + c.agent_conn_id, cr.contact_profile_id, p.display_name, p.full_name, cr.created_at, cr.xcontact_id FROM contact_requests cr JOIN connections c USING (user_contact_link_id) JOIN contact_profiles p USING (contact_profile_id) @@ -564,12 +706,12 @@ getContactRequest_ db userId contactRequestId = |] (userId, contactRequestId) -type ContactRequestRow = (Int64, ContactName, AgentInvId, Int64, AgentConnId, Int64, ContactName, Text, UTCTime) +type ContactRequestRow = (Int64, ContactName, AgentInvId, Int64, AgentConnId, Int64, ContactName, Text, UTCTime, Maybe XContactId) toContactRequest :: ContactRequestRow -> UserContactRequest -toContactRequest (contactRequestId, localDisplayName, agentInvitationId, userContactLinkId, agentContactConnId, profileId, displayName, fullName, createdAt) = do +toContactRequest (contactRequestId, localDisplayName, agentInvitationId, userContactLinkId, agentContactConnId, profileId, displayName, fullName, createdAt, xContactId) = do let profile = Profile {displayName, fullName} - in UserContactRequest {contactRequestId, agentInvitationId, userContactLinkId, agentContactConnId, localDisplayName, profileId, profile, createdAt} + in UserContactRequest {contactRequestId, agentInvitationId, userContactLinkId, agentContactConnId, localDisplayName, profileId, profile, createdAt, xContactId} getContactRequestIdByName :: StoreMonad m => SQLiteStore -> UserId -> ContactName -> m Int64 getContactRequestIdByName st userId cName = @@ -592,15 +734,15 @@ deleteContactRequest st userId contactRequestId = (userId, userId, contactRequestId) DB.execute db "DELETE FROM contact_requests WHERE user_id = ? AND contact_request_id = ?" (userId, contactRequestId) -createAcceptedContact :: MonadUnliftIO m => SQLiteStore -> UserId -> ConnId -> ContactName -> Int64 -> Profile -> m Contact -createAcceptedContact st userId agentConnId localDisplayName profileId profile = +createAcceptedContact :: MonadUnliftIO m => SQLiteStore -> UserId -> ConnId -> ContactName -> Int64 -> Profile -> Maybe XContactId -> m Contact +createAcceptedContact st userId agentConnId localDisplayName profileId profile xContactId = liftIO . withTransaction st $ \db -> do DB.execute db "DELETE FROM contact_requests WHERE user_id = ? AND local_display_name = ?" (userId, localDisplayName) currentTs <- getCurrentTime DB.execute db - "INSERT INTO contacts (user_id, local_display_name, contact_profile_id, created_at, updated_at) VALUES (?,?,?,?,?)" - (userId, localDisplayName, profileId, currentTs, currentTs) + "INSERT INTO contacts (user_id, local_display_name, contact_profile_id, created_at, updated_at, xcontact_id) VALUES (?,?,?,?,?,?)" + (userId, localDisplayName, profileId, currentTs, currentTs, xContactId) contactId <- insertedRowId db activeConn <- createConnection_ db userId ConnContact (Just contactId) agentConnId Nothing 0 currentTs pure $ Contact {contactId, localDisplayName, profile, activeConn, viaGroup = Nothing, createdAt = currentTs} @@ -2148,7 +2290,7 @@ getContactRequestChatPreviews_ db User {userId} = [sql| SELECT cr.contact_request_id, cr.local_display_name, cr.agent_invitation_id, cr.user_contact_link_id, - c.agent_conn_id, cr.contact_profile_id, p.display_name, p.full_name, cr.created_at + c.agent_conn_id, cr.contact_profile_id, p.display_name, p.full_name, cr.created_at, cr.xcontact_id FROM contact_requests cr JOIN connections c USING (user_contact_link_id) JOIN contact_profiles p USING (contact_profile_id) diff --git a/src/Simplex/Chat/Types.hs b/src/Simplex/Chat/Types.hs index 96f829f3c..b342f3077 100644 --- a/src/Simplex/Chat/Types.hs +++ b/src/Simplex/Chat/Types.hs @@ -10,7 +10,6 @@ {-# LANGUAGE NamedFieldPuns #-} {-# LANGUAGE OverloadedStrings #-} {-# LANGUAGE ScopedTypeVariables #-} -{-# LANGUAGE TypeApplications #-} {-# LANGUAGE UndecidableInstances #-} module Simplex.Chat.Types where @@ -100,13 +99,52 @@ data UserContactRequest = UserContactRequest localDisplayName :: ContactName, profileId :: Int64, profile :: Profile, - createdAt :: UTCTime + createdAt :: UTCTime, + xContactId :: Maybe XContactId } deriving (Eq, Show, Generic, FromJSON) instance ToJSON UserContactRequest where toEncoding = J.genericToEncoding J.defaultOptions +newtype XContactId = XContactId ByteString + deriving (Eq, Show) + +instance FromField XContactId where fromField f = XContactId <$> fromField f + +instance ToField XContactId where toField (XContactId m) = toField m + +instance StrEncoding XContactId where + strEncode (XContactId m) = strEncode m + strDecode s = XContactId <$> strDecode s + strP = XContactId <$> strP + +instance FromJSON XContactId where + parseJSON = strParseJSON "XContactId" + +instance ToJSON XContactId where + toJSON = strToJSON + toEncoding = strToJEncoding + +newtype ConnReqUriHash = ConnReqUriHash {unConnReqUriHash :: ByteString} + deriving (Eq, Show) + +instance FromField ConnReqUriHash where fromField f = ConnReqUriHash <$> fromField f + +instance ToField ConnReqUriHash where toField (ConnReqUriHash m) = toField m + +instance StrEncoding ConnReqUriHash where + strEncode (ConnReqUriHash m) = strEncode m + strDecode s = ConnReqUriHash <$> strDecode s + strP = ConnReqUriHash <$> strP + +instance FromJSON ConnReqUriHash where + parseJSON = strParseJSON "ConnReqUriHash" + +instance ToJSON ConnReqUriHash where + toJSON = strToJSON + toEncoding = strToJEncoding + type ContactName = Text type GroupName = Text diff --git a/src/Simplex/Chat/View.hs b/src/Simplex/Chat/View.hs index ffbf8f1ce..800b646d7 100644 --- a/src/Simplex/Chat/View.hs +++ b/src/Simplex/Chat/View.hs @@ -65,8 +65,10 @@ responseToView cmd testView = \case CRInvitation cReq -> r' $ viewConnReqInvitation cReq CRSentConfirmation -> r' ["confirmation sent!"] CRSentInvitation -> r' ["connection request sent!"] - CRContactDeleted Contact {localDisplayName = c} -> r' [ttyContact c <> ": contact is deleted"] - CRAcceptingContactRequest Contact {localDisplayName = c} -> r' [ttyContact c <> ": accepting contact request..."] + CRContactDeleted c -> r' [ttyContact' c <> ": contact is deleted"] + CRAcceptingContactRequest c -> r' [ttyFullContact c <> ": accepting contact request..."] + CRContactAlreadyExists c -> r [ttyFullContact c <> ": contact already exists"] + CRContactRequestAlreadyAccepted c -> r' [ttyFullContact c <> ": sent you a duplicate contact request, but you are already connected, no action needed"] CRUserContactLinkCreated cReq -> r' $ connReqContact_ "Your new chat address is created!" cReq CRUserContactLinkDeleted -> r' viewUserContactLinkDeleted CRUserAcceptedGroupSent _g -> r' [] -- [ttyGroup' g <> ": joining the group..."] diff --git a/tests/ChatTests.hs b/tests/ChatTests.hs index d08f99b98..6fbc627aa 100644 --- a/tests/ChatTests.hs +++ b/tests/ChatTests.hs @@ -52,6 +52,8 @@ chatTests = do it "send and receive file to group" testGroupFileTransfer describe "user contact link" $ do it "should create and connect via contact link" testUserContactLink + it "should deduplicate contact requests" testDeduplicateContactRequests + it "should deduplicate contact requests with profile change" testDeduplicateContactRequestsProfileChange it "should reject contact and delete contact link" testRejectContactAndDeleteUserContact it "should delete connection requests when contact link deleted" testDeleteConnectionRequests @@ -700,7 +702,7 @@ testUserContactLink = testChat3 aliceProfile bobProfile cathProfile $ alice <#? bob alice #$$> ("/_get chats", [("<@bob", "")]) alice ##> "/ac bob" - alice <## "bob: accepting contact request..." + alice <## "bob (Bob): accepting contact request..." concurrently_ (bob <## "alice (Alice): contact is connected") (alice <## "bob (Bob): contact is connected") @@ -711,13 +713,128 @@ testUserContactLink = testChat3 aliceProfile bobProfile cathProfile $ alice <#? cath alice #$$> ("/_get chats", [("<@cath", ""), ("@bob", "hey")]) alice ##> "/ac cath" - alice <## "cath: accepting contact request..." + alice <## "cath (Catherine): accepting contact request..." concurrently_ (cath <## "alice (Alice): contact is connected") (alice <## "cath (Catherine): contact is connected") alice #$$> ("/_get chats", [("@cath", ""), ("@bob", "hey")]) alice <##> cath +testDeduplicateContactRequests :: IO () +testDeduplicateContactRequests = testChat3 aliceProfile bobProfile cathProfile $ + \alice bob cath -> do + alice ##> "/ad" + cLink <- getContactLink alice True + + bob ##> ("/c " <> cLink) + alice <#? bob + alice #$$> ("/_get chats", [("<@bob", "")]) + + bob ##> ("/c " <> cLink) + alice <#? bob + bob ##> ("/c " <> cLink) + alice <#? bob + alice #$$> ("/_get chats", [("<@bob", "")]) + + alice ##> "/ac bob" + alice <## "bob (Bob): accepting contact request..." + concurrently_ + (bob <## "alice (Alice): contact is connected") + (alice <## "bob (Bob): contact is connected") + + bob ##> ("/c " <> cLink) + bob <## "alice (Alice): contact already exists" + alice #$$> ("/_get chats", [("@bob", "")]) + bob #$$> ("/_get chats", [("@alice", "")]) + + alice <##> bob + alice #$$> ("/_get chats", [("@bob", "hey")]) + bob #$$> ("/_get chats", [("@alice", "hey")]) + + bob ##> ("/c " <> cLink) + bob <## "alice (Alice): contact already exists" + + alice <##> bob + alice #$> ("/_get chat @2 count=100", chat, [(1, "hi"), (0, "hey"), (1, "hi"), (0, "hey")]) + bob #$> ("/_get chat @2 count=100", chat, [(0, "hi"), (1, "hey"), (0, "hi"), (1, "hey")]) + + cath ##> ("/c " <> cLink) + alice <#? cath + alice #$$> ("/_get chats", [("<@cath", ""), ("@bob", "hey")]) + alice ##> "/ac cath" + alice <## "cath (Catherine): accepting contact request..." + concurrently_ + (cath <## "alice (Alice): contact is connected") + (alice <## "cath (Catherine): contact is connected") + alice #$$> ("/_get chats", [("@cath", ""), ("@bob", "hey")]) + alice <##> cath + +testDeduplicateContactRequestsProfileChange :: IO () +testDeduplicateContactRequestsProfileChange = testChat3 aliceProfile bobProfile cathProfile $ + \alice bob cath -> do + alice ##> "/ad" + cLink <- getContactLink alice True + + bob ##> ("/c " <> cLink) + alice <#? bob + alice #$$> ("/_get chats", [("<@bob", "")]) + + bob ##> "/p bob" + bob <## "user full name removed (your contacts are notified)" + bob ##> ("/c " <> cLink) + bob <## "connection request sent!" + alice <## "bob wants to connect to you!" + alice <## "to accept: /ac bob" + alice <## "to reject: /rc bob (the sender will NOT be notified)" + alice #$$> ("/_get chats", [("<@bob", "")]) + + bob ##> "/p bob Bob Ross" + bob <## "user full name changed to Bob Ross (your contacts are notified)" + bob ##> ("/c " <> cLink) + alice <#? bob + alice #$$> ("/_get chats", [("<@bob", "")]) + + bob ##> "/p robert Robert" + bob <## "user profile is changed to robert (Robert) (your contacts are notified)" + bob ##> ("/c " <> cLink) + alice <#? bob + alice #$$> ("/_get chats", [("<@robert", "")]) + + alice ##> "/ac bob" + alice <## "no contact request from bob" + alice ##> "/ac robert" + alice <## "robert (Robert): accepting contact request..." + concurrently_ + (bob <## "alice (Alice): contact is connected") + (alice <## "robert (Robert): contact is connected") + + bob ##> ("/c " <> cLink) + bob <## "alice (Alice): contact already exists" + alice #$$> ("/_get chats", [("@robert", "")]) + bob #$$> ("/_get chats", [("@alice", "")]) + + alice <##> bob + alice #$$> ("/_get chats", [("@robert", "hey")]) + bob #$$> ("/_get chats", [("@alice", "hey")]) + + bob ##> ("/c " <> cLink) + bob <## "alice (Alice): contact already exists" + + alice <##> bob + alice #$> ("/_get chat @2 count=100", chat, [(1, "hi"), (0, "hey"), (1, "hi"), (0, "hey")]) + bob #$> ("/_get chat @2 count=100", chat, [(0, "hi"), (1, "hey"), (0, "hi"), (1, "hey")]) + + cath ##> ("/c " <> cLink) + alice <#? cath + alice #$$> ("/_get chats", [("<@cath", ""), ("@robert", "hey")]) + alice ##> "/ac cath" + alice <## "cath (Catherine): accepting contact request..." + concurrently_ + (cath <## "alice (Alice): contact is connected") + (alice <## "cath (Catherine): contact is connected") + alice #$$> ("/_get chats", [("@cath", ""), ("@robert", "hey")]) + alice <##> cath + testRejectContactAndDeleteUserContact :: IO () testRejectContactAndDeleteUserContact = testChat3 aliceProfile bobProfile cathProfile $ \alice bob cath -> do diff --git a/tests/ProtocolTests.hs b/tests/ProtocolTests.hs index 5ba91200f..9491e1fb6 100644 --- a/tests/ProtocolTests.hs +++ b/tests/ProtocolTests.hs @@ -84,15 +84,18 @@ decodeChatMessageTest = describe "Chat message encoding/decoding" $ do it "x.file.acpt" $ "{\"event\":\"x.file.acpt\",\"params\":{\"fileName\":\"photo.jpg\"}}" #==# XFileAcpt "photo.jpg" it "x.info" $ "{\"event\":\"x.info\",\"params\":{\"profile\":{\"fullName\":\"Alice\",\"displayName\":\"alice\"}}}" #==# XInfo testProfile it "x.info" $ "{\"event\":\"x.info\",\"params\":{\"profile\":{\"fullName\":\"\",\"displayName\":\"alice\"}}}" #==# XInfo Profile {displayName = "alice", fullName = ""} - it "x.contact without content field" $ + it "x.contact with xContactId" $ + "{\"event\":\"x.contact\",\"params\":{\"contactReqId\":\"AQIDBA==\",\"profile\":{\"fullName\":\"Alice\",\"displayName\":\"alice\"}}}" + #==# XContact testProfile (Just $ XContactId "\1\2\3\4") + it "x.contact without XContactId" $ "{\"event\":\"x.contact\",\"params\":{\"profile\":{\"fullName\":\"Alice\",\"displayName\":\"alice\"}}}" #==# XContact testProfile Nothing it "x.contact with content null" $ "{\"event\":\"x.contact\",\"params\":{\"content\":null,\"profile\":{\"fullName\":\"Alice\",\"displayName\":\"alice\"}}}" ==# XContact testProfile Nothing - it "x.contact with content" $ + it "x.contact with content (ignored)" $ "{\"event\":\"x.contact\",\"params\":{\"content\":{\"text\":\"hello\",\"type\":\"text\"},\"profile\":{\"fullName\":\"Alice\",\"displayName\":\"alice\"}}}" - #==# XContact testProfile (Just $ MCText "hello") + ==# XContact testProfile Nothing it "x.grp.inv" $ "{\"event\":\"x.grp.inv\",\"params\":{\"groupInvitation\":{\"connRequest\":\"https://simplex.chat/invitation#/?v=1&smp=smp%3A%2F%2F1234-w%3D%3D%40smp.simplex.im%3A5223%2F3456-w%3D%3D%23MCowBQYDK2VuAyEAjiswwI3O_NlS8Fk3HJUW870EY2bAwmttMBsvRB9eV3o%3D&e2e=v%3D1%26x3dh%3DMEIwBQYDK2VvAzkAmKuSYeQ_m0SixPDS8Wq8VBaTS1cW-Lp0n0h4Diu-kUpR-qXx4SDJ32YGEFoGFGSbGPry5Ychr6U%3D%2CMEIwBQYDK2VvAzkAmKuSYeQ_m0SixPDS8Wq8VBaTS1cW-Lp0n0h4Diu-kUpR-qXx4SDJ32YGEFoGFGSbGPry5Ychr6U%3D\",\"invitedMember\":{\"memberRole\":\"member\",\"memberId\":\"BQYHCA==\"},\"groupProfile\":{\"fullName\":\"Team\",\"displayName\":\"team\"},\"fromMember\":{\"memberRole\":\"admin\",\"memberId\":\"AQIDBA==\"}}}}" #==# XGrpInv GroupInvitation {fromMember = MemberIdRole (MemberId "\1\2\3\4") GRAdmin, invitedMember = MemberIdRole (MemberId "\5\6\7\8") GRMember, connRequest = testConnReq, groupProfile = testGroupProfile}