Migrate user IDs from email strings to UUIDs#763
Migrate user IDs from email strings to UUIDs#763darioalessandro wants to merge 5 commits intomainfrom
Conversation
👋 Thanks for your contribution!A maintainer will review your changes shortly. ✅ Automated ChecksCI/CD tests will run automatically. Please ensure all checks pass before requesting review. 🚀 Preview DeploymentsMaintainers can test this PR in a live environment using these commands:
Preview URLs will be posted automatically once deployed. |
There was a problem hiding this comment.
good idea Dario. I asked Claude for review. For your consideration:
Critical
1. DiagnosticsPacket.sender_id → sender_session_id reuses field number 2 with a different wire type
protobuf/types/diagnostics_packet.proto — The field changes from string sender_id = 2 to uint64 sender_session_id = 2. Reusing field number 2 with a different wire type means old clients sending string data to a new server (or vice versa) will get deserialization errors, not graceful fallback. In protobuf best practice, the old field number should be reserved and a new field number used. This matters if there's any rolling deployment window where old and new code coexist.
2. Repeated parse_user_id().expect() on every packet in hot paths
In bot/src/audio_producer.rs, video_producer.rs, and webtransport_client.rs, user_id is parsed via parse_user_id(&user_id).expect(...) on every packet send. Should be parsed once at initialization and cached.
Same issue in videocall-client — Inner::user_id_bytes() and VideoCallClient::user_id_bytes() call parse_user_id on every invocation. These are used in on_inbound_media, RTT handling, heartbeats, etc. — hot paths during active calls. Parse once in the constructor and store the result.
3. dbmate down migration is empty
dbmate/db/migrations/20260314000000_add_uuid_user_ids.sql:45 — The -- migrate:down section is empty. dbmate requires this section to exist with valid SQL or it may behave unexpectedly. Add SELECT 1; or RAISE EXCEPTION 'irreversible migration'; so dbmate handles it explicitly.
Moderate
4. Home page auth dropdown shows profile.name twice
dioxus-ui/src/pages/home.rs:160 and yew-ui/src/pages/home.rs:265 — Previously showed name + email. Now:
p { class: "auth-dropdown-name", "{profile.name}" } p { class: "auth-dropdown-email", "{profile.name}" } // duplicateEither hide the second element or remove it. Showing the same string twice with different CSS classes is confusing.
5. "Unknown participant" in waiting room gives host zero identifying info
dioxus-ui/src/components/host_controls.rs and yew-ui/src/components/host_controls.rs — When display_name is empty/None, the waiting room now shows "Unknown participant" instead of the user_id. Correct to not show raw UUIDs, but the host has no way to distinguish multiple waiting participants. Consider showing a truncated UUID or ordinal ("Guest #1", "Guest #2").
6. Tooltip on peer tiles changed from user_id to display_name — now redundant
dioxus-ui/src/components/canvas_generator.rs and yew-ui/src/components/canvas_generator.rs — The title attribute (tooltip) changed from peer_user_id to peer_display_name, making the tooltip identical to the visible text. The tooltip previously served a purpose (showing the underlying identity when display names might clash).
Also: the E2E tests at two-users-meeting.spec.ts:139-151 assert the title contains UUIDs, but the code now sets it to peer_display_name. These assertions will only pass if display_name happens to equal the UUID in the test setup — verify this is intentional.
7. PeerList tooltip same issue
dioxus-ui/src/components/peer_list.rs:230-231 and yew-ui/src/components/peer_list.rs:377 — tooltip changed from user_id to display_name, making it redundant.
Minor / Nits
8. server_diagnostics.rs fallback sends non-UUID bytes
actix-api/src/server_diagnostics.rs:195-199 — When parse_user_id fails, falls back to user_id.as_bytes().to_vec() (variable-length, not a UUID). This could cause downstream confusion in metrics/diagnostics consumers expecting 16-byte UUIDs. Consider failing instead of silently sending malformed data.
9. uuid v4 feature pulls significant WASM transitive deps
Cargo.lock shows new deps: wasip3, wit-bindgen 0.51, wasm-encoder, wasmparser, wit-parser, etc. Worth checking if v4 (random UUID generation) is only needed server-side and can be feature-gated to avoid WASM bundle bloat.
10. Unrelated cleanups mixed in
div_ceil() replacements, is_empty() instead of == "", is_none() instead of matches!(x, None), HashMap::new as closure, clone removals — good cleanups but could be a separate commit for clean bisection.
Summary
DiagnosticsPacket.sender_id→sender_session_id(uint64) to correctly reflect it carries a session ID, not a user UUIDType of Change
Testing
Other
🤖 Generated with Claude Code