From bbff64b7205e6953ef57df6cb6709e9deddc7d23 Mon Sep 17 00:00:00 2001 From: maiconpavi <75434446+maiconpavi@users.noreply.github.com> Date: Fri, 23 Jun 2023 17:57:05 -0300 Subject: [PATCH 1/6] RUST-1687 adding `human_readable` option to `replace_one_common` and `find_one_and_replace_common` --- src/coll.rs | 12 ++++++++---- src/coll/options.rs | 8 ++++++++ 2 files changed, 16 insertions(+), 4 deletions(-) diff --git a/src/coll.rs b/src/coll.rs index 4ccaf3e87..9487878b4 100644 --- a/src/coll.rs +++ b/src/coll.rs @@ -1119,14 +1119,16 @@ where options: impl Into>, session: impl Into>, ) -> Result> { + let mut options = options.into(); let replacement = to_document_with_options( replacement.borrow(), - SerializerOptions::builder().human_readable(false).build(), + SerializerOptions::builder() + .human_readable(options.as_ref().and_then(|opts| opts.human_readable)) + .build(), )?; let session = session.into(); - let mut options = options.into(); resolve_write_concern_with_session!(self, options, session.as_ref())?; let op = FindAndModify::::with_replace(self.namespace(), filter, replacement, options)?; @@ -1382,16 +1384,18 @@ where options: impl Into>, session: impl Into>, ) -> Result { + let mut options = options.into(); let replacement = to_document_with_options( replacement.borrow(), - SerializerOptions::builder().human_readable(false).build(), + SerializerOptions::builder() + .human_readable(options.as_ref().and_then(|opts| opts.human_readable)) + .build(), )?; bson_util::replacement_document_check(&replacement)?; let session = session.into(); - let mut options = options.into(); resolve_write_concern_with_session!(self, options, session.as_ref())?; let update = Update::new( diff --git a/src/coll/options.rs b/src/coll/options.rs index 140e6b910..dc05de60d 100644 --- a/src/coll/options.rs +++ b/src/coll/options.rs @@ -299,6 +299,10 @@ pub struct ReplaceOptions { /// /// This option is only available on server versions 4.4+. pub comment: Option, + + /// Sets the [`SerializerOptions::human_readable`] option for the [`Bson`] serializer. + /// The default value is true. + pub human_readable: Option, } /// Specifies the options to a @@ -443,6 +447,10 @@ pub struct FindOneAndReplaceOptions { /// /// This option is only available on server versions 4.4+. pub comment: Option, + + /// Sets the [`SerializerOptions::human_readable`] option for the [`Bson`] serializer. + /// The default value is true. + pub human_readable: Option, } /// Specifies the options to a From 1d9a60fbfd15c813dc37b0204fc2d5e074e45f7d Mon Sep 17 00:00:00 2001 From: maiconpavi <75434446+maiconpavi@users.noreply.github.com> Date: Fri, 23 Jun 2023 18:10:26 -0300 Subject: [PATCH 2/6] RUST-1687 fixing `human_readable` field documentation --- src/coll/options.rs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/coll/options.rs b/src/coll/options.rs index dc05de60d..47872e7cc 100644 --- a/src/coll/options.rs +++ b/src/coll/options.rs @@ -300,7 +300,7 @@ pub struct ReplaceOptions { /// This option is only available on server versions 4.4+. pub comment: Option, - /// Sets the [`SerializerOptions::human_readable`] option for the [`Bson`] serializer. + /// Sets the [`bson::SerializerOptions::human_readable`] option for the [`Bson`] serializer. /// The default value is true. pub human_readable: Option, } @@ -448,7 +448,7 @@ pub struct FindOneAndReplaceOptions { /// This option is only available on server versions 4.4+. pub comment: Option, - /// Sets the [`SerializerOptions::human_readable`] option for the [`Bson`] serializer. + /// Sets the [`bson::SerializerOptions::human_readable`] option for the [`Bson`] serializer. /// The default value is true. pub human_readable: Option, } From 496fd3be56305a24094b59f0febe43cb309c0840 Mon Sep 17 00:00:00 2001 From: maiconpavi <75434446+maiconpavi@users.noreply.github.com> Date: Sat, 5 Aug 2023 17:03:30 -0300 Subject: [PATCH 3/6] RUST-1687 adding `human_readable_serialization` field to `CollectionOptions` --- src/coll.rs | 16 +++++++++++++--- src/coll/options.rs | 12 ++++-------- src/operation/insert.rs | 14 ++++++++++++-- 3 files changed, 29 insertions(+), 13 deletions(-) diff --git a/src/coll.rs b/src/coll.rs index 9487878b4..7378015aa 100644 --- a/src/coll.rs +++ b/src/coll.rs @@ -143,6 +143,7 @@ struct CollectionInner { selection_criteria: Option, read_concern: Option, write_concern: Option, + human_readable_serialization: bool, } impl Collection { @@ -157,6 +158,7 @@ impl Collection { let write_concern = options .write_concern .or_else(|| db.write_concern().cloned()); + let human_readable_serialization = options.human_readable_serialization.unwrap_or_default(); Self { inner: Arc::new(CollectionInner { @@ -166,6 +168,7 @@ impl Collection { selection_criteria, read_concern, write_concern, + human_readable_serialization, }), _phantom: Default::default(), } @@ -1123,7 +1126,7 @@ where let replacement = to_document_with_options( replacement.borrow(), SerializerOptions::builder() - .human_readable(options.as_ref().and_then(|opts| opts.human_readable)) + .human_readable(self.inner.human_readable_serialization) .build(), )?; @@ -1207,7 +1210,13 @@ where while n_attempted < ds.len() { let docs: Vec<&T> = ds.iter().skip(n_attempted).map(Borrow::borrow).collect(); - let insert = Insert::new_encrypted(self.namespace(), docs, options.clone(), encrypted); + let insert = Insert::new_encrypted( + self.namespace(), + docs, + options.clone(), + encrypted, + self.inner.human_readable_serialization, + ); match self .client() @@ -1334,6 +1343,7 @@ where self.namespace(), vec![doc], options.map(InsertManyOptions::from_insert_one_options), + self.inner.human_readable_serialization, ); self.client() .execute_operation(insert, session) @@ -1388,7 +1398,7 @@ where let replacement = to_document_with_options( replacement.borrow(), SerializerOptions::builder() - .human_readable(options.as_ref().and_then(|opts| opts.human_readable)) + .human_readable(self.inner.human_readable_serialization) .build(), )?; diff --git a/src/coll/options.rs b/src/coll/options.rs index 47872e7cc..efbeff886 100644 --- a/src/coll/options.rs +++ b/src/coll/options.rs @@ -28,6 +28,10 @@ pub struct CollectionOptions { /// The default write concern for operations. pub write_concern: Option, + + /// Sets the [`bson::SerializerOptions::human_readable`] option for the [`Bson`] serializer. + /// The default value is `false`. + pub human_readable_serialization: Option, } /// Specifies whether a @@ -299,10 +303,6 @@ pub struct ReplaceOptions { /// /// This option is only available on server versions 4.4+. pub comment: Option, - - /// Sets the [`bson::SerializerOptions::human_readable`] option for the [`Bson`] serializer. - /// The default value is true. - pub human_readable: Option, } /// Specifies the options to a @@ -447,10 +447,6 @@ pub struct FindOneAndReplaceOptions { /// /// This option is only available on server versions 4.4+. pub comment: Option, - - /// Sets the [`bson::SerializerOptions::human_readable`] option for the [`Bson`] serializer. - /// The default value is true. - pub human_readable: Option, } /// Specifies the options to a diff --git a/src/operation/insert.rs b/src/operation/insert.rs index 90307e8a7..b055f8c1b 100644 --- a/src/operation/insert.rs +++ b/src/operation/insert.rs @@ -31,6 +31,7 @@ pub(crate) struct Insert<'a, T> { inserted_ids: Vec, options: Option, encrypted: bool, + human_readable_serialization: bool, } impl<'a, T> Insert<'a, T> { @@ -38,8 +39,9 @@ impl<'a, T> Insert<'a, T> { ns: Namespace, documents: Vec<&'a T>, options: Option, + human_readable_serialization: bool, ) -> Self { - Self::new_encrypted(ns, documents, options, false) + Self::new_encrypted(ns, documents, options, false, human_readable_serialization) } pub(crate) fn new_encrypted( @@ -47,6 +49,7 @@ impl<'a, T> Insert<'a, T> { documents: Vec<&'a T>, options: Option, encrypted: bool, + human_readable_serialization: bool, ) -> Self { Self { ns, @@ -54,6 +57,7 @@ impl<'a, T> Insert<'a, T> { documents, inserted_ids: vec![], encrypted, + human_readable_serialization, } } @@ -75,6 +79,9 @@ impl<'a, T: Serialize> OperationWithDefaults for Insert<'a, T> { let mut docs = RawArrayBuf::new(); let mut size = 0; let batch_size_limit = description.max_bson_object_size as u64; + let serializer_options = bson::SerializerOptions::builder() + .human_readable(self.human_readable_serialization) + .build(); for (i, d) in self .documents @@ -82,7 +89,10 @@ impl<'a, T: Serialize> OperationWithDefaults for Insert<'a, T> { .take(description.max_write_batch_size as usize) .enumerate() { - let mut doc = bson::to_raw_document_buf(d)?; + let mut doc = bson::RawDocumentBuf::from_document(&bson::to_document_with_options( + d, + serializer_options.clone(), + )?)?; let id = match doc.get("_id")? { Some(b) => b.try_into()?, None => { From dd11157775f341b2d973a67d73c0b88012337aff Mon Sep 17 00:00:00 2001 From: maiconpavi <75434446+maiconpavi@users.noreply.github.com> Date: Fri, 11 Aug 2023 09:03:55 -0300 Subject: [PATCH 4/6] RUST-1687 improving `Insert` operation serialization performance when `human_readable_serialization` is false --- src/operation/insert.rs | 18 +++++++++++------- 1 file changed, 11 insertions(+), 7 deletions(-) diff --git a/src/operation/insert.rs b/src/operation/insert.rs index b055f8c1b..f9f1e3254 100644 --- a/src/operation/insert.rs +++ b/src/operation/insert.rs @@ -79,9 +79,6 @@ impl<'a, T: Serialize> OperationWithDefaults for Insert<'a, T> { let mut docs = RawArrayBuf::new(); let mut size = 0; let batch_size_limit = description.max_bson_object_size as u64; - let serializer_options = bson::SerializerOptions::builder() - .human_readable(self.human_readable_serialization) - .build(); for (i, d) in self .documents @@ -89,10 +86,17 @@ impl<'a, T: Serialize> OperationWithDefaults for Insert<'a, T> { .take(description.max_write_batch_size as usize) .enumerate() { - let mut doc = bson::RawDocumentBuf::from_document(&bson::to_document_with_options( - d, - serializer_options.clone(), - )?)?; + let mut doc = if self.human_readable_serialization { + let serializer_options = bson::SerializerOptions::builder() + .human_readable(true) + .build(); + bson::RawDocumentBuf::from_document(&bson::to_document_with_options( + d, + serializer_options, + )?)? + } else { + bson::to_raw_document_buf(d)? + }; let id = match doc.get("_id")? { Some(b) => b.try_into()?, None => { From f42bebdd9884148c34a3ba12293ad308d16e87dd Mon Sep 17 00:00:00 2001 From: maiconpavi <75434446+maiconpavi@users.noreply.github.com> Date: Fri, 11 Aug 2023 09:06:22 -0300 Subject: [PATCH 5/6] RUST-1687 improving `CollectionOptions::human_readable_serialization` field documentation --- src/coll/options.rs | 1 + 1 file changed, 1 insertion(+) diff --git a/src/coll/options.rs b/src/coll/options.rs index efbeff886..a6a1747d8 100644 --- a/src/coll/options.rs +++ b/src/coll/options.rs @@ -31,6 +31,7 @@ pub struct CollectionOptions { /// Sets the [`bson::SerializerOptions::human_readable`] option for the [`Bson`] serializer. /// The default value is `false`. + /// Note: Specifying `true` for this value will decrease the performance of insert operations. pub human_readable_serialization: Option, } From 2c02d2569e16bb9b96d2b0ec2d2a211e86c9a63b Mon Sep 17 00:00:00 2001 From: Isabel Atkinson Date: Mon, 21 Aug 2023 11:38:22 -0600 Subject: [PATCH 6/6] fix tests and add test --- src/operation/insert/test.rs | 10 +- src/test/coll.rs | 121 ++++++++++++++++++++++ src/test/spec/unified_runner/test_file.rs | 1 + 3 files changed, 129 insertions(+), 3 deletions(-) diff --git a/src/operation/insert/test.rs b/src/operation/insert/test.rs index 4e2b02ad8..97de1e9b1 100644 --- a/src/operation/insert/test.rs +++ b/src/operation/insert/test.rs @@ -49,6 +49,7 @@ fn fixtures(opts: Option) -> TestFixtures { }, DOCUMENTS.iter().collect(), Some(options.clone()), + false, ); TestFixtures { @@ -116,7 +117,7 @@ fn build() { #[test] fn build_ordered() { let docs = vec![Document::new()]; - let mut insert = Insert::new(Namespace::empty(), docs.iter().collect(), None); + let mut insert = Insert::new(Namespace::empty(), docs.iter().collect(), None, false); let cmd = insert .build(&StreamDescription::new_testing()) .expect("should succeed"); @@ -128,6 +129,7 @@ fn build_ordered() { Namespace::empty(), docs.iter().collect(), Some(InsertManyOptions::builder().ordered(false).build()), + false, ); let cmd = insert .build(&StreamDescription::new_testing()) @@ -140,6 +142,7 @@ fn build_ordered() { Namespace::empty(), docs.iter().collect(), Some(InsertManyOptions::builder().ordered(true).build()), + false, ); let cmd = insert .build(&StreamDescription::new_testing()) @@ -152,6 +155,7 @@ fn build_ordered() { Namespace::empty(), docs.iter().collect(), Some(InsertManyOptions::builder().build()), + false, ); let cmd = insert .build(&StreamDescription::new_testing()) @@ -170,7 +174,7 @@ struct Documents { fn generate_ids() { let docs = vec![doc! { "x": 1 }, doc! { "_id": 1_i32, "x": 2 }]; - let mut insert = Insert::new(Namespace::empty(), docs.iter().collect(), None); + let mut insert = Insert::new(Namespace::empty(), docs.iter().collect(), None, false); let cmd = insert.build(&StreamDescription::new_testing()).unwrap(); let serialized = insert.serialize_command(cmd).unwrap(); @@ -253,7 +257,7 @@ fn serialize_all_types() { "_id": ObjectId::new(), }]; - let mut insert = Insert::new(Namespace::empty(), docs.iter().collect(), None); + let mut insert = Insert::new(Namespace::empty(), docs.iter().collect(), None, false); let cmd = insert.build(&StreamDescription::new_testing()).unwrap(); let serialized = insert.serialize_command(cmd).unwrap(); let cmd: Documents = bson::from_slice(serialized.as_slice()).unwrap(); diff --git a/src/test/coll.rs b/src/test/coll.rs index 9ee16486b..3104c86c5 100644 --- a/src/test/coll.rs +++ b/src/test/coll.rs @@ -1214,3 +1214,124 @@ fn test_namespace_fromstr() { assert_eq!(t.db, "something"); assert_eq!(t.coll, "something.else"); } + +#[cfg_attr(feature = "tokio-runtime", tokio::test)] +#[cfg_attr(feature = "async-std-runtime", async_std::test)] +async fn configure_human_readable_serialization() { + #[derive(Deserialize)] + struct StringOrBytes(String); + + impl Serialize for StringOrBytes { + fn serialize(&self, serializer: S) -> std::result::Result + where + S: serde::Serializer, + { + if serializer.is_human_readable() { + serializer.serialize_str(&self.0) + } else { + serializer.serialize_bytes(self.0.as_bytes()) + } + } + } + + #[derive(Deserialize, Serialize)] + struct Data { + id: u32, + s: StringOrBytes, + } + + let client = TestClient::new().await; + + let collection_options = CollectionOptions::builder() + .human_readable_serialization(false) + .build(); + + let non_human_readable_collection: Collection = client + .database("db") + .collection_with_options("nonhumanreadable", collection_options); + non_human_readable_collection.drop(None).await.unwrap(); + + non_human_readable_collection + .insert_one( + Data { + id: 0, + s: StringOrBytes("non human readable!".into()), + }, + None, + ) + .await + .unwrap(); + + // The inserted bytes will not deserialize to StringOrBytes properly, so find as a document + // instead. + let document_collection = non_human_readable_collection.clone_with_type::(); + let doc = document_collection + .find_one(doc! { "id": 0 }, None) + .await + .unwrap() + .unwrap(); + assert!(doc.get_binary_generic("s").is_ok()); + + non_human_readable_collection + .replace_one( + doc! { "id": 0 }, + Data { + id: 1, + s: StringOrBytes("non human readable!".into()), + }, + None, + ) + .await + .unwrap(); + + let doc = document_collection + .find_one(doc! { "id": 1 }, None) + .await + .unwrap() + .unwrap(); + assert!(doc.get_binary_generic("s").is_ok()); + + let collection_options = CollectionOptions::builder() + .human_readable_serialization(true) + .build(); + + let human_readable_collection: Collection = client + .database("db") + .collection_with_options("humanreadable", collection_options); + human_readable_collection.drop(None).await.unwrap(); + + human_readable_collection + .insert_one( + Data { + id: 0, + s: StringOrBytes("human readable!".into()), + }, + None, + ) + .await + .unwrap(); + + // Proper deserialization to a string demonstrates that the data was correctly serialized as a + // string. + human_readable_collection + .find_one(doc! { "id": 0 }, None) + .await + .unwrap(); + + human_readable_collection + .replace_one( + doc! { "id": 0 }, + Data { + id: 1, + s: StringOrBytes("human readable!".into()), + }, + None, + ) + .await + .unwrap(); + + human_readable_collection + .find_one(doc! { "id": 1 }, None) + .await + .unwrap(); +} diff --git a/src/test/spec/unified_runner/test_file.rs b/src/test/spec/unified_runner/test_file.rs index ec9daf7b6..e1876b751 100644 --- a/src/test/spec/unified_runner/test_file.rs +++ b/src/test/spec/unified_runner/test_file.rs @@ -378,6 +378,7 @@ impl CollectionOrDatabaseOptions { read_concern: self.read_concern.clone(), selection_criteria: self.selection_criteria.clone(), write_concern: self.write_concern.clone(), + human_readable_serialization: None, } } }