[native] implement `GetDeviceListsForUsers` RPC in `native_rust_library`
ClosedPublic
Actions

Authored by kamil on Apr 29 2024, 7:25 AM.

Details

Reviewers

tomek
bartek

Commits

rCOMM760b1b52559e: [native] implement `GetDeviceListsForUsers` RPC in `native_rust_library`

Summary

Added client method calling the RPC to native rust library, exposed it to C++.

Test Plan

Tested in D11834

Diff Detail

Repository

rCOMM Comm

Branch

init-peer-list

Lint

No Lint Coverage

Unit

No Test Coverage

Event Timeline

kamil created this revision.Apr 29 2024, 7:25 AM

kamil held this revision as a draft.

Herald added a subscriber: ashoat. · View Herald TranscriptApr 29 2024, 7:25 AM

Harbormaster completed remote builds in B28577: Diff 39657.Apr 29 2024, 7:42 AM

kamil added a child revision: D11833: [CommRustModule] add JSI bindings for `GetDeviceListsForUsers` RPC.Apr 29 2024, 7:46 AM

kamil edited the test plan for this revision. (Show Details)Apr 29 2024, 7:59 AM

kamil published this revision for review.Apr 29 2024, 8:50 AM

tomek accepted this revision.Apr 30 2024, 1:40 AM

tomek added inline comments.

native/native_rust_library/src/identity/device_list.rs
74–101	Why do we need two separate endpoints? It seems like `get_device_list_for_user_helper` should be a special case of `get_device_lists_for_users_helper`.
116	There's a difference between this API and `get_device_list_for_user` - only in one of them we're using `since_timestamp`. Do we know why is that? @bartek

This revision is now accepted and ready to land.Apr 30 2024, 1:40 AM

Discussed this in the office with @tomek and going to land this, we can continue the discussion when @bartek is back but this code is only a wrapper to already landed D11570.
When we decide to make changes we'd need to update protos so then we can also update the code here.

Closed by commit rCOMM760b1b52559e: [native] implement `GetDeviceListsForUsers` RPC in `native_rust_library` (authored by kamil). · Explain WhyApr 30 2024, 3:23 AM

This revision was automatically updated to reflect the committed changes.

kamil added a commit: rCOMM760b1b52559e: [native] implement `GetDeviceListsForUsers` RPC in `native_rust_library`.

bartek added inline comments.May 5 2024, 11:05 PM

native/native_rust_library/src/identity/device_list.rs
74–101	These are different endpoints so it should be the way it is done in this diff
116	GetDeviceList_[History]_ForUser - this is for fetching single user's device list history since given timestamp. It's used for device list verification when receiving an update from peer. GetDeviceListsForUsers (introduced here) - fetches only latest device list for multiple users. Used in the initial migration to peer list, to avoid calling the former RPC in a loop for hundreds of users https://linear.app/comm/issue/ENG-7591/add-rpc-for-fetching-devicelists-for-multiple-users