RUST-445 Round trip decimal128 values (#177)

Author: patrickfreed

src/bson.rs

@@ -29,12 +29,11 @@ use std::{
 use chrono::{DateTime, Datelike, SecondsFormat, TimeZone, Utc};
 use serde_json::{json, Value};
 
-#[cfg(feature = "decimal128")]
-use crate::decimal128::Decimal128;
 pub use crate::document::Document;
 use crate::{
     oid::{self, ObjectId},
     spec::{BinarySubtype, ElementType},
+    Decimal128,
 };
 
 /// Possible BSON value types.
@@ -73,7 +72,6 @@ pub enum Bson {
     /// Symbol (Deprecated)
     Symbol(String),
     /// [128-bit decimal floating point](https://github.com/mongodb/specifications/blob/master/source/bson-decimal128/decimal128.rst)
-    #[cfg(feature = "decimal128")]
     Decimal128(Decimal128),
     /// Undefined value (Deprecated)
     Undefined,
@@ -139,8 +137,7 @@ impl Display for Bson {
             Bson::ObjectId(ref id) => write!(fmt, "ObjectId(\"{}\")", id),
             Bson::UtcDatetime(date_time) => write!(fmt, "Date(\"{}\")", date_time),
             Bson::Symbol(ref sym) => write!(fmt, "Symbol(\"{}\")", sym),
-            #[cfg(feature = "decimal128")]
-            Bson::Decimal128(ref d) => write!(fmt, "Decimal128({})", d),
+            Bson::Decimal128(ref d) => write!(fmt, "{}", d),
             Bson::Undefined => write!(fmt, "undefined"),
             Bson::MinKey => write!(fmt, "MinKey"),
             Bson::MaxKey => write!(fmt, "MaxKey"),
@@ -331,6 +328,10 @@ impl From<Bson> for Value {
 
 impl Bson {
     /// Converts the Bson value into its [relaxed extended JSON representation](https://docs.mongodb.com/manual/reference/mongodb-extended-json/).
+    ///
+    /// Note: extended json encoding for `Decimal128` values is not supported without the
+    /// "decimal128" feature flag. If this method is called on a case which contains a
+    /// `Decimal128` value, it will panic.
     pub fn into_relaxed_extjson(self) -> Value {
         match self {
             Bson::FloatingPoint(v) if v.is_nan() => {
@@ -408,6 +409,11 @@ impl Bson {
             Bson::Symbol(v) => json!({ "$symbol": v }),
             #[cfg(feature = "decimal128")]
             Bson::Decimal128(ref v) => json!({ "$numberDecimal": v.to_string() }),
+            #[cfg(not(feature = "decimal128"))]
+            Bson::Decimal128(_) => panic!(
+                "Decimal128 extended JSON not implemented yet. Use the decimal128 feature to \
+                 enable experimental support for it."
+            ),
             Bson::Undefined => json!({ "$undefined": true }),
             Bson::MinKey => json!({ "$minKey": 1 }),
             Bson::MaxKey => json!({ "$maxKey": 1 }),
@@ -426,6 +432,10 @@ impl Bson {
     }
 
     /// Converts the Bson value into its [canonical extended JSON representation](https://docs.mongodb.com/manual/reference/mongodb-extended-json/).
+    ///
+    /// Note: extended json encoding for `Decimal128` values is not supported without the
+    /// "decimal128" feature flag. If this method is called on a case which contains a
+    /// `Decimal128` value, it will panic.
     pub fn into_canonical_extjson(self) -> Value {
         match self {
             Bson::I32(i) => json!({ "$numberInt": i.to_string() }),
@@ -482,7 +492,6 @@ impl Bson {
             Bson::ObjectId(..) => ElementType::ObjectId,
             Bson::UtcDatetime(..) => ElementType::UtcDatetime,
             Bson::Symbol(..) => ElementType::Symbol,
-            #[cfg(feature = "decimal128")]
             Bson::Decimal128(..) => ElementType::Decimal128Bit,
             Bson::Undefined => ElementType::Undefined,
             Bson::MaxKey => ElementType::MaxKey,

src/decimal128.rs

@@ -1,15 +1,28 @@
 //! [BSON Decimal128](https://github.com/mongodb/specifications/blob/master/source/bson-decimal128/decimal128.rst) data type representation
 
-use std::{fmt, str::FromStr};
-
+#[cfg(feature = "decimal128")]
 use decimal::d128;
-
-/// Decimal128 type
+use std::fmt;
+
+/// Decimal128 type.
+///
+/// Currently, this type does not have any functionality and can only be serialized and
+/// deserialized from existing documents that contain BSON decimal128s.
+///
+/// Experimental functionality can be enabled through the usage of the `"decimal128"`
+/// feature flag. Note that the API and behavior of such functionality are unstable and
+/// subject to change.
 #[derive(Clone, PartialEq, PartialOrd)]
 pub struct Decimal128 {
-    inner: d128,
+    #[cfg(not(feature = "decimal128"))]
+    /// BSON bytes containing the decimal128. Stored for round tripping.
+    pub(crate) bytes: [u8; 128 / 8],
+
+    #[cfg(feature = "decimal128")]
+    inner: decimal::d128,
 }
 
+#[cfg(feature = "decimal128")]
 impl Decimal128 {
     /// Construct a `Decimal128` from string.
     ///
@@ -180,55 +193,74 @@ impl Decimal128 {
 }
 
 impl fmt::Debug for Decimal128 {
+    #[cfg(feature = "decimal128")]
     fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result {
-        write!(f, "Decimal(\"{:?}\")", self.inner)
+        write!(f, "Decimal128(\"{:?}\")", self.inner)
+    }
+
+    #[cfg(not(feature = "decimal128"))]
+    fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result {
+        write!(f, "Decimal128(...)")
     }
 }
 
 impl fmt::Display for Decimal128 {
+    #[cfg(feature = "decimal128")]
     fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result {
         write!(f, "{}", self.inner)
     }
+
+    #[cfg(not(feature = "decimal128"))]
+    fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result {
+        write!(f, "{:?}", self)
+    }
 }
 
+#[cfg(feature = "decimal128")]
 impl fmt::LowerHex for Decimal128 {
     fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result {
         <d128 as fmt::LowerHex>::fmt(&self.inner, f)
     }
 }
 
+#[cfg(feature = "decimal128")]
 impl fmt::LowerExp for Decimal128 {
     fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result {
         <d128 as fmt::LowerExp>::fmt(&self.inner, f)
     }
 }
 
-impl FromStr for Decimal128 {
+#[cfg(feature = "decimal128")]
+impl std::str::FromStr for Decimal128 {
     type Err = ();
     fn from_str(s: &str) -> Result<Decimal128, ()> {
         Ok(Decimal128::from_str(s))
     }
 }
 
+#[cfg(feature = "decimal128")]
 impl Into<d128> for Decimal128 {
     fn into(self) -> d128 {
         self.inner
     }
 }
 
+#[cfg(feature = "decimal128")]
 impl From<d128> for Decimal128 {
     fn from(d: d128) -> Decimal128 {
         Decimal128 { inner: d }
     }
 }
 
+#[cfg(feature = "decimal128")]
 impl Default for Decimal128 {
     fn default() -> Decimal128 {
         Decimal128::zero()
     }
 }
 
 #[cfg(test)]
+#[cfg(feature = "decimal128")]
 mod test {
     use super::*;
 

src/decoder/error.rs

@@ -67,7 +67,7 @@ impl fmt::Display for DecoderError {
                 element_type,
             } => write!(
                 fmt,
-                "unrecognized element type for key \"{}\": `{}`",
+                "unrecognized element type for key \"{}\": `{:#x}`",
                 key, element_type
             ),
             DecoderError::SyntaxError { ref message } => message.fmt(fmt),
@@ -124,7 +124,6 @@ impl Bson {
             Bson::Symbol(_) => Unexpected::Other("symbol"),
             Bson::TimeStamp(_) => Unexpected::Other("timestamp"),
             Bson::UtcDatetime(_) => Unexpected::Other("datetime"),
-            #[cfg(feature = "decimal128")]
             Bson::Decimal128(_) => Unexpected::Other("decimal128"),
         }
     }

src/decoder/mod.rs

@@ -37,12 +37,11 @@ use chrono::{
     Utc,
 };
 
-#[cfg(feature = "decimal128")]
-use crate::decimal128::Decimal128;
 use crate::{
     bson::{Array, Binary, Bson, DbPointer, Document, JavaScriptCodeWithScope, Regex, TimeStamp},
     oid,
     spec::{self, BinarySubtype},
+    Decimal128,
 };
 
 use ::serde::de::{Deserialize, Error};
@@ -98,6 +97,16 @@ fn read_i64<R: Read + ?Sized>(reader: &mut R) -> DecoderResult<i64> {
     reader.read_i64::<LittleEndian>().map_err(From::from)
 }
 
+/// Placeholder decoder for `Decimal128`. Reads 128 bits and just stores them, does no validation or
+/// parsing.
+#[cfg(not(feature = "decimal128"))]
+#[inline]
+fn read_f128<R: Read + ?Sized>(reader: &mut R) -> DecoderResult<Decimal128> {
+    let mut buf = [0u8; 128 / 8];
+    reader.read_exact(&mut buf)?;
+    Ok(Decimal128 { bytes: buf })
+}
+
 #[cfg(feature = "decimal128")]
 #[inline]
 fn read_f128<R: Read + ?Sized>(reader: &mut R) -> DecoderResult<Decimal128> {
@@ -219,7 +228,6 @@ pub(crate) fn decode_bson_kvp<R: Read + ?Sized>(
             }
         }
         Some(ElementType::Symbol) => read_string(reader, utf8_lossy).map(Bson::Symbol)?,
-        #[cfg(feature = "decimal128")]
         Some(ElementType::Decimal128Bit) => read_f128(reader).map(Bson::Decimal128)?,
         Some(ElementType::Undefined) => Bson::Undefined,
         Some(ElementType::DbPointer) => {

src/encoder/mod.rs

@@ -153,6 +153,11 @@ pub(crate) fn encode_bson<W: Write + ?Sized>(
         ),
         Bson::Null => Ok(()),
         Bson::Symbol(ref v) => write_string(writer, &v),
+        #[cfg(not(feature = "decimal128"))]
+        Bson::Decimal128(ref v) => {
+            writer.write_all(&v.bytes)?;
+            Ok(())
+        }
         #[cfg(feature = "decimal128")]
         Bson::Decimal128(ref v) => write_f128(writer, v.clone()),
         Bson::Undefined => Ok(()),

src/lib.rs

@@ -186,8 +186,6 @@
 
 #![allow(clippy::cognitive_complexity)]
 
-#[cfg(feature = "decimal128")]
-pub use self::decimal128::Decimal128;
 pub use self::{
     bson::{
         Array,
@@ -200,6 +198,7 @@ pub use self::{
         TimeStamp,
         UtcDateTime,
     },
+    decimal128::Decimal128,
     decoder::{from_bson, Decoder, DecoderError, DecoderResult},
     document::{ValueAccessError, ValueAccessResult},
     encoder::{to_bson, Encoder, EncoderError, EncoderResult},
@@ -209,7 +208,6 @@ pub use self::{
 mod macros;
 mod bson;
 pub mod compat;
-#[cfg(feature = "decimal128")]
 pub mod decimal128;
 mod decoder;
 pub mod document;

src/spec.rs

@@ -101,7 +101,6 @@ pub enum ElementType {
     /// 64-bit integer
     Integer64Bit = ELEMENT_TYPE_64BIT_INTEGER,
     /// [128-bit decimal floating point](https://github.com/mongodb/specifications/blob/master/source/bson-decimal128/decimal128.rst)
-    #[cfg(feature = "decimal128")]
     Decimal128Bit = ELEMENT_TYPE_128BIT_DECIMAL,
     MaxKey = ELEMENT_TYPE_MAXKEY,
     MinKey = ELEMENT_TYPE_MINKEY,
@@ -131,7 +130,6 @@ impl ElementType {
             ELEMENT_TYPE_32BIT_INTEGER => Integer32Bit,
             ELEMENT_TYPE_TIMESTAMP => TimeStamp,
             ELEMENT_TYPE_64BIT_INTEGER => Integer64Bit,
-            #[cfg(feature = "decimal128")]
             ELEMENT_TYPE_128BIT_DECIMAL => Decimal128Bit,
             ELEMENT_TYPE_MAXKEY => MaxKey,
             ELEMENT_TYPE_MINKEY => MinKey,

tests/spec/corpus.rs

@@ -52,10 +52,6 @@ struct ParseError {
 }
 
 fn run_test(test: TestFile) {
-    if test.bson_type == "0x13" && !cfg!(feature = "decimal128") {
-        return;
-    }
-
     for valid in test.valid {
         let description = format!("{}: {}", test.description, valid.description);
 
@@ -71,6 +67,12 @@ fn run_test(test: TestFile) {
             .encode(&mut native_to_bson_bson_to_native_cv)
             .expect(&description);
 
+        // TODO RUST-36: Enable decimal128 tests.
+        // extJSON not implemented for decimal128 without the feature flag, so we must stop here.
+        if test.bson_type == "0x13" && !cfg!(feature = "decimal128") {
+            continue;
+        }
+
         let cej: serde_json::Value =
             serde_json::from_str(&valid.canonical_extjson).expect(&description);
 
@@ -109,7 +111,8 @@ fn run_test(test: TestFile) {
             }
         }
 
-        if test.bson_type == "0x13" {
+        // TODO RUST-36: Enable decimal128 tests.
+        if test.bson_type != "0x13" {
             assert_eq!(
                 Bson::Document(bson_to_native_cb.clone()).into_canonical_extjson(),
                 cej_updated_float,