Skip to content

AVRO-3484: Add support for deriving a default value for a record field #1651

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.

Sign up for GitHub

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

Merged
martin-g merged 1 commit into from
Apr 20, 2022
Merged

AVRO-3484: Add support for deriving a default value for a record field #1651

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
5 changes: 3 additions & 2 deletions lang/rust/avro_derive/Cargo.toml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -32,10 +32,11 @@ documentation = "https://docs.rs/apache-avro-derive"
proc-macro = true

[dependencies]
syn = {version= "1.0.91", features=["full", "fold"]}
darling = "0.14.0"
quote = "1.0.18"
syn = {version= "1.0.91", features=["full", "fold"]}
proc-macro2 = "1.0.37"
darling = "0.14.0"
serde_json = "1.0.79"

[dev-dependencies]
serde = { version = "1.0.136", features = ["derive"] }
Expand Down
18 changes: 14 additions & 4 deletions lang/rust/avro_derive/src/lib.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,8 @@ use syn::{
struct FieldOptions {
#[darling(default)]
doc: Option<String>,
#[darling(default)]
default: Option<String>,
}

#[derive(FromAttributes)]
Expand Down Expand Up @@ -117,16 +119,24 @@ fn get_data_struct_schema_def(
syn::Fields::Named(ref a) => {
for (position, field) in a.named.iter().enumerate() {
let name = field.ident.as_ref().unwrap().to_string(); // we know everything has a name
let field_documented =
let field_attrs =
FieldOptions::from_attributes(&field.attrs[..]).map_err(darling_to_syn)?;
let doc = preserve_optional(field_documented.doc);
let doc = preserve_optional(field_attrs.doc);
let default_value = match field_attrs.default {
Some(default_value) => {
quote! {
Copy link
Contributor

@jklamer jklamer Apr 22, 2022

Choose a reason for hiding this comment

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

NIT: I would prefer to run serde_json::from_str(#default_value) here and throw an error if invalid. To avoid runtime errors.

Copy link
Member Author

@martin-g martin-g Apr 22, 2022

Choose a reason for hiding this comment

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

I will need your help with this!

I changed it to:

diff --git lang/rust/avro_derive/src/lib.rs lang/rust/avro_derive/src/lib.rs
index 423625375..d8305452a 100644
--- lang/rust/avro_derive/src/lib.rs
+++ lang/rust/avro_derive/src/lib.rs
@@ -124,14 +124,16 @@ fn get_data_struct_schema_def(
                 let doc = preserve_optional(field_attrs.doc);
                 let default_value = match field_attrs.default {
                     Some(default_value) => {
+                        let default_value: serde_json::Value = serde_json::from_str(&default_value)
+                            // .expect(format!("Invalid JSON: {:?}", default_value).as_str())
+                            .map_err(|e| vec![syn::Error::new(error_span, e.to_string())])?;
                         quote! {
-                            Some(serde_json::from_str(#default_value).expect(format!("Invalid JSON: {:?}", #default_value).as_str()))
+                            Some(#default_value)
                         }
                     }
                     None => quote! { None },

but it fails with:

  Compiling apache-avro-derive v0.14.0 (/home/martin/git/apache/avro/lang/rust/avro_derive)
error[E0277]: the trait bound `Value: ToTokens` is not satisfied
   --> avro_derive/src/lib.rs:130:25
    |
130 | /                         quote! {
131 | |                             Some(#default_value)
132 | |                         }
    | |_________________________^ the trait `ToTokens` is not implemented for `Value`
    |
    = note: this error originates in the macro `$crate::quote` (in Nightly builds, run with -Z macro-backtrace for more info)

and I have no idea what to do :-/

Copy link
Contributor

@jklamer jklamer Apr 23, 2022

Choose a reason for hiding this comment

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

oooo yeah I wasn't thinking anything so sophisticated as using the json::value in the quotes. Just using a dummy command and translating the error: #1668

Some(serde_json::from_str(#default_value).expect(format!("Invalid JSON: {:?}", #default_value).as_str()))
}
}
None => quote! { None },
};
let schema_expr = type_to_schema_expr(&field.ty)?;
let position = position;
record_field_exprs.push(quote! {
apache_avro::schema::RecordField {
name: #name.to_string(),
doc: #doc,
default: Option::None,
default: #default_value,
schema: #schema_expr,
order: apache_avro::schema::RecordFieldOrder::Ascending,
position: #position,
Expand Down Expand Up @@ -186,7 +196,7 @@ fn get_data_enum_schema_def(
name: apache_avro::schema::Name::new(#full_schema_name).expect(&format!("Unable to parse enum name for schema {}", #full_schema_name)[..]),
aliases: #enum_aliases,
doc: #doc,
symbols: vec![#(#symbols.to_owned()),*]
symbols: vec![#(#symbols.to_owned()),*],
}
})
} else {
Expand Down
130 changes: 130 additions & 0 deletions lang/rust/avro_derive/tests/derive.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1209,4 +1209,134 @@ mod test_derive {

serde_assert(TestBasicEnumWithAliases2::B);
}

#[test]
fn test_basic_struct_with_defaults() {
#[derive(Debug, Deserialize, Serialize, AvroSchema, Clone, PartialEq)]
enum MyEnum {
Foo,
Bar,
Baz,
}

#[derive(Debug, Serialize, Deserialize, AvroSchema, Clone, PartialEq)]
struct TestBasicStructWithDefaultValues {
#[avro(default = "123")]
a: i32,
#[avro(default = r#""The default value for 'b'""#)]
b: String,
#[avro(default = "true")]
condition: bool,
// no default value for 'c'
c: f64,
#[avro(default = r#"{"a": 1, "b": 2}"#)]
map: HashMap<String, i32>,

#[avro(default = "[1, 2, 3]")]
array: Vec<i32>,

#[avro(default = r#""Foo""#)]
myenum: MyEnum,
}

let schema = r#"
{
"type":"record",
"name":"TestBasicStructWithDefaultValues",
"fields": [
{
"name":"a",
"type":"int",
"default":123
},
{
"name":"b",
"type":"string",
"default": "The default value for 'b'"
},
{
"name":"condition",
"type":"boolean",
"default":true
},
{
"name":"c",
"type":"double"
},
{
"name":"map",
"type":{
"type":"map",
"values":"int"
},
"default": {
"a": 1,
"b": 2
}
},
{
"name":"array",
"type":{
"type":"array",
"items":"int"
},
"default": [1, 2, 3]
},
{
"name":"myenum",
"type":{
"type":"enum",
"name":"MyEnum",
"symbols":["Foo", "Bar", "Baz"]
},
"default":"Foo"
}
]
}
"#;

let schema = Schema::parse_str(schema).unwrap();
if let Schema::Record { name, fields, .. } = TestBasicStructWithDefaultValues::get_schema()
{
assert_eq!("TestBasicStructWithDefaultValues", name.fullname(None));
use serde_json::json;
for field in fields {
match field.name.as_str() {
"a" => assert_eq!(Some(json!(123_i32)), field.default),
"b" => assert_eq!(
Some(json!(r#"The default value for 'b'"#.to_owned())),
field.default
),
"condition" => assert_eq!(Some(json!(true)), field.default),
"array" => assert_eq!(Some(json!([1, 2, 3])), field.default),
"map" => assert_eq!(
Some(json!({
"a": 1,
"b": 2
})),
field.default
),
"c" => assert_eq!(None, field.default),
"myenum" => assert_eq!(Some(json!("Foo")), field.default),
_ => panic!("Unexpected field name"),
}
}
} else {
panic!("TestBasicStructWithDefaultValues schema must be a record schema")
}
assert_eq!(schema, TestBasicStructWithDefaultValues::get_schema());

serde_assert(TestBasicStructWithDefaultValues {
a: 321,
b: "A custom value for 'b'".to_owned(),
condition: false,
c: 987.654,
map: [("a".to_owned(), 1), ("b".to_owned(), 2)]
.iter()
.cloned()
.collect(),
array: vec![4, 5, 6],
myenum: MyEnum::Bar,
});
}
}