RUST-1129 Make the dependency on chrono optional and provide equivalent time interop
#352
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -26,7 +26,6 @@ use std::{ | |
| fmt::{self, Debug, Display, Formatter}, | ||
| }; | ||
|
|
||
| use serde_json::{json, Value}; | ||
|
|
||
| pub use crate::document::Document; | ||
|
|
@@ -311,6 +310,14 @@ impl From<oid::ObjectId> for Bson { | |
| } | ||
| } | ||
|
|
||
| #[cfg(feature = "time-0_3")] | ||
| #[cfg_attr(docsrs, doc(cfg(feature = "time-0_3")))] | ||
| impl From<time::OffsetDateTime> for Bson { | ||
| fn from(a: time::OffsetDateTime) -> Bson { | ||
| Bson::DateTime(crate::DateTime::from(a)) | ||
| } | ||
| } | ||
|
|
||
| #[cfg(feature = "chrono-0_4")] | ||
| #[cfg_attr(docsrs, doc(cfg(feature = "chrono-0_4")))] | ||
| impl<T: chrono::TimeZone> From<chrono::DateTime<T>> for Bson { | ||
|
|
@@ -430,7 +437,7 @@ impl Bson { | |
| }) | ||
| } | ||
| Bson::ObjectId(v) => json!({"$oid": v.to_hex()}), | ||
| Bson::DateTime(v) if v.timestamp_millis() >= 0 && v.to_time().year() <= 99999 => { | ||
|
There was a problem hiding this comment. I'm not sure if the year constraint here is correct - it's different than the one below. There was a problem hiding this comment. from my reading of the extJSON spec we should drop a 9 here to match below... see datetime entries in this table. I was wondering how we would pass the corresponding corpus test, but it looks like that test case does not have a relaxed extJSON version specified so we likely make no assertions on it -- maybe it's omitted because it's the same as the canonical, but IMO it seems like test runners should probably confirm their relaxed output in that case matches canonical. perhaps we should add an assertion to our corpus runner so that for a test case with no expected relaxed output, we check that the relaxed output is equal to the expected canonical output? if that just works, I'd propose we file a DRIVERS ticket that suggests amending the corpus spec to require this assertion. thoughts? There was a problem hiding this comment. It looks like that doesn't just work, unfortunately. I filed RUST-1261 to follow up on that. |
||
| json!({ | ||
| "$date": v.to_rfc3339_string(), | ||
| }) | ||
|
|
@@ -592,7 +599,7 @@ impl Bson { | |
| Bson::DateTime(v) if rawbson => doc! { | ||
| "$date": v.timestamp_millis(), | ||
| }, | ||
| Bson::DateTime(v) if v.timestamp_millis() >= 0 && v.to_time().year() <= 9999 => { | ||
| doc! { | ||
| "$date": v.to_rfc3339_string(), | ||
| } | ||
|
|
@@ -776,8 +783,8 @@ impl Bson { | |
| } | ||
|
|
||
| if let Ok(date) = doc.get_str("$date") { | ||
| if let Ok(dt) = crate::DateTime::parse_rfc3339_str(date) { | ||
| return Bson::DateTime(dt); | ||
| } | ||
| } | ||
| } | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,16 +1,25 @@ | ||
| use std::{ | ||
| convert::TryInto, | ||
| error, | ||
| fmt::{self, Display}, | ||
| result, | ||
| time::{Duration, SystemTime}, | ||
| }; | ||
|
|
||
| use time::format_description::well_known::Rfc3339; | ||
|
|
||
| #[cfg(feature = "chrono-0_4")] | ||
| use chrono::{LocalResult, TimeZone, Utc}; | ||
| #[cfg(all( | ||
| feature = "serde_with", | ||
| any(feature = "chrono-0_4", feature = "time-0_3") | ||
| ))] | ||
| use serde::{Deserialize, Deserializer, Serialize}; | ||
| #[cfg(all( | ||
| feature = "serde_with", | ||
| any(feature = "chrono-0_4", feature = "time-0_3") | ||
| ))] | ||
| use serde_with::{DeserializeAs, SerializeAs}; | ||
|
|
||
| /// Struct representing a BSON datetime. | ||
| /// Note: BSON datetimes have millisecond precision. | ||
|
|
@@ -114,11 +123,6 @@ impl crate::DateTime { | |
| Self::from_system_time(SystemTime::now()) | ||
| } | ||
|
|
||
| /// Convert the given `chrono::DateTime` into a `bson::DateTime`, truncating it to millisecond | ||
| /// precision. | ||
| #[cfg(feature = "chrono-0_4")] | ||
|
|
@@ -127,7 +131,23 @@ impl crate::DateTime { | |
| Self::from_millis(dt.timestamp_millis()) | ||
| } | ||
|
|
||
| /// Convert this [`DateTime`] to a [`chrono::DateTime<Utc>`]. | ||
| /// | ||
| /// Note: Not every BSON datetime can be represented as a [`chrono::DateTime`]. For such dates, | ||
| /// [`chrono::MIN_DATETIME`] or [`chrono::MAX_DATETIME`] will be returned, whichever is closer. | ||
| /// | ||
| /// ``` | ||
| /// let bson_dt = bson::DateTime::now(); | ||
| /// let chrono_dt = bson_dt.to_chrono(); | ||
| /// assert_eq!(bson_dt.timestamp_millis(), chrono_dt.timestamp_millis()); | ||
| /// | ||
| /// let big = bson::DateTime::from_millis(i64::MAX); | ||
| /// let chrono_big = big.to_chrono(); | ||
| /// assert_eq!(chrono_big, chrono::MAX_DATETIME) | ||
| /// ``` | ||
| #[cfg(feature = "chrono-0_4")] | ||
| #[cfg_attr(docsrs, doc(cfg(feature = "chrono-0_4")))] | ||
| pub fn to_chrono(self) -> chrono::DateTime<Utc> { | ||
| match Utc.timestamp_millis_opt(self.0) { | ||
| LocalResult::Single(dt) => dt, | ||
| _ => { | ||
|
|
@@ -140,30 +160,78 @@ impl crate::DateTime { | |
| } | ||
| } | ||
|
|
||
| fn from_time_private(dt: time::OffsetDateTime) -> Self { | ||
| let millis = dt.unix_timestamp_nanos() / 1_000_000; | ||
| match millis.try_into() { | ||
| Ok(ts) => Self::from_millis(ts), | ||
| _ => { | ||
| if millis > 0 { | ||
| Self::MAX | ||
| } else { | ||
| Self::MIN | ||
| } | ||
| } | ||
| } | ||
| } | ||
|
|
||
| #[cfg(not(feature = "time-0_3"))] | ||
| #[allow(unused)] | ||
| pub(crate) fn from_time(dt: time::OffsetDateTime) -> Self { | ||
| Self::from_time_private(dt) | ||
| } | ||
|
|
||
| /// Convert the given `time::OffsetDateTime` into a `bson::DateTime`, truncating it to | ||
| /// millisecond precision. | ||
| /// | ||
| /// If the provided time is too far in the future or too far in the past to be represented | ||
| /// by a BSON datetime, either [`DateTime::MAX`] or [`DateTime::MIN`] will be | ||
| /// returned, whichever is closer. | ||
| #[cfg(feature = "time-0_3")] | ||
| pub fn from_time(dt: time::OffsetDateTime) -> Self { | ||
patrickfreed marked this conversation as resolved.
Show resolved
Hide resolved
|
||
| Self::from_time_private(dt) | ||
| } | ||
|
|
||
| fn to_time_private(self) -> time::OffsetDateTime { | ||
| match self.to_time_opt() { | ||
| Some(dt) => dt, | ||
| None => if self.0 < 0 { | ||
| time::PrimitiveDateTime::MIN | ||
| } else { | ||
| time::PrimitiveDateTime::MAX | ||
| } | ||
| .assume_utc(), | ||
| } | ||
| } | ||
|
|
||
| pub(crate) fn to_time_opt(self) -> Option<time::OffsetDateTime> { | ||
| time::OffsetDateTime::UNIX_EPOCH.checked_add(time::Duration::milliseconds(self.0)) | ||
| } | ||
|
|
||
| #[cfg(not(feature = "time-0_3"))] | ||
| #[allow(unused)] | ||
| pub(crate) fn to_time(self) -> time::OffsetDateTime { | ||
| self.to_time_private() | ||
| } | ||
|
|
||
| /// Convert this [`DateTime`] to a [`time::OffsetDateTime`]. | ||
| /// | ||
| /// Note: Not every BSON datetime can be represented as a [`time::OffsetDateTime`]. For such | ||
| /// dates, [`time::PrimitiveDateTime::MIN`] or [`time::PrimitiveDateTime::MAX`] will be | ||
| /// returned, whichever is closer. | ||
| /// | ||
| /// ``` | ||
| /// let bson_dt = bson::DateTime::now(); | ||
| /// let time_dt = bson_dt.to_time(); | ||
| /// assert_eq!(bson_dt.timestamp_millis() / 1000, time_dt.unix_timestamp()); | ||
| /// | ||
| /// let big = bson::DateTime::from_millis(i64::MIN); | ||
| /// let time_big = big.to_time(); | ||
| /// assert_eq!(time_big, time::PrimitiveDateTime::MIN.assume_utc()) | ||
| /// ``` | ||
| #[cfg(feature = "time-0_3")] | ||
| #[cfg_attr(docsrs, doc(cfg(feature = "time-0_3")))] | ||
| pub fn to_time(self) -> time::OffsetDateTime { | ||
| self.to_time_private() | ||
| } | ||
|
|
||
| /// Convert the given [`std::time::SystemTime`] to a [`DateTime`]. | ||
|
|
@@ -210,26 +278,30 @@ impl crate::DateTime { | |
|
|
||
| /// Convert this [`DateTime`] to an RFC 3339 formatted string. | ||
| pub fn to_rfc3339_string(self) -> String { | ||
| use std::result::Result; | ||
| match self.to_time().format(&Rfc3339) { | ||
| Result::Ok(s) => s, | ||
| Result::Err(e) => e.to_string(), | ||
|
There was a problem hiding this comment. I'm not thrilled about handling the error this way but I couldn't think of a better option that wouldn't be a breaking change. There was a problem hiding this comment. given that the chrono equivalent never errored, any idea what can lead this method to fail / how likely we are to encounter this? There was a problem hiding this comment. From the source, the The There was a problem hiding this comment. hmmm, should we consider deprecating this method in favor of a new version that propagates the error correctly? seems like it might help cut down on the chances anyone hits this weirdness There was a problem hiding this comment. The Also, rather than erroring, it may be more appropriate to panic here. Could we test some inputs that error in There was a problem hiding this comment. that solution SGTM. since we use the flag already it probably seems worth chiming in on that discussion linked above to let them know why There was a problem hiding this comment. And as a last check, are we okay with the risk of breaking user code with this? It's technically possible that a user has a super large date in their DB somewhere that used to format okay but will panic now via this method or fail to serialize via the serde helper. There was a problem hiding this comment. I think the risk is okay here - we're being loud about this change with the deprecation warning, and I think we should also call it out in the patch notes. It's also worth considering that the RFC itself is explicit about only applying to years in the 0000-9999 range and that the year field is fixed at four digits, so the chrono implementation is producing output that's probably not going to be accepted elsewhere. That said, I'd be interested to hear if @kmahar or @isabelatkinson have other opinions. There was a problem hiding this comment. Oh nice, I didn't know that about the RFC, that definitely makes this change seem fine to me then. |
||
| } | ||
| } | ||
|
|
||
| /// Convert the given RFC 3339 formatted string to a [`DateTime`], truncating it to millisecond | ||
| /// precision. | ||
| pub fn parse_rfc3339_str(s: impl AsRef<str>) -> Result<Self> { | ||
| let odt = time::OffsetDateTime::parse(s.as_ref(), &Rfc3339).map_err(|e| { | ||
| Error::InvalidTimestamp { | ||
| message: e.to_string(), | ||
| } | ||
| })?; | ||
| Ok(Self::from_time(odt)) | ||
| } | ||
| } | ||
|
|
||
| impl fmt::Debug for crate::DateTime { | ||
| fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { | ||
| let mut tup = f.debug_tuple("DateTime"); | ||
| match self.to_time_opt() { | ||
| Some(dt) => tup.field(&dt), | ||
| _ => tup.field(&self.0), | ||
| }; | ||
| tup.finish() | ||
|
|
@@ -238,8 +310,8 @@ impl fmt::Debug for crate::DateTime { | |
|
|
||
| impl Display for crate::DateTime { | ||
| fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { | ||
| match self.to_time_opt() { | ||
| Some(dt) => Display::fmt(&dt, f), | ||
| _ => Display::fmt(&self.0, f), | ||
| } | ||
| } | ||
|
|
@@ -300,6 +372,49 @@ impl SerializeAs<chrono::DateTime<Utc>> for crate::DateTime { | |
| } | ||
| } | ||
|
|
||
| #[cfg(feature = "time-0_3")] | ||
| #[cfg_attr(docsrs, doc(cfg(feature = "time-0_3")))] | ||
| impl From<crate::DateTime> for time::OffsetDateTime { | ||
| fn from(bson_dt: DateTime) -> Self { | ||
| bson_dt.to_time() | ||
| } | ||
| } | ||
|
|
||
| #[cfg(feature = "time-0_3")] | ||
| #[cfg_attr(docsrs, doc(cfg(feature = "time-0_3")))] | ||
| impl From<time::OffsetDateTime> for crate::DateTime { | ||
| fn from(x: time::OffsetDateTime) -> Self { | ||
| Self::from_time(x) | ||
| } | ||
| } | ||
|
|
||
| #[cfg(all(feature = "time-0_3", feature = "serde_with"))] | ||
| #[cfg_attr(docsrs, doc(cfg(all(feature = "time-0_3", feature = "serde_with"))))] | ||
| impl<'de> DeserializeAs<'de, time::OffsetDateTime> for crate::DateTime { | ||
| fn deserialize_as<D>(deserializer: D) -> std::result::Result<time::OffsetDateTime, D::Error> | ||
| where | ||
| D: Deserializer<'de>, | ||
| { | ||
| let dt = DateTime::deserialize(deserializer)?; | ||
| Ok(dt.to_time()) | ||
| } | ||
| } | ||
|
|
||
| #[cfg(all(feature = "time-0_3", feature = "serde_with"))] | ||
| #[cfg_attr(docsrs, doc(cfg(all(feature = "time-0_3", feature = "chrono-0_4"))))] | ||
| impl SerializeAs<time::OffsetDateTime> for crate::DateTime { | ||
| fn serialize_as<S>( | ||
| source: &time::OffsetDateTime, | ||
| serializer: S, | ||
| ) -> std::result::Result<S::Ok, S::Error> | ||
| where | ||
| S: serde::Serializer, | ||
| { | ||
| let dt = DateTime::from_time(*source); | ||
| dt.serialize(serializer) | ||
| } | ||
| } | ||
|
|
||
| /// Errors that can occur during [`DateTime`] construction and generation. | ||
| #[derive(Clone, Debug)] | ||
| #[non_exhaustive] | ||
|
|
||
Uh oh!
There was an error while loading. Please reload this page.