Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add serde support to Blob type #2647

Merged
merged 20 commits into from
Jun 6, 2023
Merged
Show file tree
Hide file tree
Changes from 6 commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
10 changes: 10 additions & 0 deletions rust-runtime/aws-smithy-types/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,7 @@ serde = { version = "1", features = ["derive"] }
serde_json = "1"
criterion = "0.4"
rand = "0.8.4"
ciborium = "0.2.0"

[package.metadata.docs.rs]
all-features = true
Expand All @@ -32,3 +33,12 @@ rustdoc-args = ["--cfg", "docsrs"]
[[bench]]
name = "base64"
harness = false


[target."cfg(aws_sdk_unstable)".dependencies.serde]
version = "1"
features = ["derive"]

[features]
serde-serialize = []
serde-deserialize = []
4 changes: 3 additions & 1 deletion rust-runtime/aws-smithy-types/additional-ci
Original file line number Diff line number Diff line change
Expand Up @@ -5,8 +5,10 @@
#

# This script contains additional CI checks to run for this specific package

set -e

echo "### Checking for duplicate dependency versions in the normal dependency graph with all features enabled"
cargo tree -d --edges normal --all-features

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It should be working now.

echo "### Checking whether the features are properly feature-gated"
! cargo tree -e no-dev | grep serde
thomas-k-cameron marked this conversation as resolved.
Show resolved Hide resolved
125 changes: 125 additions & 0 deletions rust-runtime/aws-smithy-types/src/blob.rs
Original file line number Diff line number Diff line change
Expand Up @@ -30,3 +30,128 @@ impl AsRef<[u8]> for Blob {
&self.inner
}
}

#[cfg(all(aws_sdk_unstable, feature = "serde-serialize"))]
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

what about refactoring this into a module so you can feature gate it once?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Gotcha.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

By the way, why do you have both base64 and base64-simd?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We should only be using base64-simd, I think. base64 is there for this bench comparing base64 to base64-simd.

mod serde_serialize {
use super::*;
use crate::base64;
use serde::Serialize;

impl Serialize for Blob {
fn serialize<S>(&self, serializer: S) -> Result<S::Ok, S::Error>
where
S: serde::Serializer,
{
if serializer.is_human_readable() {
serializer.serialize_str(&crate::base64::encode(&self.inner))
} else {
serializer.serialize_bytes(&self.inner)
}
}
}
}

#[cfg(all(aws_sdk_unstable, feature = "serde-deserialize"))]
mod serde_deserialize {
use super::*;
use crate::base64;
use serde::{de::Visitor, Deserialize};

struct HumanReadableBlobVisitor;
impl<'de> Visitor<'de> for HumanReadableBlobVisitor {
type Value = Blob;
fn expecting(&self, formatter: &mut std::fmt::Formatter<'_>) -> std::fmt::Result {
formatter.write_str("expected base64 encoded string")
}

fn visit_str<E>(self, v: &str) -> Result<Self::Value, E>
where
E: serde::de::Error,
{
match base64::decode(v) {
Ok(inner) => Ok(Blob { inner }),
Err(e) => Err(E::custom(e)),
}
}
}

struct NotHumanReadableBlobVisitor;
impl<'de> Visitor<'de> for NotHumanReadableBlobVisitor {
type Value = Blob;
fn expecting(&self, formatter: &mut std::fmt::Formatter<'_>) -> std::fmt::Result {
formatter.write_str("expected base64 encoded string")
thomas-k-cameron marked this conversation as resolved.
Show resolved Hide resolved
}

fn visit_byte_buf<E>(self, v: Vec<u8>) -> Result<Self::Value, E>
where
E: serde::de::Error,
{
Ok(Blob { inner: v })
}
}

impl<'de> Deserialize<'de> for Blob {
fn deserialize<D>(deserializer: D) -> Result<Self, D::Error>
where
D: serde::Deserializer<'de>,
{
if deserializer.is_human_readable() {
deserializer.deserialize_str(HumanReadableBlobVisitor)
} else {
deserializer.deserialize_byte_buf(NotHumanReadableBlobVisitor)
}
}
}
}

#[cfg(test)]
#[cfg(all(
aws_sdk_unstable,
feature = "serde-serialize",
feature = "serde-deserialize"
))]
mod test_serde {
use crate::Blob;
use serde::{Deserialize, Serialize};
use std::collections::HashMap;

#[derive(Deserialize, Serialize, Debug, PartialEq)]
struct ForTest {
blob: Blob,
}

#[test]
fn human_readable_blob() {
let aws_in_base64 = r#"{"blob":"QVdT"}"#;
let for_test = ForTest {
blob: Blob {
inner: vec![b'A', b'W', b'S'],
},
};
assert_eq!(for_test, serde_json::from_str(aws_in_base64).unwrap());
assert_eq!(serde_json::to_string(&for_test).unwrap(), aws_in_base64);
}

#[test]
fn not_human_readable_blob() {
use std::ffi::CString;

let for_test = ForTest {
blob: Blob {
inner: vec![b'A', b'W', b'S'],
},
};
let mut buf = vec![];
let res = ciborium::ser::into_writer(&for_test, &mut buf);
assert!(res.is_ok());

// checks whether the bytes are deserialiezd properly
thomas-k-cameron marked this conversation as resolved.
Show resolved Hide resolved
let n: HashMap<String, CString> =
ciborium::de::from_reader(std::io::Cursor::new(buf.clone())).unwrap();
assert!(n.get("blob").is_some());
assert!(n.get("blob") == CString::new([65, 87, 83]).ok().as_ref());

let de: ForTest = ciborium::de::from_reader(std::io::Cursor::new(buf)).unwrap();
assert_eq!(for_test, de);
}
}