RUST-1082 Implement `Serialize` and `Deserialize` for raw BSON types #318

patrickfreed · 2021-11-01T23:55:59Z

This PR adds Serialize and Deserialize implementations for the raw BSON types.

e.g. the following is now possible:

#[derive(Debug, Deserialize, Serialize)]
struct MyData<'a> {
    name: &'a str,

    #[serde(borrow)]
    binary: RawBinary<'a>,

    #[serde(borrow)]
    doc: &'a RawDocument,

    #[serde(borrow)]
    some_field: RawBson<'a>,
}

The implementations behave as follows:

&RawDocument:
- for human readable formats: iterate over document and serialize each kvp
- for binary formats: serialize the document as a newtype around a slice of bytes.
  - The BSON serializers handle this by interpreting the name of the newtype
- Open Question: should this use something like { "$rawBsonDocument": <bytes> } for binary, non-BSON formats? This will allow RawBson to round trip, but that's about it.
RawDocumentBuf: same as RawDocument. In a future PR it will be able to deserialize from arbitrary maps, but that requires the ability to append.
RawBson:
- Defer to the wrapped type
- The types that map directly to the serde data model (e.g. integers) are deserialized as-is, similar to Bson.
- For everything else, the extended JSON serde data model equivalent is used, also similar to Bson
  - for many types, the model needed to be changed a little bit to ensure to extra allocations are done
&RawArray
- Same as &RawDocument, but uses a different newtype name to signal an array.
RawJavaScriptCodeWithScope
- serialize to/from: { "$code": <borrowed string>, "$scope": <borrowed string> }
RawDbPointer
- serialize to/from: { "$ref": <borrowed string>, "$id": <objectid> }
ObjectId
- Same as before, but it can now also deserialize from { "$oid": <12 byte slice> }
RawBinary
- On human readable platforms, serializes to ext JSON format in serde data model
- On binary formats, serializes to { "$binary": { "bytes": <borrowed bytes>, subType: <u8> } } in the serde data model
- In both cases, the corresponding serializer handles it and correctly encodes as BSON
- In order for RawBinary to round trip on non-BSON formats, the separate bytes model (rather than base64 string model) was required. It is also much faster.
- Unfortunately, we can't also update Binary to exhibit this behavior, as it would technically be a breaking change.
RawRegex:
- serialize to/from extended JSON format but uses borrowed instead of owned strings
DateTime: updated to also deserialize from { "$date": <i64> } to avoid a string allocation during deserialization.

patrickfreed · 2021-11-01T23:57:16Z

serde-tests/test.rs

+}
+
+#[test]
+fn all_types() {


This test is the same, but I moved the struct definition and initialization out of the body to reuse it in other tests.

patrickfreed · 2021-11-02T00:01:30Z

serde-tests/test.rs

+        "max_key": { "$maxKey": 1 },
+    });
+
+    assert_eq!(serde_json::to_value(&v).unwrap(), json);


this test only verifies that serialization works, since deserializing binary values from JSON is very limited (serde_json treats byte slices as arrays of integers, which Binary can't round trip from). I filed RUST-1091 to cover the work for adding more robust tests in this area though.

patrickfreed · 2021-11-02T00:05:06Z

src/de/raw.rs

+/// Hint provided to the deserializer via `deserialize_newtype_struct` as to the type of thing
+/// being deserialized.
+#[derive(Debug, Clone, Copy)]
+enum DeserializerHint {


I introduced this hint type as a way of tracking whether or not we're deserializing raw BSON types. If we are, we use slightly different visiting patterns. For example, a document will be visited as a slice of bytes rather than as a map. It's also used for the UUID serialization hinting behavior introduced in #314. The hint communicated for raw BSON the same way it is for UUIDs, i.e. via a specifically named newtype struct. See deserialize_newtype_struct for where that happens.

patrickfreed · 2021-11-02T00:08:50Z

src/de/raw.rs

-    ) -> Result<V::Value>
+    /// Deserialize the next element in the BSON, using the type of the element along with the
+    /// provided hint to determine how to visit the data.
+    fn deserialize_next<V>(&mut self, visitor: V, hint: DeserializerHint) -> Result<V::Value>


In order to borrow from the underling BSON data, we had to introduce new MapAccess implementations for the various types that didn't get them in the initial implementation (e.g. Regex).

patrickfreed · 2021-11-02T00:21:15Z

src/ser/raw/mod.rs

+}
+
+#[derive(Debug, Clone, Copy)]
+enum SerializerHint {


Similar to deserialization, we use a hint here to indicate what the next call to serialize_bytes is serializing.

patrickfreed · 2021-11-02T00:33:36Z

src/tests/spec/corpus.rs

@@ -59,6 +65,34 @@ struct ParseError {
    string: String,
 }

+struct FieldVisitor<'a, T>(&'a str, PhantomData<T>);


This type is used to deserialize the value provided by a valid corpus test. We need to do some runtime hackery since the field name isn't the same per test and is instead provided in the field_name field.

abr-egn

LGTM! Extremely minor comments only.

On a larger scope, it's difficult for me to keep all the cogs in my head to get a sense of the overall machine operation - a walkthrough would be very welcome at some point.

abr-egn · 2021-11-03T20:51:35Z

src/de/raw.rs

+    /// Whether the first key has been deserialized yet or not.
+    deserialized_first: bool,
+
+    /// Whether or not this document being deserialized is for anarray or not.


Typo: "anarray" => "an array"

abr-egn · 2021-11-04T14:02:56Z

src/raw/bson.rs

+    where
+        E: serde::de::Error,
+    {
+        crate::de::convert_unsigned_to_signed_raw(value)


Nit: might be worth just useing convert_unsigned_to_signed_raw?

patrickfreed

Yeah I agree, this code is pretty hard to read due to the sheer amount of boilerplate involved, especially across multiple serializer/deserializer implementations. I filed RUST-1098 to cover the work for writing up a doc that explains it all at a high level.

isabelatkinson

lgtm with one typo fix

isabelatkinson · 2021-11-09T15:56:30Z

src/raw/document.rs

+            RawBson::Document(d) => Ok(d),
+
+            // For non-BSON formats, RawDocument gets serialized as bytes, so we need to deserialize
+            // from them here too. For BSON, the deserialzier will return an error if it


Suggested change

// from them here too. For BSON, the deserialzier will return an error if it

// from them here too. For BSON, the deserializer will return an error if it

patrickfreed added 18 commits November 1, 2021 14:59

implement deserialize for RawBson, RawDocument, etc.

f8ba83b

cleanup debug print, add comments

fbe1a87

WIP serialize

176d5c6

finish RawBinary serialization

f1a5650

RawRegex serialization

766f370

RawJavaScriptCodeWithScope serialization

f0ce883

RawDbPointer serialize

abba244

RawDocument serialize

da88ab2

RawBson serialize

d42f467

RawArray serialization

58a6d31

add corpus tests, fix discovered bugs

3c96965

extend corpus tests, add serde-tests

c3b6f70

simplify binary deserialization

9f03606

revert breaking changes to binary serialization output

162000b

fix clippy, simplify binary serialization

bec78c7

ensure RawDocument and co can round trip through non-BSON

8a53f7b

ensure hint is cleared after use

efb8c5a

fix compilation on 1.48

bfb0dd8

patrickfreed commented Nov 2, 2021

View reviewed changes

patrickfreed marked this pull request as ready for review November 2, 2021 15:57

patrickfreed requested review from isabelatkinson and abr-egn November 2, 2021 15:57

abr-egn approved these changes Nov 4, 2021

View reviewed changes

patrickfreed commented Nov 8, 2021

View reviewed changes

isabelatkinson approved these changes Nov 9, 2021

View reviewed changes

patrickfreed added 3 commits November 9, 2021 11:50

typo fix

b05569b

add use

6da7574

fix typo

2e56b81

abr-egn approved these changes Nov 9, 2021

View reviewed changes

patrickfreed merged commit e47b276 into mongodb:master Nov 9, 2021

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

RUST-1082 Implement `Serialize` and `Deserialize` for raw BSON types #318

RUST-1082 Implement `Serialize` and `Deserialize` for raw BSON types #318

Uh oh!

patrickfreed commented Nov 1, 2021

Uh oh!

patrickfreed Nov 1, 2021

Uh oh!

patrickfreed Nov 2, 2021

Uh oh!

patrickfreed Nov 2, 2021

Uh oh!

patrickfreed Nov 2, 2021

Uh oh!

patrickfreed Nov 2, 2021

Uh oh!

patrickfreed Nov 2, 2021

Uh oh!

abr-egn left a comment

Uh oh!

abr-egn Nov 3, 2021

Uh oh!

patrickfreed Nov 8, 2021

Uh oh!

abr-egn Nov 4, 2021

Uh oh!

patrickfreed Nov 8, 2021

Uh oh!

patrickfreed left a comment

Uh oh!

isabelatkinson left a comment

Uh oh!

isabelatkinson Nov 9, 2021

Uh oh!

Uh oh!

	// from them here too. For BSON, the deserialzier will return an error if it
	// from them here too. For BSON, the deserializer will return an error if it

RUST-1082 Implement Serialize and Deserialize for raw BSON types #318

RUST-1082 Implement Serialize and Deserialize for raw BSON types #318

Uh oh!

Conversation

patrickfreed commented Nov 1, 2021

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

abr-egn left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

patrickfreed left a comment

Choose a reason for hiding this comment

Uh oh!

isabelatkinson left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

RUST-1082 Implement `Serialize` and `Deserialize` for raw BSON types #318

RUST-1082 Implement `Serialize` and `Deserialize` for raw BSON types #318