Use timestamps with timezones

It generally appears to be recommended to use `timestamptz` (timestamp with time zone) instead of `timestamp` (timestamp without time zone), unless the saved timestamp explicitly should not have a timezone. Since we always treat these timestamps as UTC though, we should be using `timestamptz` instead.

This commit accordingly also switches `NaiveDateTime` for `DateTime<Utc>` in most cases in our codebase. This change also allowed us to remove the `rfc3339` serde helper module, since we no longer need the `DateTime<Utc> -> NaiveDateTime` conversion performed by this module.

Author: Turbo87

crates/crates_io_database/src/models/action.rs

@@ -1,7 +1,7 @@
 use crate::models::{ApiToken, User, Version};
 use crate::schema::*;
 use bon::Builder;
-use chrono::NaiveDateTime;
+use chrono::{DateTime, Utc};
 use crates_io_diesel_helpers::pg_enum;
 use diesel::prelude::*;
 use diesel_async::{AsyncPgConnection, RunQueryDsl};
@@ -49,7 +49,7 @@ pub struct VersionOwnerAction {
     pub user_id: i32,
     pub api_token_id: Option<i32>,
     pub action: VersionAction,
-    pub time: NaiveDateTime,
+    pub time: DateTime<Utc>,
 }
 
 impl VersionOwnerAction {

crates/crates_io_database/src/models/category.rs

@@ -1,6 +1,6 @@
 use crate::models::Crate;
 use crate::schema::*;
-use chrono::NaiveDateTime;
+use chrono::{DateTime, Utc};
 use diesel::dsl;
 use diesel::prelude::*;
 use diesel_async::scoped_futures::ScopedFutureExt;
@@ -15,7 +15,7 @@ pub struct Category {
     pub slug: String,
     pub description: String,
     pub crates_cnt: i32,
-    pub created_at: NaiveDateTime,
+    pub created_at: DateTime<Utc>,
 }
 
 type WithSlug<'a> = dsl::Eq<categories::slug, crates_io_diesel_helpers::lower<&'a str>>;

crates/crates_io_database/src/models/crate_owner_invitation.rs

@@ -1,4 +1,4 @@
-use chrono::{DateTime, NaiveDateTime, Utc};
+use chrono::{DateTime, Utc};
 use diesel::prelude::*;
 use diesel_async::scoped_futures::ScopedFutureExt;
 use diesel_async::{AsyncConnection, AsyncPgConnection, RunQueryDsl};
@@ -63,7 +63,7 @@ pub struct CrateOwnerInvitation {
     pub invited_user_id: i32,
     pub invited_by_user_id: i32,
     pub crate_id: i32,
-    pub created_at: NaiveDateTime,
+    pub created_at: DateTime<Utc>,
     #[diesel(deserialize_as = String)]
     pub token: SecretString,
     pub expires_at: DateTime<Utc>,

crates/crates_io_database/src/models/email.rs

@@ -1,5 +1,5 @@
 use bon::Builder;
-use chrono::NaiveDateTime;
+use chrono::{DateTime, Utc};
 use diesel::prelude::*;
 use diesel_async::{AsyncPgConnection, RunQueryDsl};
 use secrecy::SecretString;
@@ -16,7 +16,7 @@ pub struct Email {
     pub verified: bool,
     #[diesel(deserialize_as = String, serialize_as = String)]
     pub token: SecretString,
-    pub token_generated_at: Option<NaiveDateTime>,
+    pub token_generated_at: Option<DateTime<Utc>>,
 }
 
 #[derive(Debug, Insertable, AsChangeset, Builder)]

crates/crates_io_database/src/models/keyword.rs

@@ -1,4 +1,4 @@
-use chrono::NaiveDateTime;
+use chrono::{DateTime, Utc};
 use diesel::prelude::*;
 use diesel_async::scoped_futures::ScopedFutureExt;
 use diesel_async::{AsyncConnection, AsyncPgConnection, RunQueryDsl};
@@ -12,7 +12,7 @@ pub struct Keyword {
     pub id: i32,
     pub keyword: String,
     pub crates_cnt: i32,
-    pub created_at: NaiveDateTime,
+    pub created_at: DateTime<Utc>,
 }
 
 #[derive(Associations, Insertable, Identifiable, Debug, Clone, Copy)]

crates/crates_io_database/src/models/krate.rs

@@ -2,7 +2,7 @@ use crate::models::helpers::with_count::*;
 use crate::models::version::TopVersions;
 use crate::models::{CrateOwner, Owner, OwnerKind, ReverseDependency, User, Version};
 use crate::schema::*;
-use chrono::NaiveDateTime;
+use chrono::{DateTime, Utc};
 use crates_io_diesel_helpers::canon_crate_name;
 use diesel::associations::Identifiable;
 use diesel::dsl;
@@ -40,8 +40,8 @@ pub struct CrateName {
 pub struct Crate {
     pub id: i32,
     pub name: String,
-    pub updated_at: NaiveDateTime,
-    pub created_at: NaiveDateTime,
+    pub updated_at: DateTime<Utc>,
+    pub created_at: DateTime<Utc>,
     pub description: Option<String>,
     pub homepage: Option<String>,
     pub documentation: Option<String>,

crates/crates_io_database/src/models/token.rs

@@ -1,16 +1,16 @@
 mod scopes;
 
 use bon::Builder;
-use chrono::NaiveDateTime;
+use chrono::{DateTime, Utc};
 use diesel::dsl::now;
 use diesel::prelude::*;
+use diesel::sql_types::Timestamptz;
 use diesel_async::scoped_futures::ScopedFutureExt;
 use diesel_async::{AsyncConnection, AsyncPgConnection, RunQueryDsl};
 
 pub use self::scopes::{CrateScope, EndpointScope};
 use crate::models::User;
 use crate::schema::api_tokens;
-use crate::utils::rfc3339;
 use crate::utils::token::{HashedToken, PlainToken};
 
 #[derive(Debug, Insertable, Builder)]
@@ -25,7 +25,7 @@ pub struct NewApiToken {
     pub crate_scopes: Option<Vec<CrateScope>>,
     /// A list of endpoint scopes or `None` for the `legacy` endpoint scope (see RFC #2947)
     pub endpoint_scopes: Option<Vec<EndpointScope>>,
-    pub expired_at: Option<NaiveDateTime>,
+    pub expired_at: Option<DateTime<Utc>>,
 }
 
 impl NewApiToken {
@@ -46,18 +46,15 @@ pub struct ApiToken {
     #[serde(skip)]
     pub user_id: i32,
     pub name: String,
-    #[serde(with = "rfc3339")]
-    pub created_at: NaiveDateTime,
-    #[serde(with = "rfc3339::option")]
-    pub last_used_at: Option<NaiveDateTime>,
+    pub created_at: DateTime<Utc>,
+    pub last_used_at: Option<DateTime<Utc>>,
     #[serde(skip)]
     pub revoked: bool,
     /// `None` or a list of crate scope patterns (see RFC #2947)
     pub crate_scopes: Option<Vec<CrateScope>>,
     /// A list of endpoint scopes or `None` for the `legacy` endpoint scope (see RFC #2947)
     pub endpoint_scopes: Option<Vec<EndpointScope>>,
-    #[serde(with = "rfc3339::option")]
-    pub expired_at: Option<NaiveDateTime>,
+    pub expired_at: Option<DateTime<Utc>>,
 }
 
 impl ApiToken {
@@ -80,7 +77,7 @@ impl ApiToken {
             .transaction(|conn| {
                 async move {
                     diesel::update(tokens)
-                        .set(api_tokens::last_used_at.eq(now.nullable()))
+                        .set(api_tokens::last_used_at.eq(now.into_sql::<Timestamptz>().nullable()))
                         .returning(ApiToken::as_returning())
                         .get_result(conn)
                         .await
@@ -111,20 +108,23 @@ mod tests {
             created_at: NaiveDate::from_ymd_opt(2017, 1, 6)
                 .unwrap()
                 .and_hms_opt(14, 23, 11)
-                .unwrap(),
-            last_used_at: NaiveDate::from_ymd_opt(2017, 1, 6)
                 .unwrap()
-                .and_hms_opt(14, 23, 12),
+                .and_utc(),
+            last_used_at: Some(
+                NaiveDate::from_ymd_opt(2017, 1, 6)
+                    .unwrap()
+                    .and_hms_opt(14, 23, 12)
+                    .unwrap()
+                    .and_utc(),
+            ),
             crate_scopes: None,
             endpoint_scopes: None,
             expired_at: None,
         };
         let json = serde_json::to_string(&tok).unwrap();
+        assert_some!(json.as_str().find(r#""created_at":"2017-01-06T14:23:11Z""#));
         assert_some!(json
             .as_str()
-            .find(r#""created_at":"2017-01-06T14:23:11+00:00""#));
-        assert_some!(json
-            .as_str()
-            .find(r#""last_used_at":"2017-01-06T14:23:12+00:00""#));
+            .find(r#""last_used_at":"2017-01-06T14:23:12Z""#));
     }
 }

crates/crates_io_database/src/models/user.rs

@@ -1,5 +1,5 @@
 use bon::Builder;
-use chrono::NaiveDateTime;
+use chrono::{DateTime, Utc};
 use diesel::dsl::sql;
 use diesel::prelude::*;
 use diesel::sql_types::Integer;
@@ -22,7 +22,7 @@ pub struct User {
     pub gh_avatar: Option<String>,
     pub gh_id: i32,
     pub account_lock_reason: Option<String>,
-    pub account_lock_until: Option<NaiveDateTime>,
+    pub account_lock_until: Option<DateTime<Utc>>,
     pub is_admin: bool,
     pub publish_notifications: bool,
 }

crates/crates_io_database/src/models/version.rs

@@ -1,7 +1,7 @@
 use std::collections::BTreeMap;
 
 use bon::Builder;
-use chrono::NaiveDateTime;
+use chrono::{DateTime, Utc};
 use crates_io_index::features::FeaturesMap;
 use diesel::prelude::*;
 use diesel_async::scoped_futures::ScopedFutureExt;
@@ -18,8 +18,8 @@ pub struct Version {
     pub id: i32,
     pub crate_id: i32,
     pub num: String,
-    pub updated_at: NaiveDateTime,
-    pub created_at: NaiveDateTime,
+    pub updated_at: DateTime<Utc>,
+    pub created_at: DateTime<Utc>,
     pub downloads: i32,
     pub features: serde_json::Value,
     pub yanked: bool,
@@ -84,7 +84,7 @@ pub struct NewVersion<'a> {
     num: &'a str,
     #[builder(default = strip_build_metadata(num))]
     pub num_no_build: &'a str,
-    created_at: Option<&'a NaiveDateTime>,
+    created_at: Option<&'a DateTime<Utc>>,
     yanked: Option<bool>,
     #[builder(default = serde_json::Value::Object(Default::default()))]
     features: serde_json::Value,
@@ -170,10 +170,10 @@ impl TopVersions {
     /// highest version (in semver order) for a collection of date/version pairs.
     pub fn from_date_version_pairs<T>(pairs: T) -> Self
     where
-        T: IntoIterator<Item = (NaiveDateTime, String)>,
+        T: IntoIterator<Item = (DateTime<Utc>, String)>,
     {
         // filter out versions that we can't parse
-        let pairs: Vec<(NaiveDateTime, semver::Version)> = pairs
+        let pairs: Vec<(DateTime<Utc>, semver::Version)> = pairs
             .into_iter()
             .filter_map(|(date, version)| {
                 semver::Version::parse(&version)
@@ -201,12 +201,12 @@ impl TopVersions {
 
 #[cfg(test)]
 mod tests {
-    use super::TopVersions;
+    use super::*;
     use chrono::NaiveDateTime;
 
     #[track_caller]
-    fn date(str: &str) -> NaiveDateTime {
-        str.parse().unwrap()
+    fn date(str: &str) -> DateTime<Utc> {
+        str.parse::<NaiveDateTime>().unwrap().and_utc()
     }
 
     #[track_caller]

crates/crates_io_database/src/schema.patch

@@ -22,11 +22,11 @@
 +        endpoint_scopes -> Nullable<Array<Text>>,
          /// The `expired_at` column of the `api_tokens` table.
          ///
-         /// Its SQL type is `Nullable<Timestamp>`.
+         /// Its SQL type is `Nullable<Timestamptz>`.
 @@ -180,12 +178,6 @@
          ///
          /// (Automatically generated by Diesel.)
-         created_at -> Timestamp,
+         created_at -> Timestamptz,
 -        /// The `path` column of the `categories` table.
 -        ///
 -        /// Its SQL type is `Ltree`.