cr feedback

Author: isabelatkinson

src/error.rs

@@ -458,9 +458,9 @@ pub enum ErrorKind {
     DnsResolve { message: String },
 
     /// A GridFS error occurred.
-    #[error("{message}")]
+    #[error("{0:?}")]
     #[non_exhaustive]
-    GridFS { message: String },
+    GridFs(GridFsErrorKind),
 
     #[error("Internal error: {message}")]
     #[non_exhaustive]
@@ -698,6 +698,61 @@ impl WriteFailure {
     }
 }
 
+/// An error that occurred during a GridFS operation.
+#[derive(Clone, Debug)]
+#[allow(missing_docs)]
+#[non_exhaustive]
+pub enum GridFsErrorKind {
+    /// The file with the given identifier was not found.
+    #[non_exhaustive]
+    FileNotFound { identifier: GridFsFileIdentifier },
+
+    /// The file with the given revision was not found.
+    #[non_exhaustive]
+    RevisionNotFound { revision: i32 },
+
+    /// The chunk at index 'n' was missing.
+    #[non_exhaustive]
+    MissingChunk { n: u32 },
+
+    /// The chunk was the incorrect size.
+    #[non_exhaustive]
+    WrongSizeChunk {
+        actual_size: u32,
+        expected_size: u32,
+    },
+
+    /// An incorrect number of chunks was present for the file.
+    #[non_exhaustive]
+    WrongNumberOfChunks {
+        actual_number: u32,
+        expected_number: u32,
+    },
+}
+
+/// An identifier for a file stored in a GridFS bucket.
+#[derive(Clone, Debug)]
+#[non_exhaustive]
+pub enum GridFsFileIdentifier {
+    /// The name of the file. Not guaranteed to be unique.
+    Filename(String),
+
+    /// The file's unique [`Bson`] ID.
+    Id(Bson),
+}
+
+impl From<&str> for GridFsFileIdentifier {
+    fn from(filename: &str) -> Self {
+        Self::Filename(filename.into())
+    }
+}
+
+impl From<Bson> for GridFsFileIdentifier {
+    fn from(id: Bson) -> Self {
+        Self::Id(id)
+    }
+}
+
 /// Translates ErrorKind::BulkWriteError cases to ErrorKind::WriteErrors, leaving all other errors
 /// untouched.
 pub(crate) fn convert_bulk_errors(error: Error) -> Error {

src/gridfs.rs

@@ -43,6 +43,7 @@ struct Chunk {
 /// collection document per stored file.
 #[derive(Debug, Deserialize, Serialize)]
 #[serde(rename_all = "camelCase")]
+#[non_exhaustive]
 pub struct FilesCollectionDocument {
     #[serde(rename = "_id")]
     pub id: Bson,

src/gridfs/download.rs

@@ -4,14 +4,14 @@ use tokio_util::compat::FuturesAsyncWriteCompatExt;
 use super::{options::GridFsDownloadByNameOptions, FilesCollectionDocument, GridFsBucket};
 use crate::{
     bson::{doc, Bson},
-    error::{ErrorKind, Result},
+    error::{ErrorKind, GridFsErrorKind, Result},
     options::{FindOneOptions, FindOptions},
 };
 
 impl GridFsBucket {
     /// Downloads the contents of the stored file specified by `id` and writes
     /// the contents to the `destination`.
-    pub async fn download_to_tokio_writer<T>(&self, id: Bson, destination: &mut T) -> Result<()>
+    pub async fn download_to_tokio_writer<T>(&self, id: Bson, destination: T) -> Result<()>
     where
         T: tokio::io::AsyncWrite + std::marker::Unpin,
     {
@@ -23,14 +23,14 @@ impl GridFsBucket {
         let file = match self
             .inner
             .files
-            .find_one(doc! { "_id": &id }, options)
+            .find_one(doc! { "_id": id.clone() }, options)
             .await?
         {
             Some(fcd) => fcd,
             None => {
-                return Err(ErrorKind::InvalidArgument {
-                    message: format!("couldn't find file with id {}", &id),
-                }
+                return Err(ErrorKind::GridFs(GridFsErrorKind::FileNotFound {
+                    identifier: id.into(),
+                })
                 .into());
             }
         };
@@ -40,11 +40,7 @@ impl GridFsBucket {
 
     /// Downloads the contents of the stored file specified by `id` and writes
     /// the contents to the `destination`.
-    pub async fn download_to_futures_0_3_writer<T>(
-        &self,
-        id: Bson,
-        destination: &mut T,
-    ) -> Result<()>
+    pub async fn download_to_futures_0_3_writer<T>(&self, id: Bson, destination: T) -> Result<()>
     where
         T: futures_util::io::AsyncWrite + std::marker::Unpin,
     {
@@ -59,7 +55,7 @@ impl GridFsBucket {
     pub async fn download_to_tokio_writer_by_name<T>(
         &self,
         filename: impl AsRef<str>,
-        destination: &mut T,
+        destination: T,
         options: impl Into<Option<GridFsDownloadByNameOptions>>,
     ) -> Result<()>
     where
@@ -79,17 +75,27 @@ impl GridFsBucket {
             .build();
 
         let file = match self
-            .inner
-            .files
+            .files()
             .find_one(doc! { "filename": filename.as_ref() }, options)
             .await?
         {
             Some(fcd) => fcd,
             None => {
-                return Err(ErrorKind::InvalidArgument {
-                    message: format!("couldn't find file with name {}", &filename.as_ref()),
+                if self
+                    .files()
+                    .find_one(doc! { "filename": filename.as_ref() }, None)
+                    .await?
+                    .is_some()
+                {
+                    return Err(
+                        ErrorKind::GridFs(GridFsErrorKind::RevisionNotFound { revision }).into(),
+                    );
+                } else {
+                    return Err(ErrorKind::GridFs(GridFsErrorKind::FileNotFound {
+                        identifier: filename.as_ref().into(),
+                    })
+                    .into());
                 }
-                .into());
             }
         };
 
@@ -104,7 +110,7 @@ impl GridFsBucket {
     pub async fn download_to_futures_0_3_writer_by_name<T>(
         &self,
         filename: impl AsRef<str>,
-        destination: &mut T,
+        destination: T,
         options: impl Into<Option<GridFsDownloadByNameOptions>>,
     ) -> Result<()>
     where
@@ -117,7 +123,7 @@ impl GridFsBucket {
     async fn download_to_tokio_writer_common<T>(
         &self,
         file: FilesCollectionDocument,
-        destination: &mut T,
+        mut destination: T,
     ) -> Result<()>
     where
         T: tokio::io::AsyncWrite + std::marker::Unpin,
@@ -144,22 +150,17 @@ impl GridFsBucket {
         while cursor.advance().await? {
             let chunk = cursor.deserialize_current()?;
             if chunk.n != n {
-                return Err(ErrorKind::GridFS {
-                    message: format!("missing chunk {} in file", n),
-                }
-                .into());
+                return Err(ErrorKind::GridFs(GridFsErrorKind::MissingChunk { n }).into());
             }
 
             let chunk_length = chunk.data.bytes.len();
             let expected_length =
                 std::cmp::min(total_bytes_expected - chunk_size * n as u64, chunk_size);
             if chunk_length as u64 != expected_length {
-                return Err(ErrorKind::GridFS {
-                    message: format!(
-                        "chunk {} was expected to be {} bytes but is {} bytes",
-                        n, chunk_length, expected_length
-                    ),
-                }
+                return Err(ErrorKind::GridFs(GridFsErrorKind::WrongSizeChunk {
+                    actual_size: chunk_length as u32,
+                    expected_size: expected_length as u32,
+                })
                 .into());
             }
 
@@ -169,10 +170,11 @@ impl GridFsBucket {
 
         let expected_n =
             total_bytes_expected / chunk_size + u64::from(total_bytes_expected % chunk_size != 0);
-        if (n as u64) < expected_n {
-            return Err(ErrorKind::GridFS {
-                message: "missing last chunk in file".into(),
-            }
+        if (n as u64) != expected_n {
+            return Err(ErrorKind::GridFs(GridFsErrorKind::WrongNumberOfChunks {
+                actual_number: n,
+                expected_number: expected_n as u32,
+            })
             .into());
         }
 

src/gridfs/options.rs

@@ -38,7 +38,6 @@ pub struct GridFsBucketOptions {
 pub struct GridFsUploadOptions {
     /// The number of bytes per chunk of this file. Defaults to the `chunk_size_bytes` specified
     /// in the [`GridFsBucketOptions`].
-    #[serde(rename = "chunkSizeBytes")]
     pub chunk_size_bytes: Option<u32>,
 
     /// User data for the 'metadata' field of the files collection document.

src/test/spec/unified_runner/test_runner.rs

@@ -129,10 +129,12 @@ impl TestRunner {
         ));
 
         for test_case in test_file.tests {
-            // to run a single test, set the TEST_DESCRIPTION environment variable to the test's
-            // description
             if let Ok(description) = std::env::var("TEST_DESCRIPTION") {
-                if description != test_case.description {
+                if !test_case
+                    .description
+                    .to_lowercase()
+                    .contains(&description.to_lowercase())
+                {
                     continue;
                 }
             }