Skip to content
Open
Show file tree
Hide file tree
Changes from all 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
7 changes: 3 additions & 4 deletions zbus/tests/iface_and_proxy/client.rs
Comment thread
z33ky marked this conversation as resolved.
Original file line number Diff line number Diff line change
Expand Up @@ -70,13 +70,12 @@ pub async fn my_iface_test(conn: Connection, event: Event) -> zbus::Result<u32>
.find(|i| i.name() == "org.freedesktop.MyIface")
.unwrap();
// Test if the r# prefix for the keyword was removed
assert!(my_iface.methods().iter().any(|m| m.name() == "Type"));
assert!(my_iface.properties().iter().any(|p| p.name() == "Let"));
assert!(my_iface.signals().iter().any(|s| s.name() == "Match"));
assert!(my_iface.methods().any(|m| m.name() == "Type"));
assert!(my_iface.properties().any(|p| p.name() == "Let"));
assert!(my_iface.signals().any(|s| s.name() == "Match"));
assert_eq!(
my_iface
.methods()
.iter()
.find(|m| m.name() == "RawIdentifierParameter")
.unwrap()
.args()
Expand Down
167 changes: 128 additions & 39 deletions zbus_xml/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -436,18 +436,8 @@ pub struct Interface<'a> {
#[serde(rename = "@name", borrow)]
name: InterfaceName<'a>,

#[serde(rename = "method", default)]
methods: Vec<Method<'a>>,
#[serde(rename = "property", default)]
properties: Vec<Property<'a>>,
#[serde(rename = "signal", default)]
signals: Vec<Signal<'a>>,
#[serde(rename = "annotation", default)]
annotations: Vec<Annotation>,
#[serde(skip)]
docstring: Option<String>,
#[serde(skip)]
telepathy_types: Vec<telepathy::TypeDef>,
#[serde(default)]
Comment on lines 438 to +439

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

🤖 ChatGPT GPT-6 Astra Pro on behalf of zeenix: [P2] Preserve the Serde mapping for interface members. The retained Deserialize implementation now recognizes a children field, not the existing <method>, <signal>, <property>, or <annotation> elements. Consequently, deserializing <node><interface name="org.example.Test"><method name="Ping"/></interface></node> with quick_xml::de::from_str::<Node<'_>> ignores method and defaults children to an empty vector instead of producing the Ping method.

Please map this collection to the XML child elements (e.g. $value with lowercase variant tag names, or an equivalent custom implementation) and add a direct Serde regression test. The existing test named serde only exercises Node::from_reader/to_writer, which now use the hand-written parser/writer and do not validate these derives.

children: Vec<Child<'a>>,
}

impl<'a> Interface<'a> {
Expand All @@ -456,24 +446,53 @@ impl<'a> Interface<'a> {
self.name.as_ref()
}

/// Returns the child elements.
Comment thread
z33ky marked this conversation as resolved.
pub fn children(&self) -> impl Iterator<Item = &Child<'a>> {
self.children.iter()
}

/// Returns the interface methods.
pub fn methods(&self) -> &[Method<'a>] {
&self.methods
pub fn methods(&self) -> impl Iterator<Item = &Method<'a>> {
self.children.iter().filter_map(|child| {
if let Child::Method(m) = child {
Some(m)
} else {
None
}
})
}

/// Returns the interface signals.
pub fn signals(&self) -> &[Signal<'a>] {
&self.signals
pub fn signals(&self) -> impl Iterator<Item = &Signal<'a>> {
self.children.iter().filter_map(|child| {
if let Child::Signal(s) = child {
Some(s)
} else {
None
}
})
}

/// Returns the interface properties.
pub fn properties(&self) -> &[Property<'_>] {
&self.properties
pub fn properties(&self) -> impl Iterator<Item = &Property<'a>> {
self.children.iter().filter_map(|child| {
if let Child::Property(p) = child {
Some(p)
} else {
None
}
})
}

/// Return the associated annotations.
pub fn annotations(&self) -> &[Annotation] {
&self.annotations
pub fn annotations(&self) -> impl Iterator<Item = &Annotation> {
self.children.iter().filter_map(|child| {
if let Child::Annotation(a) = child {
Some(a)
} else {
None
}
})
}

/// Return the content of the Telepathy `tp:docstring` extension element, if any.
Expand All @@ -482,40 +501,110 @@ impl<'a> Interface<'a> {
/// surrounding whitespace trimmed. Note that docstrings are only captured when parsing;
/// the writer does not emit them.
pub fn docstring(&self) -> Option<&str> {
self.docstring.as_deref()
self.children
.iter()
.filter_map(|child| {
if let Child::Docstring(s) = child {
Some(s.as_ref())
} else {
None
}
})
.next()
}

/// Return the Telepathy type definitions on this interface.
pub fn telepathy_types(&self) -> &[telepathy::TypeDef] {
&self.telepathy_types
pub fn telepathy_types(&self) -> impl Iterator<Item = &telepathy::TypeDef> {
self.children.iter().filter_map(|child| {
if let Child::TelepathyTypeDef(t) = child {
Some(t)
} else {
None
}
})
}

fn write_xml<W: Write>(&self, w: &mut W) -> std::io::Result<()> {
write!(w, "<interface name=\"{}\"", escape(self.name.as_str()))?;
if self.methods.is_empty()
&& self.properties.is_empty()
&& self.signals.is_empty()
&& self.annotations.is_empty()
{
if !self.children.iter().any(|child| {
matches!(
child,
Child::Method(_) | Child::Signal(_) | Child::Property(_) | Child::Annotation(_)
)
}) {
return write!(w, "/>");
}
write!(w, ">")?;
for method in &self.methods {
method.write_xml(w)?;
}
for property in &self.properties {
property.write_xml(w)?;
}
for signal in &self.signals {
signal.write_xml(w)?;
}
for annotation in &self.annotations {
annotation.write_xml(w)?;
for child in &self.children {
child.write_xml(w)?;
}
write!(w, "</interface>")
}
}

#[derive(Debug, Serialize, Deserialize, Clone, PartialEq)]
pub enum Child<'a> {
#[serde(borrow)]
Method(Method<'a>),
#[serde(borrow)]
Property(Property<'a>),
#[serde(borrow)]
Signal(Signal<'a>),
Annotation(Annotation),
Docstring(String),
#[serde(skip)]
TelepathyTypeDef(telepathy::TypeDef),
Comment on lines +549 to +551

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

These 2 are not really children so a bit strange to have them here. Besides we don't write neither of them so what's the usecase of having them here? Having them aslo means that our check of "if children present" becomes more inefficient than it would otherwise be.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

🤖 ChatGPT GPT-6 Astra Pro on behalf of zeenix: [P2] This also introduces a Serde serialization failure. #[serde(skip)] on an enum variant returns an error when that variant is serialized; it does not omit the item from the surrounding Vec. Consequently, parsing the existing telepathy_type_definitions test input and calling serde_json::to_string(&node) now fails at Child::TelepathyTypeDef. Previously, the skipped Interface::telepathy_types field was simply omitted.

Keeping these definitions outside the serializable children, as suggested above, avoids this regression; otherwise the serializer needs to filter them explicitly. Please cover this with a direct Serde test—Node::to_writer takes a separate path and will not catch it.

}

impl<'a> From<Method<'a>> for Child<'a> {
fn from(method: Method<'a>) -> Self {
Self::Method(method)
}
}

impl<'a> From<Property<'a>> for Child<'a> {
fn from(property: Property<'a>) -> Self {
Self::Property(property)
}
}

impl<'a> From<Signal<'a>> for Child<'a> {
fn from(signal: Signal<'a>) -> Self {
Self::Signal(signal)
}
}

impl<'a> From<Annotation> for Child<'a> {
fn from(annotation: Annotation) -> Self {
Self::Annotation(annotation)
}
}

impl<'a> From<String> for Child<'a> {
fn from(docstring: String) -> Self {
Self::Docstring(docstring)
}
}

impl<'a> From<telepathy::TypeDef> for Child<'a> {
fn from(type_def: telepathy::TypeDef) -> Self {
Self::TelepathyTypeDef(type_def)
}
}

impl<'a> Child<'a> {
fn write_xml<W: Write>(&'a self, w: &mut W) -> std::io::Result<()> {
match self {
Self::Method(m) => m.write_xml(w),
Self::Property(p) => p.write_xml(w),
Self::Signal(s) => s.write_xml(w),
Self::Annotation(a) => a.write_xml(w),
Self::Docstring(_) => Ok(()),
Self::TelepathyTypeDef(_) => Ok(()),
}
}
}

/// An introspection tree node (typically the root of the XML document).
#[derive(Debug, Deserialize, Serialize, Clone, PartialEq)]
pub struct Node<'a> {
Expand Down
28 changes: 10 additions & 18 deletions zbus_xml/src/xml.rs
Original file line number Diff line number Diff line change
Expand Up @@ -249,40 +249,37 @@ fn interface<'i>(
self_closing: bool,
) -> PResult<Interface<'static>> {
let name = attrs.name(|n| InterfaceName::try_from(n).map_err(Error::Zbus))?;
let mut methods = Vec::new();
let mut properties = Vec::new();
let mut signals = Vec::new();
let mut annotations = Vec::new();
let mut docstring = None;
let mut telepathy_types = Vec::new();
let mut interface_children = Vec::<crate::Child<'static>>::new();
children(
input,
tag,
self_closing,
|input, child, attrs, sc| match child {
"method" => {
methods.push(method(input, child, attrs, sc)?);
interface_children.push(method(input, child, attrs, sc)?.into());
Ok(true)
}
"property" => {
properties.push(property(input, child, attrs, sc)?);
interface_children.push(property(input, child, attrs, sc)?.into());
Ok(true)
}
"signal" => {
signals.push(signal(input, child, attrs, sc)?);
interface_children.push(signal(input, child, attrs, sc)?.into());
Ok(true)
}
"annotation" => {
annotations.push(annotation(input, child, attrs, sc)?);
interface_children.push(annotation(input, child, attrs, sc)?.into());
Ok(true)
}
other if is_docstring(other) => {
docstring = capture_docstring(input, other, sc)?.or(docstring.take());
if let Some(docstring) = capture_docstring(input, other, sc)? {
interface_children.push(docstring.into());
}
Ok(true)
}
other => {
if let Some(def) = telepathy_type_def(input, other, &attrs, sc)? {
telepathy_types.push(def);
interface_children.push(def.into());
}
Ok(true)
}
Expand All @@ -291,12 +288,7 @@ fn interface<'i>(

Ok(Interface {
name,
methods,
properties,
signals,
annotations,
docstring,
telepathy_types,
children: interface_children,

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

nit: can be simpler if we just name interface_children to children.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

🤖 ChatGPT GPT-6 Astra Pro on behalf of zeenix: Renaming the local also requires qualifying the children(...) parser-helper call immediately below as self::children(...); otherwise the new local shadows the function.

})
}

Expand Down
Loading
Loading