Page MenuHomePhabricator

identity] Add function to get current device list timestamp
ClosedPublic

Authored by bartek on Dec 7 2023, 12:09 AM.
Tags
None
Referenced Files
F2154120: D10216.id34891.diff
Sun, Jun 30, 6:41 PM
F2153303: D10216.id.diff
Sun, Jun 30, 4:51 PM
Unknown Object (File)
Sat, Jun 29, 3:00 AM
Unknown Object (File)
Fri, Jun 28, 8:31 PM
Unknown Object (File)
Thu, Jun 27, 4:26 PM
Unknown Object (File)
Wed, Jun 26, 6:47 PM
Unknown Object (File)
Wed, Jun 26, 4:14 PM
Unknown Object (File)
Tue, Jun 25, 12:53 PM
Subscribers

Details

Summary

When doing a device list update, we need to make sure no concurrent update hapoened in meanwhile.
We can do it by storing last update time, then each update is a three-step transaction:

  1. Get last update time
  2. Do the update
  3. Commit the update only if the last update time is the same as the one we got in step 1.

This function will return the current timestamp of the device list (a.k.a last update time).

Depends on D10215

Test Plan

Manually created this attribute and checked that it is returned by the function.

Diff Detail

Repository
rCOMM Comm
Lint
Lint Not Applicable
Unit
Tests Not Applicable

Event Timeline

bartek held this revision as a draft.
bartek published this revision for review.Dec 7 2023, 1:04 AM
bartek added inline comments.
services/identity/src/database/device_list.rs
311–312 ↗(On Diff #34366)

I commented about this in D10213 - Decided to introduce this pattern to avoid adding all these functions directly in impl DatabaseClient because that file is already huge.
I'd like to keep these in a separate file at least until device list is fully migrated.

I'm aware that this has a downside that some functions are more difficult to find if not in one place, but "public" API is going to have a separate abstraction layer directly in DatabaseClient (see e.g. D10221)

varun added inline comments.
services/identity/src/database/device_list.rs
308 ↗(On Diff #34366)
This revision is now accepted and ready to land.Dec 7 2023, 9:25 PM
services/identity/src/database/device_list.rs
311–312 ↗(On Diff #34366)

Could we use multiple impl blocks (one in each file)?

services/identity/src/database/device_list.rs
311–312 ↗(On Diff #34366)

yeah this is what i was going to suggest too

services/identity/src/database/device_list.rs
311–312 ↗(On Diff #34366)

@bartek was this feedback responded to? Not good enough at Rust to be able to tell 😅

services/identity/src/database/device_list.rs
311–312 ↗(On Diff #34366)

Yeah, the device_list.rs has its own impl DatabaseClient block for all functions except private helpers (like this one here). See e.g. D10217