fix(propdefs): insert actual seen_at, floor less aggressively (#24865)

rust/property-defs-rs/src/types.rs

@@ -142,11 +142,11 @@ impl From<&Event> for EventDefinition {
         EventDefinition {
             name: sanitize_event_name(&event.event),
             team_id: event.team_id,
-            // We round last seen to the nearest day, as per the TS impl. Unwrap is safe here because we
+            // We round last seen to the nearest hour. Unwrap is safe here because we
             // the duration is positive, non-zero, and smaller than time since epoch. We use this
             // in the hash value, so updates which would modify this in the DB are issued even
             // if another otherwise-identical event definition is in the cache
-            last_seen_at: floor_datetime(Utc::now(), Duration::days(1)).unwrap(),
+            last_seen_at: floor_datetime(Utc::now(), Duration::hours(1)).unwrap(),
         }
     }
 }
@@ -427,7 +427,7 @@ impl EventDefinition {
             Uuid::now_v7(),
             self.name,
             self.team_id,
-            self.last_seen_at
+            Utc::now() // We floor the update datetime to the nearest day for cache purposes, but can insert the exact time we see the event
         ).execute(executor).await.map(|_| ())
     }
 }
@@ -479,3 +479,21 @@ impl EventProperty {
         .map(|_| ())
     }
 }
+
+#[cfg(test)]
+mod test {
+    use chrono::{Timelike, Utc};
+
+    use crate::types::floor_datetime;
+
+    #[test]
+    fn test_date_flooring() {
+        let timestamp = Utc::now();
+        let rounded = floor_datetime(timestamp, chrono::Duration::days(1)).unwrap();
+        assert_eq!(rounded.hour(), 0);
+        assert_eq!(rounded.minute(), 0);
+        assert_eq!(rounded.second(), 0);
+        assert_eq!(rounded.nanosecond(), 0);
+        assert!(rounded <= timestamp);
+    }
+}

rust/property-defs-rs/tests/updates.rs

@@ -1,5 +1,5 @@
 use chrono::{DateTime, Duration, Utc};
-use property_defs_rs::types::{floor_datetime, Event, PropertyParentType, PropertyValueType};
+use property_defs_rs::types::{Event, PropertyParentType, PropertyValueType};
 use serde_json::json;
 use sqlx::PgPool;
 
@@ -32,14 +32,16 @@ async fn test_updates(db: PgPool) {
     let updates = event.into_updates(1000);
     assert!(updates.len() == 13);
 
+    let before = Utc::now();
+
     // issue them, and then query the database to see that everthing we expect to exist does
     for update in updates {
         update.issue(&db).await.unwrap();
     }
 
-    let today = floor_datetime(Utc::now(), Duration::days(1)).unwrap();
-
-    assert_eventdefinition_exists(&db, "update", 1, today).await;
+    assert_eventdefinition_exists(&db, "update", 1, before, Duration::seconds(1))
+        .await
+        .unwrap();
     assert_propertydefinition_exists(
         &db,
         "TIMESTAMP",
@@ -48,7 +50,8 @@ async fn test_updates(db: PgPool) {
         1,
         PropertyValueType::DateTime,
     )
-    .await;
+    .await
+    .unwrap();
     assert_propertydefinition_exists(
         &db,
         "some_string",
@@ -57,7 +60,8 @@ async fn test_updates(db: PgPool) {
         1,
         PropertyValueType::String,
     )
-    .await;
+    .await
+    .unwrap();
     assert_propertydefinition_exists(
         &db,
         "some_int",
@@ -66,7 +70,8 @@ async fn test_updates(db: PgPool) {
         1,
         PropertyValueType::Numeric,
     )
-    .await;
+    .await
+    .unwrap();
     assert_propertydefinition_exists(
         &db,
         "some_float",
@@ -75,7 +80,8 @@ async fn test_updates(db: PgPool) {
         1,
         PropertyValueType::Numeric,
     )
-    .await;
+    .await
+    .unwrap();
     assert_propertydefinition_exists(
         &db,
         "some_bool",
@@ -84,7 +90,8 @@ async fn test_updates(db: PgPool) {
         1,
         PropertyValueType::Boolean,
     )
-    .await;
+    .await
+    .unwrap();
     assert_propertydefinition_exists(
         &db,
         "some_bool_as_string",
@@ -93,30 +100,41 @@ async fn test_updates(db: PgPool) {
         1,
         PropertyValueType::Boolean,
     )
-    .await;
+    .await
+    .unwrap();
 }
 
 async fn assert_eventdefinition_exists(
     db: &PgPool,
     name: &str,
     team_id: i32,
-    last_seen_at: DateTime<Utc>,
-) {
+    before: DateTime<Utc>,
+    last_seen_range: Duration,
+) -> Result<(), ()> {
+    // Event definitions are inserted with a last_seen that's exactly when the insert is done. We check if an entry exists in the
+    // database with a last_seen that's in the right range to ensure that the event definition was inserted correctly.
+    let after = before + last_seen_range;
+
     let count: Option<i64> = sqlx::query_scalar(
         r#"
         SELECT COUNT(*)
         FROM posthog_eventdefinition
-        WHERE name = $1 AND team_id = $2 AND last_seen_at = $3
+        WHERE name = $1 AND team_id = $2 AND last_seen_at >= $3 AND last_seen_at <= $4
         "#,
     )
     .bind(name)
     .bind(team_id)
-    .bind(last_seen_at)
+    .bind(before)
+    .bind(after)
     .fetch_one(db)
     .await
     .unwrap();
 
-    assert_eq!(count, Some(1));
+    if count == Some(1) {
+        Ok(())
+    } else {
+        Err(())
+    }
 }
 
 async fn assert_propertydefinition_exists(
@@ -126,7 +144,7 @@ async fn assert_propertydefinition_exists(
     is_numerical: bool,
     team_id: i32,
     property_type: PropertyValueType,
-) {
+) -> Result<(), ()> {
     println!("Checking property definition for {}", name);
     let count: Option<i64> = sqlx::query_scalar(
         r#"
@@ -144,5 +162,9 @@ async fn assert_propertydefinition_exists(
     .await
     .unwrap();
 
-    assert_eq!(count, Some(1));
+    if count == Some(1) {
+        Ok(())
+    } else {
+        Err(())
+    }
 }