fix(db): fix bugs based on sqlite date format difference from chrono (#…

…1951)

* fix(db): fix repository refresh job

* Provide fix for sqlite date format

* Update implementation for repository provider

* Fix formatting

* Add compiletime test case for operation types

* Fix tests

* Make end bound exclusive

* Fix formatting

ee/tabby-db/src/github_repository_provider.rs

@@ -3,7 +3,7 @@ use chrono::{DateTime, Utc};
 use sqlx::{prelude::FromRow, query, query_as};
 use tabby_db_macros::query_paged_as;
 
-use crate::{DbConn, SQLXResultExt};
+use crate::{AsSQLiteDateTime, DbConn, SQLXResultExt};
 
 #[derive(FromRow)]
 pub struct GithubRepositoryProviderDAO {
@@ -156,8 +156,12 @@ impl DbConn {
     ) -> Result<i64> {
         let res = query!(
             "INSERT INTO github_provided_repositories (github_repository_provider_id, vendor_id, name, git_url) VALUES ($1, $2, $3, $4)
-                ON CONFLICT(github_repository_provider_id, vendor_id) DO UPDATE SET github_repository_provider_id = $1, name = $2, git_url = $3",
-            github_provider_id, vendor_id, name, git_url).execute(&self.pool).await?;
+                ON CONFLICT(github_repository_provider_id, vendor_id) DO UPDATE SET name = $3, git_url = $4, updated_at = DATETIME('now')",
+            github_provider_id,
+            vendor_id,
+            name,
+            git_url
+        ).execute(&self.pool).await?;
         Ok(res.last_insert_rowid())
     }
 
@@ -177,6 +181,7 @@ impl DbConn {
         github_provider_id: i64,
         cutoff_timestamp: DateTime<Utc>,
     ) -> Result<()> {
+        let cutoff_timestamp = cutoff_timestamp.as_sqlite_datetime();
         query!(
             "DELETE FROM github_provided_repositories WHERE github_repository_provider_id = ? AND updated_at < ?;",
             github_provider_id,

ee/tabby-db/src/lib.rs

@@ -1,9 +1,14 @@
-use std::{ops::Deref, path::Path, sync::Arc};
+use std::{
+    fmt::Display,
+    ops::{Add, Deref, Sub},
+    path::Path,
+    sync::Arc,
+};
 
 use anyhow::anyhow;
 use cache::Cache;
 use cached::TimedSizedCache;
-use chrono::{DateTime, NaiveDateTime, Utc};
+use chrono::{DateTime, Duration, NaiveDateTime, Utc};
 pub use email_setting::EmailSettingDAO;
 pub use github_repository_provider::{GithubProvidedRepositoryDAO, GithubRepositoryProviderDAO};
 pub use invitations::InvitationDAO;
@@ -256,9 +261,53 @@ where
     }
 }
 
+pub trait AsSQLiteDateTime {
+    fn as_sqlite_datetime(&self) -> String;
+}
+
+impl AsSQLiteDateTime for DateTime<Utc> {
+    fn as_sqlite_datetime(&self) -> String {
+        self.format("%F %X").to_string()
+    }
+}
+
 #[derive(Default, Clone)]
 pub struct DateTimeUtc(DateTime<Utc>);
 
+impl Display for DateTimeUtc {
+    fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result {
+        write!(f, "{}", self.as_sqlite_datetime())
+    }
+}
+
+impl std::fmt::Debug for DateTimeUtc {
+    fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result {
+        write!(f, "{}", self.as_sqlite_datetime())
+    }
+}
+
+impl DateTimeUtc {
+    pub fn now() -> Self {
+        Utc::now().into()
+    }
+}
+
+impl Add<Duration> for DateTimeUtc {
+    type Output = Self;
+
+    fn add(self, rhs: Duration) -> Self::Output {
+        ((self.0) + rhs).into()
+    }
+}
+
+impl Sub<Duration> for DateTimeUtc {
+    type Output = Self;
+
+    fn sub(self, rhs: Duration) -> Self::Output {
+        ((self.0) - rhs).into()
+    }
+}
+
 impl From<DateTime<Utc>> for DateTimeUtc {
     fn from(value: DateTime<Utc>) -> Self {
         Self(value)
@@ -291,7 +340,7 @@ impl<'a> Encode<'a, Sqlite> for DateTimeUtc {
         &self,
         buf: &mut <Sqlite as sqlx::database::HasArguments<'a>>::ArgumentBuffer,
     ) -> sqlx::encode::IsNull {
-        self.0.encode_by_ref(buf)
+        <String as Encode<Sqlite>>::encode(self.0.as_sqlite_datetime(), buf)
     }
 }
 
@@ -352,6 +401,34 @@ mod tests {
         assert_eq!(new_token.len(), 36);
         assert_ne!(old_token, new_token);
     }
+
+    #[tokio::test]
+    async fn test_timestamp_format() {
+        let db = DbConn::new_in_memory().await.unwrap();
+
+        let time = DateTimeUtc::now();
+
+        let time_str = time.as_sqlite_datetime();
+        let sql_time: String = sqlx::query_scalar::<_, String>("SELECT ?;")
+            .bind(time)
+            .fetch_one(&db.pool)
+            .await
+            .unwrap();
+
+        assert_eq!(time_str, sql_time);
+
+        let sql_time: String = sqlx::query_scalar::<_, String>("SELECT DATETIME('now');")
+            .fetch_one(&db.pool)
+            .await
+            .unwrap();
+        assert_eq!(sql_time, DateTimeUtc::now().as_sqlite_datetime());
+
+        // No assertions, these will fail at compiletime if adding/subtracting from these types
+        // yields DateTime<Utc>, which could be dangerous
+        let time = DateTimeUtc::now();
+        let _added_time: DateTimeUtc = time.clone() + Duration::milliseconds(1);
+        let _subbed_time: DateTimeUtc = time - Duration::milliseconds(1);
+    }
 }
 
 #[cfg(any(test, feature = "testutils"))]

ee/tabby-db/src/user_completions.rs

@@ -5,7 +5,7 @@ use cached::CachedAsync;
 use chrono::{DateTime, Utc};
 use sqlx::{prelude::FromRow, query};
 
-use crate::{DateTimeUtc, DbConn};
+use crate::{AsSQLiteDateTime, DateTimeUtc, DbConn};
 
 #[derive(FromRow)]
 pub struct UserCompletionDAO {
@@ -41,7 +41,9 @@ impl DbConn {
         let duration = Duration::from_millis(ts as u64);
         let created_at =
             DateTime::from_timestamp(duration.as_secs() as i64, duration.subsec_nanos())
-                .context("Invalid created_at timestamp")?;
+                .context("Invalid created_at timestamp")?
+                .as_sqlite_datetime();
+
         let res = query!(
             "INSERT INTO user_completions (user_id, completion_id, language, created_at) VALUES (?, ?, ?, ?);",
             user_id,
@@ -109,7 +111,7 @@ impl DbConn {
                    SUM(selects) as selects,
                    SUM(views) as views
             FROM user_completions JOIN users ON user_id = users.id AND users.active
-            WHERE user_completions.created_at >= DATE('now', '-1 year')
+            WHERE user_completions.created_at >= DATETIME('now', '-1 year')
                 AND ({users_empty} OR user_id IN ({users}))
             GROUP BY 1, 2
             ORDER BY 1, 2 ASC
@@ -171,8 +173,8 @@ impl DbConn {
             no_selected_users = users.is_empty(),
             no_selected_languages = languages.is_empty(),
         ))
-        .bind(start)
-        .bind(end)
+        .bind(start.as_sqlite_datetime())
+        .bind(end.as_sqlite_datetime())
         .fetch_all(&self.pool)
         .await?;
         Ok(res)

ee/tabby-db/src/user_events.rs

@@ -5,7 +5,7 @@ use chrono::{DateTime, Utc};
 use sqlx::{prelude::FromRow, query};
 use tabby_db_macros::query_paged_as;
 
-use crate::DbConn;
+use crate::{AsSQLiteDateTime, DbConn};
 
 #[derive(FromRow)]
 pub struct UserEventDAO {
@@ -50,8 +50,8 @@ impl DbConn {
     ) -> Result<Vec<UserEventDAO>> {
         let condition = Some(format!(
             "created_at >= '{}' AND created_at < '{}'",
-            start.to_rfc3339(),
-            end.to_rfc3339()
+            start.as_sqlite_datetime(),
+            end.as_sqlite_datetime()
         ));
         let events = query_paged_as!(
             UserEventDAO,

ee/tabby-webserver/src/integrations/github.rs

@@ -62,7 +62,7 @@ pub fn routes(
     };
     Router::new()
         .route("/connect/:id", routing::get(connect))
-        // Routes defined past this point require authentication
+        // Routes defined above this point require authentication
         .layer(from_fn_with_state(auth, require_login_middleware))
         .route("/callback", routing::get(callback))
         .with_state(state)

ee/tabby-webserver/src/service/analytic.rs

@@ -90,7 +90,7 @@ pub fn new_analytic_service(db: DbConn) -> Arc<dyn AnalyticService> {
 
 #[cfg(test)]
 mod tests {
-    use chrono::Days;
+    use chrono::{Days, Duration};
 
     use super::*;
     use crate::service::AsID;
@@ -200,7 +200,7 @@ mod tests {
             .unwrap();
 
         let svc = new_analytic_service(db);
-        let end = Utc::now();
+        let end = Utc::now() + Duration::days(1);
         let start = end.checked_sub_days(Days::new(100)).unwrap();
 
         // Test that there is a single completion stat (1 day of history) with 1 completion and 1 select
@@ -243,7 +243,7 @@ mod tests {
             .unwrap();
 
         let service = new_analytic_service(db);
-        let end = Utc::now();
+        let end = Utc::now() + Duration::days(1);
         let start = end.checked_sub_days(Days::new(100)).unwrap();
 
         let stats = service
@@ -288,7 +288,7 @@ mod tests {
             .unwrap()
             .is_empty());
 
-        let end = Utc::now();
+        let end = Utc::now() + Duration::days(1);
         let start = end.checked_sub_days(Days::new(100)).unwrap();
 
         assert!(service