fix: preserve the uid ordering by sorting in reverse (#2017)

for the case when created_ats are identical (likely just in tests) we
want the uid DESC from the get_users query used as a tie breaker

Closes STOR-462

Author: pjenvey

tokenserver-db-common/src/lib.rs

@@ -5,7 +5,10 @@ mod error;
 pub mod params;
 pub mod results;
 
-use std::time::{Duration, SystemTime, UNIX_EPOCH};
+use std::{
+    cmp,
+    time::{Duration, SystemTime, UNIX_EPOCH},
+};
 
 use async_trait::async_trait;
 use syncserver_common::Metrics;
@@ -127,8 +130,8 @@ pub trait Db {
                 old_client_states: vec![],
             })
         } else {
-            raw_users.sort_by_key(|raw_user| (raw_user.generation, raw_user.created_at));
-            raw_users.reverse();
+            raw_users
+                .sort_by_key(|raw_user| cmp::Reverse((raw_user.generation, raw_user.created_at)));
 
             // The user with the greatest `generation` and `created_at` is the current user
             let raw_user = raw_users[0].clone();

tokenserver-db/src/tests.rs

@@ -1192,6 +1192,80 @@ async fn test_correct_created_at_used_during_user_retrieval() -> DbResult<()> {
     Ok(())
 }
 
+#[tokio::test]
+async fn test_latest_created_at() -> DbResult<()> {
+    let pool = db_pool().await?;
+    let mut db = pool.get().await?;
+
+    let service_id = db
+        .get_service_id(params::GetServiceId {
+            service: "sync-1.5".to_owned(),
+        })
+        .await?
+        .id;
+
+    // Add a node
+    let node_id = db
+        .post_node(params::PostNode {
+            service_id,
+            node: "https://node1".to_owned(),
+            current_load: 0,
+            capacity: 100,
+            available: 100,
+            ..Default::default()
+        })
+        .await?
+        .id;
+
+    let email = "test_user";
+    let now = SystemTime::now()
+        .duration_since(UNIX_EPOCH)
+        .unwrap()
+        .as_millis() as i64;
+
+    // Add a user marked as replaced
+    let post_user = params::PostUser {
+        service_id,
+        email: email.to_owned(),
+        node_id,
+        client_state: "aaaa".to_owned(),
+        generation: 1234,
+        keys_changed_at: Some(1234),
+        created_at: now,
+    };
+    let uid1 = db.post_user(post_user.clone()).await?.uid;
+    db.replace_user(params::ReplaceUser {
+        uid: uid1,
+        service_id,
+        replaced_at: now,
+    })
+    .await?;
+
+    // User's latest record w/ a new client_state, otherwise identical to the
+    // replaced record (even created_at)
+    let uid2 = db
+        .post_user(params::PostUser {
+            client_state: "bbbb".to_owned(),
+            ..post_user
+        })
+        .await?
+        .uid;
+    assert_ne!(uid1, uid2);
+
+    // Should return the latest record even with the identical created_at
+    let user = db
+        .get_or_create_user(params::GetOrCreateUser {
+            service_id,
+            email: email.to_owned(),
+            ..Default::default()
+        })
+        .await?;
+    assert_eq!(user.uid, uid2);
+    assert_eq!(user.client_state, "bbbb");
+
+    Ok(())
+}
+
 #[tokio::test]
 async fn test_get_spanner_node() -> DbResult<()> {
     let pool = db_pool().await?;