Skip to content

Commit

Permalink
AVRO-3818: [Rust] Let inner named types inherit their enclosing names…
Browse files Browse the repository at this point in the history 
…pace if they have no their own namespace (#2405)

* Fix to inherit enclosing namespace.

* AVRO-3818: Fix a minor typo in comment

* AVRO-3818: Fix a minor typo in comment

---------

Co-authored-by: Martin Grigorov <martin-g@users.noreply.github.com>
  • Loading branch information
@sarutak @martin-g
sarutak and martin-g authored Jul 31, 2023
1 parent 0b82afc commit dd7d8ee
Showing 1 changed file with 178 additions and 17 deletions.
195 changes: 178 additions & 17 deletions lang/rust/avro/src/schema.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -239,7 +239,10 @@ impl Name {
}

/// Parse a `serde_json::Value` into a `Name`.
pub(crate) fn parse(complex: &Map<String, Value>) -> AvroResult<Self> {
pub(crate) fn parse(
complex: &Map<String, Value>,
enclosing_namespace: &Namespace,
) -> AvroResult<Self> {
let (name, namespace_from_name) = complex
.name()
.map(|name| Name::get_name_and_namespace(name.as_str()).unwrap())
Expand All @@ -252,8 +255,12 @@ impl Name {

Ok(Self {
name: type_name.unwrap_or(name),
namespace: namespace_from_name
.or_else(|| complex.string("namespace").filter(|ns| !ns.is_empty())),
namespace: namespace_from_name.or_else(|| {
complex
.string("namespace")
.or(enclosing_namespace.clone())
.filter(|ns| !ns.is_empty())
}),
})
}

Expand Down Expand Up @@ -320,7 +327,7 @@ impl<'de> Deserialize<'de> for Name {
Value::deserialize(deserializer).and_then(|value| {
use serde::de::Error;
if let Value::Object(json) = value {
Name::parse(&json).map_err(Error::custom)
Name::parse(&json, &None).map_err(Error::custom)
} else {
Err(Error::custom(format!("Expected a JSON object: {value:?}")))
}
Expand Down Expand Up @@ -918,7 +925,7 @@ impl Schema {
for js in input {
let schema: Value = serde_json::from_str(js).map_err(Error::ParseSchemaJson)?;
if let Value::Object(inner) = &schema {
let name = Name::parse(inner)?;
let name = Name::parse(inner, &None)?;
let previous_value = input_schemas.insert(name.clone(), schema);
if previous_value.is_some() {
return Err(Error::NameCollision(name.fullname(None)));
Expand Down Expand Up @@ -1087,7 +1094,9 @@ impl Parser {
let fully_qualified_name = name.fully_qualified_name(enclosing_namespace);

if self.parsed_schemas.get(&fully_qualified_name).is_some() {
return Ok(Schema::Ref { name });
return Ok(Schema::Ref {
name: fully_qualified_name,
});
}
if let Some(resolving_schema) = self.resolving_schemas.get(&fully_qualified_name) {
return Ok(resolving_schema.clone());
Expand Down Expand Up @@ -1403,11 +1412,12 @@ impl Parser {
}
}

let name = Name::parse(complex)?;
let name = Name::parse(complex, enclosing_namespace)?;
let aliases = fix_aliases_namespace(complex.aliases(), &name.namespace);

let mut lookup = BTreeMap::new();
let fully_qualified_name = name.fully_qualified_name(enclosing_namespace);
let fully_qualified_name = name.clone();

self.register_resolving_schema(&fully_qualified_name, &aliases);

let fields: Vec<RecordField> = fields_opt
Expand Down Expand Up @@ -1435,7 +1445,7 @@ impl Parser {
}

let schema = Schema::Record(RecordSchema {
name,
name: fully_qualified_name.clone(),
aliases: aliases.clone(),
doc: complex.doc(),
fields,
Expand Down Expand Up @@ -1478,8 +1488,8 @@ impl Parser {
}
}

let name = Name::parse(complex)?;
let fully_qualified_name = name.fully_qualified_name(enclosing_namespace);
let name = Name::parse(complex, enclosing_namespace)?;
let fully_qualified_name = name.clone();
let aliases = fix_aliases_namespace(complex.aliases(), &name.namespace);

let symbols: Vec<String> = symbols_opt
Expand Down Expand Up @@ -1518,7 +1528,7 @@ impl Parser {
}

let schema = Schema::Enum(EnumSchema {
name,
name: fully_qualified_name.clone(),
aliases: aliases.clone(),
doc: complex.doc(),
symbols,
Expand Down Expand Up @@ -1620,12 +1630,12 @@ impl Parser {
None => Err(Error::GetFixedSizeField),
}?;

let name = Name::parse(complex)?;
let fully_qualified_name = name.fully_qualified_name(enclosing_namespace);
let name = Name::parse(complex, enclosing_namespace)?;
let fully_qualified_name = name.clone();
let aliases = fix_aliases_namespace(complex.aliases(), &name.namespace);

let schema = Schema::Fixed(FixedSchema {
name,
name: fully_qualified_name.clone(),
aliases: aliases.clone(),
doc,
size: size as usize,
Expand Down Expand Up @@ -2626,7 +2636,7 @@ mod tests {

let schema = Schema::parse_str(schema)?;
let schema_str = schema.canonical_form();
let expected = r#"{"name":"office.User","type":"record","fields":[{"name":"details","type":[{"name":"Employee","type":"record","fields":[{"name":"gender","type":{"name":"Gender","type":"enum","symbols":["male","female"]}}]},{"name":"Manager","type":"record","fields":[{"name":"gender","type":"Gender"}]}]}]}"#;
let expected = r#"{"name":"office.User","type":"record","fields":[{"name":"details","type":[{"name":"office.Employee","type":"record","fields":[{"name":"gender","type":{"name":"office.Gender","type":"enum","symbols":["male","female"]}}]},{"name":"office.Manager","type":"record","fields":[{"name":"gender","type":"office.Gender"}]}]}]}"#;
assert_eq!(schema_str, expected);

Ok(())
Expand Down Expand Up @@ -2676,7 +2686,7 @@ mod tests {

let schema = Schema::parse_str(schema)?;
let schema_str = schema.canonical_form();
let expected = r#"{"name":"office.User","type":"record","fields":[{"name":"details","type":[{"name":"Employee","type":"record","fields":[{"name":"id","type":{"name":"EmployeeId","type":"fixed","size":16}}]},{"name":"Manager","type":"record","fields":[{"name":"id","type":"EmployeeId"}]}]}]}"#;
let expected = r#"{"name":"office.User","type":"record","fields":[{"name":"details","type":[{"name":"office.Employee","type":"record","fields":[{"name":"id","type":{"name":"office.EmployeeId","type":"fixed","size":16}}]},{"name":"office.Manager","type":"record","fields":[{"name":"id","type":"office.EmployeeId"}]}]}]}"#;
assert_eq!(schema_str, expected);

Ok(())
Expand Down Expand Up @@ -4865,4 +4875,155 @@ mod tests {

Ok(())
}

#[test]
fn test_avro_3818_inherit_enclosing_namespace() -> TestResult {
// Enclosing namespace is specified but inner namespaces are not.
let schema_str = r#"
{
"namespace": "my_ns",
"type": "record",
"name": "my_schema",
"fields": [
{
"name": "f1",
"type": {
"name": "enum1",
"type": "enum",
"symbols": ["a"]
}
}, {
"name": "f2",
"type": {
"name": "fixed1",
"type": "fixed",
"size": 1
}
}
]
}
"#;

let expected = r#"{"name":"my_ns.my_schema","type":"record","fields":[{"name":"f1","type":{"name":"my_ns.enum1","type":"enum","symbols":["a"]}},{"name":"f2","type":{"name":"my_ns.fixed1","type":"fixed","size":1}}]}"#;
let schema = Schema::parse_str(schema_str)?;
let canonical_form = schema.canonical_form();
assert_eq!(canonical_form, expected);

// Enclosing namespace and inner namespaces are specified
// but inner namespaces are ""
let schema_str = r#"
{
"namespace": "my_ns",
"type": "record",
"name": "my_schema",
"fields": [
{
"name": "f1",
"type": {
"name": "enum1",
"type": "enum",
"namespace": "",
"symbols": ["a"]
}
}, {
"name": "f2",
"type": {
"name": "fixed1",
"type": "fixed",
"namespace": "",
"size": 1
}
}
]
}
"#;

let expected = r#"{"name":"my_ns.my_schema","type":"record","fields":[{"name":"f1","type":{"name":"enum1","type":"enum","symbols":["a"]}},{"name":"f2","type":{"name":"fixed1","type":"fixed","size":1}}]}"#;
let schema = Schema::parse_str(schema_str)?;
let canonical_form = schema.canonical_form();
assert_eq!(canonical_form, expected);

// Enclosing namespace is "" and inner non-empty namespaces are specified.
let schema_str = r#"
{
"namespace": "",
"type": "record",
"name": "my_schema",
"fields": [
{
"name": "f1",
"type": {
"name": "enum1",
"type": "enum",
"namespace": "f1.ns",
"symbols": ["a"]
}
}, {
"name": "f2",
"type": {
"name": "f2.ns.fixed1",
"type": "fixed",
"size": 1
}
}
]
}
"#;

let expected = r#"{"name":"my_schema","type":"record","fields":[{"name":"f1","type":{"name":"f1.ns.enum1","type":"enum","symbols":["a"]}},{"name":"f2","type":{"name":"f2.ns.fixed1","type":"fixed","size":1}}]}"#;
let schema = Schema::parse_str(schema_str)?;
let canonical_form = schema.canonical_form();
assert_eq!(canonical_form, expected);

// Nested complex types with non-empty enclosing namespace.
let schema_str = r#"
{
"type": "record",
"name": "my_ns.my_schema",
"fields": [
{
"name": "f1",
"type": {
"name": "inner_record1",
"type": "record",
"fields": [
{
"name": "f1_1",
"type": {
"name": "enum1",
"type": "enum",
"symbols": ["a"]
}
}
]
}
}, {
"name": "f2",
"type": {
"name": "inner_record2",
"type": "record",
"namespace": "inner_ns",
"fields": [
{
"name": "f2_1",
"type": {
"name": "enum2",
"type": "enum",
"symbols": ["a"]
}
}
]
}
}
]
}
"#;

let expected = r#"{"name":"my_ns.my_schema","type":"record","fields":[{"name":"f1","type":{"name":"my_ns.inner_record1","type":"record","fields":[{"name":"f1_1","type":{"name":"my_ns.enum1","type":"enum","symbols":["a"]}}]}},{"name":"f2","type":{"name":"inner_ns.inner_record2","type":"record","fields":[{"name":"f2_1","type":{"name":"inner_ns.enum2","type":"enum","symbols":["a"]}}]}}]}"#;
let schema = Schema::parse_str(schema_str)?;
let canonical_form = schema.canonical_form();
assert_eq!(canonical_form, expected);

Ok(())
}
}

0 comments on commit dd7d8ee

Please sign in to comment.