dbus2 / zbus

Rust D-Bus crate.
Other
319 stars 70 forks source link

Sunset `DictSerializer` and `DictDeserializer` #746

Closed jonathanhood closed 1 week ago

jonathanhood commented 2 months ago

This umbrella issue is tracking a desire to replace DictSerializer and DictDeserializer with functionality available in a more generally applicable Serializer and Deserializer implementation that can handle these dict use cases as well. There are several issues that would be solved by such an implementation - and the intent is to track those issues here and create a "burn down" list of those items as well as record new items found/solved along the way.

When we are done, the Serialize and Deserialize implementations should properly serialize arbitrary structs into dictionaries as needed:

use serde::{Deserialize, Serialize};
use zvariant::Type;

#[derive(Serialize, Deserialize, Type)]
#[zvariant(signature = "a{ss}")]
struct SomeNestedData {
    the_insides: String
}

#[derive(Serialize, Deserialize, Type)]
#[zvariant(signature = "a{sv}")]
struct SomeData {
    a: i32,
    b: Option<String>,
    nested: Option<SomeNestedData>
}

Doing so should remove the long term need for seperate Dict* implementations of these items.

This should include support for:

Relevant Issues

Relevant Prior Art

This top comment will be updated over time as issues are found/resolved/etc and PRs are pushed through.

adamcavendish commented 2 months ago

Dict implementation is pretty useful if you implement a generic client which parses stuffs from cmdline i.e. in JSON format. I do upvote this arbitrary struct support, however I still suggest that we should consider keeping the dict-alike container.

zeenix commented 2 months ago
#[zvariant(signature = "a{ss}")]

I don't think we need this supported. a{sv} is the super common type (which can also cover a{ss} if the interface designer wants) and we should focus on that only.

however I still suggest that we should consider keeping the dict-alike container.

@adamcavendish could you elaborate on this part, please? I don't see any suggestion to remove anything except for the macros here. :thinking:

adamcavendish commented 2 months ago
#[zvariant(signature = "a{ss}")]

I don't think we need this supported. a{sv} is the super common type (which can also cover a{ss} if the interface designer wants) and we should focus on that only.

however I still suggest that we should consider keeping the dict-alike container.

@adamcavendish could you elaborate on this part, please? I don't see any suggestion to remove anything except for the macros here. 🤔

If the suggestions are about removing those macros, I have nothing to complain about. I was taking the sentence

Doing so should remove the long term need for seperate Dict* implementations of these items.

not only removing those macros but also remove the zvariant::Dict container.

To elaborate what I meant about the generic client, I previously proposed and maintained a safe D-Bus generic client based on the system's available D-Bus conf interfaces in my former company. Because of they used a read-only filesystem mechanism, the client is designed to use code generation to only generate allowed APIs to use to limit the attack surface.

The zvariant::Dict container plays an important role to hold the deeply nested JSON objects which doesn't have a structure name, i.e. taking "a{sa{ss}}" as an example.

zeenix commented 2 months ago

Doing so should remove the long term need for seperate Dict* implementations of these items.

not only removing those macros but also remove the zvariant::Dict container.

Thanks for explaining in detail. :) However, I seriously doubt Dict type was implied here by @jonathanhood.

jonathanhood commented 2 months ago

Doing so should remove the long term need for seperate Dict* implementations of these items.

not only removing those macros but also remove the zvariant::Dict container.

Thanks for explaining in detail. :) However, I seriously doubt Dict type was implied here by @jonathanhood.

Yes, sorry. I didn't realize my * was covering more things than I intended 🤣

jonathanhood commented 2 months ago
#[zvariant(signature = "a{ss}")]

I don't think we need this supported. a{sv} is the super common type (which can also cover a{ss} if the interface designer wants) and we should focus on that only.

The implied wire encoding of a{sv} and a{ss} are pretty different. I think for correctness you'll want to handle both of those cases. Regardless, I think doing both in going to fall out of the work is a rather straightforward way. At least, it has so far.

zeenix commented 1 week ago

@jonathanhood do you still intend to work on this and other issues you created? If not, please close them all.