martin-g commented on code in PR #493:
URL: https://github.com/apache/avro-rs/pull/493#discussion_r2871029594
##########
avro/src/schema/name.rs:
##########
@@ -126,22 +149,39 @@ impl Name {
/// ```
/// # use apache_avro::{Error, schema::Name};
/// assert_eq!(
- ///
Name::new("some_name")?.fully_qualified_name(&Some("some_namespace".into())),
+ ///
Name::new("some_name")?.fully_qualified_name(Some("some_namespace")).into_owned(),
/// Name::new("some_namespace.some_name")?
/// );
/// assert_eq!(
- ///
Name::new("some_namespace.some_name")?.fully_qualified_name(&Some("other_namespace".into())),
+ ///
Name::new("some_namespace.some_name")?.fully_qualified_name(Some("other_namespace")).into_owned(),
/// Name::new("some_namespace.some_name")?
/// );
/// # Ok::<(), Error>(())
/// ```
- pub fn fully_qualified_name(&self, enclosing_namespace: &Namespace) ->
Name {
- Name {
- name: self.name.clone(),
- namespace: self
- .namespace
- .clone()
- .or_else(|| enclosing_namespace.clone().filter(|ns|
!ns.is_empty())),
+ pub fn fully_qualified_name(&self, enclosing_namespace: NamespaceRef) ->
Cow<'_, Name> {
+ if self.index_of_name == 0
+ && let Some(namespace) = enclosing_namespace
+ && !namespace.is_empty()
+ {
+ Cow::Owned(Self {
+ namespace_and_name: format!("{namespace}.{}",
self.namespace_and_name),
+ index_of_name: namespace.len() + 1,
+ })
+ } else {
+ Cow::Borrowed(self)
+ }
+ }
+
+ /// Create an empty name.
+ ///
+ /// This name is invalid and should never be used anywhere! The only valid
use is filling
+ /// a `Name` field that will not be used.
+ ///
+ /// Using this name will cause a panic.
+ pub(crate) fn invalid_empty_name() -> Self {
+ Self {
+ namespace_and_name: String::new(),
Review Comment:
```suggestion
namespace_and_name: String::from("invalid_empty_name"),
```
for easier debugging if a Name created with this helper method is ever used
as Debug/Display target
##########
avro/src/validator.rs:
##########
@@ -77,25 +86,26 @@ pub trait SchemaNameValidator: Send + Sync {
})
}
- /// Validates the schema name and returns the name and the optional
namespace.
+ /// Validates the schema name and returns the start byte of the name.
+ ///
+ /// Requires that the implementation of [`Self::regex`] provides a capture
group named `name`
+ /// that captures the name part of the full name.
///
/// Should return [`Details::InvalidSchemaName`] if it is invalid.
- fn validate(&self, schema_name: &str) -> AvroResult<(String, Namespace)>;
-}
-
-impl SchemaNameValidator for SpecificationValidator {
- fn validate(&self, schema_name: &str) -> AvroResult<(String, Namespace)> {
+ fn validate(&self, schema_name: &str) -> AvroResult<usize> {
let regex = SchemaNameValidator::regex(self);
let caps = regex
.captures(schema_name)
.ok_or_else(|| Details::InvalidSchemaName(schema_name.to_string(),
regex.as_str()))?;
- Ok((
- caps["name"].to_string(),
- caps.name("namespace").map(|s| s.as_str().to_string()),
- ))
+ Ok(caps
+ .name("name")
+ .expect("Regex has no group named `name`")
+ .start())
Review Comment:
return an error when there is no `name` group instead of panicking
```suggestion
caps.name("name")
.map(|m| m.start())
.ok_or_else(||
Details::InvalidSchemaName(schema_name.to_string(), regex.as_str()).into())
```
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
