RUST-1364 Collapse comment and comment_bson (#1070)

Author: abr-egn

src/action/find.rs

@@ -86,8 +86,7 @@ impl<'a, T: Send + Sync, Session> Find<'a, T, Session> {
         allow_disk_use: bool,
         allow_partial_results: bool,
         batch_size: u32,
-        comment: String,
-        comment_bson: Bson,
+        comment: Bson,
         cursor_type: CursorType,
         hint: Hint,
         limit: i64,
@@ -168,8 +167,7 @@ impl<'a, T: Send + Sync> FindOne<'a, T> {
     option_setters! { options: FindOneOptions;
         allow_partial_results: bool,
         collation: Collation,
-        comment: String,
-        comment_bson: Bson,
+        comment: Bson,
         hint: Hint,
         max: Document,
         max_scan: u64,

src/coll/options.rs

@@ -544,8 +544,7 @@ pub struct AggregateOptions {
     /// Tags the query with an arbitrary [`Bson`] value to help trace the operation through the
     /// database profiler, currentOp and logs.
     ///
-    /// This option is only supported on server versions 4.4+. Use the `comment` option on
-    /// older server versions.
+    /// For server versions less than 4.4, only a string value may be provided.
     pub comment: Option<Bson>,
 
     /// The index to use for the operation.
@@ -767,22 +766,11 @@ pub struct FindOptions {
     #[serde(serialize_with = "serde_util::serialize_u32_option_as_i32")]
     pub batch_size: Option<u32>,
 
-    /// Tags the query with an arbitrary string to help trace the operation through the
-    /// database profiler, currentOp and logs.
-    ///
-    /// If both this option and `comment_bson` are specified, `comment_bson` will take precedence.
-    // TODO RUST-1364: Update this field to be of type Option<Bson>
-    #[serde(skip_serializing)]
-    pub comment: Option<String>,
-
     /// Tags the query with an arbitrary [`Bson`] value to help trace the operation through the
     /// database profiler, currentOp and logs.
     ///
-    /// This option is only supported on server versions 4.4+. Use the `comment` option on
-    /// older server versions.
-    // TODO RUST-1364: Remove this field
-    #[serde(rename(serialize = "comment"))]
-    pub comment_bson: Option<Bson>,
+    /// For server versions less than 4.4, only a string value may be provided.
+    pub comment: Option<Bson>,
 
     /// The type of cursor to return.
     #[serde(skip)]
@@ -878,7 +866,6 @@ impl From<FindOneOptions> for FindOptions {
             allow_partial_results: options.allow_partial_results,
             collation: options.collation,
             comment: options.comment,
-            comment_bson: options.comment_bson,
             hint: options.hint,
             max: options.max,
             max_scan: options.max_scan,
@@ -932,20 +919,11 @@ pub struct FindOneOptions {
     /// information on how to use this option.
     pub collation: Option<Collation>,
 
-    /// Tags the query with an arbitrary string value to help trace the operation through the
-    /// database profiler, currentOp and logs.
-    ///
-    /// If both this option and `comment_bson` are specified, `comment_bson` will take precedence.
-    // TODO RUST-1364: Update this field to be of type Option<Bson>
-    pub comment: Option<String>,
-
     /// Tags the query with an arbitrary [`Bson`] value to help trace the operation through the
     /// database profiler, currentOp and logs.
     ///
-    /// This option is only supported on server versions 4.4+. Use the `comment` option on
-    /// older server versions.
-    // TODO RUST-1364: Remove this field
-    pub comment_bson: Option<Bson>,
+    /// For server versions less than 4.4, only a string value may be provided.
+    pub comment: Option<Bson>,
 
     /// The index to use for the operation.
     pub hint: Option<Hint>,

src/operation/find.rs

@@ -22,15 +22,7 @@ pub(crate) struct Find {
 }
 
 impl Find {
-    pub(crate) fn new(ns: Namespace, filter: Document, mut options: Option<FindOptions>) -> Self {
-        if let Some(ref mut options) = options {
-            if let Some(ref comment) = options.comment {
-                if options.comment_bson.is_none() {
-                    options.comment_bson = Some(comment.clone().into());
-                }
-            }
-        }
-
+    pub(crate) fn new(ns: Namespace, filter: Document, options: Option<FindOptions>) -> Self {
         Self {
             ns,
             filter,
@@ -108,9 +100,7 @@ impl OperationWithDefaults for Find {
         let comment = if description.max_wire_version.unwrap_or(0) < SERVER_4_4_0_WIRE_VERSION {
             None
         } else {
-            self.options
-                .as_ref()
-                .and_then(|opts| opts.comment_bson.clone())
+            self.options.as_ref().and_then(|opts| opts.comment.clone())
         };
 
         Ok(CursorSpecification::new(

src/test/spec/unified_runner/operation.rs

@@ -553,20 +553,13 @@ impl Find {
     ) -> Result<TestCursor> {
         let collection = test_runner.get_collection(id).await;
 
-        let (comment, comment_bson) = match &self.comment {
-            Some(Bson::String(string)) => (Some(string.clone()), None),
-            Some(bson) => (None, Some(bson.clone())),
-            None => (None, None),
-        };
-
         // `FindOptions` is constructed without the use of `..Default::default()` to enforce at
         // compile-time that any new fields added there need to be considered here.
         let options = FindOptions {
             allow_disk_use: self.allow_disk_use,
             allow_partial_results: self.allow_partial_results,
             batch_size: self.batch_size,
-            comment,
-            comment_bson,
+            comment: self.comment.clone(),
             hint: self.hint.clone(),
             limit: self.limit,
             max: self.max.clone(),
@@ -1040,21 +1033,11 @@ impl<'de> Deserialize<'de> for FindOne {
         #[derive(Deserialize)]
         struct Helper {
             filter: Option<Document>,
-            comment: Option<Bson>,
             #[serde(flatten)]
             options: FindOneOptions,
         }
 
-        let mut helper = Helper::deserialize(deserializer)?;
-        match helper.comment {
-            Some(Bson::String(string)) => {
-                helper.options.comment = Some(string);
-            }
-            Some(bson) => {
-                helper.options.comment_bson = Some(bson);
-            }
-            _ => {}
-        }
+        let helper = Helper::deserialize(deserializer)?;
 
         Ok(Self {
             filter: helper.filter,