Use delegate_methods macro for delegating to minicbor

Author: Fahad Zubair

rust-runtime/aws-smithy-cbor/src/decode.rs

@@ -5,6 +5,13 @@ use minicbor::decode::Error;
 
 use crate::data::Type;
 
+/// Provides functions for decoding a CBOR object with a known schema.
+///
+/// Although CBOR is a self-describing format, this decoder is tailored for cases where the schema
+/// is known in advance. Therefore, the caller can determine which object key exists at the current
+/// position by calling `str` method, and call the relevant function based on the predetermined schema
+/// for that key. If an unexpected key is encountered, the caller can use the `skip` method to skip
+/// over the element.
 #[derive(Debug, Clone)]
 pub struct Decoder<'b> {
     decoder: minicbor::Decoder<'b>,
@@ -18,15 +25,14 @@ pub struct DeserializeError {
 
 impl std::fmt::Display for DeserializeError {
     fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result {
-        // TODO? Is this good enough?
         self._inner.fmt(f)
     }
 }
 
 impl std::error::Error for DeserializeError {}
 
 impl DeserializeError {
-    fn new(inner: Error) -> Self {
+    pub(crate) fn new(inner: Error) -> Self {
         Self { _inner: inner }
     }
 
@@ -55,60 +61,86 @@ impl DeserializeError {
     }
 }
 
+
+/// Macro for delegating method calls to the decoder.
+///
+/// This macro generates wrapper methods for calling specific encoder methods on the decoder
+/// and returning the result with error handling.
+///
+/// # Example
+///
+/// ```
+/// delegate_method! {
+///     /// Wrapper method for encoding method `encode_str` on the decoder.
+///     encode_str_wrapper => encode_str(String);
+///     /// Wrapper method for encoding method `encode_int` on the decoder.
+///     encode_int_wrapper => encode_int(i32);
+/// }
+/// ```
+macro_rules! delegate_method {
+    ($($(#[$meta:meta])* $wrapper_name:ident => $encoder_name:ident($result_type:ty);)+) => {
+        $(
+            pub fn $wrapper_name(&mut self) -> Result<$result_type, DeserializeError> {
+                self.decoder.$encoder_name().map_err(DeserializeError::new)
+            }
+        )+
+    };
+}
+
 impl<'b> Decoder<'b> {
     pub fn new(bytes: &'b [u8]) -> Self {
         Self {
             decoder: minicbor::Decoder::new(bytes),
         }
     }
 
-    pub fn map(&mut self) -> Result<Option<u64>, DeserializeError> {
-        self.decoder.map().map_err(DeserializeError::new)
-    }
-
     pub fn datatype(&self) -> Result<Type, DeserializeError> {
         self.decoder
             .datatype()
             .map(Type::new)
             .map_err(DeserializeError::new)
     }
 
-    pub fn skip(&mut self) -> Result<(), DeserializeError> {
-        self.decoder.skip().map_err(DeserializeError::new)
-    }
-
-    // TODO-David: confirm benchmarks and keep either `str_alt` or `str`.
-    // The following seems to be a bit slower than the one we have kept.
-    pub fn str_alt(&mut self) -> Result<Cow<'b, str>, DeserializeError> {
-        // This implementation uses `next` twice to see if there is
-        // another str chunk. If there is, it returns a owned `String`.
-        let mut chunks_iter = self.decoder.str_iter().map_err(DeserializeError::new)?;
-        let head = match chunks_iter.next() {
-            Some(Ok(head)) => head,
-            None => return Ok(Cow::Borrowed("")),
-            Some(Err(e)) => return Err(DeserializeError::new(e)),
-        };
-
-        match chunks_iter.next() {
-            None => Ok(Cow::Borrowed(head)),
-            Some(Err(e)) => Err(DeserializeError::new(e)),
-            Some(Ok(next)) => {
-                let mut concatenated_string = String::from(head);
-                concatenated_string.push_str(next);
-                for chunk in chunks_iter {
-                    concatenated_string.push_str(chunk.map_err(DeserializeError::new)?);
-                }
-                Ok(Cow::Owned(concatenated_string))
-            }
-        }
+    delegate_method! {
+        /// Skips the current CBOR element.
+        skip => skip(());
+        /// Reads a boolean at the current position.
+        boolean => bool(bool);
+        /// Reads a byte at the current position.
+        byte => i8(i8);
+        /// Reads a short at the current position.
+        short => i16(i16);
+        /// Reads a integer at the current position.
+        integer => i32(i32);
+        /// Reads a long at the current position.
+        long => i64(i64);
+        /// Reads a float at the current position.
+        float => f32(f32);
+        /// Reads a double at the current position.
+        double => f64(f64);
+        /// Reads a null CBOR element at the current position.
+        null => null(());
+        /// Returns the number of elements in a definite list. For indefinite lists it returns a `None`.
+        list => array(Option<u64>);
+        /// Returns the number of elements in a definite map. For indefinite map it returns a `None`.
+        map => map(Option<u64>);
+    }
+
+    /// Returns the current position of the buffer, which will be decoded when any of the methods is called.
+    pub fn position(&self) -> usize {
+        self.decoder.position()
     }
 
+    /// Returns a `cow::Borrowed(&str)` if the element at the current position in the buffer is a definite
+    /// length string. Otherwise, it returns a `cow::Owned(String)` if the element at the current position is an
+    /// indefinite-length string. An error is returned if the element is neither a definite length nor an
+    /// indefinite-length string.
     pub fn str(&mut self) -> Result<Cow<'b, str>, DeserializeError> {
         let bookmark = self.decoder.position();
         match self.decoder.str() {
             Ok(str_value) => Ok(Cow::Borrowed(str_value)),
             Err(e) if e.is_type_mismatch() => {
-                // Move the position back to the start of the Cbor element and then try
+                // Move the position back to the start of the CBOR element and then try
                 // decoding it as a indefinite length string.
                 self.decoder.set_position(bookmark);
                 Ok(Cow::Owned(self.string()?))
@@ -117,25 +149,15 @@ impl<'b> Decoder<'b> {
         }
     }
 
-    // TODO-David: confirm benchmarks and keep either `string_alt` or `string` implementation.
-    // The following seems to be a bit slower than the one we have kept.
-    pub fn string_alt(&mut self) -> Result<String, DeserializeError> {
-        let s: Result<String, _> = self
-            .decoder
-            .str_iter()
-            .map_err(DeserializeError::new)?
-            .collect();
-        s.map_err(DeserializeError::new)
-    }
-
+    /// Allocates and returns a `String` if the element at the current position in the buffer is either a
+    /// definite-length or an indefinite-length string. Otherwise, an error is returned if the element is not a string type.
     pub fn string(&mut self) -> Result<String, DeserializeError> {
         let mut iter = self.decoder.str_iter().map_err(DeserializeError::new)?;
         let head = iter.next();
 
         let decoded_string = match head {
             None => String::new(),
             Some(head) => {
-                // The following is faster in benchmarks than using `Collect()` on a `String`.
                 let mut combined_chunks = String::from(head.map_err(DeserializeError::new)?);
                 for chunk in iter {
                     combined_chunks.push_str(chunk.map_err(DeserializeError::new)?);
@@ -147,6 +169,8 @@ impl<'b> Decoder<'b> {
         Ok(decoded_string)
     }
 
+    /// Returns a `blob` if the element at the current position in the buffer is a byte string. Otherwise,
+    /// a `DeserializeError` error is returned.
     pub fn blob(&mut self) -> Result<Blob, DeserializeError> {
         let iter = self.decoder.bytes_iter().map_err(DeserializeError::new)?;
         let parts: Vec<&[u8]> = iter
@@ -160,57 +184,20 @@ impl<'b> Decoder<'b> {
         })
     }
 
-    pub fn boolean(&mut self) -> Result<bool, DeserializeError> {
-        self.decoder.bool().map_err(DeserializeError::new)
-    }
-
-    pub fn position(&self) -> usize {
-        self.decoder.position()
-    }
-
-    pub fn byte(&mut self) -> Result<i8, DeserializeError> {
-        self.decoder.i8().map_err(DeserializeError::new)
-    }
-
-    pub fn short(&mut self) -> Result<i16, DeserializeError> {
-        self.decoder.i16().map_err(DeserializeError::new)
-    }
-
-    pub fn integer(&mut self) -> Result<i32, DeserializeError> {
-        self.decoder.i32().map_err(DeserializeError::new)
-    }
-
-    pub fn long(&mut self) -> Result<i64, DeserializeError> {
-        self.decoder.i64().map_err(DeserializeError::new)
-    }
-
-    pub fn float(&mut self) -> Result<f32, DeserializeError> {
-        self.decoder.f32().map_err(DeserializeError::new)
-    }
-
-    pub fn double(&mut self) -> Result<f64, DeserializeError> {
-        self.decoder.f64().map_err(DeserializeError::new)
-    }
-
+    /// Returns a `DateTime` if the element at the current position in the buffer is a `timestamp`. Otherwise,
+    /// a `DeserializeError` error is returned.
     pub fn timestamp(&mut self) -> Result<DateTime, DeserializeError> {
         let tag = self.decoder.tag().map_err(DeserializeError::new)?;
 
         if !matches!(tag, minicbor::data::Tag::Timestamp) {
-            // TODO
-            todo!()
+            Err(DeserializeError::new(Error::message(
+                "expected timestamp tag",
+            )))
         } else {
             let epoch_seconds = self.decoder.f64().map_err(DeserializeError::new)?;
             Ok(DateTime::from_secs_f64(epoch_seconds))
         }
     }
-
-    pub fn null(&mut self) -> Result<(), DeserializeError> {
-        self.decoder.null().map_err(DeserializeError::new)
-    }
-
-    pub fn list(&mut self) -> Result<Option<u64>, DeserializeError> {
-        self.decoder.array().map_err(DeserializeError::new)
-    }
 }
 
 #[derive(Debug)]