Fix receive routing for UUID conversations
ober
5bc60779b71f2a1cc1232877d556b807714c5cb7
--- a/Makefile +++ b/Makefile @@ -55,6 +55,7 @@ run-tui: binary tui-shim test: binary $(JEXEC) tests/test-send-result.ss + $(JEXEC) tests/test-rpc-demux.ss $(JEXEC) tests/test-receive-normalization.ss ./$(BIN) --help >/dev/null && echo "smoke ok" --- a/signal/capture.ss +++ b/signal/capture.ss @@ -78,9 +78,9 @@ (htref env "sourceUuid") (htref env "source") "unknown")] - [src-target (first-string (htref env "sourceNumber") - (htref env "sourceUuid") - (htref env "source"))] + [src-target (first-string (htref env "sourceUuid") + (htref env "source") + (htref env "sourceNumber"))] [env-ts (htref env "timestamp")]) (cond [(hashtable? data) @@ -112,9 +112,9 @@ (cond [(hashtable? sent) (let ([gid (data-group-id sent)] - [dest (first-string (htref sent "destinationNumber") + [dest (first-string (htref sent "destinationUuid") (htref sent "destination") - (htref sent "destinationUuid"))]) + (htref sent "destinationNumber"))]) (list "out" (conv-id gid (or gid dest "unknown")) "You" @@ -126,8 +126,8 @@ [story (htref sent-story "dataMessage")]) (if (hashtable? story) (story-row "out" story "You" - (first-string (htref sent-story "destinationNumber") - (htref sent-story "destinationUuid")) + (first-string (htref sent-story "destinationUuid") + (htref sent-story "destinationNumber")) env-ts) (list "out" "" "You" env-ts "sync" "")))] [else (list "out" "" "You" env-ts "sync" "")]))] --- a/signal/rpc-actor.ss +++ b/signal/rpc-actor.ss @@ -15,7 +15,11 @@ actor-call actor-try-event - actor-drain-events) + actor-drain-events + + ;; JSON-RPC frame classification helpers. + has-rpc-id? + rpc-notification?) (import (except (chezscheme) make-hash-table hash-table? @@ -324,12 +328,14 @@ (def (has-rpc-id? msg) (and (hashtable? msg) - (hashtable-ref msg "id" #f))) + (let ([id (hashtable-ref msg "id" #f)]) + (and id (not (eq? id (void))) #t)))) (def (rpc-notification? msg) (and (hashtable? msg) - (not (hashtable-ref msg "id" #f)) - (hashtable-ref msg "method" #f))) + (not (has-rpc-id? msg)) + (let ([method (hashtable-ref msg "method" #f)]) + (and (string? method) #t)))) (def (make-bad-frame line) (cons 'bad-frame line)) --- a/signal/tui/main.ss +++ b/signal/tui/main.ss @@ -41,8 +41,8 @@ ;; retries, but other conversations can still be tried. (defstruct tui-state (account version receive-mode width height input status quit? event-count - conversations selected-index mode picker-query picker-index removed logdb - resend challenge rate-limits)) + conversations selected-index mode picker-query picker-index removed aliases + logdb resend challenge rate-limits)) (def (run-tui-terminal account actor version receive-mode) ;; Prompt for the log passphrase and open the DB BEFORE termbox grabs the @@ -89,6 +89,7 @@ "" 0 removed + (make-hashtable equal-hash equal?) logdb #f #f @@ -147,7 +148,7 @@ ;; row = (direction conversation sender timestamp kind body); see logdb-recent. (def (apply-history-row! state row) (let* ([direction (list-ref row 0)] - [conv-id (list-ref row 1)] + [conv-id (canonical-conversation-id state (list-ref row 1))] [sender (list-ref row 2)] [ts (list-ref row 3)] [body (list-ref row 5)] @@ -180,6 +181,31 @@ (string=? (substring id 0 plen) prefix) (substring id plen (string-length id))))) + (def (conversation-alias! state from-id to-id) + (when (and (non-empty-string? from-id) + (non-empty-string? to-id) + (not (string=? from-id to-id))) + (hashtable-set! (tui-state-aliases state) from-id to-id))) + + (def (target-alias! state kind from-target to-target) + (when (and (non-empty-string? from-target) + (non-empty-string? to-target)) + (conversation-alias! + state + (conversation-id-for kind from-target from-target) + (conversation-id-for kind to-target to-target)))) + + (def (canonical-conversation-id state id) + (let loop ([cur id] [seen '()]) + (if (and (non-empty-string? cur) + (not (member cur seen))) + (let ([next (hashtable-ref (tui-state-aliases state) cur #f)]) + (if (and (non-empty-string? next) + (not (string=? next cur))) + (loop next (cons cur seen)) + cur)) + cur))) + (def *idle-poll-ms* 250) (def (event-loop state actor) @@ -333,7 +359,8 @@ ;; Typing is ephemeral: STARTED arms a 15s expiry on an EXISTING thread, ;; STOPPED clears it. We never resurrect a thread the user has closed. (def (apply-typing-event! state id sender started?) - (let ([conv (find-conversation (tui-state-conversations state) id)]) + (let ([conv (find-conversation (tui-state-conversations state) + (canonical-conversation-id state id))]) (when conv (conversation-typing-set! conv @@ -415,9 +442,10 @@ (def (seed-group-conversation! state group) (and (hashtable? group) (hashtable-ref group "isMember" #f) - (let ([id (first-non-empty-string (hashtable-ref group "id" #f))]) + (let ([id (group-canonical-target group)]) (and id (let ([conv-id (conversation-id-for 'group id id)]) + (register-group-aliases! state group id) (and (not (conversation-removed? state conv-id)) (begin (ensure-conversation! @@ -432,9 +460,13 @@ (and (hashtable? contact) (not (hashtable-ref contact "isHidden" #f)) (not (hashtable-ref contact "unregistered" #f)) - (let ([target (contact-target contact)]) - (and target - (let ([id (conversation-id-for 'direct target target)]) + (let ([canonical-target (contact-canonical-target contact)]) + (and canonical-target + (let* ([target (contact-send-target contact canonical-target)] + [id (conversation-id-for 'direct + canonical-target + canonical-target)]) + (register-contact-aliases! state contact canonical-target) (and (not (conversation-removed? state id)) (begin (ensure-conversation! @@ -452,10 +484,14 @@ [else (find-conversation (cdr conversations) id)])) (def (ensure-conversation! state id title kind target) - ;; Re-creating a conversation (inbound message or explicit pick) means the - ;; user wants it back, so clear any persisted removal. - (unmark-removed! state id) - (let ([existing (find-conversation (tui-state-conversations state) id)]) + (let* ([id (canonical-conversation-id state id)] + [kind-target (parse-conversation-id id)] + [kind (or kind (and kind-target (car kind-target)))] + [target (or target (and kind-target (cdr kind-target)))]) + ;; Re-creating a conversation (inbound message or explicit pick) means the + ;; user wants it back, so clear any persisted removal. + (unmark-removed! state id) + (let ([existing (find-conversation (tui-state-conversations state) id)]) (if existing (begin (when (non-empty-string? title) @@ -463,7 +499,12 @@ (when kind (conversation-kind-set! existing kind)) (when target - (conversation-target-set! existing target)) + (conversation-target-set! + existing + (preferred-conversation-target + (conversation-kind existing) + (conversation-target existing) + target))) existing) (let* ([conv (make-conversation id (if (non-empty-string? title) title id) @@ -479,7 +520,7 @@ (string=? (conversation-id selected) "system") (not (string=? id "system"))) (select-conversation! state (- (length next) 1)))) - conv)))) + conv))))) (def (selected-conversation state) (let* ([conversations (tui-state-conversations state)] @@ -490,7 +531,8 @@ (clamp-index (tui-state-selected-index state) count))))) (def (selected-conversation? state id) - (let ([conv (selected-conversation state)]) + (let ([conv (selected-conversation state)] + [id (canonical-conversation-id state id)]) (and conv (string=? (conversation-id conv) id)))) (def (select-conversation! state index) @@ -554,10 +596,65 @@ [(non-empty-string? (car values)) (car values)] [else (apply first-non-empty-string (cdr values))])) - (def (contact-target contact) + (def (group-canonical-target group) (first-non-empty-string - (hashtable-ref contact "number" #f) - (hashtable-ref contact "uuid" #f))) + (hashtable-ref group "id" #f) + (hashtable-ref group "groupId" #f) + (hashtable-ref group "groupIdBase64" #f) + (hashtable-ref group "masterKey" #f))) + + (def (register-group-aliases! state group canonical-target) + (target-alias! state 'group (hashtable-ref group "id" #f) canonical-target) + (target-alias! state 'group (hashtable-ref group "groupId" #f) canonical-target) + (target-alias! state 'group (hashtable-ref group "groupIdBase64" #f) + canonical-target) + (target-alias! state 'group (hashtable-ref group "masterKey" #f) + canonical-target)) + + (def (contact-canonical-target contact) + (first-non-empty-string + (hashtable-ref contact "uuid" #f) + (hashtable-ref contact "aci" #f) + (hashtable-ref contact "serviceId" #f) + (hashtable-ref contact "serviceIdentifier" #f) + (hashtable-ref contact "pni" #f) + (hashtable-ref contact "number" #f))) + + (def (contact-send-target contact canonical-target) + (or (first-non-empty-string (hashtable-ref contact "number" #f)) + canonical-target)) + + (def (register-contact-aliases! state contact canonical-target) + (target-alias! state 'direct (hashtable-ref contact "number" #f) + canonical-target) + (target-alias! state 'direct (hashtable-ref contact "uuid" #f) + canonical-target) + (target-alias! state 'direct (hashtable-ref contact "aci" #f) + canonical-target) + (target-alias! state 'direct (hashtable-ref contact "serviceId" #f) + canonical-target) + (target-alias! state 'direct (hashtable-ref contact "serviceIdentifier" #f) + canonical-target) + (target-alias! state 'direct (hashtable-ref contact "pni" #f) + canonical-target)) + + (def (phone-number-target? target) + (and (non-empty-string? target) + (char=? (string-ref target 0) #\+))) + + (def (preferred-conversation-target kind current next) + (cond + [(not (non-empty-string? next)) current] + [(not (non-empty-string? current)) next] + [(and (eq? kind 'direct) + (phone-number-target? current) + (not (phone-number-target? next))) + current] + [(and (eq? kind 'direct) + (phone-number-target? next) + (not (phone-number-target? current))) + next] + [else next])) (def (contact-display-name contact) (or (first-non-empty-string @@ -567,7 +664,9 @@ (hashtable-ref contact "familyName" #f)) (profile-display-name (hashtable-ref contact "profile" #f)) (hashtable-ref contact "number" #f) - (hashtable-ref contact "uuid" #f)) + (hashtable-ref contact "uuid" #f) + (hashtable-ref contact "aci" #f) + (hashtable-ref contact "pni" #f)) "Unknown contact")) (def (profile-display-name profile) @@ -1676,16 +1775,37 @@ ht)) (def (conversation-removed? state id) - (hashtable-ref (tui-state-removed state) id #f)) + (let ([removed (tui-state-removed state)] + [canonical-id (canonical-conversation-id state id)]) + (or (and (non-empty-string? id) + (hashtable-ref removed id #f)) + (and (non-empty-string? canonical-id) + (hashtable-ref removed canonical-id #f))))) (def (mark-removed! state id) - (hashtable-set! (tui-state-removed state) id #t) - (persist-removed! state)) + (let ([id (canonical-conversation-id state id)]) + (when (non-empty-string? id) + (hashtable-set! (tui-state-removed state) id #t) + (persist-removed! state)))) (def (unmark-removed! state id) - (when (hashtable-ref (tui-state-removed state) id #f) - (hashtable-delete! (tui-state-removed state) id) - (persist-removed! state))) + (let ([removed (tui-state-removed state)] + [canonical-id (canonical-conversation-id state id)] + [changed? #f]) + (for-each + (lambda (key) + (let ([key-canonical (canonical-conversation-id state key)]) + (when (or (and (non-empty-string? key) + (non-empty-string? id) + (string=? key id)) + (and (non-empty-string? key-canonical) + (non-empty-string? canonical-id) + (string=? key-canonical canonical-id))) + (hashtable-delete! removed key) + (set! changed? #t)))) + (vector->list (hashtable-keys removed))) + (when changed? + (persist-removed! state)))) (def (persist-removed! state) (save-removed-ids (tui-state-account state) @@ -2399,9 +2519,9 @@ [(hashtable? sent) (let* ([group-target (message-group-id sent)] [dest (first-non-empty-string - (hashtable-ref sent "destinationNumber" #f) (hashtable-ref sent "destinationUuid" #f) (hashtable-ref sent "destination" #f) + (hashtable-ref sent "destinationNumber" #f) "unknown")] [kind (if group-target 'group 'direct)] [target (if group-target group-target @@ -2470,9 +2590,9 @@ "unknown")) (def (envelope-source-target envelope) - (let ([target (or (hashtable-ref envelope "sourceNumber" #f) - (hashtable-ref envelope "sourceUuid" #f) + (let ([target (or (hashtable-ref envelope "sourceUuid" #f) (hashtable-ref envelope "source" #f) + (hashtable-ref envelope "sourceNumber" #f) #f)]) (and (non-empty-string? target) (not (string=? target "unknown")) @@ -2573,8 +2693,8 @@ (let ([group-v2 (hashtable-ref msg "groupV2" #f)]) (and (hashtable? group-v2) (first-non-empty-string - (hashtable-ref group-v2 "masterKey" #f) - (hashtable-ref group-v2 "id" #f)))))) + (hashtable-ref group-v2 "id" #f) + (hashtable-ref group-v2 "masterKey" #f)))))) (def (message-group-title msg fallback) (let ([group-info (hashtable-ref msg "groupInfo" #f)]) --- a/tests/test-receive-normalization.ss +++ b/tests/test-receive-normalization.ss @@ -48,6 +48,16 @@ "dataMessage" (ht "timestamp" 1000 "message" "hello")))) +(def direct-number-and-uuid + (notif + (ht "sourceNumber" "+15550101" + "sourceUuid" "11111111-2222-3333-4444-555555555555" + "sourceName" "Alice Number" + "sourceDevice" 1 + "timestamp" 1100 + "dataMessage" (ht "timestamp" 1100 + "message" "uuid wins")))) + (def group-current (notif (ht "sourceUuid" "bbbbbbbb-cccc-dddd-eeee-ffffffffffff" @@ -59,6 +69,17 @@ "groupInfo" (ht "groupId" "GROUPID" "groupName" "Ops"))))) +(def group-v2-current + (notif + (ht "sourceUuid" "22222222-3333-4444-5555-666666666666" + "sourceName" "Gina" + "sourceDevice" 1 + "timestamp" 2100 + "dataMessage" (ht "timestamp" 2100 + "message" "group v2 hello" + "groupV2" (ht "id" "GROUPV2ID" + "masterKey" "MASTERKEY"))))) + (def top-level-edit (notif (ht "sourceUuid" "cccccccc-dddd-eeee-ffff-000000000000" @@ -76,6 +97,17 @@ "storyMessage" (ht "groupId" "STORYGROUP" "textAttachment" (ht "text" "story text"))))) +(def sync-sent-both + (notif + (ht "sourceUuid" "eeeeeeee-ffff-0000-1111-222222222222" + "timestamp" 5000 + "syncMessage" (ht "sentMessage" + (ht "destinationNumber" "+15550202" + "destinationUuid" + "33333333-4444-5555-6666-777777777777" + "timestamp" 5000 + "message" "from phone"))))) + (let ([event (notification->chat-event direct-uuid)]) (check "direct sourceUuid promoted to chat event" (and (list? event) (eq? (car event) 'message))) @@ -86,6 +118,13 @@ (string=? (list-ref event 4) "aaaaaaaa-bbbb-cccc-dddd-eeeeeeeeeeee"))) +(let ([event (notification->chat-event direct-number-and-uuid)]) + (check "direct with number and uuid promoted" + (and (list? event) (eq? (car event) 'message))) + (check "sourceUuid beats sourceNumber for stable id" + (string=? (list-ref event 1) + "direct:11111111-2222-3333-4444-555555555555"))) + (let ([event (notification->chat-event group-current)]) (check "group notification promoted" (and (list? event) (eq? (car event) 'message))) @@ -94,6 +133,12 @@ (check "groupName used as title" (string=? (list-ref event 2) "Ops"))) +(let ([event (notification->chat-event group-v2-current)]) + (check "groupV2 notification promoted" + (and (list? event) (eq? (car event) 'message))) + (check "groupV2 id beats masterKey" + (string=? (list-ref event 1) "group:GROUPV2ID"))) + (let ([event (notification->chat-event top-level-edit)]) (check "top-level edit promoted" (and (list? event) (eq? (car event) 'message))) @@ -107,17 +152,27 @@ (check "story group id used" (string=? (list-ref event 1) "group:STORYGROUP"))) +(let ([event (notification->chat-event sync-sent-both)]) + (check "sync sent message promoted" + (and (list? event) (eq? (car event) 'message))) + (check "sync sent destinationUuid beats destinationNumber" + (string=? (list-ref event 1) + "direct:33333333-4444-5555-6666-777777777777"))) + (delete-if-exists! path) (delete-if-exists! (string-append path ".tmp")) (let ([h (logdb-open path "normalization key")]) (check "open normalization log" h) (capture-notification! h "+15550000" direct-uuid) + (capture-notification! h "+15550000" direct-number-and-uuid) (capture-notification! h "+15550000" group-current) + (capture-notification! h "+15550000" group-v2-current) (capture-notification! h "+15550000" top-level-edit) (capture-notification! h "+15550000" group-story) + (capture-notification! h "+15550000" sync-sent-both) (let ([rows (logdb-recent h 10)]) - (check "four displayable rows captured" (= (length rows) 4)) + (check "seven displayable rows captured" (= (length rows) 7)) (check "story captured as displayable row" (member (list "in" "group:STORYGROUP" "Dana" 4000 "data" "story text") @@ -137,6 +192,30 @@ 1000 "data" "hello") + rows)) + (check "sourceUuid preferred over sourceNumber in capture" + (member (list "in" + "direct:11111111-2222-3333-4444-555555555555" + "Alice Number" + 1100 + "data" + "uuid wins") + rows)) + (check "groupV2 id preferred over masterKey in capture" + (member (list "in" + "group:GROUPV2ID" + "Gina" + 2100 + "data" + "group v2 hello") + rows)) + (check "sync destinationUuid preferred in capture" + (member (list "out" + "direct:33333333-4444-5555-6666-777777777777" + "You" + 5000 + "data" + "from phone") rows))) (logdb-close h)) new file mode 100644 --- /dev/null +++ b/tests/test-rpc-demux.ss @@ -0,0 +1,55 @@ +#!chezscheme +;;; Focused tests for JSON-RPC notification/response demux. + +(import (except (chezscheme) + make-hash-table hash-table? + sort sort! + printf fprintf + path-extension path-absolute? + with-input-from-string with-output-to-string + iota 1+ 1- + partition + make-date make-time) + (except (jerboa prelude) meta atom?) + (std text json) + (signal rpc-actor)) + +(def (check label pred) + (unless pred + (error 'test-rpc-demux label))) + +(def (json s) + (string->json-object s)) + +(def receive-no-id + (json "{\"jsonrpc\":\"2.0\",\"method\":\"receive\",\"params\":{}}")) + +(def receive-null-id + (json "{\"jsonrpc\":\"2.0\",\"method\":\"receive\",\"id\":null,\"params\":{}}")) + +(def response + (json "{\"jsonrpc\":\"2.0\",\"id\":1,\"result\":0}")) + +(def request + (json "{\"jsonrpc\":\"2.0\",\"id\":2,\"method\":\"version\"}")) + +(check "receive without id is a notification" + (rpc-notification? receive-no-id)) + +(check "receive with JSON null id is still a notification" + (rpc-notification? receive-null-id)) + +(check "JSON null id is not a response id" + (not (has-rpc-id? receive-null-id))) + +(check "response id is detected" + (has-rpc-id? response)) + +(check "response is not a notification" + (not (rpc-notification? response))) + +(check "request with id is not a notification" + (not (rpc-notification? request))) + +(display "rpc demux ok") +(newline)